-
Notifications
You must be signed in to change notification settings - Fork 4
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
Withdrawals #30
Withdrawals #30
Conversation
@@ -0,0 +1,483 @@ | |||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding separate bash scripts for every test scales pretty bad. The scripts should result in producing a sequence of blocks from Nethermind with the engine API and then apply those blocks on Reth.
So you can have multiple generators, each generates a directory of block JSONs blocks_test_withdrawals_eip4895
, then we have a script that you pass a blocks_
directory as argument and runs the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a better idea, i will close both PRs and do this instead
let deposit_requests = parse_deposits_from_receipts(&chain_spec, receipts.iter().flatten()) | ||
.map_err(|err| PayloadBuilderError::Internal(RethError::Execution(err.into())))?; | ||
|
||
println!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't merge this log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, removing
Closing in favor of:
|
adds test for withdrawals:
For reth, we first deploy the deposit contract address