Skip to content

evm: add eip-7907 to osaka (wip) #4052

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jgresham
Copy link

@jgresham jgresham commented May 2, 2025

  • Add new max contract size and max initcode size variables
  • Add 7907 to Osaka Hardfork
  • Test new limit of max initcode size
  • Test new limit of max contract size
  • Implement additional gas fees for size > previous max sizes
  • Set conditional gas fee on account warm and/or not code warm
  • Test additional gas fees
  • See if it is required to fix a few eip 3860 tests converted to 7907 and are failing

#4051

@jgresham
Copy link
Author

jgresham commented May 2, 2025

Questions:

  1. Are params like this.common.param('maxInitCodeSize') and this.common.param('maxCodeSize') "updated" when an eip becomes activated after a hardfork? example here and variables are set in eip-170 and 3860 and now both in 7907 here

@jochem-brouwer
Copy link
Member

Hi there! Yes, when common.param is read, the current active param is being read. So, if these params update (because of a fork) then the VM/EVM/(Tx) will handle this.

Great work so far, I would however wait until the EIP "stabilizes" a bit, because some things are underspecified or might change (see discussion: https://ethereum-magicians.org/t/eip-7907-meter-contract-code-size-and-increase-limit/23156)

Copy link

codecov bot commented May 6, 2025

Codecov Report

Attention: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.

Project coverage is 79.38%. Comparing base (e300b10) to head (256b107).
Report is 5 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 84.33% <ø> (ø)
blockchain 89.32% <ø> (ø)
client 67.49% <ø> (?)
common 97.51% <100.00%> (+<0.01%) ⬆️
devp2p 86.78% <ø> (ø)
evm 73.11% <83.33%> (+<0.01%) ⬆️
mpt 89.74% <ø> (ø)
statemanager 69.06% <ø> (ø)
static 99.11% <ø> (ø)
tx 89.91% <100.00%> (+0.01%) ⬆️
util 89.20% <100.00%> (+<0.01%) ⬆️
vm 55.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@holgerd77
Copy link
Member

To add on comments from @jochem-brouwer: at the end fully your decision of course if you want to continue based on the current EIP state. Just to manage expectations a bit, we do not merge these kind of EIP integrations until a certain level of stability/certainty about inclusion state is reached.

So might be that this this PR will need to remain open/unmerged for a couple of weeks or (eventually) months.

@jgresham
Copy link
Author

jgresham commented May 6, 2025

To add on comments from @jochem-brouwer: at the end fully your decision of course if you want to continue based on the current EIP state. Just to manage expectations a bit, we do not merge these kind of EIP integrations until a certain level of stability/certainty about inclusion state is reached.

So might be that this this PR will need to remain open/unmerged for a couple of weeks or (eventually) months.

Understood. Thanks for the info. I was thinking to leave it open until fully implemented and use the PR diff as a comparison point with other clients. I'm hoping to get more knowledgeable and involved on the remaining spec for the EIP too.

@jgresham
Copy link
Author

jgresham commented May 20, 2025

Is there a way to see if code is "loaded" in EthereumJS? There is a distinction being discussed in the EIP between gas cost for warm account and loaded code and warm account and not loaded code

"Access Lists: account will be warm if added to access list but no code is pre-loaded just the account"

@jochem-brouwer
Copy link
Member

Yes, use runState.interpreter.journal.isWarmedAddress(address) which returns a boolean. In gas.ts accessAddressEIP2929 is called to get the gas costs regarding warm/cold accounts. Ensure you check if the address is warm before the accessAddressEIP2929 call, because this will mark the address as warm if it is cold, and then incur the extra costs (note: I think the EIP will change regarding this)

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.

3 participants