stakes: fix total stake accumulator#9064
Merged
ibhatt-jumptrading merged 6 commits intomainfrom Mar 27, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR appears to be working toward tracking effective/activating/deactivating stake totals incrementally inside fd_stake_delegations_t, and updating stake-history epoch processing to use those totals rather than per-account rescans / per-update deltas on bank->f.total_*.
Changes:
- Added
effective_stake,activating_stake, anddeactivating_stakefields tofd_stake_delegations_tand updated delta application/marking APIs to acceptepoch,stake_history, andwarmup_cooldown_rate_epoch. - Updated epoch-activation stake history update to use the new totals fields and adjusted call sites in
fd_bank.c. - Temporarily disabled large portions of the stake delegations unit test by commenting it out.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/flamenco/stakes/test_stake_delegations.c | Marks helpers unused and comments out most test logic (currently no-op coverage). |
| src/flamenco/stakes/fd_stakes.h | Updates fd_stakes_init_totals signature (const removed) but leaves outdated doc. |
| src/flamenco/stakes/fd_stakes.c | Uses stake_delegations->{effective,activating,deactivating}_stake for epoch stake history and initializes totals by scanning delegations (but currently accumulates without reset). |
| src/flamenco/stakes/fd_stake_delegations.h | Adds totals fields and changes apply/mark/unmark delta function signatures (formatting inconsistent). |
| src/flamenco/stakes/fd_stake_delegations.c | Implements totals maintenance during apply/mark/unmark and adds fd_stake_delegation_root_query. |
| src/flamenco/runtime/fd_runtime.c | Reorders stake delegations frontier query location within epoch-boundary processing. |
| src/flamenco/runtime/fd_bank.c | Passes epoch/stake_history into apply/mark/unmark delta APIs and syncs totals on epoch change. |
| src/flamenco/rewards/fd_rewards.c | Removes per-reward adjustments to bank->f.total_*_stake. |
Comments suppressed due to low confidence (1)
src/flamenco/stakes/test_stake_delegations.c:185
- Large portions of this test have been commented out, so the binary no longer exercises stake delegation behaviors (and the helper functions are now unused). This effectively disables coverage for fork/mark/unmark/apply behavior and can let regressions through. Please either restore/replace these assertions (updating them for the new epoch/stake_history-aware APIs) or remove the test file from the build until it’s ready, rather than checking in a no-op test.
/* Test stake delegations where is_tombstone == 0 */
ulong const max_stake_accounts = 10UL;
ulong const expected_stake_accounts = fd_ulong_min( max_stake_accounts, FD_RUNTIME_EXPECTED_STAKE_ACCOUNTS );
ulong const max_live_slots = 32UL;
void * stake_delegations_mem = fd_wksp_alloc_laddr( wksp, fd_stake_delegations_align(), fd_stake_delegations_footprint( max_stake_accounts, expected_stake_accounts, max_live_slots ), wksp_tag );
FD_TEST( stake_delegations_mem );
FD_TEST( fd_stake_delegations_align()>=alignof(fd_stake_delegations_t) );
FD_TEST( fd_stake_delegations_align()==FD_STAKE_DELEGATIONS_ALIGN );
FD_TEST( !fd_stake_delegations_new( NULL, 0UL, max_stake_accounts, expected_stake_accounts, max_live_slots ) );
FD_TEST( !fd_stake_delegations_new( stake_delegations_mem, 0UL, 0UL, expected_stake_accounts, max_live_slots ) );
void * new_stake_delegations_mem = fd_stake_delegations_new( stake_delegations_mem, 0UL, max_stake_accounts, expected_stake_accounts, max_live_slots );
FD_TEST( new_stake_delegations_mem );
FD_TEST( !fd_stake_delegations_join( NULL ) );
void * junk_mem = fd_wksp_alloc_laddr( wksp, 1UL, 1UL, 999UL );
FD_TEST( junk_mem );
FD_TEST( !fd_stake_delegations_join( junk_mem ) );
fd_stake_delegations_t * stake_delegations = fd_stake_delegations_join( new_stake_delegations_mem );
FD_TEST( stake_delegations );
fd_pubkey_t stake_account_0 = { .ul = { 999UL, 999UL} };
fd_pubkey_t stake_account_1 = { .ul = { 1, 2 } };
fd_pubkey_t stake_account_2 = { .ul = { 3, 4 } };
fd_pubkey_t stake_account_3 = { .ul = { 5, 6 } };
fd_pubkey_t voter_pubkey_0 = { .ul = { 5, 6 } };
fd_pubkey_t voter_pubkey_1 = { .ul = { 7, 8 } };
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 0UL );
fd_stake_delegations_root_update( stake_delegations, &stake_account_0, &voter_pubkey_0, 100UL, 0UL, 0UL, 0UL, 0.09 );
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 1UL );
fd_stake_delegations_root_update( stake_delegations, &stake_account_1, &voter_pubkey_1, 200UL, 0UL, 0UL, 0UL, 0.09 );
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 2UL );
fd_stake_delegations_root_update( stake_delegations, &stake_account_2, &voter_pubkey_1, 300UL, 0UL, 0UL, 0UL, 0.09 );
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 3UL );
fd_stake_delegation_t const * stake_delegation_0 = test_stake_delegations_find( stake_delegations, &stake_account_0 );
FD_TEST( stake_delegation_0 );
FD_TEST( !memcmp( &stake_delegation_0->stake_account, &stake_account_0, sizeof(fd_pubkey_t) ) );
FD_TEST( !memcmp( &stake_delegation_0->vote_account, &voter_pubkey_0, sizeof(fd_pubkey_t) ) );
FD_TEST( stake_delegation_0->stake == 100UL );
FD_TEST( stake_delegation_0->activation_epoch == 0UL );
FD_TEST( stake_delegation_0->deactivation_epoch == 0UL );
FD_TEST( stake_delegation_0->warmup_cooldown_rate == FD_STAKE_DELEGATIONS_WARMUP_COOLDOWN_RATE_ENUM_009 );
fd_stake_delegation_t const * stake_delegation_1 = test_stake_delegations_find( stake_delegations, &stake_account_1 );
FD_TEST( stake_delegation_1 );
FD_TEST( !memcmp( &stake_delegation_1->stake_account, &stake_account_1, sizeof(fd_pubkey_t) ) );
FD_TEST( !memcmp( &stake_delegation_1->vote_account, &voter_pubkey_1, sizeof(fd_pubkey_t) ) );
FD_TEST( stake_delegation_1->stake == 200UL );
FD_TEST( stake_delegation_1->activation_epoch == 0UL );
FD_TEST( stake_delegation_1->deactivation_epoch == 0UL );
FD_TEST( stake_delegation_1->warmup_cooldown_rate == FD_STAKE_DELEGATIONS_WARMUP_COOLDOWN_RATE_ENUM_009 );
fd_stake_delegation_t const * stake_delegation_2 = test_stake_delegations_find( stake_delegations, &stake_account_2 );
FD_TEST( stake_delegation_2 );
FD_TEST( !memcmp( &stake_delegation_2->stake_account, &stake_account_2, sizeof(fd_pubkey_t) ) );
FD_TEST( !memcmp( &stake_delegation_2->vote_account, &voter_pubkey_1, sizeof(fd_pubkey_t) ) );
FD_TEST( stake_delegation_2->stake == 300UL );
FD_TEST( stake_delegation_2->activation_epoch == 0UL );
FD_TEST( stake_delegation_2->deactivation_epoch == 0UL );
FD_TEST( stake_delegation_2->warmup_cooldown_rate == FD_STAKE_DELEGATIONS_WARMUP_COOLDOWN_RATE_ENUM_009 );
FD_TEST( !test_stake_delegations_find( stake_delegations, &stake_account_3 ) );
fd_stake_delegations_root_update( stake_delegations, &stake_account_0, &voter_pubkey_0, 200UL, 0UL, 0UL, 0UL, 0.09 );
FD_TEST( stake_delegation_0 );
FD_TEST( !memcmp( &stake_delegation_0->stake_account, &stake_account_0, sizeof(fd_pubkey_t) ) );
FD_TEST( !memcmp( &stake_delegation_0->vote_account, &voter_pubkey_0, sizeof(fd_pubkey_t) ) );
FD_TEST( stake_delegation_0->stake == 200UL );
FD_TEST( stake_delegation_0->activation_epoch == 0UL );
FD_TEST( stake_delegation_0->deactivation_epoch == 0UL );
FD_TEST( stake_delegation_0->warmup_cooldown_rate == FD_STAKE_DELEGATIONS_WARMUP_COOLDOWN_RATE_ENUM_009 );
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 3UL );
ushort remove_fork = fd_stake_delegations_new_fork( stake_delegations );
fd_stake_delegations_fork_remove( stake_delegations, remove_fork, &stake_account_1 );
ulong epoch = 10;
fd_stake_history_t stake_history[1] = {0};
ulong warmup_cooldown_rate_epoch = 0UL;
fd_stake_delegations_apply_fork_delta( epoch, stake_history, &warmup_cooldown_rate_epoch, stake_delegations, remove_fork );
fd_stake_delegations_evict_fork( stake_delegations, remove_fork );
FD_TEST( !test_stake_delegations_find( stake_delegations, &stake_account_1 ) );
FD_TEST( fd_stake_delegations_cnt( stake_delegations ) == 2UL );
fd_stake_delegations_root_update( stake_delegations, &stake_account_1, &voter_pubkey_1, 10000UL, 0UL, 0UL, 0UL, 0.09 );
stake_delegation_1 = test_stake_delegations_find( stake_delegations, &stake_account_1 );
FD_TEST( stake_delegation_1 );
Performance Measurements ⏳
|
Performance Measurements ⏳
|
Performance Measurements ⏳
|
Performance Measurements ⏳
|
Performance Measurements ⏳
|
| for( ulong i=pool_indices_len; i>0; i-- ) { | ||
| ushort idx = pool_indices[i-1UL]; | ||
| fd_stake_delegations_unmark_delta( stake_delegations, idx ); | ||
| fd_stake_delegations_unmark_delta( stake_delegations, bank->f.epoch-1UL, stake_history, &bank->f.warmup_cooldown_rate_epoch, idx ); |
There was a problem hiding this comment.
fd_bank_stake_delegations_end_frontier_query() unmarks deltas using bank->f.epoch-1UL, but frontier queries are used in multiple places where bank->f.epoch is not incremented between mark/unmark (e.g., rewards recalculation). Using a different epoch than was used for mark_delta will make the stake total unwind incorrect and can corrupt stake_delegations->effective/activating/deactivating totals across queries. Track the epoch used when marking (e.g., pass it into end_frontier_query / unmark_deltas, or store it in the stake_delegations query context) and use that same epoch for unmarking.
Suggested change
| fd_stake_delegations_unmark_delta( stake_delegations, bank->f.epoch-1UL, stake_history, &bank->f.warmup_cooldown_rate_epoch, idx ); | |
| fd_stake_delegations_unmark_delta( stake_delegations, bank->f.epoch, stake_history, &bank->f.warmup_cooldown_rate_epoch, idx ); |
Comment on lines
+581
to
585
| fd_stake_history_t const * stake_history = fd_sysvar_cache_stake_history_join_const( &bank->f.sysvar_cache ); | ||
| for( ulong i=pool_indices_len; i>0; i-- ) { | ||
| ushort idx = pool_indices[i-1UL]; | ||
| fd_stake_delegations_apply_fork_delta( stake_delegations, idx ); | ||
| fd_stake_delegations_apply_fork_delta( bank->f.epoch, stake_history, &bank->f.warmup_cooldown_rate_epoch, stake_delegations, idx ); | ||
| } |
There was a problem hiding this comment.
fd_sysvar_cache_stake_history_join_const() can return NULL when the stake history sysvar is not valid in the cache. fd_stake_delegations_apply_fork_delta() relies on stake history to compute effective/activating/deactivating stake correctly; passing NULL will silently change stake accounting. Consider validating stake_history here (and failing loudly or reading it from accdb/xid) before applying deltas.
Comment on lines
+275
to
+281
| fd_stake_delegations_refresh( fd_stake_delegations_t * stake_delegations, | ||
| ulong epoch, | ||
| fd_stake_history_t const * stake_history, | ||
| ulong * warmup_cooldown_rate_epoch, | ||
| fd_accdb_user_t * accdb, | ||
| fd_funk_txn_xid_t const * xid ) { | ||
|
|
There was a problem hiding this comment.
fd_stake_delegations_refresh() now depends on the caller-provided stake_history to compute stake_delegations->{effective,activating,deactivating}_stake, but it does not validate stake_history. If stake_history is NULL (sysvar cache not populated yet), totals will be computed with the fallback paths and can be silently wrong. Consider either requiring a non-NULL stake_history (and FD_LOG_ERR/CRIT if missing) or reading stake history from accdb/xid inside refresh when stake_history==NULL.
Comment on lines
+18
to
23
| stake_activating_and_deactivating( fd_delegation_t const * self, | ||
| ulong target_epoch, | ||
| fd_stake_history_t const * stake_history, | ||
| ulong * new_rate_activation_epoch ); | ||
|
|
||
| fd_stake_history_entry_t |
There was a problem hiding this comment.
The new exported symbol stake_activating_and_deactivating() is declared in a public header but lacks the usual fd_ prefix, which risks symbol collisions and breaks the project’s public API naming consistency. Consider renaming it with an fd_ prefix (or keeping it file-local and using fd_stakes_activating_and_deactivating() / a new fd_* wrapper from other compilation units).
Suggested change
| stake_activating_and_deactivating( fd_delegation_t const * self, | |
| ulong target_epoch, | |
| fd_stake_history_t const * stake_history, | |
| ulong * new_rate_activation_epoch ); | |
| fd_stake_history_entry_t |
Comment on lines
+606
to
611
| fd_stake_history_t const * stake_history = fd_sysvar_cache_stake_history_join_const( &bank->f.sysvar_cache ); | ||
|
|
||
| for( ulong i=pool_indices_len; i>0; i-- ) { | ||
| ushort idx = pool_indices[i-1UL]; | ||
| fd_stake_delegations_mark_delta( stake_delegations, idx ); | ||
| fd_stake_delegations_mark_delta( stake_delegations, bank->f.epoch, stake_history, &bank->f.warmup_cooldown_rate_epoch, idx ); | ||
| } |
There was a problem hiding this comment.
fd_sysvar_cache_stake_history_join_const() can return NULL. Since fd_stake_delegations_mark_delta() now updates stake totals based on stake history, marking deltas with a NULL stake_history will produce incorrect effective/activating/deactivating totals. Add a validity check (or fallback read from accdb/xid) before calling mark_delta.
Comment on lines
+121
to
+124
| /* Stake totals for the current root. */ | ||
| ulong effective_stake; | ||
| ulong activating_stake; | ||
| ulong deactivating_stake; |
There was a problem hiding this comment.
Stake totals were added to fd_stake_delegations_t as part of the root state, but fd_stake_delegations_init() only resets the root map/pool. This can leave stale effective/activating/deactivating totals after init even though the root map is empty. Consider resetting these totals in fd_stake_delegations_init() so the struct is internally consistent after reinit.
| } else { | ||
| /* If the stake delegation in the delta is a tombstone, just | ||
| remove the stake delegation from the root map and subtract | ||
| it's stake from the totals. */ |
There was a problem hiding this comment.
Comment typo: use the possessive "its" (not "it's").
Suggested change
| it's stake from the totals. */ | |
| its stake from the totals. */ |
Comment on lines
+1131
to
+1139
| bank->f.warmup_cooldown_rate_epoch = fd_slot_to_epoch( &bank->f.epoch_schedule, bank->f.features.reduce_stake_warmup_cooldown, NULL ); | ||
| fd_stake_delegations_t * root_delegations = fd_banks_stake_delegations_root_query( ctx->banks ); | ||
| fd_stake_delegations_refresh( root_delegations, ctx->accdb, &xid ); | ||
|
|
||
| bank->f.warmup_cooldown_rate_epoch = fd_slot_to_epoch( &bank->f.epoch_schedule, | ||
| bank->f.features.reduce_stake_warmup_cooldown, | ||
| NULL ); | ||
|
|
||
| fd_stakes_init_totals( bank, root_delegations, ctx->accdb, &xid ); | ||
| fd_stake_delegations_refresh( | ||
| root_delegations, | ||
| bank->f.epoch, | ||
| fd_sysvar_cache_stake_history_join_const( &bank->f.sysvar_cache ), | ||
| &bank->f.warmup_cooldown_rate_epoch, | ||
| ctx->accdb, | ||
| &xid ); |
There was a problem hiding this comment.
fd_stake_delegations_refresh() is called with fd_sysvar_cache_stake_history_join_const(&bank->f.sysvar_cache) but the return value can be NULL when stake history isn't marked valid in the sysvar cache after snapshot load. That will silently change stake totals computation. Consider ensuring stake history is loaded/valid (or reading it from accdb/xid as a fallback) before refresh.
kbhargava-jump
approved these changes
Mar 27, 2026
This file contains hidden or 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
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.
No description provided.