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: drop istanbul and berlin support #3843

Merged
merged 9 commits into from
Mar 12, 2024

Conversation

charles-cooper
Copy link
Member

@charles-cooper charles-cooper commented Mar 7, 2024

What I did

per #3365,

  • drop istanbul support as it is 4 years old
  • drop berlin support since it will be 3 years old by the time of release

also:

  • change nonreentrant key values for cancun(!)

How I did it

How to verify it

Commit message

this commit drops explicit support for the istanbul and berlin hard
forks per the three year rule suggested in VIP 3365

- istanbul hard fork was 2019-12-09, over 4 years ago
- berlin hard fork was 2021-04-15, which should be 3 years ago by the
  time of 0.4.0 release (or if we can release sooner, shortly after)

this commit also changes the nonreentrant key values for cancun (since
`PUSH0` takes 1 less byte in the bytecode).

Description for the changelog

Cute Animal Picture

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

@charles-cooper charles-cooper marked this pull request as ready for review March 7, 2024 16:59
@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 86.12%. Comparing base (895b38a) to head (9197ee5).
Report is 8 commits behind head on master.

Files Patch % Lines
vyper/codegen/function_definitions/common.py 0.00% 1 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3843      +/-   ##
==========================================
+ Coverage   86.11%   86.12%   +0.01%     
==========================================
  Files          92       92              
  Lines       13915    13920       +5     
  Branches     3067     3066       -1     
==========================================
+ Hits        11983    11989       +6     
  Misses       1495     1495              
+ Partials      437      436       -1     

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

Copy link
Collaborator

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

Two general comments:

  • We should document the 3-year rule more prominently in the docs so we do our job to set expectations & we must add more prominently the currently supported EVM versions.
  • We put a disclaimer that the 3-year rule is not set in stone forever and can change over time due changing circumstances (one scenario is that there is no new EVM version for over 3 years, or there is a cascade of numerous EVM versions within 2 years that make it hard to maintain). Thus, we should state that it's subject to change.

@charles-cooper charles-cooper changed the title feat: drop istanbul support feat: drop istanbul and berlin support Mar 8, 2024
@charles-cooper
Copy link
Member Author

Two general comments:

  • We should document the 3-year rule more prominently in the docs so we do our job to set expectations & we must add more prominently the currently supported EVM versions.
  • We put a disclaimer that the 3-year rule is not set in stone forever and can change over time due changing circumstances (one scenario is that there is no new EVM version for over 3 years, or there is a cascade of numerous EVM versions within 2 years that make it hard to maintain). Thus, we should state that it's subject to change.

458c098#diff-69cdc09324658e05919bf7af42e6acf5a3d24c57e362497b89779838eb12a4e6R176

@cyberthirst
Copy link
Collaborator

Two general comments:

  • We should document the 3-year rule more prominently in the docs so we do our job to set expectations & we must add more prominently the currently supported EVM versions.
  • We put a disclaimer that the 3-year rule is not set in stone forever and can change over time due changing circumstances (one scenario is that there is no new EVM version for over 3 years, or there is a cascade of numerous EVM versions within 2 years that make it hard to maintain). Thus, we should state that it's subject to change.

Maybe rather than using absolute time metric, what about we rather focus on how the different L2s are doing wrt to supporting the latest EVM?

3 yrs is plenty of time to catch up - i think this window could be smaller and the maintenance overhead lowered

@pcaversaccio
Copy link
Collaborator

Maybe rather than using absolute time metric, what about we rather focus on how the different L2s are doing wrt to supporting the latest EVM?

3 yrs is plenty of time to catch up - i think this window could be smaller and the maintenance overhead lowered

I personally think it's all about expectation management and thus how we (= Vyper) set the expectations. People / Governance tend to think in time scales and not abstractions like EVM versions. New hard fork upgrades undergo governance (mostly DAO-based) approvals on L2s and thus, it will take time in any case (the PUSH0 case is great in that respect). Also, the largest consumers of the EVM are compilers, and dropping support for older versions can be interpreted as censoring through compilers. Cutting too early EVM versions due to fast iterations on the execution layer is also not really the way to go. Thus, we should find a good balance here to juggle these points.

@charles-cooper charles-cooper merged commit a9ee641 into vyperlang:master Mar 12, 2024
83 checks passed
@charles-cooper charles-cooper deleted the feat/drop-istanbul branch March 12, 2024 15:24
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