chore: annotate the intentionally-unused code paths and drop orphaned test fixtures - #602
Open
blacks1ne wants to merge 1 commit into
Open
Conversation
blacks1ne
force-pushed
the
chore/warnings-dead-code
branch
from
August 15, 2026 22:41
ed2386e to
7213053
Compare
… test fixtures
rustc reports 47 dead_code/unreachable_patterns warnings in the first-party
crates. Almost none of them are accidental leftovers: they are ported-for-
parity mirrors of the Go implementation, accessors on library surfaces the
node itself does not call, or scaffolding for a feature that is implemented
but not yet wired up. Deleting them would lose work; leaving them warning
trains reviewers to skim past the lint.
So each site gets #[allow(dead_code)] plus a one-line note saying why it is
unused, following the annotation already on AppShardProposal::state_bytes.
Scoped per item rather than per crate, so genuinely new dead code still warns.
That struct now carries the attribute at the struct level, since four of its
five fields were write-only.
Two exceptions are deleted outright, because they are #[cfg(test)] fixtures
no remaining test references:
- token_intrinsic/pending.rs: make_recipient/make_output/make_modern_input/
make_legacy_input/make_tx (69 lines); the four surviving round-trip tests
build their structs inline.
- global_intrinsic/conversions.rs: sample_addr_sig.
One arm is annotated rather than removed: the '_ => return None' in
consensus_wire::canonical_request_from_proto is unreachable today because
every Request variant is ported, but its comment says it exists so that
regenerating the protos drops an unported variant instead of breaking the
build. That intent is worth keeping.
Two things worth a second look, flagged but not changed here:
- engines::require_traversal_proof_for_inputs is dead. Its own doc comment
explains that without this gate 'an attacker can fabricate inputs whose
values they choose and mint QUIL from nothing'. The check is implemented
but nothing calls it.
- thread_worker's JoinHandle is never awaited; shutdown goes through the
CancellationToken only.
The unused `ratchet_state` parameter on `json_to_metadata` is renamed
here rather than in the unused-imports-and-bindings bucket, so that the
whole of this dead function -- annotation, comment and parameter -- is
described by one PR. Without that the two buckets touch adjacent lines
of the same signature and conflict on whichever merges second. The
warning is still fixed exactly once across the series; it has just moved
buckets. `metadata_to_json`, which is live, keeps its rename there.
blacks1ne
force-pushed
the
chore/warnings-dead-code
branch
from
August 18, 2026 10:34
7213053 to
e0fc7ac
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
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.
49 warnings, and the most opinionated PR in the series — this is the one to
push back on if any of it reads wrong.
Almost none of these are accidental leftovers. They are ported-for-parity
mirrors of the Go implementation, accessors on library surfaces the node itself
does not call, or scaffolding for a feature that is implemented but not wired up.
So each gets
#[allow(dead_code)]and a one-line note saying why, following theannotation already on
AppShardProposal::state_bytes. Scoped per item ratherthan per crate, so genuinely new dead code still warns.
Two exceptions are deleted, both
#[cfg(test)]fixtures with no surviving testreferencing them:
token_intrinsic/pending.rs's five builders (69 lines) andglobal_intrinsic/conversions.rs::sample_addr_sig.The unreachable
_ => return Noneinconsensus_wireis annotated rather thanremoved — its comment says it exists so that regenerating the protos drops an
unported variant instead of breaking the build, which seems worth keeping.
Two things I flagged but did not change
engines::require_traversal_proof_for_inputsis dead code. Its own doccomment says that without this gate "an attacker can fabricate inputs whose
values they choose and mint QUIL from nothing". The check is implemented;
nothing calls it. I annotated it with a
NOTEpointing at that rather thanwiring it up, because wiring it up is a decision about the token dispatch path,
not a warning fix. Flagging it here because it should not stay invisible.
thread_worker'sJoinHandleis never awaited — shutdown goes through theCancellationTokenonly.Merge-order note — resolved
This PR and #599 used to touch adjacent lines of the same
channel::json_to_metadatasignature, and would have conflicted on whichevermerged second. Rather than leave you to hit it, the whole site now lives here
(+1): the unused
ratchet_stateparameter is renamed in this PR alongside theannotation and the comment, since that entire function is dead, which is what
this bucket is about. #599 keeps the rename on
metadata_to_json, which is live.The warning is still fixed exactly once across the series — it changed bucket,
it was not dropped. Verified: the two now merge in either order and produce an
identical tree.
Series
Part of the warning-cleanup series that starts with #597. The eight PRs are
disjoint and each stands on its own, but they are meant to be read in order —
please take #597 first: it is the only one of the eight that fixes a bug
rather than a warning, and it is the shortest.
932045a3the 16 being
classgroup's GMP FFI glue, deliberately left visible rather thansilenced. Update: those 16 are now fixed rather than documented, in fix: correct classgroup GMP FFI declarations and uninitialized values #605,
which corrects the declarations and the uninitialized values instead of
annotating them. With fix: correct classgroup GMP FFI declarations and uninitialized values #605 and the channelwasm commit in chore: drop unused imports and bindings in the crypto crates #599, the combined tree
checks with zero warnings.
--exclude channelwasm. That crateis in scope now — chore: drop unused imports and bindings in the crypto crates #599 covers it — so the exclusion is gone.)
Happy to re-pace these, drop any of them, or squash the set into a single PR if
you would rather review it in one pass — just say which.
Drafted with Claude Code; every site was read individually and the reasoning is
in the commit message.