Cancun fixes + EIP 4788 #6009
Cancun fixes + EIP 4788 #6009
Conversation
- Added Eip4788 wiring to ChainSpecs
- Added Eip4788 stateful precompile
- make Instance field in Precompile static generic to match class type
src/Nethermind/Nethermind.Specs/ChainSpecStyle/ChainSpecLoader.cs
Outdated
Show resolved
Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/Data/IExecutionPayloadParams.cs
Outdated
Show resolved
Hide resolved
|
|
||
| state.RecalculateStateRoot(); | ||
| blockRequestV3.StateRoot = state.StateRoot; | ||
| TryCalculateHash(blockRequestV3, out Keccak? hash); |
There was a problem hiding this comment.
We recalculate it below CreateBlockRequestInternal<T> btw. What is interesting is that we do not execute transactions that may affect the state. Maybe state hash root was already calculated before( _beaconBlockRootHandler can be placed there if so)?
There was a problem hiding this comment.
I am not sure tbh, I'll have to take a look into it
- removed commented precompile line from VirtualMachine
- removed deprectated tests
| /// Parent Beacon Block precompile | ||
| /// </summary> | ||
| bool IsEip4788Enabled { get; } | ||
| Address Eip4788ContractAddress { get; } |
There was a problem hiding this comment.
we need test that this address is being loaded correctly from chainspec
There was a problem hiding this comment.
@Demuirgos :) please do it in next PR
This reverts commit 872bce8.
| public static int GetVersion(this PayloadAttributes executionPayload) => | ||
| executionPayload.Withdrawals is null ? 1 : 2; | ||
| executionPayload switch | ||
| { | ||
| { ParentBeaconBlockRoot: not null, Withdrawals: not null } => EngineApiVersions.Cancun, | ||
| { Withdrawals: not null } => EngineApiVersions.Shanghai, | ||
| _ => EngineApiVersions.Paris | ||
| }; | ||
|
|
||
| public static int ExpectedEngineSpecVersion(this IReleaseSpec spec) => | ||
| spec switch | ||
| { | ||
| { WithdrawalsEnabled: true, IsEip4844Enabled: true } => EngineApiVersions.Cancun, | ||
| { WithdrawalsEnabled: true } => EngineApiVersions.Shanghai, | ||
| _ => EngineApiVersions.Paris | ||
| }; |
There was a problem hiding this comment.
execution payload is treated based on a specific fork instead of what EIPs are enabled. This could result in conflicts with supporting other chains where, for example, gnosis would want to adopt 4788 but not 4844 (just an example). Ill post the same thing on the PR. but i dont want to block it. so ill probably post it as an issue to be looked at in a later PR.
designing a solution to this problem is not easy so i completely understand if it is not considered before cancun.
| @@ -17,6 +18,8 @@ protected Cancun() | |||
| IsEip5656Enabled = true; | |||
| IsEip4844Enabled = true; | |||
| IsEip6780Enabled = true; | |||
| IsEip4788Enabled = true; | |||
| Eip4788ContractAddress = Address.FromNumber(0x0b); | |||
There was a problem hiding this comment.
This might differ from one network to another (hopefully not)
Changes
PR for current Cancun branch and last minute fixes + EIP 4788
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?