From 8d65b49d2fcbbd542aa1d51d93e9c9c82903e821 Mon Sep 17 00:00:00 2001 From: ekzyis Date: Tue, 22 Sep 2026 15:46:18 +0200 Subject: [PATCH 1/2] smite-ir: add RecvShutdown operation RecvShutdown consumes the SentShutdown of a previous SendShutdown, which now carries the `shutdown` we sent, and waits for the target's `shutdown` in reply. It returns the target's scriptpubkey, or empty bytes if no `shutdown` was received. If our own scriptpubkey isn't standard, BOLT 2 says the target should send a warning instead of replying, so we accept a warning for the channel as a reply. A `shutdown` for another channel may answer one we sent there earlier, which we can't check against the `shutdown` we sent here, so it ends the program as an unexpected message. RecvShutdown is a no-op if we don't track the channel or the target already replied. Before the target sent `channel_ready`, it may choose not to reply, but we still expect one: LDK always replies, and a target that doesn't only costs us a receive timeout, which isn't reported as a violation. --- smite-ir/src/mutators/operation_param.rs | 1 + smite-ir/src/operation.rs | 21 +- smite-ir/src/tests.rs | 7 +- smite-ir/src/variable.rs | 6 +- smite-scenarios/src/executor.rs | 106 +++++++- smite-scenarios/src/executor/tests.rs | 244 +++++++++++++++++- .../src/executor/tests/programs.rs | 56 +++- smite/src/channel_tx/commitment.rs | 5 + 8 files changed, 429 insertions(+), 17 deletions(-) diff --git a/smite-ir/src/mutators/operation_param.rs b/smite-ir/src/mutators/operation_param.rs index 745aa415..89f04832 100644 --- a/smite-ir/src/mutators/operation_param.rs +++ b/smite-ir/src/mutators/operation_param.rs @@ -122,6 +122,7 @@ fn mutate_operation(op: &mut Operation, rng: &mut impl Rng) -> bool { | Operation::RecvAcceptChannel | Operation::RecvFundingSigned | Operation::RecvChannelReady + | Operation::RecvShutdown | Operation::BroadcastTransaction | Operation::LookupShortChannelId => { unreachable!("is_param_mutable returned true for {op:?}") diff --git a/smite-ir/src/operation.rs b/smite-ir/src/operation.rs index 22a17899..fd40d4e9 100644 --- a/smite-ir/src/operation.rs +++ b/smite-ir/src/operation.rs @@ -228,6 +228,17 @@ pub enum Operation { /// point unknown) and its funding transaction has enough confirmations for /// the target to have sent `channel_ready`. RecvChannelReady, + /// Receive and parse the target's `shutdown` in reply to ours. + /// Produces the target's `scriptpubkey` (`Bytes`), or empty `Bytes` if no + /// `shutdown` was received. + /// + /// This is a no-op unless the channel is tracked and the target has not + /// replied yet. A `warning` the target may send instead of replying is + /// accepted. + /// + /// Inputs (1): + /// 0: `SentShutdown` from the `SendShutdown` being answered + RecvShutdown, /// Mines the given number of blocks on the Bitcoin network. MineBlocks(u8), /// Sign wallet inputs of the transaction and broadcast it via `bitcoin-cli`. @@ -556,6 +567,7 @@ impl fmt::Display for Operation { Self::RecvAcceptChannel => write!(f, "RecvAcceptChannel"), Self::RecvFundingSigned => write!(f, "RecvFundingSigned"), Self::RecvChannelReady => write!(f, "RecvChannelReady()"), + Self::RecvShutdown => write!(f, "RecvShutdown"), Self::MineBlocks(v) => write!(f, "MineBlocks({v})"), Self::BroadcastTransaction => write!(f, "BroadcastTransaction"), Self::LookupShortChannelId => write!(f, "LookupShortChannelId"), @@ -579,7 +591,9 @@ impl Operation { Self::LoadForwardingFee(_) => Some(VariableType::ForwardingFee), Self::LoadU16(_) => Some(VariableType::U16), Self::LoadU8(_) => Some(VariableType::U8), - Self::LoadBytes(_) | Self::LoadShutdownScript(_) => Some(VariableType::Bytes), + Self::LoadBytes(_) | Self::LoadShutdownScript(_) | Self::RecvShutdown => { + Some(VariableType::Bytes) + } Self::LoadFeatures(_) | Self::LoadChannelType(_) => Some(VariableType::Features), Self::LoadPrivateKey(_) => Some(VariableType::PrivateKey), Self::LoadChannelId(_) | Self::RecvFundingSigned => Some(VariableType::ChannelId), @@ -718,6 +732,7 @@ impl Operation { ], Self::RecvAcceptChannel => vec![VariableType::SentOpenChannel], Self::RecvFundingSigned => vec![VariableType::SentFundingCreated], + Self::RecvShutdown => vec![VariableType::SentShutdown], Self::BroadcastTransaction | Self::LookupShortChannelId => { vec![VariableType::FundingTransaction] } @@ -763,6 +778,7 @@ impl Operation { | Self::SendShutdown | Self::RecvFundingSigned | Self::RecvChannelReady + | Self::RecvShutdown | Self::MineBlocks(_) | Self::BroadcastTransaction | Self::LookupShortChannelId => vec![], @@ -812,6 +828,7 @@ impl Operation { | Self::RecvAcceptChannel | Self::RecvFundingSigned | Self::RecvChannelReady + | Self::RecvShutdown | Self::MineBlocks(_) | Self::BroadcastTransaction => true, } @@ -866,6 +883,7 @@ impl Operation { | Self::RecvAcceptChannel | Self::RecvFundingSigned | Self::RecvChannelReady + | Self::RecvShutdown | Self::MineBlocks(_) | Self::BroadcastTransaction | Self::LookupShortChannelId => false, @@ -919,6 +937,7 @@ impl Operation { | Self::RecvAcceptChannel | Self::RecvFundingSigned | Self::RecvChannelReady + | Self::RecvShutdown | Self::BroadcastTransaction | Self::LookupShortChannelId => false, } diff --git a/smite-ir/src/tests.rs b/smite-ir/src/tests.rs index 2cb77285..ff5132b4 100644 --- a/smite-ir/src/tests.rs +++ b/smite-ir/src/tests.rs @@ -570,7 +570,7 @@ fn display_send_and_recv_channel_ready_program() { } #[test] -fn display_send_shutdown_program() { +fn display_send_and_recv_shutdown_program() { let instructions = vec![ Instruction { operation: Operation::LoadChannelId([0xcd; 32]), @@ -584,6 +584,10 @@ fn display_send_shutdown_program() { operation: Operation::SendShutdown, inputs: vec![0, 1], }, + Instruction { + operation: Operation::RecvShutdown, + inputs: vec![2], + }, ]; let program = Program { instructions }; @@ -596,6 +600,7 @@ fn display_send_shutdown_program() { format!("v0 = LoadChannelId(0x{cid_hex})"), format!("v1 = LoadShutdownScript(P2wpkh(0x{spk_hex}))"), "v2 = SendShutdown(v0, v1)".into(), + "v3 = RecvShutdown(v2)".into(), ]; assert_eq!(lines.len(), expected.len(), "line count mismatch"); for (i, (got, want)) in lines.iter().zip(expected.iter()).enumerate() { diff --git a/smite-ir/src/variable.rs b/smite-ir/src/variable.rs index 7afe6b34..fedb0301 100644 --- a/smite-ir/src/variable.rs +++ b/smite-ir/src/variable.rs @@ -4,7 +4,7 @@ //! The serialized program stores data only in [`Operation`] literals. use bitcoin::secp256k1::PublicKey; -use smite::bolt::{AcceptChannel, ChannelId, OpenChannel, ShortChannelId}; +use smite::bolt::{AcceptChannel, ChannelId, OpenChannel, ShortChannelId, Shutdown}; use smite::channel_tx::FundingTransaction; const CHAIN_HASH_SIZE: usize = 32; @@ -60,7 +60,7 @@ pub enum Variable { SentFundingCreated, /// `shutdown` has been sent, so the counterparty's `shutdown` may now be /// received. - SentShutdown, + SentShutdown(Shutdown), } impl Variable { @@ -88,7 +88,7 @@ impl Variable { Self::FundingTransaction(_) => VariableType::FundingTransaction, Self::SentOpenChannel => VariableType::SentOpenChannel, Self::SentFundingCreated => VariableType::SentFundingCreated, - Self::SentShutdown => VariableType::SentShutdown, + Self::SentShutdown(_) => VariableType::SentShutdown, } } } diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index bf6f6b90..a8fc897d 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -11,7 +11,7 @@ use smite::bolt::{ AcceptChannel, AnnouncementSignatures, ChannelAnnouncement, ChannelId, ChannelReady, ChannelReadyTlvs, ChannelUpdate, Features, FromMessage, FundingCreated, FundingSigned, Message, MessageType, NodeAnnouncement, OpenChannel, OpenChannelTlvs, Pong, ShortChannelId, Shutdown, - TemporaryChannelId, + TemporaryChannelId, is_standard_shutdown_script, }; use smite::channel_tx::{ ChannelConfig, ChannelPartyConfig, ChannelState, FundingTransaction, HolderIdentity, Side, @@ -484,14 +484,14 @@ impl Executor { Operation::SendShutdown => { let sd = build_shutdown(&variables, &instr.inputs); - let encoded = Message::Shutdown(sd).encode(); + let encoded = Message::Shutdown(sd.clone()).encode(); log::debug!( "[{:?}] SendShutdown: {} bytes", start.elapsed(), encoded.len() ); self.conn.send_message(&encoded)?; - Some(Variable::SentShutdown) + Some(Variable::SentShutdown(sd)) } Operation::RecvAcceptChannel => { @@ -538,6 +538,40 @@ impl Executor { None } + Operation::RecvShutdown => { + let Variable::SentShutdown(sent) = consume_affine( + &mut variables, + instr.inputs[0], + instr.operation.input_types()[0], + ) else { + unreachable!("consume_affine checked the variable type"); + }; + // TODO: we only expect `shutdown` when all HTLCs are resolved, else this is a + // no-op. + let reply = if is_shutdown_expected(&self.channel_states, sent.channel_id) { + log::debug!("[{:?}] RecvShutdown: waiting", start.elapsed()); + let reply = recv_shutdown_reply( + &mut self.conn, + &sent, + &self.context.negotiated_features, + )?; + log::debug!("[{:?}] RecvShutdown: received", start.elapsed()); + reply + } else { + None + }; + match reply { + Some(sd) => { + self.channel_states + .get_mut(&sent.channel_id) + .expect("is_shutdown_expected guarantees a tracked channel") + .counterparty_shutdown_received = true; + Some(Variable::Bytes(sd.scriptpubkey)) + } + None => Some(Variable::Bytes(Vec::new())), + } + } + Operation::MineBlocks(v) => { // Clear the private mempool and mine the requested blocks, // adding those transactions to the first block. @@ -681,8 +715,12 @@ define_resolver!( ); /// Consumes an affine variable, leaving its slot void so it cannot be used -/// again. -fn consume_affine(variables: &mut [Option], index: usize, expected: VariableType) { +/// again, and returns it. +fn consume_affine( + variables: &mut [Option], + index: usize, + expected: VariableType, +) -> Variable { assert!( expected.is_affine(), "consume_affine called with non-affine type {expected:?}; voiding the slot would break later reads" @@ -691,7 +729,9 @@ fn consume_affine(variables: &mut [Option], index: usize, expected: Va if actual != expected { type_mismatch(index, expected, actual); } - variables[index] = None; + variables[index] + .take() + .expect("resolve checked the slot is not void") } // -- Operation handlers -- @@ -1247,6 +1287,60 @@ fn is_channel_ready_expected( }) } +/// Returns `true` if the target still owes us a `shutdown` response on the given channel. +fn is_shutdown_expected( + channel_states: &HashMap, + channel_id: ChannelId, +) -> bool { + // TODO: we don't know for sure if the target will reply because if a target didn't reply with + // `channel_ready` yet, it MAY reply with `shutdown` (but doesn't have to) + // TODO: once the target has replied, a duplicate `shutdown` from it goes unread here + channel_states + .get(&channel_id) + .is_some_and(|state| !state.counterparty_shutdown_received) +} + +/// Receives the target's reply to our `shutdown`, or `None` if it sent a +/// `warning` for our channel instead, which BOLT 2 allows when our +/// `scriptpubkey` is non-standard. +/// +/// # Errors +/// +/// Returns [`ExecuteError::UnexpectedMessage`] if the received message is +/// neither a `shutdown` nor such a `warning`, or is a `shutdown` for another +/// channel. That may answer a `shutdown` we sent there earlier, which we can't +/// check against the `shutdown` we sent on this channel. +fn recv_shutdown_reply( + conn: &mut impl Connection, + sent: &Shutdown, + negotiated_features: &Features, +) -> Result, ExecuteError> { + let may_warn = !is_standard_shutdown_script(&sent.scriptpubkey, negotiated_features); + match recv_non_ping(conn, RECV_IDLE_TIMEOUT)? { + Message::Shutdown(sd) if sd.channel_id == sent.channel_id => Ok(Some(sd)), + Message::Shutdown(sd) => { + log::debug!( + "received shutdown on {} while waiting on {}", + sd.channel_id, + sent.channel_id + ); + Err(ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: MessageType::SHUTDOWN, + }) + } + Message::Warning(w) + if may_warn && (w.channel_id == sent.channel_id || w.channel_id == ChannelId::ALL) => + { + Ok(None) + } + other => Err(ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: other.msg_type(), + }), + } +} + /// Records a sent `open_channel`, keyed by `temporary_channel_id`, so the /// funding flow can build commitments from the values actually put on the wire. /// diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index ed2f6362..41459f95 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -8,7 +8,7 @@ use bitcoin::Amount; use bitcoin::secp256k1::{Secp256k1, SecretKey}; use harness::*; use programs::*; -use smite::bolt::{AcceptChannelTlvs, GossipTimestampFilter, Init, Ping}; +use smite::bolt::{AcceptChannelTlvs, GossipTimestampFilter, Init, Ping, Warning}; use smite_ir::Instruction; use smite_ir::builder::ProgramBuilder; use smite_ir::operation::ShutdownScriptVariant; @@ -1119,6 +1119,248 @@ fn execute_send_shutdown_empty_scriptpubkey() { assert!(sd.scriptpubkey.is_empty()); } +#[test] +fn execute_recv_shutdown() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + let script = ShutdownScriptVariant::P2wpkh([0xab; 20]); + let mut fx = fx.queue(&Message::Shutdown(Shutdown::for_channel( + channel_id, + script.encode(), + ))); + + fx.run(&recv_shutdown_program(channel_id, script)); + + // TODO: Once we add IR support for building closing transactions, verify + // the returned scriptpubkey through the closing transaction's output. + + assert!(fx.channel_state(&channel_id).counterparty_shutdown_received); + assert_eq!(fx.queued_len(), 0); +} + +#[test] +fn execute_recv_shutdown_unknown_channel() { + let (fx, _) = recv_channel_ready_fixture(); + let unknown = ChannelId::new([0x7a; 32]); + let script = ShutdownScriptVariant::P2wpkh([0xcd; 20]); + let mut fx = fx.queue(&Message::Shutdown(Shutdown::for_channel( + unknown, + script.encode(), + ))); + + let err = fx.run_err(&recv_shutdown_program(funding_channel_id(), script)); + + assert!(matches!( + err, + ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: MessageType::SHUTDOWN, + } + )); +} + +#[test] +fn execute_recv_shutdown_other_tracked_channel() { + let channel_id = funding_channel_id(); + let second_utxo = Utxo { + outpoint: OutPoint { + vout: 1, + ..sample_utxo().outpoint + }, + ..sample_utxo() + }; + let mut second_negotiation = sample_funding_negotiation(); + second_negotiation.open_channel.temporary_channel_id = TemporaryChannelId::new([0xee; 32]); + + // Establish our channel, then track a second one by sending its + // `funding_created`. + let mut b = ProgramBuilder::new(); + establish_channel(&mut b, 6); + let second = create_funding_tx(&mut b); + let second_temporary_channel_id = b.append(Operation::LoadChannelId([0xee; 32]), &[]); + b.append( + Operation::SendFundingCreated, + &[ + second.tx, + second.opener_privkey, + second_temporary_channel_id, + ], + ); + let (fx, _) = recv_channel_ready_fixture(); + let mut fx = fx + .with_utxos(vec![sample_utxo(), second_utxo]) + .with_negotiation(second_negotiation); + fx.run(&b.build()); + let other = *fx + .channel_states() + .keys() + .find(|id| **id != channel_id) + .expect("second channel tracked"); + + // The target may be answering a `shutdown` we sent on the other channel, + // which this `RecvShutdown` can't judge. + let script = ShutdownScriptVariant::P2wpkh([0xab; 20]); + let mut fx = fx.queue(&Message::Shutdown(Shutdown::for_channel( + other, + script.encode(), + ))); + let mut b = ProgramBuilder::new(); + let shutdown = send_shutdown(&mut b, channel_id, script); + b.append(Operation::RecvShutdown, &[shutdown.sent]); + + let err = fx.run_err(&b.build()); + + assert!(matches!( + err, + ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: MessageType::SHUTDOWN, + } + )); + assert!(!fx.channel_state(&channel_id).counterparty_shutdown_received); + assert!(!fx.channel_state(&other).counterparty_shutdown_received); +} + +#[test] +fn execute_recv_shutdown_after_response_is_noop() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + let script = ShutdownScriptVariant::P2wpkh([0xab; 20]); + // Only one reply is queued: once the target has responded, the second + // RecvShutdown must be a no-op rather than block on an empty queue. + let mut fx = fx.queue(&Message::Shutdown(Shutdown::for_channel( + channel_id, + script.encode(), + ))); + + let mut b = ProgramBuilder::new(); + establish_channel(&mut b, 6); + let first = send_shutdown(&mut b, channel_id, script); + b.append(Operation::RecvShutdown, &[first.sent]); + let second = b.append( + Operation::SendShutdown, + &[first.channel_id, first.scriptpubkey], + ); + b.append(Operation::RecvShutdown, &[second]); + + fx.run(&b.build()); + + assert_eq!(fx.queued_len(), 0); +} + +#[test] +fn execute_recv_shutdown_untracked_channel_is_noop() { + let (mut fx, _) = recv_channel_ready_fixture(); + let untracked = ChannelId::new([0x99; 32]); + + // No shutdown reply is queued: a RecvShutdown for a channel we never + // established must be a no-op rather than block on an empty queue. + fx.run(&recv_shutdown_program( + untracked, + ShutdownScriptVariant::P2wpkh([0xab; 20]), + )); + + assert_eq!(fx.queued_len(), 0); +} + +fn warning_reply(channel_id: ChannelId) -> Message { + Message::Warning(Warning { + channel_id, + data: b"non-standard scriptpubkey".to_vec(), + }) +} + +#[test] +fn execute_recv_shutdown_warning_for_non_standard_script() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + // The target SHOULD answer our non-standard `scriptpubkey` with a warning + // rather than a `shutdown`, which `RecvShutdown` must accept. + let mut fx = fx.queue(&warning_reply(channel_id)); + + fx.run(&recv_shutdown_program( + channel_id, + ShutdownScriptVariant::Empty, + )); + + assert!(!fx.channel_state(&channel_id).counterparty_shutdown_received); + assert_eq!(fx.queued_len(), 0); +} + +#[test] +fn execute_recv_shutdown_warning_for_standard_script() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + let mut fx = fx.queue(&warning_reply(channel_id)); + + let err = fx.run_err(&recv_shutdown_program( + channel_id, + ShutdownScriptVariant::P2wpkh([0xab; 20]), + )); + + assert!(matches!( + err, + ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: MessageType::WARNING, + } + )); +} + +#[test] +fn execute_recv_shutdown_reply_to_non_standard_script() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + // The target SHOULD warn about our non-standard `scriptpubkey`, but may + // reply with a `shutdown` anyway, which must still be valid. + let mut fx = fx.queue(&Message::Shutdown(Shutdown::for_channel( + channel_id, + ShutdownScriptVariant::P2wpkh([0xcd; 20]).encode(), + ))); + + fx.run(&recv_shutdown_program( + channel_id, + ShutdownScriptVariant::Empty, + )); + + assert!(fx.channel_state(&channel_id).counterparty_shutdown_received); + assert_eq!(fx.queued_len(), 0); +} + +#[test] +fn execute_recv_shutdown_warning_for_all_channels() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + let mut fx = fx.queue(&warning_reply(ChannelId::ALL)); + + fx.run(&recv_shutdown_program( + channel_id, + ShutdownScriptVariant::Empty, + )); + + assert!(!fx.channel_state(&channel_id).counterparty_shutdown_received); + assert_eq!(fx.queued_len(), 0); +} + +#[test] +fn execute_recv_shutdown_warning_for_other_channel() { + let (fx, _) = recv_channel_ready_fixture(); + let mut fx = fx.queue(&warning_reply(ChannelId::new([0x99; 32]))); + + let err = fx.run_err(&recv_shutdown_program( + funding_channel_id(), + ShutdownScriptVariant::Empty, + )); + + assert!(matches!( + err, + ExecuteError::UnexpectedMessage { + expected: MessageType::SHUTDOWN, + got: MessageType::WARNING, + } + )); +} + #[test] fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { // Corrupt the negotiated acceptor funding pubkey so the broadcast funding diff --git a/smite-scenarios/src/executor/tests/programs.rs b/smite-scenarios/src/executor/tests/programs.rs index 362ab313..d136bcec 100644 --- a/smite-scenarios/src/executor/tests/programs.rs +++ b/smite-scenarios/src/executor/tests/programs.rs @@ -11,6 +11,7 @@ use super::harness::{PointSource, SampleOpenChannel, acceptor_funding_sk, opener use crate::executor::*; use smite_ir::Instruction; use smite_ir::builder::ProgramBuilder; +use smite_ir::operation::ShutdownScriptVariant; // -- open_channel -- @@ -305,14 +306,59 @@ pub fn send_funding_created_and_recv_funding_signed_program() -> Program { b.build() } -/// A program that sends `funding_created`, receives `funding_signed`, mines -/// `confirmations` blocks, and receives the target's `channel_ready`. -pub fn recv_channel_ready_program(confirmations: u8) -> Program { - let mut b = ProgramBuilder::new(); - let funding_created = send_funding_created(&mut b); +/// Sends `funding_created`, receives `funding_signed`, mines `confirmations` +/// blocks, and receives the target's `channel_ready`. +pub fn establish_channel(b: &mut ProgramBuilder, confirmations: u8) { + let funding_created = send_funding_created(b); b.append(Operation::RecvFundingSigned, &[funding_created.sent]); b.append(Operation::MineBlocks(confirmations), &[]); b.append(Operation::RecvChannelReady, &[]); +} + +/// A program that runs [`establish_channel`]. +pub fn recv_channel_ready_program(confirmations: u8) -> Program { + let mut b = ProgramBuilder::new(); + establish_channel(&mut b, confirmations); + + b.build() +} + +// -- shutdown -- + +/// The variables a sent `shutdown` produces. +#[derive(Clone, Copy)] +pub struct SentShutdown { + pub channel_id: usize, + pub scriptpubkey: usize, + /// The `SendShutdown` result, an affine variable a single `RecvShutdown` + /// may consume. + pub sent: usize, +} + +/// Sends a `shutdown` for `channel_id` carrying `script`. +pub fn send_shutdown( + b: &mut ProgramBuilder, + channel_id: ChannelId, + script: ShutdownScriptVariant, +) -> SentShutdown { + let channel_id = b.append(Operation::LoadChannelId(channel_id.0), &[]); + let scriptpubkey = b.append(Operation::LoadShutdownScript(script), &[]); + let sent = b.append(Operation::SendShutdown, &[channel_id, scriptpubkey]); + + SentShutdown { + channel_id, + scriptpubkey, + sent, + } +} + +/// A program that establishes the funding flow's channel, sends a `shutdown` +/// for `channel_id` carrying `script`, and receives the target's `shutdown`. +pub fn recv_shutdown_program(channel_id: ChannelId, script: ShutdownScriptVariant) -> Program { + let mut b = ProgramBuilder::new(); + establish_channel(&mut b, 6); + let shutdown = send_shutdown(&mut b, channel_id, script); + b.append(Operation::RecvShutdown, &[shutdown.sent]); b.build() } diff --git a/smite/src/channel_tx/commitment.rs b/smite/src/channel_tx/commitment.rs index 3769a9e0..3b311957 100644 --- a/smite/src/channel_tx/commitment.rs +++ b/smite/src/channel_tx/commitment.rs @@ -169,6 +169,10 @@ pub struct ChannelState { /// Whether a `funding_signed` has already been accepted for this channel. /// Any later one means the target re-signed a channel it already funded. pub funding_signed_received: bool, + /// Whether the peer has already responded to our `shutdown`. A target may + /// ignore any `shutdown` after the first, so a later `RecvShutdown` for + /// this channel is a no-op. + pub counterparty_shutdown_received: bool, } impl Side { @@ -210,6 +214,7 @@ impl ChannelState { was_funding_mined_prematurely, sent_invalid_signature, funding_signed_received: false, + counterparty_shutdown_received: false, } } From 5e463ae16a831b42c22369b6a803cc111ee2697e Mon Sep 17 00:00:00 2001 From: ekzyis Date: Tue, 22 Sep 2026 15:46:38 +0200 Subject: [PATCH 2/2] smite: add shutdown oracle RecvShutdown now checks the target's `shutdown` with a new ShutdownOracle. It flags the target if: * the reply is for a channel we never established * we sent an invalid signature, so the target should have failed the channel instead of replying * its scriptpubkey isn't a standard form for the negotiated features A `shutdown` for a channel we don't track is now passed to the oracle instead of ending the program as an unexpected message. --- smite-scenarios/src/executor.rs | 20 +- smite-scenarios/src/executor/tests.rs | 73 ++++- smite-scenarios/src/executor/tests/harness.rs | 8 +- smite/src/oracles.rs | 2 + smite/src/oracles/shutdown.rs | 255 ++++++++++++++++++ smite/src/violation.rs | 10 + 6 files changed, 357 insertions(+), 11 deletions(-) create mode 100644 smite/src/oracles/shutdown.rs diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index a8fc897d..3804f3a9 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -20,6 +20,7 @@ use smite::channel_tx::{ use smite::noise::{ConnectionError, NoiseConnection}; use smite::oracles::{ AcceptChannelContext, AcceptChannelOracle, FundingSignedContext, FundingSignedOracle, Oracle, + ShutdownContext, ShutdownOracle, }; use smite::pending_channel::PendingChannel; use smite::violation::Violation; @@ -553,6 +554,7 @@ impl Executor { let reply = recv_shutdown_reply( &mut self.conn, &sent, + &self.channel_states, &self.context.negotiated_features, )?; log::debug!("[{:?}] RecvShutdown: received", start.elapsed()); @@ -562,6 +564,11 @@ impl Executor { }; match reply { Some(sd) => { + ShutdownOracle.evaluate(&ShutdownContext { + shutdown: &sd, + channel: self.channel_states.get(&sd.channel_id), + negotiated_features: &self.context.negotiated_features, + })?; self.channel_states .get_mut(&sent.channel_id) .expect("is_shutdown_expected guarantees a tracked channel") @@ -1304,20 +1311,27 @@ fn is_shutdown_expected( /// `warning` for our channel instead, which BOLT 2 allows when our /// `scriptpubkey` is non-standard. /// +/// A `shutdown` for an untracked channel is returned for the oracle to flag. +/// /// # Errors /// /// Returns [`ExecuteError::UnexpectedMessage`] if the received message is /// neither a `shutdown` nor such a `warning`, or is a `shutdown` for another -/// channel. That may answer a `shutdown` we sent there earlier, which we can't -/// check against the `shutdown` we sent on this channel. +/// tracked channel. That may answer a `shutdown` we sent there earlier, which +/// we can't check against the `shutdown` we sent on this channel. fn recv_shutdown_reply( conn: &mut impl Connection, sent: &Shutdown, + channel_states: &HashMap, negotiated_features: &Features, ) -> Result, ExecuteError> { let may_warn = !is_standard_shutdown_script(&sent.scriptpubkey, negotiated_features); match recv_non_ping(conn, RECV_IDLE_TIMEOUT)? { - Message::Shutdown(sd) if sd.channel_id == sent.channel_id => Ok(Some(sd)), + Message::Shutdown(sd) + if sd.channel_id == sent.channel_id || !channel_states.contains_key(&sd.channel_id) => + { + Ok(Some(sd)) + } Message::Shutdown(sd) => { log::debug!( "received shutdown on {} while waiting on {}", diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 41459f95..9bcd5fb4 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -1150,13 +1150,11 @@ fn execute_recv_shutdown_unknown_channel() { let err = fx.run_err(&recv_shutdown_program(funding_channel_id(), script)); - assert!(matches!( - err, - ExecuteError::UnexpectedMessage { - expected: MessageType::SHUTDOWN, - got: MessageType::SHUTDOWN, - } - )); + let ExecuteError::Violation(Violation::InvalidShutdown(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, unknown); + assert!(reason.contains("unknown channel_id: no channel was established for this channel")); } #[test] @@ -1221,6 +1219,36 @@ fn execute_recv_shutdown_other_tracked_channel() { assert!(!fx.channel_state(&other).counterparty_shutdown_received); } +#[test] +fn execute_recv_shutdown_non_standard_script() { + let (fx, _) = recv_channel_ready_fixture(); + let channel_id = funding_channel_id(); + // Legacy P2PKH is never a standard script for a sender, even for a + // channel we know and even with every shutdown feature negotiated. + let non_standard = ShutdownScriptVariant::P2pkh([0x11; 20]).encode(); + let mut fx = fx + .with_negotiated_feature(Features::OPTION_SHUTDOWN_ANYSEGWIT) + .with_negotiated_feature(Features::OPTION_SIMPLE_CLOSE) + .queue(&Message::Shutdown(Shutdown::for_channel( + channel_id, + non_standard.clone(), + ))); + + let err = fx.run_err(&recv_shutdown_program( + channel_id, + ShutdownScriptVariant::P2wpkh([0xab; 20]), + )); + + let ExecuteError::Violation(Violation::InvalidShutdown(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains(&format!( + "non-standard scriptpubkey: {}", + hex::encode(&non_standard) + ))); +} + #[test] fn execute_recv_shutdown_after_response_is_noop() { let (fx, _) = recv_channel_ready_fixture(); @@ -1263,6 +1291,37 @@ fn execute_recv_shutdown_untracked_channel_is_noop() { assert_eq!(fx.queued_len(), 0); } +#[test] +fn execute_recv_shutdown_after_invalid_signature() { + let channel_id = funding_channel_id(); + let script = ShutdownScriptVariant::P2wpkh([0xab; 20]); + let mut b = ProgramBuilder::new(); + let funding = create_funding_tx(&mut b); + b.append(Operation::BroadcastTransaction, &[funding.tx]); + // Sign the commitment with the acceptor's private key instead of the + // opener's, so the signature does not match the `funding_pubkey` negotiated + // in `open_channel`. + send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + let shutdown = send_shutdown(&mut b, channel_id, script.clone()); + b.append(Operation::RecvShutdown, &[shutdown.sent]); + + // Having signed with the wrong key, the target must fail the channel, so + // answering our `shutdown` is a violation. + let err = Fixture::new() + .with_negotiation(sample_funding_negotiation()) + .queue(&Message::Shutdown(Shutdown::for_channel( + channel_id, + script.encode(), + ))) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidShutdown(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("replied on a failed channel: we sent an invalid signature")); +} + fn warning_reply(channel_id: ChannelId) -> Message { Message::Warning(Warning { channel_id, diff --git a/smite-scenarios/src/executor/tests/harness.rs b/smite-scenarios/src/executor/tests/harness.rs index 64ca233a..1a133253 100644 --- a/smite-scenarios/src/executor/tests/harness.rs +++ b/smite-scenarios/src/executor/tests/harness.rs @@ -2,7 +2,7 @@ use crate::executor::*; use bitcoin::{Amount, Transaction}; -use smite::bolt::{AcceptChannelTlvs, ChannelTypeVariant, FromMessage}; +use smite::bolt::{AcceptChannelTlvs, ChannelTypeVariant, FeatureBit, FromMessage}; use std::collections::VecDeque; use std::str::FromStr; @@ -151,6 +151,12 @@ impl Fixture { self } + /// Adds `bit` to the features negotiated with the target. + pub fn with_negotiated_feature(mut self, bit: FeatureBit) -> Self { + self.executor.context.negotiated_features.set_bit(bit); + self + } + /// Records `pending` as the negotiation for its `temporary_channel_id`. pub fn with_negotiation(mut self, pending: PendingChannel) -> Self { self.executor diff --git a/smite/src/oracles.rs b/smite/src/oracles.rs index 937b2fc2..690e5fc6 100644 --- a/smite/src/oracles.rs +++ b/smite/src/oracles.rs @@ -4,10 +4,12 @@ mod accept_channel; mod funding_signed; +mod shutdown; use super::violation::Violation; pub use accept_channel::{AcceptChannelContext, AcceptChannelOracle}; pub use funding_signed::{FundingSignedContext, FundingSignedOracle}; +pub use shutdown::{ShutdownContext, ShutdownOracle}; /// `Oracle` evaluates a condition against some context pub trait Oracle { diff --git a/smite/src/oracles/shutdown.rs b/smite/src/oracles/shutdown.rs new file mode 100644 index 00000000..d7f045f6 --- /dev/null +++ b/smite/src/oracles/shutdown.rs @@ -0,0 +1,255 @@ +//! BOLT 2 `shutdown` oracle, for cooperative-close initiation. + +use super::Oracle; +use crate::bolt::{Features, Shutdown, is_standard_shutdown_script}; +use crate::channel_tx::ChannelState; +use crate::violation::Violation; + +/// Context for [`ShutdownOracle`]. +pub struct ShutdownContext<'a> { + /// The `shutdown` received from the peer. + pub shutdown: &'a Shutdown, + /// The channel the `shutdown` belongs to, identified by its `channel_id`, + /// or `None` if no such channel was established. + pub channel: Option<&'a ChannelState>, + /// Features negotiated between the target node and Smite, which decide the + /// standard `scriptpubkey` forms. + pub negotiated_features: &'a Features, +} + +/// Checks that a received `shutdown` references a channel we know, answers a +/// `shutdown` the target was allowed to answer, and carries a `scriptpubkey` +/// that is a standard form for the negotiated features. +pub struct ShutdownOracle; + +impl Oracle> for ShutdownOracle { + fn evaluate(&self, context: &ShutdownContext<'_>) -> Result<(), Violation> { + let Shutdown { + channel_id, + scriptpubkey, + } = context.shutdown; + + // Check that the `shutdown` references a channel we established. + let Some(channel) = context.channel else { + return Err(Violation::InvalidShutdown( + *channel_id, + "unknown channel_id: no channel was established for this channel".to_string(), + )); + }; + + // Check that the target did not respond on a channel it was required to + // fail. + if channel.sent_invalid_signature { + return Err(Violation::InvalidShutdown( + *channel_id, + "replied on a failed channel: we sent an invalid signature".to_string(), + )); + } + + // Check that the `scriptpubkey` is a form BOLT 2 permits a sender to use. + if !is_standard_shutdown_script(scriptpubkey, context.negotiated_features) { + return Err(Violation::InvalidShutdown( + *channel_id, + format!( + "non-standard scriptpubkey: {} is not permitted for the negotiated features", + hex::encode(scriptpubkey), + ), + )); + } + + // TODO: BOLT 2 forbids sending `shutdown` while HTLCs are still pending on our commitment. + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bolt::ChannelId; + use crate::channel_tx::{ChannelConfig, ChannelPartyConfig, HolderIdentity, Side}; + use bitcoin::OutPoint; + use bitcoin::opcodes::all::{ + OP_CHECKSIG, OP_DUP, OP_EQUALVERIFY, OP_HASH160, OP_PUSHBYTES_0, OP_PUSHBYTES_20, + OP_PUSHNUM_1, OP_RETURN, + }; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + + 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 state for testing. + fn channel_state() -> ChannelState { + let pkey1 = pubkey(1); + let pkey2 = pubkey(2); + + let config = ChannelConfig { + funding_outpoint: OutPoint { + txid: "09b0549b35f14ee862f63bd75811c6c27963c4dea6766ec6836952ec78df1e7e" + .parse() + .expect("valid txid hex"), + vout: 0, + }, + funding_satoshis: 10_000_000, + channel_type: Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), + opener: ChannelPartyConfig { + funding_pubkey: pkey1, + payment_basepoint: pkey1, + revocation_basepoint: pkey1, + delayed_payment_basepoint: pkey1, + dust_limit_satoshis: 546, + to_self_delay: 144, + }, + acceptor: ChannelPartyConfig { + funding_pubkey: pkey2, + payment_basepoint: pkey2, + revocation_basepoint: pkey2, + delayed_payment_basepoint: pkey2, + dust_limit_satoshis: 546, + to_self_delay: 144, + }, + minimum_depth: 8, + }; + let commitment = config + .new_initial_commitment(3_000_000_000, 15_000, pkey1, pkey2) + .expect("valid initial commitment"); + let holder = HolderIdentity { + side: Side::Opener, + funding_privkey: secret_key(1), + }; + + ChannelState::new(config, holder, commitment, true, false, false) + } + + fn p2pkh() -> Vec { + let mut spk = vec![OP_DUP.to_u8(), OP_HASH160.to_u8(), OP_PUSHBYTES_20.to_u8()]; + spk.extend_from_slice(&[0x11; 20]); + spk.extend_from_slice(&[OP_EQUALVERIFY.to_u8(), OP_CHECKSIG.to_u8()]); + spk + } + + fn p2wpkh(seed: u8) -> Vec { + let mut spk = vec![OP_PUSHBYTES_0.to_u8(), OP_PUSHBYTES_20.to_u8()]; + spk.extend_from_slice(&[seed; 20]); + spk + } + + fn anysegwit() -> Vec { + let mut spk = vec![OP_PUSHNUM_1.to_u8(), 32]; + spk.extend_from_slice(&[0x00; 32]); + spk + } + + fn simple_close() -> Vec { + let mut spk = vec![OP_RETURN.to_u8(), 6]; + spk.extend_from_slice(&[0xab; 6]); + spk + } + + fn shutdown(scriptpubkey: Vec) -> Shutdown { + Shutdown::for_channel(ChannelId::new([0x7a; 32]), scriptpubkey) + } + + #[track_caller] + fn assert_pass(shutdown: &Shutdown, channel: Option<&ChannelState>, features: &Features) { + if let Err(err) = ShutdownOracle.evaluate(&ShutdownContext { + shutdown, + channel, + negotiated_features: features, + }) { + panic!("expected pass, got: {err}"); + } + } + + #[track_caller] + fn assert_fail( + shutdown: &Shutdown, + channel: Option<&ChannelState>, + features: &Features, + expected: &str, + ) { + match ShutdownOracle.evaluate(&ShutdownContext { + shutdown, + channel, + negotiated_features: features, + }) { + Err(Violation::InvalidShutdown(chan_id, reason)) => { + assert_eq!(shutdown.channel_id, chan_id); + assert!( + reason.contains(expected), + "unexpected failure reason: {reason}" + ); + } + _ => panic!("expected failure: {expected}"), + } + } + + #[test] + fn conforming_shutdown_passes() { + assert_pass( + &shutdown(p2wpkh(0x33)), + Some(&channel_state()), + &Features::new(), + ); + } + + #[test] + fn shutdown_for_unknown_channel_id() { + assert_fail( + &shutdown(p2wpkh(0x33)), + None, + &Features::new(), + "unknown channel_id: no channel was established for this channel", + ); + } + + #[test] + fn shutdown_after_invalid_signature() { + let mut channel = channel_state(); + channel.sent_invalid_signature = true; + + assert_fail( + &shutdown(p2wpkh(0x33)), + Some(&channel), + &Features::new(), + "replied on a failed channel: we sent an invalid signature", + ); + } + + #[test] + fn shutdown_with_non_standard_script() { + assert_fail( + &shutdown(p2pkh()), + Some(&channel_state()), + &Features::new(), + &format!("non-standard scriptpubkey: {}", hex::encode(p2pkh())), + ); + } + + #[test] + fn shutdown_script_standard_only_with_negotiated_feature() { + let channel = channel_state(); + + for (spk, feature) in [ + (anysegwit(), Features::OPTION_SHUTDOWN_ANYSEGWIT), + (simple_close(), Features::OPTION_SIMPLE_CLOSE), + ] { + assert_fail( + &shutdown(spk.clone()), + Some(&channel), + &Features::new(), + "non-standard scriptpubkey", + ); + assert_pass( + &shutdown(spk), + Some(&channel), + &Features::from_bits(&[feature]), + ); + } + } +} diff --git a/smite/src/violation.rs b/smite/src/violation.rs index a2ff8358..1c053819 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -50,4 +50,14 @@ pub enum Violation { /// 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 `shutdown` broke a BOLT 2 requirement, as judged by + /// [`crate::oracles::ShutdownOracle`]. The reason names the breached + /// requirement, one of: + /// - it names a `channel_id` we never established, + /// - it answers on a channel we sent an invalid signature for, or + /// - its `scriptpubkey` is not a form BOLT 2 permits a sender to use for the + /// negotiated features. + #[error("invalid shutdown for channel_id {0}: {1}")] + InvalidShutdown(ChannelId, String), }