Skip to content

Commit 1522edd

Browse files
committed
execution/stagedsync: drop dead finalizeWithIBS + finalizeTx (delta-args) and their tests
First incremental cut toward the structural goal of #21138: one finalize function per parallel-exec result, with no IntraBlockState used outside workers. This commit removes two finalize variants that are unreachable from production code: * finalizeWithIBS (~120 LOC) — full IBS reconstruction path that was kept around for the BAL-compat strip→rebase→merge route. Zero callers on main since the BAL fix in #21177 + the parallel-exec stack in #21153 rerouted that flow through finalizeTxSimple. * finalizeTx (delta-args variant, ~250 LOC) — direct-fee-balance path consumed only by TestFinalizeTx_AllScenarios and its hasCoinbaseDelta / adjustForTransferDelta helpers. Production calls finalizeTxSimple via the dispatch in `finalize`; this variant was test-only by 958b2fb. Test cleanup removes: * runFinalizeTx test runner * TestFinalizeTx_SimpleTransfer, TestFinalizeTx_London, TestFinalizeTx_AllScenarios * coinbaseIsRecipientScenario, selfTransferScenario fixture builders * hasCoinbaseDelta, adjustForTransferDelta, buildWriteMap, fmtWriteVal, extractBalanceReads helpers that had no other consumers Live finalizeTxSimple tests (TestFinalizeTxSimple_*) and the surviving TestResolveStorageWrites_*, TestNormalizeWriteSet_*, TestSelfDestructPath_* families are untouched. Updates one stale comment in engine_api_bal_test.go that referenced finalizeWithIBS. Verified: make lint clean; full execution/{stagedsync,state,tests} and rpc/jsonrpc test packages green under EXEC3_PARALLEL=true; BAL family 8/8 parallel; EIP-7708 burn-log family green. No semantic change — dead code removal only.
1 parent f05a65e commit 1522edd

3 files changed

Lines changed: 1 addition & 690 deletions

File tree

