From 772c88a06891fb1bed80d79538a380a6c67e9618 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Mon, 12 Oct 2026 11:24:22 +0530 Subject: [PATCH 1/6] smite: add channel_ready oracle Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 29 +++--- smite-scenarios/src/executor/tests.rs | 3 +- smite/src/oracles.rs | 2 + smite/src/oracles/channel_ready.rs | 136 ++++++++++++++++++++++++++ smite/src/violation.rs | 10 +- 5 files changed, 162 insertions(+), 18 deletions(-) create mode 100644 smite/src/oracles/channel_ready.rs diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index a633501a..5c935370 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -20,7 +20,8 @@ use smite::channel_tx::{ }; use smite::noise::{ConnectionError, NoiseConnection}; use smite::oracles::{ - AcceptChannelContext, AcceptChannelOracle, FundingSignedContext, FundingSignedOracle, Oracle, + AcceptChannelContext, AcceptChannelOracle, ChannelReadyContext, ChannelReadyOracle, + FundingSignedContext, FundingSignedOracle, Oracle, }; use smite::pending_channel::{KeyOrigin, PendingChannel}; use smite::violation::Violation; @@ -1179,9 +1180,9 @@ fn build_channel_update(variables: &[Option], inputs: &[usize]) -> Cha } /// Returns `true` if a message of type `msg_type` is handled implicitly by the -/// executor: [`recv_non_ping`] records it into the executor's tracking state -/// and returns it, and explicit receives skip past it. See each message's -/// `record_*` function for the state it records. +/// executor: [`recv_non_ping`] evaluates its oracle, then records it into the +/// executor's tracking state and returns it, and explicit receives skip past +/// it. See each message's `record_*` function for the state it records. /// /// This currently includes: /// - `channel_ready` @@ -1221,7 +1222,11 @@ fn recv_non_ping( } Message::ChannelReady(ref cr) => { log::debug!("received channel_ready on {}", cr.channel_id); - record_recv_channel_ready(channel_states, revealed_pubkeys, cr)?; + ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready: cr, + channel: channel_states.get(&cr.channel_id), + })?; + record_recv_channel_ready(channel_states, revealed_pubkeys, cr); return Ok(msg); } Message::Unknown { .. } => { @@ -1422,18 +1427,18 @@ fn record_recv_funding_signed( /// counterparty's next per-commitment point on the channel it identifies, and /// in `revealed_pubkeys` as a per-commitment point the target revealed. /// -/// # Errors +/// # Panics /// -/// Returns [`Violation::UnknownChannel`] if no channel state exists for the -/// message's `channel_id`. +/// Panics if no matching channel state exists. This should be unreachable, as +/// `ChannelReadyOracle` reports such messages as a [`Violation`]. fn record_recv_channel_ready( channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, channel_ready: &ChannelReady, -) -> Result<(), Violation> { +) { let state = channel_states .get_mut(&channel_ready.channel_id) - .ok_or(Violation::UnknownChannel(channel_ready.channel_id))?; + .expect("ChannelReadyOracle guaranteed this channel_id exists"); *state.next_counterparty_per_commitment_point_mut() = Some(channel_ready.second_per_commitment_point); @@ -1441,7 +1446,7 @@ fn record_recv_channel_ready( // target revealed, for commitment number 1. It is unique, so record its // only origin. // - // TODO: Once we have the channel_ready oracle, flag a reused + // TODO: In `ChannelReadyOracle`, flag a reused // `second_per_commitment_point`, i.e. one already present in // `revealed_pubkeys`, unless the entry is a `TargetPcp` for this same // channel and commitment number. That exception allows the two legitimate @@ -1455,8 +1460,6 @@ fn record_recv_channel_ready( commitment_number: 1, }, ); - - Ok(()) } /// Extracts a field from a parsed `accept_channel` message. diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 0fefe0f5..ca2aa8f3 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1569,10 +1569,11 @@ fn execute_recv_implicit_channel_ready_unknown_channel() { .queue(&channel_ready_reply(sample_pubkey(1))) .queue(&Message::AcceptChannel(sample_accept_channel())) .run_err(&negotiate_channel_program(&announced_open_channel())); - let ExecuteError::Violation(Violation::UnknownChannel(id)) = &err else { + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { panic!("unexpected error: {err:?}"); }; assert_eq!(*id, funding_channel_id()); + assert!(reason.contains("unknown channel_id: no channel was funded for it")); } #[test] diff --git a/smite/src/oracles.rs b/smite/src/oracles.rs index 937b2fc2..8a4e395f 100644 --- a/smite/src/oracles.rs +++ b/smite/src/oracles.rs @@ -3,10 +3,12 @@ //! Oracles evaluate conditions beyond simple crashes. mod accept_channel; +mod channel_ready; mod funding_signed; use super::violation::Violation; pub use accept_channel::{AcceptChannelContext, AcceptChannelOracle}; +pub use channel_ready::{ChannelReadyContext, ChannelReadyOracle}; pub use funding_signed::{FundingSignedContext, FundingSignedOracle}; /// `Oracle` evaluates a condition against some context diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs new file mode 100644 index 00000000..5822eac7 --- /dev/null +++ b/smite/src/oracles/channel_ready.rs @@ -0,0 +1,136 @@ +//! BOLT 2 `channel_ready` oracle, for the channel funding flow. + +use super::Oracle; +use crate::bolt::ChannelReady; +use crate::channel_state::ChannelState; +use crate::violation::Violation; + +/// Context for `ChannelReadyOracle` +pub struct ChannelReadyContext<'a> { + /// The `channel_ready` received from the peer. + pub channel_ready: &'a ChannelReady, + /// The channel the `channel_ready` belongs to, identified by its + /// `channel_id`, or `None` if no channel was funded for it. + pub channel: Option<&'a ChannelState>, +} + +/// Checks whether a received `channel_ready` satisfies the BOLT 2 channel +/// establishment requirements. +pub struct ChannelReadyOracle; + +impl Oracle> for ChannelReadyOracle { + fn evaluate(&self, context: &ChannelReadyContext<'_>) -> Result<(), Violation> { + // Check that the `channel_ready` answers a channel we funded. + if context.channel.is_none() { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + "unknown channel_id: no channel was funded for it".to_string(), + )); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bolt::{ChannelId, ChannelReadyTlvs, Features}; + use crate::channel_tx::{ChannelConfig, ChannelPartyConfig, HolderIdentity, Side}; + use bitcoin::OutPoint; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + + /// Number of confirmations required to reach the minimum depth and expect + /// a `channel_ready` message from the target. + const CONFIRMED_DEPTH: u32 = 6; + + fn secret_key(seed: u8) -> SecretKey { + SecretKey::from_slice(&[seed; 32]).expect("valid secret key") + } + + fn pubkey(seed: u8) -> PublicKey { + PublicKey::from_secret_key(&Secp256k1::new(), &secret_key(seed)) + } + + /// Valid `channel_ready` message for testing. + fn channel_ready() -> ChannelReady { + ChannelReady { + channel_id: ChannelId::new([1u8; 32]), + second_per_commitment_point: pubkey(2), + tlvs: ChannelReadyTlvs::default(), + } + } + + /// Funded channel state for testing. + fn channel_state() -> ChannelState { + let pkey1 = pubkey(1); + let pkey2 = pubkey(2); + let party = |key| ChannelPartyConfig { + funding_pubkey: key, + payment_basepoint: key, + revocation_basepoint: key, + delayed_payment_basepoint: key, + dust_limit_satoshis: 546, + to_self_delay: 144, + }; + + let config = ChannelConfig { + funding_outpoint: OutPoint::null(), + funding_satoshis: 10_000_000, + channel_type: Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), + opener: party(pkey1), + acceptor: party(pkey2), + minimum_depth: CONFIRMED_DEPTH, + }; + let commitments = config + .new_initial_commitments(3_000_000_000, 15_000, pkey1, pkey2) + .expect("valid initial commitments"); + let holder = HolderIdentity { + side: Side::Opener, + funding_privkey: secret_key(1), + }; + + ChannelState::new(config, holder, commitments, true, false, false) + } + + #[track_caller] + fn assert_pass(channel_ready: &ChannelReady, channel: Option<&ChannelState>) { + if let Err(err) = ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready, + channel, + }) { + panic!("expected pass, got: {err}"); + } + } + + #[track_caller] + fn assert_fail(channel_ready: &ChannelReady, channel: Option<&ChannelState>, expected: &str) { + match ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready, + channel, + }) { + Err(Violation::InvalidChannelReady(chan_id, reason)) => { + assert_eq!(channel_ready.channel_id, chan_id); + assert!( + reason.contains(expected), + "unexpected failure reason: {reason}" + ); + } + _ => panic!("expected failure: {expected}"), + } + } + + #[test] + fn conforming_channel_ready_passes() { + assert_pass(&channel_ready(), Some(&channel_state())); + } + + #[test] + fn unknown_channel_id_fails() { + assert_fail( + &channel_ready(), + None, + "unknown channel_id: no channel was funded for it", + ); + } +} diff --git a/smite/src/violation.rs b/smite/src/violation.rs index a0987795..5aeb9385 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -47,8 +47,10 @@ pub enum Violation { #[error("invalid funding_signed for channel_id {0}: {1}")] InvalidFundingSigned(ChannelId, String), - /// The target sent a `channel_ready` for a `channel_id` we never opened, - /// i.e. one for which no state was ever established. - #[error("unknown channel: no tracked state for channel_id {0}")] - UnknownChannel(ChannelId), + /// The target's `channel_ready` broke a BOLT 2 requirement, as judged by + /// [`crate::oracles::ChannelReadyOracle`]. The reason names the breached + /// requirement, one of: + /// - it names a `channel_id` we have not funded. + #[error("invalid channel_ready for channel_id {0}: {1}")] + InvalidChannelReady(ChannelId, String), } From c9645c18f6f67b831fc65fec6cb34d463be7b0ac Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Mon, 12 Oct 2026 23:47:37 +0530 Subject: [PATCH 2/6] smite: add alias and point reuse checks to channel_ready oracle Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 17 ++- smite-scenarios/src/executor/tests.rs | 105 ++++++++++++++++++ smite/src/oracles/channel_ready.rs | 154 +++++++++++++++++++++++++- smite/src/violation.rs | 4 +- 4 files changed, 265 insertions(+), 15 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 5c935370..4d43c262 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -527,6 +527,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &self.context, RECV_IDLE_TIMEOUT, )?; log::debug!("[{:?}] RecvAcceptChannel: received", start.elapsed()); @@ -555,6 +556,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &self.context, RECV_IDLE_TIMEOUT, )?; log::debug!("[{:?}] RecvFundingSigned: received", start.elapsed()); @@ -575,6 +577,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &self.context, RECV_CHANNEL_READY_TIMEOUT, )?; log::debug!("[{:?}] RecvChannelReady: received", start.elapsed()); @@ -1207,6 +1210,7 @@ fn recv_non_ping( conn: &mut impl Connection, channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, + context: &ProgramContext, timeout: Duration, ) -> Result { let previous = conn.read_timeout()?; @@ -1225,6 +1229,8 @@ fn recv_non_ping( ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready: cr, channel: channel_states.get(&cr.channel_id), + negotiated_features: &context.negotiated_features, + revealed_pubkeys, })?; record_recv_channel_ready(channel_states, revealed_pubkeys, cr); return Ok(msg); @@ -1287,10 +1293,11 @@ fn recv_bolt( conn: &mut impl Connection, channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, + context: &ProgramContext, timeout: Duration, ) -> Result { loop { - let msg = recv_non_ping(conn, channel_states, revealed_pubkeys, timeout)?; + let msg = recv_non_ping(conn, channel_states, revealed_pubkeys, context, timeout)?; let got = msg.msg_type(); if let Some(m) = M::from_message(msg) { return Ok(m); @@ -1445,14 +1452,6 @@ fn record_recv_channel_ready( // Record the `second_per_commitment_point` as a per-commitment point the // target revealed, for commitment number 1. It is unique, so record its // only origin. - // - // TODO: In `ChannelReadyOracle`, flag a reused - // `second_per_commitment_point`, i.e. one already present in - // `revealed_pubkeys`, unless the entry is a `TargetPcp` for this same - // channel and commitment number. That exception allows the two legitimate - // repeats: the target may resend `channel_ready` with a different alias but - // the same point, and we may receive the same point again after a - // reconnection. revealed_pubkeys.insert( channel_ready.second_per_commitment_point, KeyOrigin::TargetPcp { diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index ca2aa8f3..2b2d3289 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1592,6 +1592,111 @@ fn execute_recv_channel_ready_unexpected_message() { )); } +#[test] +fn execute_recv_channel_ready_rejects_reused_our_pubkey() { + // The target's `channel_ready` reuses a pubkey we revealed in an + // `open_channel` of a different negotiation. + let mut oc = SampleOpenChannel::new(PointSource::Secret([0x11; 32])); + oc.message.temporary_channel_id = TemporaryChannelId::new([0xcc; 32]); + let reused = oc.message.funding_pubkey; + + let mut b = ProgramBuilder::new(); + let sent_open_channel = send_open_channel(&mut b, &oc); + let funding_created = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + + let err = Fixture::new() + .with_negotiation(sample_funding_negotiation()) + .queue(&funding_signed_reply(funding_channel_id())) + .queue(&channel_ready_reply(reused)) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, funding_channel_id()); + assert!(reason.contains(&format!( + "second_per_commitment_point {reused} reuses our key from instruction {}", + sent_open_channel.sent, + ))); +} + +#[test] +fn execute_recv_channel_ready_rejects_reused_target_static_pubkey() { + let second_id = TemporaryChannelId::new([0xcc; 32]); + let mut oc = announced_open_channel(); + oc.message.temporary_channel_id = second_id; + + // The target's `channel_ready` reuses a static pubkey it revealed in an + // `accept_channel` of a different negotiation. + let accept_channel = AcceptChannel { + temporary_channel_id: second_id, + ..sample_accept_channel() + }; + let reused = accept_channel.delayed_payment_basepoint; + + let mut b = ProgramBuilder::new(); + negotiate_channel(&mut b, &oc); + let funding_created = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + + let err = Fixture::new() + .with_negotiation(sample_funding_negotiation()) + .queue(&Message::AcceptChannel(accept_channel)) + .queue(&funding_signed_reply(funding_channel_id())) + .queue(&channel_ready_reply(reused)) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, funding_channel_id()); + assert!(reason.contains(&format!( + "second_per_commitment_point {reused} reuses target's delayed_payment_basepoint on channel {second_id}" + ))); +} + +#[test] +fn execute_recv_channel_ready_rejects_reused_our_channel_ready_pubkey() { + let our_sk = SecretKey::from_slice(&[0x42; 32]).expect("valid secret key"); + let reused = PublicKey::from_secret_key(&Secp256k1::new(), &our_sk); + + // Send a `channel_ready` revealing a point we hold the secret for, then the + // target's `channel_ready` reuses it. + let mut b = ProgramBuilder::new(); + let funding_created = send_funding_created(&mut b); + let channel_id = b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(6), &[]); + let sk = b.append(Operation::LoadPrivateKey(our_sk.secret_bytes()), &[]); + let our_point = b.append(Operation::DerivePoint, &[sk]); + let alias = b.append(Operation::LoadShortChannelId(0), &[]); + let sent_channel_ready = b.append( + Operation::SendChannelReady { + include_alias: true, + }, + &[channel_id, our_point, alias], + ); + b.append(Operation::RecvChannelReady, &[]); + + let err = Fixture::new() + .with_negotiation(sample_funding_negotiation()) + .queue(&funding_signed_reply(funding_channel_id())) + .queue(&channel_ready_reply(reused)) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, funding_channel_id()); + assert!(reason.contains(&format!( + "second_per_commitment_point {reused} reuses our key from instruction {sent_channel_ready}" + ))); +} + // -- extract_field tests -- // TODO: Once we can actually construct and send accept_channel messages, it diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs index 5822eac7..eac9e2b9 100644 --- a/smite/src/oracles/channel_ready.rs +++ b/smite/src/oracles/channel_ready.rs @@ -1,10 +1,14 @@ //! BOLT 2 `channel_ready` oracle, for the channel funding flow. use super::Oracle; -use crate::bolt::ChannelReady; +use crate::bolt::{ChannelReady, Features}; use crate::channel_state::ChannelState; +use crate::pending_channel::KeyOrigin; use crate::violation::Violation; +use bitcoin::secp256k1::PublicKey; +use std::collections::HashMap; + /// Context for `ChannelReadyOracle` pub struct ChannelReadyContext<'a> { /// The `channel_ready` received from the peer. @@ -12,6 +16,11 @@ pub struct ChannelReadyContext<'a> { /// The channel the `channel_ready` belongs to, identified by its /// `channel_id`, or `None` if no channel was funded for it. pub channel: Option<&'a ChannelState>, + /// Features negotiated between the target node and Smite. + pub negotiated_features: &'a Features, + /// Every pubkey revealed on the wire by either side, mapped to its first + /// origin. + pub revealed_pubkeys: &'a HashMap, } /// Checks whether a received `channel_ready` satisfies the BOLT 2 channel @@ -28,6 +37,41 @@ impl Oracle> for ChannelReadyOracle { )); } + // Check that an alias is set when `option_scid_alias` was negotiated. + if context + .negotiated_features + .supports_feature(Features::OPTION_SCID_ALIAS) + && context.channel_ready.tlvs.short_channel_id.is_none() + { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + "option_scid_alias negotiated but short_channel_id alias is missing".to_string(), + )); + } + + // Check that the `second_per_commitment_point` is not reused from a + // previously revealed key, since its secret will eventually be revealed. + // Reuse is allowed for repeated `channel_ready` messages (e.g., with + // different aliases or after reconnection) if the key origin matches + // the channel and commitment number. + let known_origin = KeyOrigin::TargetPcp { + channel: context.channel_ready.channel_id, + commitment_number: 1, + }; + if let Some(origin) = context + .revealed_pubkeys + .get(&context.channel_ready.second_per_commitment_point) + && *origin != known_origin + { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + format!( + "second_per_commitment_point {} reuses {origin}", + context.channel_ready.second_per_commitment_point, + ), + )); + } + Ok(()) } } @@ -35,7 +79,7 @@ impl Oracle> for ChannelReadyOracle { #[cfg(test)] mod tests { use super::*; - use crate::bolt::{ChannelId, ChannelReadyTlvs, Features}; + use crate::bolt::{ChannelId, ChannelReadyTlvs, ShortChannelId}; use crate::channel_tx::{ChannelConfig, ChannelPartyConfig, HolderIdentity, Side}; use bitcoin::OutPoint; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; @@ -94,20 +138,35 @@ mod tests { } #[track_caller] - fn assert_pass(channel_ready: &ChannelReady, channel: Option<&ChannelState>) { + fn assert_pass( + channel_ready: &ChannelReady, + channel: Option<&ChannelState>, + negotiated_features: &Features, + revealed_pubkeys: &HashMap, + ) { if let Err(err) = ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready, channel, + negotiated_features, + revealed_pubkeys, }) { panic!("expected pass, got: {err}"); } } #[track_caller] - fn assert_fail(channel_ready: &ChannelReady, channel: Option<&ChannelState>, expected: &str) { + fn assert_fail( + channel_ready: &ChannelReady, + channel: Option<&ChannelState>, + negotiated_features: &Features, + revealed_pubkeys: &HashMap, + expected: &str, + ) { match ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready, channel, + negotiated_features, + revealed_pubkeys, }) { Err(Violation::InvalidChannelReady(chan_id, reason)) => { assert_eq!(channel_ready.channel_id, chan_id); @@ -122,7 +181,12 @@ mod tests { #[test] fn conforming_channel_ready_passes() { - assert_pass(&channel_ready(), Some(&channel_state())); + assert_pass( + &channel_ready(), + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + ); } #[test] @@ -130,7 +194,87 @@ mod tests { assert_fail( &channel_ready(), None, + &Features::new(), + &HashMap::new(), "unknown channel_id: no channel was funded for it", ); } + + #[test] + fn conforming_option_scid_alias_with_an_alias_passes() { + let negotiated_features = Features::from_bits(&[Features::OPTION_SCID_ALIAS]); + let mut cr = channel_ready(); + cr.tlvs.short_channel_id = Some(ShortChannelId::new(800_000, 1, 0)); + + assert_pass( + &cr, + Some(&channel_state()), + &negotiated_features, + &HashMap::new(), + ); + } + + #[test] + fn alias_without_option_scid_alias_passes() { + let mut cr = channel_ready(); + cr.tlvs.short_channel_id = Some(ShortChannelId::new(800_000, 1, 0)); + + assert_pass( + &cr, + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + ); + } + + #[test] + fn option_scid_alias_without_an_alias_fails() { + let negotiated_features = Features::from_bits(&[Features::OPTION_SCID_ALIAS]); + + assert_fail( + &channel_ready(), + Some(&channel_state()), + &negotiated_features, + &HashMap::new(), + "option_scid_alias negotiated but short_channel_id alias is missing", + ); + } + + #[test] + fn channel_ready_reuses_per_commitment_point_from_previously_revealed_key_fails() { + let cr = channel_ready(); + let origin = KeyOrigin::TargetPcp { + channel: ChannelId::new([2u8; 32]), + commitment_number: 1, + }; + let revealed_pubkeys = HashMap::from([(cr.second_per_commitment_point, origin)]); + + assert_fail( + &cr, + Some(&channel_state()), + &Features::new(), + &revealed_pubkeys, + &format!( + "second_per_commitment_point {} reuses {origin}", + cr.second_per_commitment_point, + ), + ); + } + + #[test] + fn channel_ready_resends_per_commitment_point_for_same_channel_passes() { + let cr = channel_ready(); + let origin = KeyOrigin::TargetPcp { + channel: cr.channel_id, + commitment_number: 1, + }; + let revealed_pubkeys = HashMap::from([(cr.second_per_commitment_point, origin)]); + + assert_pass( + &cr, + Some(&channel_state()), + &Features::new(), + &revealed_pubkeys, + ); + } } diff --git a/smite/src/violation.rs b/smite/src/violation.rs index 5aeb9385..ad38e1f3 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -50,7 +50,9 @@ pub enum Violation { /// The target's `channel_ready` broke a BOLT 2 requirement, as judged by /// [`crate::oracles::ChannelReadyOracle`]. The reason names the breached /// requirement, one of: - /// - it names a `channel_id` we have not funded. + /// - it names a `channel_id` we have not funded, + /// - it omits the `short_channel_id` alias `option_scid_alias` requires, or + /// - it reuses a per-commitment point from a previously revealed key. #[error("invalid channel_ready for channel_id {0}: {1}")] InvalidChannelReady(ChannelId, String), } From f8670e66ee5da8c72af5df35e5d3bb24b9a1865d Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Tue, 13 Oct 2026 00:41:19 +0530 Subject: [PATCH 3/6] smite: reject channel_ready resends with mismatched point Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 11 +++++-- smite-scenarios/src/executor/tests.rs | 32 +++++++++++++++++++ smite/src/channel_state.rs | 5 +++ smite/src/oracles/channel_ready.rs | 46 +++++++++++++++++++++++++-- smite/src/violation.rs | 3 +- 5 files changed, 92 insertions(+), 5 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 4d43c262..50962c74 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -1446,8 +1446,15 @@ fn record_recv_channel_ready( let state = channel_states .get_mut(&channel_ready.channel_id) .expect("ChannelReadyOracle guaranteed this channel_id exists"); - *state.next_counterparty_per_commitment_point_mut() = - Some(channel_ready.second_per_commitment_point); + + // Only the first `channel_ready` sets these. `ChannelReadyOracle` checks + // resends against its point, and a resend's point is stale once the + // commitment state advances, so it must not overwrite the next point. + if state.received_channel_ready_point.is_none() { + state.received_channel_ready_point = Some(channel_ready.second_per_commitment_point); + *state.next_counterparty_per_commitment_point_mut() = + Some(channel_ready.second_per_commitment_point); + } // Record the `second_per_commitment_point` as a per-commitment point the // target revealed, for commitment number 1. It is unique, so record its diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 2b2d3289..a7608862 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1561,6 +1561,38 @@ fn execute_recv_channel_ready_already_recorded_is_noop() { assert_eq!(fx.queued_len(), 0); } +#[test] +fn execute_recv_channel_ready_resend_rejects_changed_point() { + let (fx, target_pcp) = recv_channel_ready_fixture(); + let changed = sample_pubkey(8); + + // After the target's first `channel_ready` is received, it resends one for + // the same channel with a different `second_per_commitment_point`. + // + // `RecvChannelReady` becomes a no-op after the first expected + // `channel_ready` has been recorded, so we negotiate a second unrelated + // channel whose `RecvAcceptChannel` receives the `channel_ready` implicitly. + let mut b = ProgramBuilder::new(); + let funding_created = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + negotiate_channel(&mut b, &announced_open_channel()); + + let err = fx + .queue(&channel_ready_reply(changed)) + .queue(&Message::AcceptChannel(sample_accept_channel())) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, funding_channel_id()); + assert!(reason.contains(&format!( + "channel_ready resend changed second_per_commitment_point from {target_pcp} to {changed}" + ))); +} + #[test] fn execute_recv_implicit_channel_ready_unknown_channel() { // No channel is tracked, so a `channel_ready` received while waiting for diff --git a/smite/src/channel_state.rs b/smite/src/channel_state.rs index 3d5d5cbc..90253e9f 100644 --- a/smite/src/channel_state.rs +++ b/smite/src/channel_state.rs @@ -28,6 +28,10 @@ pub struct ChannelState { /// revealed by `channel_ready` and then each `revoke_and_ack`. `None` until /// known. pub acceptor_next_per_commitment_point: Option, + /// The `second_per_commitment_point` from the first received `channel_ready`, + /// recorded so a resend can be checked to carry the same point. `None` + /// until a `channel_ready` is received. + pub received_channel_ready_point: Option, /// Whether the on-chain output at the advertised funding outpoint matches /// the negotiated funding script and amount. pub is_funding_outpoint_valid: bool, @@ -64,6 +68,7 @@ impl ChannelState { commitments, opener_next_per_commitment_point: None, acceptor_next_per_commitment_point: None, + received_channel_ready_point: None, is_funding_outpoint_valid, was_funding_mined_prematurely, sent_invalid_signature, diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs index eac9e2b9..e5198f37 100644 --- a/smite/src/oracles/channel_ready.rs +++ b/smite/src/oracles/channel_ready.rs @@ -30,12 +30,12 @@ pub struct ChannelReadyOracle; impl Oracle> for ChannelReadyOracle { fn evaluate(&self, context: &ChannelReadyContext<'_>) -> Result<(), Violation> { // Check that the `channel_ready` answers a channel we funded. - if context.channel.is_none() { + let Some(channel) = context.channel else { return Err(Violation::InvalidChannelReady( context.channel_ready.channel_id, "unknown channel_id: no channel was funded for it".to_string(), )); - } + }; // Check that an alias is set when `option_scid_alias` was negotiated. if context @@ -49,6 +49,20 @@ impl Oracle> for ChannelReadyOracle { )); } + // Check that a resent `channel_ready` repeats the first one's + // `second_per_commitment_point`. + if let Some(first) = channel.received_channel_ready_point + && first != context.channel_ready.second_per_commitment_point + { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + format!( + "channel_ready resend changed second_per_commitment_point from {first} to {}", + context.channel_ready.second_per_commitment_point, + ), + )); + } + // Check that the `second_per_commitment_point` is not reused from a // previously revealed key, since its secret will eventually be revealed. // Reuse is allowed for repeated `channel_ready` messages (e.g., with @@ -240,6 +254,34 @@ mod tests { ); } + #[test] + fn channel_ready_resend_different_per_commitment_point_fails() { + let mut channel = channel_state(); + channel.received_channel_ready_point = Some(pubkey(3)); + let cr = channel_ready(); + + assert_fail( + &cr, + Some(&channel), + &Features::new(), + &HashMap::new(), + &format!( + "channel_ready resend changed second_per_commitment_point from {} to {}", + pubkey(3), + cr.second_per_commitment_point, + ), + ); + } + + #[test] + fn channel_ready_resend_repeats_per_commitment_point_passes() { + let cr = channel_ready(); + let mut channel = channel_state(); + channel.received_channel_ready_point = Some(cr.second_per_commitment_point); + + assert_pass(&cr, Some(&channel), &Features::new(), &HashMap::new()); + } + #[test] fn channel_ready_reuses_per_commitment_point_from_previously_revealed_key_fails() { let cr = channel_ready(); diff --git a/smite/src/violation.rs b/smite/src/violation.rs index ad38e1f3..ab6bb2b9 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -51,7 +51,8 @@ pub enum Violation { /// [`crate::oracles::ChannelReadyOracle`]. The reason names the breached /// requirement, one of: /// - it names a `channel_id` we have not funded, - /// - it omits the `short_channel_id` alias `option_scid_alias` requires, or + /// - it omits the `short_channel_id` alias `option_scid_alias` requires, + /// - it resends with a different `second_per_commitment_point`, or /// - it reuses a per-commitment point from a previously revealed key. #[error("invalid channel_ready for channel_id {0}: {1}")] InvalidChannelReady(ChannelId, String), From 77e43746e1726a1eff1f8eb4ef591b0f1bfeeea0 Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Tue, 13 Oct 2026 13:51:42 +0530 Subject: [PATCH 4/6] smite: reject channel_ready before funding minimum_depth Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 23 ++++++++++- smite-scenarios/src/executor/tests.rs | 30 ++++++++++++++ smite/src/oracles/channel_ready.rs | 57 ++++++++++++++++++++++++++- smite/src/violation.rs | 1 + 4 files changed, 108 insertions(+), 3 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 50962c74..4da41ad0 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -528,6 +528,7 @@ impl Executor { &mut self.channel_states, &mut self.revealed_pubkeys, &self.context, + &mut self.bitcoin_cli, RECV_IDLE_TIMEOUT, )?; log::debug!("[{:?}] RecvAcceptChannel: received", start.elapsed()); @@ -557,6 +558,7 @@ impl Executor { &mut self.channel_states, &mut self.revealed_pubkeys, &self.context, + &mut self.bitcoin_cli, RECV_IDLE_TIMEOUT, )?; log::debug!("[{:?}] RecvFundingSigned: received", start.elapsed()); @@ -578,6 +580,7 @@ impl Executor { &mut self.channel_states, &mut self.revealed_pubkeys, &self.context, + &mut self.bitcoin_cli, RECV_CHANNEL_READY_TIMEOUT, )?; log::debug!("[{:?}] RecvChannelReady: received", start.elapsed()); @@ -1211,6 +1214,7 @@ fn recv_non_ping( channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, context: &ProgramContext, + bitcoin_cli: &mut impl BitcoinRpc, timeout: Duration, ) -> Result { let previous = conn.read_timeout()?; @@ -1226,11 +1230,18 @@ fn recv_non_ping( } Message::ChannelReady(ref cr) => { log::debug!("received channel_ready on {}", cr.channel_id); + let channel = channel_states.get(&cr.channel_id); + let funding_outpoint = channel + .filter(|c| c.is_funding_outpoint_valid) + .map(|c| c.config.funding_outpoint); + let funding_confirmations = funding_outpoint + .map_or(0, |o| bitcoin_cli.get_transaction_confirmations(o.txid)); ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready: cr, - channel: channel_states.get(&cr.channel_id), + channel, negotiated_features: &context.negotiated_features, revealed_pubkeys, + funding_confirmations, })?; record_recv_channel_ready(channel_states, revealed_pubkeys, cr); return Ok(msg); @@ -1294,10 +1305,18 @@ fn recv_bolt( channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, context: &ProgramContext, + bitcoin_cli: &mut impl BitcoinRpc, timeout: Duration, ) -> Result { loop { - let msg = recv_non_ping(conn, channel_states, revealed_pubkeys, context, timeout)?; + let msg = recv_non_ping( + conn, + channel_states, + revealed_pubkeys, + context, + bitcoin_cli, + timeout, + )?; let got = msg.msg_type(); if let Some(m) = M::from_message(msg) { return Ok(m); diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index a7608862..4a257b59 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1593,6 +1593,36 @@ fn execute_recv_channel_ready_resend_rejects_changed_point() { ))); } +#[test] +fn execute_recv_channel_ready_rejects_below_minimum_depth() { + let (fx, _) = recv_channel_ready_fixture(); + + // Mine one block fewer than the `minimum_depth` negotiated in + // `accept_channel`. The target's `channel_ready` arrives early, which means + // `RecvChannelReady` acts as a no-op and won't wait for it. Instead we + // negotiate a second unrelated channel whose `RecvAcceptChannel` receives + // the `channel_ready` implicitly. + let mut b = ProgramBuilder::new(); + let funding_created = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + b.append(Operation::MineBlocks(5), &[]); + negotiate_channel(&mut b, &announced_open_channel()); + + let err = fx + .queue(&Message::AcceptChannel(sample_accept_channel())) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, funding_channel_id()); + assert!( + reason.contains( + "channel_ready received with 5 funding confirmation(s), below minimum_depth 6" + ) + ); +} + #[test] fn execute_recv_implicit_channel_ready_unknown_channel() { // No channel is tracked, so a `channel_ready` received while waiting for diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs index e5198f37..72654ab5 100644 --- a/smite/src/oracles/channel_ready.rs +++ b/smite/src/oracles/channel_ready.rs @@ -21,6 +21,9 @@ pub struct ChannelReadyContext<'a> { /// Every pubkey revealed on the wire by either side, mapped to its first /// origin. pub revealed_pubkeys: &'a HashMap, + /// Number of confirmations the funding transaction had when the + /// `channel_ready` was received, or `0` if it is unconfirmed or unknown. + pub funding_confirmations: u32, } /// Checks whether a received `channel_ready` satisfies the BOLT 2 channel @@ -37,6 +40,17 @@ impl Oracle> for ChannelReadyOracle { )); }; + // Check that the funding transaction reached `minimum_depth`. + if context.funding_confirmations < channel.config.minimum_depth { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + format!( + "channel_ready received with {} funding confirmation(s), below minimum_depth {}", + context.funding_confirmations, channel.config.minimum_depth, + ), + )); + } + // Check that an alias is set when `option_scid_alias` was negotiated. if context .negotiated_features @@ -157,12 +171,14 @@ mod tests { channel: Option<&ChannelState>, negotiated_features: &Features, revealed_pubkeys: &HashMap, + funding_confirmations: u32, ) { if let Err(err) = ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready, channel, negotiated_features, revealed_pubkeys, + funding_confirmations, }) { panic!("expected pass, got: {err}"); } @@ -174,6 +190,7 @@ mod tests { channel: Option<&ChannelState>, negotiated_features: &Features, revealed_pubkeys: &HashMap, + funding_confirmations: u32, expected: &str, ) { match ChannelReadyOracle.evaluate(&ChannelReadyContext { @@ -181,6 +198,7 @@ mod tests { channel, negotiated_features, revealed_pubkeys, + funding_confirmations, }) { Err(Violation::InvalidChannelReady(chan_id, reason)) => { assert_eq!(channel_ready.channel_id, chan_id); @@ -200,6 +218,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + CONFIRMED_DEPTH, ); } @@ -210,10 +229,34 @@ mod tests { None, &Features::new(), &HashMap::new(), + CONFIRMED_DEPTH, "unknown channel_id: no channel was funded for it", ); } + #[test] + fn channel_ready_below_minimum_depth_fails() { + assert_fail( + &channel_ready(), + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + CONFIRMED_DEPTH - 1, + "channel_ready received with 5 funding confirmation(s), below minimum_depth 6", + ); + } + + #[test] + fn channel_ready_exactly_at_minimum_depth_passes() { + assert_pass( + &channel_ready(), + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + CONFIRMED_DEPTH, + ); + } + #[test] fn conforming_option_scid_alias_with_an_alias_passes() { let negotiated_features = Features::from_bits(&[Features::OPTION_SCID_ALIAS]); @@ -225,6 +268,7 @@ mod tests { Some(&channel_state()), &negotiated_features, &HashMap::new(), + CONFIRMED_DEPTH, ); } @@ -238,6 +282,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + CONFIRMED_DEPTH, ); } @@ -250,6 +295,7 @@ mod tests { Some(&channel_state()), &negotiated_features, &HashMap::new(), + CONFIRMED_DEPTH, "option_scid_alias negotiated but short_channel_id alias is missing", ); } @@ -265,6 +311,7 @@ mod tests { Some(&channel), &Features::new(), &HashMap::new(), + CONFIRMED_DEPTH, &format!( "channel_ready resend changed second_per_commitment_point from {} to {}", pubkey(3), @@ -279,7 +326,13 @@ mod tests { let mut channel = channel_state(); channel.received_channel_ready_point = Some(cr.second_per_commitment_point); - assert_pass(&cr, Some(&channel), &Features::new(), &HashMap::new()); + assert_pass( + &cr, + Some(&channel), + &Features::new(), + &HashMap::new(), + CONFIRMED_DEPTH, + ); } #[test] @@ -296,6 +349,7 @@ mod tests { Some(&channel_state()), &Features::new(), &revealed_pubkeys, + CONFIRMED_DEPTH, &format!( "second_per_commitment_point {} reuses {origin}", cr.second_per_commitment_point, @@ -317,6 +371,7 @@ mod tests { Some(&channel_state()), &Features::new(), &revealed_pubkeys, + CONFIRMED_DEPTH, ); } } diff --git a/smite/src/violation.rs b/smite/src/violation.rs index ab6bb2b9..b8b780fa 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -51,6 +51,7 @@ pub enum Violation { /// [`crate::oracles::ChannelReadyOracle`]. The reason names the breached /// requirement, one of: /// - it names a `channel_id` we have not funded, + /// - it arrives before the funding transaction reached `minimum_depth`, /// - it omits the `short_channel_id` alias `option_scid_alias` requires, /// - it resends with a different `second_per_commitment_point`, or /// - it reuses a per-commitment point from a previously revealed key. From 186aac760ab4ce40b20995810779869fc782ca3b Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Tue, 13 Oct 2026 13:53:20 +0530 Subject: [PATCH 5/6] smite: reject channel_ready with invalid funding outpoint Signed-off-by: Nishant Bansal --- smite/src/oracles/channel_ready.rs | 25 +++++++++++++++++++++++++ smite/src/violation.rs | 2 ++ 2 files changed, 27 insertions(+) diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs index 72654ab5..c2dbca8d 100644 --- a/smite/src/oracles/channel_ready.rs +++ b/smite/src/oracles/channel_ready.rs @@ -40,6 +40,16 @@ impl Oracle> for ChannelReadyOracle { )); }; + // Check that the on-chain funding output matches the negotiated funding + // script and amount. + if !channel.is_funding_outpoint_valid { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + "funding output does not match the negotiated funding script and amount" + .to_string(), + )); + } + // Check that the funding transaction reached `minimum_depth`. if context.funding_confirmations < channel.config.minimum_depth { return Err(Violation::InvalidChannelReady( @@ -234,6 +244,21 @@ mod tests { ); } + #[test] + fn channel_ready_with_invalid_funding_outpoint_fails() { + let mut channel = channel_state(); + channel.is_funding_outpoint_valid = false; + + assert_fail( + &channel_ready(), + Some(&channel), + &Features::new(), + &HashMap::new(), + CONFIRMED_DEPTH, + "funding output does not match the negotiated funding script and amount", + ); + } + #[test] fn channel_ready_below_minimum_depth_fails() { assert_fail( diff --git a/smite/src/violation.rs b/smite/src/violation.rs index b8b780fa..734676ee 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -51,6 +51,8 @@ pub enum Violation { /// [`crate::oracles::ChannelReadyOracle`]. The reason names the breached /// requirement, one of: /// - it names a `channel_id` we have not funded, + /// - its funding output does not match the negotiated funding script and + /// amount, /// - it arrives before the funding transaction reached `minimum_depth`, /// - it omits the `short_channel_id` alias `option_scid_alias` requires, /// - it resends with a different `second_per_commitment_point`, or From 42e3871ef635bdc5b443eb4794dcfab9a832987b Mon Sep 17 00:00:00 2001 From: Nishant Bansal Date: Tue, 13 Oct 2026 12:12:41 +0530 Subject: [PATCH 6/6] smite: reject channel_ready short_channel_id alias reuse Signed-off-by: Nishant Bansal --- smite-scenarios/src/executor.rs | 64 +++++++++--- smite-scenarios/src/executor/tests.rs | 137 ++++++++++++++++++++++++++ smite/src/oracles/channel_ready.rs | 76 +++++++++++++- smite/src/violation.rs | 1 + 4 files changed, 264 insertions(+), 14 deletions(-) diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 4da41ad0..e4fef171 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -259,6 +259,10 @@ pub struct Executor { /// We may send the same pubkey more than once, keeping its first origin. /// The target must not send a pubkey whose private key is or will be known. revealed_pubkeys: HashMap, + /// Every `short_channel_id` revealed on the wire, both advertised aliases + /// and derived real ones, mapped to the channel that owns it. The target + /// must not reuse one across channels. + revealed_scids: HashMap, /// Transactions stored outside Bitcoin Core's mempool, typically because they /// were rejected by mempool policy, to be included in the next `MineBlocks` /// operation. Each is stored as `(txid, raw_hex)`: re-signing the same @@ -294,6 +298,7 @@ impl Executor { channel_states: HashMap::new(), negotiations: HashMap::new(), revealed_pubkeys, + revealed_scids: HashMap::new(), private_mempool: Vec::new(), unmined_txids: HashSet::new(), mined_txids: HashSet::new(), @@ -527,6 +532,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &mut self.revealed_scids, &self.context, &mut self.bitcoin_cli, RECV_IDLE_TIMEOUT, @@ -557,6 +563,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &mut self.revealed_scids, &self.context, &mut self.bitcoin_cli, RECV_IDLE_TIMEOUT, @@ -579,6 +586,7 @@ impl Executor { &mut self.conn, &mut self.channel_states, &mut self.revealed_pubkeys, + &mut self.revealed_scids, &self.context, &mut self.bitcoin_cli, RECV_CHANNEL_READY_TIMEOUT, @@ -632,18 +640,11 @@ impl Executor { // message will simply fail on-chain validation, which is // the intended fuzzing behaviour for a valid but // unconfirmed program. - let scid = match self.bitcoin_cli.get_transaction_block_position(txid) { - Some(pos) => { - let funding_output_index = - u16::try_from(ft.vout).expect("funding output index fits in u16"); - ShortChannelId::new( - pos.block_height, - pos.tx_index, - funding_output_index, - ) - } - None => ShortChannelId::new(0, 0, 0), - }; + let scid = lookup_short_channel_id( + &mut self.bitcoin_cli, + OutPoint::new(txid, ft.vout), + ) + .unwrap_or_default(); log::debug!( "[{:?}] LookupShortChannelId: txid={} scid={}", start.elapsed(), @@ -1015,6 +1016,17 @@ fn build_shutdown(variables: &[Option], inputs: &[usize]) -> Shutdown Shutdown::for_channel(channel_id, scriptpubkey) } +/// Returns the short channel ID of the funding output `outpoint`, or `None` if +/// its transaction is unknown to the node or not yet confirmed. +fn lookup_short_channel_id( + bitcoin_cli: &mut impl BitcoinRpc, + outpoint: OutPoint, +) -> Option { + let pos = bitcoin_cli.get_transaction_block_position(outpoint.txid)?; + let vout = u16::try_from(outpoint.vout).expect("funding output index fits in u16"); + Some(ShortChannelId::new(pos.block_height, pos.tx_index, vout)) +} + /// Builds a signed `ChannelAnnouncement` from 7 input variables. fn build_channel_announcement( variables: &[Option], @@ -1213,6 +1225,7 @@ fn recv_non_ping( conn: &mut impl Connection, channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, + revealed_scids: &mut HashMap, context: &ProgramContext, bitcoin_cli: &mut impl BitcoinRpc, timeout: Duration, @@ -1231,19 +1244,30 @@ fn recv_non_ping( Message::ChannelReady(ref cr) => { log::debug!("received channel_ready on {}", cr.channel_id); let channel = channel_states.get(&cr.channel_id); + // TODO: Both lookups below call `getrawtransaction`. Returning + // confirmations alongside the block position would save one RPC. let funding_outpoint = channel .filter(|c| c.is_funding_outpoint_valid) .map(|c| c.config.funding_outpoint); let funding_confirmations = funding_outpoint .map_or(0, |o| bitcoin_cli.get_transaction_confirmations(o.txid)); + let funding_scid = + funding_outpoint.and_then(|o| lookup_short_channel_id(bitcoin_cli, o)); ChannelReadyOracle.evaluate(&ChannelReadyContext { channel_ready: cr, channel, negotiated_features: &context.negotiated_features, revealed_pubkeys, funding_confirmations, + revealed_scids, })?; - record_recv_channel_ready(channel_states, revealed_pubkeys, cr); + record_recv_channel_ready( + channel_states, + revealed_pubkeys, + revealed_scids, + funding_scid, + cr, + ); return Ok(msg); } Message::Unknown { .. } => { @@ -1304,6 +1328,7 @@ fn recv_bolt( conn: &mut impl Connection, channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, + revealed_scids: &mut HashMap, context: &ProgramContext, bitcoin_cli: &mut impl BitcoinRpc, timeout: Duration, @@ -1313,6 +1338,7 @@ fn recv_bolt( conn, channel_states, revealed_pubkeys, + revealed_scids, context, bitcoin_cli, timeout, @@ -1460,12 +1486,24 @@ fn record_recv_funding_signed( fn record_recv_channel_ready( channel_states: &mut HashMap, revealed_pubkeys: &mut HashMap, + revealed_scids: &mut HashMap, + funding_scid: Option, channel_ready: &ChannelReady, ) { let state = channel_states .get_mut(&channel_ready.channel_id) .expect("ChannelReadyOracle guaranteed this channel_id exists"); + // Record the advertised alias and the channel's real `short_channel_id` as + // owned by this channel, so `ChannelReadyOracle` can reject a later reuse + // by another channel. + if let Some(alias) = channel_ready.tlvs.short_channel_id { + revealed_scids.insert(alias, channel_ready.channel_id); + } + if let Some(real_scid) = funding_scid { + revealed_scids.insert(real_scid, channel_ready.channel_id); + } + // Only the first `channel_ready` sets these. `ChannelReadyOracle` checks // resends against its point, and a resend's point is stale once the // commitment state advances, so it must not overwrite the next point. diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 4a257b59..84a9bd87 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1532,6 +1532,143 @@ fn execute_recv_channel_ready_resend_with_new_alias() { assert_eq!(fx.channel_states().len(), 2); } +#[test] +fn execute_recv_channel_ready_rejects_alias_reused_by_another_channel() { + // A second negotiation with a different temporary channel ID and funding + // amount, funded by a second UTXO, resulting in a different channel ID. + let second_id = TemporaryChannelId::new([0xcc; 32]); + let mut second_utxo = sample_utxo(); + second_utxo.amount = Amount::from_sat(20_010_000); + second_utxo.outpoint.vout = 1; + let mut second_negotiation = sample_funding_negotiation(); + second_negotiation.open_channel.temporary_channel_id = second_id; + second_negotiation.open_channel.funding_satoshis = 20_000_000; + second_negotiation + .accept_channel + .as_mut() + .unwrap() + .temporary_channel_id = second_id; + let second_channel_id = ChannelId::v1_from_funding_outpoint(OutPoint { + txid: "cb6f11974ce6265f81037dc7c0e3f054462f645361bc1e10c208f132f4acbe8d" + .parse() + .expect("valid txid"), + vout: 0, + }); + + // Fund both channels and receive a `channel_ready` for each once the + // funding transactions reach `minimum_depth`. The second channel's + // `channel_ready` advertises the alias the first channel already + // advertised. + let mut b = ProgramBuilder::new(); + send_funding_created(&mut b); + let tx = create_funding_tx_with(&mut b, 20_000_000, 15_000); + let second_temp_chan_id = b.append(Operation::LoadChannelId(second_id.0), &[]); + b.append( + Operation::SendFundingCreated, + &[tx.tx, tx.opener_privkey, second_temp_chan_id], + ); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + b.append(Operation::RecvChannelReady, &[]); + + let alias = ShortChannelId::from_u64(42); + let err = Fixture::new() + .with_utxos(vec![sample_utxo(), second_utxo]) + .with_negotiation(sample_funding_negotiation()) + .with_negotiation(second_negotiation) + .queue(&Message::ChannelReady(ChannelReady { + channel_id: funding_channel_id(), + second_per_commitment_point: sample_pubkey(7), + tlvs: ChannelReadyTlvs { + short_channel_id: Some(alias), + }, + })) + .queue(&Message::ChannelReady(ChannelReady { + channel_id: second_channel_id, + second_per_commitment_point: sample_pubkey(8), + tlvs: ChannelReadyTlvs { + short_channel_id: Some(alias), + }, + })) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, second_channel_id); + assert!(reason.contains(&format!( + "short_channel_id alias {alias} reuses the short_channel_id of channel {}", + funding_channel_id(), + ))); +} + +#[test] +fn execute_recv_channel_ready_rejects_alias_matching_another_channels_scid() { + // A second negotiation with a different temporary channel ID and funding + // amount, funded by a second UTXO, resulting in a different channel ID. + let second_id = TemporaryChannelId::new([0xcc; 32]); + let mut second_utxo = sample_utxo(); + second_utxo.amount = Amount::from_sat(20_010_000); + second_utxo.outpoint.vout = 1; + let mut second_negotiation = sample_funding_negotiation(); + second_negotiation.open_channel.temporary_channel_id = second_id; + second_negotiation.open_channel.funding_satoshis = 20_000_000; + second_negotiation + .accept_channel + .as_mut() + .unwrap() + .temporary_channel_id = second_id; + let second_channel_id = ChannelId::v1_from_funding_outpoint(OutPoint { + txid: "cb6f11974ce6265f81037dc7c0e3f054462f645361bc1e10c208f132f4acbe8d" + .parse() + .expect("valid txid"), + vout: 0, + }); + + // Fund both channels and receive a `channel_ready` for each once the + // funding transactions reach `minimum_depth`. The second channel advertises + // an alias matching the first channel's real short channel ID. + let mut b = ProgramBuilder::new(); + send_funding_created(&mut b); + let tx = create_funding_tx_with(&mut b, 20_000_000, 15_000); + let second_temp_chan_id = b.append(Operation::LoadChannelId(second_id.0), &[]); + b.append( + Operation::SendFundingCreated, + &[tx.tx, tx.opener_privkey, second_temp_chan_id], + ); + b.append(Operation::MineBlocks(6), &[]); + b.append(Operation::RecvChannelReady, &[]); + b.append(Operation::RecvChannelReady, &[]); + + let alias = ShortChannelId::new(800_042, 7, 0); + let err = Fixture::new() + .with_utxos(vec![sample_utxo(), second_utxo]) + .with_negotiation(sample_funding_negotiation()) + .with_negotiation(second_negotiation) + .queue(&Message::ChannelReady(ChannelReady { + channel_id: funding_channel_id(), + second_per_commitment_point: sample_pubkey(7), + tlvs: ChannelReadyTlvs::default(), + })) + .queue(&Message::ChannelReady(ChannelReady { + channel_id: second_channel_id, + second_per_commitment_point: sample_pubkey(8), + tlvs: ChannelReadyTlvs { + short_channel_id: Some(alias), + }, + })) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidChannelReady(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, second_channel_id); + assert!(reason.contains(&format!( + "short_channel_id alias {alias} reuses the short_channel_id of channel {}", + funding_channel_id(), + ))); +} + #[test] fn execute_recv_channel_ready_already_recorded_is_noop() { let (fx, target_pcp) = recv_channel_ready_fixture(); diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs index c2dbca8d..45ba6f8a 100644 --- a/smite/src/oracles/channel_ready.rs +++ b/smite/src/oracles/channel_ready.rs @@ -1,7 +1,7 @@ //! BOLT 2 `channel_ready` oracle, for the channel funding flow. use super::Oracle; -use crate::bolt::{ChannelReady, Features}; +use crate::bolt::{ChannelId, ChannelReady, Features, ShortChannelId}; use crate::channel_state::ChannelState; use crate::pending_channel::KeyOrigin; use crate::violation::Violation; @@ -21,6 +21,9 @@ pub struct ChannelReadyContext<'a> { /// Every pubkey revealed on the wire by either side, mapped to its first /// origin. pub revealed_pubkeys: &'a HashMap, + /// Every `short_channel_id` revealed on the wire, both advertised aliases + /// and derived real ones, mapped to the channel that owns it. + pub revealed_scids: &'a HashMap, /// Number of confirmations the funding transaction had when the /// `channel_ready` was received, or `0` if it is unconfirmed or unknown. pub funding_confirmations: u32, @@ -73,6 +76,22 @@ impl Oracle> for ChannelReadyOracle { )); } + // Check that an advertised alias does not reuse a `short_channel_id` + // already revealed by another channel (its alias or real + // `short_channel_id`). Reuse is allowed only for a resend of this + // channel's own alias. + if let Some(alias) = context.channel_ready.tlvs.short_channel_id + && let Some(origin) = context.revealed_scids.get(&alias) + && *origin != context.channel_ready.channel_id + { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + format!( + "short_channel_id alias {alias} reuses the short_channel_id of channel {origin}" + ), + )); + } + // Check that a resent `channel_ready` repeats the first one's // `second_per_commitment_point`. if let Some(first) = channel.received_channel_ready_point @@ -181,6 +200,7 @@ mod tests { channel: Option<&ChannelState>, negotiated_features: &Features, revealed_pubkeys: &HashMap, + revealed_scids: &HashMap, funding_confirmations: u32, ) { if let Err(err) = ChannelReadyOracle.evaluate(&ChannelReadyContext { @@ -188,6 +208,7 @@ mod tests { channel, negotiated_features, revealed_pubkeys, + revealed_scids, funding_confirmations, }) { panic!("expected pass, got: {err}"); @@ -200,6 +221,7 @@ mod tests { channel: Option<&ChannelState>, negotiated_features: &Features, revealed_pubkeys: &HashMap, + revealed_scids: &HashMap, funding_confirmations: u32, expected: &str, ) { @@ -208,6 +230,7 @@ mod tests { channel, negotiated_features, revealed_pubkeys, + revealed_scids, funding_confirmations, }) { Err(Violation::InvalidChannelReady(chan_id, reason)) => { @@ -228,6 +251,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, ); } @@ -239,6 +263,7 @@ mod tests { None, &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, "unknown channel_id: no channel was funded for it", ); @@ -254,6 +279,7 @@ mod tests { Some(&channel), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, "funding output does not match the negotiated funding script and amount", ); @@ -266,6 +292,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH - 1, "channel_ready received with 5 funding confirmation(s), below minimum_depth 6", ); @@ -278,6 +305,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, ); } @@ -293,6 +321,7 @@ mod tests { Some(&channel_state()), &negotiated_features, &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, ); } @@ -307,6 +336,7 @@ mod tests { Some(&channel_state()), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, ); } @@ -320,11 +350,51 @@ mod tests { Some(&channel_state()), &negotiated_features, &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, "option_scid_alias negotiated but short_channel_id alias is missing", ); } + #[test] + fn channel_ready_alias_reuses_another_channels_scid_fails() { + let alias = ShortChannelId::new(800_000, 1, 0); + let origin = ChannelId::new([2u8; 32]); + let mut cr = channel_ready(); + cr.tlvs.short_channel_id = Some(alias); + let revealed_scids = HashMap::from([(alias, origin)]); + + assert_fail( + &cr, + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + &revealed_scids, + CONFIRMED_DEPTH, + &format!( + "short_channel_id alias {alias} reuses the short_channel_id of channel {origin}" + ), + ); + } + + #[test] + fn channel_ready_alias_resend_for_same_channel_passes() { + let alias = ShortChannelId::new(800_000, 1, 0); + let mut cr = channel_ready(); + let origin = cr.channel_id; + cr.tlvs.short_channel_id = Some(alias); + let revealed_scids = HashMap::from([(alias, origin)]); + + assert_pass( + &cr, + Some(&channel_state()), + &Features::new(), + &HashMap::new(), + &revealed_scids, + CONFIRMED_DEPTH, + ); + } + #[test] fn channel_ready_resend_different_per_commitment_point_fails() { let mut channel = channel_state(); @@ -336,6 +406,7 @@ mod tests { Some(&channel), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, &format!( "channel_ready resend changed second_per_commitment_point from {} to {}", @@ -356,6 +427,7 @@ mod tests { Some(&channel), &Features::new(), &HashMap::new(), + &HashMap::new(), CONFIRMED_DEPTH, ); } @@ -374,6 +446,7 @@ mod tests { Some(&channel_state()), &Features::new(), &revealed_pubkeys, + &HashMap::new(), CONFIRMED_DEPTH, &format!( "second_per_commitment_point {} reuses {origin}", @@ -396,6 +469,7 @@ mod tests { Some(&channel_state()), &Features::new(), &revealed_pubkeys, + &HashMap::new(), CONFIRMED_DEPTH, ); } diff --git a/smite/src/violation.rs b/smite/src/violation.rs index 734676ee..5e45eb03 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -55,6 +55,7 @@ pub enum Violation { /// amount, /// - it arrives before the funding transaction reached `minimum_depth`, /// - it omits the `short_channel_id` alias `option_scid_alias` requires, + /// - its alias reuses a `short_channel_id` revealed by another channel, /// - it resends with a different `second_per_commitment_point`, or /// - it reuses a per-commitment point from a previously revealed key. #[error("invalid channel_ready for channel_id {0}: {1}")]