Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hyperblock processor #4

Merged
merged 16 commits into from
Mar 29, 2024
Merged

Hyperblock processor #4

merged 16 commits into from
Mar 29, 2024

Conversation

ssd04
Copy link

@ssd04 ssd04 commented Mar 14, 2024

Added hyper outport block processor components:

  • initial implementation for blocks pool handler
  • outport block data aggregator
  • firehose publisher

@ssd04 ssd04 self-assigned this Mar 14, 2024
@ssd04 ssd04 marked this pull request as ready for review March 19, 2024 11:19
return err
}

if outportBlock == nil || outportBlock.BlockData == nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can outportBlock ever be nil? thinking of removing the check since it might be redundant

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if marshalledData is nil, the marshaller will return nil on Unmarshall

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right. missed that, thanks for the clarification.

@ssd04 ssd04 changed the base branch from update-stdout-fields to feat/hyperblock March 25, 2024 09:25
go.mod Outdated
github.com/multiversx/mx-chain-communication-go v1.0.12
github.com/multiversx/mx-chain-core-go v1.2.18
github.com/multiversx/mx-chain-logger-go v1.0.13
github.com/multiversx/mx-chain-communication-go v1.0.13-0.20240126121117-627adccf10ad
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the changes here needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest storage version does not work with core 1.2.18, we might need to have a new storage version

Copy link
Author

@ssd04 ssd04 Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the meantime, we have storage v1.0.15 which works well with latest core version; updated storage version

)
if err != nil {
return nil, err
}

// TODO: move cache to config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will you move this to config in the next task?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this is already done on next PR

blocksPool BlocksPool,
dataAggregator DataAggregator,
) (DataProcessor, error) {
if publisher == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to include IsInterfaceNil in the interface and then check here with check.ifnil?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to use check.IfNil

}

// Close will close the internal writer
func (dp *dataProcessor) Close() error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

close will need to persist also block pool data.
can be done on the next PR.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is done on next PR 👍

func TestDataProcessor_ProcessPayload(t *testing.T) {
t.Parallel()

t.Run("nil outport block, should return error", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change description to nil outport block data... instead of nil outport block...?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

// notarized shards data
func (da *dataAggregator) ProcessHyperBlock(outportBlock *outport.OutportBlock) (*data.HyperOutportBlock, error) {
hyperOutportBlock := &data.HyperOutportBlock{}
hyperOutportBlock.MetaOutportBlock = outportBlock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe some check that the received outport block is indeed a metablock, e.g. check the ShardID?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added shard id check

@ssd04 ssd04 merged commit d480d5a into feat/hyperblock Mar 29, 2024
3 checks passed
@ssd04 ssd04 deleted the hyperblock-processor branch March 29, 2024 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants