Feature - Epoch boundary recompilation phase #33477

Lichtso · 2023-10-01T21:55:04Z

Problem

This is a different approach to the problem as #32172 was flawed. It ended the recompilation phase at the first block after the epoch boundary, but we need to wait for a rerooting to be sure that we won't replay any blocks from before the epoch boundary anymore.

Summary of Changes

Adds LoadedPrograms::upcoming_environments: Which is Some during the recompilation phase, even if there is no change to any environment.
Moves LoadedPrograms::prune_feature_set_transition() into LoadedPrograms::prune(): Delays it to happen on the first rerooting after the epoch boundary.
Removes the manually managed FEATURES_AFFECTING_RBPF list: Because we can now automatically detect changes by using BuiltinProgram::eq().
Adds parameter recompile to Bank::load_program(): Which is Some when called from within the recompilation phase.

runtime/src/bank.rs

runtime/src/bank/tests.rs

codecov · 2023-10-03T00:59:13Z

Codecov Report

Merging #33477 (a5db6d5) into master (1814b2b) will decrease coverage by 0.1%.
The diff coverage is 77.2%.

@@            Coverage Diff            @@
##           master   #33477     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         809      809             
  Lines      218062   218105     +43     
=========================================
+ Hits       178672   178684     +12     
- Misses      39390    39421     +31

pgarg66

Looks great!

runtime/src/bank.rs

pgarg66 · 2023-10-26T22:01:34Z

runtime/src/bank.rs

+                    let mut loaded_programs_cache = new.loaded_programs_cache.write().unwrap();
+                    loaded_programs_cache.replenish(key, recompiled);
+                }
+            } else if new.epoch() != loaded_programs_cache.latest_root_epoch


Could a malicious leader trigger this check by adding a bank in a future epoch? Does it make sense to make this check more specific (e.g. new.epoch == loaded_programs_cache.latest_root_epoch + 1, and slot is within N slots in the new epoch)?

Aren't leaders bound by the leader schedule in that they can't just submit a block for any slot, but only the one they were assigned to?

I remember seeing this in one of the outages where the leader submitted a slot way in future. Maybe its not an issue anymore due to other checks.

Would making this check more strict cause any problems? It can be done in a separate PR if that's preferred.

https://github.com/solana-labs/solana/blob/1814b2bc81aca29b322f8e1f280494c1b9ee6b32/turbine/src/sigverify_shreds.rs#L141C6-L141C6

Would making this check more strict cause any problems?

Probably not, but haven't thought it through.

runtime/src/bank.rs

…ams::prune().

* Adds LoadedPrograms::upcoming_environments. * Moves LoadedPrograms::prune_feature_set_transition() into LoadedPrograms::prune(). * Adds parameter recompile to Bank::load_program(). * Sets latest_root_slot/epoch and environments in Bank::finish_init(). * Removes FEATURES_AFFECTING_RBPF list. * Adjusts test_feature_activation_loaded_programs_recompilation_phase(). (cherry picked from commit a9509f5) # Conflicts: # runtime/src/bank/tests.rs

* Adds LoadedPrograms::upcoming_environments. * Moves LoadedPrograms::prune_feature_set_transition() into LoadedPrograms::prune(). * Adds parameter recompile to Bank::load_program(). * Sets latest_root_slot/epoch and environments in Bank::finish_init(). * Removes FEATURES_AFFECTING_RBPF list. * Adjusts test_feature_activation_loaded_programs_recompilation_phase(). (cherry picked from commit a9509f5)

