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: default code offset = 3 #3454

Merged
merged 9 commits into from
Feb 21, 2024

Conversation

bout3fiddy
Copy link
Contributor

What I did

Set default code offset for blueprint contracts to 3

How I did it

Just change the default value from zero_value (which is IRnode.from_list(0, typ=UINT256_T)) to IRnode.from_list(3, typ=UINT256_T).

How to verify it

Added a test that simply uses no code_offset for initcode with the EIP5202 magic val appended at the top

Commit message

Sets default code offset to 3 (instead of 0).

Description for the changelog

Update: Sets default code offset to 3 (instead of 0).

Cute Animal Picture

image

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.

hmm i think this breaks some tests, also needs a docs update

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7bdebbf) 84.93% compared to head (bea6a63) 84.89%.
Report is 14 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3454      +/-   ##
==========================================
- Coverage   84.93%   84.89%   -0.05%     
==========================================
  Files          92       92              
  Lines       13685    13734      +49     
  Branches     3074     3078       +4     
==========================================
+ Hits        11623    11659      +36     
- Misses       1572     1586      +14     
+ Partials      490      489       -1     

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

@charles-cooper
Copy link
Member

alright i fixed the tests and updated the docs. since this is a breaking change (and we don't really want to break user code with 'patch' changes), going to hold off on this until the 0.4.0 series.

also i'm not a huge fan of the duplicated code in the new test case - it would be nice to think of a way around that.

@charles-cooper charles-cooper added this to the v0.4.0 milestone May 24, 2023
@charles-cooper charles-cooper mentioned this pull request Feb 10, 2024
33 tasks
@charles-cooper
Copy link
Member

i fixed the merge conflicts and updated the test so it passes. @pcaversaccio @fubuloubu want to take a look in case i missed anything?

docs/built-in-functions.rst Outdated Show resolved Hide resolved
tests/functional/builtins/codegen/test_create_functions.py Outdated Show resolved Hide resolved
@charles-cooper charles-cooper enabled auto-merge (squash) February 21, 2024 00:20
@charles-cooper charles-cooper merged commit 015cf81 into vyperlang:master Feb 21, 2024
84 checks passed
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.

5 participants