fix(node): look up owned shard filters by address, not by filter - #653
Open
blacks1ne wants to merge 1 commit into
Open
fix(node): look up owned shard filters by address, not by filter#653blacks1ne wants to merge 1 commit into
blacks1ne wants to merge 1 commit into
Conversation
`GetShardInfo(include_all: false)` built its owned-filter set with `ProverRegistry::get_provers(&self_address)`. That lookup is keyed by confirmation filter — `SharedProverRegistry::get_provers` indexes `filter_cache` — so an address never matches, the set was always empty, and every shard was filtered out of the response. `qclient node prover shards` printed "No allocated shards" on a node whose `node prover status` listed twenty-seven live allocations. `GetNodeInfo` already builds the same view from the address-keyed `get_prover_info`; `owned_filters` is that, extracted. It also takes `GetNodeInfo`'s liveness predicate, which admits `ExpiredEpoch` — an Active data-shard allocation that missed this epoch's re-confirm is recoverable, not gone, and `status` reports it as `re-confirm!`. Using the stricter `is_live` would have left the two commands disagreeing over exactly the allocation an operator has to act on. `ShardDetail.is_allocated` was always false for the same reason. The TUI was unaffected because it ORs that flag with a set it rebuilds from `GetNodeInfo`, which is why only the non-TUI command showed the fault. The test carries its own registry stub: `quil_engine::test_support::TestProverRegistry::get_provers` ignores its filter argument and returns every prover, so under that stub the wrong call and the right one are indistinguishable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
blacks1ne
force-pushed
the
fix/shard-info-owned-filters
branch
from
September 9, 2026 12:13
1e77bd4 to
d7403a8
Compare
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
1 participant
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.
GetShardInfowithinclude_all: falsereturned nothing on a prover holding twenty-seven allocations —prover shardsprinted "No allocated shards" whileprover statuslisted all twenty-seven.The registry has two lookups and both take a bare
&[u8]:get_proversis keyed by confirmation filter,get_prover_infoby address. This site passed the local address toget_provers, which misses the filter cache and returns empty, so the owned set was always empty and every shard was filtered away.The owned set is now built from
get_prover_info, matchingGetNodeInfo's liveness predicate so the two surfaces agree allocation for allocation — includingExpiredEpoch, whichstatusshows asre-confirm!and which comes back the moment the prover re-registers.Verified on a live prover: before the fix
prover shardsprinted "No allocated shards"; after it, twenty-seven, an exact set match against the twenty-seven allocations inprover statuswith nothing on either side. Theinclude_all: trueview is unchanged at ninety-one.Four unit tests cover it. They need a filter-keyed registry stub: the shared
TestProverRegistry::get_proversignores its filter argument, which is why the existing suite could not distinguish the wrong call from the right one.Base:
4eaf1f79