Skip to content

execution: tidy move some bal selfdestruct tests to engine_api_bal_test#21599

Merged
taratorio merged 2 commits into
mainfrom
worktree-move-bal_selfdestruct_systemaddress_test
Jun 3, 2026
Merged

execution: tidy move some bal selfdestruct tests to engine_api_bal_test#21599
taratorio merged 2 commits into
mainfrom
worktree-move-bal_selfdestruct_systemaddress_test

Conversation

@taratorio

Copy link
Copy Markdown
Member

minor tidy follow up to #21333 to move the tests to be EngineApiTester based
should've done it back then but was in a rush to fix the issue

@taratorio taratorio requested review from mh0lt and yperbasis as code owners June 3, 2026 09:53
@taratorio

Copy link
Copy Markdown
Member Author

Move SELFDESTRUCT/SystemAddress BAL tests to the engine-API tester

Summary

Test-only refactor. Moves the three EIP-7928 SELFDESTRUCT-beneficiary BAL
regression tests from execution/execmodule/bal_selfdestruct_systemaddress_test.go
(driven by blockgen.GenerateChain — proposer/assembler path only) into
execution/engineapi/engine_api_bal_test.go, where they run through the full
engine BuildCanonicalBlock round-trip (serial proposer and parallel
validator via newPayload). No production code changes.

These tests guard the regression fixed in #21333: per EIP-7928 a SELFDESTRUCT
records the beneficiary as a state access independent of any value transfer, and
the SYSTEM_ADDRESS carve-out keeps that entry because the SELFDESTRUCT is itself
the access. Pre-fix, a zero-balance SELFDESTRUCT to SYSTEM_ADDRESS dropped the
BAL entry → BAL hash mismatch → block rejected / sync stall.

Changes

  • Add to execution/engineapi/engine_api_bal_test.go:
    • TestEngineApiBALIncludesSystemAddressOnSelfdestructToItWithZeroBalance — the
      bug scenario: CREATE tx with init code PUSH20 SYSTEM_ADDRESS; SELFDESTRUCT
      and zero contract balance. Asserts the BAL contains a SystemAddress entry
      with every change-set empty.
    • TestEngineApiBALIncludesSystemAddressOnSelfdestructToItWithNonZeroBalance
      guard: same init code, 1 wei sent to the contract; asserts a balance change on
      the SystemAddress entry.
    • TestEngineApiBALIncludesOrdinaryBeneficiaryOnSelfdestructWithZeroBalance
      guard: an ordinary EOA beneficiary (carve-out N/A) still appears in the BAL.
    • Helpers newSelfdestructBALTester, signCreateTx, pushAddrSelfdestruct.
  • Delete execution/execmodule/bal_selfdestruct_systemaddress_test.go.

Notes

  • Consistent with the existing BAL tests in the file: each is gated with
    if !dbg.Exec3Parallel { t.Skip("requires parallel exec") } and runs under
    ERIGON_EXEC3_PARALLEL=true (the BAL is only produced under parallel exec).
  • A node accepts its own BAL, so BuildCanonicalBlock stays VALID even pre-fix
    (proposer and validator compute the same wrong BAL). The regression is caught
    by asserting BAL contents (the missing SystemAddress entry), not a
    payload-INVALID status.

Test plan / TDD verification

ERIGON_EXEC3_PARALLEL=true go test -count=1 -run TestEngineApiBALIncludes ./execution/engineapi/

Step Result
Tests vs current (fixed) code 3/3 pass
Revert #21333 (operations_acl.go + intra_block_state.go) ...ZeroBalance FAILSExpected value not to be nil / "BAL must include a SystemAddress entry…"; other two pass
Re-add #21333 3/3 pass

The red/green split (only the zero-balance SystemAddress case fails) matches
#21333's own TDD note — the bug is specific to SYSTEM_ADDRESS + zero balance.

  • golangci-lint on execution/engineapi/... and execution/execmodule/...: 0 issues.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Moves the BAL SELFDESTRUCT beneficiary regression tests (introduced in the follow-up to #21333 / EIP-7928) from the execmodule/blockgen-based harness into the Engine API BAL test suite so they run through the EngineApiTester/BuildCanonicalBlock path that validates proposer vs parallel-executor BAL consistency.

Changes:

  • Remove the standalone execution/execmodule regression test file for SELFDESTRUCT→SystemAddress/EOA BAL inclusion.
  • Reintroduce the same scenarios in execution/engineapi/engine_api_bal_test.go using EngineApiTester, plus small local helpers for CREATE+SELFDESTRUCT initcode testing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
execution/execmodule/bal_selfdestruct_systemaddress_test.go Removed: blockgen-based BAL SELFDESTRUCT beneficiary regression tests.
execution/engineapi/engine_api_bal_test.go Added: Engine API-based versions of the BAL SELFDESTRUCT beneficiary regression tests and supporting helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@taratorio taratorio added this pull request to the merge queue Jun 3, 2026
Merged via the queue into main with commit 93f47e1 Jun 3, 2026
90 checks passed
@taratorio taratorio deleted the worktree-move-bal_selfdestruct_systemaddress_test branch June 3, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants