Skip to content
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.

Sign up for GitHub

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

fix(ghost): voting xepoch & polish #3890

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ghost): voting xepoch & polish #3890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/app/fdctl/run/tiles/fd_replay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ after_frag( fd_replay_tile_ctx_t * ctx,
}

fd_forks_print( ctx->forks );
fd_ghost_print( ctx->ghost, ctx->epoch, fd_ghost_root( ctx-> ghost ) );
fd_ghost_print( ctx->ghost, ctx->epoch->total_stake, fd_ghost_root( ctx->ghost ) );
fd_tower_print( ctx->tower, ctx->root );

ulong vote_slot = fd_tower_vote_slot( ctx->tower, ctx->epoch, ctx->funk, child->slot_ctx.funk_txn, ctx->ghost );
Expand Down
4 changes: 2 additions & 2 deletions src/choreo/epoch/fd_epoch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ fd_epoch_init( fd_epoch_t * epoch, fd_epoch_bank_t const * epoch_bank ) {
FD_TEST( fd_epoch_voters_query( epoch_voters, voter->key, NULL ) );
#endif

voter->stake = curr->elem.stake;

voter->prev_stake = 0UL;
voter->stake = curr->elem.stake;
voter->replay_vote = FD_SLOT_NULL;
voter->gossip_vote = FD_SLOT_NULL;
voter->rooted_vote = FD_SLOT_NULL;
Expand Down
23 changes: 23 additions & 0 deletions src/choreo/epoch/test_epoch.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#include "fd_epoch.h"
#include <stdarg.h>

fd_epoch_t *
epoch( fd_wksp_t * wksp, ulong total_stake, ulong voter_cnt, ... ) {
void * epoch_mem = fd_wksp_alloc_laddr( wksp, fd_epoch_align(), fd_epoch_footprint( voter_cnt ), 1UL );
FD_TEST( epoch_mem );
fd_epoch_t * epoch = fd_epoch_join( fd_epoch_new( epoch_mem, voter_cnt ) );
FD_TEST( epoch );

va_list ap;
va_start( ap, voter_cnt );
for( ulong i = 0; i < voter_cnt; i++ ) {
fd_pubkey_t key = va_arg( ap, fd_pubkey_t );
fd_voter_t * voter = fd_epoch_voters_insert( fd_epoch_voters( epoch ), key );
voter->stake = va_arg( ap, ulong );
voter->replay_vote = FD_SLOT_NULL;
}
va_end( ap );

epoch->total_stake = total_stake;
return epoch;
}
110 changes: 37 additions & 73 deletions src/choreo/ghost/fd_ghost.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,15 +384,15 @@ fd_ghost_replay_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong slot ) {
#endif

#if FD_GHOST_USE_HANDHOLDING
int cf = __builtin_usubl_overflow( node->replay_stake, voter->stake, &node->replay_stake );
int cf = __builtin_usubl_overflow( node->replay_stake, voter->prev_stake, &node->replay_stake );
if( FD_UNLIKELY( cf ) ) FD_LOG_ERR(( "[%s] sub overflow. node stake %lu voter stake %lu", __func__, node->replay_stake, voter->stake ));
#else
node->replay_stake -= voter->stake;
#endif

fd_ghost_node_t * ancestor = node;
while( ancestor ) {
cf = __builtin_usubl_overflow( ancestor->weight, voter->stake, &ancestor->weight );
cf = __builtin_usubl_overflow( ancestor->weight, voter->prev_stake, &ancestor->weight );
#if FD_GHOST_USE_HANDHOLDING
if( FD_UNLIKELY( cf ) ) FD_LOG_ERR(( "[%s] sub overflow. ancestor->weight %lu latest_vote->stake %lu", __func__, ancestor->weight, voter->stake ));
#else
Expand Down Expand Up @@ -430,6 +430,7 @@ fd_ghost_replay_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong slot ) {
ancestor = fd_ghost_node_pool_ele( node_pool, ancestor->parent_idx );
}

voter->prev_stake = voter->stake;
voter->replay_vote = slot; /* update the cached replay vote slot on voter */
Copy link
Contributor

@emwang-jump emwang-jump Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like now voter->replay_vote will update to slot even when the if( ( slot <= last_slot ) ) return; condition is hit and we aren't actually updating to this slot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

}

