feat: Eip 6110 #8204
feat: Eip 6110 #8204
Conversation
gakonst
force-pushed
the
eip-6110
branch
2 times, most recently
from
99b4802
to
e4f3932
Compare
gakonst
requested review from
mattsse,
joshieDo,
shekhirin,
onbjerg,
Rjected and
emhane
as code owners
| /// Error when decoding deposit requests from receipts [EIP-6110] | ||
| /// | ||
| /// [EIP-6110]: https://eips.ethereum.org/EIPS/eip-6110 | ||
| // TODO(gakonst): This is an RLP decoding error but we don't import alloy_rlp here, |
There was a problem hiding this comment.
why not? It should be in reth_primitives already
crates/ethereum/evm/src/execute.rs
Outdated
| } | ||
|
|
||
| // Collect all EIP-6110 deposits | ||
| let deposits = parse_deposits_from_receipts(&receipts)?; | ||
|
|
||
| // Collect all EIP-7685 requests | ||
| let withdrawal_requests = | ||
| post_block_withdrawal_requests(&self.chain_spec, block.timestamp, &mut evm)?; |
There was a problem hiding this comment.
Should accumulate, extending a vector, rather than two vecs and concatenating after. Can be done with &mut Vec as argument and transforming the iterator into a for loop
There was a problem hiding this comment.
The withdrawal function returns a Vec<Request> via Vec<Request>::decode and I cannot break out of the iterator without collecting so not sure how to do that here, also prob not big perf given smol list
There was a problem hiding this comment.
i see, what about moving the parse_deposits to after post_block and extending that list?
onbjerg
added
C-enhancement
| } | ||
|
|
||
| fn parse_deposit_from_log(log: &Log<DepositEvent>) -> DepositRequest { | ||
| // SAFETY: These `expect` https://github.com/ethereum/consensus-specs/blob/5f48840f4d768bf0e0a8156a3ed06ec333589007/solidity_deposit_contract/deposit_contract.sol#L107-L110 |
There was a problem hiding this comment.
nit: should be NOTE or something else, SAFETY should be reserved for unsafe {}
There was a problem hiding this comment.
# Panics in the doc of the function actually
| @@ -158,11 +158,11 @@ impl BundleStateWithReceipts { | |||
| /// Transform block number to the index of block. | |||
| fn block_number_to_index(&self, block_number: BlockNumber) -> Option<usize> { | |||
| if self.first_block > block_number { | |||
| return None | |||
| return None; | |||
crates/ethereum/evm/src/eip6110.rs
Outdated
| use reth_primitives::Receipt; | ||
| use revm_primitives::Log; | ||
|
|
||
| pub(crate) fn parse_deposits_from_receipts( |
| sol! { | ||
| #[allow(missing_docs)] | ||
| event DepositEvent( | ||
| bytes pubkey, | ||
| bytes withdrawal_credentials, | ||
| bytes amount, | ||
| bytes signature, | ||
| bytes index | ||
| ); | ||
| } | ||
|
|
||
| fn parse_deposit_from_log(log: &Log<DepositEvent>) -> DepositRequest { |
There was a problem hiding this comment.
maybe we should put the DepositEvent in alloy-eips for the relevant eip, and have a conversion from DepositEvent → DepositRequest in alloy
There was a problem hiding this comment.
agreed, let's do it after
crates/ethereum/evm/src/execute.rs
Outdated
| // TODO: Does order matter? | ||
| let requests = [deposits, withdrawal_requests].concat(); |
There was a problem hiding this comment.
yes and this is the correct order
Co-authored-by: Alexey Shekhirin <a.shekhirin@gmail.com>
| @@ -311,7 +315,7 @@ where | |||
| receipts.iter(), | |||
| ) { | |||
| debug!(target: "evm", %error, ?receipts, "receipts verification failed"); | |||
| return Err(error) | |||
| return Err(error); | |||
There was a problem hiding this comment.
hehe
| return Err(error); | |
| return Err(error) |
Adds EIP-6110, blocked on #8053