diff --git a/changelog/jcolvin-classic-ci.md b/changelog/jcolvin-classic-ci.md new file mode 100644 index 0000000000..551c7af719 --- /dev/null +++ b/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 diff --git a/execution/gethexec/classicMessage.go b/execution/gethexec/classicMessage.go index 152910d780..b243c07a2f 100644 --- a/execution/gethexec/classicMessage.go +++ b/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{} diff --git a/execution/gethexec/classicMessage_test.go b/execution/gethexec/classicMessage_test.go new file mode 100644 index 0000000000..0cdfa6bbcb --- /dev/null +++ b/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) + } + }) + } +} diff --git a/execution/gethexec/classicMessage_testhelpers.go b/execution/gethexec/classicMessage_testhelpers.go new file mode 100644 index 0000000000..e00cd2ca7b --- /dev/null +++ b/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 +} diff --git a/execution/gethexec/node.go b/execution/gethexec/node.go index ef09d1e1d6..32fd4ad015 100644 --- a/execution/gethexec/node.go +++ b/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 } } diff --git a/system_tests/classic_msg_test.go b/system_tests/classic_msg_test.go new file mode 100644 index 0000000000..21f0fd67b3 --- /dev/null +++ b/system_tests/classic_msg_test.go @@ -0,0 +1,80 @@ +// Copyright 2024-2026, Offchain Labs, Inc. +// For license information, see https://github.com/OffchainLabs/nitro/blob/master/LICENSE.md + +package arbtest + +import ( + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/core/rawdb" + "github.com/ethereum/go-ethereum/node" + + "github.com/offchainlabs/nitro/execution/gethexec" + "github.com/offchainlabs/nitro/util/testhelpers" +) + +// TestOpenClassicOutboxFromStack verifies that OpenClassicOutboxFromStack +// correctly opens a pre-existing classic-msg database with NoFreezer and +// ReadOnly options, and that data is readable through the ClassicOutboxRetriever. +// This exercises the same production code path used by CreateExecutionNode. +func TestOpenClassicOutboxFromStack(t *testing.T) { + t.Parallel() + dataDir := t.TempDir() + stackConfig := testhelpers.CreateStackConfigForTest(dataDir) + stackConfig.DBEngine = rawdb.DBPebble + + // Create and populate the classic-msg database + stack, err := node.New(stackConfig) + Require(t, err) + + db, err := stack.OpenDatabaseWithOptions("classic-msg", node.DatabaseOptions{ + Cache: 16, + Handles: 16, + NoFreezer: true, + }) + Require(t, err) + + leaves := [][]byte{[]byte("classic-outbox-test-msg")} + root, merkleSize, err := gethexec.BuildClassicMerkleTree(db, leaves) + Require(t, err) + Require(t, gethexec.WriteClassicBatchHeader(db, big.NewInt(0), root, merkleSize)) + db.Close() + stack.Close() + + // Reopen through the production function + stack2, err := node.New(stackConfig) + Require(t, err) + defer stack2.Close() + + retriever, err := gethexec.OpenClassicOutboxFromStack(stack2) + Require(t, err) + if retriever == nil { + t.Fatal("OpenClassicOutboxFromStack returned nil for existing database") + } + + msg, err := retriever.GetMsg(big.NewInt(0), 0) + Require(t, err) + if string(msg.Data) != "classic-outbox-test-msg" { + t.Errorf("GetMsg data = %q, want %q", msg.Data, "classic-outbox-test-msg") + } +} + +// TestOpenClassicOutboxFromStackMissing verifies that OpenClassicOutboxFromStack +// returns nil (not an error) when the classic-msg database does not exist. +func TestOpenClassicOutboxFromStackMissing(t *testing.T) { + t.Parallel() + dataDir := t.TempDir() + stackConfig := testhelpers.CreateStackConfigForTest(dataDir) + stackConfig.DBEngine = rawdb.DBPebble + + stack, err := node.New(stackConfig) + Require(t, err) + defer stack.Close() + + retriever, err := gethexec.OpenClassicOutboxFromStack(stack) + Require(t, err) + if retriever != nil { + t.Fatal("OpenClassicOutboxFromStack should return nil when database does not exist") + } +}