ci: make clippy blocking (-D warnings) — 221 warnings to clear first

[defenwycke]

PROBLEM: the MSRV job runs on stable (never enforces 1.85); clippy has no -D warnings so it is advisory only.
EVIDENCE: .github/workflows/ci.yml:156 (rust-toolchain@stable), ci.yml:46 (clippy, no -D warnings).
TEST: CI fails on a deliberate clippy warning and on a post-1.85 feature.
ACCEPTANCE: MSRV job pinned to 1.85; clippy runs -D warnings keeping the existing -A allow-list.

[defenwycke]

Measured both halves before touching anything. They are very different sizes.

The clippy half is not a CI change — it is a cleanup

Running exactly what CI runs today, on 64332f98:

cargo clippy --workspace --exclude wraith-wallet-gui --exclude ghost-tap-desktop \
  --all-targets --all-features -- -W clippy::all -A clippy::derivable_impls \
  -A clippy::clone_on_copy -A clippy::match_like_matches_macro -A clippy::manual_flatten \
  -A clippy::redundant_slicing -A clippy::skip_while_next

221 warnings. Adding -D warnings would fail CI on all of them on the first push. None are in vendored code — 0 of 189 located warnings are under vendor/. They are ours:

crate	count
bins/jd-client-sv2	28
bins/ghost-pool	25
bins/pool-sv2	22
crates/ghost-reaper	21
apps/wraith-wallet	19
apps/ghost-tap	18
crates/ghost-verification	10
crates/ghost-storage	9
others	~40

Top categories:

32  the Err-variant returned from this closure is very large
17  field assignment outside of initializer for an instance created with Default::default()
15  consider using sort_by_key
11  you seem to want to iterate on a map's values
 7  doc list item without indentation
 6  used assert_eq! with a literal bool
 5  very complex type used. Consider factoring parts into type definitions

103 carry a suggested fix, so a good chunk is mechanical — but the Err-variant-too-large ones (32) and the complex-type ones are judgement calls, and ghost-pool / pool-sv2 are consensus-adjacent, so I would not want them swept up in a bulk commit alongside a release.

So the acceptance as written — clippy runs -D warnings keeping the existing allow-list — is not reachable without either fixing 221 warnings or widening the allow-list until the gate means little.

Suggested split:

1. Clear the mechanical categories per crate, smallest first, one PR each so the diffs stay reviewable.
2. Decide the Err-variant-too-large ones deliberately — that is 32 of them, and boxing an error type is an API change.
3. Turn on -D warnings last, once the count is zero. Turning it on before then just means every PR is red and people learn to ignore it.

Not doing this tonight. It wants to be its own sequence of small PRs, not a 221-warning sweep at the end of a release roll.

The MSRV half is small and I am doing it

ci.yml:164 uses dtolnay/rust-toolchain@stable in the job named MSRV, so it has never enforced anything — it is a second stable build wearing a different name. Cargo.toml declares rust-version = "1.85".

That is a one-line change, and a PR is exactly the right place to test it: if the workspace does not in fact build on 1.85, the PR goes red and we learn that without main ever breaking. Opening it separately.

[defenwycke]

Retitled — the MSRV half landed in #441, so the remaining work here is only the clippy gate.

For the record, pinning the MSRV job did not just start enforcing the declared minimum, it showed the declaration was wrong: rust-version = "1.85" while time@0.3.47 requires 1.88. Corrected to 1.88 and the job pinned to match, so the two can no longer drift apart silently.

What is left here is the 221-warning cleanup, breakdown in the comment above.

[defenwycke]

Progress and a corrected picture. The issue's headline of 171 was stale — it predates the dead-code cleanup (#500), so everything below was re-measured on current main.

Started at 208 warning lines. Now roughly 55, across four merged PRs and one open.

PR	scope	state
#503	--fix across 12 non-consensus crates, plus ghost-reaper by hand	merged
#504	doc-comment lists — including one genuinely wrong doc	merged
#505	clamp / sort_by_key / iterator loops	merged
#506	ghost-pool + pool_sv2, hand-read	open

What remains, and why it isn't just "keep going"

34 are result_large_err — 19 in the pool crates, 15 in jd_client_sv2. That is over half the remainder and it is one decision, not 34 fixes: either box the error type across a vendored-derived crate, or add a crate-scoped allow. The variant sits at exactly clippy's 128-byte threshold, and the existing CI command already carries six -A entries as precedent. I have not decided this unattended.

5 are sort_by in payout.rs and shares.rs. sort_by_key is provably equivalent — both sorts are stable, the keys are Copy — but payout.rs:204 orders nodes immediately before hashing them into the ledger root, and shares.rs orders top-100 node selection. Changing ordering there changes a consensus hash. The return is a style lint. These want a human and a ledger-root test.

2 are too_many_arguments (8/7 and 11/7) — real refactors, or an allow if the signatures are considered fine.

The rest are field_reassign_with_default in config-mutation paths and a few complex types wanting aliases.

One thing worth recording for whoever finishes this

--fix was deliberately not run on ghost-pool or pool_sv2, and that was justified almost immediately. Clippy flags return Err(e)?; as having a redundant return. Applying that reading does not compile:

error[E0308]: `match` arms have incompatible types

Those sit in match arms that must yield a channel: return … gives the arm type !, which coerces to anything, while Err(e)?; ends in a semicolon so the arm becomes (). The correct fix drops the ?, not the return. cargo check caught it — review did not.

A subtler variant of that same mistake would have compiled and been wrong. That is the case for reading these two crates rather than sweeping them.

On the -D warnings flip

It should stay last. Flipping it before the count reaches zero converts a backlog into a wall, and the two decisions above are the gate — 34 of the remaining ~55 hinge on the result_large_err call alone.