From 063097b04578a22dfca6e9bb8de25b8f1f332af6 Mon Sep 17 00:00:00 2001 From: Julian Meyer Date: Mon, 1 Jun 2026 10:31:00 -0700 Subject: [PATCH] fix(simulator): OpcodeStats Add/Sub must use union of keys Before this change, both OpcodeStats.Add and OpcodeStats.Sub iterated only the 'other' argument: for opcode, count := range other { result[opcode] = o[opcode] + count } This silently dropped any key present in the receiver 'o' but absent from 'other'. For Add(empty) the result was an empty map, losing 'o' entirely. Concrete consequence in sendTxs: per-tx blockCounts is computed as blockCounts = expected.Sub(actual).Round() where 'expected' is base x Mul(numCalls+1 x scaleFactor) (carrying all base opcodes + precompiles) and 'actual' starts empty for the first tx. With the buggy Sub iterating actual=empty, blockCounts.Opcodes and blockCounts.Precompiles were ALWAYS empty for every tx in every block. The downstream effect: workloads with precompile counts in their config (e.g. base-mainnet-simulation: bls12381MapG2, ecrecover, bls12381G1Add, bls12381G1MultiExp) had per-tx execution that skipped all precompile work. Each tx therefore consumed far less gas than the gas estimate predicted, and blocks under-filled the gas budget. Example: base-mainnet-simulation @ GasLimit=250M (block_time=1s) reported gas/per_block ~51M (20% of limit) in production runs because per-tx skipped the heaviest workload component. Fix: both Add and Sub now iterate the union of keys. For Sub, a key present only in 'other' produces a negative result entry (matches arithmetic semantics). Includes unit tests for the union semantics and a regression test for the first-tx scenario in sendTxs. --- .../payload/simulator/simulatorstats/types.go | 14 +++- .../simulator/simulatorstats/types_test.go | 76 +++++++++++++++++++ 2 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 runner/payload/simulator/simulatorstats/types_test.go diff --git a/runner/payload/simulator/simulatorstats/types.go b/runner/payload/simulator/simulatorstats/types.go index 0285b908..b65b7198 100644 --- a/runner/payload/simulator/simulatorstats/types.go +++ b/runner/payload/simulator/simulatorstats/types.go @@ -27,9 +27,12 @@ func (o OpcodeStats) Round() OpcodeStats { } func (o OpcodeStats) Add(other OpcodeStats) OpcodeStats { - result := make(OpcodeStats) + result := make(OpcodeStats, len(o)+len(other)) + for opcode, count := range o { + result[opcode] = count + } for opcode, count := range other { - result[opcode] = o[opcode] + count + result[opcode] += count } return result } @@ -43,9 +46,12 @@ func (o OpcodeStats) Pow(n float64) OpcodeStats { } func (o OpcodeStats) Sub(other OpcodeStats) OpcodeStats { - result := make(OpcodeStats) + result := make(OpcodeStats, len(o)+len(other)) + for opcode, count := range o { + result[opcode] = count + } for opcode, count := range other { - result[opcode] = o[opcode] - count + result[opcode] -= count } return result } diff --git a/runner/payload/simulator/simulatorstats/types_test.go b/runner/payload/simulator/simulatorstats/types_test.go new file mode 100644 index 00000000..0d2b0ea1 --- /dev/null +++ b/runner/payload/simulator/simulatorstats/types_test.go @@ -0,0 +1,76 @@ +package simulatorstats + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestOpcodeStatsAdd_UnionOfKeys(t *testing.T) { + a := OpcodeStats{"A": 1, "B": 2} + b := OpcodeStats{"B": 10, "C": 100} + + got := a.Add(b) + + require.Equal(t, 1.0, got["A"], "key only in receiver must be preserved") + require.Equal(t, 12.0, got["B"], "shared key must sum") + require.Equal(t, 100.0, got["C"], "key only in arg must be preserved") + require.Len(t, got, 3) +} + +func TestOpcodeStatsAdd_EmptyOther(t *testing.T) { + a := OpcodeStats{"A": 1, "B": 2} + got := a.Add(OpcodeStats{}) + require.Equal(t, 1.0, got["A"]) + require.Equal(t, 2.0, got["B"]) + require.Len(t, got, 2) +} + +func TestOpcodeStatsAdd_EmptyReceiver(t *testing.T) { + got := OpcodeStats{}.Add(OpcodeStats{"A": 1, "B": 2}) + require.Equal(t, 1.0, got["A"]) + require.Equal(t, 2.0, got["B"]) + require.Len(t, got, 2) +} + +func TestOpcodeStatsSub_UnionOfKeys(t *testing.T) { + a := OpcodeStats{"A": 10, "B": 20} + b := OpcodeStats{"B": 5, "C": 100} + + got := a.Sub(b) + + require.Equal(t, 10.0, got["A"], "key only in receiver must be preserved") + require.Equal(t, 15.0, got["B"], "shared key must subtract") + require.Equal(t, -100.0, got["C"], "key only in arg must be included (negated)") + require.Len(t, got, 3) +} + +func TestOpcodeStatsSub_EmptyOther(t *testing.T) { + a := OpcodeStats{"A": 10, "B": 20} + got := a.Sub(OpcodeStats{}) + require.Equal(t, 10.0, got["A"]) + require.Equal(t, 20.0, got["B"]) + require.Len(t, got, 2) +} + +func TestStatsSubAdd_FirstTxBlockCountsIncludePrecompiles(t *testing.T) { + base := &Stats{ + Precompiles: OpcodeStats{"ecrecover": 0.5, "bls12381MapG2": 1.0}, + Opcodes: OpcodeStats{"KECCAK256": 10.0}, + } + + expected := base.Mul(1.0) + actual := NewStats() + + blockCounts := expected.Sub(actual).Round() + + require.Equal(t, 1.0, blockCounts.Precompiles["ecrecover"], + "precompiles missing in blockCounts means worker txs skip precompile execution") + require.Equal(t, 1.0, blockCounts.Precompiles["bls12381MapG2"]) + require.Equal(t, 10.0, blockCounts.Opcodes["KECCAK256"]) + + actual = actual.Add(blockCounts) + require.Equal(t, 1.0, actual.Precompiles["ecrecover"], + "accumulated actual must remember the keys we added") + require.Equal(t, 1.0, actual.Precompiles["bls12381MapG2"]) +}