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

More deneb tests #3457

Merged
merged 3 commits into from
Jul 20, 2023
Merged

More deneb tests #3457

merged 3 commits into from
Jul 20, 2023

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Jul 18, 2023

  1. Add test cases of multiple blob txs in one block
  2. Add test cases of mixed blob txs and non-blob txs
  3. Enhance randomized block tests:
    1. Use build_randomized_execution_payload helper in random tests
    2. Randomize blob_count

@hwwhww hwwhww mentioned this pull request Jul 18, 2023
7 tasks
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

very nice! just a couple of nits

blob_kzg_commitments += commits

for _ in range(non_blob_tx_count):
txs.append(get_random_tx(rng=rng))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe should use rng to shuffle the txs list before applying to the el payload

@@ -259,7 +259,7 @@ def build_randomized_execution_payload(spec, state, rng):

num_transactions = rng.randint(0, 100)
execution_payload.transactions = [
spec.Transaction(get_random_bytes_list(rng, rng.randint(0, 1000)))
get_random_tx(rng)
Copy link
Contributor

Choose a reason for hiding this comment

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

This helper should probably condition on is_post_bellatrix (or whatever the appropriate helper is) to toss in some random blob TXs in Deneb and after

Copy link
Contributor Author

@hwwhww hwwhww Jul 19, 2023

Choose a reason for hiding this comment

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

hmm I thought about it, but it would need build_randomized_execution_payload to return blob_kzg_commitments, which is not well-compatible and clean with pre-deneb.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see

@djrtwo
Copy link
Contributor

djrtwo commented Jul 19, 2023

ready to merge when you are

@hwwhww hwwhww merged commit aca1202 into dev Jul 20, 2023
26 checks passed
@hwwhww hwwhww deleted the more-deneb-tests branch July 20, 2023 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants