Binary sv2 cleanup

[Shourya742]

closes: #2171 #2173 #2175 #2176 #2177 #2178 #2179 #2213 #2214 #2215 #2216

Apart from the issues mentioned above, this PR introduces several additional improvements:

1. Removes a significant amount of redundant code.
2. Expands the API surface. In particular, conversions for copy types are now lossless, while conversions for variable-sized types are performed through TryFrom. (More details can be found in the companion sv2-apps PR. I'm open to suggestions for alternative APIs these additions are primarily based on my experience working with the sv2-apps codebase.)
3. Introduces a dedicated MAC type, bringing us into full compliance with the specification.
4. Replaces a large number of handwritten From implementations with MBE macros, significantly reducing duplication.
5. Fixes issues in the streaming APIs. Previously, certain code paths could lead to unbounded reads, creating potential attack vectors. Reads are now properly bounded, and recursion-related bounds have been removed from the writer implementation.
6. Removes U32Ref and U8Owned, as they do not provide any meaningful structural advantages.
7. Adds iterator support for Seq types, allowing users to iterate over elements and perform in-place modifications more ergonomically.

companion stratum-mining/sv2-apps#576

[Shourya742]

Awesome, the test are passing, atleast I have not broken any invariants. Need to improve API ergonomics and should be good to go.

[Shourya742]

I will open issues for all the extra points mentioned in the description.

[GitGab19]

My clanker found this:

High: the new bounded from_reader can still block/read until EOF on an invalid B032 length. [decodable.rs (line 70)](/tmp/stratum-pr2200/sv2/binary-sv2/src/codec/decodable.rs:70) treats every ReadError as “need one more byte”, but [inner.rs (line 109)](/tmp/stratum-pr2200/sv2/binary-sv2/src/datatypes/non_copy_data_types/inner.rs:109) also returns ReadError when a variable-sized type declares more than its max, e.g. B032 header 33. For messages with B032 fields, a peer can send an oversized length and keep the socket open; the loop keeps reading instead of rejecting the message. I’d split “insufficient bytes” from “declared size exceeds max”, or make from_reader retry only on the former.

[GitGab19]

@Shourya742 just a suggestion about the point raised by the clanker:

Yes. The core fix is to stop using ReadError for two different meanings:

1. “I need more bytes to know the size”
2. “The size is known and invalid”

Right now both flow through ReadError, and decodable.rs treats every ReadError as recoverable by reading one more byte.

The best fix is in Inner::expected_length / expected_length_for_reader:

// pseudo-shape
if payload_len <= MAXSIZE {
    Ok(payload_len + HEADERSIZE)
} else {
    Err(Error::ValueExceedsMaxSize(
        ISFIXED,
        SIZE,
        HEADERSIZE,
        MAXSIZE,
        data.to_vec(),
        payload_len,
    ))
}

For the reader path:

if expected_length <= MAXSIZE {
    Ok(expected_length)
} else {
    Err(Error::ValueExceedsMaxSize(
        ISFIXED,
        SIZE,
        HEADERSIZE,
        MAXSIZE,
        header.to_vec(),
        expected_length,
    ))
}

Then Decodable::from_reader can keep retrying only on true partial-read errors:

Err(Error::OutOfBound | Error::ReadError(_, _)) => read_one_more_byte()
Err(error) => return Err(error)

With that change, B032 input [33] would return immediately because the header is complete and declares a payload larger than 32. It would no longer read/block waiting for 33 bytes.

A slightly cleaner design would add a dedicated error like Error::InvalidDeclaredLength { declared, max }, because ValueExceedsMaxSize carries a value vector and is a bit awkward for “length header is invalid”. But if the PR wants a small surgical fix, returning ValueExceedsMaxSize from the known-invalid length paths is enough.

[Shourya742]

Loupe tests are in, and all are passing, I will link the commits to corresponding loupe issue for reference.

[Shourya742]

@GitGab19 the commits might be a bit different now, I rewrote the history with test and your suggestions.

[plebhash]

I asked gpt-5.3-codex whether these .expects were infallible:

Short answer: conditionally, not absolutely.

• They are infallible if the invariant holds: for Inner<'_, true, SIZE, 0, 0>, payload length is always exactly SIZE.
• With current API, that invariant is usually enforced by checked constructors (TryFrom, from_bytes_, from_reader_), so in normal flow these expects should not fire.
• But Inner is a pub enum, so callers can still construct invalid fixed values directly (e.g. Inner::Owned(vec![0; SIZE - 1])), and then as_array() / into_array() will panic.

So: infallible by convention, not by type-level encapsulation.

not a blocker for me, but I thought I'd bring it up anyways

[Shourya742]

We gonna be removing Inner, thats the end goal.