fix: genesis: feature-gated vote state#9467
Conversation
|
The Firedancer team maintains a line-for-line reimplementation of the |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #9467 +/- ##
===========================================
- Coverage 82.7% 67.5% -15.2%
===========================================
Files 901 803 -98
Lines 324743 225552 -99191
===========================================
- Hits 268704 152471 -116233
- Misses 56039 73081 +17042 🚀 New features to boost your workflow:
|
|
If this PR represents a change to the public RPC API:
Thank you for keeping the RPC clients in sync with the server API @buffalojoec. |
AshwinSekar
left a comment
There was a problem hiding this comment.
LGTM, cc @t-nelson for approval since I believe we want to backport this to 3.1
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
|
can we get the feature set plumbing changes factored out of the first commit and i think the last two commits of ag slop dropped? |
Ok so you basically want just a simpler fix here for the sake of backport? I'll probably just rewrite the commits. |
63c88b8 to
3899768
Compare
@t-nelson I think this new commit will backport better, and avoids any unnecessary refactoring. |
3899768 to
c16465b
Compare
runtime/src/genesis_utils.rs
Outdated
| initial_accounts: Vec<(Pubkey, AccountSharedData)>, | ||
| mut initial_accounts: Vec<(Pubkey, AccountSharedData)>, | ||
| ) -> GenesisConfig { | ||
| if cluster_type == ClusterType::Development { |
There was a problem hiding this comment.
this isn't really the dependency tho. we should
- factor feature gate activation as early as possible
- plumb the feature set to where it needs to be
- select the correct account format based on the feature gate status
There was a problem hiding this comment.
This is precisely what my first PR was doing - maybe I misunderstood your request, but you said you wanted feature set plumbing factored out of the commit?
There was a problem hiding this comment.
yeah I just wanted it in separate commits so it was actually reviewable
There was a problem hiding this comment.
Ok I pushed up commits as you requested.
|
|
||
| #[cfg(test)] | ||
| pub fn create_account_with_authorized( | ||
| pub fn create_v3_account_with_authorized( |
c16465b to
f6eacf8
Compare
f6eacf8 to
f080d70
Compare
| ) | ||
| } else { | ||
| // Vote state v4 feature inactive. Create a v3 account. | ||
| if bls_pubkey_compressed.is_some() { |
There was a problem hiding this comment.
probably for a followup, but this can probably be enforced by clap. if not, it should at least be moved up to where cli args are initially resolved
* factor feature gate activation as early as possible * plumb the feature set to where it needs to be * select the proper account format based on the feature gate status * cleanup * apply similar patch to solana-genesis (cherry picked from commit 50c813a) # Conflicts: # genesis/src/main.rs # test-validator/src/lib.rs
* factor feature gate activation as early as possible * plumb the feature set to where it needs to be * select the proper account format based on the feature gate status * cleanup * apply similar patch to solana-genesis
fix: genesis: feature-gated vote state (#9467) * factor feature gate activation as early as possible * plumb the feature set to where it needs to be * select the proper account format based on the feature gate status * cleanup * apply similar patch to solana-genesis Co-authored-by: Joe C <joe.caulfield@anza.xyz>
Problem
As described in #9377, development clusters who deactivate the vote state v4 feature are stalling since the genesis config will create vote state v4 accounts anyway.
Summary of Changes
Changes the no-features variant (
create_genesis_config_with_leader_ex_no_features) to first check the providedinitial_accountsfor the vote account, and default to vote state V3 if not provided. For the variant with all features enabled (create_genesis_config_with_leader_ex), use vote state V4.Also removes the BLS pubkey arg from the no-features variant since it's unused.
Fixes #9377