Drop ReceivedMessage and handle sending decoys in handshake

[nyonson]

Drop ReceivedMessage

• Dropping the ReceivedMessage struct in favor of the PacketType enum and Payload struct.
  • This negates the need for fix(lib): avoid message cloning within ReceivedMessage #59 and closes ReceivedMessage isn't quite right #57
• The heart of the refactor is in PacketReader::decrypt_contents, everything kinda bubbles up from there. That is still the spot where I am not quite sure to do with the header byte (which is currently only used to flag decoy packets). The caller probably doesn't care about the header byte, but it is difficult to ask for memory allocation to include it, but then ignore it. Figure most will use the with_alloc version though where we wrap it behind the Payload struct, so keeping it simple here for now.
• The PacketHandler functions where greatly simplified after this refactor, so went ahead and just dropped the wrapper functions and made the underlying reader/writers public.

Send Decoy Packets

Extend the complete_materials step of the handshake to optionally send decoy packets. This brings the library 100% up to spec, but more importantly for now, allows for an easy nice unit tests which covers both parties tossing garbage and decoys into the handshake.

Small Library Tweaks

• Removed Debug derivations on structs which hold secret values, best practice for security.
• Standardized error display punctuation.
• Renamed the MessageLengthTooSmall error variant to CiphertextTooSmall for clarity.
• Dropped un-used error variants to simplify API.
• Standardized use of NUM_* prefix in consts which are specifying the number of bytes and not the bytes themselves.
• Lint'd some assert usage in the tests.

Proxy Updates

• Handle decoy packets, they are simply ignored on the V2 side.

Documentation

• Consistent usage of "packet", "contents", and "message". Based it on the BIP324 spec. Added a blurb at the top of the module to help document this since we never use a struct to define the whole thing.
• Standardized bullet point usage with * instead of -.

[rustaceanrob]

Because this is always the first element, each subsequent element will be shifted to the left on the stack. See this. I don't think this is worth the performance hit if we can help it

[nyonson]

Ah good catch. Removing front elements is such a pain...lot of tradeoffs here and not sure I have thought of something good yet.

[rustaceanrob]

I think VecDeque has an additional byte of overhead but would allow pop_front in constant time. Either that or we just give the caller the decoy byte back and let them take a slice [1..]

[nyonson]

VecDeque would definitely help the impl, but then returning that is weird for the caller. I am trying to find some uses of it in the ecosystem and am struggling.

[nyonson]

Few ideas I am kicking around, trying to find the right balance between performance and complexity on the caller. And just to recap, the issue here is the header byte is covered by the auth tag so needs to present in the lower level libs, but is pretty useless for higher level callers.

1. Took a look at the bytes crate just for some inspiration since I don't think we would want to add a new dependency. But didn't see anything revolutionary.
2. Use a VecDeque instead of a Vec for the performance, but requires returning a VecDeque (or otherwise cancel out performance gains). I don't think these are used very often, for example can't find a single use in rust-bitcoin. Feels like a strange requirement to place on the caller given the hope is to lower confusion by removing the header byte.
3. Anything involving Vec (e.g. drain) will at some point require some byte coping, no way around it. Could just leave it for now, but does feel pretty bad.

Does this lead us back to a custom type? Maybe one which owns the underlying vector but supplies a contents slice to the caller so they don't have to remember what the header byte is?

[rustaceanrob]

Preferably I would like to address the copies now because it will be a significant pain point for large messages like Block. I agree that the VecDeque is not a good return value because iterating over a message backwards is meaningless to the caller. I can't exactly envision how the custom type would resolve the problem. You can fork this and try a new patch or just write over this one if you think you can resolve it that way

[rustaceanrob]

utACK ed6e361

[rustaceanrob]

Did you happen to test a handshake against a regtest instance?

[rustaceanrob]

Might want to add something like "excluding the ignore bit" for clarity. Not important at the moment though, just before next release

[nyonson]

Did you happen to test a handshake against a regtest instance?

Gave that a shot now. I had to add the --ignored flag to the regtest.sh script to make the regtest_handshake.sh run. The test passes, but the stop command is error'ing out for some reason. I'll take a look at that in a separate PR.