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

Sanity-check system deposit tx gas marginal wrt EIP-7623 #576

Open
emhane opened this issue Feb 4, 2025 · 5 comments
Open

Sanity-check system deposit tx gas marginal wrt EIP-7623 #576

emhane opened this issue Feb 4, 2025 · 5 comments
Labels
A-batcher Area: batcher A-execution Area: execution layer C-needs-benchmark Category: Set of changes needs performance benchmarking to double-check that they help H-isthmus Hardfork: change is planned for Isthmus upgrade H-pectra Hardfork: change planned for Pectra (L1) upgrade U-node Upgrade: involving changes to node component (cl, el, etc.)

Comments

@emhane
Copy link
Member

emhane commented Feb 4, 2025

we need to sanity-check something: with the increased calldata cost, what effect does this have on system deposit transactions gas usage? Specifically the L1BlockInfo? I think we have some margin in the gas-limit of these, but it may be good to sanity-check anyway.

Ref: ethereum-optimism/pm#25 (comment)

@emhane emhane added A-batcher Area: batcher A-execution Area: execution layer H-isthmus Hardfork: change is planned for Isthmus upgrade H-pectra Hardfork: change planned for Pectra (L1) upgrade U-node Upgrade: involving changes to node component (cl, el, etc.) C-needs-benchmark Category: Set of changes needs performance benchmarking to double-check that they help labels Feb 4, 2025
@elihaims
Copy link

elihaims commented Feb 7, 2025

discussed with @ajsutton in slack and don't think that this is an issue, but would catch it quickly in kurtosis/devnet if it was

Image

@sebastianst
Copy link
Member

From Slack Definitely a good callout. I see the max usage is 64k in @elihaims's statistics (can you post it here too?). The worst-case increased gas usage with EIP-7623 is by a factor of 10/4 (that's how non-zero calldata tokens are priced before/after, multiplied by 4). So even if those txs were calldata-only-txs, they would increase usage to 160k, still well below the limit. But in actuality, they aren't calldata-only txs, so their cost probably just stays the same. I think much more gas goes into storage writes etc.

But an end2end test/Kurtosis test will deliver the definitive proof.

@elihaims
Copy link

Image

@refcell refcell moved this from In Progress to TODO in Project Tracking Feb 11, 2025
@refcell
Copy link
Contributor

refcell commented Feb 11, 2025

cc @meyer9 in case you are working on an E2E test for this

@meyer9
Copy link
Contributor

meyer9 commented Feb 12, 2025

Not currently - my e2e test for network upgrade txs might cover this. It checks to make sure they were successfully executed, although not for older upgrade txs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-batcher Area: batcher A-execution Area: execution layer C-needs-benchmark Category: Set of changes needs performance benchmarking to double-check that they help H-isthmus Hardfork: change is planned for Isthmus upgrade H-pectra Hardfork: change planned for Pectra (L1) upgrade U-node Upgrade: involving changes to node component (cl, el, etc.)
Projects
None yet
Development

No branches or pull requests

5 participants