set-identity: adding support for alpenglow [wip] - #11373
set-identity: adding support for alpenglow [wip]#11373ibhatt-jumptrading wants to merge 2 commits into
Conversation
┌─ ⚡ PERF · 2982644 vs main@548c20c ─────────────────────────────────
│ SUITE BASELINE NEW Δ
│ replay tps, mainnet 29,593 tps 29,558 tps · -0.12%
│ bench tps, localnet 128,647 tps 128,645 tps · 0.00%
│ snapshot load, testnet 9.27 s 9.17 s · -1.07%
│ mem total, mainnet 171.43 GiB 171.43 GiB · 0.00%
│ mem total, testnet 102.30 GiB 102.30 GiB · 0.00%
│ clean compile, firedancer 232.3 cpu·s 228.6 cpu·s · -1.60%
│ binary size, firedancer 85.84 MB 85.86 MB · +0.02%
├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 0 IMPROVED · 7 NOISE @@
└─────────────────────────────────────────────────────────────────────history · 3 pushes ┌─ HISTORY · Δ vs main, per push, newest first ─────────────────────────
│ HEAD TPS BENCH SNAP MEM·M MEM·T COMPILE BINARY
│ 2982644 -0.12% 0.00% -1.07% 0.00% 0.00% -1.60% +0.02%
│ 8e643e5 -0.01% 0.00% +0.41% 0.00% 0.00% +0.62% +0.02%
!│ 658fa31 -0.53% 0.00% -0.40% 0.00% 0.00% +0.98% +0.01%
└─────────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
🟡 Changes recommended
fd_votor_tile.c advances replay_in_seq in before_frag even for replay fragments that are rejected/returned, which can incorrectly satisfy the keyswitch replay barrier and switch identities too early.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds infrastructure to support identity switching (“set-identity”) in Alpenglow mode by coordinating Replay + consensus (Tower/Votor) + TxSend/signers, and by routing Votor’s QUIC CertificateVerify signing through keyguard with a dedicated role.
Changes:
- Implement Replay-driven identity switch quiescing with a pause-sequence barrier for downstream consumers (Tower/Votor), and update admin set-identity state machine accordingly.
- Add Votor QUIC CertificateVerify signing via keyguard (new
FD_KEYGUARD_ROLE_VOTOR) and wire new sign links in Firedancer topology. - Add/extend unit tests for identity switching behavior across Replay/Tower/Votor, plus keyguard authorization coverage.
File summaries
| File | Description |
|---|---|
| src/discof/votor/test_votor_tile.c | New unit test covering Votor identity keyswitch housekeeping behavior. |
| src/discof/votor/Local.mk | Adds test_votor_tile unit test target. |
| src/discof/votor/fd_votor_tile.c | Adds Votor keyswitch state handling, halting logic, keyguard QUIC signer, and replay sequence/slot tracking. |
| src/discof/tower/test_tower_tile.c | Adds tests validating Tower keyswitch barriers (replay seq, publish drain, replay backpressure). |
| src/discof/tower/fd_tower_tile.c | Tracks replay input sequence and uses it to gate identity switching; adjusts replay frag handling. |
| src/discof/replay/test_replay_tile.c | Adds test that identity switching quiesces replay and reports pause sequence. |
| src/discof/replay/fd_replay_tile.c | Reports replay_out pause sequence in keyswitch result; renames halt flag and quiesces work while halted. |
| src/discof/replay/fd_replay_tile_private.h | Renames halt_leader to halt_replay and stores replay_out_seq pointer. |
| src/discof/admin/fd_admin_tile.c | Reworks set-identity state machine to pause Replay first, then consensus, then TxSend/signers, then unpause Replay. |
| src/disco/keyguard/test_keyguard.c | Adds unit coverage for Votor TLS CertificateVerify authorization. |
| src/disco/keyguard/fuzz_keyguard.c | Extends fuzz to attempt authorization for TLS-CV payloads under Votor role. |
| src/disco/keyguard/fd_sign_tile.c | Adds sign-tile link/role wiring for votor_sign/sign_votor. |
| src/disco/keyguard/fd_keyguard.h | Introduces FD_KEYGUARD_ROLE_VOTOR and bumps role count. |
| src/disco/keyguard/fd_keyguard_authorize.c | Adds payload authorization rules for Votor TLS 1.3 CertificateVerify signing. |
| src/choreo/votor/test_ag_votor.c | Adds test ensuring identity switch discards pending identity-specific vote decisions. |
| src/choreo/votor/test_ag_pool.c | Adds test ensuring pool identity switch updates own-rank tracking. |
| src/choreo/votor/ag_votor.h | Declares new ag_votor_set_identity API. |
| src/choreo/votor/ag_votor.c | Implements ag_votor_set_identity. |
| src/choreo/votor/ag_pool.h | Declares new ag_pool_set_identity API. |
| src/choreo/votor/ag_pool.c | Implements ag_pool_set_identity. |
| src/app/firedancer/topology.c | Adds keyguard workspaces/links and sign wiring for Votor signing when Alpenglow is enabled. |
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| case IN_KIND_REPLAY: | ||
| ctx->replay_in_seq = seq+1UL; | ||
| if( FD_UNLIKELY( !ctx->curr_epoch_info ) ) return 1; | ||
| return sig!=REPLAY_SIG_SLOT_COMPLETED && sig!=REPLAY_SIG_SLOT_DEAD; |
658fa31 to
8e643e5
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The admin set-identity state machine reads/writes shared fd_keyswitch_t state via plain loads/stores in places, which should use the fd_keyswitch_state_query/fd_keyswitch_state helpers to avoid undefined behavior under optimization and ensure correct cross-tile synchronization.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
src/discof/admin/fd_admin_tile.c:342
- Keyswitch state is read via plain loads (e.g. replay->state). Since this is shared memory concurrently modified by other tiles, use fd_keyswitch_state_query() (volatile load + fences) to avoid undefined behavior and potential compiler hoisting of the load in optimized builds.
if( FD_LIKELY( replay->state==FD_KEYSWITCH_STATE_COMPLETED ) ) {
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
| FD_COMPILER_MFENCE(); | ||
| replay->state = FD_KEYSWITCH_STATE_SWITCH_PENDING; | ||
| FD_COMPILER_MFENCE(); |
8e643e5 to
1d9364e
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It modifies a security- and liveness-critical identity-switch state machine across multiple tiles (Replay/consensus/TxSend/signing/topology), so it warrants final human review despite added tests.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 1
- Review effort level: Lite
| case FD_KEYGUARD_ROLE_VOTOR: { | ||
| static char const client_prefix[ 98 ] = | ||
| " " | ||
| " " | ||
| "TLS 1.3, client CertificateVerify"; | ||
| static char const server_prefix[ 98 ] = | ||
| " " | ||
| " " | ||
| "TLS 1.3, server CertificateVerify"; | ||
| int tls_ok = !!( payload_mask & FD_KEYGUARD_PAYLOAD_TLS_CV ) && | ||
| sign_type==FD_KEYGUARD_SIGN_TYPE_ED25519 && | ||
| sz==130UL && | ||
| ( fd_memeq( client_prefix, data, sizeof(client_prefix) ) || | ||
| fd_memeq( server_prefix, data, sizeof(server_prefix) ) ); | ||
| if( FD_UNLIKELY( !tls_ok ) ) { | ||
| FD_LOG_WARNING(( "unauthorized payload type for votor (mask=%#lx)", payload_mask )); | ||
| return 0; | ||
| } | ||
| return 1; | ||
| } |
1d9364e to
1eb9bd8
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The Votor unhalt path currently uses FD_TEST around fd_quic_init, which can abort the validator on transient init failure instead of remaining safely halted and retrying.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 1
- Review effort level: Lite
| if( FD_UNLIKELY( fd_keyswitch_state_query( ctx->identity_keyswitch )==FD_KEYSWITCH_STATE_UNHALT_PENDING ) ) { | ||
| FD_CHECK_CRIT( ctx->halted, "state machine corruption" ); | ||
| memcpy( ctx->quic_client->config.identity_public_key, ctx->id_key.uc, sizeof(fd_pubkey_t) ); | ||
| memcpy( ctx->quic_server->config.identity_public_key, ctx->id_key.uc, sizeof(fd_pubkey_t) ); | ||
| FD_TEST( fd_quic_init( ctx->quic_client ) ); | ||
| FD_TEST( fd_quic_init( ctx->quic_server ) ); | ||
| ctx->halted = 0; | ||
| connect_peers( ctx, fd_log_wallclock() ); | ||
| fd_keyswitch_state( ctx->identity_keyswitch, FD_KEYSWITCH_STATE_COMPLETED ); | ||
| } |
1eb9bd8 to
2982644
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes cross-tile identity-switch coordination (Replay/consensus/TxSend/sign) and validator control flow, which is high-impact and warrants careful human validation beyond automated review.
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 1
- Review effort level: Lite
| } else if ( !strcmp(in_link->name, "votor_sign" ) ) { | ||
| ctx->in[ i ].role = FD_KEYGUARD_ROLE_VOTOR; | ||
| FD_TEST( !strcmp( out_link->name, "sign_votor" ) ); | ||
| FD_TEST( in_link->mtu==FD_KEYGUARD_SIGN_REQ_MTU ); | ||
| FD_TEST( out_link->mtu==64UL ); |
No description provided.