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

fix(forge/script): deployment size check #3202

Merged
merged 6 commits into from
Sep 14, 2022

Conversation

DaniPopes
Copy link
Member

@DaniPopes DaniPopes commented Sep 14, 2022

Motivation

Deployment sizes check introduced in #2619 would not work for arbitrary bytecode deployed without an artifact and thus pass without errors.

Solution

Check against bytecodes from both artifacts and traces

Tested with, closes #2923:

Before: https://mumbai.polygonscan.com/tx/0xcf74fcecf15120bcf703598b4f47411f46210a5b6ffcef46a3e3919fd04f629b
(max codesize exceeded revert)

After:
image

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.

ty, test look good.

just need some clarification on the added check

cli/src/cmd/forge/script/mod.rs Outdated Show resolved Hide resolved
cli/src/cmd/forge/script/mod.rs Outdated Show resolved Hide resolved
@mattsse mattsse added the Cmd-forge-script Command: forge script label Sep 14, 2022
@DaniPopes DaniPopes requested a review from mattsse September 14, 2022 15:58
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.

I see, that makes sense

cli/src/cmd/forge/script/mod.rs Outdated Show resolved Hide resolved
@mattsse mattsse added T-feature Type: feature T-bug Type: bug labels Sep 14, 2022
@gakonst gakonst merged commit 359dd77 into foundry-rs:master Sep 14, 2022
iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
* fix(forge/script): deployment size check

* chore: clippy

* chore: fmt

* add comments

* fix: prompt user out of the loop

* fix: bail instead of panicking
@DaniPopes DaniPopes deleted the fix/script-sizes branch January 30, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-script Command: forge script T-bug Type: bug T-feature Type: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Max code size exceeded" still remains
3 participants