diff --git a/execution/stagedsync/exec3_fee_merge_temp_test.go b/execution/stagedsync/exec3_fee_merge_temp_test.go index 942ae076439..55deb3f18af 100644 --- a/execution/stagedsync/exec3_fee_merge_temp_test.go +++ b/execution/stagedsync/exec3_fee_merge_temp_test.go @@ -27,7 +27,7 @@ import ( "github.com/erigontech/erigon/execution/types/accounts" ) -func feeMergeTestWrites(t *testing.T, addr accounts.Address, balance uint64) *state.WriteSet { +func feeMergeTestWrites(t testing.TB, addr accounts.Address, balance uint64) *state.WriteSet { t.Helper() ws := &state.WriteSet{} ws.SetBalance(addr, &state.VersionedWrite[uint256.Int]{ @@ -37,6 +37,15 @@ func feeMergeTestWrites(t *testing.T, addr accounts.Address, balance uint64) *st return ws } +func feeMergeTestExecutor(t testing.TB) *blockExecutor { + t.Helper() + return &blockExecutor{feeMergeTemp: map[int]feeMerge{}, blockIO: state.NewVersionedIO(2)} +} + +func feeMergeTestAddr(hex string) accounts.Address { + return accounts.InternAddress(common.HexToAddress(hex)) +} + // TestRecordFeeMergeReleasesSupersededTemp pins the three transitions the fee // merge goes through for one tx: the first merge has no temp to reclaim, a // revalidation round reclaims the temp it replaces, and a round whose input is @@ -45,34 +54,111 @@ func feeMergeTestWrites(t *testing.T, addr accounts.Address, balance uint64) *st func TestRecordFeeMergeReleasesSupersededTemp(t *testing.T) { t.Parallel() - addr := accounts.InternAddress(common.HexToAddress("0x1111111111111111111111111111111111111111")) - be := &blockExecutor{feeMergeTemp: map[int]*state.WriteSet{}} + addr := feeMergeTestAddr("0x1111111111111111111111111111111111111111") + be := feeMergeTestExecutor(t) + version := state.Version{TxIndex: 0} // First round: prev is the worker's TxOut, so nothing may be released. txOut := feeMergeTestWrites(t, addr, 1) - temp1 := feeMergeTestWrites(t, addr, 2) - be.recordFeeMerge(0, txOut, temp1) - require.Same(t, temp1, be.feeMergeTemp[0]) + tip1 := feeMergeTestWrites(t, addr, 2) + be.recordFeeMerge(version, txOut, tip1) + require.Same(t, tip1, be.feeMergeTemp[0].writes) + require.Same(t, tip1, be.blockIO.WriteSet(version.TxIndex), + "the merge product must be the tx's recorded write set") require.Equal(t, 1, txOut.Count(), "TxOut must survive the fee merge") // Revalidation round: prev is the temp the first round recorded, so it is // superseded and reclaimed. - temp2 := feeMergeTestWrites(t, addr, 3) - be.recordFeeMerge(0, temp1, temp2) + tip2 := feeMergeTestWrites(t, addr, 3) + be.recordFeeMerge(version, tip1, tip2) be.awaitMapReleases() - require.Same(t, temp2, be.feeMergeTemp[0]) - require.Equal(t, 0, temp1.Count(), "superseded fee-merge temp must be released") - require.Equal(t, 1, temp2.Count()) + require.Same(t, tip2, be.feeMergeTemp[0].writes) + require.Equal(t, 0, tip1.Count(), "superseded fee-merge temp must be released") + require.Equal(t, 1, tip2.Count()) // After a re-execution the recorded slot is the new TxOut again, so the // stale temp does not match prev and stays untouched. txOut2 := feeMergeTestWrites(t, addr, 4) - temp3 := feeMergeTestWrites(t, addr, 5) - be.recordFeeMerge(0, txOut2, temp3) + tip3 := feeMergeTestWrites(t, addr, 5) + be.recordFeeMerge(version, txOut2, tip3) be.awaitMapReleases() - require.Same(t, temp3, be.feeMergeTemp[0]) + require.Same(t, tip3, be.feeMergeTemp[0].writes) require.Equal(t, 1, txOut2.Count(), "TxOut must survive the fee merge") - require.Equal(t, 1, temp2.Count(), "a temp that is not prev must not be released") + require.Equal(t, 1, tip2.Count(), "a temp that is not prev must not be released") +} + +// TestRecordFeeMergeSkipsEmptyTip covers the round the skip produces: with no +// credit to fold in, the recorded set stays the worker's own output — which +// calcFees must keep reading as the pre-credit balance. +func TestRecordFeeMergeSkipsEmptyTip(t *testing.T) { + t.Parallel() + + addr := feeMergeTestAddr("0x5555555555555555555555555555555555555555") + be := feeMergeTestExecutor(t) + version := state.Version{TxIndex: 0} + + txOut := feeMergeTestWrites(t, addr, 1) + be.recordWorkerWrites(version, txOut) + be.recordFeeMerge(version, txOut, nil) + + require.Same(t, txOut, be.blockIO.WriteSet(version.TxIndex)) + require.Nil(t, be.creditedWrites(version, txOut), + "a skipped credit must not mark the worker's TxOut as carrying one") +} + +// TestRecordWorkerWritesDropsCreditedTemp drives the re-execution path through +// recordWorkerWrites: the new TxOut displaces the credited set, so the credit +// must be gone and the set it lived in reclaimed. +func TestRecordWorkerWritesDropsCreditedTemp(t *testing.T) { + t.Parallel() + + addr := feeMergeTestAddr("0x4444444444444444444444444444444444444444") + be := feeMergeTestExecutor(t) + version := state.Version{TxIndex: 0} + + txOut := feeMergeTestWrites(t, addr, 1) + be.recordWorkerWrites(version, txOut) + require.Nil(t, be.creditedWrites(version, be.blockIO.WriteSet(version.TxIndex)), + "the worker's own output carries no credit") + + tip := feeMergeTestWrites(t, addr, 2) + be.recordFeeMerge(version, txOut, tip) + require.Same(t, tip, be.creditedWrites(version, be.blockIO.WriteSet(version.TxIndex))) + + reTxOut := feeMergeTestWrites(t, addr, 3) + be.recordWorkerWrites(version, reTxOut) + be.awaitMapReleases() + + require.Nil(t, be.creditedWrites(version, be.blockIO.WriteSet(version.TxIndex)), + "a re-executed tx must be credited again, not handed the stale credit") + require.Equal(t, 0, tip.Count(), "the displaced fee-merge temp must be released") + require.Equal(t, 1, reTxOut.Count(), "the new TxOut must survive") +} + +// TestCreditedWritesPinsVersion pins the credit to the incarnation it was +// computed for, so a re-executed tx cannot inherit it even if the set it lives +// in is still the recorded one. +func TestCreditedWritesPinsVersion(t *testing.T) { + t.Parallel() + + addr := feeMergeTestAddr("0x6666666666666666666666666666666666666666") + be := feeMergeTestExecutor(t) + version := state.Version{TxIndex: 0} + + tip := feeMergeTestWrites(t, addr, 2) + be.recordFeeMerge(version, feeMergeTestWrites(t, addr, 1), tip) + require.Same(t, tip, be.creditedWrites(version, tip)) + + reExecuted := version + reExecuted.Incarnation = 1 + require.Nil(t, be.creditedWrites(reExecuted, tip), + "a credit computed for an earlier incarnation is not this incarnation's") + + otherTx := state.Version{TxIndex: 1} + require.Nil(t, be.creditedWrites(otherTx, tip), + "another tx's fee-merge product says nothing about this tx") + require.Nil(t, be.creditedWrites(version, nil), + "a tx with no writes at all must not read as credited") } // TestRecordFeeMergeReleaseKeepsSharedWrites pins what makes the release safe: @@ -81,21 +167,48 @@ func TestRecordFeeMergeReleasesSupersededTemp(t *testing.T) { func TestRecordFeeMergeReleaseKeepsSharedWrites(t *testing.T) { t.Parallel() - shared := accounts.InternAddress(common.HexToAddress("0x2222222222222222222222222222222222222222")) - fresh := accounts.InternAddress(common.HexToAddress("0x3333333333333333333333333333333333333333")) - be := &blockExecutor{feeMergeTemp: map[int]*state.WriteSet{}} + shared := feeMergeTestAddr("0x2222222222222222222222222222222222222222") + fresh := feeMergeTestAddr("0x3333333333333333333333333333333333333333") + be := feeMergeTestExecutor(t) + version := state.Version{TxIndex: 0} temp1 := feeMergeTestWrites(t, shared, 7) - be.recordFeeMerge(0, feeMergeTestWrites(t, shared, 1), temp1) + be.recordFeeMerge(version, feeMergeTestWrites(t, shared, 1), temp1) tipWrites := feeMergeTestWrites(t, fresh, 9) - merged := temp1.MergeInto(tipWrites) - require.Same(t, tipWrites, merged) - be.recordFeeMerge(0, temp1, merged) + be.recordFeeMerge(version, temp1, tipWrites) be.awaitMapReleases() require.Equal(t, 0, temp1.Count()) - vw, ok := merged.GetBalance(shared) + require.Same(t, tipWrites, be.blockIO.WriteSet(version.TxIndex)) + vw, ok := tipWrites.GetBalance(shared) require.True(t, ok, "entry shared from the released temp must still be reachable") require.Equal(t, uint64(7), vw.Val.Uint64()) } + +// TestCalcFeesRoundThroughBlockExecutor runs the round the way the apply loop +// does — creditedWrites in, recordFeeMerge out — so the skip is exercised +// against the real feeMergeTemp bookkeeping rather than the fixture's stand-in. +func TestCalcFeesRoundThroughBlockExecutor(t *testing.T) { + t.Parallel() + s := simpleTransferScenario() + r := newFeeCreditRound(t, s) + be := feeMergeTestExecutor(t) + version := r.task.Version() + be.recordWorkerWrites(version, r.result.TxOut) + + round := func() *state.WriteSet { + recorded := be.blockIO.WriteSet(version.TxIndex) + tip, err := r.result.calcFees(r.task, r.vm, r.reader, r.rules, be.creditedWrites(version, recorded)) + require.NoError(t, err) + be.recordFeeMerge(version, recorded, tip) + return tip + } + + first := round() + require.False(t, first.IsEmpty(), "the first round must credit the tip") + require.Same(t, first, be.blockIO.WriteSet(version.TxIndex)) + require.True(t, round().IsEmpty(), "the recorded set already carries this credit") + require.Same(t, first, be.blockIO.WriteSet(version.TxIndex), + "a skipped round must leave the recorded set alone") +} diff --git a/execution/stagedsync/exec3_finalize_test.go b/execution/stagedsync/exec3_finalize_test.go index e39fa202c4a..8c19a3904b0 100644 --- a/execution/stagedsync/exec3_finalize_test.go +++ b/execution/stagedsync/exec3_finalize_test.go @@ -200,7 +200,8 @@ func (s *testFinalizeScenario) makeReader() *mapStateReader { return r } -// buildExecResult creates an execResult for testing. +// buildExecResult creates an execResult for testing, with the scenario's IO +// copied in so repeated runs off one scenario see identical inputs. func (s *testFinalizeScenario) buildExecResult() *execResult { blockNum := s.header.Number.Uint64() @@ -238,6 +239,9 @@ func (s *testFinalizeScenario) buildExecResult() *execResult { }, Coinbase: s.coinbase, } + txResult.TxIn = copyReadSet(s.txIn) + txResult.TxOut = copyWrites(s.txOut) + txResult.CollectorWrites = copyWrites(s.collectorWrites) return &execResult{TxResult: txResult} } @@ -543,11 +547,6 @@ func findAddress(writes *state.WriteSet, addr accounts.Address) *state.Versioned func (s *testFinalizeScenario) runFinalizeTx(t *testing.T, priorCoinbaseBalance *uint256.Int) *state.WriteSet { t.Helper() result := s.buildExecResult() - result.TxIn = copyReadSet(s.txIn) - result.TxOut = copyWrites(s.txOut) - if s.collectorWrites != nil { - result.CollectorWrites = copyWrites(s.collectorWrites) - } vm := state.NewVersionMap(nil) reader := s.makeReader() @@ -565,7 +564,7 @@ func (s *testFinalizeScenario) runFinalizeTx(t *testing.T, priorCoinbaseBalance task := result.Task.(*taskVersion) - writes, err := result.calcFees(task, vm, reader, s.rules) + writes, err := result.calcFees(task, vm, reader, s.rules, nil) require.NoError(t, err) return writes } @@ -818,9 +817,6 @@ func TestFinalizeTxSimple_SenderIsCoinbase_AccumulatedAcrossTxs(t *testing.T) { build() result := s.buildExecResult() - result.TxIn = copyReadSet(s.txIn) - result.TxOut = copyWrites(s.txOut) - result.CollectorWrites = copyWrites(s.collectorWrites) // Set this tx's version explicitly so versionMap reads land on // the right tx-index for the floor-read semantics. @@ -829,7 +825,7 @@ func TestFinalizeTxSimple_SenderIsCoinbase_AccumulatedAcrossTxs(t *testing.T) { vm.FlushVersionedWrites(result.TxOut, true, "") - writes, err := result.calcFees(task, vm, reader, s.rules) + writes, err := result.calcFees(task, vm, reader, s.rules, nil) require.NoError(t, err, "tx %d: calcFees", txIdx) // Flush finalize writes so the next tx sees them via versionMap. @@ -876,9 +872,6 @@ func TestFinalizeTxSimple_SenderIsCoinbase_ReExecutedIncarnation(t *testing.T) { // Set incarnation > 0 on the task to reflect re-execution. result := s.buildExecResult() - result.TxIn = copyReadSet(s.txIn) - result.TxOut = copyWrites(s.txOut) - result.CollectorWrites = copyWrites(s.collectorWrites) task := result.Task.(*taskVersion) task.version.Incarnation = 1 // re-execution @@ -897,7 +890,7 @@ func TestFinalizeTxSimple_SenderIsCoinbase_ReExecutedIncarnation(t *testing.T) { // Now flush the re-executed TxOut at incarnation 1. vm.FlushVersionedWrites(result.TxOut, true, "") - writes, err := result.calcFees(task, vm, reader, s.rules) + writes, err := result.calcFees(task, vm, reader, s.rules, nil) require.NoError(t, err) coinbaseWrite := findBalance(writes, s.coinbase) @@ -978,9 +971,6 @@ func TestFinalizeTxSimple_AccumulatedFees(t *testing.T) { for txIdx := 1; txIdx <= 3; txIdx++ { result := s.buildExecResult() - result.TxIn = copyReadSet(s.txIn) - result.TxOut = copyWrites(s.txOut) - result.CollectorWrites = copyWrites(s.collectorWrites) result.ExecutionResult.FeeTipped = *tipPerTx task := result.Task.(*taskVersion) @@ -989,7 +979,7 @@ func TestFinalizeTxSimple_AccumulatedFees(t *testing.T) { // Flush TxOut to versionMap (simulates line 1928). vm.FlushVersionedWrites(result.TxOut, true, "") - writes, err := result.calcFees(task, vm, reader, s.rules) + writes, err := result.calcFees(task, vm, reader, s.rules, nil) require.NoError(t, err) // Flush finalize writes to versionMap for next TX. @@ -1782,3 +1772,249 @@ func TestCalcFees_EmitsAddressPathForCoinbase(t *testing.T) { require.Equal(t, coinbaseBalance.WriteHeader.Version, coinbaseAddress.WriteHeader.Version, "AddressPath sibling must share version with the BalancePath write") } + +// feeCreditRound mirrors one apply-loop validation round for a single tx: +// calcFees derives the credit, and a non-empty result is folded into the +// recorded write set the way nextResult does it. +type feeCreditRound struct { + result *execResult + task *taskVersion + vm *state.VersionMap + reader *mapStateReader + rules *chain.Rules + recorded *state.WriteSet + // credited tracks the recorded set once a fee merge produced it, which is + // blockExecutor.feeMergeTemp's job in the apply loop. + credited *state.WriteSet +} + +func newFeeCreditRound(t testing.TB, s *testFinalizeScenario) *feeCreditRound { + t.Helper() + + result := s.buildExecResult() + + vm := state.NewVersionMap(nil) + vm.FlushVersionedWrites(result.TxOut, true, "") + + return &feeCreditRound{ + result: result, + task: result.Task.(*taskVersion), + vm: vm, + reader: s.makeReader(), + rules: s.rules, + recorded: result.TxOut, + } +} + +// run performs one round and returns the credit calcFees produced, nil when it +// found the recorded set already carries it. The returned set is a copy: the +// merge folds the recorded set into the credit in place, so the credit itself is +// only observable before it. +func (r *feeCreditRound) run(t testing.TB) *state.WriteSet { + t.Helper() + + tip, err := r.result.calcFees(r.task, r.vm, r.reader, r.rules, r.credited) + require.NoError(t, err) + if tip.IsEmpty() { + return nil + } + credit := copyWrites(tip) + r.recorded = r.recorded.MergeInto(tip) + r.credited = r.recorded + return credit +} + +func TestCalcFees_SkipsRedundantReCredit(t *testing.T) { + t.Parallel() + r := newFeeCreditRound(t, simpleTransferScenario()) + + require.NotNil(t, r.run(t), "the first round must credit the tip") + require.Nil(t, r.run(t), + "re-crediting a set that already carries this exact credit rebuilds an identical "+ + "write set and re-runs the merge for nothing") +} + +func TestCalcFees_ReCreditsWhenPriorBalanceChanged(t *testing.T) { + t.Parallel() + s := simpleTransferScenario() + r := newFeeCreditRound(t, s) + + require.NotNil(t, r.run(t), "the first round must credit the tip") + + // A prior tx moved the coinbase balance, so the tip lands on a new base. + // Only the balance changes — the rest of the account must stay put, or the + // test would pass even if recordedIn stopped comparing balances. + priorBalance := uint256.NewInt(7_000_000) + r.reader.accounts[s.coinbase].Balance = *priorBalance + + tip := r.run(t) + require.NotNil(t, tip, "a changed base balance must produce a fresh credit") + + credited := findBalance(tip, s.coinbase) + require.NotNil(t, credited) + require.Equal(t, *new(uint256.Int).Add(priorBalance, &s.feeTipped), credited.Val) +} + +func TestCalcFees_ReCreditsWhenAddressPathMissing(t *testing.T) { + t.Parallel() + s := simpleTransferScenario() + r := newFeeCreditRound(t, s) + + first := r.run(t) + require.NotNil(t, first, "the first round must credit the tip") + + // A half-recorded credit is not a credit: the balance alone leaves + // downstream reads without an account record. + balanceOnly := &state.WriteSet{} + bw, ok := first.GetBalance(s.coinbase) + require.True(t, ok) + balanceOnly.SetBalance(s.coinbase, bw) + + tip, err := r.result.calcFees(r.task, r.vm, r.reader, r.rules, balanceOnly) + require.NoError(t, err) + require.False(t, tip.IsEmpty(), "a recorded balance without its AddressPath sibling must be re-credited") + require.NotNil(t, findAddress(tip, s.coinbase)) +} + +func TestCalcFees_SkipsRedundantReCreditWithBurntContract(t *testing.T) { + t.Parallel() + s := londonTransferScenario() + r := newFeeCreditRound(t, s) + + first := r.run(t) + require.NotNil(t, first, "the first round must credit the tip") + require.NotNil(t, findBalance(first, s.burntAddr), "London burns to the burnt contract") + + require.Nil(t, r.run(t), + "both halves of the credit are already recorded, so the round is a no-op") +} + +func TestCalcFees_SkipsRedundantReCreditOnEmptyRemoval(t *testing.T) { + t.Parallel() + s := simpleTransferScenario() + // A zero tip on an already-empty coinbase is the EIP-161 case: the credit + // is a delete rather than a balance write, and takes a different path + // through both recordedIn and writeTo. + s.feeTipped = uint256.Int{} + r := newFeeCreditRound(t, s) + + first := r.run(t) + require.NotNil(t, first, "an emptied coinbase must still be touched") + sd, ok := first.GetSelfDestruct(s.coinbase) + require.True(t, ok, "the credit is a SelfDestructPath delete") + require.True(t, sd.Val) + + require.Nil(t, r.run(t), + "the delete is already recorded, so the round is a no-op") +} + +// TestFeeEntryWriteToIsRecordedIn pins the two halves against each other: if +// recordedIn stops accepting what writeTo produces the skip silently never +// fires, and if it accepts more the credit can be skipped without ever having +// been written. +func TestFeeEntryWriteToIsRecordedIn(t *testing.T) { + t.Parallel() + version := state.Version{TxIndex: 3, Incarnation: 1} + addr := fAddr("credited") + + entries := []*feeEntry{ + { + addr: addr, + acc: accounts.Account{Balance: *uint256.NewInt(7), Nonce: 2, Incarnation: 1, CodeHash: accounts.EmptyCodeHash}, + reason: tracing.BalanceIncreaseRewardTransactionFee, + }, + { + addr: addr, + acc: accounts.Account{Balance: *uint256.NewInt(11), CodeHash: accounts.EmptyCodeHash}, + reason: tracing.BalanceDecreaseGasBuy, + }, + {addr: addr, deleted: true}, + } + + for i, e := range entries { + ws := &state.WriteSet{} + e.writeTo(ws, version) + + require.True(t, e.recordedIn(ws, version), + "entry %d: recordedIn must accept what writeTo wrote", i) + require.False(t, e.recordedIn(ws, state.Version{TxIndex: 3, Incarnation: 2}), + "entry %d: a credit stamped at another incarnation is not this credit", i) + require.False(t, e.recordedIn(&state.WriteSet{}, version), + "entry %d: an empty set carries no credit", i) + } +} + +// TestFeeEntryNilIsAbsent pins the absent entry: an adjustment that does not +// touch this address writes nothing and reads as already recorded, so the skip +// turns on the entries that do exist. +func TestFeeEntryNilIsAbsent(t *testing.T) { + t.Parallel() + var absent *feeEntry + ws := &state.WriteSet{} + + absent.writeTo(ws, state.Version{TxIndex: 3}) + require.True(t, ws.IsEmpty(), "an absent entry has nothing to write") + require.True(t, absent.recordedIn(ws, state.Version{TxIndex: 3})) +} + +// TestFeeEntryDeletedMatchesAnyWriterAtThisVersion pins what the deleted arm +// accepts. It keys on version and value only, so a worker's own SELFDESTRUCT at +// this version reads as the credit — harmless, because the entry writes that +// same delete. +func TestFeeEntryDeletedMatchesAnyWriterAtThisVersion(t *testing.T) { + t.Parallel() + addr := fAddr("emptied") + version := state.Version{TxIndex: 3, Incarnation: 1} + e := &feeEntry{addr: addr, deleted: true} + + selfDestruct := func(v state.Version, val bool) *state.WriteSet { + ws := &state.WriteSet{} + ws.SetSelfDestruct(addr, &state.VersionedWrite[bool]{ + WriteHeader: state.WriteHeader{Address: addr, Path: state.SelfDestructPath, Version: v}, + Val: val, + }) + return ws + } + + require.False(t, e.recordedIn(selfDestruct(state.Version{TxIndex: 3}, true), version), + "a delete stamped at another incarnation is not this credit") + require.False(t, e.recordedIn(selfDestruct(version, false), version), + "a SelfDestruct write that is not a delete carries no empty-removal") + require.True(t, e.recordedIn(selfDestruct(version, true), version), + "any delete at this version counts, whichever writer produced it") +} + +var feeCreditSink *state.WriteSet + +func BenchmarkCalcFees(b *testing.B) { + for _, bc := range []struct { + name string + recredit bool + }{ + {"first_credit", false}, + {"redundant_recredit", true}, + } { + b.Run(bc.name, func(b *testing.B) { + r := newFeeCreditRound(b, simpleTransferScenario()) + var credited *state.WriteSet + if bc.recredit { + require.NotNil(b, r.run(b)) + credited = r.credited + } + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + tip, err := r.result.calcFees(r.task, r.vm, r.reader, r.rules, credited) + if err != nil { + b.Fatal(err) + } + feeCreditSink = tip + // The apply loop recycles the emitted set's maps through + // recordFeeMerge; without this the pools stay empty and the + // emit arm is measured against a permanently cold pool. + tip.ReleaseMaps() + } + }) + } +} diff --git a/execution/stagedsync/exec3_parallel.go b/execution/stagedsync/exec3_parallel.go index fa00620e13b..27b1e85fa54 100644 --- a/execution/stagedsync/exec3_parallel.go +++ b/execution/stagedsync/exec3_parallel.go @@ -1907,6 +1907,7 @@ func (result *execResult) calcFees( vm *state.VersionMap, stateReader state.StateReader, chainRules *chain.Rules, + credited *state.WriteSet, ) (*state.WriteSet, error) { txIndex := task.Version().TxIndex taskVersion := task.Version() @@ -2002,92 +2003,120 @@ func (result *execResult) calcFees( // and normalizeWriteSet's sdSet filter drops them. coinbaseEmptyPre := (coinbaseAcc == nil || coinbaseAcc.Balance.IsZero()) && coinbaseNonce == 0 && coinbaseEmptyCodeHash && !coinbaseHasCodeHashWrite - emitCoinbase := newCoinbaseBalance != oldCoinbaseBalance || - (coinbaseEmptyRemoval && coinbaseEmptyPre && newCoinbaseBalance.IsZero()) + coinbaseEmptied := coinbaseEmptyRemoval && coinbaseEmptyPre && newCoinbaseBalance.IsZero() + emitCoinbase := newCoinbaseBalance != oldCoinbaseBalance || coinbaseEmptied if !emitCoinbase && !emitBurnt { return nil, nil } - addWrites := &state.WriteSet{} + var coinbaseEntry, burntEntry *feeEntry + if emitCoinbase { + coinbaseEntry = &feeEntry{addr: result.Coinbase, deleted: coinbaseEmptied} + if !coinbaseEmptied { + coinbaseEntry.acc = feeAddressAccount(coinbaseAcc, newCoinbaseBalance, coinbaseNonce) + coinbaseEntry.reason = tracing.BalanceIncreaseRewardTransactionFee + } + } + if emitBurnt { + burntEntry = &feeEntry{ + addr: burntAddr, + acc: feeAddressAccount(burntAcc, newBurntBalance, 0), + reason: tracing.BalanceDecreaseGasBuy, + } + } + // The apply loop re-credits a tx once per validation round, and the credit + // only moves when a prior tx's writes moved under it. An unchanged credit + // would rebuild a set identical to the one already recorded — including the + // CollectorWrites entries an earlier round already put there. A coinbase that + // is also the burnt address never matches: one write cannot carry both + // tracing reasons, so those blocks pay the full rebuild every round. + if coinbaseEntry.recordedIn(credited, taskVersion) && burntEntry.recordedIn(credited, taskVersion) { + return nil, nil + } + if emitCoinbase { result.CollectorWrites = result.CollectorWrites.SetAccountBalanceOrDelete( result.Coinbase, coinbaseAcc, newCoinbaseBalance, tracing.BalanceIncreaseRewardTransactionFee, coinbaseEmptyRemoval) - if coinbaseEmptyRemoval && coinbaseEmptyPre && newCoinbaseBalance.IsZero() { - addWrites.SetSelfDestruct(result.Coinbase, &state.VersionedWrite[bool]{ - WriteHeader: state.WriteHeader{ - Address: result.Coinbase, - Path: state.SelfDestructPath, - Version: taskVersion, - }, - Val: true, - }) - } else { - addWrites.SetBalance(result.Coinbase, &state.VersionedWrite[uint256.Int]{ - WriteHeader: state.WriteHeader{ - Address: result.Coinbase, - Path: state.BalancePath, - Version: taskVersion, - Reason: tracing.BalanceIncreaseRewardTransactionFee, - }, - Val: newCoinbaseBalance, - }) - // Emit an AddressPath sibling write so downstream parallel txs - // reading this address see an account record. Serial's AddBalance - // implicitly creates the account on first credit; parallel calcFees - // must mirror that, otherwise getVersionedAccount returns nil for - // a freshly-credited coinbase (no pre-block storage entry, no - // versionMap AddressPath) and Empty() returns true — charging the - // stale CallNewAccountGas (+25000) for a CALL-with-value to the - // coinbase mid-tx. Mainnet block 25151825 tx 31's SD+CREATE2-on- - // coinbase MEV pattern surfaced this divergence. - addrAcc := feeAddressAccount(coinbaseAcc, newCoinbaseBalance, coinbaseNonce) - addWrites.SetAddress(result.Coinbase, &state.VersionedWrite[*accounts.Account]{ - WriteHeader: state.WriteHeader{ - Address: result.Coinbase, - Path: state.AddressPath, - Version: taskVersion, - }, - Val: addrAcc, - }) - } } if emitBurnt { result.CollectorWrites = result.CollectorWrites.SetAccountBalanceOrDelete( burntAddr, burntAcc, newBurntBalance, tracing.BalanceDecreaseGasBuy, state.EIP161EmptyRemoval(chainRules.IsEIP161Enabled(), chainRules.IsAura, burntAddr)) - addWrites.SetBalance(burntAddr, &state.VersionedWrite[uint256.Int]{ - WriteHeader: state.WriteHeader{ - Address: burntAddr, - Path: state.BalancePath, - Version: taskVersion, - Reason: tracing.BalanceDecreaseGasBuy, - }, - Val: newBurntBalance, - }) - // Mirror the AddressPath emission above for the burnt address. - burntAddrAcc := feeAddressAccount(burntAcc, newBurntBalance, 0) - addWrites.SetAddress(burntAddr, &state.VersionedWrite[*accounts.Account]{ - WriteHeader: state.WriteHeader{ - Address: burntAddr, - Path: state.AddressPath, - Version: taskVersion, - }, - Val: burntAddrAcc, - }) } + addWrites := &state.WriteSet{} + coinbaseEntry.writeTo(addWrites, taskVersion) + burntEntry.writeTo(addWrites, taskVersion) + return addWrites, nil } +// feeEntry is one address's share of a fee adjustment: the post-adjustment +// account, whose Balance is also the BalancePath value, or a delete when EIP-161 +// removes the emptied account instead. A nil *feeEntry is an address this +// adjustment does not touch. +type feeEntry struct { + addr accounts.Address + acc accounts.Account + reason tracing.BalanceChangeReason + deleted bool +} + +// recordedIn reports whether ws already carries this entry verbatim. Reason +// fences the balance arm from a worker's own balance write. The deleted arm has +// no fence: a worker's SELFDESTRUCT carries the same version and reads as +// recorded, which is harmless because the entry writes that identical delete. +func (e *feeEntry) recordedIn(ws *state.WriteSet, version state.Version) bool { + if e == nil { + return true + } + if e.deleted { + sd, ok := ws.GetSelfDestruct(e.addr) + return ok && sd.Val && sd.Version == version + } + bw, ok := ws.GetBalance(e.addr) + if !ok || bw.Val != e.acc.Balance || bw.Version != version || bw.Reason != e.reason { + return false + } + aw, ok := ws.GetAddress(e.addr) + return ok && aw.Val != nil && aw.Val.Equals(&e.acc) && aw.Version == version +} + +func (e *feeEntry) writeTo(ws *state.WriteSet, version state.Version) { + if e == nil { + return + } + if e.deleted { + ws.SetSelfDestruct(e.addr, &state.VersionedWrite[bool]{ + WriteHeader: state.WriteHeader{Address: e.addr, Path: state.SelfDestructPath, Version: version}, + Val: true, + }) + return + } + ws.SetBalance(e.addr, &state.VersionedWrite[uint256.Int]{ + WriteHeader: state.WriteHeader{Address: e.addr, Path: state.BalancePath, Version: version, Reason: e.reason}, + Val: e.acc.Balance, + }) + // The AddressPath sibling mirrors serial's AddBalance creating the account + // on first credit. Without it getVersionedAccount returns nil for a + // freshly-credited address and Empty() is true, charging the stale + // CallNewAccountGas for a CALL-with-value to the coinbase mid-tx. + acc := e.acc + ws.SetAddress(e.addr, &state.VersionedWrite[*accounts.Account]{ + WriteHeader: state.WriteHeader{Address: e.addr, Path: state.AddressPath, Version: version}, + Val: &acc, + }) +} + // feeAddressAccount builds the AddressPath value for a fee credit from read, the // address's pre-credit account. nonce applies only when there is no pre-state. -func feeAddressAccount(read *accounts.Account, balance uint256.Int, nonce uint64) *accounts.Account { +func feeAddressAccount(read *accounts.Account, balance uint256.Int, nonce uint64) accounts.Account { if read == nil { - return &accounts.Account{Balance: balance, Nonce: nonce, CodeHash: accounts.EmptyCodeHash} + return accounts.Account{Balance: balance, Nonce: nonce, CodeHash: accounts.EmptyCodeHash} } - return &accounts.Account{Balance: balance, Nonce: read.Nonce, Incarnation: read.Incarnation, CodeHash: read.CodeHash} + return accounts.Account{Balance: balance, Nonce: read.Nonce, Incarnation: read.Incarnation, CodeHash: read.CodeHash} } func (result *execResult) finalizeTx( @@ -2264,10 +2293,10 @@ type blockExecutor struct { tasks []*execTask results []*execResult - // feeMergeTemp[tx] is the write set the fee merge created and recorded for - // tx. A revalidation round merges again and supersedes it; every other + // feeMergeTemp[txIndex] is the write set the fee merge created and recorded + // for a tx. A revalidation round merges again and supersedes it; every other // recorded set is some execResult's TxOut, which stays live. - feeMergeTemp map[int]*state.WriteSet + feeMergeTemp map[int]feeMerge mapReleasing sync.WaitGroup @@ -2417,7 +2446,7 @@ func newBlockExec(blockNum uint64, blockHash common.Hash, gasPool *protocol.GasP begin: time.Now(), stats: map[int]ExecutionStat{}, finalizedResults: map[int]*execResult{}, - feeMergeTemp: map[int]*state.WriteSet{}, + feeMergeTemp: map[int]feeMerge{}, settledInput: map[int]bool{}, estimateDeps: map[int][]int{}, preValidated: map[int]bool{}, @@ -2447,16 +2476,53 @@ func (be *blockExecutor) invalidBlockResult(err error) *blockResult { } } -// recordFeeMerge takes ownership of the set the fee merge just recorded for tx -// and reclaims the one it superseded. Only a set an earlier fee merge created -// may be released: prev is otherwise some execResult's TxOut, which stays live. +// feeMerge is a recorded fee-merge product pinned to the tx version whose +// credit it carries. The pin is what keeps a credit from outliving the +// incarnation it was computed for. +type feeMerge struct { + writes *state.WriteSet + version state.Version +} + +// recordWorkerWrites installs a worker result's write set and drops the fee +// credit along with it: the credit belonged to the incarnation this result +// supersedes, and crediting is what the apply loop does next. The displaced +// merge product is feeMergeTemp's to reclaim — every other recorded set is some +// execResult's TxOut, which stays live. +func (be *blockExecutor) recordWorkerWrites(txVersion state.Version, writes *state.WriteSet) { + be.blockIO.RecordWrites(txVersion, writes) + if temp, ok := be.feeMergeTemp[txVersion.TxIndex]; ok { + be.queueMapRelease(temp.writes) + delete(be.feeMergeTemp, txVersion.TxIndex) + } +} + +// creditedWrites returns ws when an earlier fee merge produced it for this tx +// version, which makes it the one set already carrying the version's fee +// credit. Anything else is the worker's own output, which calcFees reads as the +// pre-credit balance. +func (be *blockExecutor) creditedWrites(txVersion state.Version, ws *state.WriteSet) *state.WriteSet { + if temp, ok := be.feeMergeTemp[txVersion.TxIndex]; ok && temp.writes == ws && temp.version == txVersion { + return ws + } + return nil +} + +// recordFeeMerge folds a fee credit into the tx's recorded write set and +// reclaims the set it supersedes. Only a previous fee-merge product may be +// released: prev is otherwise some execResult's TxOut, which stays live. // MergeInto shares VersionedWrite pointers rather than the maps holding them, -// so pooling prev's maps leaves the writes merged now holds intact. -func (be *blockExecutor) recordFeeMerge(tx int, prev, merged *state.WriteSet) { - if temp := be.feeMergeTemp[tx]; temp != nil && temp == prev && merged != temp { - be.queueMapRelease(temp) +// so pooling those maps leaves the merged writes intact. +func (be *blockExecutor) recordFeeMerge(txVersion state.Version, prev, tipWrites *state.WriteSet) { + if tipWrites.IsEmpty() { + return } - be.feeMergeTemp[tx] = merged + merged := prev.MergeInto(tipWrites) + be.blockIO.RecordWrites(txVersion, merged) + if temp, ok := be.feeMergeTemp[txVersion.TxIndex]; ok && temp.writes == prev { + be.queueMapRelease(temp.writes) + } + be.feeMergeTemp[txVersion.TxIndex] = feeMerge{writes: merged, version: txVersion} } // ReleaseMaps clears every map before pooling it, which is O(entries), and a @@ -2633,7 +2699,7 @@ func (be *blockExecutor) nextResult(ctx context.Context, pe *parallelExecutor, r be.blockIO.RecordAccesses(txVersion, res.AccessedAddresses) if res.Version().Incarnation == 0 { - be.blockIO.RecordWrites(txVersion, res.TxOut) + be.recordWorkerWrites(txVersion, res.TxOut) } else { prevWrites := be.blockIO.WriteSet(txVersion.TxIndex) hasWriteChange := res.TxOut.HasNewWrite(prevWrites) @@ -2647,7 +2713,7 @@ func (be *blockExecutor) nextResult(ctx context.Context, pe *parallelExecutor, r } } - be.blockIO.RecordWrites(txVersion, res.TxOut) + be.recordWorkerWrites(txVersion, res.TxOut) if hasWriteChange { be.validateTasks.pushPendingSet(be.execTasks.getRevalidationRange(tx + 1)) @@ -2722,16 +2788,13 @@ func (be *blockExecutor) nextResult(ctx context.Context, pe *parallelExecutor, r stateReader = state.NewCurrentCachedReaderV3(pe.rs.Domains().AsGetter(applyTx), be.blockStateCache) } } - tipWrites, err := txResult.calcFees(taskVer, be.versionMap, stateReader, txTask.Rules()) + existingWrites := be.blockIO.WriteSet(txVersion.TxIndex) + tipWrites, err := txResult.calcFees(taskVer, be.versionMap, stateReader, txTask.Rules(), + be.creditedWrites(txVersion, existingWrites)) if err != nil { return nil, err } - if !tipWrites.IsEmpty() { - existingWrites := be.blockIO.WriteSet(txVersion.TxIndex) - merged := existingWrites.MergeInto(tipWrites) - be.blockIO.RecordWrites(txVersion, merged) - be.recordFeeMerge(tx, existingWrites, merged) - } + be.recordFeeMerge(txVersion, existingWrites, tipWrites) } validity := be.versionMap.ValidateVersion(txVersion.TxIndex, be.blockIO,