tower: produce vote with parent bank's blockhash#10171
Open
ibhatt-jumptrading wants to merge 5 commits into
Open
tower: produce vote with parent bank's blockhash#10171ibhatt-jumptrading wants to merge 5 commits into
ibhatt-jumptrading wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates tower vote-transaction generation to use the current tower root’s blockhash as the transaction’s recent blockhash, and adds a unit test to assert this behavior.
Changes:
- Generate vote transactions using the root slot’s
block_hash(recent blockhash) instead of the voted slot’svote_block_hash. - Add a regression test that constructs a minimal tower state and verifies the serialized vote txn uses the root blockhash.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/discof/tower/fd_tower_tile.c |
Switch vote txn recent blockhash source to the root tower block’s block_hash. |
src/discof/tower/test_tower_tile.c |
Add test coverage ensuring the vote txn recent blockhash matches the root blockhash. |
Comment on lines
+603
to
+605
| fd_txn_p_t txn[1]; | ||
| fd_hash_t const * root_blockhash = fd_type_pun_const( fd_tower_blocks_query( ctx->tower, ctx->tower->root )->block_hash.uc ); | ||
| fd_tower_to_vote_txn( ctx->tower, &out->vote_bank_hash, &out->vote_block_id, root_blockhash, ctx->identity_key, authority, ctx->vote_account, txn ); |
Comment on lines
+247
to
+248
| fd_hash_t const * root_blockhash = fd_type_pun_const( fd_tower_blocks_query( ctx->tower, ctx->tower->root )->block_hash.uc ); | ||
| fd_tower_to_vote_txn( ctx->tower, &bank_hash, &block_id, root_blockhash, &validator_identity, &validator_identity, &vote_acc, txnp ); |
Performance Measurements ⏳
|
Greptile SummaryChanges the vote transaction's
Confidence Score: 4/5This PR is a targeted, well-tested behavioral change that is safe to merge. The change is small, focused, and comes with a proper unit test. The root block is guaranteed to exist in the block map at the point of access (validated by tracing all code paths). The only minor concern is the now-unused vote_block_hash field in fd_tower_out_t, which is a cleanup opportunity but not a correctness issue. No files require special attention. The vote_block_hash field in fd_tower.h is now unused and could be cleaned up in a follow-up. Important Files Changed
Sequence DiagramsequenceDiagram
participant RT as replay_slot_completed
participant TT as tower_tile
participant TV as fd_tower_vote_and_reset
participant BM as tower block_map
participant VTX as fd_tower_to_vote_txn
RT->>TT: slot_completed msg
TT->>BM: fd_tower_blocks_insert(slot)
TT->>TV: fd_tower_vote_and_reset()
TV-->>TT: out (vote_slot, root_slot, ...)
alt root advanced
TT->>BM: remove old root blocks
TT->>TT: ctx->tower->root = out.root_slot
end
TT->>TT: publish_slot_done()
TT->>BM: fd_tower_blocks_query(tower->root)
BM-->>TT: root block (block_hash)
TT->>VTX: fd_tower_to_vote_txn(..., root_blockhash, ...)
VTX-->>TT: vote_txn with root's blockhash as recent_blockhash
Reviews (1): Last reviewed commit: "tower: produce vote with root bank's blo..." | Re-trigger Greptile |
Performance Measurements ⏳
|
Performance Measurements ⏳
|
ibhatt-jumptrading
changed the title
ibhatt-jumptrading
requested review from
emwang-jump and
lidatong
as code owners
Performance Measurements ⏳
|
Performance Measurements ⏳
|
Comment on lines
+629
to
+632
| fd_txn_p_t txn[1]; | ||
| fd_tower_blk_t * parent_tower_blk = fd_tower_blocks_query( ctx->tower, slot_completed->parent_slot ); | ||
| FD_TEST( parent_tower_blk ); | ||
| fd_hash_t const * recent_blockhash = &parent_tower_blk->block_hash; |
|
|
||
| fd_tower_blk_t * recent_blockhash_blk = fd_tower_blocks_query( ctx->tower, slot_completed.parent_slot ); | ||
| FD_TEST( recent_blockhash_blk ); | ||
| fd_hash_t const * recent_blockhash = fd_type_pun_const( recent_blockhash_blk->block_hash.uc ); |
yufeng-jump
approved these changes
Jun 12, 2026
Contributor
|
why? |
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.