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

Verification Node utilizes separation of results and receipts in block Payload #546

Merged

Conversation

AlexHentschel
Copy link
Member

@AlexHentschel AlexHentschel commented Mar 18, 2021

implements https://github.com/dapperlabs/flow-go/issues/5397:

  • updated logic in the verifier's assigner.Engine to utilize separation of results from receipts in block Payload

there are also some utility logic included in this PR:

  • added grouping and Lookup table generation to ExecutionReceiptMeta and unit tests
  • added grouping and Lookup table generation to ExecutionResult and unit tests
  • updated Payload to use ExecutionReceiptMetaList and ExecutionResultList
  • replaced payload.ResultsById() with payload.Results.Lookup()

• added grouping for ExecutionReceiptMeta and unit tests;
• added grouping for ExecutionResult  and unit tests;
• updated Payload to use ExecutionReceiptMetaList and ExecutionResultList
• minor refactoring on verifier's assigner.Engine;
@AlexHentschel AlexHentschel self-assigned this Mar 18, 2021
Base automatically changed from yurii/4798-payload-receipts-optimization to master April 8, 2021 13:32
…results

# Conflicts:
#	engine/verification/assigner/engine.go
#	utils/unittest/fixtures.go
@AlexHentschel AlexHentschel changed the title updated logic in the Verifier Node to utilize separation of results from receipts in block Payload Verification Node utilizes separation of results and receipts in block Payload Apr 14, 2021
@AlexHentschel AlexHentschel marked this pull request as ready for review April 14, 2021 03:02
Copy link
Member

@durkmurder durkmurder left a comment

Choose a reason for hiding this comment

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

LGTM, left a few optional comments.

Copy link
Member

@zhangchiqing zhangchiqing left a comment

Choose a reason for hiding this comment

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

Looks good, a few minor suggestions

engine/verification/assigner/engine.go Outdated Show resolved Hide resolved
engine/verification/assigner/engine.go Outdated Show resolved Hide resolved
engine/verification/assigner/engine.go Show resolved Hide resolved
engine/verification/assigner/engine.go Outdated Show resolved Hide resolved
engine/verification/assigner/engine.go Outdated Show resolved Hide resolved
Alexander Hentschel and others added 4 commits April 15, 2021 20:28
@AlexHentschel AlexHentschel merged commit 1e8a225 into master Apr 19, 2021
@AlexHentschel AlexHentschel deleted the alex/5397_verifier_processes_incorporated_results branch April 19, 2021 21:56
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.

None yet

4 participants