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: Anvil Cancun support #7242

Merged
merged 61 commits into from
May 2, 2024
Merged

feat: Anvil Cancun support #7242

merged 61 commits into from
May 2, 2024

Conversation

Evalir
Copy link
Member

@Evalir Evalir commented Feb 26, 2024

Enables anvil dencun support by:

For handling EIP4844:

  • Adding the tx & block validation changes needed, as per the specs
  • Calculating & adding the header fields blob_gas_used / excess_blob_gas and tracking the blob gas price and excess blob gas in anvil's fee manager.
  • Do blob validation checks for EIP 4844 txs.
  • turning on c-kzg on revm, meaning the point evaluation precompile should also be available, both in anvil, and in forge.

For handling EIP-4788, locally the parent beacon block root is just set to B256::ZERO.

Merges into #7202

@Evalir Evalir mentioned this pull request Feb 26, 2024
10 tasks
@Evalir Evalir changed the title [wip] feat: Anvil 4844 support feat: Anvil 4844 support Feb 27, 2024
}

pub fn blob_gas(&self) -> Option<u64> {
match self {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tx.tx().tx() lol

Copy link
Member

Choose a reason for hiding this comment

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

not great but maybe we need to life with it cc @mattsse - this was already here

crates/anvil/core/src/eth/transaction/mod.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/mod.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/fees.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

last style nits
then pending @DaniPopes and @klkvr

crates/anvil/src/eth/backend/mem/mod.rs Outdated Show resolved Hide resolved
crates/anvil/src/eth/backend/mem/mod.rs Outdated Show resolved Hide resolved
@yash-atreya yash-atreya changed the title feat: Anvil Cancun support feat: [do not merge] Anvil Cancun support May 1, 2024
@yash-atreya
Copy link
Member

Certain error handling is pending. Do not merge

@yash-atreya yash-atreya changed the title feat: [do not merge] Anvil Cancun support feat: Anvil Cancun support May 2, 2024
@@ -129,7 +129,7 @@ pub fn transaction_request_to_typed(
}))
}
// EIP4844
(Some(3), None, _, _, _, Some(_), Some(_), Some(sidecar), Some(to)) => {
(Some(3), None, _, _, _, Some(_), Some(_), Some(sidecar), to) => {
Copy link
Member

Choose a reason for hiding this comment

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

we should refactor this to use alloy's TransactionRequest -> TypedTransaction conversion at some point

@@ -138,7 +138,7 @@ pub fn transaction_request_to_typed(
gas_limit: gas.unwrap_or_default(),
value: value.unwrap_or(U256::ZERO),
input: input.into_input().unwrap_or_default(),
to: match to {
to: match to.unwrap_or(TxKind::Create) {
Copy link
Member

@klkvr klkvr May 2, 2024

Choose a reason for hiding this comment

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

I believe this should only accept TxKind::To instead of setting address to zero, ref alloy-rs/alloy#355

crates/anvil/src/eth/api.rs Outdated Show resolved Hide resolved
Copy link
Member

@klkvr klkvr left a comment

Choose a reason for hiding this comment

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

lgtm

comments above are not critical, we should do a follow-up at some point which gets rid of transaction_request_to_typed in favor of alloy's conversion fn

@mattsse
Copy link
Member

mattsse commented May 2, 2024

@yash-atreya merge whenever :)

@yash-atreya yash-atreya merged commit 7a676f8 into master May 2, 2024
20 checks passed
@yash-atreya yash-atreya deleted the evalir/4844-exec-changes branch May 2, 2024 21:09
0xgregthedev pushed a commit to phylaxsystems/phoundry that referenced this pull request May 2, 2024
* feat(anvil-core): EIP4844 variant support

* chore: proper support when converting txs

* feat: add more type support

* chore: lock

* feat: missing type conversions, decoding test

* use correct eip check

* force no blob hashes for eip1559

* feat: support sidecar with 4844 types

* fmt

* feat: turn on c-kzg revm feature

* chore: add new invalid tx errors

* feat: execution validation steps

* feat: enable c-kzg

* feat: use main branch for consensus, update

* chore: rename

* lockfile

* fmt

* fmt

* fmt

* clippy

* feat: update blob fees

* set current blob excess gas and price when creating block

* blob gas checks

* clippy

* chore: remove unneeded fns

* chore: handle fee history

* chore: add excess blob gas and price to feehistory cache

* chore: remove unused

* chore: properly sum cumulative blob gas

* chore: rewrite validation checks

* chore: handle eip4844 variant when decoding

* max blob validation check

* chore: correct and rename blob fee capp err

* feat: fee history response changes

* docs

* several fee fixes

* chore: set blob gas used on rpc response

* fix: use primitives types

* fix: satisfy clippy

* feat(anvil/tests): can_send_eip4844_transaction - fails

* use sidecar builder in tests

* fix: tx_req_to_typed

* nits

* fix: return `blob_gas_price` and `blob_gas_used` in tx receipt

* nits

* fix: gas_price calc in backend::tx_build and nits

* feat(anvil-tests): `can_send_multiple_blobs_in_one_tx`, `cannot_exceed_six_blobs`

* nits

* fix: eip4844 test

* nits

Co-authored-by: DaniPopes <[email protected]>

* feat(anvil-test): 4844 - test should fail.

* fix(anvil): check MAX_BLOB_GAS_PER_BLOCK in tx executor

* nits

* fix: blob error handling

* nits

* type nits

* nit

---------

Co-authored-by: Yash Atreya <[email protected]>
Co-authored-by: DaniPopes <[email protected]>
@Thegaram
Copy link

Thegaram commented May 3, 2024

Looks like this can be marked as ready in this tracker? #5574

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants