Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions smite/src/bolt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ mod tx_abort;
mod tx_complete;
mod tx_remove_input;
mod tx_remove_output;
mod tx_signatures;
mod types;
mod warning;
mod wire;
Expand All @@ -37,6 +38,7 @@ pub use tx_abort::TxAbort;
pub use tx_complete::TxComplete;
pub use tx_remove_input::TxRemoveInput;
pub use tx_remove_output::TxRemoveOutput;
pub use tx_signatures::TxSignatures;
pub use types::{
BigSize, CHANNEL_ID_SIZE, COMPACT_SIGNATURE_SIZE, ChannelId, MAX_MESSAGE_SIZE, TXID_SIZE, Txid,
};
Expand Down Expand Up @@ -124,6 +126,8 @@ pub mod msg_type {
pub const TX_REMOVE_OUTPUT: u16 = 69;
/// `tx_complete` message (BOLT 2).
pub const TX_COMPLETE: u16 = 70;
/// `tx_signatures` message (BOLT 2).
pub const TX_SIGNATURES: u16 = 71;
/// `tx_abort` message (BOLT 2).
pub const TX_ABORT: u16 = 74;
/// Gossip timestamp filter message (BOLT 7).
Expand Down Expand Up @@ -160,6 +164,8 @@ pub enum Message {
TxRemoveOutput(TxRemoveOutput),
/// `tx_complete` message (type 70).
TxComplete(TxComplete),
/// `tx_signatures` message (type 71).
TxSignatures(TxSignatures),
/// `tx_abort` message (type 74).
TxAbort(TxAbort),
/// Gossip timestamp filter message (type 265).
Expand Down Expand Up @@ -194,6 +200,7 @@ impl Message {
Self::TxRemoveInput(_) => msg_type::TX_REMOVE_INPUT,
Self::TxRemoveOutput(_) => msg_type::TX_REMOVE_OUTPUT,
Self::TxComplete(_) => msg_type::TX_COMPLETE,
Self::TxSignatures(_) => msg_type::TX_SIGNATURES,
Self::TxAbort(_) => msg_type::TX_ABORT,
Self::GossipTimestampFilter(_) => msg_type::GOSSIP_TIMESTAMP_FILTER,
Self::Unknown { msg_type, .. } => *msg_type,
Expand All @@ -219,6 +226,7 @@ impl Message {
Self::TxRemoveInput(m) => out.extend(m.encode()),
Self::TxRemoveOutput(m) => out.extend(m.encode()),
Self::TxComplete(m) => out.extend(m.encode()),
Self::TxSignatures(m) => out.extend(m.encode()),
Self::TxAbort(m) => out.extend(m.encode()),
Self::GossipTimestampFilter(m) => out.extend(m.encode()),
Self::Unknown { payload, .. } => out.extend(payload),
Expand Down Expand Up @@ -251,6 +259,7 @@ impl Message {
msg_type::TX_REMOVE_INPUT => Ok(Self::TxRemoveInput(TxRemoveInput::decode(cursor)?)),
msg_type::TX_REMOVE_OUTPUT => Ok(Self::TxRemoveOutput(TxRemoveOutput::decode(cursor)?)),
msg_type::TX_COMPLETE => Ok(Self::TxComplete(TxComplete::decode(cursor)?)),
msg_type::TX_SIGNATURES => Ok(Self::TxSignatures(TxSignatures::decode(cursor)?)),
msg_type::TX_ABORT => Ok(Self::TxAbort(TxAbort::decode(cursor)?)),
msg_type::GOSSIP_TIMESTAMP_FILTER => Ok(Self::GossipTimestampFilter(
GossipTimestampFilter::decode(cursor)?,
Expand Down Expand Up @@ -517,6 +526,22 @@ mod tests {
assert_eq!(decoded, Message::TxAbort(tx_abort));
}

#[test]
fn message_tx_signatures_roundtrip() {
let tx_sigs = TxSignatures {
channel_id: ChannelId::new([0xef; CHANNEL_ID_SIZE]),
txid: Txid::from_byte_array([0xcc; TXID_SIZE]),
witnesses: vec![
vec![vec![0xde, 0xad], vec![0xbe, 0xef]],
vec![vec![0x01]],
],
};
let msg = Message::TxSignatures(tx_sigs.clone());
let encoded = msg.encode();
let decoded = Message::decode(&encoded).unwrap();
assert_eq!(decoded, Message::TxSignatures(tx_sigs));
}

#[test]
fn message_unknown_roundtrip() {
let msg = Message::Unknown {
Expand Down Expand Up @@ -584,6 +609,15 @@ mod tests {
.msg_type(),
msg_type::TX_COMPLETE
);
assert_eq!(
Message::TxSignatures(TxSignatures {
channel_id: ChannelId::new([0; CHANNEL_ID_SIZE]),
txid: Txid::from_byte_array([0; TXID_SIZE]),
witnesses: vec![],
})
.msg_type(),
msg_type::TX_SIGNATURES
);
assert_eq!(
Message::TxAbort(TxAbort::new(ChannelId::new([0; CHANNEL_ID_SIZE]), "")).msg_type(),
msg_type::TX_ABORT
Expand Down
229 changes: 229 additions & 0 deletions smite/src/bolt/tx_signatures.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,229 @@
//! BOLT 2 `tx_signatures` message.

use super::BoltError;
use super::types::{ChannelId, Txid};
use super::wire::WireFormat;

/// BOLT 2 `tx_signatures` message (type 71).
///
/// Sent during interactive transaction construction to provide the sender's
/// witnesses for the negotiated transaction. Both peers exchange
/// `tx_signatures` before broadcasting.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct TxSignatures {
/// The channel ID.
pub channel_id: ChannelId,
/// The transaction ID (little-endian, Bitcoin serialization).
pub txid: Txid,
/// Witnesses for the transaction inputs.
///
/// 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>>>,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>.

}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also should implement the tlvs field.


impl TxSignatures {
/// Encodes to wire format (without message type prefix).
#[must_use]
pub fn encode(&self) -> Vec<u8> {
let mut out = Vec::new();
self.channel_id.write(&mut out);
self.txid.write(&mut out);
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);
Comment thread
morehouse marked this conversation as resolved.
Outdated
for item in witness {
item.write(&mut out);
}
}
out
}

/// Decodes from wire format (without message type prefix).
///
/// # Errors
///
/// Returns `Truncated` if the payload is too short for any fixed field or
/// declared variable-length data.
pub fn decode(payload: &[u8]) -> Result<Self, BoltError> {
let mut cursor = payload;
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fuzzer shouldn't be choosing values that get decoded, only values that get encoded.

Either way, num_witnesses is capped at 64K due to u16::MAX. I think it makes sense to preallocate in this case.

for _ in 0..num_witnesses {
let num_items: u16 = WireFormat::read(&mut cursor)?;
let mut stack = Vec::with_capacity(num_items as usize);
for _ in 0..num_items {
let item: Vec<u8> = WireFormat::read(&mut cursor)?;
stack.push(item);
}
witnesses.push(stack);
}
Ok(Self {
channel_id,
txid,
witnesses,
})
}
}

#[cfg(test)]
mod tests {
use super::super::{CHANNEL_ID_SIZE, TXID_SIZE};
use super::*;
use secp256k1::hashes::Hash;

fn sample_txid() -> Txid {
Txid::from_byte_array([0xcc; TXID_SIZE])
}

#[test]
fn roundtrip() {
let original = TxSignatures {
channel_id: ChannelId::new([0xab; CHANNEL_ID_SIZE]),
txid: sample_txid(),
witnesses: vec![
vec![vec![0x01, 0x02], vec![0x03]],
vec![vec![0xde, 0xad, 0xbe, 0xef]],
],
};
let encoded = original.encode();
let decoded = TxSignatures::decode(&encoded).unwrap();
assert_eq!(original, decoded);
}

#[test]
fn roundtrip_empty_witnesses() {
let original = TxSignatures {
channel_id: ChannelId::new([0x00; CHANNEL_ID_SIZE]),
txid: sample_txid(),
witnesses: vec![],
};
let encoded = original.encode();
// channel_id(32) + txid(32) + num_witnesses(2) = 66
assert_eq!(encoded.len(), 66);
let decoded = TxSignatures::decode(&encoded).unwrap();
assert_eq!(original, decoded);
}

#[test]
fn roundtrip_empty_stack_items() {
// A witness with zero stack items is valid wire-wise.
let original = TxSignatures {
channel_id: ChannelId::new([0x11; CHANNEL_ID_SIZE]),
txid: sample_txid(),
witnesses: vec![vec![], vec![]],
};
let encoded = original.encode();
let decoded = TxSignatures::decode(&encoded).unwrap();
assert_eq!(original, decoded);
}

#[test]
fn decode_ignores_trailing_bytes() {
let original = TxSignatures {
channel_id: ChannelId::new([0xff; CHANNEL_ID_SIZE]),
txid: sample_txid(),
witnesses: vec![],
};
let mut encoded = original.encode();
encoded.extend_from_slice(&[0xaa, 0xbb, 0xcc]);
let decoded = TxSignatures::decode(&encoded).unwrap();
assert_eq!(decoded, original);
}

#[test]
fn encode_size() {
// channel_id(32) + txid(32) + num_witnesses(2) + num_items(2) + item_len(2) + item(3) = 73
let msg = TxSignatures {
channel_id: ChannelId::new([0x42; CHANNEL_ID_SIZE]),
txid: sample_txid(),
witnesses: vec![vec![vec![0xaa, 0xbb, 0xcc]]],
};
let encoded = msg.encode();
assert_eq!(encoded.len(), 32 + 32 + 2 + 2 + 2 + 3);
}

#[test]
fn decode_empty() {
assert_eq!(
TxSignatures::decode(&[]),
Err(BoltError::Truncated {
expected: CHANNEL_ID_SIZE,
actual: 0
})
);
}

#[test]
fn decode_truncated_channel_id() {
assert_eq!(
TxSignatures::decode(&[0x00; 20]),
Err(BoltError::Truncated {
expected: CHANNEL_ID_SIZE,
actual: 20
})
);
}

#[test]
fn decode_truncated_txid() {
let mut data = vec![0xaa; CHANNEL_ID_SIZE];
data.extend_from_slice(&[0x00; 20]); // only 20 bytes of txid
assert_eq!(
TxSignatures::decode(&data),
Err(BoltError::Truncated {
expected: TXID_SIZE,
actual: 20
})
);
}

#[test]
fn decode_truncated_witnesses_count() {
let mut data = vec![0xaa; CHANNEL_ID_SIZE];
data.extend_from_slice(&[0xcc; TXID_SIZE]);
data.push(0x00); // only 1 byte of witnesses count
assert_eq!(
TxSignatures::decode(&data),
Err(BoltError::Truncated {
expected: 2,
actual: 1
})
);
}

#[test]
fn decode_truncated_stack_items_count() {
let mut data = vec![0xaa; CHANNEL_ID_SIZE];
data.extend_from_slice(&[0xcc; TXID_SIZE]);
data.extend_from_slice(&[0x00, 0x01]); // num_witnesses = 1
data.push(0x00); // only 1 byte of stack items count
assert_eq!(
TxSignatures::decode(&data),
Err(BoltError::Truncated {
expected: 2,
actual: 1
})
);
}

#[test]
fn decode_truncated_item_data() {
let mut data = vec![0xaa; CHANNEL_ID_SIZE];
data.extend_from_slice(&[0xcc; TXID_SIZE]);
data.extend_from_slice(&[0x00, 0x01]); // num_witnesses = 1
data.extend_from_slice(&[0x00, 0x01]); // num_stack_items = 1
data.extend_from_slice(&[0x00, 0x05]); // item_len = 5
data.extend_from_slice(&[0x11, 0x22]); // only 2 bytes of item data
assert_eq!(
TxSignatures::decode(&data),
Err(BoltError::Truncated {
expected: 5,
actual: 2
})
);
}
}
Loading