…) (#34003) Feature - Epoch boundary recompilation phase (#33477) * Adds LoadedPrograms::upcoming_environments. * Moves LoadedPrograms::prune_feature_set_transition() into LoadedPrograms::prune(). * Adds parameter recompile to Bank::load_program(). * Sets latest_root_slot/epoch and environments in Bank::finish_init(). * Removes FEATURES_AFFECTING_RBPF list. * Adjusts test_feature_activation_loaded_programs_recompilation_phase(). (cherry picked from commit a9509f5) Co-authored-by: Alexander Meißner <[email protected]>

…na-labs#33477) (solana-labs#34003) Feature - Epoch boundary recompilation phase (solana-labs#33477) * Adds LoadedPrograms::upcoming_environments. * Moves LoadedPrograms::prune_feature_set_transition() into LoadedPrograms::prune(). * Adds parameter recompile to Bank::load_program(). * Sets latest_root_slot/epoch and environments in Bank::finish_init(). * Removes FEATURES_AFFECTING_RBPF list. * Adjusts test_feature_activation_loaded_programs_recompilation_phase(). (cherry picked from commit a9509f5) Co-authored-by: Alexander Meißner <[email protected]>

Lichtso requested a review from pgarg66 October 1, 2023 21:55

Lichtso force-pushed the feature_transition_recompilation branch 3 times, most recently from c74fd46 to 496b3d8 Compare October 2, 2023 09:09

Lichtso mentioned this pull request Oct 2, 2023

Refactor - LoadedPrograms #33482

Merged

Lichtso commented Oct 2, 2023

View reviewed changes

runtime/src/bank.rs Show resolved Hide resolved

Lichtso commented Oct 2, 2023

View reviewed changes

runtime/src/bank/tests.rs Outdated Show resolved Hide resolved

Lichtso force-pushed the feature_transition_recompilation branch 4 times, most recently from 2cd41cf to b82339f Compare October 2, 2023 23:22

pgarg66 reviewed Oct 2, 2023

View reviewed changes

runtime/src/bank/tests.rs Outdated Show resolved Hide resolved

Lichtso force-pushed the feature_transition_recompilation branch 3 times, most recently from e5d13d3 to 0fac427 Compare October 3, 2023 13:51

pgarg66 previously approved these changes Oct 3, 2023

View reviewed changes

Lichtso dismissed pgarg66’s stale review via 82231d6 October 11, 2023 17:06

Lichtso force-pushed the feature_transition_recompilation branch 3 times, most recently from 7a850e0 to b9c2b05 Compare October 13, 2023 17:41

Lichtso mentioned this pull request Oct 13, 2023

Refactor - LoadedPrograms part 2 #33694

Merged

Lichtso force-pushed the feature_transition_recompilation branch from b9c2b05 to 3a9ca95 Compare October 13, 2023 20:48

Lichtso force-pushed the feature_transition_recompilation branch 3 times, most recently from a29f8cc to 0d25f22 Compare October 25, 2023 08:23

Lichtso added the v1.17 PRs that should be backported to v1.17 label Oct 25, 2023

Lichtso force-pushed the feature_transition_recompilation branch from 0d25f22 to a5814fe Compare October 26, 2023 19:27

pgarg66 reviewed Oct 26, 2023

View reviewed changes

Lichtso force-pushed the feature_transition_recompilation branch from a5814fe to 5347856 Compare October 27, 2023 11:02

pgarg66 reviewed Oct 27, 2023

View reviewed changes

runtime/src/bank.rs Show resolved Hide resolved

Lichtso force-pushed the feature_transition_recompilation branch 2 times, most recently from 8e7c67d to 1bad18d Compare October 27, 2023 17:51

Lichtso added 6 commits October 27, 2023 20:08

Adds LoadedPrograms::upcoming_environments.

79e40b5

Moves LoadedPrograms::prune_feature_set_transition() into LoadedProgr…

ee5837a

…ams::prune().

Adds parameter recompile to Bank::load_program().

9837443

Sets latest_root_slot/epoch and environments in Bank::finish_init().

7f68eff

Removes FEATURES_AFFECTING_RBPF list.

47cdaea

Adjusts test_feature_activation_loaded_programs_recompilation_phase().

a5db6d5

Lichtso force-pushed the feature_transition_recompilation branch from 1bad18d to a5db6d5 Compare October 27, 2023 18:09

pgarg66 approved these changes Oct 27, 2023

View reviewed changes

Lichtso mentioned this pull request Nov 7, 2023

Program-Runtime v2 - Road Map #28755

Open

Lichtso merged commit a9509f5 into solana-labs:master Nov 9, 2023
17 checks passed

Lichtso deleted the feature_transition_recompilation branch November 9, 2023 12:11

mergify bot mentioned this pull request Nov 9, 2023

v1.17: Feature - Epoch boundary recompilation phase (backport of #33477) #34003

Merged

HaoranYi mentioned this pull request Apr 8, 2024

pr634 1.17 new filter anza-xyz/agave#657

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature - Epoch boundary recompilation phase #33477

Feature - Epoch boundary recompilation phase #33477

Lichtso commented Oct 1, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading

pgarg66 left a comment

pgarg66 Oct 26, 2023

Lichtso Oct 27, 2023

pgarg66 Oct 27, 2023

Lichtso Oct 27, 2023

Feature - Epoch boundary recompilation phase #33477

Feature - Epoch boundary recompilation phase #33477

Conversation

Lichtso commented Oct 1, 2023 • edited Loading

Problem

Summary of Changes

codecov bot commented Oct 3, 2023 • edited Loading

Codecov Report

pgarg66 left a comment

Choose a reason for hiding this comment

pgarg66 Oct 26, 2023

Choose a reason for hiding this comment

Lichtso Oct 27, 2023

Choose a reason for hiding this comment

pgarg66 Oct 27, 2023

Choose a reason for hiding this comment

Lichtso Oct 27, 2023

Choose a reason for hiding this comment

Lichtso commented Oct 1, 2023 •

edited

Loading

codecov bot commented Oct 3, 2023 •

edited

Loading