-
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
[Access] add config to limit script execution range - master #5283
Conversation
minCompatibleHeight *atomic.Uint64 | ||
maxCompatibleHeight *atomic.Uint64 |
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.
are they usually configured together? Do we consider using compabitleHeightRange
?
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.
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
// 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
addressed in 2134775
builder.ScriptExecutor = backend.NewScriptExecutor() | ||
builder.ScriptExecutor.SetMinCompatibleHeight(builder.scriptExecMinBlock) | ||
builder.ScriptExecutor.SetMaxCompatibleHeight(builder.scriptExecMaxBlock) |
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
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5283 +/- ##
==========================================
- Coverage 55.60% 55.59% -0.02%
==========================================
Files 1003 1003
Lines 96785 96812 +27
==========================================
+ Hits 53818 53819 +1
- Misses 38894 38923 +29
+ Partials 4073 4070 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Backports: #5075
Limit the block range an AN will use for executing scripts and getting accounts when configured to use local data.
This is a stop-gap solution for node upgrades before #5040 is completed.
Usage:
When a cadence/fvm upgrade is planned: after the stop height is known and before it is reached, add the
--script-execution-min-height
flag passing the stop height and update the AN to the new version. This will cause the AN to stop locally executing scripts for any height below the stop height.When running a "historic" node, set
--script-execution-min-height
to the first block supported by the current version--script-execution-max-height
to the last block supported by the current versionThe node will then locally execute scripts for blocks between the heights (inclusive), and fall back to ENs for the rest.