rust-bitcoin / rust-bitcoin / 12560498891

83%

push

github

Merge rust-bitcoin/rust-bitcoin#3789: Add hash data to Inventory's Error variant

72e97c637 Add a hash value to Inventory's Error variant (Nick Johnson)

Pull request description:

  I am working on adding BIP324 V2 p2p network message support to the p2p module and ran into a little snag. I can post a draft branch of that work if helpful, but the general strategy is to add a `V2NetworkMessage` which operates in parallel with the existing `RawNetworkMessage` type (which is a bit of misnomer and may be better described in the future as `V1NetworkMessage`, see #3157). This allows both p2p message types to hook into the existing `Encodable`/`Decodeable` chain.

  But the `Error` variant of the `Inventory` type message does not have symmetrical encoding and decoding paths. Encoding writes out a zero'd 32 bytes while decoding ignores it completely. And while it is not clear to me if there is [requirement written down](https://en.bitcoin.it/wiki/Protocol_documentation#Inventory_Vectors) anywhere, [bitcoin core does appear to always expect a hash](https://github.com/bitcoin/bitcoin/blob/c1252b14d/src/protocol.h#L499) even with the `Error` variant where it is meaningless.

  I believe rust-bitcoin's handling of this was never a problem before because the top level `RawNetworkPackage` pulls all the required bytes off a reader before decoding them. But this is not as easy to do with the v2 p2p network messages since the length is decoded at the transport level, not the message itself. I believe it is preferable for the Decoders to *not* assume that all bytes have been pulled off already given their input stream interface. Maybe somewhat surprisingly, this is the only issue I have run into so far adding the v2 encoding and decoding paths. As it is now, the code panics with an `Unconsumed` because it hasn't pulled the dummy zero bytes off the reader.

  This patch adds the 32 byte array to the Error var... (continued)

3 of 4 new or added lines in 2 files covered. (75.0%)

20418 of 24659 relevant lines covered (82.8%)

17598.91 hits per line