Merge pull request #56 from ethpandaops/fix/overflow-gas-cost

fix(structlog): sanitize corrupted gasCost values from Erigon underflow bug

Author: mattevans

pkg/ethereum/execution/geth/rpc.go

@@ -118,7 +118,7 @@ func (n *RPCNode) traceTransactionErigon(ctx context.Context, hash string, optio
 		Gas:         rsp.Gas,
 		Failed:      rsp.Failed,
 		ReturnValue: returnValue,
-		Structlogs:  []execution.StructLog{},
+		Structlogs:  make([]execution.StructLog, 0, len(rsp.StructLogs)),
 	}
 
 	// Empty array on transfer
@@ -143,6 +143,9 @@ func (n *RPCNode) traceTransactionErigon(ctx context.Context, hash string, optio
 		})
 	}
 
+	// Sanitize gasCost values to fix Erigon's unsigned integer underflow bug.
+	execution.SanitizeStructLogs(result.Structlogs)
+
 	return result, nil
 }
 

pkg/ethereum/execution/sanitize.go

@@ -0,0 +1,27 @@
+package execution
+
+// SanitizeGasCost detects and corrects corrupted gasCost values from Erigon's
+// debug_traceTransaction RPC.
+//
+// Bug: Erigon has an unsigned integer underflow bug in gas.go:callGas() where
+// `availableGas - base` underflows when availableGas < base, producing huge
+// corrupted values (e.g., 18158513697557845033).
+//
+// Detection: gasCost can never legitimately exceed the available gas at that
+// opcode. If gasCost > Gas, the value is corrupted.
+//
+// Correction: Set gasCost = Gas (all available gas consumed), matching Reth's
+// behavior for failed CALL opcodes.
+func SanitizeGasCost(log *StructLog) {
+	if log.GasCost > log.Gas {
+		log.GasCost = log.Gas
+	}
+}
+
+// SanitizeStructLogs applies gas cost sanitization to all structlogs.
+// This corrects corrupted values from Erigon's unsigned integer underflow bug.
+func SanitizeStructLogs(logs []StructLog) {
+	for i := range logs {
+		SanitizeGasCost(&logs[i])
+	}
+}

pkg/ethereum/execution/sanitize_test.go

@@ -0,0 +1,74 @@
+package execution
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSanitizeGasCost(t *testing.T) {
+	tests := []struct {
+		name            string
+		gas             uint64
+		gasCost         uint64
+		expectedGasCost uint64
+	}{
+		{
+			name:            "normal gasCost unchanged",
+			gas:             10000,
+			gasCost:         3,
+			expectedGasCost: 3,
+		},
+		{
+			name:            "gasCost equals gas unchanged",
+			gas:             5058,
+			gasCost:         5058,
+			expectedGasCost: 5058,
+		},
+		{
+			name:            "corrupted gasCost from Erigon underflow bug",
+			gas:             5058,
+			gasCost:         18158513697557845033, // 0xfc00000000001429
+			expectedGasCost: 5058,
+		},
+		{
+			name:            "another corrupted value",
+			gas:             9974,
+			gasCost:         18158513697557850263, // From test case
+			expectedGasCost: 9974,
+		},
+		{
+			name:            "max uint64 corrupted",
+			gas:             1000,
+			gasCost:         ^uint64(0), // max uint64
+			expectedGasCost: 1000,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			log := &StructLog{
+				Gas:     tt.gas,
+				GasCost: tt.gasCost,
+			}
+
+			SanitizeGasCost(log)
+
+			assert.Equal(t, tt.expectedGasCost, log.GasCost)
+		})
+	}
+}
+
+func TestSanitizeStructLogs(t *testing.T) {
+	logs := []StructLog{
+		{Op: "PUSH1", Gas: 10000, GasCost: 3},
+		{Op: "CALL", Gas: 5058, GasCost: 18158513697557845033}, // Corrupted
+		{Op: "STOP", Gas: 100, GasCost: 0},
+	}
+
+	SanitizeStructLogs(logs)
+
+	assert.Equal(t, uint64(3), logs[0].GasCost, "normal gasCost should be unchanged")
+	assert.Equal(t, uint64(5058), logs[1].GasCost, "corrupted gasCost should be sanitized")
+	assert.Equal(t, uint64(0), logs[2].GasCost, "zero gasCost should be unchanged")
+}

