-
Notifications
You must be signed in to change notification settings - Fork 149
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
EIP-4895: Beacon chain push withdrawals as operations #2559
base: master
Are you sure you want to change the base?
Conversation
BlockchainTests/GeneralStateTests/Pyspecs/shanghai/eip4895_withdrawals/withdrawing_to_precompiles.json,src/GeneralStateTestsFiller/Pyspecs/shanghai/eip4895_withdrawals/test_withdrawals.py::test_withdrawing_to_precompiles[fork_Cancun-precompile_0x000000000000000000000000000000000000000a-blockchain_test-amount_0] | ||
BlockchainTests/GeneralStateTests/Pyspecs/shanghai/eip4895_withdrawals/withdrawing_to_precompiles.json,src/GeneralStateTestsFiller/Pyspecs/shanghai/eip4895_withdrawals/test_withdrawals.py::test_withdrawing_to_precompiles[fork_Cancun-precompile_0x000000000000000000000000000000000000000a-blockchain_test-amount_1] |
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.
These two remain skipped as the 0x0a
precompiled contract has not yet been added.
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.
I didn't review the whole PR in detail, but it largely looks fine and I did review the changes to block finalization in detail and they look correct.
You might consider using some kind of named constant to represent the conversion factor from GWEI to WEI though. It's not critical, but something to consider for readability.
Implementing withdrawal operations as specified in EIP-4895.
<withdrawals>
cell map subconfiguration to the<network>
that is used to keep track of withdrawals.Similar to how
<messages>
is implemented,<withdrawals>
has two additional Lists:<withdrawalsPending>
to keep track of the withdrawals that need to be applied.<withdrawalsOrder>
to preserve the order of the withdrawals during thecheck
rules after the execution has finished.EthereumCommands
productions:load "withdraw"
that handles the rlp decoding of the withdrawals array. For each item, the following two productions will be applied:mkWD
- that creates a new<withdrawal>
cell map item.loadWithdraw
- that initialises the newly created<withdrawal>
with values that are provided from the rlp payload.newWithdrawalID
- function that is used to generate a new index**..k
specs**
newWithdrawalID
shouldn't be necesary, as according to the specs:So the provided withdrawal
index
should be enough to keep track of the withdrawals.However, the conformance tests have multiple withdrawals with the
0
index and0
amount (example).