From c11e30be63f6e47df6ee958436df7cd0741464d9 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Tue, 28 Nov 2023 12:12:02 -0800 Subject: [PATCH 1/4] [Access] add config to limit script execution range --- .../node_builder/access_node_builder.go | 15 ++++++++ engine/access/rpc/backend/script_executor.go | 34 +++++++++++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 6d48a429f57..7e5b756e217 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math" "os" "path" "path/filepath" @@ -147,6 +148,8 @@ type AccessNodeConfig struct { registersDBPath string checkpointFile string scriptExecutorConfig query.QueryConfig + scriptExecMinBlock uint64 + scriptExecMaxBlock uint64 } type PublicNetworkConfig struct { @@ -237,6 +240,8 @@ func DefaultAccessNodeConfig() *AccessNodeConfig { registersDBPath: filepath.Join(homedir, ".flow", "execution_state"), checkpointFile: cmd.NotSet, scriptExecutorConfig: query.NewDefaultConfig(), + scriptExecMinBlock: 0, + scriptExecMaxBlock: math.MaxUint64, } } @@ -1107,6 +1112,14 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { "script-execution-timeout", defaultConfig.scriptExecutorConfig.ExecutionTimeLimit, "timeout value for locally executed scripts. default: 10s") + flags.Uint64Var(&builder.scriptExecMinBlock, + "script-execution-min-height", + defaultConfig.scriptExecMinBlock, + "lowest block height to allow for script execution. default: no limit") + flags.Uint64Var(&builder.scriptExecMaxBlock, + "script-execution-max-height", + defaultConfig.scriptExecMaxBlock, + "highest block height to allow for script execution. default: no limit") }).ValidateFlags(func() error { if builder.supportsObserver && (builder.PublicNetworkConfig.BindAddress == cmd.NotSet || builder.PublicNetworkConfig.BindAddress == "") { @@ -1422,6 +1435,8 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { }). Module("backend script executor", func(node *cmd.NodeConfig) error { builder.ScriptExecutor = backend.NewScriptExecutor() + builder.ScriptExecutor.SetMinExecutableHeight(builder.scriptExecMinBlock) + builder.ScriptExecutor.SetMaxExecutableHeight(builder.scriptExecMaxBlock) return nil }). Module("async register store", func(node *cmd.NodeConfig) error { diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index 12d64a0daa9..c817780a5a1 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -2,7 +2,7 @@ package backend import ( "context" - "sync" + "math" "go.uber.org/atomic" @@ -21,25 +21,41 @@ type ScriptExecutor struct { // initialized is used to signal that the index and executor are ready initialized *atomic.Bool - // init is used to ensure that the object is initialized only once - init sync.Once + // minHeight and maxHeight are used to limit the block range that can be queried using local execution + // to ensure only blocks that are compatible with the node's current software version are allowed. + // Note: this is a temporary solution for cadence/fvm upgrades while version beacon support is added + minHeight *atomic.Uint64 + maxHeight *atomic.Uint64 } func NewScriptExecutor() *ScriptExecutor { return &ScriptExecutor{ initialized: atomic.NewBool(false), + minHeight: atomic.NewUint64(0), + maxHeight: atomic.NewUint64(math.MaxUint64), } } +// SetMinExecutableHeight sets the lowest block height (inclusive) that can be queried using local execution +// Use this to limit the executable block range supported by the node's current software version. +func (s *ScriptExecutor) SetMinExecutableHeight(height uint64) { + s.minHeight.Store(height) +} + +// SetMaxExecutableHeight sets the highest block height (inclusive) that can be queried using local execution +// Use this to limit the executable block range supported by the node's current software version. +func (s *ScriptExecutor) SetMaxExecutableHeight(height uint64) { + s.maxHeight.Store(height) +} + // InitReporter initializes the indexReporter and script executor // This method can be called at any time after the ScriptExecutor object is created. Any requests // made to the other methods will return execution.ErrDataNotAvailable until this method is called. func (s *ScriptExecutor) InitReporter(indexReporter state_synchronization.IndexReporter, scriptExecutor *execution.Scripts) { - s.init.Do(func() { - defer s.initialized.Store(true) + if s.initialized.CompareAndSwap(false, true) { s.indexReporter = indexReporter s.scriptExecutor = scriptExecutor - }) + } } // ExecuteAtBlockHeight executes provided script at the provided block height against a local execution state. @@ -71,5 +87,9 @@ func (s *ScriptExecutor) GetAccountAtBlockHeight(ctx context.Context, address fl } func (s *ScriptExecutor) isDataAvailable(height uint64) bool { - return s.initialized.Load() && height <= s.indexReporter.HighestIndexedHeight() && height >= s.indexReporter.LowestIndexedHeight() + return s.initialized.Load() && + height <= s.indexReporter.HighestIndexedHeight() && + height >= s.indexReporter.LowestIndexedHeight() && + height <= s.maxHeight.Load() && + height >= s.minHeight.Load() } From 6813f97460038ce669d0745fe1a39ea6def515c0 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 29 Nov 2023 10:40:41 -0800 Subject: [PATCH 2/4] return specific errors --- .../node_builder/access_node_builder.go | 4 +- engine/access/rpc/backend/script_executor.go | 57 ++++++++++++------- 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 7e5b756e217..4d33cbe277e 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1435,8 +1435,8 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { }). Module("backend script executor", func(node *cmd.NodeConfig) error { builder.ScriptExecutor = backend.NewScriptExecutor() - builder.ScriptExecutor.SetMinExecutableHeight(builder.scriptExecMinBlock) - builder.ScriptExecutor.SetMaxExecutableHeight(builder.scriptExecMaxBlock) + builder.ScriptExecutor.SetMinCompatibleHeight(builder.scriptExecMinBlock) + builder.ScriptExecutor.SetMaxCompatibleHeight(builder.scriptExecMaxBlock) return nil }). Module("async register store", func(node *cmd.NodeConfig) error { diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index c817780a5a1..5265396d46c 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -2,6 +2,7 @@ package backend import ( "context" + "fmt" "math" "go.uber.org/atomic" @@ -21,31 +22,31 @@ type ScriptExecutor struct { // initialized is used to signal that the index and executor are ready initialized *atomic.Bool - // minHeight and maxHeight are used to limit the block range that can be queried using local execution + // minCompatibleHeight and maxCompatibleHeight are used to limit the block range that can be queried using local execution // to ensure only blocks that are compatible with the node's current software version are allowed. // Note: this is a temporary solution for cadence/fvm upgrades while version beacon support is added - minHeight *atomic.Uint64 - maxHeight *atomic.Uint64 + minCompatibleHeight *atomic.Uint64 + maxCompatibleHeight *atomic.Uint64 } func NewScriptExecutor() *ScriptExecutor { return &ScriptExecutor{ - initialized: atomic.NewBool(false), - minHeight: atomic.NewUint64(0), - maxHeight: atomic.NewUint64(math.MaxUint64), + initialized: atomic.NewBool(false), + minCompatibleHeight: atomic.NewUint64(0), + maxCompatibleHeight: atomic.NewUint64(math.MaxUint64), } } -// SetMinExecutableHeight sets the lowest block height (inclusive) that can be queried using local execution +// SetMinCompatibleHeight sets the lowest block height (inclusive) that can be queried using local execution // Use this to limit the executable block range supported by the node's current software version. -func (s *ScriptExecutor) SetMinExecutableHeight(height uint64) { - s.minHeight.Store(height) +func (s *ScriptExecutor) SetMinCompatibleHeight(height uint64) { + s.minCompatibleHeight.Store(height) } -// SetMaxExecutableHeight sets the highest block height (inclusive) that can be queried using local execution +// SetMaxCompatibleHeight sets the highest block height (inclusive) that can be queried using local execution // Use this to limit the executable block range supported by the node's current software version. -func (s *ScriptExecutor) SetMaxExecutableHeight(height uint64) { - s.maxHeight.Store(height) +func (s *ScriptExecutor) SetMaxCompatibleHeight(height uint64) { + s.maxCompatibleHeight.Store(height) } // InitReporter initializes the indexReporter and script executor @@ -65,8 +66,8 @@ func (s *ScriptExecutor) InitReporter(indexReporter state_synchronization.IndexR // - execution.ErrDataNotAvailable if the data for the block height is not available. this could be because // the height is not within the index block range, or the index is not ready. func (s *ScriptExecutor) ExecuteAtBlockHeight(ctx context.Context, script []byte, arguments [][]byte, height uint64) ([]byte, error) { - if !s.isDataAvailable(height) { - return nil, execution.ErrDataNotAvailable + if err := s.checkDataAvailable(height); err != nil { + return nil, err } return s.scriptExecutor.ExecuteAtBlockHeight(ctx, script, arguments, height) @@ -79,17 +80,29 @@ func (s *ScriptExecutor) ExecuteAtBlockHeight(ctx context.Context, script []byte // - execution.ErrDataNotAvailable if the data for the block height is not available. this could be because // the height is not within the index block range, or the index is not ready. func (s *ScriptExecutor) GetAccountAtBlockHeight(ctx context.Context, address flow.Address, height uint64) (*flow.Account, error) { - if !s.isDataAvailable(height) { - return nil, execution.ErrDataNotAvailable + if err := s.checkDataAvailable(height); err != nil { + return nil, err } return s.scriptExecutor.GetAccountAtBlockHeight(ctx, address, height) } -func (s *ScriptExecutor) isDataAvailable(height uint64) bool { - return s.initialized.Load() && - height <= s.indexReporter.HighestIndexedHeight() && - height >= s.indexReporter.LowestIndexedHeight() && - height <= s.maxHeight.Load() && - height >= s.minHeight.Load() +func (s *ScriptExecutor) checkDataAvailable(height uint64) error { + if !s.initialized.Load() { + return fmt.Errorf("%w: script executor not initialized", execution.ErrDataNotAvailable) + } + + if height > s.indexReporter.HighestIndexedHeight() { + return fmt.Errorf("%w: block not indexed yet", execution.ErrDataNotAvailable) + } + + if height < s.indexReporter.LowestIndexedHeight() { + return fmt.Errorf("%w: block is before lowest indexed height", execution.ErrDataNotAvailable) + } + + if height > s.maxCompatibleHeight.Load() || height < s.minCompatibleHeight.Load() { + return fmt.Errorf("%w: node software is not compatible with version required to executed block", execution.ErrDataNotAvailable) + } + + return nil } From 21347755c21a534794ddf2a73d7d54c7bac559b6 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 24 Jan 2024 09:32:01 -0800 Subject: [PATCH 3/4] add logging --- .../node_builder/access_node_builder.go | 4 +--- engine/access/rpc/backend/script_executor.go | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/cmd/access/node_builder/access_node_builder.go b/cmd/access/node_builder/access_node_builder.go index 4d33cbe277e..d2394ccb44b 100644 --- a/cmd/access/node_builder/access_node_builder.go +++ b/cmd/access/node_builder/access_node_builder.go @@ -1434,9 +1434,7 @@ func (builder *FlowAccessNodeBuilder) Build() (cmd.Node, error) { return nil }). Module("backend script executor", func(node *cmd.NodeConfig) error { - builder.ScriptExecutor = backend.NewScriptExecutor() - builder.ScriptExecutor.SetMinCompatibleHeight(builder.scriptExecMinBlock) - builder.ScriptExecutor.SetMaxCompatibleHeight(builder.scriptExecMaxBlock) + builder.ScriptExecutor = backend.NewScriptExecutor(builder.Logger, builder.scriptExecMinBlock, builder.scriptExecMaxBlock) return nil }). Module("async register store", func(node *cmd.NodeConfig) error { diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index 5265396d46c..03fec892709 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -3,16 +3,18 @@ package backend import ( "context" "fmt" - "math" "go.uber.org/atomic" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/execution" "github.com/onflow/flow-go/module/state_synchronization" + "github.com/rs/zerolog" ) type ScriptExecutor struct { + log zerolog.Logger + // scriptExecutor is used to interact with execution state scriptExecutor *execution.Scripts @@ -29,11 +31,18 @@ type ScriptExecutor struct { maxCompatibleHeight *atomic.Uint64 } -func NewScriptExecutor() *ScriptExecutor { +func NewScriptExecutor(log zerolog.Logger, minHeight, maxHeight uint64) *ScriptExecutor { + logger := log.With().Str("component", "script-executor").Logger() + logger.Info(). + Uint64("min_height", minHeight). + Uint64("max_height", maxHeight). + Msg("script executor created") + return &ScriptExecutor{ + log: logger, initialized: atomic.NewBool(false), - minCompatibleHeight: atomic.NewUint64(0), - maxCompatibleHeight: atomic.NewUint64(math.MaxUint64), + minCompatibleHeight: atomic.NewUint64(minHeight), + maxCompatibleHeight: atomic.NewUint64(maxHeight), } } @@ -41,12 +50,14 @@ func NewScriptExecutor() *ScriptExecutor { // Use this to limit the executable block range supported by the node's current software version. func (s *ScriptExecutor) SetMinCompatibleHeight(height uint64) { s.minCompatibleHeight.Store(height) + s.log.Info().Uint64("height", height).Msg("minimum compatible height set") } // SetMaxCompatibleHeight sets the highest block height (inclusive) that can be queried using local execution // Use this to limit the executable block range supported by the node's current software version. func (s *ScriptExecutor) SetMaxCompatibleHeight(height uint64) { s.maxCompatibleHeight.Store(height) + s.log.Info().Uint64("height", height).Msg("maximum compatible height set") } // InitReporter initializes the indexReporter and script executor @@ -54,6 +65,7 @@ func (s *ScriptExecutor) SetMaxCompatibleHeight(height uint64) { // made to the other methods will return execution.ErrDataNotAvailable until this method is called. func (s *ScriptExecutor) InitReporter(indexReporter state_synchronization.IndexReporter, scriptExecutor *execution.Scripts) { if s.initialized.CompareAndSwap(false, true) { + s.log.Info().Msg("script executor initialized") s.indexReporter = indexReporter s.scriptExecutor = scriptExecutor } From 7627fe865ac1bf81afa6e2e62ea7993afa5aaa59 Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 24 Jan 2024 09:38:01 -0800 Subject: [PATCH 4/4] fix lint --- engine/access/rpc/backend/script_executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/access/rpc/backend/script_executor.go b/engine/access/rpc/backend/script_executor.go index 03fec892709..f0ec0a85c27 100644 --- a/engine/access/rpc/backend/script_executor.go +++ b/engine/access/rpc/backend/script_executor.go @@ -4,12 +4,12 @@ import ( "context" "fmt" + "github.com/rs/zerolog" "go.uber.org/atomic" "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/module/execution" "github.com/onflow/flow-go/module/state_synchronization" - "github.com/rs/zerolog" ) type ScriptExecutor struct {