-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Access] add config to limit script execution range - master #5283
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,8 @@ package backend | |
|
||
import ( | ||
"context" | ||
"sync" | ||
"fmt" | ||
"math" | ||
|
||
"go.uber.org/atomic" | ||
|
||
|
@@ -21,25 +22,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 | ||
// 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 | ||
minCompatibleHeight *atomic.Uint64 | ||
maxCompatibleHeight *atomic.Uint64 | ||
Comment on lines
+30
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are they usually configured together? Do we consider using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, generally only one would be defined. both are only defined for ANs running a version that's older than the HCU version. FWIW, we'll be able to remove these once the version beacon support is added |
||
} | ||
|
||
func NewScriptExecutor() *ScriptExecutor { | ||
return &ScriptExecutor{ | ||
initialized: atomic.NewBool(false), | ||
initialized: atomic.NewBool(false), | ||
minCompatibleHeight: atomic.NewUint64(0), | ||
maxCompatibleHeight: atomic.NewUint64(math.MaxUint64), | ||
} | ||
} | ||
|
||
// 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) SetMinCompatibleHeight(height uint64) { | ||
s.minCompatibleHeight.Store(height) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better to log the change of the height range? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. addressed in 2134775 |
||
} | ||
|
||
// 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) | ||
} | ||
|
||
// 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. | ||
|
@@ -49,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) | ||
|
@@ -63,13 +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() | ||
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 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can initialize the script executor with the height range? and also log the actual height range?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in 2134775