Skip to content

Conversation

@mablr
Copy link
Contributor

@mablr mablr commented Nov 22, 2025

Motivation

#12406 follow-up

Clean up the Anvil codebase by removing custom implementations that are no longer needed.

Solution

  • replace custom effective_gas_price impl w/ alloy's Transaction trait fn

PR Checklist

mattsse
mattsse previously approved these changes Nov 23, 2025
@mattsse
Copy link
Member

mattsse commented Nov 23, 2025

I think we need to update the failing transaction_receipt test

.saturating_add(t.tx().max_priority_fee_per_gas),
TypedTransaction::Deposit(_) => 0_u128,
};
let effective_gas_price = transaction.effective_gas_price(block.header.base_fee_per_gas);
Copy link
Contributor Author

@mablr mablr Nov 23, 2025

Choose a reason for hiding this comment

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

Imo using block.header.base_fee_per_gas as base fee arg is okay and non-breaking, despite self.base_fee() not being used anymore

Indeed after careful review of the blocks init/mining logic, I've not found any scenario where block.header.base_fee_per_gas could be None at this point. Which would mean that in the previous implementation we never had a fallback to the default value self.base_fee() (giving always base_fee_per_gas + max_priority_fee_per_gas for TXs >= eip1559).

So Alloy's impl would "more correct", I guess.

Let me know, if you think i'm wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If my observation is correct, we could improve robustness by making base_fee non-optional in block header ?

@mattsse mattsse added this pull request to the merge queue Nov 24, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 24, 2025
@mattsse mattsse added this pull request to the merge queue Nov 24, 2025
Merged via the queue into foundry-rs:master with commit 75151e9 Nov 24, 2025
15 checks passed
@github-project-automation github-project-automation bot moved this to Done in Foundry Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants