Context
Surfaced by CodeRabbit on PR #978 (comment 3153861519). Tracking as a follow-up.
WorkUnit::total_shards() (crates/core/src/work_unit.rs:67) returns u32, defaulting to 1. Nothing in the type system stops an implementation from returning 0. If it does, run_lifecycle (crates/core/src/sync.rs::run_lifecycle) skips every per-shard phase but still runs initialize() → finalize() + tip notifications, so the work unit can "complete" without ever processing a shard.
Current state
PR #978 mitigates the worst symptom: crates/cardano/src/shard.rs::shard_key_ranges now panic!s in all build profiles if total_shards == 0 (commit 56b6dbb9), so any sharded Cardano work unit fails loudly at the first load() call. But:
- The trait contract still permits
0.
- Future non-Cardano work units (or any code path that doesn't go through
shard_key_ranges) still silently no-op.
Proposed fix
Change the trait to return NonZeroU32:
fn total_shards(&self) -> NonZeroU32 {
NonZeroU32::new(1).unwrap()
}run_lifecycle then loops start_shard..total_shards.get(). Implementations that derive shard count from persisted state (the three sharded Cardano units) wrap their cached u32 in NonZeroU32::new(...).expect(\"shard count must be >= 1\") after reading *_progress.total (already validated by validate_total_shards indirectly via shard_key_ranges, but the type makes it explicit at the boundary).
Alternative: keep u32 but add a runtime check in run_lifecycle that rejects 0 with a clear DomainError. Less type-safe but smaller blast radius if NonZeroU32 propagates inconveniently.
Files to touch
crates/core/src/work_unit.rs — trait signature + default implcrates/core/src/sync.rs::run_lifecycle — loop boundscrates/cardano/src/lib.rs — CardanoWorkUnit::total_shards delegationcrates/cardano/src/{ewrap,estart,rupd}/work_unit.rs — return wrapped valuecrates/cardano/src/{genesis,roll}/work_unit.rs — return NonZeroU32::new(1).unwrap()
Priority
Low. The Cardano sharded path is already protected at runtime by the shard.rs panic. This is a contract-tightening / future-proofing issue.
Context
Surfaced by CodeRabbit on PR #978 (comment 3153861519). Tracking as a follow-up.
WorkUnit::total_shards()(crates/core/src/work_unit.rs:67) returnsu32, defaulting to1. Nothing in the type system stops an implementation from returning0. If it does,run_lifecycle(crates/core/src/sync.rs::run_lifecycle) skips every per-shard phase but still runsinitialize()→finalize()+ tip notifications, so the work unit can "complete" without ever processing a shard.Current state
PR #978 mitigates the worst symptom:
crates/cardano/src/shard.rs::shard_key_rangesnowpanic!s in all build profiles iftotal_shards == 0(commit56b6dbb9), so any sharded Cardano work unit fails loudly at the firstload()call. But:0.shard_key_ranges) still silently no-op.Proposed fix
Change the trait to return
NonZeroU32:run_lifecyclethen loopsstart_shard..total_shards.get(). Implementations that derive shard count from persisted state (the three sharded Cardano units) wrap their cachedu32inNonZeroU32::new(...).expect(\"shard count must be >= 1\")after reading*_progress.total(already validated byvalidate_total_shardsindirectly viashard_key_ranges, but the type makes it explicit at the boundary).Alternative: keep
u32but add a runtime check inrun_lifecyclethat rejects0with a clearDomainError. Less type-safe but smaller blast radius ifNonZeroU32propagates inconveniently.Files to touch
crates/core/src/work_unit.rs— trait signature + default implcrates/core/src/sync.rs::run_lifecycle— loop boundscrates/cardano/src/lib.rs—CardanoWorkUnit::total_shardsdelegationcrates/cardano/src/{ewrap,estart,rupd}/work_unit.rs— return wrapped valuecrates/cardano/src/{genesis,roll}/work_unit.rs— returnNonZeroU32::new(1).unwrap()Priority
Low. The Cardano sharded path is already protected at runtime by the
shard.rspanic. This is a contract-tightening / future-proofing issue.