Implement BCS deserialization from_reader

[preston-evans98]

This Pull Request implements BCS deserialization from implementers of std::io::Read.

Strategy

The PR aims to maintain a minimal diff over the previous implementation. To that end, the following changes have been made.

• Make Deserializer generic over an inner type R
• Factor as much of the implementation as possible into a new trait BcsReader, which implements BCS in terms of a few primitive operations. Implement serde::Deserialize<'de> for Deserializer<R> where Deserializer<R>: BcsReader<'de>
• Implement BcsReader<'de> for Deserializer<&'de [u8]> using existing code
• Implement BcsReader generically for (a version of) Deserializer<R: Read>.

Implementing BcsReader generically

The existing implementation is very close to supporting deserialization from readers. There are only two places in which its behavior relied on access to a byte slice was in the implementation of de::MapAccess.

The first case is map deserialization. bcs needs access to the serialized representation of map keys in order to enforce canonicity. When deserializing from a slice, this is straightforward - the implementation can simply deserialize a map key, and then "look back" at the original input slice to determine its serialized representation. When deserializing from a reader, however, this kind of rewinding is not generally possible. To solve this problem, I introduce a the TeeReader struct, which wraps a Reader and optionally copies its bytes into a capture_buffer for later retrieval. Then, I simply implement BcsReader for Deserializer<TeeReader<...>>.

The second case is the end method, which checks that all input bytes have been consumed. To handle this case, I simply attempt to read one extra byte from the Reader after deserialization and assert that an EOF error is returned from the underlying Reader.

Testing

All existing unit tests except for zero_copy_parse have been modified to test that deserialization from_bytes and from_reader yield the same output. That test is not applicable since readers are not capable of zero-copy deserialization.

[ma2bd]

Thanks @preston-evans98 for the changes. Hopefully, @bmwill and I will find time for a proper code review soon.

[preston-evans98]

It looks like CI failed initially due to an outdated clippy allow directive, and because cargo selected a version of thiserror which is only compatible with the 2021 rust edition. I removed the lint, and pinned thiserror to exactly the version specified in the Cargo.toml. This should be safe to do because thiserror does not appear in the crate's public API.

[preston-evans98]

Hmm, looks like the MSRV failing due to unrelated changes. Are you opposed to checking in a Cargo.lock?

[ma2bd]

@preston-evans98 Thanks for investigating. I fixed the CI on the main branch. Can you rebase your PR?

[ma2bd]

@preston-evans98 @bmwill This was just released in bcs v0.1.6

[preston-evans98]

Fantastic, thanks for the review @ma2bd @bmwill!