bug: unvalidated Fin proposal-part sequence overflows expected_messages (panics in overflow-checked node builds, wedges the stream in release)

[Jr-kenny]

Hi, I was reading through the proposal-part streaming code after the recent consensus streaming changes and ran into something that looks off in crates/malachite-app/src/streaming.rs.

When a stream message comes in, StreamState::insert works out how many messages it should expect once it sees the Fin:

// crates/malachite-app/src/streaming.rs:287
self.expected_messages = msg.sequence as usize + 1;

The comment right above it says the + 1 can't overflow "because MAX_MESSAGES_PER_STREAM << u64::MAX". The catch is that expected_messages isn't derived from MAX_MESSAGES_PER_STREAM at all, it's taken straight from the Fin message's sequence, and that sequence is a wire field the sending peer fully controls. So the bound the comment leans on doesn't actually apply here.

Here's what happens step by step. A peer gossips a single Fin proposal part with sequence set to u64::MAX. It comes in through received_proposal_part::handle then on_received_proposal_part, which hands the raw message straight to streams_map.insert (received_proposal_part.rs:163), and the sequence isn't checked anywhere along the way. From there it reaches PartStreamsMap::insert and then StreamState::insert. A lone Fin skips the chunk-size check (it isn't a Data part) and passes the message-count check (the count is still 0), so it lands on that line with sequence = u64::MAX, and u64::MAX as usize + 1 overflows.

In debug or debug-assertions builds (that includes cargo test, and any node not built in release) this is an attempt to add with overflow panic, so one malformed gossip message can take the process down. In release it wraps instead of panicking, and the stream then can never complete, so it just sits there holding a per-peer slot until the 60s age sweep clears it. Either way a single unvalidated peer message pushes the node into a bad state.

There's a smaller related thing in the same spot: even setting the overflow aside, any Fin whose sequence is >= MAX_MESSAGES_PER_STREAM is asking for more messages than a stream is ever allowed to hold (the count is capped at MAX_MESSAGES_PER_STREAM), so that stream can never complete but still takes up the peer's stream budget until it ages out.

Repro

Dropping this into the existing streaming::tests module panics today:

#[test]
fn repro_fin_large_sequence_overflow() {
    let peer = PeerId::random();
    let stream = new_stream_id(Height::new(1), Round::new(0), 0);
    let mut map = PartStreamsMap::new(Height::new(1), NUM_VALIDATORS);
    let msg = make_fin_message(&stream, u64::MAX);
    let _ = map.insert(peer, msg);
}

thread '...repro_fin_large_sequence_overflow' panicked at crates/malachite-app/src/streaming.rs:287:42:
attempt to add with overflow

Fix

A complete stream holds fin.sequence + 1 messages and can never hold more than MAX_MESSAGES_PER_STREAM, so a Fin with sequence >= MAX_MESSAGES_PER_STREAM describes a stream that can never complete. The fix is to reject that case up front and evict the stream, the same way the other malformed-stream paths already do, instead of recording an expected_messages it can never reach. That also keeps the + 1 from ever overflowing.

I've got the change ready on a branch with the comment corrected and a regression test covering the u64::MAX and boundary cases. Happy to open the PR if that works for you.

Thanks!