Expand Down Expand Up @@ -471,54 +472,36 @@ fd_ghost_rooted_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong root ) {
node->rooted_stake += voter->stake;
}

fd_ghost_node_t const *
void
fd_ghost_publish( fd_ghost_t * ghost, ulong slot ) {
FD_LOG_NOTICE(( "[%s] slot %lu", __func__, slot ));
VER_INC;

FD_LOG_DEBUG(( "[%s] slot %lu", __func__, slot ));

fd_ghost_node_map_t * node_map = fd_ghost_node_map( ghost );
fd_ghost_node_t * node_pool = fd_ghost_node_pool( ghost );
fd_ghost_node_t const * root_node = fd_ghost_root( ghost );
ulong null_idx = fd_ghost_node_pool_idx_null( node_pool );

#if FD_GHOST_USE_HANDHOLDING
if( FD_UNLIKELY( slot < root_node->slot ) ) {
FD_LOG_WARNING(( "[fd_ghost_publish] trying to publish slot %lu older than ghost->root %lu.",
slot,
root_node->slot ));
return NULL;
}
if( FD_UNLIKELY( slot == root_node->slot ) ) {
FD_LOG_WARNING(( "[fd_ghost_publish] publishing same slot %lu as ghost->root %lu.",
slot,
root_node->slot ));
return NULL;
}
#endif

// new root
fd_ghost_node_t * root = fd_ghost_node_map_ele_query( node_map,
&slot,
NULL,
node_pool );

#if FD_GHOST_USE_HANDHOLDING
if( FD_UNLIKELY( !root ) ) {
FD_LOG_ERR(( "[fd_ghost_publish] publish slot %lu not found in ghost", slot ));
}
#if FD_GHOST_USE_HANDHOLDING
FD_TEST( slot > root_node->slot ); /* caller error */
#endif

#endif
fd_ghost_node_t * root = fd_ghost_node_map_ele_query( node_map, &slot, NULL, node_pool ); /* new root */
#if FD_GHOST_USE_HANDHOLDING
FD_TEST( root ); /* caller error */
#endif

/* First, remove the previous root, and add it to the prune list.

In this context, head is the list head (not to be confused with the
ghost head.) */

fd_ghost_node_t * head = fd_ghost_node_map_ele_remove( node_map,
&root_node->slot,
NULL,
node_pool );
head->next = fd_ghost_node_pool_idx_null( node_pool );
fd_ghost_node_t * head = fd_ghost_node_map_ele_remove( node_map, &root_node->slot, NULL, node_pool );
#if FD_GHOST_USE_HANDHOLDING
FD_TEST( head ); /* memory corruption */
#endif
head->next = null_idx;
fd_ghost_node_t * tail = head;

/* Second, BFS down the tree, adding nodes to the prune queue except
Expand All @@ -528,7 +511,6 @@ fd_ghost_publish( fd_ghost_t * ghost, ulong slot ) {

while( head ) {
fd_ghost_node_t * child = fd_ghost_node_pool_ele( node_pool, head->child_idx );

while( FD_LIKELY( child ) ) {

/* Do not prune the new root. */
Expand All @@ -537,18 +519,12 @@ fd_ghost_publish( fd_ghost_t * ghost, ulong slot ) {

/* Remove the child from the map and push the child onto the list. */

tail->next = fd_ghost_node_map_idx_remove( node_map,
&child->slot,
fd_ghost_node_pool_idx_null( node_pool ),
node_pool );
#if FD_GHOST_USE_HANDHOLDING
if( FD_UNLIKELY( tail->next == fd_ghost_node_pool_idx_null( node_pool ) ) ) {
FD_LOG_ERR(( "Failed to remove child. Child must exist given the while condition. "
"Possible memory corruption or data race." ));
}
#endif
tail->next = fd_ghost_node_map_idx_remove( node_map, &child->slot, null_idx, node_pool );
#if FD_GHOST_USE_HANDHOLDING
FD_TEST( tail->next != null_idx ); /* memory corruption */
#endif
tail = fd_ghost_node_pool_ele( node_pool, tail->next );
tail->next = fd_ghost_node_pool_idx_null( node_pool );
tail->next = null_idx;
}

child = fd_ghost_node_pool_ele( node_pool, child->sibling_idx );
Expand All @@ -565,40 +541,28 @@ fd_ghost_publish( fd_ghost_t * ghost, ulong slot ) {

root->parent_idx = null_idx;
ghost->root_idx = fd_ghost_node_map_idx_query( node_map, &slot, null_idx, node_pool );

return root;
}

fd_ghost_node_t const *
fd_ghost_gca( fd_ghost_t const * ghost, ulong slot1, ulong slot2 ) {
fd_ghost_node_t const * node_pool = fd_ghost_node_pool_const( ghost );
fd_ghost_node_t const * node1 = fd_ghost_query( ghost, slot1 );
fd_ghost_node_t const * node2 = fd_ghost_query( ghost, slot2 );

#if FD_GHOST_USE_HANDHOLDING
if( FD_UNLIKELY( !node1 ) ) {
FD_LOG_WARNING(( "slot1 %lu is missing from ghost", slot1 ));
return NULL;
}
fd_ghost_node_t const * anc1 = fd_ghost_query( ghost, slot1 );
fd_ghost_node_t const * anc2 = fd_ghost_query( ghost, slot2 );

if( FD_UNLIKELY( !node2 ) ) {
FD_LOG_WARNING(( "slot2 %lu is missing from ghost", slot2 ));
return NULL;
}
#endif
#if FD_GHOST_USE_HANDHOLDING
/* caller error */
FD_TEST( anc1 );
FD_TEST( anc2 );
#endif

/* Find the greatest common ancestor. */

while( node1 && node2 ) {
if( FD_UNLIKELY( node1->slot == node2->slot ) ) return node1;
if( node1->slot > node2->slot ) {
node1 = fd_ghost_node_pool_ele_const( node_pool, node1->parent_idx );
} else {
node2 = fd_ghost_node_pool_ele_const( node_pool, node2->parent_idx );
}
while( FD_LIKELY( anc1 && anc2 ) ) {
if( FD_UNLIKELY( anc1->slot == anc2->slot ) ) return anc1; /* found */
if( anc1->slot > anc2->slot ) anc1 = fd_ghost_parent( ghost, anc1 ); /* move anc1 up the tree */
else anc2 = fd_ghost_parent( ghost, anc2 ); /* move anc2 up the tree */
}

FD_LOG_ERR(( "Unable to find GCA. Is this a valid ghost?" ));
FD_LOG_ERR(( "[%s] invariant violation: unable to find gca", __func__ )); /* memory corruption */
}

int
Expand Down Expand Up @@ -670,8 +634,8 @@ print( fd_ghost_t const * ghost, fd_ghost_node_t const * node, int space, const
}

void
fd_ghost_print( fd_ghost_t const * ghost, fd_epoch_t const * epoch, fd_ghost_node_t const * node ) {
fd_ghost_print( fd_ghost_t const * ghost, ulong total_stake, fd_ghost_node_t const * node ) {
FD_LOG_NOTICE( ( "\n\n[Ghost]" ) );
print( ghost, node, 0, "", epoch->total_stake );
print( ghost, node, 0, "", total_stake );
printf( "\n\n" );
}
62 changes: 36 additions & 26 deletions src/choreo/ghost/fd_ghost.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@ fd_ghost_insert( fd_ghost_t * ghost, ulong parent_slot, ulong slot );

/* fd_ghost_replay_vote votes for slot, adding pubkey's stake to the
`stake` field for slot and to the `weight` field for both slot and
slot's ancestors. If pubkey has previously voted, pubkey's stake is
also subtracted from `weight` for its previous vote slot and its
ancestors.
slot's ancestors. If pubkey has previously voted, pubkey's stake as
of the previous vote slot's epoch is also subtracted from `weight`
for its previous vote slot and its ancestors.

Assumes slot is present in ghost (if handholding is enabled,
explicitly checks and errors). Returns the ghost node keyed by slot.
Expand All @@ -331,38 +331,44 @@ fd_ghost_insert( fd_ghost_t * ghost, ulong parent_slot, ulong slot );
void
fd_ghost_replay_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong slot );

/* fd_ghost_next_epoch_replay_vote is fd_ghost_replay_vote but the voter
is voting for a slot in the next epoch. */

void
fd_ghost_next_epoch_replay_vote( fd_ghost_t * ghost,
fd_voter_t * curr,
fd_voter_t * next,
ulong slot );

/* fd_ghost_gossip_vote adds stake amount to the gossip_stake field of
slot.
the node keyed by slot.

Assumes slot is present in ghost (if handholding is enabled,
explicitly checks and errors). Returns the ghost node keyed by slot.

Unlike fd_ghost_replay_vote, this stake is not propagated to
the weight field for slot and slot's ancestors. It is only counted
Unlike fd_ghost_replay_vote, this stake is not propagated to the
weight field for slot and slot's ancestors. It is only counted
towards slot itself, as gossip votes are only used for optimistic
confirmation and not fork choice. */
confirmation and not fork choice. The LMD-rule also applies to
gossip votes. */

void
fd_ghost_gossip_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong slot );

/* fd_ghost_rooted_vote adds stake amount to the rooted_stake field of
slot.

Assumes slot is present in ghost (if handholding is enabled,
explicitly checks and errors). Returns the ghost node keyed by slot.

Note rooting a slot implies rooting its ancestor, but ghost does not
explicitly track this. */
slot. Assumes slot is present in ghost (if handholding is enabled,
explicitly checks and errors). Rooting doesn't incorporate the
LMD-rule: a voter's stake can count towards multiple root votes. */

void
fd_ghost_rooted_vote( fd_ghost_t * ghost, fd_voter_t * voter, ulong root );

/* fd_ghost_publish publishes slot as the new ghost root, setting the
subtree beginning from slot as the new ghost tree (ie. slot and all
its descendants). Prunes all nodes not in slot's ancestry. Assumes
slot is present in ghost. Returns the new root. */
slot is present in ghost. */

fd_ghost_node_t const *
void
fd_ghost_publish( fd_ghost_t * ghost, ulong slot );

/* Misc */
Expand All @@ -375,26 +381,30 @@ fd_ghost_publish( fd_ghost_t * ghost, ulong slot );
FD_FN_PURE int
fd_ghost_verify( fd_ghost_t const * ghost );

/* fd_ghost_print pretty-prints a formatted ghost tree. Printing begins
from `node` (it will appear as the root in the print output).
/* fd_ghost_print pretty-prints a formatted ghost tree beginning from
`node`. If `total` is non-zero nodes will print as a percentage
`node->weight / total` otherwise will print `node->weight`.

The most straightforward way to print a ghost from the root with
percentages:

The most straightforward and commonly used printing pattern is:
`fd_ghost_print( ghost, fd_ghost_root( ghost ) )`
```
fd_ghost_print( ghost, epoch->total_stake, fd_ghost_root( ghost ) )
```

This would print ghost beginning from the root.
This would print ghost beginning from the root as percentages.

Alternatively, caller can print a more localized view, for example
starting from the grandparent of the most recently executed slot:
Another example is printing the ghost starting from the grandparent
of the most recently executed slot without percentages:

```
fd_ghost_node_t const * node = fd_ghost_query( slot );
fd_ghost_print( ghost, fd_ghost_parent( fd_ghost_parent( node ) ) )
fd_ghost_print( fd_ghost_query( slot ), 0, fd_ghost_parent( ghost, fd_ghost_parent( ghost, node ) ) );
```

Callers should add null-checks as appropriate in actual usage. */

void
fd_ghost_print( fd_ghost_t const * ghost, fd_epoch_t const * epoch, fd_ghost_node_t const * node );
fd_ghost_print( fd_ghost_t const * ghost, ulong total, fd_ghost_node_t const * node );

FD_PROTOTYPES_END

Expand Down
Loading
Loading