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

feat[tool]: support storage layouts via json and .vyz inputs #4370

Merged
merged 44 commits into from
Jan 3, 2025

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented Nov 23, 2024

What I did

Resolves #4367

How I did it

  • For solc_json and archive outputs, emit the storage layout as output if it was overriden.
  • Extract a storage layout for solc_json and archive inputs if provided.

How to verify it

See tests.

Commit message

This commit adds support for overriding the storage layout using
`solc_json` and archive inputs, and consequently adding the storage
layout if it was provided to these formats as output. This makes it
possible for verifiers to verify code compiled with a storage layout
override with no changes on their end.

A design decision was made to have the storage layout override affect
the integrity hash. This is so you can tell that a contract was
compiled with storage layout override (even if it does not affect the
bytecode).

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

Copy link

codecov bot commented Nov 24, 2024

Codecov Report

Attention: Patch coverage is 95.83333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.67%. Comparing base (a29b49d) to head (d465568).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
vyper/compiler/output_bundle.py 90.00% 1 Missing ⚠️
vyper/compiler/phases.py 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4370      +/-   ##
==========================================
- Coverage   91.86%   90.67%   -1.19%     
==========================================
  Files         119      119              
  Lines       16640    16676      +36     
  Branches     2801     2807       +6     
==========================================
- Hits        15286    15121     -165     
- Misses        929     1083     +154     
- Partials      425      472      +47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tserg
Copy link
Collaborator Author

tserg commented Nov 24, 2024

@charles-cooper requesting a review before I update the docs

@tserg tserg marked this pull request as ready for review November 24, 2024 09:36
@tserg tserg requested a review from charles-cooper November 24, 2024 09:36
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

looks overall pretty good to me. left a few comments. @cyberthirst could you also take a look?

@tserg tserg requested a review from charles-cooper December 12, 2024 15:37
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

lgtm. tagging @cyberthirst for final review


for path, value in input_dict.get("storage_layout_overrides", {}).items():
if path not in input_dict["sources"]:
raise JSONError(f"unknown target for storage layout override: {path}")
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

@cyberthirst
Copy link
Collaborator

@tserg could you please add one more test for multiple compilation targets for each of which you'd provide an override file, where the target is either in the json/archive format?

@charles-cooper
Copy link
Member

charles-cooper commented Dec 17, 2024

i think we currently don't allow multiple compilation targets for archive bundles (

if len(compilation_targets) != 1:
raise BadArchive("Multiple compilation targets not supported!")
), so it can't be tested. for the json input bundle, yeah i think we should add the test

Comment on lines +107 to +110
"storage_layout_overrides": {
"contracts/foo.vy": FOO_STORAGE_LAYOUT_OVERRIDES,
"contracts/bar.vy": BAR_STORAGE_LAYOUT_OVERRIDES,
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a second storage layout override to this test. Is this sufficient?

@charles-cooper
Copy link
Member

@tserg looks like there's some merge conflict

@charles-cooper
Copy link
Member

@tserg looks like the latest merge from master broke some tests - could you take a look?

@charles-cooper charles-cooper changed the title feat[tool]: support storage layouts via json and vyz feat[tool]: support storage layouts via json and .vyz inputs Jan 3, 2025
@charles-cooper charles-cooper changed the title feat[tool]: support storage layouts via json and .vyz inputs feat[tool]: support storage layouts via json and .vyz inputs Jan 3, 2025
@charles-cooper charles-cooper merged commit d67e57c into vyperlang:master Jan 3, 2025
160 of 161 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release - must release blocker
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tool: allow storage layout overrides in standard-json and vyz files
3 participants