[Audit - TOB-CELOL2-3] Add error checks to migration script (#283)

* addresses audit comments

* fix errors

* addresses feedback

* missed defer statements

op-chain-ops/cmd/celo-migrate/ancients.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 
@@ -38,27 +39,31 @@ func NewChainFreezer(datadir string, namespace string, readonly bool) (*rawdb.Fr
 	return rawdb.NewFreezer(datadir, namespace, readonly, freezerTableSize, chainFreezerNoSnappy)
 }
 
-func migrateAncientsDb(ctx context.Context, oldDBPath, newDBPath string, batchSize, bufferSize uint64) (uint64, uint64, error) {
+func migrateAncientsDb(ctx context.Context, oldDBPath, newDBPath string, batchSize, bufferSize uint64) (numAncientsNewBefore uint64, numAncientsNewAfter uint64, err error) {
 	defer timer("ancients")()
 
 	oldFreezer, err := NewChainFreezer(filepath.Join(oldDBPath, "ancient"), "", false) // Can't be readonly because we need the .meta files to be created
 	if err != nil {
 		return 0, 0, fmt.Errorf("failed to open old freezer: %w", err)
 	}
-	defer oldFreezer.Close()
+	defer func() {
+		err = errors.Join(err, oldFreezer.Close())
+	}()
 
 	newFreezer, err := NewChainFreezer(filepath.Join(newDBPath, "ancient"), "", false)
 	if err != nil {
 		return 0, 0, fmt.Errorf("failed to open new freezer: %w", err)
 	}
-	defer newFreezer.Close()
+	defer func() {
+		err = errors.Join(err, newFreezer.Close())
+	}()
 
 	numAncientsOld, err := oldFreezer.Ancients()
 	if err != nil {
 		return 0, 0, fmt.Errorf("failed to get number of ancients in old freezer: %w", err)
 	}
 
-	numAncientsNewBefore, err := newFreezer.Ancients()
+	numAncientsNewBefore, err = newFreezer.Ancients()
 	if err != nil {
 		return 0, 0, fmt.Errorf("failed to get number of ancients in new freezer: %w", err)
 	}
@@ -84,11 +89,15 @@ func migrateAncientsDb(ctx context.Context, oldDBPath, newDBPath string, batchSi
 		return 0, 0, fmt.Errorf("failed to migrate ancients: %w", err)
 	}
 
-	numAncientsNewAfter, err := newFreezer.Ancients()
+	numAncientsNewAfter, err = newFreezer.Ancients()
 	if err != nil {
 		return 0, 0, fmt.Errorf("failed to get number of ancients in new freezer: %w", err)
 	}
 
+	if numAncientsNewAfter != numAncientsOld {
+		return 0, 0, fmt.Errorf("failed to migrate all ancients from old to new db. Expected %d, got %d", numAncientsOld, numAncientsNewAfter)
+	}
+
 	log.Info("Ancient Block Migration Ended", "process", "ancients", "ancientsInOldDB", numAncientsOld, "ancientsInNewDB", numAncientsNewAfter, "migrated", numAncientsNewAfter-numAncientsNewBefore)
 	return numAncientsNewBefore, numAncientsNewAfter, nil
 }
@@ -214,14 +223,16 @@ func writeAncientBlocks(ctx context.Context, freezer *rawdb.Freezer, in <-chan R
 }
 
 // getStrayAncientBlocks returns a list of ancient block numbers / hashes that somehow were not removed from leveldb
-func getStrayAncientBlocks(dbPath string) ([]*rawdb.NumberHash, error) {
+func getStrayAncientBlocks(dbPath string) (blocks []*rawdb.NumberHash, err error) {
 	defer timer("getStrayAncientBlocks")()
 
 	db, err := openDB(dbPath, true)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open database: %w", err)
 	}
-	defer db.Close()
+	defer func() {
+		err = errors.Join(err, db.Close())
+	}()
 
 	numAncients, err := db.Ancients()
 	if err != nil {

op-chain-ops/cmd/celo-migrate/db.go

@@ -38,7 +38,8 @@ func headerKey(number uint64, hash common.Hash) []byte {
 
 // Opens a database with access to AncientsDb
 func openDB(chaindataPath string, readOnly bool) (ethdb.Database, error) {
-	if _, err := os.Stat(chaindataPath); errors.Is(err, os.ErrNotExist) {
+	// Will throw an error if the chaindataPath does not exist
+	if _, err := os.Stat(chaindataPath); err != nil {
 		return nil, err
 	}
 
@@ -100,14 +101,16 @@ func removeBlocks(ldb ethdb.Database, numberHashes []*rawdb.NumberHash) error {
 	return nil
 }
 
-func getHeadHeader(dbpath string) (*types.Header, error) {
+func getHeadHeader(dbpath string) (headHeader *types.Header, err error) {
 	db, err := openDBWithoutFreezer(dbpath, true)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open database at %q err: %w", dbpath, err)
 	}
-	defer db.Close()
+	defer func() {
+		err = errors.Join(err, db.Close())
+	}()
 
-	headHeader := rawdb.ReadHeadHeader(db)
+	headHeader = rawdb.ReadHeadHeader(db)
 	if headHeader == nil {
 		return nil, fmt.Errorf("head header not in database at: %s", dbpath)
 	}

op-chain-ops/cmd/celo-migrate/main.go

@@ -84,7 +84,7 @@ var (
 	batchSizeFlag = &cli.Uint64Flag{
 		Name:  "batch-size",
 		Usage: "Batch size to use for block migration, larger batch sizes can speed up migration but require more memory. If increasing the batch size consider also increasing the memory-limit",
-		Value: 50000, // TODO(Alec) optimize default parameters
+		Value: 50000,
 	}
 	bufferSizeFlag = &cli.Uint64Flag{
 		Name:  "buffer-size",
@@ -315,14 +315,16 @@ func runPreMigration(opts preMigrationOptions) ([]*rawdb.NumberHash, uint64, err
 	return strayAncientBlocks, numAncientsNewAfter, nil
 }
 
-func runNonAncientMigration(newDBPath string, strayAncientBlocks []*rawdb.NumberHash, batchSize, numAncients uint64) error {
+func runNonAncientMigration(newDBPath string, strayAncientBlocks []*rawdb.NumberHash, batchSize, numAncients uint64) (err error) {
 	defer timer("non-ancient migration")()
 
 	newDB, err := openDBWithoutFreezer(newDBPath, false)
 	if err != nil {
 		return fmt.Errorf("failed to open new database: %w", err)
 	}
-	defer newDB.Close()
+	defer func() {
+		err = errors.Join(err, newDB.Close())
+	}()
 
 	// get the last block number
 	hash := rawdb.ReadHeadHeaderHash(newDB)

op-chain-ops/cmd/celo-migrate/non-ancients.go

@@ -19,7 +19,10 @@ func copyDbExceptAncients(oldDbPath, newDbPath string) error {
 
 	// Get rsync help output
 	cmdHelp := exec.Command("rsync", "--help")
-	output, _ := cmdHelp.CombinedOutput()
+	output, err := cmdHelp.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("failed to get rsync help output: %w", err)
+	}
 
 	// Convert output to string
 	outputStr := string(output)
@@ -113,7 +116,9 @@ func migrateNonAncientBlock(number uint64, hash common.Hash, newDB ethdb.Databas
 	// write header and body
 	batch := newDB.NewBatch()
 	rawdb.WriteBodyRLP(batch, hash, number, newBody)
-	_ = batch.Put(headerKey(number, hash), newHeader)
+	if err := batch.Put(headerKey(number, hash), newHeader); err != nil {
+		return fmt.Errorf("failed to write header: block %d - %x: %w", number, hash, err)
+	}
 	if err := batch.Write(); err != nil {
 		return fmt.Errorf("failed to write header and body: block %d - %x: %w", number, hash, err)
 	}