Skip to content

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

Merged
charles-cooper merged 44 commits intovyperlang:masterfrom
tserg:tool/storage_layout_json
Jan 3, 2025
Merged

feat[tool]: support storage layouts via json and .vyz inputs#4370
charles-cooper merged 44 commits intovyperlang:masterfrom
tserg:tool/storage_layout_json

Conversation

@tserg
Copy link
Copy Markdown
Contributor

@tserg tserg commented Nov 23, 2024 •
edited by charles-cooper
Loading

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-->

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 24, 2024 •
edited
Loading

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 67 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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tserg
Copy link
Copy Markdown
Contributor 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
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Nov 24, 2024
View reviewed changes
Copy link
Copy Markdown
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?

charles-cooper
charles-cooper reviewed Nov 25, 2024
View reviewed changes
@tserg tserg requested a review from charles-cooper December 12, 2024 15:37
charles-cooper
charles-cooper approved these changes Dec 12, 2024
View reviewed changes
Copy link
Copy Markdown
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

charles-cooper
charles-cooper reviewed Dec 13, 2024
View reviewed changes
vyper/cli/vyper_json.py

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
Copy Markdown
Member

@charles-cooper charles-cooper Dec 13, 2024

Choose a reason for hiding this comment

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

no coverage for this line https://github.com/vyperlang/vyper/pull/4370/checks?check_run_id=34325903794

charles-cooper
charles-cooper reviewed Dec 13, 2024
View reviewed changes
charles-cooper
charles-cooper reviewed Dec 13, 2024
View reviewed changes
Copy link
Copy Markdown
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.

https://github.com/vyperlang/vyper/pull/4370/files#r1882232779

and a couple lines are missing coverage

@cyberthirst
Copy link
Copy Markdown
Collaborator

cyberthirst commented Dec 17, 2024

@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
Copy Markdown
Member

charles-cooper commented Dec 17, 2024 •
edited
Loading

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

vyper/vyper/cli/compile_archive.py

Lines 48 to 49 in ebe26a6

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

tserg
tserg commented Dec 18, 2024
View reviewed changes
tests/unit/cli/vyper_json/test_compile_json.py
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
Copy Markdown
Contributor Author

@tserg tserg Dec 18, 2024

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
Copy Markdown
Member

charles-cooper commented Dec 30, 2024

@tserg looks like there's some merge conflict

@charles-cooper
Copy link
Copy Markdown
Member

charles-cooper commented Jan 2, 2025

@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
This was referenced Mar 14, 2025
Closed
Open
@kuzdogan kuzdogan mentioned this pull request Mar 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charles-cooper charles-cooper charles-cooper approved these changes

@cyberthirst cyberthirst cyberthirst approved these changes

Assignees

No one assigned

Labels

release - must release blocker

Projects

None yet

Milestone

v0.4.1

Development

Successfully merging this pull request may close these issues.

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

3 participants

@tserg @charles-cooper @cyberthirst