Skip to content

fix: gas cost calculation in eip-3860#159

Merged
sorpaas merged 3 commits intorust-ethereum:masterfrom
vimpunk:fix/eip-3860-initcode-gas-cost-calc
Apr 18, 2023
Merged

fix: gas cost calculation in eip-3860#159
sorpaas merged 3 commits intorust-ethereum:masterfrom
vimpunk:fix/eip-3860-initcode-gas-cost-calc

Conversation

@vimpunk
Copy link
Copy Markdown
Contributor

@vimpunk vimpunk commented Mar 20, 2023

The gas calculation for the initcode analysis introduced in EIP-3860 contained a bug in that it did not round up the code size to the nearest 32-byte word multiple.

The evm-tests repo has been updated with the most recent Ethereum tests for Shanghai: rust-blockchain/evm-tests#19.

FYI, there are more test failures for the Shanghai changes, this was just one of the bugs I could find. I'll send fixes for the other bugs as separate PRs.

@vimpunk
Copy link
Copy Markdown
Contributor Author

vimpunk commented Mar 30, 2023

The rust toolchain version was on 1.64 but one of the dependencies must have updated to a newer Rust version (with breaking if-let statements) without bumping the major version, I suspect, so that caused CI to fail here.

I bumped the Rust version to 1.68.2 in the rust-toolchain.toml file. Could you please re-run CI @sorpaas and merge if good?

@vimpunk
Copy link
Copy Markdown
Contributor Author

vimpunk commented Mar 31, 2023

There were still a bunch of unrelated clippy lints (assuming that came from the update). I fixed those now. Can we retry @sorpaas?

Comment thread core/src/utils.rs
assert_eq!(Wrapping(i8::MIN) / Wrapping(-1), Wrapping(i8::MIN));
assert_eq!(i8::MIN / 1, i8::MIN);
assert_eq!(i8::MAX / 1, i8::MAX);
assert_eq!(i8::MAX / -1, -i8::MAX);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not a 100% sure it's okay to remove these but clippy was complaining that they had no effect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm curious about the point of these lines 😄

@sorpaas sorpaas merged commit 0c75304 into rust-ethereum:master Apr 18, 2023
mattsse added a commit to mattsse/evm that referenced this pull request Jan 17, 2026
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