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.

Sign up for GitHub

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

Jump to bottom

feat: implement EIP-2935 #1354

Merged
merged 9 commits into from
May 2, 2024
Merged

feat: implement EIP-2935 #1354

merged 9 commits into from
May 2, 2024

Conversation

onbjerg
Copy link
Collaborator

@onbjerg onbjerg commented Apr 22, 2024 •
edited

Implements the changes for BLOCKHASH as outlined in EIP-2935, and adds EIP-2935 constants.

This requires an external processing step to fill the contract's storage with data. See paradigmxyz/reth#7818

Additionally, since BLOCKHASH now does an SLOAD internally, I pulled the sload logic into a reusable sload! macro.

@onbjerg onbjerg added feature New feature or lib ability hf-prague Prague related EIPs labels Apr 22, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 22, 2024 •
edited

Valgrind Results:

==4092== Cachegrind, a cache and branch-prediction profiler
==4092== Copyright (C) 2002-2017, and GNU GPL'd, by Nicholas Nethercote et al.
==4092== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==4092== Command: target/release/snailtracer
==4092== 
--4092-- warning: L3 cache found, using its data for the LL simulation.
Running snailtracer example!
elapsed: 1.861004356s
==4092== 
==4092== I   refs:      438,899,950
==4092== I1  misses:          4,662
==4092== LLi misses:          2,846
==4092== I1  miss rate:        0.00%
==4092== LLi miss rate:        0.00%
==4092== 
==4092== D   refs:      207,994,002  (135,262,776 rd   + 72,731,226 wr)
==4092== D1  misses:        323,104  (    183,195 rd   +    139,909 wr)
==4092== LLd misses:        137,609  (      4,275 rd   +    133,334 wr)
==4092== D1  miss rate:         0.2% (        0.1%     +        0.2%  )
==4092== LLd miss rate:         0.1% (        0.0%     +        0.2%  )
==4092== 
==4092== LL refs:           327,766  (    187,857 rd   +    139,909 wr)
==4092== LL misses:         140,455  (      7,121 rd   +    133,334 wr)
==4092== LL miss rate:          0.0% (        0.0%     +        0.2%  )

@onbjerg onbjerg mentioned this pull request Apr 23, 2024
Merged
DaniPopes
DaniPopes approved these changes Apr 23, 2024
View reviewed changes
crates/interpreter/src/instructions/host.rs Outdated
interpreter.instruction_result = InstructionResult::FatalExternalError;
return;
};
gas!(interpreter, gas::sload_cost(SPEC::SPEC_ID, is_cold));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe extract the SLOAD logic from its function into a separate function where we also call here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i had this in mind too, i just forgor

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 tried making a func but mut borrow of interpreter from pop_top! prevented me, so I made an sload! macro instead

crates/primitives/src/constants.rs Outdated Show resolved Hide resolved
crates/interpreter/src/instructions/host.rs Outdated Show resolved Hide resolved
@onbjerg onbjerg requested a review from DaniPopes April 24, 2024 11:53
@onbjerg onbjerg marked this pull request as ready for review April 24, 2024 11:55
@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 26, 2024

Putting this in draft, unfortunately the current impl of EIP-2935 requires some changes to how we persist accounts and storage since it is exempt from state clearing. This will be changed in a later change to the EIP but the spec is currently frozen for devnet 0

@onbjerg onbjerg marked this pull request as draft April 26, 2024 12:35
@onbjerg
Copy link
Collaborator Author

onbjerg commented Apr 29, 2024 •
edited

After some consulting in Eth R&D discord we are likely going to add a nonce > 1 or a balance of 1 wei to the EIP-2935 system account until the EIP is fixed, so this is RFR again as there are no changes required to how we handle state clearing.

The balance hack has been implemented upstream in Reth for now

@onbjerg onbjerg marked this pull request as ready for review April 29, 2024 10:19
@rakita rakita mentioned this pull request Apr 29, 2024
Open
3 tasks
rakita
rakita approved these changes May 1, 2024
View reviewed changes
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm with one nit on naming

@onbjerg onbjerg requested a review from rakita May 1, 2024 22:50
@onbjerg
Copy link
Collaborator Author

onbjerg commented May 1, 2024

@rakita Renamed the constants, PTAL

@rakita rakita merged commit 3e089f3 into main May 2, 2024
25 checks passed
@github-actions github-actions bot mentioned this pull request May 2, 2024
Merged
@onbjerg onbjerg deleted the onbjerg/eip-2935 branch May 2, 2024 13:49
rakita added a commit that referenced this pull request May 16, 2024
@rakita
Revert "feat: implement EIP-2935 (#1354)"
dc9b67b 
This reverts commit 3e089f3.
rakita added a commit that referenced this pull request May 16, 2024
@rakita
Revert "feat: implement EIP-2935 (#1354)" (#1424)
f07c0ef 
* Revert "feat: implement EIP-2935 (#1354)"

This reverts commit 3e089f3.

* dont revert some commend changes

* Revert "fix(eip2935): Preload blockchash storage address (#1395)"

This reverts commit aeefcda.
@github-actions github-actions bot mentioned this pull request May 16, 2024
Closed
rakita added a commit that referenced this pull request May 16, 2024
@rakita
Revert "Revert "feat: implement EIP-2935 (#1354)" (#1424)"
1d19f20 
This reverts commit f07c0ef.
gakonst pushed a commit that referenced this pull request May 16, 2024
@rakita
Revert "Revert "feat: implement EIP-2935 (#1354)" (#1424)" (#1426)
eed27d9 
This reverts commit f07c0ef.
This was referenced May 17, 2024
Closed
Closed
This was referenced May 24, 2024
Closed
Closed
Closed
Closed
Closed
This was referenced Jun 3, 2024
Closed
Closed
Closed
This was referenced Jun 11, 2024
Closed
Closed
Closed
@github-actions github-actions bot mentioned this pull request Jun 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaniPopes DaniPopes DaniPopes approved these changes

@rakita rakita Awaiting requested review from rakita

Assignees
No one assigned
Labels
feature New feature or lib ability hf-prague Prague related EIPs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@onbjerg @rakita @DaniPopes