Optionally expose per-peer bitfields in PeerStats

[adamgreenwell]

Closes #643.

Three commits. The first two stand alone; the third is the actual ask.

1. Re-export PeerStats and PeerStatsFilter. Both appear in the signature
of the public per_peer_stats_snapshot but live in a private module, so a
dependent crate can only get a filter from Default::default() and cannot name
either type.

2. Clear a peer bitfield's padding on ingest. Your call on the review, and
it is the better one — see below. Independent of the stats work, and takeable
without it.

3. Add have_bitfield and have_pieces to PeerStats, behind a new
PeerStatsFilter::include_bitfield, off by default.

Against your three concerns

Performance for callers that do not need it. Nothing is computed or copied
unless the flag is set — both fields are None through a guard on the same
match, so the default path does exactly what it did before. Default has the
flag off, which bitfields_are_off_by_default pins. The HTTP API and every
existing caller are untouched.

API complexity. One new field on the filter and two on PeerStats, all
opt-in. I put the count behind the same flag rather than adding a second one:
it is nearly free once the bitfield is in hand, and it saves each caller from
re-deriving it.

Code complexity. +155/-18 across three files, and taking your ingest
suggestion made it smaller rather than larger — the counting helper is gone and
from_peer no longer needs a total_pieces argument. The only structural
change is From<&Peer> for PeerStats becoming PeerStats::from_peer(peer, include_bitfield), since the conversion now takes the flag. Peer is
pub(crate), so that impl was not reachable externally.

Why the bitfield rather than just a count

The issue originally asked for a count; that was my mistake and I corrected it
there. A count gives the mean copies per piece, but the useful figure is the
minimum. Two peers holding 500 pieces each may overlap entirely or not at
all — identical counts, and only one of those torrents can finish.

The padding, and the second way in

An earlier revision of this PR sliced to total_pieces when counting and left
the stored bitfield alone. You asked for it to be fixed at ingest instead, so
that is what it does now: on_bitfield clears the spare trailing bits, and
count_ones() means "pieces this peer has" everywhere without each caller
having to know about the padding.

Doing it there surfaced something. on_have is a second way in, and it bounds
the index with get_mut — which checks against the bitfield's length. That
length is byte-padded, so it overshoots the piece count by up to 7, and a peer
could set a padding bit one Have message after ingest and undo the masking.
It is bounded by the piece count now, in the same commit. That also means a
peer can no longer claim a piece that does not exist.

Neither was harmful before this, which is presumably why it went unnoticed:
every other reader either slices to total_pieces first — as on_bitfield's
own "peer has full torrent" check does — or indexes by an already-validated
piece id. The padding was only ever visible to something that counted the whole
bitfield.

Why have_bitfield is Vec<u8> and not BF

PeerStats derives Serialize and bitvec is built without its serde
feature, so BitBox has no Serialize impl and cloning the field does not
compile. Enabling the feature would also put bitvec in the public API, where
a dependent would need a matching version just to name the type. The allocation
is the same either way.

Checks

cargo test -p librqbit (38 passed, 5 ignored), cargo fmt --check, and
cargo clippy --all-targets -- -D warnings all clean — the last is stricter
than CI's, and worth running here because lib.rs opts into
clippy::cast_possible_truncation and the count is a usize.

Also running as a [patch.crates-io] in a desktop client, on a branch carrying
these three commits plus #645; its own suite passes.

Rebased onto current main. Happy to reshape or drop any of it.

[ikatson]

this seems to be from another PR #645, let's split

[adamgreenwell]

Split. That commit was on this branch because Flume needs both patches and [patch.crates-io] takes a single source — my problem rather than yours, and there's a separate integration branch carrying both now.

#644 is the bitfield work only, rebased onto main. The Windows change lives in #645 alone.

[ikatson]

can we just fix the original bitfield length "at ingest" as it's called in description instead of here?

[adamgreenwell]

Done — on_bitfield clears the padding now, count_have_pieces is gone, and count_ones() is correct everywhere. from_peer lost its total_pieces parameter as a result, so the diff got smaller.

One thing it turned up, worth a look: on_have is a second way in. It bounds the index with get_mut, which checks against the bitfield's length — byte-padded, so up to 7 over the piece count — so a peer could set a padding bit one Have after ingest and put the count back where it was. It's bounded by the piece count now, in the same commit. That also stops a peer claiming a piece that does not exist.

Neither was harmful before this: every other reader either slices to total_pieces first, as on_bitfield's own "peer has full torrent" check does, or indexes by an already-validated piece id. Which is presumably why it went unnoticed — the padding is only visible to something that counts the whole bitfield.

It's a separate commit (Clear a peer bitfield's padding on ingest) so you can take it on its own, independently of the stats work.

[ikatson]

why convert to vec and not just clone?

[adamgreenwell]

PeerStats derives Serialize and bitvec is built without its serde feature, so BitBox has no Serialize impl — l.bitfield.clone() does not compile.

Enabling the feature would also put bitvec in the public API: a dependent would need a matching bitvec version just to name the field's type. The allocation is the same either way, so Vec<u8> looked like the cheaper commitment. I have put that reasoning in a comment on the field, since it is not obvious from the call site.