execution: update EIP-8282 contracts for glamsterdam-devnet-7 - #22526
Conversation
|
same PR which updates contracts parms #22490 |
Yes, I'm aware. Unfortunately, as I have mentioned before, I can't chain PRs in sequence when some of them involve external forks. That's usually how I try to sequence the work so it is easier to review and merge to |
There was a problem hiding this comment.
Pull request overview
Updates Erigon’s execution-layer EIP-8282 builder contract artifacts (addresses + runtime bytecode) to match glamsterdam-devnet-7 / v7.2.0 expectations, and aligns local/spec test harnesses and CI tolerances accordingly.
Changes:
- Updated EIP-8282 builder deposit/exit predeploy addresses and runtime bytecode, plus added hash-based test assertions for the artifacts.
- Ensured the developer genesis alloc embeds the updated builder contracts and added a chainspec test to enforce it.
- Adjusted state test transaction nonce handling to support wider JSON nonce values, and tuned EEST/Hive failure thresholds for devnet shards.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/eest-spec-shards.yml | Updates allowed-failure thresholds for devnet-related EEST shards. |
| execution/tests/testutil/state_test_util.go | Switches state-test tx nonce to 256-bit parsing and adds uint64-range validation before message creation. |
| execution/protocol/params/protocol.go | Updates EIP-8282 builder deposit/exit predeploy addresses. |
| execution/protocol/misc/eip8282.go | Replaces embedded EIP-8282 builder deposit/exit runtime bytecode. |
| execution/protocol/misc/eip8282_test.go | Updates address assertions and adds SHA-256 checks for the embedded bytecode. |
| execution/chain/spec/eip8282_test.go | Adds a test ensuring DeveloperGenesis alloc includes the builder contracts with correct code/nonce. |
| execution/chain/spec/allocs/dev.json | Updates dev genesis alloc entries for the new builder addresses and bytecode. |
| .github/workflows/test-hive-eest.yml | Lowers allowed failures for the glamsterdam-devnet Hive EEST job. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nonce := (*big.Int)(&tx.Nonce) | ||
| if !nonce.IsUint64() { | ||
| return nil, fmt.Errorf("invalid txn nonce (overflowed) %q", nonce) | ||
| } |
mh0lt
left a comment
mh0lt
left a comment
There was a problem hiding this comment.
Reviewed at opus/high effort — LGTM, approving.
- Verified: recomputed SHA256 of both builder bytecodes match the test expectations; new addresses
…0D8282/…0E8282match EIPs PR #11899 across params/protocol.go, both eip8282_test.go, and allocs/dev.json; old addresses fully removed. Correctly IsAmsterdam-gated. - Nit:
addAmsterdamBuilderContractsin execmoduletester omits the storage slot-0 sentinel (0xff…ff) that dev.json sets — behavior-preserving today, but a latent divergence if a future in-process test drives the dequeue queue logic. Consider matching dev.json's storage. - Nit: TestBuilderContractArtifacts dropped the descriptive require messages — a one-liner ("bytecode changed — update address if intended") would make an accidental-bytecode-change failure self-explanatory.
taratorio
changed the title
| Balance: new(big.Int), | ||
| Code: misc.BuilderDepositRequestCode, | ||
| Nonce: 1, | ||
| Storage: map[common.Hash]common.Hash{slot: sentinel}, |
There was a problem hiding this comment.
[P2] Match devnet-7’s constructor state. The pinned v7.2.0 deposit initcode only copies/returns runtime code and never writes slot 0; the deployment script explicitly says the deposit has no inhibitor. Seeding it here makes deposits revert until the first system call. Please leave the deposit slot unset and align dev.json plus the genesis test. (The EIP prose currently conflicts with the released artifact.)
closes #22382
merge after #22517