From e94aadd424e4c71522d0158a45a3970d2a7fd9e4 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Tue, 17 Oct 2023 14:46:38 -0700 Subject: [PATCH 1/5] [Access] Verify checkpoint's root hash --- .../node_builder/access_node_builder.go | 34 ++++++++++--------- cmd/bootstrap/run/execution_state.go | 5 ++- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index e18a6b6d699..2af1c04b0a9 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/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 { @@ -230,7 +231,6 @@ func DefaultAccessNodeConfig() *AccessNodeConfig { executionDataIndexingEnabled: false, registersDBPath: filepath.Join(homedir, ".flow", "execution_state"), checkpointFile: cmd.NotSet, - checkpointHeight: 0, } } @@ -682,24 +682,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) } @@ -911,7 +914,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") }).ValidateFlags(func() error { if builder.supportsObserver && (builder.PublicNetworkConfig.BindAddress == cmd.NotSet || builder.PublicNetworkConfig.BindAddress == "") { return errors.New("public-network-address must be set if supports-observer is true") diff --git a/cmd/bootstrap/run/execution_state.go b/cmd/bootstrap/run/execution_state.go index 72de2d57488..939f0adf3e8 100644 --- a/cmd/bootstrap/run/execution_state.go +++ b/cmd/bootstrap/run/execution_state.go @@ -14,12 +14,11 @@ import ( ledger "github.com/onflow/flow-go/ledger/complete" "github.com/onflow/flow-go/ledger/complete/mtrie/trie" "github.com/onflow/flow-go/ledger/complete/wal" + modelbootstrap "github.com/onflow/flow-go/model/bootstrap" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/metrics" ) -const bootstrapCheckpointFile = "bootstrap-checkpoint" - func GenerateExecutionState( dbDir string, accountKey flow.AccountPublicKey, @@ -75,7 +74,7 @@ func GenerateExecutionState( return flow.DummyStateCommitment, fmt.Errorf("bootstraping failed to produce a checkpoint for trie %v", stateCommitment) } - err = wal.StoreCheckpointV6([]*trie.MTrie{matchTrie}, dbDir, bootstrapCheckpointFile, zerolog.Nop(), 1) + err = wal.StoreCheckpointV6([]*trie.MTrie{matchTrie}, dbDir, modelbootstrap.FilenameWALRootCheckpoint, zerolog.Nop(), 1) if err != nil { return flow.DummyStateCommitment, fmt.Errorf("failed to store bootstrap checkpoint: %w", err) } From 8d03c69e67271ffbb991b7e57d4ca5898aea81a3 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:34:26 -0700 Subject: [PATCH 2/5] add initial metrics/log --- module/metrics.go | 5 +++++ module/metrics/execution_state_indexer.go | 7 +++++++ module/metrics/noop.go | 1 + module/state_synchronization/indexer/indexer_core.go | 10 +++++++++- 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/module/metrics.go b/module/metrics.go index 7829e72f600..d9fce7ab0c9 100644 --- a/module/metrics.go +++ b/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 { diff --git a/module/metrics/execution_state_indexer.go b/module/metrics/execution_state_indexer.go index ae1eee079a1..6c3f68b4d69 100644 --- a/module/metrics/execution_state_indexer.go +++ b/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())) diff --git a/module/metrics/noop.go b/module/metrics/noop.go index 4e6bc9f534b..0ee6bb394fa 100644 --- a/module/metrics/noop.go +++ b/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) {} diff --git a/module/state_synchronization/indexer/indexer_core.go b/module/state_synchronization/indexer/indexer_core.go index b127ccda9fb..640947d4c9d 100644 --- a/module/state_synchronization/indexer/indexer_core.go +++ b/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, From 0065fda235f4c714057c73225dcf8de143899e3c Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 18 Oct 2023 14:51:59 -0700 Subject: [PATCH 3/5] update mocks --- module/mock/execution_state_indexer_metrics.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/module/mock/execution_state_indexer_metrics.go b/module/mock/execution_state_indexer_metrics.go index 4dbb0e3b2d9..f3a2586bfaf 100644 --- a/module/mock/execution_state_indexer_metrics.go +++ b/module/mock/execution_state_indexer_metrics.go @@ -23,6 +23,11 @@ func (_m *ExecutionStateIndexerMetrics) BlockReindexed() { _m.Called() } +// InitializeLatestHeight provides a mock function with given fields: height +func (_m *ExecutionStateIndexerMetrics) InitializeLatestHeight(height uint64) { + _m.Called(height) +} + type mockConstructorTestingTNewExecutionStateIndexerMetrics interface { mock.TestingT Cleanup(func()) From ea679b4ff0712a94c771ff17b22b8d401d63f816 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:54:24 -0700 Subject: [PATCH 4/5] fix unittests after metrics change --- .../indexer/indexer_core_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/module/state_synchronization/indexer/indexer_core_test.go b/module/state_synchronization/indexer/indexer_core_test.go index c41e9f1a6b2..5b4289dc4d8 100644 --- a/module/state_synchronization/indexer/indexer_core_test.go +++ b/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) From b203b00ea24a7b9efd94493f756acce629e96696 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 18 Oct 2023 16:57:57 -0700 Subject: [PATCH 5/5] fix access_api_test --- integration/tests/access/access_api_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/integration/tests/access/access_api_test.go b/integration/tests/access/access_api_test.go index 018592018e6..2b39488dcd2 100644 --- a/integration/tests/access/access_api_test.go +++ b/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), )