Add regression tests for arbitrum classic database

changelog/jcolvin-classic-ci.md

@@ -0,0 +1,2 @@
+### Ignored
+- Add regression tests for classic database and fix off-by-one error that would just error in a different way

execution/gethexec/classicMessage.go

@@ -13,12 +13,40 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/ethdb"
+	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/node"
+
+	"github.com/offchainlabs/nitro/util/dbutil"
 )
 
 type ClassicOutboxRetriever struct {
 	db ethdb.Database
 }
 
+// OpenClassicOutboxFromStack opens the classic-msg database from a node stack
+// and returns a ClassicOutboxRetriever, or nil if the database does not exist.
+func OpenClassicOutboxFromStack(stack *node.Node) (*ClassicOutboxRetriever, error) {
+	classicMsgDB, err := stack.OpenDatabaseWithOptions("classic-msg", node.DatabaseOptions{
+		MetricsNamespace: "classicmsg/",
+		Cache:            0, // will be sanitized to minimum
+		Handles:          0, // will be sanitized to minimum
+		ReadOnly:         true,
+		NoFreezer:        true,
+	})
+	if dbutil.IsNotExistError(err) {
+		log.Warn("Classic Msg Database not found", "err", err)
+		return nil, nil
+	}
+	if err != nil {
+		return nil, fmt.Errorf("Failed to open classic-msg database: %w", err)
+	}
+	if err := dbutil.UnfinishedConversionCheck(classicMsgDB); err != nil {
+		classicMsgDB.Close()
+		return nil, fmt.Errorf("classic-msg unfinished database conversion check error: %w", err)
+	}
+	return NewClassicOutboxRetriever(classicMsgDB), nil
+}
+
 func NewClassicOutboxRetriever(db ethdb.Database) *ClassicOutboxRetriever {
 	return &ClassicOutboxRetriever{
 		db: db,
@@ -47,7 +75,7 @@ func (m *ClassicOutboxRetriever) GetMsg(batchNum *big.Int, index uint64) (*Class
 	lowest := uint64(0)
 	var root common.Hash
 	copy(root[:], batchHeader[8:40])
-	if merkleSize < index {
+	if merkleSize <= index {
 		return nil, fmt.Errorf("batch %d only has %d indexes", batchNum, merkleSize)
 	}
 	proofNodes := [][32]byte{}

execution/gethexec/classicMessage_test.go

@@ -0,0 +1,191 @@
+// Copyright 2024-2026, Offchain Labs, Inc.
+// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md
+
+package gethexec
+
+import (
+	"fmt"
+	"math/big"
+	"testing"
+
+	"github.com/ethereum/go-ethereum/core/rawdb"
+)
+
+func TestClassicOutboxRetrieverGetMsg(t *testing.T) {
+	t.Parallel()
+	db := rawdb.NewMemoryDatabase()
+
+	leaves := [][]byte{
+		[]byte("message-0"),
+		[]byte("message-1"),
+		[]byte("message-2"),
+		[]byte("message-3"),
+	}
+	root, merkleSize, err := BuildClassicMerkleTree(db, leaves)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteClassicBatchHeader(db, big.NewInt(0), root, merkleSize); err != nil {
+		t.Fatal(err)
+	}
+
+	retriever := NewClassicOutboxRetriever(db)
+
+	for i, expected := range leaves {
+		msg, err := retriever.GetMsg(big.NewInt(0), uint64(i)) //#nosec G115
+		if err != nil {
+			t.Fatalf("GetMsg(batch=0, index=%d) error: %v", i, err)
+		}
+		if string(msg.Data) != string(expected) {
+			t.Errorf("GetMsg(batch=0, index=%d) data = %q, want %q", i, msg.Data, expected)
+		}
+		if msg.PathInt == nil {
+			t.Errorf("GetMsg(batch=0, index=%d) PathInt is nil", i)
+		}
+		// 4-leaf tree has depth 2, so proof should have 2 sibling nodes
+		if len(msg.ProofNodes) != 2 {
+			t.Errorf("GetMsg(batch=0, index=%d) proof length = %d, want 2", i, len(msg.ProofNodes))
+		}
+	}
+}
+
+func TestClassicOutboxRetrieverSingleLeaf(t *testing.T) {
+	t.Parallel()
+	db := rawdb.NewMemoryDatabase()
+
+	leaves := [][]byte{[]byte("only-message")}
+	root, merkleSize, err := BuildClassicMerkleTree(db, leaves)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteClassicBatchHeader(db, big.NewInt(1), root, merkleSize); err != nil {
+		t.Fatal(err)
+	}
+
+	retriever := NewClassicOutboxRetriever(db)
+	msg, err := retriever.GetMsg(big.NewInt(1), 0)
+	if err != nil {
+		t.Fatalf("GetMsg error: %v", err)
+	}
+	if string(msg.Data) != "only-message" {
+		t.Errorf("data = %q, want %q", msg.Data, "only-message")
+	}
+	// Single leaf: no merkle traversal needed, so no proof nodes
+	if len(msg.ProofNodes) != 0 {
+		t.Errorf("proof length = %d, want 0", len(msg.ProofNodes))
+	}
+}
+
+func TestClassicOutboxRetrieverNonPowerOfTwoLeaves(t *testing.T) {
+	t.Parallel()
+	db := rawdb.NewMemoryDatabase()
+
+	// 3 leaves exercises the non-power-of-two branch (bits.OnesCount64 != 1)
+	leaves := [][]byte{
+		[]byte("leaf-0"),
+		[]byte("leaf-1"),
+		[]byte("leaf-2"),
+	}
+	root, merkleSize, err := BuildClassicMerkleTree(db, leaves)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteClassicBatchHeader(db, big.NewInt(0), root, merkleSize); err != nil {
+		t.Fatal(err)
+	}
+
+	retriever := NewClassicOutboxRetriever(db)
+	for i, expected := range leaves {
+		msg, err := retriever.GetMsg(big.NewInt(0), uint64(i)) //#nosec G115
+		if err != nil {
+			t.Fatalf("GetMsg(index=%d) error: %v", i, err)
+		}
+		if string(msg.Data) != string(expected) {
+			t.Errorf("GetMsg(index=%d) data = %q, want %q", i, msg.Data, expected)
+		}
+	}
+}
+
+func TestClassicOutboxRetrieverErrors(t *testing.T) {
+	t.Parallel()
+	db := rawdb.NewMemoryDatabase()
+
+	leaves := [][]byte{[]byte("msg-0"), []byte("msg-1")}
+	root, merkleSize, err := BuildClassicMerkleTree(db, leaves)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := WriteClassicBatchHeader(db, big.NewInt(0), root, merkleSize); err != nil {
+		t.Fatal(err)
+	}
+
+	retriever := NewClassicOutboxRetriever(db)
+
+	// Non-existent batch
+	_, err = retriever.GetMsg(big.NewInt(99), 0)
+	if err == nil {
+		t.Error("expected error for non-existent batch, got nil")
+	}
+
+	// Index out of range
+	_, err = retriever.GetMsg(big.NewInt(0), 999)
+	if err == nil {
+		t.Error("expected error for out-of-range index, got nil")
+	}
+
+	// Index exactly equal to merkleSize (one past last valid index).
+	// Valid indices for merkleSize=2 are 0 and 1; index 2 should be rejected.
+	_, err = retriever.GetMsg(big.NewInt(0), merkleSize)
+	if err == nil {
+		t.Errorf("expected error for index == merkleSize (%d), got nil", merkleSize)
+	}
+}
+
+func TestClassicOutboxRetrieverBoundaryIndex(t *testing.T) {
+	t.Parallel()
+	// Test the boundary between valid and invalid indices across different tree sizes.
+	// The last valid index is merkleSize-1; merkleSize itself must be rejected.
+	treeSizes := []int{1, 2, 3, 4, 5, 7, 8}
+	for _, size := range treeSizes {
+		size := size
+		t.Run(fmt.Sprintf("size-%d", size), func(t *testing.T) {
+			t.Parallel()
+			db := rawdb.NewMemoryDatabase()
+			leaves := make([][]byte, size)
+			for i := range leaves {
+				leaves[i] = []byte(fmt.Sprintf("leaf-%d", i))
+			}
+			root, merkleSize, err := BuildClassicMerkleTree(db, leaves)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err := WriteClassicBatchHeader(db, big.NewInt(0), root, merkleSize); err != nil {
+				t.Fatal(err)
+			}
+			retriever := NewClassicOutboxRetriever(db)
+
+			// Last valid index should succeed
+			lastValid := merkleSize - 1
+			msg, err := retriever.GetMsg(big.NewInt(0), lastValid)
+			if err != nil {
+				t.Fatalf("GetMsg(index=%d) should succeed for merkleSize=%d, got: %v", lastValid, merkleSize, err)
+			}
+			expected := fmt.Sprintf("leaf-%d", lastValid)
+			if string(msg.Data) != expected {
+				t.Errorf("GetMsg(index=%d) data = %q, want %q", lastValid, msg.Data, expected)
+			}
+
+			// First invalid index (== merkleSize) should fail
+			_, err = retriever.GetMsg(big.NewInt(0), merkleSize)
+			if err == nil {
+				t.Errorf("GetMsg(index=%d) should fail for merkleSize=%d", merkleSize, merkleSize)
+			}
+
+			// One beyond that should also fail
+			_, err = retriever.GetMsg(big.NewInt(0), merkleSize+1)
+			if err == nil {
+				t.Errorf("GetMsg(index=%d) should fail for merkleSize=%d", merkleSize+1, merkleSize)
+			}
+		})
+	}
+}

execution/gethexec/classicMessage_testhelpers.go

@@ -0,0 +1,64 @@
+// Copyright 2024-2026, Offchain Labs, Inc.
+// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md
+
+package gethexec
+
+import (
+	"encoding/binary"
+	"fmt"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/ethdb"
+)
+
+// BuildClassicMerkleTree populates db with a binary merkle tree over the given leaf data.
+// Returns the root hash and the number of leaves (merkleSize).
+// Exported for use in tests across packages.
+func BuildClassicMerkleTree(db ethdb.KeyValueWriter, leaves [][]byte) (common.Hash, uint64, error) {
+	if len(leaves) == 0 {
+		return common.Hash{}, 0, fmt.Errorf("BuildClassicMerkleTree requires at least one leaf")
+	}
+	hashes := make([]common.Hash, len(leaves))
+	for i, leaf := range leaves {
+		h := crypto.Keccak256Hash(leaf)
+		hashes[i] = h
+		if err := db.Put(h.Bytes(), leaf); err != nil {
+			return common.Hash{}, 0, fmt.Errorf("failed to store leaf %d: %w", i, err)
+		}
+	}
+	for len(hashes) > 1 {
+		var next []common.Hash
+		for i := 0; i < len(hashes); i += 2 {
+			if i+1 < len(hashes) {
+				var nodeData [64]byte
+				copy(nodeData[0:32], hashes[i].Bytes())
+				copy(nodeData[32:64], hashes[i+1].Bytes())
+				parentHash := crypto.Keccak256Hash(nodeData[:])
+				if err := db.Put(parentHash.Bytes(), nodeData[:]); err != nil {
+					return common.Hash{}, 0, fmt.Errorf("failed to store internal node: %w", err)
+				}
+				next = append(next, parentHash)
+			} else {
+				next = append(next, hashes[i])
+			}
+		}
+		hashes = next
+	}
+	return hashes[0], uint64(len(leaves)), nil
+}
+
+// WriteClassicBatchHeader writes a classic-msg batch header (8-byte merkleSize + 32-byte root)
+// keyed by keccak256("msgBatch" || batchNum.Bytes()).
+// Exported for use in tests across packages.
+func WriteClassicBatchHeader(db ethdb.KeyValueWriter, batchNum *big.Int, root common.Hash, merkleSize uint64) error {
+	key := msgBatchKey(batchNum)
+	header := make([]byte, 40)
+	binary.BigEndian.PutUint64(header[0:8], merkleSize)
+	copy(header[8:40], root.Bytes())
+	if err := db.Put(key, header); err != nil {
+		return fmt.Errorf("failed to write batch header: %w", err)
+	}
+	return nil
+}

execution/gethexec/node.go

@@ -39,7 +39,6 @@ import (
 	"github.com/offchainlabs/nitro/util"
 	"github.com/offchainlabs/nitro/util/arbmath"
 	"github.com/offchainlabs/nitro/util/containers"
-	"github.com/offchainlabs/nitro/util/dbutil"
 	"github.com/offchainlabs/nitro/util/headerreader"
 	"github.com/offchainlabs/nitro/util/rpcclient"
 	"github.com/offchainlabs/nitro/util/rpcserver"
@@ -340,23 +339,10 @@ func CreateExecutionNode(
 	var classicOutbox *ClassicOutboxRetriever
 
 	if l2BlockChain.Config().ArbitrumChainParams.GenesisBlockNum > 0 {
-		classicMsgDB, err := stack.OpenDatabaseWithOptions("classic-msg", node.DatabaseOptions{
-			MetricsNamespace: "classicmsg/",
-			Cache:            0, // will be sanitized to minimum
-			Handles:          0, // will be sanitized to minimum
-			ReadOnly:         true,
-			NoFreezer:        true,
-		})
-		if dbutil.IsNotExistError(err) {
-			log.Warn("Classic Msg Database not found", "err", err)
-			classicOutbox = nil
-		} else if err != nil {
-			return nil, fmt.Errorf("Failed to open classic-msg database: %w", err)
-		} else {
-			if err := dbutil.UnfinishedConversionCheck(classicMsgDB); err != nil {
-				return nil, fmt.Errorf("classic-msg unfinished database conversion check error: %w", err)
-			}
-			classicOutbox = NewClassicOutboxRetriever(classicMsgDB)
+		var err error
+		classicOutbox, err = OpenClassicOutboxFromStack(stack)
+		if err != nil {
+			return nil, err
 		}
 	}