bolt: implement tx_signatures message codec#28
bolt: implement tx_signatures message codec#28harsh04044 wants to merge 2 commits intomorehouse:masterfrom
Conversation
smite/src/bolt/tx_signatures.rs
Outdated
| /// Outer `Vec`: one entry per input. | ||
| /// Middle `Vec`: the witness stack for that input. | ||
| /// Inner `Vec<u8>`: a single witness stack item. | ||
| pub witnesses: Vec<Vec<Vec<u8>>>, |
There was a problem hiding this comment.
This looks wrong -- shouldn't there only be an outer and an inner vec (outer: witnesses, inner: witness_data)? I think it would also help readability to define a newtype for Witness in this file. Then we just have witnesses: Vec<Witness>.
| /// Middle `Vec`: the witness stack for that input. | ||
| /// Inner `Vec<u8>`: a single witness stack item. | ||
| pub witnesses: Vec<Vec<Vec<u8>>>, | ||
| } |
There was a problem hiding this comment.
We also should implement the tlvs field.
smite/src/bolt/tx_signatures.rs
Outdated
| u16::try_from(self.witnesses.len()).unwrap_or(u16::MAX).write(&mut out); | ||
| for witness in &self.witnesses { | ||
| u16::try_from(witness.len()).unwrap_or(u16::MAX).write(&mut out); |
There was a problem hiding this comment.
Rather than silently truncating, I think we should assert that u16::MAX is not exceeded, since that indicates a programming error on our part.
|
Done: added the Witness newtype, TxSignaturesTlvs, and swapped the unwrap_or(u16::MAX) calls for asserts. |
|
Since we're panicking in |
| let channel_id: ChannelId = WireFormat::read(&mut cursor)?; | ||
| let txid: Txid = WireFormat::read(&mut cursor)?; | ||
| let num_witnesses: u16 = WireFormat::read(&mut cursor)?; | ||
| let mut witnesses = Vec::with_capacity(num_witnesses as usize); |
There was a problem hiding this comment.
Looks like recipe for an Out-Of-Memory (OOM) crash in case the fuzzer decides on a high value for num_witnesses or num_items. This would mean the WireFormat::read loop doesn't even get a chance to fail on a truncated payload.
I'd say it's better to let the vector grow dynamically:
let mut witnesses = Vec::new();
3 participants
Implements the
tx_signaturescodec (type 71) for BOLT 2 interactive-tx.The witnesses field is a nested structure -- one entry per input, each containing a witness stack of variable-length items. Encoded as num_witnesses (u16) followed by num_items (u16) and length-prefixed items for each witness.
Tests cover roundtrip, empty witnesses, empty stack items, trailing bytes, wire size, and truncation at each field boundary.
Part of #5.