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

Patch block RLP decoding to handle addition of PrevRandao field #472

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Aug 24, 2024

Description

The block structure has changed, and now includes a new field: PrevRandao.
Hence, we need to patch the RLP decoding for the blocks with the previous format, which lack the PrevRandao field. We also need to properly handle the block hash calculation, to not take into account this field, for past blocks.


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Summary by CodeRabbit

  • New Features

    • Enhanced error handling for block decoding processes.
    • Introduced a new ToBytes method for the Block structure to improve encoding capabilities.
    • Added support for an earlier block version with the blockV0 type.
  • Bug Fixes

    • Improved error management in block decoding to ensure robustness and clarity.
  • Tests

    • Added a new test to validate decoding for past block formats, enhancing overall test coverage.

@m-Peter m-Peter added this to the Flow-EVM-M2 milestone Aug 24, 2024
@m-Peter m-Peter self-assigned this Aug 24, 2024

This comment was marked as outdated.

type blockV0 struct {
Block *blockV0Fields
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For some strange reason, if we define the blockV0 type as:

type blockV0 struct {
	*blockV0Fields
	TransactionHashes []gethCommon.Hash
}

We get the following error:

rlp: input string too short for common.Hash, decoding into (models.blockV0).TransactionHashes[1]

when decoding the block.
That's why it's best if we have a named field for blockV0Fields.

coderabbitai[bot]

This comment was marked as outdated.

@m-Peter m-Peter force-pushed the patch-block-rlp-decoding branch from 8755fda to b26a3c5 Compare August 24, 2024 10:22
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8755fda and b26a3c5.

Files selected for processing (2)
  • models/block.go (4 hunks)
  • models/block_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • models/block.go
  • models/block_test.go

@sideninja sideninja merged commit 0a4b81a into main Aug 26, 2024
2 checks passed
@m-Peter m-Peter deleted the patch-block-rlp-decoding branch September 5, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants