From 033ae9c6580b054e52905cc799e513c554591f42 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Tue, 8 Sep 2026 15:18:01 +0000 Subject: [PATCH 1/2] chainer: cleanup abandon references --- src/discof/chainer/fd_chainer.c | 110 +++++++---------------------- src/discof/chainer/fd_chainer.h | 39 ++++++---- src/discof/chainer/test_chainer.c | 50 ++++++------- src/discof/rotor/fd_rotor_tile.h | 50 ++++++------- src/discof/rotor/test_rotor_tile.c | 85 +++++++++++----------- 5 files changed, 141 insertions(+), 193 deletions(-) diff --git a/src/discof/chainer/fd_chainer.c b/src/discof/chainer/fd_chainer.c index 3195444fead..1e46672778e 100644 --- a/src/discof/chainer/fd_chainer.c +++ b/src/discof/chainer/fd_chainer.c @@ -98,7 +98,6 @@ acquire_slotv( fd_chainer_t * chainer, ulong slot ) { fd_chainer_slotv_t * slotv = fd_slotv_pool_ele_acquire( slotv_pool ); slotv->slot = slot; slotv->turbine = 0; - slotv->abandoned = 0; slotv->parent_slot = AG_UNKNOWN_SLOT; slotv->parent_slot_batch = UINT_MAX; slotv->complete_idx = UINT_MAX; @@ -250,29 +249,6 @@ fec_join( fd_chainer_t * chainer, return fec; } -/* slotv_abandon freezes a turbine slotv. Removed from the repair - worklists, and (via the abandoned flag) excluded from delivery and - block_id finalization. */ -static void -slotv_abandon( fd_chainer_t * chainer, fd_chainer_slotv_t * slotv ) { - FD_TEST( slotv->turbine ); - fd_chainer_repair_remove( chainer, slotv ); - fd_chainer_orphan_remove( chainer, slotv ); - slotv->abandoned = 1; -} - -/* abandon_turbine abandons slot's turbine version, if one exists. */ - -static void -abandon_turbine( fd_chainer_t * chainer, ulong slot ) { - for( ulong i=slotv_iter_init( chainer, slot ); i!=ULONG_MAX; i=slotv_iter_next( chainer, i ) ) { - fd_chainer_slotv_t * slotv = slotv_iter_ele( chainer, i ); - if( FD_LIKELY( !slotv->turbine || slotv->abandoned ) ) continue; - if( FD_LIKELY( fd_hash_check_zero( &slotv->block_id ) ) ) slotv_abandon( chainer, slotv ); - return; - } -} - /* turbine_slotv_query returns the turbine version of slot -- creating it if none exists.*/ @@ -366,12 +342,6 @@ fd_chainer_verify( fd_chainer_t const * chainer ) { if( FD_UNLIKELY( slotv->buffered_fec_idx!=UINT_MAX && ( slotv->buffered_idx==UINT_MAX || slotv->buffered_idxbuffered_fec_idx ) ) ) FAIL( "buffered_fec_idx runs ahead of buffered_idx" ); - - /* An abandoned version is always a turbine version and never on a - worklist (see slotv_abandon). */ - - if( FD_UNLIKELY( slotv->abandoned && !slotv->turbine ) ) FAIL( "abandoned non-turbine slotv" ); - if( FD_UNLIKELY( slotv->abandoned && ( fd_chainer_in_repair( chainer_, slotv ) || fd_chainer_in_orphan( chainer_, slotv ) ) ) ) FAIL( "abandoned slotv on a worklist" ); } /* Worklist consistency. Every sched ele must shadow a live slotv, be @@ -386,7 +356,7 @@ fd_chainer_verify( fd_chainer_t const * chainer ) { !fd_sched_map_iter_done( it, sched_map, sched_pool ); it = fd_sched_map_iter_next( it, sched_map, sched_pool ) ) { fd_sched_ele_t const * ele = fd_sched_map_iter_ele_const( it, sched_map, sched_pool ); - if( FD_UNLIKELY( ele->slotv_idx>=ele_max ) ) FAIL( "sched ele slotv_idx out of range" ); + if( FD_UNLIKELY( ele->slotv_idx>=ele_max ) ) FAIL( "sched ele slotv_idx out of range" ); if( FD_UNLIKELY( !ele->in_repair && !ele->in_orphan ) ) FAIL( "sched ele in neither treap (should be gc'd)" ); if( FD_UNLIKELY( ele->slot!=fd_slotv_pool_ele_const( slotv_pool, ele->slotv_idx )->slot ) ) FAIL( "sched ele slot mismatches slotv" ); in_treap_cnt += !!ele->in_repair; @@ -490,18 +460,6 @@ fd_chainer_shred_insert( fd_chainer_t * chainer, fd_chainer_slotv_t * turbine = turbine_slotv_query( chainer, slot ); - /* If a votor-driven version of the slot already exists (block-id - repair started before this turbine shred arrived), abandon the - turbine version. */ - if( FD_UNLIKELY( !turbine->abandoned && fd_hash_check_zero( &turbine->block_id ) ) ) { - for( ulong i=slotv_iter_init( chainer, slot ); i!=ULONG_MAX; i=slotv_iter_next( chainer, i ) ) { - if( FD_UNLIKELY( i!=fd_slotv_pool_idx( chainer->slotv_pool, turbine ) ) ) { - slotv_abandon( chainer, turbine ); - break; - } - } - } - /* Find or create the FEC for this shred's root If the turbine version holds no root at this position it adopts @@ -545,7 +503,7 @@ fd_chainer_shred_insert( fd_chainer_t * chainer, slotv->parent_slot_batch = shred_idx; FD_TEST( parent_block_id ); /* TODO do handholding check */ if( fd_chainer_slot_version_query( chainer, parent_slot, parent_block_id ) ) { - // TODO is this safe? -- check for FLH case + // TODO is this safe in FLH case? fd_chainer_orphan_remove( chainer, slotv ); /* no longer unknown */ } @@ -584,7 +542,6 @@ chainer_advance( fd_chainer_t * chainer, fd_chainer_slotv_t * root ) { while( FD_LIKELY( !bfs_empty( bfs ) ) ) { fd_chainer_slotv_t * slotv = fd_slotv_pool_ele( slotv_pool, bfs_pop_head( bfs ) ); if( FD_UNLIKELY( !slotv->connected ) ) continue; - if( FD_UNLIKELY( slotv->abandoned ) ) continue; fd_chainer_slotv_t * parent = fd_chainer_slot_version_query( chainer, slotv->parent_slot, &slotv->parent_block_id ); if( FD_UNLIKELY( !parent || parent->complete_idx == UINT_MAX || parent->delivered_idx != parent->complete_idx ) ) continue; @@ -651,6 +608,7 @@ fd_chainer_fec_complete( fd_chainer_t * chainer, for( uint i = 0; i < FD_FEC_SHRED_CNT; i++ ) { fd_chainer_shred_insert( chainer, slot, fec_set_idx_ + i, slot_complete && (i == FD_FEC_SHRED_CNT - 1), mr, AG_UNKNOWN_SLOT, NULL ); } + /* By the time we get here the FEC exists unless turbine refused an unauthorized equivocating root -- in which case it was dropped and there is nothing to complete. */ @@ -668,35 +626,28 @@ fd_chainer_fec_complete( fd_chainer_t * chainer, skipped entirely. */ uint fec_idx = (uint)fd_fec_pool_idx( chainer->fec_pool, fec ); - fd_chainer_slotv_t * turbine = NULL; - for( ulong _i=slotv_iter_init( chainer, slot ); _i!=ULONG_MAX; _i=slotv_iter_next( chainer, _i ) ) { + for( ulong _i=slotv_iter_init( chainer, slot ); + _i!=ULONG_MAX; + _i=slotv_iter_next( chainer, _i ) ) { fd_chainer_slotv_t * slotv = slotv_iter_ele( chainer, _i ); - if( FD_LIKELY( slotv->turbine && !slotv->abandoned ) ) { turbine = slotv; break; } - } + if( FD_UNLIKELY( fd_chainer_slotv_fecs( chainer, slotv )[ k ]!=fec_idx ) ) continue; - if( FD_LIKELY( turbine && fd_chainer_slotv_fecs( chainer, turbine )[ k ]==fec_idx ) ) { - extend_buffered_fec( chainer, turbine ); + extend_buffered_fec( chainer, slotv ); - /* Slot is complete -> we can record the block_id, and we must have - all of its components at this point. Only a turbine version needs - it computed: a notar-fallback version already learned its block_id - from the cert. */ - if( FD_UNLIKELY( turbine->complete_idx != UINT_MAX && turbine->buffered_fec_idx == turbine->complete_idx && fd_hash_check_zero( &turbine->block_id ) ) ) { - if( FD_UNLIKELY( turbine->parent_slot == AG_UNKNOWN_SLOT ) ) FD_LOG_WARNING(( "slot %lu is complete, but parent_slot is still unknown", slot )); - if( FD_UNLIKELY( !finalize_block_id( chainer, turbine ) ) ) FD_LOG_WARNING(( "failed to finalize block_id for slot %lu", slot )); + if( FD_LIKELY( slotv->turbine ) ) { + fd_chainer_slotv_t * turbine = slotv; + /* Slot is complete means we can record the block_id, and we must have + all of its components at this point. Only a turbine version needs + it computed: a notar-fallback version already learned its block_id + from the cert. */ + if( FD_UNLIKELY( turbine->complete_idx!=UINT_MAX + && turbine->buffered_fec_idx==turbine->complete_idx + && fd_hash_check_zero( &turbine->block_id ) ) ) { + if( FD_UNLIKELY( !finalize_block_id( chainer, turbine ) ) ) FD_LOG_WARNING(( "failed to finalize block_id for slot %lu, parent_slot %lu, parent_bid is zero %d", slot, turbine->parent_slot, fd_hash_check_zero( &turbine->parent_block_id ) )); + } } - chainer_advance( chainer, turbine ); - } - - /* Advance every remaining version that owns this FEC root at this - position. */ - for( ulong _i=slotv_iter_init( chainer, slot ); _i!=ULONG_MAX; _i=slotv_iter_next( chainer, _i ) ) { - fd_chainer_slotv_t * slotv = slotv_iter_ele( chainer, _i ); - if( FD_UNLIKELY( slotv==turbine || slotv->abandoned ) ) continue; - if( FD_UNLIKELY( fd_chainer_slotv_fecs( chainer, slotv )[ k ]!=fec_idx ) ) continue; - - extend_buffered_fec( chainer, slotv ); + /* all versions advance */ chainer_advance( chainer, slotv ); } @@ -738,7 +689,7 @@ fd_chainer_fec_evicted( fd_chainer_t * chainer, if( FD_UNLIKELY( slotv->highest_requested != UINT_MAX && slotv->highest_requested >= fec_set_idx ) ) { slotv->highest_requested = fec_set_idx - 1U; } - if( FD_LIKELY( !slotv->abandoned ) ) fd_chainer_repair_add( chainer, slotv ); /* abandoned versions stay off the worklists */ + fd_chainer_repair_add( chainer, slotv ); } } @@ -761,7 +712,6 @@ fd_chainer_verified_parent_fec_count( fd_chainer_t * chainer, if( FD_UNLIKELY( !parent_slotv ) ) { parent_slotv = acquire_slotv( chainer, parent_slot ); parent_slotv->block_id = *parent_block_id; - abandon_turbine( chainer, parent_slot ); } fd_chainer_orphan_remove( chainer, slotv ); @@ -772,8 +722,6 @@ fd_chainer_verified_parent_fec_count( fd_chainer_t * chainer, return slotv; } -/* Called by repair_tile on getFecRoot responses, after - verifying the hash is correct for a notar-fallback-ed block */ void fd_chainer_verified_hash_insert( fd_chainer_t * chainer, ulong slot, @@ -822,9 +770,11 @@ fd_chainer_fec_rekey( fd_chainer_t * chainer, if( FD_UNLIKELY( existing ) ) { /* A separate full-root FEC already exists -- e.g. turbine saw this FEC first. Merge the versions. */ + uint sentinel_idx = (uint)fd_fec_pool_idx( chainer->fec_pool, fec ); uint existing_idx = (uint)fd_fec_pool_idx( chainer->fec_pool, existing ); uint k = fec_set_idx / FD_FEC_SHRED_CNT; + for( ulong _i=slotv_iter_init( chainer, slot ); _i!=ULONG_MAX; _i=slotv_iter_next( chainer, _i ) ) { uint * fecs = fd_chainer_slotv_fecs( chainer, slotv_iter_ele( chainer, _i ) ); if( FD_UNLIKELY( fecs[ k ]==sentinel_idx ) ) fecs[ k ] = existing_idx; @@ -866,21 +816,13 @@ fd_chainer_notar_fallback( fd_chainer_t * chainer, fd_hash_t block_id ) { FD_TEST( slot > chainer->root ); /* if we already have an slotv with this block_id, no-op. Note - turbine block_id may not be computed yet. */ + turbine block_id may not be computed yet: an in-flight turbine + version of the same block gets a redundant votor-driven version + alongside it (see the header comment in fd_chainer.h). */ if( FD_LIKELY( fd_chainer_slot_version_query( chainer, slot, &block_id ) ) ) return; fd_chainer_slotv_t * slotv = acquire_slotv( chainer, slot ); slotv->block_id = block_id; - - fd_chainer_slotv_t * turbine = turbine_slotv_query( chainer, slot ); - if( FD_UNLIKELY( turbine && fd_hash_check_zero( &turbine->block_id ) ) ) { - /* Turbine slotv is not yet complete, but votor repair events for this - slot have already started arriving, suggesting we are way behind - on repairing this slot. At this point just abandon the turbine version - and only deliver a notar fallback version. */ - slotv_abandon( chainer, turbine ); - } - } /* Out queue must be drained before calling this function, else there diff --git a/src/discof/chainer/fd_chainer.h b/src/discof/chainer/fd_chainer.h index cfbf512cacc..5863d129c57 100644 --- a/src/discof/chainer/fd_chainer.h +++ b/src/discof/chainer/fd_chainer.h @@ -47,14 +47,14 @@ re-requesting the shreds. This case should be rare enough that the redundancy is worth the simplicity. - When that happens the turbine version is ABANDONED: arriving shreds - are still accepted and fill the FECs, but it never delivers to - replay, never finalizes a block_id, and is dropped from the repair - worklists. Were it to keep delivering, and its block_id to finalize - to the same block a votor version is repairing, replay would - materialize two banks for the same {slot, block_id} (see - fd_rotor_tile.h). An abandoned slotv is pruned with its slot at - publish. + When that happens both versions stay live. They share the FECs + (the votor version's getFecRoot sentinels resolve to the roots the + turbine version already holds), so each completed FEC is delivered + once per version, and the turbine version finalizes its block_id to + the same value the votor version learned from the cert. Replay owns + the deduplication of that second stream (see fd_rotor_tile.h), and + fd_chainer_publish prunes the non-canonical duplicate when the slot + roots. *Parent Discovery* @@ -117,12 +117,6 @@ struct fd_chainer_slotv { ulong prev; /* reserved by map_chain */ uchar turbine; /* 1 for the slotv created through turbine */ - uchar abandoned; /* 1 once a votor-driven version of the slot was - created while this (turbine) version's block_id - was still unknown: keeps accepting shred/FEC - bookkeeping but never delivers, never finalizes - a block_id, and stays off the repair worklists. - See the header comment above. */ fd_hash_t block_id; uint complete_idx; uint buffered_idx; /* idx of highest buffered shred */ @@ -389,6 +383,14 @@ fd_chainer_notar_fallback( fd_chainer_t * chainer, ulong slot, fd_hash_t block_id ); +/* fd_chainer_verified_parent_fec_count is chainer's entrypoint for + updating information on what a slots fec set count, parent slot, and + parent block id are. This mirrors the Alpenglow repair type + getParentAndFecSetCount. The information should be verified before + calling this function; chainer does no verification. Will CRIT if + {slot, block_id} does not exist in the chainer yet, otherwise creates + {parent, p_bid} slotv if it doesn't exist yet, and returns the slotv + associated with {slot, block_id}. */ fd_chainer_slotv_t * fd_chainer_verified_parent_fec_count( fd_chainer_t * chainer, ulong slot, @@ -397,6 +399,15 @@ fd_chainer_verified_parent_fec_count( fd_chainer_t * chainer, ulong parent_slot, fd_hash_t * parent_block_id ); +/* fd_chainer_verified_hash_insert is chainer's entrypoint for updating + information on what a slotv's FEC root is. This mirrors the Alpenglow + repair type getFecSetRoot. The information should be verified before + calling this function; chainer does no verification. Will CRIT if + {slot, block_id} does not exist in the chainer yet, otherwise creates + the FEC entry if it doesn't exist yet and updates bookkeeping. + + Note this assumes the merkle root is the truncated 20-byte root + prefix. */ void fd_chainer_verified_hash_insert( fd_chainer_t * chainer, ulong slot, diff --git a/src/discof/chainer/test_chainer.c b/src/discof/chainer/test_chainer.c index 4a349b4475b..5dbe0571f5c 100644 --- a/src/discof/chainer/test_chainer.c +++ b/src/discof/chainer/test_chainer.c @@ -414,10 +414,8 @@ test_shared_prefix( fd_wksp_t * wksp ) { /* (c) A notar-fallback cert for a block that is still in flight from turbine. We cannot compute the in-flight block's id yet, so we cannot tell the cert names the same block: a redundant slotv is created by - design, and the turbine version is abandoned -- it may be the same - block the cert version is repairing, and delivering both would hand - replay two banks for the same {slot, block_id}. The structure must - stay consistent. */ + design and both versions stay live (replay dedups the second + delivery stream). The structure must stay consistent. */ static void test_notar_fallback_in_flight( fd_wksp_t * wksp ) { @@ -456,13 +454,13 @@ test_notar_fallback_in_flight( fd_wksp_t * wksp ) { FD_TEST( fd_chainer_slotv_shred_cnt( chainer, v1 )==0UL ); FD_TEST( !fec_at( chainer, 31UL, 0U, 1UL ) ); - /* version 0 keeps its data but is abandoned: off the worklists, and - it will never deliver or finalize a block_id */ + /* version 0 keeps its data and stays live: still in flight, still on + the repair worklist, parent known so not an orphan */ FD_TEST( v0->buffered_idx==31U && v0->buffered_fec_idx==31U ); FD_TEST( fec_at( chainer, 31UL, 0U, 0UL ) ); - FD_TEST( v0->abandoned ); - FD_TEST( !fd_chainer_in_repair( chainer, v0 ) && !fd_chainer_in_orphan( chainer, v0 ) ); + FD_TEST( v0->turbine ); + FD_TEST( fd_chainer_in_repair( chainer, v0 ) && !fd_chainer_in_orphan( chainer, v0 ) ); /* a repeat of the same cert is a no-op -- no third version */ @@ -540,16 +538,15 @@ test_sentinel_before_turbine( fd_wksp_t * wksp ) { FD_TEST( v0->complete_idx ==63U ); FD_TEST( v0->buffered_idx ==63U ); - /* The cert abandoned version 0, so even though the turbine block is - whole its FEC prefix is not extended, its block_id never finalizes, - and its slot-complete FEC is not delivered. Only set 0 -- queued - before the cert arrived -- ever reached replay. */ + /* Version 0 stays live: the turbine block is whole, its FEC prefix + extends over set 1, its block_id finalizes, and its slot-complete + FEC is delivered under the turbine version. Version 1 still lacks + set 0's root and delivers nothing yet. */ - FD_TEST( v0->abandoned ); - FD_TEST( v0->buffered_fec_idx==31U ); - FD_TEST( fd_hash_check_zero( &v0->block_id ) ); - out_rec_t exp[] = { { 41UL, 0U, r0 } }; - expect_out( chainer, exp, 1UL ); + FD_TEST( v0->buffered_fec_idx==63U ); + FD_TEST( !fd_hash_check_zero( &v0->block_id ) ); + out_rec_t exp[] = { { 41UL, 0U, r0 }, { 41UL, 32U, r1 } }; + expect_out( chainer, exp, 2UL ); FD_TEST( !fd_chainer_verify( chainer ) ); teardown( chainer ); @@ -602,17 +599,14 @@ test_turbine_shred_after_notar_fallback( fd_wksp_t * wksp ) { FD_TEST( v0->complete_idx ==63U ); FD_TEST( v0->buffered_idx ==63U ); - /* But the cert abandoned version 0: the FEC prefix is not extended, - the block_id never finalizes, and the whole block -- possibly the - very one the cert version is repairing -- is not delivered under - the turbine version. Only set 0, queued before the cert arrived, - ever reached replay. */ - - FD_TEST( v0->abandoned ); - FD_TEST( v0->buffered_fec_idx==31U ); - FD_TEST( fd_hash_check_zero( &v0->block_id ) ); - out_rec_t exp[] = { { 51UL, 0U, r0 } }; - expect_out( chainer, exp, 1UL ); + /* Version 0 stays live despite the cert: its FEC prefix extends, its + block_id finalizes, and the whole block is delivered under the + turbine version. Version 1 has no roots yet and delivers nothing. */ + + FD_TEST( v0->buffered_fec_idx==63U ); + FD_TEST( !fd_hash_check_zero( &v0->block_id ) ); + out_rec_t exp[] = { { 51UL, 0U, r0 }, { 51UL, 32U, r1 } }; + expect_out( chainer, exp, 2UL ); FD_TEST( !fd_chainer_verify( chainer ) ); teardown( chainer ); diff --git a/src/discof/rotor/fd_rotor_tile.h b/src/discof/rotor/fd_rotor_tile.h index 6186bb5bb81..3bc763ebf6d 100644 --- a/src/discof/rotor/fd_rotor_tile.h +++ b/src/discof/rotor/fd_rotor_tile.h @@ -26,20 +26,12 @@ is set to the correct value starting from fec 0. For these blocks, the verified bit is 1. - There is a race in the case no equivocation occurred, but we - suffered some network disconnection and are slow to complete the - block (but we get a votor event for the block_id). Then we would be - simultaneously completing the same block through turbine and ag - block_id repair, and the turbine copy's slot-complete FEC would - re-key its replay bank from {slot, 0} to a {slot, block_id} that the - verified copy's bank already occupies. - - To prevent that, the chainer ABANDONS the turbine version of a slot - the moment a votor-driven version of it is created while the turbine - block_id is still unknown (see fd_chainer.h): the abandoned version - keeps absorbing turbine shreds (they fill the FECs the verified - version shares) but never delivers another FEC and never finalizes a - block_id. + There is a race in the case no equivocation occurred, but we are + slow to complete the block (network blip, or simply the cert + arriving before the last FEC set) and get a votor event for the + block_id. Then we complete the same block through turbine and ag + block_id repair simultaneously, as two chainer versions that share + the same FECs (see fd_chainer.h), and every FEC is delivered twice. Consider this case: Slot A (started receiving through turbine): received FEC 0, 1, and 5 @@ -48,19 +40,23 @@ *blip* Get a notar fallback for slot A'. No equivocation occurred, but we - can't tell, so we also start repairing A' using ag block id repair, - and the turbine version of the slot is abandoned. Slot A' is - immediately able to complete FEC 0 and 1 (the shreds are local), and - they are re-delivered to replay with {verified=1, block_id=A'}. - Remaining shreds of FEC 2 -- whether they arrive through turbine or - ShredForBlockId repair -- fill the shared FEC, and FEC 2 is delivered - once, under A', with {verified=1, block_id=A'}. - - The effect is that in time of network blips, replay ends up - allocating up to two banks for the same slot/block: the turbine bank - keyed {slot, 0} receives only a prefix of the block, never completes, - never gets re-keyed (so it can never collide with the verified bank - keyed {slot, block_id}), and is eventually evicted or pruned. */ + can't tell, so we also start repairing A' using ag block id repair. + Slot A' is immediately able to complete FEC 0 and 1 (the shreds are + local), and they are re-delivered to replay with {verified=1, + block_id=A'}. Remaining shreds of FEC 2 -- whether they arrive + through turbine or ShredForBlockId repair -- fill the shared FEC, + and FEC 2 is delivered twice: once under the turbine version, whose + block_id finalizes to A' at that point, and once under A'. + + Replay handles the second stream. The verified copy's FEC 0 misses + the turbine bank (keyed {slot, 0}) and allocates its own bank keyed + {slot, A'}, and its mid-slot FECs are ingested into it. Whichever + copy completes first re-keys onto {slot, A'}, unlinking the other + copy's map entry; the other copy's slot-complete FEC then finds the + completed bank and is skipped. The loser is left an incomplete + sibling bank of the same slot and is pruned when the slot roots. So + the cost of the race is one extra bank and the execution of the + duplicate's prefix, not a correctness problem. */ // TODO remove after reasm removal #define REPAIR_SIG_FEC (0UL) diff --git a/src/discof/rotor/test_rotor_tile.c b/src/discof/rotor/test_rotor_tile.c index 0b229b70bb5..2971403db6a 100644 --- a/src/discof/rotor/test_rotor_tile.c +++ b/src/discof/rotor/test_rotor_tile.c @@ -1433,13 +1433,12 @@ test_fec_rekey_merge( fd_wksp_t * wksp ) { Turbine delivers FEC sets 0 and 1 of a 3-set block unverified ({verified=0, block_id=null}), then a notar-fallback cert arrives for the very same block. The turbine block_id is not computable yet, so - we cannot tell it is the same block: the turbine version is abandoned - and the cert version re-delivers the prefix verified. The remaining - shreds then arrive through turbine: they fill the shared FEC, and the - final set is delivered exactly once, verified, under the cert - version. The turbine version must never deliver its slot-complete - FEC: replay would re-key its {slot, 0} bank onto the {slot, block_id} - the cert version's bank already occupies. */ + we cannot tell it is the same block: a cert version is created + alongside the live turbine version and re-delivers the prefix + verified. The remaining shreds then arrive through turbine: they + fill the shared FEC, and the final set is delivered once per version, + turbine first (its block_id finalizes to the cert's), then the cert + version. Replay dedups the second stream (see fd_rotor_tile.h). */ static void test_notar_fallback_same_block( fd_wksp_t * wksp ) { @@ -1470,14 +1469,14 @@ test_notar_fallback_same_block( fd_wksp_t * wksp ) { rep_expect( 1UL, slot, FD_FEC_SHRED_CNT, &blk->fec_root[ 1 ], NULL, 0 ); FD_TEST( !rep_log[ 0 ].known_id && !rep_log[ 1 ].known_id ); - /* Notar-fallback cert for the same block abandons the in-flight - turbine version. */ + /* Notar-fallback cert for the same block: a cert version is created, + the in-flight turbine version stays live. */ ulong mark = req_cnt; deliver_votor( ctx, FD_VOTOR_SIG_REPAIR, slot, &blk->block_id ); fd_hash_t zero = {0}; fd_chainer_slotv_t * vT = fd_chainer_slot_version_query( ctx->chainer, slot, &zero ); - FD_TEST( vT && vT->turbine && vT->abandoned ); + FD_TEST( vT && vT->turbine ); fd_chainer_slotv_t * vC = fd_chainer_slot_version_query( ctx->chainer, slot, &blk->block_id ); FD_TEST( vC && vC!=vT && !vC->turbine ); @@ -1508,10 +1507,11 @@ test_notar_fallback_same_block( fd_wksp_t * wksp ) { FD_TEST( rep_log[ rep_mark ].known_id && rep_log[ rep_mark+1UL ].known_id ); /* The remaining set arrives through turbine (not repair): its shreds - fill the cert version's sentinel (same root), and the slot-complete - FEC is delivered exactly once -- verified, under the cert version. - The abandoned turbine version never delivers it and never finalizes - a block id. */ + fill the shared FEC, and the slot-complete FEC is delivered once + per version. The turbine version goes first: its block_id + finalizes to the cert's, so its slot-complete FEC carries that id + but is not marked known_id (it is a turbine version). The cert + version follows, verified. */ rep_mark = rep_cnt; for( uint i=0U; ifec_root[ 2 ] ); pump( ctx ); - FD_TEST( rep_cnt==rep_mark+1UL ); - rep_expect( rep_mark, slot, 2U*FD_FEC_SHRED_CNT, &blk->fec_root[ 2 ], &blk->block_id, 1 ); - FD_TEST( rep_log[ rep_mark ].known_id ); + FD_TEST( rep_cnt==rep_mark+2UL ); + rep_expect( rep_mark, slot, 2U*FD_FEC_SHRED_CNT, &blk->fec_root[ 2 ], &blk->block_id, 1 ); + rep_expect( rep_mark+1UL, slot, 2U*FD_FEC_SHRED_CNT, &blk->fec_root[ 2 ], &blk->block_id, 1 ); + FD_TEST( !rep_log[ rep_mark ].known_id ); + FD_TEST( rep_log[ rep_mark+1UL ].known_id ); FD_TEST( fd_chainer_highest_repaired_slot( ctx->chainer )==slot ); - FD_TEST( fd_hash_check_zero( &vT->block_id ) ); /* never finalized */ - FD_TEST( slot_version_cnt( ctx->chainer, slot )==2UL ); /* cert version + abandoned anchor */ + FD_TEST( fd_hash_eq( &vT->block_id, &blk->block_id ) ); /* turbine version finalized to the cert's id */ + FD_TEST( slot_version_cnt( ctx->chainer, slot )==2UL ); /* both versions live, same {slot, block_id} */ - /* Exactly one slot-complete delivery across the whole run, and - everything delivered after the cert arrived is verified: replay's - turbine bank (keyed {slot, 0}) never re-keys, so it can never - collide with the cert bank keyed {slot, block_id}. */ + /* Two slot-complete deliveries across the whole run, one per version, + both carrying the block id. */ ulong sc_cnt = 0UL; for( ulong i=0UL; i=2UL ) FD_TEST( rep_log[ i ].known_id ); + if( rep_log[ i ].slot==slot && rep_log[ i ].slot_complete ) { sc_cnt++; FD_TEST( fd_hash_eq( &rep_log[ i ].block_id, &blk->block_id ) ); } } - FD_TEST( sc_cnt==1UL ); + FD_TEST( sc_cnt==2UL ); FD_TEST( !fd_chainer_verify( ctx->chainer ) ); - /* Rooting the block prunes the abandoned anchor. */ + /* Rooting the block prunes the non-canonical duplicate. */ deliver_votor( ctx, FD_VOTOR_SIG_ROOTED, slot, &blk->block_id ); FD_TEST( ctx->chainer->root==slot ); @@ -1623,22 +1622,28 @@ test_block_id_repair_only( fd_wksp_t * wksp ) { serve_shred_requests( ctx, mark, blk, UINT_MAX, NULL, served ); pump( ctx ); - /* Repair shreds still land on turbine-side bookkeeping, but the - turbine version of the slot was created abandoned (the cert version - predates it), so it only anchors the roots and shred bitmaps: it - never delivers and never finalizes a block id. Every FEC is - delivered exactly once, under the cert version, verified and - carrying the cert block id. */ + /* Repair shreds land on turbine-side bookkeeping too: the turbine + version of the slot (created by the first repaired shred, after the + cert version) shares every FEC with the cert version, so each FEC + is delivered twice, turbine copy first. The turbine copy of FEC 0 + is unverified ({known_id=0, block_id=0}); at the slot-complete FEC + its block_id finalizes to the cert's, so that copy carries the id + but is still not known_id. The cert copies are verified with the + cert block id throughout. Replay dedups the turbine stream. */ FD_TEST( fd_chainer_highest_repaired_slot( ctx->chainer )==slot ); - FD_TEST( rep_cnt==2UL ); - rep_expect( 0UL, slot, 0U, &blk->fec_root[ 0 ], &blk->block_id, 0 ); - rep_expect( 1UL, slot, FD_FEC_SHRED_CNT, &blk->fec_root[ 1 ], &blk->block_id, 1 ); - FD_TEST( rep_log[ 0 ].known_id ); - FD_TEST( rep_log[ 1 ].known_id ); + FD_TEST( rep_cnt==4UL ); + rep_expect( 0UL, slot, 0U, &blk->fec_root[ 0 ], NULL, 0 ); + rep_expect( 1UL, slot, 0U, &blk->fec_root[ 0 ], &blk->block_id, 0 ); + rep_expect( 2UL, slot, FD_FEC_SHRED_CNT, &blk->fec_root[ 1 ], &blk->block_id, 1 ); + rep_expect( 3UL, slot, FD_FEC_SHRED_CNT, &blk->fec_root[ 1 ], &blk->block_id, 1 ); + FD_TEST( !rep_log[ 0 ].known_id ); + FD_TEST( rep_log[ 1 ].known_id ); + FD_TEST( !rep_log[ 2 ].known_id ); + FD_TEST( rep_log[ 3 ].known_id ); - /* Both versions are tracked (the abandoned turbine anchor plus the - cert version); rooting below prunes down to the canonical one. */ + /* Both versions are tracked with the same {slot, block_id}; rooting + below prunes down to the canonical one. */ FD_TEST( slot_version_cnt( ctx->chainer, slot )==2UL ); FD_TEST( !fd_chainer_verify( ctx->chainer ) ); From 16f7825089f3bfa91a617e8d6f234cc106eb6fd5 Mon Sep 17 00:00:00 2001 From: Emily Wang Date: Tue, 8 Sep 2026 16:30:24 +0000 Subject: [PATCH 2/2] fix test --- src/discof/rotor/test_rotor_tile.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/discof/rotor/test_rotor_tile.c b/src/discof/rotor/test_rotor_tile.c index 2971403db6a..898e9b7a75c 100644 --- a/src/discof/rotor/test_rotor_tile.c +++ b/src/discof/rotor/test_rotor_tile.c @@ -1508,10 +1508,12 @@ test_notar_fallback_same_block( fd_wksp_t * wksp ) { /* The remaining set arrives through turbine (not repair): its shreds fill the shared FEC, and the slot-complete FEC is delivered once - per version. The turbine version goes first: its block_id - finalizes to the cert's, so its slot-complete FEC carries that id - but is not marked known_id (it is a turbine version). The cert - version follows, verified. */ + per version. The turbine version's block_id finalizes to the + cert's, so its slot-complete FEC carries that id but is not marked + known_id (it is a turbine version). The cert version's copy is + verified. Delivery order between the two versions follows the + chainer's version-chain order (newest first), so only require one + copy of each kind. */ rep_mark = rep_cnt; for( uint i=0U; ifec_root[ 2 ], &blk->block_id, 1 ); rep_expect( rep_mark+1UL, slot, 2U*FD_FEC_SHRED_CNT, &blk->fec_root[ 2 ], &blk->block_id, 1 ); - FD_TEST( !rep_log[ rep_mark ].known_id ); - FD_TEST( rep_log[ rep_mark+1UL ].known_id ); + FD_TEST( rep_log[ rep_mark ].known_id != rep_log[ rep_mark+1UL ].known_id ); /* one turbine copy, one cert copy */ FD_TEST( fd_chainer_highest_repaired_slot( ctx->chainer )==slot ); FD_TEST( fd_hash_eq( &vT->block_id, &blk->block_id ) ); /* turbine version finalized to the cert's id */ FD_TEST( slot_version_cnt( ctx->chainer, slot )==2UL ); /* both versions live, same {slot, block_id} */