[DRAFT]: Core BPF Migrate Live #22
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
buffalojoec
force-pushed
the
core-bpf-migrate-live
branch
2 times, most recently
from
b91e528 to
cc9d599
Compare
buffalojoec
force-pushed
the
core-bpf-migrate-setup
branch
from
a3684d6 to
9161cac
Compare
buffalojoec
force-pushed
the
core-bpf-migrate-live
branch
2 times, most recently
from
d5b2b76 to
4d674d5
Compare
buffalojoec
commented
Apr 2, 2024
runtime/src/bank.rs
Outdated
Comment on lines
6106
to
6115
| // The built-in should be added if it has no enable feature ID | ||
| // and the feature to migrate it to Core BPF is not active. | ||
| let should_add_builtin = builtin.enable_feature_id.is_none() && { | ||
| if let Some(core_bpf_migration) = &builtin.core_bpf_migration_config { | ||
| !self.feature_set.is_active(&core_bpf_migration.feature_id) | ||
| } else { | ||
| true | ||
| } | ||
| }; | ||
| if should_add_builtin { |
There was a problem hiding this comment.
We have to figure out how to handle the case where the feature may have been activated, but the migration failed and the program wasn't migrated to BPF.
If this check is left as-is, we're toast!
buffalojoec
force-pushed
the
core-bpf-migrate-setup
branch
from
08b75cb to
5019f95
Compare
* transaction performance tracking -- streamer stage
…abs#525) * runtime: builtins: add `core_bpf_migration_config` to prototypes * runtime: builtins: set up test builtins * macro-ize builtin testing
* ElGamal: add From impls; deprecate from/to_bytes * variable names
) * Introduce SchedulingStateMachine * Apply all typo fixes from code review Co-authored-by: Andrew Fitzgerald <[email protected]> * Update word wrapping * Clarify Token::assume_exclusive_mutating_thread() * Use slice instead of &Vec<_> * Improve non-const explanation * Document consecutive readonly rescheduling opt. * Make test_gradual_locking terminate for miri * Avoid unnecessary Task::clone() * Rename: lock_{status,result} and no attempt_...() * Add safety comment for get_account_locks_unchecked * Reduce and comment about Page::blocked_tasks cap. * Document SchedulingStateMachine::schedule_task() * Add justification of closure in create_task * Use the From trait for PageUsage * Replace unneeded if-let with .expect() * Add helpful comments for peculiar crossbeam usage * Fix typo * Make bug-bounty-exempt statement more clear * Add test_enfoced_get_account_locks_verification * Fix typos... * Big rename: Page => UsageQueue * Document UsageQueueLoader * Various minor cleanings for beautifier diff * Ensure reinitialize() is maintained for new fields * Remove uneeded impl Send for TokenCell & doc upd. * Apply typo fixes from code review Co-authored-by: Andrew Fitzgerald <[email protected]> * Merge similar tests into one * Remove test_debug * Remove assertions of task_index() * Fix UB in TokenCell * Make schedule_task doc comment simpler * Document deschedule_task * Simplify unlock_usage_queue() args * Add comment for try_unblock() -> None * Switch to Option<Usage> for fewer assert!s * Add assert_matches!() to UsageQueue methods * Add panicking test case for ::reinitialize() * Use UsageFromTask * Rename: LockAttempt => LockContext * Move locking and unlocking methods to usage queue * Remove outdated comment... * Remove redundant fn: pop_unblocked_usage_from_task * Document the index of task * Clarifty comment a bit * Update .current_usage inside try_lock() * Use inspect_err to simplify code * fix ci... * Use ()... * Rename: schedule{,_next}_unblocked_task() * Rename: {try_lock,unlock}_{for_task,usage_queues} * Test solana-unified-scheduler-logic under miri * Test UB to illustrate limitation of TokenCell * Test UB of using multiple tokens at the same time --------- Co-authored-by: Andrew Fitzgerald <[email protected]>
buffalojoec
force-pushed
the
core-bpf-migrate-live
branch
from
4d674d5 to
f905e58
Compare
buffalojoec
force-pushed
the
core-bpf-migrate-live
branch
from
f905e58 to
57ebee2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is chunk 7/7 of the broken up PR 79.
Problem
Now that the full migration path is in place, it's time to wire it up to the
runtime's feature activation process.
The bank requires two main pieces of functionality:
Bankafter it has been migrated.Summary of Changes
Add logic to evaluate a
BuiltinPrototypefor a containedSome(CoreBpfMigrationConfig)and use that configuration to call intomigrate_builtin_to_core_bpffromapply_builtin_program_feature_transitionsto migrate the program to Core BPF.
Add a check to
finish_initto ensure a builtin is not added to the list if itwas migrated to Core BPF.
Add some test coverage!