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

refactor: random_block(s) function #10499

Closed
wants to merge 5 commits into from

Conversation

jenpaff
Copy link
Collaborator

@jenpaff jenpaff commented Aug 24, 2024

Closes #10457

Implementation Details:

  • created new struct BlockParams used to pass arguments for random block generation
  • refactored both random_block (generates one block) + random_block_range functions to use new struct (could've been done separately, but decided to combine both since they were so similar)

@github-actions github-actions bot added C-debt Refactor of code section that is hard to understand or maintain C-test A change that impacts how or what we test labels Aug 24, 2024
@jenpaff jenpaff changed the title refactor: random_blocks function refactor: random_block function Aug 24, 2024
@jenpaff jenpaff force-pushed the jenpaff/refactor-random_blocks-function branch from 92c82c5 to 33110dc Compare August 24, 2024 15:28
@jenpaff jenpaff changed the title refactor: random_block function refactor: random_block(s) function Aug 26, 2024
@jenpaff jenpaff force-pushed the jenpaff/refactor-random_blocks-function branch 2 times, most recently from 7d44bb4 to 7ba1c18 Compare August 26, 2024 12:31
@jenpaff jenpaff force-pushed the jenpaff/refactor-random_blocks-function branch from 7ba1c18 to 2ff861c Compare August 26, 2024 16:54
let blocks = random_block_range(
&mut rng,
1..=10,
BlockParams { parent: Some(B256::ZERO), tx_count: Some(0..2), ..Default::default() },
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm why is this change from 2..3 to 0..2 required?

Copy link
Collaborator Author

@jenpaff jenpaff Aug 26, 2024

Choose a reason for hiding this comment

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

This is incorrect, but i think the test is rightfully failing- will fix in a bit, just debugging 2 other tests

@@ -2348,8 +2354,7 @@ mod tests {
&mut rng,
TEST_BLOCKS_COUNT,
TEST_BLOCKS_COUNT,
None,
None,
BlockParams::default(),
Copy link
Collaborator Author

@jenpaff jenpaff Aug 26, 2024

Choose a reason for hiding this comment

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

@shekhirin seems like the test_receipt_provider_* tests are failing on here. it seems to me like they are passing on main cos both lists are empty, debugging..

@jenpaff
Copy link
Collaborator Author

jenpaff commented Aug 27, 2024

superseded by #10563

@jenpaff jenpaff closed this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-debt Refactor of code section that is hard to understand or maintain C-test A change that impacts how or what we test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor random_blocks function
2 participants