Backport: Make SendCmpct idempotent#6525
Conversation
The SendCmpct type uses the impl_consensus_encoding macro to implement Encodable and Decodable. Since we want to include a new failure mode for the decoding side, the full logic will need to be brought out first. Replace call of impl_consensus_encoding for SendCmpct with trait impls for Encodable and Decodable.
Normalizing the mode bit makes serialization/deserialization _not_ idempotent. That's due to the checksum that's created before normalization which if serialized again, no longer has the same checksum. This cases a failure to serialize the same `SendCmpct` which was received if the mode bit is anything other then 0 or 1. Furthermore, the spec https://bips.dev/152/ defines only 0 (low-bandwidth mode) or 1 (high-bandwidth mode) so anything other than 0 or 1 is unspecified and should result in an error. As quoted from spec https://bips.dev/152/: The first integer SHALL be interpreted as a boolean (and MUST have a value of either 1 or 0) Original work by yancy<github@yancy.lol>
Add a test that if the mode bit of the sendcmpct should fail if not 1 or 0. Original work by: - Abeeujah <abeeujah@gmail.com> - yancy <github@yancy.lol>
|
Should this also be backported to |
|
Can/should this be draft until the original merges? |
If you can be bothered then I"ll review it. |
This one can go first I believe. The "original" is not passing Andrews fuzz tests because 0.32.x does not have this change. Although I guess arguably they could go in any order since the result is that then both 0.32.x and p2p@master will behave the same. |
|
I'd like them to go in around the same time. Let's undraft this and get all the needed ACKs in, and I'll merge them in one go. |
|
Oh, I see, what actually needs to happen is:
So there is no need to do anything "at the same time", but we do need to do the backport first. |
|
There. Now we have to get #6317 in. |
|
Release is ready to go: #6471 |
Backport of #6317
From the spec https://bips.dev/152/
It seems like the spec says that anything besides 1 or 0 is actually an error, though..
Original work by: