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

feat(evm): EIP-7002 withdrawal requests #8212

Merged
merged 23 commits into from
May 13, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented May 13, 2024

Implements the EIP-7002 post-block system contract call. Tests are pending, will be added in the next PR.

@shekhirin shekhirin force-pushed the alexey/eip-7002-withdrawal-requests branch from 198370e to 602bd89 Compare May 13, 2024 06:37
@shekhirin shekhirin force-pushed the alexey/eip-7002-withdrawal-requests branch from 48f448f to dbfdbc8 Compare May 13, 2024 07:54
@shekhirin shekhirin changed the base branch from main to devnet-0 May 13, 2024 08:01
///
/// Returns an error if execution fails or receipt verification fails.
fn execute_and_verify(
&mut self,
block: &BlockWithSenders,
total_difficulty: U256,
) -> Result<(Vec<Receipt>, u64), BlockExecutionError> {
) -> Result<(Vec<Receipt>, Vec<Request>, u64), BlockExecutionError> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we add a type for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added

Comment on lines +84 to +85
/// All the EIP-7685 requests of the transactions in the block.
pub requests: Vec<Request>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is fine for now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how would you improve it?

Self { bundle, receipts, first_block }
}
}

// TODO(mattsse): unify the types, currently there's a cyclic dependency between
#[cfg(any(test, feature = "test-utils"))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

we still need this I believe

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah but it doesn't include the requests in the bundle state now, so for now I gated it behind the cfg and added a todo

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

good start

crates/ethereum/evm/src/execute.rs Outdated Show resolved Hide resolved
Comment on lines 200 to 208
Ok((receipts, cumulative_gas_used))
let withdrawal_requests =
post_block_withdrawal_requests(&self.chain_spec, block.timestamp, evm)?;

Ok((receipts, cumulative_gas_used, withdrawal_requests))
Copy link
Member

Choose a reason for hiding this comment

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

TODO: we should convert this to a struct later

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

converted now

crates/ethereum/evm/src/execute.rs Outdated Show resolved Hide resolved
crates/primitives/src/revm/env.rs Show resolved Hide resolved
/// If Prague is not active at the given timestamp, then this is a no-op, and an empty vector is
/// returned. Otherwise, the withdrawal requests are returned.
#[inline]
pub fn post_block_withdrawal_requests<EXT, DB: Database + DatabaseCommit>(
Copy link
Member

Choose a reason for hiding this comment

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

needs unit test but OK with moving fast

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gonna add it after we merge this PR

@shekhirin shekhirin force-pushed the alexey/eip-7002-withdrawal-requests branch from 83139dd to 0d9deb9 Compare May 13, 2024 08:52
@shekhirin shekhirin force-pushed the alexey/eip-7002-withdrawal-requests branch from 0d9deb9 to f50b10c Compare May 13, 2024 08:55
@shekhirin shekhirin marked this pull request as ready for review May 13, 2024 09:09
@shekhirin shekhirin added C-enhancement New feature or request A-execution Related to the Execution and EVM E-prague Related to the prague network upgrade labels May 13, 2024
@gakonst gakonst mentioned this pull request May 13, 2024
10 tasks
Comment on lines +110 to +112
/// Helper type for the output of executing a block.
#[derive(Debug, Clone)]
struct EthExecuteOutput {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I will combine this with the other output types separately

@shekhirin shekhirin merged commit faf609b into devnet-0 May 13, 2024
22 of 23 checks passed
@shekhirin shekhirin deleted the alexey/eip-7002-withdrawal-requests branch May 13, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-execution Related to the Execution and EVM C-enhancement New feature or request E-prague Related to the prague network upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants