-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Bumps solana_rbpf to v0.2.14 #18869
Bumps solana_rbpf to v0.2.14 #18869
Conversation
4075902
to
a6723c4
Compare
Codecov Report
@@ Coverage Diff @@
## master #18869 +/- ##
=========================================
- Coverage 82.8% 82.7% -0.1%
=========================================
Files 449 449
Lines 127997 128047 +50
=========================================
- Hits 105993 105983 -10
- Misses 22004 22064 +60 |
@jackcmay Can you see if the feature gate looks about right? If so you can merge. |
Sounds good, reviewing and will commit if it looks good, thanks! |
@@ -83,6 +82,8 @@ pub fn create_executor( | |||
max_call_depth: compute_budget.max_call_depth, | |||
stack_frame_size: compute_budget.stack_frame_size, | |||
enable_instruction_tracing: log_enabled!(Trace), | |||
verify_mul64_imm_nonzero: !invoke_context | |||
.is_feature_active(&stop_verify_mul64_imm_nonzero::id()), // TODO: Feature gate and then remove me | |||
..Config::default() |
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 the stack frame gaps disabled by default now? If so they should be featurized
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.
Is the Config's instruction_meter_checkpoint_distance
default or the compute meter handling change in v0.2.14?? If so should be feature gated
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.
Still enabled by default: enable_stack_frame_gaps: true
https://github.com/solana-labs/rbpf/blob/302de4d10888dfb76534693308f4ef8060b86fcd/src/vm.rs#L206
Should we disable it in the next release?
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.
instruction_meter_checkpoint_distance
should have no influence on what the program sees, the point where programs are aborted and what error is thrown has to be the same for interpreter and JIT anyway. The option only makes the detection of that condition faster (so it affects the performance only).
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.
Sounds good as long as there isn't an edge case that's falls in the middle. Also, maybe check with Tao about cost model stuff to make sure this change doesn't result in a behavioral change there.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in #17520.
This reverts commit 9d66458.
This reverts commit 9d66458.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in solana-labs#17520.
* Feature gate for verify_mul64_imm_nonzero as discussed in #17520.
Problem
Summary of Changes
See RBPF release
Fixes #17520
Fixes #18683