execution/engineapi/engine_api_bal_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import (
4444
// TestEngineApiBALMultiSenderBlock packs transfers from many independent senders
4545
// into a single block. Because the senders are independent the parallel executor
4646
// speculatively executes them concurrently, exercising the coinbase-balance
47-
// strip→rebase→merge path in finalizeWithIBS. Any divergence between the
47+
// fee-aggregation path in finalizeTxSimple. Any divergence between the
4848
// assembler's BAL (sequential) and the parallel executor's BAL surfaces as a
4949
// BAL hash mismatch returned by ProcessBAL.
5050
func TestEngineApiBALMultiSenderBlock(t *testing.T) {

execution/stagedsync/exec3_finalize_test.go

Lines changed: 0 additions & 316 deletions
Original file line numberDiff line numberDiff line change
@@ -167,39 +167,6 @@ func (s *testFinalizeScenario) buildExecResult() *execResult {
167167
return &execResult{TxResult: txResult}
168168
}
169169

170-
// runFinalizeTx runs the direct finalize path.
171-
func (s *testFinalizeScenario) runFinalizeTx(t *testing.T) (state.ReadSet, state.VersionedWrites) {
172-
t.Helper()
173-
result := s.buildExecResult()
174-
result.TxIn = copyReadSet(s.txIn)
175-
result.TxOut = copyWrites(s.txOut)
176-
if s.collectorWrites != nil {
177-
result.CollectorWrites = copyWrites(s.collectorWrites)
178-
}
179-
vm := state.NewVersionMap(nil)
180-
reader := s.makeReader()
181-
182-
// Strip coinbase/burnt.
183-
txOut, coinbaseDelta, coinbaseDeltaIncrease, hasCoinbaseDelta := result.TxOut.StripBalanceWrite(result.Coinbase, result.TxIn)
184-
result.TxOut = txOut
185-
txOut, burntDelta, burntDeltaIncrease, hasBurntDelta := result.TxOut.StripBalanceWrite(result.ExecutionResult.BurntContractAddress, result.TxIn)
186-
result.TxOut = txOut
187-
result.TxIn.Delete(result.Coinbase, state.AccountKey{Path: state.BalancePath})
188-
result.TxIn.Delete(result.ExecutionResult.BurntContractAddress, state.AccountKey{Path: state.BalancePath})
189-
190-
task := result.Task.(*taskVersion)
191-
txTask := task.Task.(*exec.TxTask)
192-
193-
_, reads, writes, err := result.finalizeTx(
194-
task, txTask, nil, nil, vm, reader,
195-
coinbaseDelta, coinbaseDeltaIncrease, hasCoinbaseDelta,
196-
burntDelta, burntDeltaIncrease, hasBurntDelta,
197-
s.rules, false, "",
198-
)
199-
require.NoError(t, err)
200-
return reads, writes
201-
}
202-
203170
func fAddr(name string) accounts.Address {
204171
var a [20]byte
205172
copy(a[:], name)
@@ -312,246 +279,6 @@ func londonTransferScenario() *testFinalizeScenario {
312279
return s
313280
}
314281

315-
// TestFinalizeTx_SimpleTransfer verifies the direct finalize path produces
316-
// expected reads and writes for a simple ETH transfer.
317-
func TestFinalizeTx_SimpleTransfer(t *testing.T) {
318-
t.Parallel()
319-
s := simpleTransferScenario()
320-
reads, writes := s.runFinalizeTx(t)
321-
322-
assert.NotNil(t, reads, "direct path should produce reads")
323-
assert.Greater(t, len(writes), 0, "direct path should produce writes")
324-
325-
coinbaseWrite := findWrite(writes, s.coinbase, state.BalancePath)
326-
require.NotNil(t, coinbaseWrite, "should have coinbase balance write")
327-
coinbaseBalance := coinbaseWrite.Val.(uint256.Int)
328-
assert.Equal(t, s.feeTipped, coinbaseBalance, "coinbase should receive the tip")
329-
}
330-
331-
// TestFinalizeTx_London verifies the direct finalize path with burnt fees.
332-
func TestFinalizeTx_London(t *testing.T) {
333-
t.Parallel()
334-
s := londonTransferScenario()
335-
reads, writes := s.runFinalizeTx(t)
336-
337-
assert.NotNil(t, reads)
338-
assert.Greater(t, len(writes), 0)
339-
340-
burntWrite := findWrite(writes, s.burntAddr, state.BalancePath)
341-
require.NotNil(t, burntWrite, "should have burnt contract balance write")
342-
burntBalance := burntWrite.Val.(uint256.Int)
343-
expected := new(uint256.Int).Add(uint256.NewInt(500_000), &s.feeBurnt)
344-
assert.Equal(t, *expected, burntBalance, "burnt contract should receive base fee")
345-
}
346-
347-
// coinbaseIsRecipientScenario: coinbase is the recipient of the transfer.
348-
// Coinbase balance in TxOut has the transfer amount (from execution) plus
349-
// it will get the tip from finalize. The StripBalanceWrite must correctly
350-
// handle the case where coinbase has a real balance change from execution.
351-
func coinbaseIsRecipientScenario() *testFinalizeScenario {
352-
sender := fAddr("sender")
353-
coinbase := fAddr("coinbase") // coinbase is also the recipient
354-
355-
senderBal := uint256.NewInt(100_000_000_000)
356-
coinbaseBal := uint256.NewInt(5_000_000_000)
357-
transferAmt := uint256.NewInt(1_000_000_000)
358-
tip := uint256.NewInt(21_000)
359-
360-
// During calcFees=false execution:
361-
// - sender loses transfer amount (but NOT gas — fees deferred)
362-
// - coinbase gains transfer amount (but NOT tip — fees deferred)
363-
newSenderBal := new(uint256.Int).Sub(senderBal, transferAmt)
364-
newCoinbaseBal := new(uint256.Int).Add(coinbaseBal, transferAmt)
365-
366-
rules := &chain.Rules{IsSpuriousDragon: true}
367-
config := &chain.Config{ChainID: big.NewInt(1)}
368-
369-
txIn := state.ReadSet{}
370-
txIn.Set(state.VersionedRead{Address: sender, Path: state.AddressPath, Val: fMakeAccount(senderBal.Uint64(), 0)})
371-
txIn.Set(state.VersionedRead{Address: sender, Path: state.BalancePath, Val: *senderBal})
372-
txIn.Set(state.VersionedRead{Address: sender, Path: state.NoncePath, Val: uint64(0)})
373-
// Coinbase IS touched during execution (as transfer recipient).
374-
txIn.Set(state.VersionedRead{Address: coinbase, Path: state.AddressPath, Val: fMakeAccount(coinbaseBal.Uint64(), 0)})
375-
txIn.Set(state.VersionedRead{Address: coinbase, Path: state.BalancePath, Val: *coinbaseBal})
376-
377-
// TxOut: coinbase has transfer amount but NOT tip (fees deferred).
378-
txOut := state.VersionedWrites{
379-
{Address: sender, Path: state.BalancePath, Val: *newSenderBal, Reason: tracing.BalanceDecreaseGasBuy},
380-
{Address: sender, Path: state.NoncePath, Val: uint64(1)},
381-
{Address: coinbase, Path: state.BalancePath, Val: *newCoinbaseBal, Reason: tracing.BalanceChangeTransfer},
382-
}
383-
384-
collectorWrites := state.VersionedWrites{
385-
{Address: sender, Path: state.BalancePath, Val: *newSenderBal},
386-
{Address: sender, Path: state.NoncePath, Val: uint64(1)},
387-
{Address: sender, Path: state.IncarnationPath, Val: uint64(1)},
388-
{Address: sender, Path: state.CodeHashPath, Val: accounts.EmptyCodeHash},
389-
{Address: coinbase, Path: state.BalancePath, Val: *newCoinbaseBal},
390-
{Address: coinbase, Path: state.NoncePath, Val: uint64(0)},
391-
{Address: coinbase, Path: state.IncarnationPath, Val: uint64(1)},
392-
{Address: coinbase, Path: state.CodeHashPath, Val: accounts.EmptyCodeHash},
393-
}
394-
395-
return &testFinalizeScenario{
396-
name: "coinbase_is_recipient",
397-
accts: map[accounts.Address]*accounts.Account{
398-
sender: fMakeAccount(senderBal.Uint64(), 0),
399-
coinbase: fMakeAccount(coinbaseBal.Uint64(), 0),
400-
},
401-
txIn: txIn,
402-
txOut: txOut,
403-
collectorWrites: collectorWrites,
404-
feeTipped: *tip,
405-
coinbase: coinbase,
406-
burntAddr: accounts.NilAddress,
407-
rules: rules,
408-
config: config,
409-
header: &types.Header{
410-
Number: *uint256.NewInt(1),
411-
GasLimit: 30_000_000,
412-
GasUsed: 21000,
413-
},
414-
}
415-
}
416-
417-
// selfTransferScenario: sender sends ETH to themselves. Coinbase is separate.
418-
// Tests that same-address sender+recipient doesn't confuse the finalize logic.
419-
func selfTransferScenario() *testFinalizeScenario {
420-
sender := fAddr("sender")
421-
coinbase := fAddr("coinbase")
422-
423-
senderBal := uint256.NewInt(100_000_000_000)
424-
425-
// Self-transfer: balance doesn't change (transfer cancels out).
426-
// Gas is NOT deducted during calcFees=false, so balance stays the same.
427-
// But nonce increments.
428-
rules := &chain.Rules{IsSpuriousDragon: true}
429-
config := &chain.Config{ChainID: big.NewInt(1)}
430-
tip := uint256.NewInt(21_000)
431-
432-
txIn := state.ReadSet{}
433-
txIn.Set(state.VersionedRead{Address: sender, Path: state.AddressPath, Val: fMakeAccount(senderBal.Uint64(), 0)})
434-
txIn.Set(state.VersionedRead{Address: sender, Path: state.BalancePath, Val: *senderBal})
435-
txIn.Set(state.VersionedRead{Address: sender, Path: state.NoncePath, Val: uint64(0)})
436-
437-
txOut := state.VersionedWrites{
438-
{Address: sender, Path: state.BalancePath, Val: *senderBal, Reason: tracing.BalanceChangeTransfer},
439-
{Address: sender, Path: state.NoncePath, Val: uint64(1)},
440-
}
441-
442-
collectorWrites := state.VersionedWrites{
443-
{Address: sender, Path: state.BalancePath, Val: *senderBal},
444-
{Address: sender, Path: state.NoncePath, Val: uint64(1)},
445-
{Address: sender, Path: state.IncarnationPath, Val: uint64(1)},
446-
{Address: sender, Path: state.CodeHashPath, Val: accounts.EmptyCodeHash},
447-
}
448-
449-
return &testFinalizeScenario{
450-
name: "self_transfer",
451-
accts: map[accounts.Address]*accounts.Account{
452-
sender: fMakeAccount(senderBal.Uint64(), 0),
453-
coinbase: fMakeAccount(0, 0),
454-
},
455-
txIn: txIn,
456-
txOut: txOut,
457-
collectorWrites: collectorWrites,
458-
feeTipped: *tip,
459-
coinbase: coinbase,
460-
burntAddr: accounts.NilAddress,
461-
rules: rules,
462-
config: config,
463-
header: &types.Header{
464-
Number: *uint256.NewInt(1),
465-
GasLimit: 30_000_000,
466-
GasUsed: 21000,
467-
},
468-
}
469-
}
470-
471-
// TestFinalizeTx_AllScenarios verifies the direct finalize path produces
472-
// correct writes and reads across all test scenarios.
473-
func TestFinalizeTx_AllScenarios(t *testing.T) {
474-
t.Parallel()
475-
scenarios := []*testFinalizeScenario{
476-
simpleTransferScenario(),
477-
londonTransferScenario(),
478-
coinbaseIsRecipientScenario(),
479-
selfTransferScenario(),
480-
}
481-
482-
for _, s := range scenarios {
483-
t.Run(s.name, func(t *testing.T) {
484-
// TODO(#20962): when coinbase is also the transfer recipient,
485-
// finalizeTx no longer adds the tip on top of the existing TxOut
486-
// coinbase write. Either StripBalanceWrite or the post-strip
487-
// re-credit shifted; re-validate before un-skipping.
488-
if s.name == "coinbase_is_recipient" {
489-
t.Skip("stale coinbase-is-recipient expectation, see #20962")
490-
}
491-
reads, writes := s.runFinalizeTx(t)
492-
493-
assert.NotNil(t, reads, "should produce reads")
494-
assert.Greater(t, len(writes), 0, "should produce writes")
495-
496-
// Coinbase must receive the tip.
497-
coinbaseWrite := findWrite(writes, s.coinbase, state.BalancePath)
498-
require.NotNil(t, coinbaseWrite, "should have coinbase balance write")
499-
coinbaseBalance := coinbaseWrite.Val.(uint256.Int)
500-
501-
expectedCoinbase := new(uint256.Int).Set(&s.feeTipped)
502-
if acc, ok := s.accts[s.coinbase]; ok {
503-
expectedCoinbase.Add(expectedCoinbase, &acc.Balance)
504-
}
505-
// If coinbase was also a transfer recipient, add the delta.
506-
if hasCoinbaseDelta(s) {
507-
expectedCoinbase = adjustForTransferDelta(s, expectedCoinbase)
508-
}
509-
assert.Equal(t, *expectedCoinbase, coinbaseBalance,
510-
"coinbase balance should be original + tip (+ transfer delta if recipient)")
511-
512-
// If London: burnt contract must receive base fee.
513-
if s.rules.IsLondon && s.burntAddr != accounts.NilAddress {
514-
burntWrite := findWrite(writes, s.burntAddr, state.BalancePath)
515-
require.NotNil(t, burntWrite, "should have burnt contract balance write")
516-
burntBalance := burntWrite.Val.(uint256.Int)
517-
expectedBurnt := new(uint256.Int).Add(&s.accts[s.burntAddr].Balance, &s.feeBurnt)
518-
assert.Equal(t, *expectedBurnt, burntBalance,
519-
"burnt contract should receive base fee")
520-
}
521-
522-
// BalancePath reads must include coinbase.
523-
balReads := extractBalanceReads(reads)
524-
_, hasCoinbaseRead := balReads[s.coinbase]
525-
assert.True(t, hasCoinbaseRead, "should read coinbase balance")
526-
})
527-
}
528-
}
529-
530-
// hasCoinbaseDelta checks if coinbase was touched during execution (transfer recipient).
531-
func hasCoinbaseDelta(s *testFinalizeScenario) bool {
532-
for _, w := range s.txOut {
533-
if w.Address == s.coinbase && w.Path == state.BalancePath {
534-
return true
535-
}
536-
}
537-
return false
538-
}
539-
540-
// adjustForTransferDelta computes the expected coinbase balance when coinbase
541-
// is also the transfer recipient. The delta is the difference between the
542-
// execution-written balance and the original balance.
543-
func adjustForTransferDelta(s *testFinalizeScenario, base *uint256.Int) *uint256.Int {
544-
for _, w := range s.txOut {
545-
if w.Address == s.coinbase && w.Path == state.BalancePath {
546-
execBal := w.Val.(uint256.Int)
547-
origBal := s.accts[s.coinbase].Balance
548-
delta := new(uint256.Int).Sub(&execBal, &origBal)
549-
return new(uint256.Int).Add(base, delta)
550-
}
551-
}
552-
return base
553-
}
554-
555282
func findWrite(writes state.VersionedWrites, addr accounts.Address, path state.AccountPath) *state.VersionedWrite {
556283
for _, w := range writes {
557284
if w.Address == addr && w.Path == path {
@@ -561,49 +288,6 @@ func findWrite(writes state.VersionedWrites, addr accounts.Address, path state.A
561288
return nil
562289
}
563290

564-
func buildWriteMap(writes state.VersionedWrites) map[string]string {
565-
m := make(map[string]string, len(writes))
566-
for _, w := range writes {
567-
key := w.Address.String() + ":" + w.Path.String() + ":" + w.Key.String()
568-
m[key] = fmtWriteVal(w)
569-
}
570-
return m
571-
}
572-
573-
func fmtWriteVal(w *state.VersionedWrite) string {
574-
switch v := w.Val.(type) {
575-
case uint256.Int:
576-
return v.Hex()
577-
case uint64:
578-
return uint256.NewInt(v).Hex()
579-
case bool:
580-
if v {
581-
return "true"
582-
}
583-
return "false"
584-
case accounts.CodeHash:
585-
return v.String()
586-
default:
587-
return "<unknown>"
588-
}
589-
}
590-
591-
func extractBalanceReads(reads state.ReadSet) map[accounts.Address]string {
592-
m := make(map[accounts.Address]string)
593-
if reads == nil {
594-
return m
595-
}
596-
reads.Scan(func(vr *state.VersionedRead) bool {
597-
if vr.Path == state.BalancePath {
598-
if val, ok := vr.Val.(uint256.Int); ok {
599-
m[vr.Address] = val.Hex()
600-
}
601-
}
602-
return true
603-
})
604-
return m
605-
}
606-
607291
// --- finalizeTxSimple tests ---
608292
// These test the split fee logic: worker debits sender only,
609293
// finalize credits coinbase/burnt. No StripBalanceWrite or delta computation.

0 commit comments

Comments
 (0)