Merge pull request #4830 from onflow/petera/48060-access-verify-check…

…point-root-hash

[Access] Validate checkpoint's root hash

cmd/access/node_builder/access_node_builder.go

@@ -48,6 +48,8 @@ import (
 	"github.com/onflow/flow-go/engine/common/requester"
 	synceng "github.com/onflow/flow-go/engine/common/synchronization"
 	"github.com/onflow/flow-go/engine/execution/computation/query"
+	"github.com/onflow/flow-go/ledger"
+	"github.com/onflow/flow-go/ledger/complete/wal"
 	"github.com/onflow/flow-go/model/bootstrap"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/model/flow/filter"
@@ -143,7 +145,6 @@ type AccessNodeConfig struct {
 	executionDataIndexingEnabled bool
 	registersDBPath              string
 	checkpointFile               string
-	checkpointHeight             uint64
 }
 
 type PublicNetworkConfig struct {
@@ -229,7 +230,6 @@ func DefaultAccessNodeConfig() *AccessNodeConfig {
 		executionDataIndexingEnabled: false,
 		registersDBPath:              filepath.Join(homedir, ".flow", "execution_state"),
 		checkpointFile:               cmd.NotSet,
-		checkpointHeight:             0,
 	}
 }
 
@@ -681,24 +681,27 @@ func (builder *FlowAccessNodeBuilder) BuildExecutionSyncComponents() *FlowAccess
 				}
 
 				if !bootstrapped {
-					// since indexing is done using execution data, make sure that execution data
-					// will be available starting at the checkpoint height. otherwise, the indexer
-					// will fail to progress.
-					if builder.checkpointHeight < builder.executionDataConfig.InitialBlockHeight {
-						return nil, fmt.Errorf(
-							"checkpoint is from height %d, but execution data is only available from height %d. "+
-								"either provide a more recent checkpoint, or configure execution sync to start from a lower height",
-							builder.checkpointHeight,
-							builder.executionDataConfig.InitialBlockHeight,
-						)
-					}
-
 					checkpointFile := builder.checkpointFile
 					if checkpointFile == cmd.NotSet {
 						checkpointFile = path.Join(builder.BootstrapDir, bootstrap.PathRootCheckpoint)
 					}
 
-					bootstrap, err := pStorage.NewRegisterBootstrap(pdb, checkpointFile, builder.checkpointHeight, builder.Logger)
+					// currently, the checkpoint must be from the root block.
+					// read the root hash from the provided checkpoint and verify it matches the
+					// state commitment from the root snapshot.
+					err := wal.CheckpointHasRootHash(
+						node.Logger,
+						"", // checkpoint file already full path
+						checkpointFile,
+						ledger.RootHash(node.RootSeal.FinalState),
+					)
+					if err != nil {
+						return nil, fmt.Errorf("could not verify checkpoint file: %w", err)
+					}
+
+					checkpointHeight := builder.SealedRootBlock.Header.Height
+
+					bootstrap, err := pStorage.NewRegisterBootstrap(pdb, checkpointFile, checkpointHeight, builder.Logger)
 					if err != nil {
 						return nil, fmt.Errorf("could not create registers bootstrapper: %w", err)
 					}
@@ -908,7 +911,6 @@ func (builder *FlowAccessNodeBuilder) extraFlags() {
 		flags.BoolVar(&builder.executionDataIndexingEnabled, "execution-data-indexing-enabled", defaultConfig.executionDataIndexingEnabled, "whether to enable the execution data indexing")
 		flags.StringVar(&builder.registersDBPath, "execution-state-dir", defaultConfig.registersDBPath, "directory to use for execution-state database")
 		flags.StringVar(&builder.checkpointFile, "execution-state-checkpoint", defaultConfig.checkpointFile, "execution-state checkpoint file")
-		flags.Uint64Var(&builder.checkpointHeight, "execution-state-checkpoint-height", defaultConfig.checkpointHeight, "block height at which the execution-state checkpoint was generated")
 
 		// Script Execution
 		flags.StringVar(&builder.rpcConf.BackendConfig.ScriptExecutionMode, "script-execution-mode", defaultConfig.rpcConf.BackendConfig.ScriptExecutionMode, "mode to use when executing scripts. one of (local-only, execution-nodes-only, failover, compare)")

integration/tests/access/access_api_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/onflow/flow-go/integration/testnet"
 	"github.com/onflow/flow-go/integration/tests/lib"
 	"github.com/onflow/flow-go/integration/tests/mvp"
-	"github.com/onflow/flow-go/model/bootstrap"
 	"github.com/onflow/flow-go/model/flow"
 	"github.com/onflow/flow-go/utils/unittest"
 )
@@ -80,8 +79,6 @@ func (s *AccessAPISuite) SetupTest() {
 		testnet.WithAdditionalFlag("--execution-data-retry-delay=1s"),
 		testnet.WithAdditionalFlag("--execution-data-indexing-enabled=true"),
 		testnet.WithAdditionalFlagf("--execution-state-dir=%s", testnet.DefaultExecutionStateDir),
-		testnet.WithAdditionalFlagf("--execution-state-checkpoint=/bootstrap/%s", bootstrap.PathRootCheckpoint),
-		testnet.WithAdditionalFlag("--execution-state-checkpoint-height=0"),
 		testnet.WithAdditionalFlagf("--script-execution-mode=%s", backend.ScriptExecutionModeLocalOnly),
 	)
 

module/metrics.go

@@ -577,6 +577,11 @@ type ExecutionStateIndexerMetrics interface {
 
 	// BlockReindexed records that a previously indexed block was indexed again.
 	BlockReindexed()
+
+	// InitializeLatestHeight records the latest height that has been indexed.
+	// This should only be used during startup. After startup, use BlockIndexed to record newly
+	// indexed heights.
+	InitializeLatestHeight(height uint64)
 }
 
 type RuntimeMetrics interface {

module/metrics/execution_state_indexer.go

@@ -75,6 +75,13 @@ func NewExecutionStateIndexerCollector() module.ExecutionStateIndexerMetrics {
 	}
 }
 
+// InitializeLatestHeight records the latest height that has been indexed.
+// This should only be used during startup. After startup, use BlockIndexed to record newly
+// indexed heights.
+func (c *ExecutionStateIndexerCollector) InitializeLatestHeight(height uint64) {
+	c.highestIndexedHeight.Set(float64(height))
+}
+
 // BlockIndexed records metrics from indexing execution data from a single block.
 func (c *ExecutionStateIndexerCollector) BlockIndexed(height uint64, duration time.Duration, registers, events, transactionResults int) {
 	c.indexDuration.Observe(float64(duration.Milliseconds()))

module/metrics/noop.go

@@ -320,3 +320,4 @@ var _ module.ExecutionStateIndexerMetrics = (*NoopCollector)(nil)
 
 func (nc *NoopCollector) BlockIndexed(uint64, time.Duration, int, int, int) {}
 func (nc *NoopCollector) BlockReindexed()                                   {}
+func (nc *NoopCollector) InitializeLatestHeight(height uint64)              {}

module/state_synchronization/indexer/indexer_core.go

@@ -41,8 +41,16 @@ func New(
 	events storage.Events,
 	results storage.LightTransactionResults,
 ) (*IndexerCore, error) {
+	log = log.With().Str("component", "execution_indexer").Logger()
+	metrics.InitializeLatestHeight(registers.LatestHeight())
+
+	log.Info().
+		Uint64("first_height", registers.FirstHeight()).
+		Uint64("latest_height", registers.LatestHeight()).
+		Msg("indexer initialized")
+
 	return &IndexerCore{
-		log:       log.With().Str("component", "execution_indexer").Logger(),
+		log:       log,
 		metrics:   metrics,
 		batcher:   batcher,
 		registers: registers,

module/state_synchronization/indexer/indexer_core_test.go

@@ -85,8 +85,12 @@ func (i *indexCoreTest) setLastHeight(f func(t *testing.T) uint64) *indexCoreTes
 		})
 	return i
 }
-
-func (i *indexCoreTest) useDefaultLastHeight() *indexCoreTest {
+func (i *indexCoreTest) useDefaultHeights() *indexCoreTest {
+	i.registers.
+		On("FirstHeight").
+		Return(func() uint64 {
+			return i.blocks[0].Header.Height
+		})
 	i.registers.
 		On("LatestHeight").
 		Return(func() uint64 {
@@ -154,6 +158,8 @@ func (i *indexCoreTest) initIndexer() *indexCoreTest {
 		require.NoError(i.t, os.RemoveAll(dbDir))
 	})
 
+	i.useDefaultHeights()
+
 	indexer, err := New(zerolog.New(os.Stdout), metrics.NewNoopCollector(), db, i.registers, i.headers, i.events, i.results)
 	require.NoError(i.t, err)
 	i.indexer = indexer
@@ -188,7 +194,6 @@ func TestExecutionState_IndexBlockData(t *testing.T) {
 
 		err := newIndexCoreTest(t, blocks, execData).
 			initIndexer().
-			useDefaultLastHeight().
 			useDefaultEvents().
 			useDefaultTransactionResults().
 			// make sure update registers match in length and are same as block data ledger payloads
@@ -233,7 +238,6 @@ func TestExecutionState_IndexBlockData(t *testing.T) {
 		err = newIndexCoreTest(t, blocks, execData).
 			initIndexer().
 			useDefaultEvents().
-			useDefaultLastHeight().
 			useDefaultTransactionResults().
 			// make sure update registers match in length and are same as block data ledger payloads
 			setStoreRegisters(func(t *testing.T, entries flow.RegisterEntries, height uint64) error {
@@ -268,7 +272,6 @@ func TestExecutionState_IndexBlockData(t *testing.T) {
 
 		err := newIndexCoreTest(t, blocks, execData).
 			initIndexer().
-			useDefaultLastHeight().
 			// make sure all events are stored at once in order
 			setStoreEvents(func(t *testing.T, actualBlockID flow.Identifier, actualEvents []flow.EventsList) error {
 				assert.Equal(t, block.ID(), actualBlockID)
@@ -310,7 +313,6 @@ func TestExecutionState_IndexBlockData(t *testing.T) {
 
 		err := newIndexCoreTest(t, blocks, execData).
 			initIndexer().
-			useDefaultLastHeight().
 			// make sure an empty set of events were stored
 			setStoreEvents(func(t *testing.T, actualBlockID flow.Identifier, actualEvents []flow.EventsList) error {
 				assert.Equal(t, block.ID(), actualBlockID)
@@ -366,7 +368,6 @@ func TestExecutionState_IndexBlockData(t *testing.T) {
 
 		err := newIndexCoreTest(t, blocks, execData).
 			initIndexer().
-			useDefaultLastHeight().
 			// make sure all events are stored at once in order
 			setStoreEvents(func(t *testing.T, actualBlockID flow.Identifier, actualEvents []flow.EventsList) error {
 				assert.Equal(t, block.ID(), actualBlockID)