Implement and add changes for EIPs 7002, 6110, 7685 #10533
Conversation
|
P.S. There would be subsequent PRs/Changes for accommodating other protocol changes around these EIPs, this one adds type and execution support |
| @@ -65,6 +65,7 @@ type Genesis struct { | |||
| BlobGasUsed *uint64 `json:"blobGasUsed"` // EIP-4844 | |||
| ExcessBlobGas *uint64 `json:"excessBlobGas"` // EIP-4844 | |||
| ParentBeaconBlockRoot *common.Hash `json:"parentBeaconBlockRoot"` // EIP-4788 | |||
| RequestsRoot *common.Hash `json:"requestsRoot"` // EIP-7685 | |||
There was a problem hiding this comment.
Do we actually need to support non-default (nil pre-Pectra, empty post-Pectra) RequestsRoot in genesis?
There was a problem hiding this comment.
The genesis block could be Pectra
There was a problem hiding this comment.
Yeah, but GenesisToBlock already correctly sets requests to empty (but non-nil) for Pectra. head.RequestsRoot will be set accordingly by NewBlock at the end of GenesisToBlock.
There was a problem hiding this comment.
So, this has been kept at the moment as support for the type, as the spec tests have requestsRoot in genesis header. Could be removed in a later PR
| return nil, nil, nil, fmt.Errorf("error: invalid requests root hash in header, expected: %v, got :%v", header.RequestsRoot, rh) | ||
| } | ||
| sds := requests.Deposits() | ||
| if !reflect.DeepEqual(sds, ds.Deposits()) { |
There was a problem hiding this comment.
We probably need a similar check for withdrawal requests - see https://discord.com/channels/595666850260713488/892088344438255616/1247841586147229696
There was a problem hiding this comment.
Withdrawal request validations are done at the smart contract itself, so dequeue would get out the exact set of withdrawals.
Deposits are obtained from logs, which happens outside of EVM and in the above lines, and must be checked.
However, this check is already embedded with checking the requestsRoot as there is no scenario in which a different set of deposits passed in would lead to the same requestsRoot.
There was a problem hiding this comment.
I still don't understand why there should be a difference between deposit and withdrawal requests in terms of checks. At the end of a block expected deposit requests are calculated by ParseDepositLogs(logs), while withdrawal requests are calculated by DequeueWithdrawalRequests7002(syscall). If we rely solely on the *header.RequestsRoot != rh check for withdrawal requests, then we should do the same for deposit ones and remove the !reflect.DeepEqual(sds, ds.Deposits()) check.
There was a problem hiding this comment.
Yes, I will add the DeepEqual check for withdrawal_requests in a later PR - under the assumption that there can be other request types in future.
Alternatively, until there is another type of request introduced, we can remove this check altogether and rely on block.HashCheck() combined with the root check above
Cherry pick #10533 --------- Co-authored-by: Somnath <[email protected]>
Cherry pick #10533 --------- Co-authored-by: Somnath <[email protected]>
Context
The three EIPs are inter-linked through EIP-7685. Withdrawal requests (EIP-7002) are to be dequeued from execution through system call, and deposit requests (EIP-6110) are to be extracted from logs; both updated into
Requestslist in the block and custom RLP-encodedSummary of changes
Requestan interface and can be eitherDepositorWithdrawalRequestsis an array of requests with custom encoding rulesFinalizeinmerge.go- the only consensus for supporting thesegencodec