augment existing pruning and finality tests

Signed-off-by: Igor Braga <[email protected]>

Author: bragaigor

cmd/pruning/pruning.go

@@ -84,7 +84,7 @@ func (r *importantRoots) addHeader(header *types.Header, overwrite bool) error {
 var hashListRegex = regexp.MustCompile("^(0x)?[0-9a-fA-F]{64}(,(0x)?[0-9a-fA-F]{64})*$")
 
 // Finds important roots to retain while proving
-func findImportantRoots(ctx context.Context, executionDB ethdb.Database, stack *node.Node, initConfig *conf.InitConfig, cacheConfig *core.BlockChainConfig, persistentConfig *conf.PersistentConfig, l1Client *ethclient.Client, rollupAddrs chaininfo.RollupAddresses, validatorRequired, melEnabled bool) ([]common.Hash, error) {
+func findImportantRoots(ctx context.Context, executionDB ethdb.Database, initConfig *conf.InitConfig, l1Client *ethclient.Client, rollupAddrs chaininfo.RollupAddresses, validatorRequired bool) ([]common.Hash, error) {
 	chainConfig := gethexec.TryReadStoredChainConfig(executionDB)
 	if chainConfig == nil {
 		return nil, errors.New("database doesn't have a chain config (was this node initialized?)")
@@ -164,7 +164,7 @@ func findImportantRoots(ctx context.Context, executionDB ethdb.Database, stack *
 	} else {
 		return nil, fmt.Errorf("unknown pruning mode: \"%v\"", initConfig.Prune)
 	}
-	if initConfig.Prune != "minimal" && l1Client != nil {
+	if initConfig.Prune != "minimal" {
 		// in pruning modes other than "minimal", get the latest finalized block and add it as a pruning target
 		finalizedBlockHash := rawdb.ReadFinalizedBlockHash(executionDB)
 		finalizedBlockNumber, ok := rawdb.ReadHeaderNumber(executionDB, finalizedBlockHash)
@@ -245,7 +245,7 @@ func PruneExecutionDB(ctx context.Context, executionDB ethdb.Database, stack *no
 	if initConfig.Prune == "" {
 		return pruner.RecoverPruning(stack.InstanceDir(), executionDB, initConfig.PruneThreads)
 	}
-	root, err := findImportantRoots(ctx, executionDB, stack, initConfig, cacheConfig, persistentConfig, l1Client, rollupAddrs, validatorRequired, melEnabled)
+	root, err := findImportantRoots(ctx, executionDB, initConfig, l1Client, rollupAddrs, validatorRequired)
 	if err != nil {
 		return fmt.Errorf("failed to find root to retain for pruning: %w", err)
 	}

execution/gethexec/sync_monitor.go

@@ -237,43 +237,73 @@ func (s *SyncMonitor) BlockMetadataByNumber(ctx context.Context, blockNum uint64
 	return nil, nil
 }
 
+func (s *SyncMonitor) getBlockHeaderForFinalityData(
+	finalityData *arbutil.FinalityData,
+	finalityDataType string,
+	modifier func(curr *arbutil.FinalityData) (arbutil.MessageIndex, common.Hash, error),
+) (*types.Header, error) {
+	if finalityData == nil {
+		return nil, nil
+	}
+
+	msgIdx := finalityData.MsgIdx
+	blockHash := finalityData.BlockHash
+
+	if modifier != nil {
+		var err error
+		msgIdx, blockHash, err = modifier(finalityData)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	blockNumber := s.exec.MessageIndexToBlockNumber(msgIdx)
+	block := s.exec.bc.GetBlockByNumber(blockNumber)
+	if block == nil {
+		log.Debug("block not found", "finalityDataType", finalityDataType, "blockNumber", blockNumber)
+		return nil, nil
+	}
+	if block.Hash() != blockHash {
+		errorMsg := fmt.Sprintf(
+			"block hash mismatch,finalityDataType=%s blockNumber=%v, block hash provided by consensus=%v, block hash from execution=%v",
+			finalityDataType,
+			blockNumber,
+			blockHash,
+			block.Hash(),
+		)
+		return nil, errors.New(errorMsg)
+	}
+	return block.Header(), nil
+}
+
 func (s *SyncMonitor) getFinalityBlockHeader(
 	waitForBlockValidator bool,
 	validatedFinalityData *arbutil.FinalityData,
 	finalityFinalityData *arbutil.FinalityData,
+	finalityDataType string,
 ) (*types.Header, error) {
 	if finalityFinalityData == nil {
 		return nil, nil
 	}
 
 	finalityMsgIdx := finalityFinalityData.MsgIdx
 	finalityBlockHash := finalityFinalityData.BlockHash
-	if waitForBlockValidator {
+
+	validatorModifier := func(curr *arbutil.FinalityData) (arbutil.MessageIndex, common.Hash, error) {
+		if !waitForBlockValidator {
+			return finalityMsgIdx, finalityBlockHash, nil
+		}
 		if validatedFinalityData == nil {
-			return nil, errors.New("block validator not set")
+			return 0, common.Hash{}, errors.New("block validator not set")
 		}
+		// If the finalized index is ahead of the validated one, cap it at the validated one
 		if finalityFinalityData.MsgIdx > validatedFinalityData.MsgIdx {
-			finalityMsgIdx = validatedFinalityData.MsgIdx
-			finalityBlockHash = validatedFinalityData.BlockHash
+			return validatedFinalityData.MsgIdx, validatedFinalityData.BlockHash, nil
 		}
+		return finalityMsgIdx, finalityBlockHash, nil
 	}
 
-	finalityBlockNumber := s.exec.MessageIndexToBlockNumber(finalityMsgIdx)
-	finalityBlock := s.exec.bc.GetBlockByNumber(finalityBlockNumber)
-	if finalityBlock == nil {
-		log.Debug("Finality block not found", "blockNumber", finalityBlockNumber)
-		return nil, nil
-	}
-	if finalityBlock.Hash() != finalityBlockHash {
-		errorMsg := fmt.Sprintf(
-			"finality block hash mismatch, blockNumber=%v, block hash provided by consensus=%v, block hash from execution=%v",
-			finalityBlockNumber,
-			finalityBlockHash,
-			finalityBlock.Hash(),
-		)
-		return nil, errors.New(errorMsg)
-	}
-	return finalityBlock.Header(), nil
+	return s.getBlockHeaderForFinalityData(finalityFinalityData, finalityDataType, validatorModifier)
 }
 
 func (s *SyncMonitor) SetFinalityData(
@@ -282,28 +312,42 @@ func (s *SyncMonitor) SetFinalityData(
 	finalizedFinalityData *arbutil.FinalityData,
 	validatedFinalityData *arbutil.FinalityData,
 ) error {
+	// Check finalized blocks
 	finalizedBlockHeader, err := s.getFinalityBlockHeader(
 		s.config.FinalizedBlockWaitForBlockValidator,
 		validatedFinalityData,
 		finalizedFinalityData,
+		"finalized",
 	)
 	if err != nil {
 		return err
 	}
 	s.exec.bc.SetFinalized(finalizedBlockHeader)
 
-	if executionDB != nil && finalizedBlockHeader != nil {
-		finalizedBlockHash := finalizedBlockHeader.Hash()
-		err := executionDB.Put(ValidatedBlockHashKey, finalizedBlockHash.Bytes())
+	// Check validated blocks
+	validatedBlockHeader, err := s.getBlockHeaderForFinalityData(
+		validatedFinalityData,
+		"validated",
+		nil,
+	)
+	if err != nil {
+		return err
+	}
+
+	if executionDB != nil && validatedBlockHeader != nil {
+		validatedBlockHash := validatedBlockHeader.Hash()
+		err := executionDB.Put(ValidatedBlockHashKey, validatedBlockHash.Bytes())
 		if err != nil {
 			return err
 		}
 	}
 
+	// Check safe blocks
 	safeBlockHeader, err := s.getFinalityBlockHeader(
 		s.config.SafeBlockWaitForBlockValidator,
 		validatedFinalityData,
 		safeFinalityData,
+		"safe",
 	)
 	if err != nil {
 		return err

system_tests/finality_data_test.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/offchainlabs/nitro/arbnode"
 	"github.com/offchainlabs/nitro/arbutil"
+	"github.com/offchainlabs/nitro/execution/gethexec"
 	"github.com/offchainlabs/nitro/util/testhelpers/env"
 )
 
@@ -174,20 +175,52 @@ func TestFinalityDataWaitForBlockValidator(t *testing.T) {
 		BlockHash: validatedMsgResult.BlockHash,
 	}
 
-	err = builder.L2.ExecNode.SyncMonitor.SetFinalityData(nil, &safeFinalityData, &finalizedFinalityData, &validatedFinalityData)
+	err = builder.L2.ExecNode.SyncMonitor.SetFinalityData(builder.L2.ExecNode.ExecutionDB, &safeFinalityData, &finalizedFinalityData, &validatedFinalityData)
 	Require(t, err)
 
+	// Make sure finalized and validated blocks in executionDB match the expected ones for builder.L2
+	data, err := builder.L2.ExecNode.ExecutionDB.Get(gethexec.ValidatedBlockHashKey)
+	Require(t, err)
+	validatedBlockHash := common.BytesToHash(data)
+	finalizedBlockHash := rawdb.ReadFinalizedBlockHash(builder.L2.ExecNode.ExecutionDB)
+
+	if validatedBlockHash != validatedMsgResult.BlockHash {
+		t.Fatalf("validatedBlockHash: %s does not match expected validatedBlockHash: %s", validatedBlockHash.Hex(), validatedMsgResult.BlockHash.Hex())
+	}
+
+	// We compare finalizedBlockHash against validated block hash because FinalizedBlockWaitForBlockValidator is
+	// set to true therefore we should expect finalized block hash to match the validated block hash
+	if finalizedBlockHash != validatedMsgResult.BlockHash {
+		t.Fatalf("finalizedBlockHash: %s does not match expected finalizedBlockHash: %s", finalizedBlockHash.Hex(), validatedMsgResult.BlockHash.Hex())
+	}
+
 	// wait for block validator is set to true in second node
 	checksFinalityData(t, "first node", ctx, builder.L2, validatedMsgIdx, validatedMsgIdx)
 
-	err = testClient2ndNode.ExecNode.SyncMonitor.SetFinalityData(nil, &safeFinalityData, &finalizedFinalityData, &validatedFinalityData)
+	err = testClient2ndNode.ExecNode.SyncMonitor.SetFinalityData(testClient2ndNode.ExecNode.ExecutionDB, &safeFinalityData, &finalizedFinalityData, &validatedFinalityData)
 	Require(t, err)
 
+	// Make sure finalized and validated blocks in executionDB match the expected ones for second node
+	data, err = testClient2ndNode.ExecNode.ExecutionDB.Get(gethexec.ValidatedBlockHashKey)
+	Require(t, err)
+	validatedBlockHash2ndNode := common.BytesToHash(data)
+	finalizedBlockHash2ndNode := rawdb.ReadFinalizedBlockHash(testClient2ndNode.ExecNode.ExecutionDB)
+
+	if validatedBlockHash2ndNode != validatedMsgResult.BlockHash {
+		t.Fatalf("validatedBlockHash: %s does not match expected validatedBlockHash: %s", validatedBlockHash2ndNode.Hex(), validatedMsgResult.BlockHash.Hex())
+	}
+
+	// Since FinalizedBlockWaitForBlockValidator is set to true for the second node we can
+	// compare finalizedBlockHash with expected finalized block hash
+	if finalizedBlockHash2ndNode != finalizedMsgResult.BlockHash {
+		t.Fatalf("finalizedBlockHash: %s does not match expected finalizedBlockHash: %s", finalizedBlockHash2ndNode.Hex(), finalizedMsgResult.BlockHash.Hex())
+	}
+
 	// wait for block validator is no set to true in second node
 	checksFinalityData(t, "2nd node", ctx, testClient2ndNode, finalizedMsgIdx, safeMsgIdx)
 
 	// if validatedFinalityData is nil, error should be returned if waitForBlockValidator is set to true
-	err = builder.L2.ExecNode.SyncMonitor.SetFinalityData(nil, &safeFinalityData, &finalizedFinalityData, nil)
+	err = builder.L2.ExecNode.SyncMonitor.SetFinalityData(builder.L2.ExecNode.ExecutionDB, &safeFinalityData, &finalizedFinalityData, nil)
 	if err == nil {
 		t.Fatalf("err should not be nil")
 	}

system_tests/pruning_test.go

@@ -48,8 +48,8 @@ func testPruning(t *testing.T, mode string, pruneParallelStorageTraversal bool)
 	// PathScheme prunes the state trie by itself, so only HashScheme should be tested
 	builder.RequireScheme(t, rawdb.HashScheme)
 
-	// Needed to create safe blocks; hence forcing SetFinalityData call
 	builder.nodeConfig.ParentChainReader.UseFinalityData = true
+	builder.nodeConfig.BlockValidator.Enable = true
 
 	_ = builder.Build(t)
 	l2cleanupDone := false
@@ -78,8 +78,8 @@ func testPruning(t *testing.T, mode string, pruneParallelStorageTraversal bool)
 	// add to the new executionDB below
 	data, err := builder.L2.ExecNode.ExecutionDB.Get(gethexec.ValidatedBlockHashKey)
 	Require(t, err)
-	validatedBlockHash := common.BytesToHash(data)
-	finalizedBlockHash := rawdb.ReadFinalizedBlockHash(builder.L2.ExecNode.ExecutionDB)
+	expectedValidatedBlockHash := common.BytesToHash(data)
+	expectedFinalizedBlockHash := rawdb.ReadFinalizedBlockHash(builder.L2.ExecNode.ExecutionDB)
 
 	l2cleanupDone = true
 	builder.L2.cleanup()
@@ -94,11 +94,18 @@ func testPruning(t *testing.T, mode string, pruneParallelStorageTraversal bool)
 		defer executionDB.Close()
 		executionDBEntriesBeforePruning := countStateEntries(executionDB)
 
-		// Since we're dealing with a new executionDB we store both validatedBlockHash and
-		// finalized blocks back into this new executionDB.
-		err = executionDB.Put(gethexec.ValidatedBlockHashKey, validatedBlockHash.Bytes())
+		data, err := executionDB.Get(gethexec.ValidatedBlockHashKey)
 		Require(t, err)
-		rawdb.WriteFinalizedBlockHash(executionDB, finalizedBlockHash)
+		validatedBlockHash := common.BytesToHash(data)
+		finalizedBlockHash := rawdb.ReadFinalizedBlockHash(executionDB)
+
+		if validatedBlockHash != expectedValidatedBlockHash {
+			t.Fatalf("validatedBlockHash: %s does not match expected ValidatedBlockHash: %s", validatedBlockHash.Hex(), expectedValidatedBlockHash.Hex())
+		}
+
+		if finalizedBlockHash != expectedFinalizedBlockHash {
+			t.Fatalf("finalizedBlockHash: %s does not match expected finalizedBlockHash: %s", finalizedBlockHash.Hex(), expectedFinalizedBlockHash.Hex())
+		}
 
 		prand := testhelpers.NewPseudoRandomDataSource(t, 1)
 		var testKeys [][]byte