pkg/processor/transaction/structlog_agg/aggregator.go

@@ -262,18 +262,25 @@ func (fa *FrameAggregator) Finalize(trace *execution.TraceTransaction, receiptGa
 
 		// Root frame: compute gas refund and intrinsic gas
 		if frameID == 0 {
+			// Check trace.Failed to detect transaction failure from REVERT opcodes.
+			// REVERT executes successfully (no opcode error), but causes transaction failure.
+			// Individual opcode errors (like "out of gas") are already counted via acc.ErrorCount.
+			if trace.Failed && summaryRow.ErrorCount == 0 {
+				summaryRow.ErrorCount = 1
+			}
+
 			// For successful transactions, refunds are applied to the final gas calculation.
 			// For failed transactions, refunds are accumulated during execution but NOT applied
 			// (all gas up to failure is consumed), so we set refund to nil.
-			if acc.ErrorCount == 0 {
+			if summaryRow.ErrorCount == 0 {
 				summaryRow.GasRefund = &acc.MaxRefund
 			}
 
 			// Intrinsic gas is ALWAYS charged (before EVM execution begins), regardless of
 			// whether the transaction succeeds or fails. For failed txs, use refund=0 in
 			// the formula since refunds aren't applied.
 			refundForCalc := acc.MaxRefund
-			if acc.ErrorCount > 0 {
+			if summaryRow.ErrorCount > 0 {
 				refundForCalc = 0
 			}
 

pkg/processor/transaction/structlog_agg/aggregator_test.go

@@ -575,6 +575,54 @@ func TestFrameAggregator_SuccessfulTransaction_HasRefundAndIntrinsic(t *testing.
 	// Note: might be nil if computed value is 0, so we just check the logic is exercised
 }
 
+func TestFrameAggregator_RevertWithoutOpcodeError(t *testing.T) {
+	// Test that REVERT transactions are correctly detected as failed even when
+	// the REVERT opcode itself has no error field set.
+	//
+	// REVERT is a successful opcode execution that causes transaction failure.
+	// Unlike "out of gas" errors where the opcode has an error field, REVERT
+	// executes successfully but reverts state changes. The failure is indicated
+	// by trace.Failed = true, NOT by individual opcode errors.
+	aggregator := NewFrameAggregator()
+
+	// Simulate a transaction that reverts: PUSH1 -> REVERT
+	aggregator.ProcessStructlog(&execution.StructLog{
+		Op:    "PUSH1",
+		Depth: 1,
+		Gas:   50000,
+	}, 0, 0, []uint32{0}, 3, 3, nil, nil)
+
+	// REVERT opcode with NO error field (realistic behavior)
+	aggregator.ProcessStructlog(&execution.StructLog{
+		Op:    "REVERT",
+		Depth: 1,
+		Gas:   49997,
+		// Note: NO Error field set - REVERT executes successfully
+	}, 1, 0, []uint32{0}, 0, 0, nil, &execution.StructLog{Op: "PUSH1", Depth: 1})
+
+	// trace.Failed is true because the transaction reverted
+	trace := &execution.TraceTransaction{
+		Gas:    50000,
+		Failed: true, // This is how REVERT is indicated
+	}
+
+	frames := aggregator.Finalize(trace, 30000)
+
+	assert.Equal(t, 1, countSummaryRows(frames))
+
+	summaryRow := getSummaryRow(frames, 0)
+	require.NotNil(t, summaryRow)
+
+	// Error count MUST be 1 even though no opcode had an error field
+	// This is the key assertion: trace.Failed should set error_count = 1
+	assert.Equal(t, uint64(1), summaryRow.ErrorCount,
+		"ErrorCount should be 1 for REVERT transactions even without opcode errors")
+
+	// GasRefund should be nil for failed transactions
+	assert.Nil(t, summaryRow.GasRefund,
+		"GasRefund should be nil for reverted transactions")
+}
+
 func TestMapOpcodeToCallType(t *testing.T) {
 	tests := []struct {
 		opcode   string