From ad39920d669534d2c6d506fbeae67f27b4968c37 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Wed, 15 Jan 2025 16:27:25 +0100 Subject: [PATCH] apply review comments --- .../computation/computer/computer.go | 5 +++-- engine/execution/computation/manager.go | 3 ++- .../execution/computation/query/executor.go | 5 +++-- .../computation/snapshot_provider.go | 14 ++++++------- engine/execution/testutil/fixtures.go | 11 +++++----- fvm/context.go | 3 ++- model/verification/convert/convert.go | 3 ++- model/verification/verifiableChunkData.go | 17 ++++++++-------- module/execution/scripts.go | 3 ++- .../protocol/execution.go | 20 ++++++++++--------- 10 files changed, 47 insertions(+), 37 deletions(-) rename model/flow/snapshot_subset.go => state/protocol/execution.go (70%) diff --git a/engine/execution/computation/computer/computer.go b/engine/execution/computation/computer/computer.go index e0097bc47e0..4254d996545 100644 --- a/engine/execution/computation/computer/computer.go +++ b/engine/execution/computation/computer/computer.go @@ -24,6 +24,7 @@ import ( "github.com/onflow/flow-go/module/executiondatasync/provider" "github.com/onflow/flow-go/module/mempool/entity" "github.com/onflow/flow-go/module/trace" + "github.com/onflow/flow-go/state/protocol" "github.com/onflow/flow-go/utils/logging" ) @@ -114,7 +115,7 @@ type blockComputer struct { spockHasher hash.Hasher receiptHasher hash.Hasher colResCons []result.ExecutedCollectionConsumer - protocolState flow.ProtocolSnapshotExecutionSubsetProvider + protocolState protocol.SnapshotExecutionSubsetProvider maxConcurrency int } @@ -145,7 +146,7 @@ func NewBlockComputer( signer module.Local, executionDataProvider provider.Provider, colResCons []result.ExecutedCollectionConsumer, - state flow.ProtocolSnapshotExecutionSubsetProvider, + state protocol.SnapshotExecutionSubsetProvider, maxConcurrency int, ) (BlockComputer, error) { if maxConcurrency < 1 { diff --git a/engine/execution/computation/manager.go b/engine/execution/computation/manager.go index 6b6ffa89318..9dedb65f005 100644 --- a/engine/execution/computation/manager.go +++ b/engine/execution/computation/manager.go @@ -18,6 +18,7 @@ import ( "github.com/onflow/flow-go/module" "github.com/onflow/flow-go/module/executiondatasync/provider" "github.com/onflow/flow-go/module/mempool/entity" + "github.com/onflow/flow-go/state/protocol" "github.com/onflow/flow-go/utils/logging" ) @@ -90,7 +91,7 @@ func New( metrics module.ExecutionMetrics, tracer module.Tracer, me module.Local, - protoState flow.ProtocolSnapshotExecutionSubsetProvider, + protoState protocol.SnapshotExecutionSubsetProvider, vmCtx fvm.Context, committer computer.ViewCommitter, executionDataProvider provider.Provider, diff --git a/engine/execution/computation/query/executor.go b/engine/execution/computation/query/executor.go index c4dcd016059..1027cebcbfa 100644 --- a/engine/execution/computation/query/executor.go +++ b/engine/execution/computation/query/executor.go @@ -18,6 +18,7 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module" + "github.com/onflow/flow-go/state/protocol" "github.com/onflow/flow-go/utils/debug" "github.com/onflow/flow-go/utils/rand" ) @@ -117,7 +118,7 @@ type QueryExecutor struct { vmCtx fvm.Context derivedChainData *derived.DerivedChainData rngLock *sync.Mutex - protocolStateSnapshot flow.ProtocolSnapshotExecutionSubsetProvider + protocolStateSnapshot protocol.SnapshotExecutionSubsetProvider } var _ Executor = &QueryExecutor{} @@ -129,7 +130,7 @@ func NewQueryExecutor( vm fvm.VM, vmCtx fvm.Context, derivedChainData *derived.DerivedChainData, - protocolStateSnapshot flow.ProtocolSnapshotExecutionSubsetProvider, + protocolStateSnapshot protocol.SnapshotExecutionSubsetProvider, ) *QueryExecutor { if config.ComputationLimit > 0 { vmCtx = fvm.NewContextFromParent(vmCtx, fvm.WithComputationLimit(config.ComputationLimit)) diff --git a/engine/execution/computation/snapshot_provider.go b/engine/execution/computation/snapshot_provider.go index 5b95a050a25..4819ca4b16d 100644 --- a/engine/execution/computation/snapshot_provider.go +++ b/engine/execution/computation/snapshot_provider.go @@ -5,25 +5,25 @@ import ( "github.com/onflow/flow-go/state/protocol" ) -// ProtocolSnapshotExecutionSubset is a subset of the protocol state snapshot that is needed by the FVM -var _ flow.ProtocolSnapshotExecutionSubset = (protocol.Snapshot)(nil) +// SnapshotExecutionSubset is a subset of the protocol state snapshot that is needed by the FVM +var _ protocol.SnapshotExecutionSubset = (protocol.Snapshot)(nil) -// protocolStateWrapper just wraps the protocol.State and returns a ProtocolSnapshotExecutionSubset +// protocolStateWrapper just wraps the protocol.State and returns a SnapshotExecutionSubset // from the AtBlockID method instead of the protocol.Snapshot interface. type protocolStateWrapper struct { protocol.State } // protocolStateWrapper implements `EntropyProviderPerBlock` -var _ flow.ProtocolSnapshotExecutionSubsetProvider = (*protocolStateWrapper)(nil) +var _ protocol.SnapshotExecutionSubsetProvider = (*protocolStateWrapper)(nil) -func (p protocolStateWrapper) AtBlockID(blockID flow.Identifier) flow.ProtocolSnapshotExecutionSubset { +func (p protocolStateWrapper) AtBlockID(blockID flow.Identifier) protocol.SnapshotExecutionSubset { return p.State.AtBlockID(blockID) } // NewProtocolStateWrapper wraps the protocol.State input so that the AtBlockID method returns a -// ProtocolSnapshotExecutionSubset instead of the protocol.Snapshot interface. +// SnapshotExecutionSubset instead of the protocol.Snapshot interface. // This is used in the FVM for execution. -func NewProtocolStateWrapper(s protocol.State) flow.ProtocolSnapshotExecutionSubsetProvider { +func NewProtocolStateWrapper(s protocol.State) protocol.SnapshotExecutionSubsetProvider { return protocolStateWrapper{s} } diff --git a/engine/execution/testutil/fixtures.go b/engine/execution/testutil/fixtures.go index 9477e8e391a..be2c758e43a 100644 --- a/engine/execution/testutil/fixtures.go +++ b/engine/execution/testutil/fixtures.go @@ -28,6 +28,7 @@ import ( "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/epochs" "github.com/onflow/flow-go/module/executiondatasync/execution_data" + "github.com/onflow/flow-go/state/protocol" "github.com/onflow/flow-go/utils/unittest" ) @@ -656,7 +657,7 @@ func EntropyProviderFixture(source []byte) environment.EntropyProvider { // supports AtBlockID to return a snapshot mock. // The snapshot mock only supports RandomSource(). // If input is nil, a random source fixture is generated. -func ProtocolStateWithSourceFixture(source []byte) flow.ProtocolSnapshotExecutionSubsetProvider { +func ProtocolStateWithSourceFixture(source []byte) protocol.SnapshotExecutionSubsetProvider { if source == nil { source = unittest.SignatureFixture() } @@ -670,7 +671,7 @@ func ProtocolStateWithSourceFixture(source []byte) flow.ProtocolSnapshotExecutio } provider := mockProtocolStateSnapshotProvider{ - snapshotFunc: func(blockID flow.Identifier) flow.ProtocolSnapshotExecutionSubset { + snapshotFunc: func(blockID flow.Identifier) protocol.SnapshotExecutionSubset { return snapshot }, } @@ -678,10 +679,10 @@ func ProtocolStateWithSourceFixture(source []byte) flow.ProtocolSnapshotExecutio } type mockProtocolStateSnapshotProvider struct { - snapshotFunc func(blockID flow.Identifier) flow.ProtocolSnapshotExecutionSubset + snapshotFunc func(blockID flow.Identifier) protocol.SnapshotExecutionSubset } -func (m mockProtocolStateSnapshotProvider) AtBlockID(blockID flow.Identifier) flow.ProtocolSnapshotExecutionSubset { +func (m mockProtocolStateSnapshotProvider) AtBlockID(blockID flow.Identifier) protocol.SnapshotExecutionSubset { return m.snapshotFunc(blockID) } @@ -704,4 +705,4 @@ func (m mockSnapshotSubset) VersionBeacon() (*flow.SealedVersionBeacon, error) { return m.versionBeaconFunc() } -var _ flow.ProtocolSnapshotExecutionSubset = (*mockSnapshotSubset)(nil) +var _ protocol.SnapshotExecutionSubset = (*mockSnapshotSubset)(nil) diff --git a/fvm/context.go b/fvm/context.go index 5f25616800b..7a579ebc979 100644 --- a/fvm/context.go +++ b/fvm/context.go @@ -14,6 +14,7 @@ import ( "github.com/onflow/flow-go/fvm/tracing" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module" + "github.com/onflow/flow-go/state/protocol" ) const ( @@ -405,7 +406,7 @@ func WithExecutionVersionProvider(provider environment.ExecutionVersionProvider) // WithProtocolStateSnapshot sets all the necessary components from a subset of the protocol state // to the virtual machine context. -func WithProtocolStateSnapshot(snapshot flow.ProtocolSnapshotExecutionSubset) Option { +func WithProtocolStateSnapshot(snapshot protocol.SnapshotExecutionSubset) Option { return func(ctx Context) Context { ctx = WithEntropyProvider(snapshot)(ctx) diff --git a/model/verification/convert/convert.go b/model/verification/convert/convert.go index 109f69667de..3ffbcd3a8d8 100644 --- a/model/verification/convert/convert.go +++ b/model/verification/convert/convert.go @@ -5,13 +5,14 @@ import ( "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/model/verification" + "github.com/onflow/flow-go/state/protocol" ) func FromChunkDataPack( chunk *flow.Chunk, chunkDataPack *flow.ChunkDataPack, header *flow.Header, - snapshot flow.ProtocolSnapshotExecutionSubset, + snapshot protocol.SnapshotExecutionSubset, result *flow.ExecutionResult, ) (*verification.VerifiableChunkData, error) { diff --git a/model/verification/verifiableChunkData.go b/model/verification/verifiableChunkData.go index 510c207881b..ec2ec448351 100644 --- a/model/verification/verifiableChunkData.go +++ b/model/verification/verifiableChunkData.go @@ -2,17 +2,18 @@ package verification import ( "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/state/protocol" ) // VerifiableChunkData represents a ready-to-verify chunk // It contains the execution result as well as all resources needed to verify it type VerifiableChunkData struct { - IsSystemChunk bool // indicates whether this is a system chunk - Chunk *flow.Chunk // the chunk to be verified - Header *flow.Header // BlockHeader that contains this chunk - Snapshot flow.ProtocolSnapshotExecutionSubset // state snapshot at the chunk's block - Result *flow.ExecutionResult // execution result of this block - ChunkDataPack *flow.ChunkDataPack // chunk data package needed to verify this chunk - EndState flow.StateCommitment // state commitment at the end of this chunk - TransactionOffset uint32 // index of the first transaction in a chunk within a block + IsSystemChunk bool // indicates whether this is a system chunk + Chunk *flow.Chunk // the chunk to be verified + Header *flow.Header // BlockHeader that contains this chunk + Snapshot protocol.SnapshotExecutionSubset // state snapshot at the chunk's block + Result *flow.ExecutionResult // execution result of this block + ChunkDataPack *flow.ChunkDataPack // chunk data package needed to verify this chunk + EndState flow.StateCommitment // state commitment at the end of this chunk + TransactionOffset uint32 // index of the first transaction in a chunk within a block } diff --git a/module/execution/scripts.go b/module/execution/scripts.go index 968de301f58..0bb337b5f56 100644 --- a/module/execution/scripts.go +++ b/module/execution/scripts.go @@ -13,6 +13,7 @@ import ( "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module" + "github.com/onflow/flow-go/state/protocol" "github.com/onflow/flow-go/storage" ) @@ -75,7 +76,7 @@ func NewScripts( log zerolog.Logger, metrics module.ExecutionMetrics, chainID flow.ChainID, - protocolSnapshotProvider flow.ProtocolSnapshotExecutionSubsetProvider, + protocolSnapshotProvider protocol.SnapshotExecutionSubsetProvider, header storage.Headers, registerAtHeight RegisterAtHeight, queryConf query.QueryConfig, diff --git a/model/flow/snapshot_subset.go b/state/protocol/execution.go similarity index 70% rename from model/flow/snapshot_subset.go rename to state/protocol/execution.go index 613c82f16d5..bae36c83706 100644 --- a/model/flow/snapshot_subset.go +++ b/state/protocol/execution.go @@ -1,16 +1,18 @@ -package flow +package protocol -// ProtocolSnapshotExecutionSubset is a subset of the protocol state snapshot that is needed by the FVM +import "github.com/onflow/flow-go/model/flow" + +// SnapshotExecutionSubset is a subset of the protocol state snapshot that is needed by the FVM // for execution. -type ProtocolSnapshotExecutionSubset interface { +type SnapshotExecutionSubset interface { // RandomSource provides a source of entropy that can be // expanded into randoms (using a pseudo-random generator). // The returned slice should have at least 128 bits of entropy. // The function doesn't error in normal operations, any // error should be treated as an exception. // - // `protocol.ProtocolSnapshotExecutionSubset` implements `EntropyProvider` interface - // Note that `ProtocolSnapshotExecutionSubset` possible errors for RandomSource() are: + // `protocol.SnapshotExecutionSubset` implements `EntropyProvider` interface + // Note that `SnapshotExecutionSubset` possible errors for RandomSource() are: // - storage.ErrNotFound if the QC is unknown. // - state.ErrUnknownSnapshotReference if the snapshot reference block is unknown // However, at this stage, snapshot reference block should be known and the QC should also be known, @@ -28,11 +30,11 @@ type ProtocolSnapshotExecutionSubset interface { // The remaining entries are for all future block heights. Future version boundaries // can be removed, in which case the emitted event will not contain the removed version // boundaries. - VersionBeacon() (*SealedVersionBeacon, error) + VersionBeacon() (*flow.SealedVersionBeacon, error) } -// ProtocolSnapshotExecutionSubsetProvider is an interface that provides a subset of the protocol state +// SnapshotExecutionSubsetProvider is an interface that provides a subset of the protocol state // at a specific block. -type ProtocolSnapshotExecutionSubsetProvider interface { - AtBlockID(blockID Identifier) ProtocolSnapshotExecutionSubset +type SnapshotExecutionSubsetProvider interface { + AtBlockID(blockID flow.Identifier) SnapshotExecutionSubset }