Pectra EIPs 6110, 7002, 7685 #7421
Conversation
…h/nethermind into systemTx_experiment
Co-authored-by: Amirul Ashraf <asdacap@gmail.com> Co-authored-by: Ben Adams <thundercat@illyriad.co.uk>
|
|
||
| [TestFixture] | ||
| [Parallelizable(ParallelScope.All)] | ||
| [Explicit("These tests are not ready yet")] |
There was a problem hiding this comment.
It will be updated after merging all Pectra's PRs: #7422
MarekM25
changed the title
MarekM25
changed the title
| // The below addresses are added for all forks, but the given EIPs can be enabled at a specific timestamp or block. | ||
| Eip7002ContractAddress = Eip7002Constants.WithdrawalRequestPredeployAddress; | ||
| DepositContractAddress = Eip6110Constants.MainnetDepositContractAddress; |
There was a problem hiding this comment.
Why not add them in Prague fork only?
There was a problem hiding this comment.
If you move them to Prague, the tests will fail. The addresses are not activated at any specific block/timestamp; they exist in the chainspec.
A potential solution would be moving them to a separate class of ForkIndependent properties.
cc: @rjnrohit
There was a problem hiding this comment.
Can you explain why tests would fail?
| WithdrawalRequest = 1 | ||
| } | ||
|
|
||
| public class ConsensusRequest |
There was a problem hiding this comment.
Maybe out of scope of this PR and we can do that in the next one: Shouldn't we create proper class hierarchy deserialization?
There was a problem hiding this comment.
There is a class hierarchy, not sure what exactly do you mean. But let's treat it out of scope of this PR :) You can discuss with @rjnrohit
There was a problem hiding this comment.
Not put every property in base class, but put them in correct derived classes:
https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism
There was a problem hiding this comment.
Even better: https://stackoverflow.com/a/59744873/1187056
|
Addressed most of the comments in #7458 |
Changes
https://eips.ethereum.org/EIPS/eip-6110
https://eips.ethereum.org/EIPS/eip-7002
https://eips.ethereum.org/EIPS/eip-7685
This PR contains Pectra EIPs without consolidations, 7702, and changes to payloadBodies.
Please note, the PR was initially reviewed by a few people but wasn't merged because of the wrong system call approach. There were hundreds of conflicts with master, so pay attention to it during the review.
I can't test Pectra features in this PR in isolation from other Pectra PRs. However, I decided to split the PRs for easier review. Of course, we can do regression testing with the current master.
Types of changes
What types of changes does your code introduce?
Testing
Requires testing
If yes, did you write tests?
Notes on testing
Optional. Remove if not applicable.
Documentation
Requires documentation update
If yes, link the PR to the docs update or the issue with the details labeled
docs. Remove if not applicable.Requires explanation in Release Notes
If yes, fill in the details here. Remove if not applicable.