diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 28fb7bc6..4210e943 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -18,7 +18,10 @@ use smite::channel_tx::{ build_funding_transaction, }; use smite::noise::{ConnectionError, NoiseConnection}; -use smite::oracles::{AcceptChannelContext, AcceptChannelOracle, Oracle}; +use smite::oracles::{ + AcceptChannelContext, AcceptChannelOracle, ChannelReadyContext, ChannelReadyOracle, + FundingSignedContext, FundingSignedOracle, Oracle, +}; use smite::pending_channel::PendingChannel; use smite::violation::Violation; @@ -251,6 +254,9 @@ pub struct Executor { /// `temporary_channel_id`, so the funding flow can build commitments from /// the parameters actually sent on the wire. negotiations: HashMap, + /// Per-commitment points revealed by either us or the target, used to + /// detect points revealed more than once by the target. + per_commitment_points: HashSet, /// 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 @@ -277,6 +283,7 @@ impl Executor { context, channel_states: HashMap::new(), negotiations: HashMap::new(), + per_commitment_points: HashSet::new(), private_mempool: Vec::new(), unmined_txids: HashSet::new(), mined_txids: HashSet::new(), @@ -434,7 +441,11 @@ impl Executor { Operation::SendOpenChannel => { let oc = resolve_open_channel_message(&variables, instr.inputs[0]); - record_send_open_channel(&mut self.negotiations, oc); + record_send_open_channel( + &mut self.negotiations, + &mut self.per_commitment_points, + oc, + ); let encoded = Message::OpenChannel(oc.clone()).encode(); log::debug!( "[{:?}] SendOpenChannel: {} bytes", @@ -469,6 +480,7 @@ impl Executor { &instr.inputs, *include_alias, &mut self.channel_states, + &mut self.per_commitment_points, ); let encoded = Message::ChannelReady(cr).encode(); log::debug!( @@ -505,8 +517,13 @@ impl Executor { accept_channel: &ac, negotiation: self.negotiations.get(&ac.temporary_channel_id), negotiated_features: &self.context.negotiated_features, + per_commitment_points: &self.per_commitment_points, })?; - record_recv_accept_channel(&mut self.negotiations, &ac); + record_recv_accept_channel( + &mut self.negotiations, + &mut self.per_commitment_points, + &ac, + ); Some(Variable::AcceptChannel(ac)) } @@ -519,15 +536,31 @@ impl Executor { log::debug!("[{:?}] RecvFundingSigned: waiting", start.elapsed()); let fs: FundingSigned = recv_bolt(&mut self.conn, RECV_IDLE_TIMEOUT)?; log::debug!("[{:?}] RecvFundingSigned: received", start.elapsed()); - verify_funding_signed(&fs, &self.channel_states)?; + FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed: &fs, + channel: self.channel_states.get(&fs.channel_id), + })?; + record_recv_funding_signed(&mut self.channel_states, &fs); Some(Variable::ChannelId(fs.channel_id)) } Operation::RecvChannelReady => { if is_channel_ready_expected(&self.channel_states, &mut self.bitcoin_cli) { log::debug!("[{:?}] RecvChannelReady: waiting", start.elapsed()); - recv_channel_ready(&mut self.conn, &mut self.channel_states)?; + let cr: ChannelReady = + recv_bolt(&mut self.conn, RECV_CHANNEL_READY_TIMEOUT)?; log::debug!("[{:?}] RecvChannelReady: received", start.elapsed()); + ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready: &cr, + channel: self.channel_states.get(&cr.channel_id), + negotiated_features: &self.context.negotiated_features, + per_commitment_points: &self.per_commitment_points, + })?; + record_recv_channel_ready( + &mut self.channel_states, + &mut self.per_commitment_points, + &cr, + ); } None } @@ -852,17 +885,6 @@ fn build_funding_created( }; let signature = config.sign_counterparty_commitment(&state, &holder); - let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint); - - // Check whether the funding outpoint is valid and contains the negotiated - // amount and funding script. If not, there is a good chance the target will - // neither complete the funding flow nor send an error message. - let is_funding_outpoint_valid = funding_tx.matches_funding_output( - &open_channel.funding_pubkey, - &accept_channel.funding_pubkey, - open_channel.funding_satoshis, - ); - // Only track a new channel when this negotiation has not built a // `funding_created` yet. If it has, we are likely resending one for the // same `temporary_channel_id` with a different outpoint, which the target @@ -872,6 +894,24 @@ fn build_funding_created( // This also means that building the same message again must not clobber a // channel whose state has already been established (and possibly advanced). if !pending.funding_built { + let channel_id = ChannelId::v1_from_funding_outpoint(config.funding_outpoint); + + // Check whether the funding outpoint is valid and contains the + // negotiated amount and funding script. If not, there is a good chance + // the target will neither complete the funding flow nor send an error + // message. + let is_funding_outpoint_valid = funding_tx.matches_funding_output( + &open_channel.funding_pubkey, + &accept_channel.funding_pubkey, + open_channel.funding_satoshis, + ); + // TODO: Once we support sending malformed signatures, update this state + // when constructing one so that the peer's acceptance can be detected + // as a violation. + let opener_funding_pubkey = + PublicKey::from_secret_key(&Secp256k1::new(), &opener_funding_privkey); + let sent_invalid_signature = opener_funding_pubkey != open_channel.funding_pubkey; + channel_states.entry(channel_id).or_insert_with(|| { ChannelState::new( config, @@ -879,6 +919,7 @@ fn build_funding_created( state, is_funding_outpoint_valid, mined_txids.contains(&funding_outpoint.txid), + sent_invalid_signature, ) }); } @@ -905,6 +946,7 @@ fn build_channel_ready( inputs: &[usize], include_alias: bool, channel_states: &mut HashMap, + per_commitment_points: &mut HashSet, ) -> ChannelReady { let channel_id = resolve_channel_id(variables, inputs[0]); let second_per_commitment_point = resolve_pubkey(variables, inputs[1]); @@ -916,12 +958,15 @@ fn build_channel_ready( // yet recorded: `channel_ready` may be resent, but BOLT peers ignore // redundant ones, so recording a resend would leave us with the wrong point // and make us reject a valid received commitment signature as invalid. + // + // The same point is added to `per_commitment_points` as revealed by us. if let Some(state) = channel_states.get_mut(&channel_id) && state.commitment.commitment_number == 0 { let next_point = state.next_holder_per_commitment_point_mut(); if next_point.is_none() { *next_point = Some(second_per_commitment_point); + per_commitment_points.insert(second_per_commitment_point); } } @@ -1186,38 +1231,14 @@ fn recv_bolt( }) } -/// Receives and decodes a `channel_ready` message. -/// -/// The `second_per_commitment_point` is recorded as the counterparty's next -/// per-commitment point on the channel it identifies. -/// -/// # Errors -/// -/// Returns [`ExecuteError::UnexpectedMessage`] if the received message is not a -/// `channel_ready`, or [`Violation::UnknownChannel`] if no channel state exists -/// for the message's `channel_id`. -fn recv_channel_ready( - conn: &mut impl Connection, - channel_states: &mut HashMap, -) -> Result<(), ExecuteError> { - let cr: ChannelReady = recv_bolt(conn, RECV_CHANNEL_READY_TIMEOUT)?; - - let state = channel_states - .get_mut(&cr.channel_id) - .ok_or(Violation::UnknownChannel(cr.channel_id))?; - *state.next_counterparty_per_commitment_point_mut() = Some(cr.second_per_commitment_point); - - Ok(()) -} - /// Returns `true` if the target owes us a `channel_ready` message. /// /// A `channel_ready` is expected when a tracked channel is still at commitment /// number 0, the counterparty's next per-commitment point is unknown, the /// advertised funding outpoint pays the negotiated funding output, the funding -/// transaction was mined only after we sent `funding_created`, and it has at -/// least `minimum_depth` confirmations (as specified in the received -/// `accept_channel`). +/// transaction was mined only after we sent `funding_created`, we have not sent +/// a signature the peer is required to reject, and it has at least +/// `minimum_depth` confirmations (as specified in the received `accept_channel`). fn is_channel_ready_expected( channel_states: &HashMap, bitcoin_cli: &mut impl BitcoinRpc, @@ -1227,34 +1248,12 @@ fn is_channel_ready_expected( && state.next_counterparty_per_commitment_point().is_none() && state.is_funding_outpoint_valid && !state.was_funding_mined_prematurely + && !state.sent_invalid_signature && bitcoin_cli.get_transaction_confirmations(state.config.funding_outpoint.txid) >= state.config.minimum_depth }) } -/// Verifies the counterparty's signature from a `funding_signed` message using -/// the channel state associated with the message's `channel_id`. -/// -/// # Errors -/// -/// Returns [`Violation::UnknownChannel`] if no channel state exists for the -/// given `channel_id`, or [`Violation::InvalidCounterpartySignature`] if the -/// signature is invalid for the holder's initial commitment transaction. -fn verify_funding_signed( - fs: &FundingSigned, - channel_states: &HashMap, -) -> Result<(), Violation> { - let state = channel_states - .get(&fs.channel_id) - .ok_or(Violation::UnknownChannel(fs.channel_id))?; - - state - .config - .verify_counterparty_signature(&state.commitment, &state.holder, &fs.signature) - .then_some(()) - .ok_or(Violation::InvalidCounterpartySignature(fs.channel_id)) -} - /// 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. /// @@ -1262,8 +1261,12 @@ fn verify_funding_signed( /// it is left untouched, preserving the first `open_channel`. Once a /// `funding_created` has been built, it is overwritten, allowing the /// `temporary_channel_id` to be reused for a new negotiation. +/// +/// Whenever a negotiation is recorded, its `first_per_commitment_point` is +/// added to `per_commitment_points` as revealed by us. fn record_send_open_channel( negotiations: &mut HashMap, + per_commitment_points: &mut HashSet, open_channel: &OpenChannel, ) { if negotiations @@ -1281,10 +1284,12 @@ fn record_send_open_channel( funding_built: false, }, ); + per_commitment_points.insert(open_channel.first_per_commitment_point); } /// Pairs a received `accept_channel` with the recorded `open_channel` of the -/// same `temporary_channel_id`. +/// same `temporary_channel_id`, and adds its `first_per_commitment_point` to +/// `per_commitment_points` as revealed by the target. /// /// # Panics /// @@ -1292,12 +1297,51 @@ fn record_send_open_channel( /// `AcceptChannelOracle` reports such messages as a [`Violation`]. fn record_recv_accept_channel( negotiations: &mut HashMap, + per_commitment_points: &mut HashSet, accept_channel: &AcceptChannel, ) { negotiations .get_mut(&accept_channel.temporary_channel_id) .expect("AcceptChannelOracle guaranteed this temporary_channel_id exists") .accept_channel = Some(accept_channel.clone()); + per_commitment_points.insert(accept_channel.first_per_commitment_point); +} + +/// Records that a `funding_signed` has been accepted for its channel. +/// +/// # Panics +/// +/// Panics if no matching channel exists. This should be unreachable, as +/// `FundingSignedOracle` reports such messages as a [`Violation`]. +fn record_recv_funding_signed( + channel_states: &mut HashMap, + funding_signed: &FundingSigned, +) { + channel_states + .get_mut(&funding_signed.channel_id) + .expect("FundingSignedOracle guaranteed this channel_id exists") + .funding_signed_received = true; +} + +/// Records a received `channel_ready`'s `second_per_commitment_point` as the +/// counterparty's next per-commitment point on the channel it identifies, and +/// adds it to `per_commitment_points` as revealed by the target. +/// +/// # Panics +/// +/// 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, + per_commitment_points: &mut HashSet, + channel_ready: &ChannelReady, +) { + 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); + per_commitment_points.insert(channel_ready.second_per_commitment_point); } /// 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 92bbc936..e55e6f71 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -422,6 +422,13 @@ fn execute_records_negotiation_for_open_and_accept() { let accept_channel = pending.accept_channel.as_ref().unwrap(); assert_eq!(accept_channel.clone(), sample_accept_channel()); assert!(!pending.funding_built); + assert_eq!( + *fx.per_commitment_points(), + HashSet::from([ + pending.open_channel.first_per_commitment_point, + accept_channel.first_per_commitment_point, + ]) + ); } #[test] @@ -484,9 +491,18 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() { ); b.append(Operation::RecvAcceptChannel, &[resent]); + // Use a fresh `first_per_commitment_point` so the resent `accept_channel` + // is otherwise valid, with only its `temporary_channel_id` reused before + // funding_created. + let accept_channel = sample_accept_channel(); + let resent_accept_channel = AcceptChannel { + first_per_commitment_point: sample_pubkey(8), + ..sample_accept_channel() + }; + let err = Fixture::new() - .queue(&Message::AcceptChannel(sample_accept_channel())) - .queue(&Message::AcceptChannel(sample_accept_channel())) + .queue(&Message::AcceptChannel(accept_channel)) + .queue(&Message::AcceptChannel(resent_accept_channel)) .run_err(&b.build()); let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { @@ -498,18 +514,57 @@ fn execute_recv_accept_channel_rejects_reuse_before_funding() { )); } +#[test] +fn execute_recv_accept_channel_rejects_reused_per_commitment_point() { + let temporary_channel_id = TemporaryChannelId::new([0xcc; 32]); + + // Negotiate a channel, then negotiate a second one on a different + // `temporary_channel_id`. + let mut b = ProgramBuilder::new(); + negotiate_channel(&mut b, &announced_open_channel()); + let mut second_open_channel = announced_open_channel(); + second_open_channel.message.temporary_channel_id = temporary_channel_id; + negotiate_channel(&mut b, &second_open_channel); + + // Use the first `accept_channel`'s `first_per_commitment_point` so the second + // `accept_channel` is otherwise valid, with only its + // `first_per_commitment_point` reused. + let earlier_point = sample_accept_channel().first_per_commitment_point; + let second_accept_channel = AcceptChannel { + temporary_channel_id, + ..sample_accept_channel() + }; + + let err = Fixture::new() + .queue(&Message::AcceptChannel(sample_accept_channel())) + .queue(&Message::AcceptChannel(second_accept_channel)) + .run_err(&b.build()); + + let ExecuteError::Violation(Violation::InvalidAcceptChannel(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, temporary_channel_id); + assert!(reason.contains(&format!( + "first_per_commitment_point {earlier_point} was reused from an earlier negotiation" + ))); +} + #[test] fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() { let temporary_channel_id = TemporaryChannelId::new([0xbb; 32]); // First open_channel: funding_satoshis = 100_000. - // Second open_channel: same temporary_channel_id, funding_satoshis = 200_000. + // Second open_channel: same temporary_channel_id, funding_satoshis = 200_000, + // and a fresh first_per_commitment_point. let mut b = ProgramBuilder::new(); let first = send_open_channel(&mut b, &announced_open_channel()); - // Override only funding_satoshis; reuse the first open_channel's other 19 inputs. + // Override only funding_satoshis and first_per_commitment_point; reuse the + // first open_channel's other 18 inputs. let mut second = first.vars; second.funding_satoshis = b.append(Operation::LoadAmount(200_000), &[]); + let sk = b.append(Operation::LoadPrivateKey([0x11; 32]), &[]); + second.first_per_commitment_point = b.append(Operation::DerivePoint, &[sk]); second.built = b.append(Operation::BuildOpenChannel, &second.build_inputs()); b.append(Operation::SendOpenChannel, &[second.built]); @@ -523,6 +578,18 @@ fn execute_records_only_first_open_channel_for_duplicate_id_before_funding() { assert_eq!(fx.sent::(1).funding_satoshis, 200_000); let pending = fx.negotiation(&temporary_channel_id); assert_eq!(pending.open_channel.funding_satoshis, 100_000); + + // The two `open_channel`s went out with different + // `first_per_commitment_point`s, but only the recorded negotiation's point + // counts as revealed by us. + assert_ne!( + fx.sent::(0).first_per_commitment_point, + fx.sent::(1).first_per_commitment_point, + ); + assert_eq!( + *fx.per_commitment_points(), + HashSet::from([fx.sent::(0).first_per_commitment_point]) + ); } #[test] @@ -542,6 +609,12 @@ fn execute_records_open_channel_for_duplicate_id_after_funding() { assert_eq!(pending.open_channel.funding_satoshis, 100_000); assert!(pending.accept_channel.is_none()); assert!(!pending.funding_built); + // The earlier negotiation was seeded rather than executed, so only the + // new `open_channel`'s point is recorded. + assert_eq!( + *fx.per_commitment_points(), + HashSet::from([pending.open_channel.first_per_commitment_point]) + ); } // -- Panic path tests -- @@ -861,12 +934,12 @@ fn execute_send_funding_created_uses_wire_funding_pubkey() { let mut b = ProgramBuilder::new(); let funding = create_funding_tx(&mut b); b.append(Operation::BroadcastTransaction, &[funding.tx]); - let funding_created = send_funding_created_with(&mut b, funding, funding.acceptor_privkey); - b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + send_funding_created_with(&mut b, funding, funding.acceptor_privkey); - // The acceptor's signature still verifies, because the config is built - // from the wire pubkeys rather than from the swapped privkey. - let mut fx = recv_funding_signed_fixture(); + // Signing with a key the peer did not negotiate marks the channel as + // having sent an invalid signature, so receiving a `funding_signed` would + // be a violation. We therefore stop after sending `funding_created`. + let mut fx = Fixture::new().with_negotiation(sample_funding_negotiation()); fx.run(&b.build()); let secp = Secp256k1::new(); @@ -1004,10 +1077,11 @@ fn execute_recv_funding_signed_unknown_channel() { .with_negotiation(sample_funding_negotiation()) .queue(&funding_signed_reply(channel_id)) .run_err(&send_funding_created_and_recv_funding_signed_program()); - assert!(matches!( - err, - ExecuteError::Violation(Violation::UnknownChannel(id)) if id == channel_id - )); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("unknown channel_id: no funding_created was sent for this channel")); } #[test] @@ -1021,10 +1095,53 @@ fn execute_recv_funding_signed_invalid_signature() { .expect("zero bytes parse as a signature"), })) .run_err(&send_funding_created_and_recv_funding_signed_program()); - assert!(matches!( - err, - ExecuteError::Violation(Violation::InvalidCounterpartySignature(id)) if id == channel_id - )); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("invalid funding_signed: signature is not valid")); +} + +#[test] +fn execute_recv_funding_signed_after_invalid_funding_created() { + let channel_id = funding_channel_id(); + + // 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`. + let mut b = ProgramBuilder::new(); + let funding = create_funding_tx(&mut b); + let funding_created = send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + b.append(Operation::RecvFundingSigned, &[funding_created.sent]); + + let err = recv_funding_signed_fixture().run_err(&b.build()); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("accepted invalid funding_created: signature is not valid")); +} + +#[test] +fn execute_recv_funding_signed_duplicate() { + let channel_id = funding_channel_id(); + + // Resend the same `funding_created`, which maps to the same channel, and + // receive a valid `funding_signed` for each. + let mut b = ProgramBuilder::new(); + let first = send_funding_created(&mut b); + b.append(Operation::RecvFundingSigned, &[first.sent]); + let second = send_funding_created_with(&mut b, first.tx, first.tx.opener_privkey); + b.append(Operation::RecvFundingSigned, &[second.sent]); + + let err = recv_funding_signed_fixture() + .queue(&funding_signed_reply(channel_id)) + .run_err(&b.build()); + let ExecuteError::Violation(Violation::InvalidFundingSigned(id, reason)) = &err else { + panic!("unexpected error: {err:?}"); + }; + assert_eq!(*id, channel_id); + assert!(reason.contains("duplicate funding_signed: channel already funded")); } #[test] @@ -1089,6 +1206,7 @@ fn execute_send_channel_ready() { *state.next_holder_per_commitment_point(), Some(expected_pcp1) ); + assert_eq!(*fx.per_commitment_points(), HashSet::from([expected_pcp1])); } #[test] @@ -1134,11 +1252,20 @@ fn execute_send_shutdown_empty_scriptpubkey() { #[test] fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { - // Corrupt the negotiated opener funding pubkey so the broadcast funding + // Corrupt the negotiated acceptor funding pubkey so the broadcast funding // transaction's output no longer pays the negotiated 2-of-2 script, // marking the funding outpoint invalid. + // + // We corrupt the acceptor's and not the opener's, since the latter would no + // longer match the resolved opener private key and so also set + // `sent_invalid_signature`, leaving the invalid funding outpoint gate + // unreached. let mut negotiation = sample_funding_negotiation(); - negotiation.open_channel.funding_pubkey = sample_pubkey(1); + negotiation + .accept_channel + .as_mut() + .expect("accept_channel must be present") + .funding_pubkey = sample_pubkey(1); // The corrupted pubkey changes the funding script, so our precomputed // funding_signed signature will no longer verify correctly. That @@ -1159,8 +1286,12 @@ fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(!state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); + assert!(fx.per_commitment_points().is_empty()); } #[test] @@ -1178,8 +1309,12 @@ fn execute_recv_channel_ready_below_minimum_depth_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); + assert!(fx.per_commitment_points().is_empty()); } #[test] @@ -1201,6 +1336,7 @@ fn execute_recv_channel_ready_at_minimum_depth_records_point() { Some(target_pcp) ); assert_eq!(fx.queued_len(), 0); + assert_eq!(*fx.per_commitment_points(), HashSet::from([target_pcp])); } #[test] @@ -1225,9 +1361,44 @@ fn execute_recv_channel_ready_funding_mined_prematurely_is_noop() { // The target's next per-commitment point is still unknown and the queued // `channel_ready` remains untouched. let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); assert!(state.was_funding_mined_prematurely); + assert!(!state.sent_invalid_signature); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); + assert!(fx.per_commitment_points().is_empty()); +} + +#[test] +fn execute_recv_channel_ready_invalid_signature_is_noop() { + let (mut fx, _) = recv_channel_ready_fixture(); + + 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`. + // + // A `funding_signed` answering a `funding_created` we signed with the wrong + // key is itself a violation, which `FundingSignedOracle` reports. So we + // don't receive one, letting `RecvChannelReady` be reached. + send_funding_created_with(&mut b, funding, funding.acceptor_privkey); + b.append(Operation::MineBlocks(8), &[]); + b.append(Operation::RecvChannelReady, &[]); + + // Having signed with the wrong key, the target does not owe us a + // `channel_ready`, so `RecvChannelReady` must be a no-op. + fx.run(&b.build()); + + // The target's next per-commitment point is still unknown and the queued + // `funding_signed` and `channel_ready` remain untouched. + let state = fx.channel_state(&funding_channel_id()); + assert!(state.is_funding_outpoint_valid); + assert!(!state.was_funding_mined_prematurely); + assert!(state.sent_invalid_signature); + assert!(state.next_counterparty_per_commitment_point().is_none()); + assert_eq!(fx.queued_len(), 2); } // -- extract_field tests -- @@ -1286,7 +1457,7 @@ fn extract_pubkeys() { let ac = sample_accept_channel(); assert_eq!( extract_field(&ac, AcceptChannelField::FundingPubkey), - Variable::Point(sample_pubkey(1)) + Variable::Point(sample_pubkey(7)) ); assert_eq!( extract_field(&ac, AcceptChannelField::RevocationBasepoint), diff --git a/smite-scenarios/src/executor/tests/harness.rs b/smite-scenarios/src/executor/tests/harness.rs index 64ca233a..9a9d4eb0 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, FromMessage, REGTEST_CHAIN_HASH}; use std::collections::VecDeque; use std::str::FromStr; @@ -215,6 +215,11 @@ impl Fixture { &self.executor.rpc } + /// Returns the per-commitment points revealed by either us or the target. + pub fn per_commitment_points(&self) -> &HashSet { + &self.executor.per_commitment_points + } + /// Returns the transactions held outside Bitcoin Core's mempool. pub fn private_mempool(&self) -> &[(Txid, String)] { &self.executor.private_mempool @@ -254,7 +259,7 @@ pub fn sample_pubkey(byte: u8) -> PublicKey { pub fn sample_context() -> ProgramContext { ProgramContext { target_pubkey: sample_pubkey(1), - chain_hash: [0xcc; 32], + chain_hash: REGTEST_CHAIN_HASH, block_height: 800_000, negotiated_features: Features::from_bits(&[ Features::OPTION_STATIC_REMOTEKEY, @@ -296,7 +301,7 @@ pub fn sample_accept_channel() -> AcceptChannel { minimum_depth: 6, to_self_delay: 144, max_accepted_htlcs: 483, - funding_pubkey: sample_pubkey(1), + funding_pubkey: sample_pubkey(7), revocation_basepoint: sample_pubkey(2), payment_basepoint: sample_pubkey(3), delayed_payment_basepoint: sample_pubkey(4), @@ -467,7 +472,7 @@ pub fn sample_funding_negotiation() -> PendingChannel { PendingChannel { open_channel: OpenChannel { - chain_hash: [0xcc; 32], + chain_hash: REGTEST_CHAIN_HASH, temporary_channel_id: TemporaryChannelId::new([0xbb; 32]), funding_satoshis: 10_000_000, push_msat: 3_000_000_000, diff --git a/smite-scenarios/src/scenarios.rs b/smite-scenarios/src/scenarios.rs index 48a43b3f..7ba62b3c 100644 --- a/smite-scenarios/src/scenarios.rs +++ b/smite-scenarios/src/scenarios.rs @@ -10,7 +10,7 @@ pub use encrypted_bytes::EncryptedBytesScenario; pub use init::InitScenario; pub use ir::IrScenario; pub use noise::NoiseScenario; -pub use setup::{PostInitSetup, REGTEST_CHAIN_HASH, SnapshotSetup}; +pub use setup::{PostInitSetup, SnapshotSetup}; use smite::scenarios::ScenarioError; use std::time::Duration; diff --git a/smite-scenarios/src/scenarios/setup.rs b/smite-scenarios/src/scenarios/setup.rs index c626279d..2cfe5773 100644 --- a/smite-scenarios/src/scenarios/setup.rs +++ b/smite-scenarios/src/scenarios/setup.rs @@ -2,7 +2,7 @@ use std::time::Duration; -use smite::bolt::{FeatureBit, Features, Init, InitTlvs, Message}; +use smite::bolt::{FeatureBit, Features, Init, InitTlvs, Message, REGTEST_CHAIN_HASH}; use smite::noise::NoiseConnection; use smite::scenarios::ScenarioError; @@ -10,12 +10,6 @@ use super::{handshake_with_target, ping_pong}; use crate::executor::ProgramContext; use crate::targets::{INITIAL_BLOCKS, Target}; -/// Bitcoin regtest genesis hash (in BOLT 2 network byte order). -pub const REGTEST_CHAIN_HASH: [u8; 32] = [ - 0x06, 0x22, 0x6e, 0x46, 0x11, 0x1a, 0x0b, 0x59, 0xca, 0xaf, 0x12, 0x60, 0x43, 0xeb, 0x5b, 0xbf, - 0x28, 0xc3, 0x4f, 0x3a, 0x5e, 0x33, 0x2a, 0x1f, 0xc7, 0xb2, 0xb7, 0x3c, 0xf1, 0x88, 0x91, 0x0f, -]; - const TIMEOUT: Duration = Duration::from_secs(5); /// Pre-snapshot setup that establishes a ready-to-use connection and produces diff --git a/smite/src/bolt.rs b/smite/src/bolt.rs index 978fa434..95d025eb 100644 --- a/smite/src/bolt.rs +++ b/smite/src/bolt.rs @@ -74,10 +74,10 @@ pub use tx_init_rbf::{TxInitRbf, TxInitRbfTlvs}; pub use tx_remove_input::TxRemoveInput; pub use tx_remove_output::TxRemoveOutput; pub use types::{ - BigSize, CHANNEL_ID_SIZE, COMPACT_SIGNATURE_SIZE, ChannelId, ChannelTypeVariant, - MAX_MESSAGE_SIZE, PAYMENT_ONION_PACKET_SIZE, PER_COMMITMENT_SECRET_SIZE, PUBLIC_KEY_SIZE, - SHA256_HASH_SIZE, SHORT_CHANNEL_ID_SIZE, ShortChannelId, TXID_SIZE, TemporaryChannelId, Tu32, - Tu64, + BigSize, CHAIN_HASH_SIZE, CHANNEL_ID_SIZE, COMPACT_SIGNATURE_SIZE, ChannelId, + ChannelTypeVariant, MAX_MESSAGE_SIZE, PAYMENT_ONION_PACKET_SIZE, PER_COMMITMENT_SECRET_SIZE, + PUBLIC_KEY_SIZE, REGTEST_CHAIN_HASH, SHA256_HASH_SIZE, SHORT_CHANNEL_ID_SIZE, ShortChannelId, + TXID_SIZE, TemporaryChannelId, Tu32, Tu64, }; pub use update_add_htlc::{UpdateAddHtlc, UpdateAddHtlcTlvs}; pub use update_fail_htlc::{UpdateFailHtlc, UpdateFailHtlcTlvs}; diff --git a/smite/src/bolt/accept_channel.rs b/smite/src/bolt/accept_channel.rs index 9fa072a4..baa2ce6f 100644 --- a/smite/src/bolt/accept_channel.rs +++ b/smite/src/bolt/accept_channel.rs @@ -60,6 +60,19 @@ pub struct AcceptChannelTlvs { } impl AcceptChannel { + /// Returns the channel acceptor's pubkeys, in wire order. + #[must_use] + pub fn pubkeys(&self) -> [PublicKey; 6] { + [ + self.funding_pubkey, + self.revocation_basepoint, + self.payment_basepoint, + self.delayed_payment_basepoint, + self.htlc_basepoint, + self.first_per_commitment_point, + ] + } + /// Encodes to wire format (without message type prefix). #[must_use] pub fn encode(&self) -> Vec { diff --git a/smite/src/bolt/open_channel.rs b/smite/src/bolt/open_channel.rs index 93820ff6..02fb17f3 100644 --- a/smite/src/bolt/open_channel.rs +++ b/smite/src/bolt/open_channel.rs @@ -67,6 +67,19 @@ pub struct OpenChannelTlvs { } impl OpenChannel { + /// Returns the channel initiator's pubkeys, in wire order. + #[must_use] + pub fn pubkeys(&self) -> [PublicKey; 6] { + [ + self.funding_pubkey, + self.revocation_basepoint, + self.payment_basepoint, + self.delayed_payment_basepoint, + self.htlc_basepoint, + self.first_per_commitment_point, + ] + } + /// Encodes to wire format (without message type prefix). #[must_use] pub fn encode(&self) -> Vec { diff --git a/smite/src/bolt/types.rs b/smite/src/bolt/types.rs index 5bdbb694..45e0ea6c 100644 --- a/smite/src/bolt/types.rs +++ b/smite/src/bolt/types.rs @@ -37,6 +37,12 @@ pub const PAYMENT_ONION_PACKET_SIZE: usize = 1366; /// Size of a per-commitment secret in bytes. pub const PER_COMMITMENT_SECRET_SIZE: usize = 32; +/// Bitcoin regtest genesis hash (in BOLT 2 network byte order). +pub const REGTEST_CHAIN_HASH: [u8; CHAIN_HASH_SIZE] = [ + 0x06, 0x22, 0x6e, 0x46, 0x11, 0x1a, 0x0b, 0x59, 0xca, 0xaf, 0x12, 0x60, 0x43, 0xeb, 0x5b, 0xbf, + 0x28, 0xc3, 0x4f, 0x3a, 0x5e, 0x33, 0x2a, 0x1f, 0xc7, 0xb2, 0xb7, 0x3c, 0xf1, 0x88, 0x91, 0x0f, +]; + /// A 32-byte channel identifier. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Hash)] pub struct ChannelId(pub [u8; CHANNEL_ID_SIZE]); diff --git a/smite/src/channel_tx/commitment.rs b/smite/src/channel_tx/commitment.rs index d4cfebec..3769a9e0 100644 --- a/smite/src/channel_tx/commitment.rs +++ b/smite/src/channel_tx/commitment.rs @@ -133,6 +133,7 @@ struct TxCreationKeys { /// State of a single channel, including its static configuration, holder /// identity, and current commitment state. +#[allow(clippy::struct_excessive_bools)] // Independent flags, not a state machine pub struct ChannelState { /// Channel configuration established at channel creation and unchanged /// for the lifetime of the channel. @@ -159,6 +160,15 @@ pub struct ChannelState { /// after the block height at which they receive `funding_created`, so they /// may never observe it and never send `channel_ready`. pub was_funding_mined_prematurely: bool, + /// Whether we have ever sent a signature the peer was required to reject, + /// such as a `funding_created`, `commitment_signed`, or HTLC signature etc. + /// it cannot verify. Set on the first occurrence and never cleared, since + /// BOLT 2 requires the peer to fail the channel or disconnect in response, + /// any subsequent positive response is therefore a violation. + pub sent_invalid_signature: bool, + /// 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, } impl Side { @@ -188,6 +198,7 @@ impl ChannelState { commitment: CommitmentState, is_funding_outpoint_valid: bool, was_funding_mined_prematurely: bool, + sent_invalid_signature: bool, ) -> Self { Self { config, @@ -197,6 +208,8 @@ impl ChannelState { acceptor_next_per_commitment_point: None, is_funding_outpoint_valid, was_funding_mined_prematurely, + sent_invalid_signature, + funding_signed_received: false, } } diff --git a/smite/src/oracles.rs b/smite/src/oracles.rs index ee2d7042..8a4e395f 100644 --- a/smite/src/oracles.rs +++ b/smite/src/oracles.rs @@ -3,9 +3,13 @@ //! 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 pub trait Oracle { diff --git a/smite/src/oracles/accept_channel.rs b/smite/src/oracles/accept_channel.rs index 54af06a1..df6dfad8 100644 --- a/smite/src/oracles/accept_channel.rs +++ b/smite/src/oracles/accept_channel.rs @@ -2,14 +2,18 @@ use super::Oracle; use crate::bolt::{ - AcceptChannel, ChannelTypeVariant, Features, OpenChannel, is_acceptable_shutdown_script, - is_standard_shutdown_script, + AcceptChannel, ChannelTypeVariant, Features, OpenChannel, REGTEST_CHAIN_HASH, + is_acceptable_shutdown_script, is_standard_shutdown_script, }; use crate::channel_tx::CommitmentCost; use crate::pending_channel::PendingChannel; use crate::violation::Violation; use bitcoin::Amount; +use bitcoin::hex::DisplayHex; +use bitcoin::secp256k1::PublicKey; + +use std::collections::HashSet; // Constants from the BOLT 2 `open_channel` and `accept_channel` requirements: // https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#requirements-8 @@ -22,6 +26,22 @@ const MIN_DUST_LIMIT_SATOSHIS: u64 = 354; // whether the initiator wishes to announce the channel publicly. const ANNOUNCE_CHANNEL_FLAG: u8 = 1; +// A high dust limit leaves too much of the channel unenforceable on-chain, +// so values above this are considered unreasonably large. +const MAX_DUST_LIMIT_SATOSHIS: u64 = 10_000; + +// Below Bitcoin Core's minimum relay feerate, commitment transactions may not +// be relayed, so values below this are considered unreasonably small. +const MIN_FEERATE_PER_KW: u32 = 253; + +// A long delay locks the receiver's funds for too long after a unilateral +// close, so values above this (~2 weeks) are considered unreasonably large. +const MAX_TO_SELF_DELAY: u16 = 2016; + +// A one-day delay gives a node time to detect and punish a revoked commitment, +// so values below this are considered unreasonably short. +const MIN_TO_SELF_DELAY: u16 = 144; + /// Context for `AcceptChannelOracle` pub struct AcceptChannelContext<'a> { /// The `accept_channel` received from the peer. @@ -31,6 +51,8 @@ pub struct AcceptChannelContext<'a> { pub negotiation: Option<&'a PendingChannel>, /// Features negotiated between the target node and Smite. pub negotiated_features: &'a Features, + /// Per-commitment points revealed by either us or the target. + pub per_commitment_points: &'a HashSet, } /// Checks whether the `open_channel` answered by an `accept_channel` satisfied @@ -69,6 +91,7 @@ impl Oracle> for AcceptChannelOracle { context.accept_channel, open_channel, context.negotiated_features, + context.per_commitment_points, ) { return Err(Violation::InvalidAcceptChannel( context.accept_channel.temporary_channel_id, @@ -99,6 +122,7 @@ impl Oracle> for AcceptChannelOracle { /// be less than or equal to the channel reserve. However, implementations /// such as LDK accept zero channel reserves on the receiving side, so we do /// not enforce this check on the target's receiving side. +#[allow(clippy::too_many_lines)] fn verify_accepted_open_channel( open_channel: &OpenChannel, negotiated_features: &Features, @@ -108,6 +132,18 @@ fn verify_accepted_open_channel( return Err("option_dual_fund has been negotiated".to_string()); } + // Check that the channel is opened on regtest. + // + // TODO: Take the chain hash in use from the scenario context once Smite + // fuzzes targets on other chains. + if open_channel.chain_hash != REGTEST_CHAIN_HASH { + return Err(format!( + "chain_hash {} is not the chain hash {} in use", + open_channel.chain_hash.as_hex(), + REGTEST_CHAIN_HASH.as_hex(), + )); + } + // Check that the funding amounts are valid. let max_funding = max_funding_satoshis(negotiated_features); if open_channel.funding_satoshis > max_funding { @@ -125,6 +161,14 @@ fn verify_accepted_open_channel( )); } + // Check that the channel reserve leaves a spendable balance. + if open_channel.channel_reserve_satoshis >= open_channel.funding_satoshis { + return Err(format!( + "channel_reserve_satoshis {} is not below funding_satoshis {}", + open_channel.channel_reserve_satoshis, open_channel.funding_satoshis, + )); + } + // Check that the upfront shutdown script is present and valid when negotiated. if negotiated_features.supports_feature(Features::OPTION_UPFRONT_SHUTDOWN_SCRIPT) { let Some(script) = &open_channel.tlvs.upfront_shutdown_script else { @@ -158,16 +202,30 @@ fn verify_accepted_open_channel( return Err("channel_type is not a known variant".to_string()); } - // Check that feerate_per_kw is 0 when `zero_fee_commitments` is negotiated. - if channel_type.supports_feature(Features::ZERO_FEE_COMMITMENTS) - && open_channel.feerate_per_kw != 0 - { + // Check that feerate_per_kw is 0 when `zero_fee_commitments` is negotiated, + // and that it pays for relay otherwise. + if channel_type.supports_feature(Features::ZERO_FEE_COMMITMENTS) { + if open_channel.feerate_per_kw != 0 { + return Err(format!( + "zero_fee_commitments requires feerate_per_kw to be 0, but got {}", + open_channel.feerate_per_kw, + )); + } + } else if open_channel.feerate_per_kw < MIN_FEERATE_PER_KW { return Err(format!( - "zero_fee_commitments requires feerate_per_kw to be 0, but got {}", + "feerate_per_kw {} is below the minimum of {MIN_FEERATE_PER_KW}", open_channel.feerate_per_kw, )); } + // Check that to_self_delay is not unreasonably large. + if open_channel.to_self_delay > MAX_TO_SELF_DELAY { + return Err(format!( + "to_self_delay {} exceeds the maximum of {MAX_TO_SELF_DELAY} blocks", + open_channel.to_self_delay, + )); + } + // Check that option_scid_alias is only negotiated for private channels. let announce_channel = open_channel.channel_flags & ANNOUNCE_CHANNEL_FLAG != 0; if announce_channel && channel_type.supports_feature(Features::OPTION_SCID_ALIAS) { @@ -183,13 +241,19 @@ fn verify_accepted_open_channel( )); } - // Check the dust limit is not below the minimum. + // Check the dust limit is within the acceptable range. if open_channel.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS { return Err(format!( "dust_limit_satoshis {} is below the minimum of {MIN_DUST_LIMIT_SATOSHIS} sat", open_channel.dust_limit_satoshis, )); } + if open_channel.dust_limit_satoshis > MAX_DUST_LIMIT_SATOSHIS { + return Err(format!( + "dust_limit_satoshis {} exceeds the maximum of {MAX_DUST_LIMIT_SATOSHIS} sat", + open_channel.dust_limit_satoshis, + )); + } // Check the initial commitment satisfies the channel reserve. verify_initial_commitment( @@ -208,10 +272,12 @@ fn verify_accepted_open_channel( /// requires the dust limit to be less than or equal to the channel reserve. /// However, implementations such as LDK accept zero channel reserves on the /// receiving side, so we enforce this only on the target's sending side. +#[allow(clippy::too_many_lines)] fn verify_accept_channel( accept_channel: &AcceptChannel, open_channel: &OpenChannel, negotiated_features: &Features, + per_commitment_points: &HashSet, ) -> Result<(), String> { // Check that the upfront shutdown script is present and valid when negotiated. if negotiated_features.supports_feature(Features::OPTION_UPFRONT_SHUTDOWN_SCRIPT) { @@ -270,7 +336,7 @@ fn verify_accept_channel( )); } - // Check the HTLC limit is within the maximum. + // Check the HTLC limit is within the acceptable range. let htlc_limit = max_accepted_htlcs_limit(&channel_type); if accept_channel.max_accepted_htlcs > htlc_limit { return Err(format!( @@ -278,14 +344,64 @@ fn verify_accept_channel( accept_channel.max_accepted_htlcs, )); } + if accept_channel.max_accepted_htlcs == 0 { + return Err("max_accepted_htlcs 0 leaves the channel unable to carry HTLCs".to_string()); + } - // Check the dust limit is not below the minimum. + // Check the dust limit is within the acceptable range. if accept_channel.dust_limit_satoshis < MIN_DUST_LIMIT_SATOSHIS { return Err(format!( "dust_limit_satoshis {} is below the minimum of {MIN_DUST_LIMIT_SATOSHIS} sat", accept_channel.dust_limit_satoshis, )); } + if accept_channel.dust_limit_satoshis > MAX_DUST_LIMIT_SATOSHIS { + return Err(format!( + "dust_limit_satoshis {} exceeds the maximum of {MAX_DUST_LIMIT_SATOSHIS} sat", + accept_channel.dust_limit_satoshis, + )); + } + + // Check the minimum HTLC is within the in-flight limit and the channel capacity. + if accept_channel.htlc_minimum_msat > accept_channel.max_htlc_value_in_flight_msat { + return Err(format!( + "htlc_minimum_msat {} exceeds max_htlc_value_in_flight_msat {}", + accept_channel.htlc_minimum_msat, accept_channel.max_htlc_value_in_flight_msat, + )); + } + let funding_msat = open_channel.funding_satoshis * 1000; + if accept_channel.htlc_minimum_msat > funding_msat { + return Err(format!( + "htlc_minimum_msat {} exceeds the open_channel funding amount {funding_msat} msat", + accept_channel.htlc_minimum_msat, + )); + } + + // Check the acceptor gives itself time to punish a revoked commitment. + if accept_channel.to_self_delay < MIN_TO_SELF_DELAY { + return Err(format!( + "to_self_delay {} is below the minimum of {MIN_TO_SELF_DELAY} blocks", + accept_channel.to_self_delay, + )); + } + + // Check that the acceptor's pubkeys are distinct from the opener's and each other. + let mut pubkeys = HashSet::from(open_channel.pubkeys()); + if !accept_channel + .pubkeys() + .into_iter() + .all(|pubkey| pubkeys.insert(pubkey)) + { + return Err("accept_channel reuses a pubkey from the negotiation".to_string()); + } + + // Check that the first_per_commitment_point is not reused from an earlier negotiation. + if per_commitment_points.contains(&accept_channel.first_per_commitment_point) { + return Err(format!( + "first_per_commitment_point {} was reused from an earlier negotiation", + accept_channel.first_per_commitment_point, + )); + } // Check the initial commitment satisfies the channel reserve. verify_initial_commitment( @@ -368,7 +484,7 @@ fn max_accepted_htlcs_limit(channel_type: &Features) -> u16 { #[cfg(test)] mod tests { use super::*; - use crate::bolt::{AcceptChannelTlvs, OpenChannelTlvs, TemporaryChannelId}; + use crate::bolt::{AcceptChannelTlvs, CHAIN_HASH_SIZE, OpenChannelTlvs, TemporaryChannelId}; use bitcoin::hashes::Hash; use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; use bitcoin::{PubkeyHash, ScriptBuf, WPubkeyHash}; @@ -382,7 +498,7 @@ mod tests { fn open_channel() -> OpenChannel { let key = pubkey(1); OpenChannel { - chain_hash: [0u8; 32], + chain_hash: REGTEST_CHAIN_HASH, temporary_channel_id: TemporaryChannelId::new([1u8; 32]), funding_satoshis: 10_000_000, push_msat: 3_000_000_000, @@ -409,7 +525,6 @@ mod tests { /// Valid `accept_channel` message for testing. fn accept_channel() -> AcceptChannel { - let key = pubkey(2); AcceptChannel { temporary_channel_id: TemporaryChannelId::new([1u8; 32]), dust_limit_satoshis: 546, @@ -419,12 +534,12 @@ mod tests { minimum_depth: 6, to_self_delay: 144, max_accepted_htlcs: 483, - funding_pubkey: key, - revocation_basepoint: key, - payment_basepoint: key, - delayed_payment_basepoint: key, - htlc_basepoint: key, - first_per_commitment_point: key, + funding_pubkey: pubkey(2), + revocation_basepoint: pubkey(3), + payment_basepoint: pubkey(4), + delayed_payment_basepoint: pubkey(5), + htlc_basepoint: pubkey(6), + first_per_commitment_point: pubkey(7), tlvs: AcceptChannelTlvs { upfront_shutdown_script: None, channel_type: Some(vec![0x10, 0x00]), @@ -464,6 +579,7 @@ mod tests { accept_channel, negotiation, negotiated_features, + per_commitment_points: &HashSet::new(), }) { panic!("expected pass, got: {err}"); } @@ -480,6 +596,7 @@ mod tests { accept_channel, negotiation, negotiated_features, + per_commitment_points: &HashSet::new(), }) { Err(Violation::InvalidAcceptChannel(chan_id, reason)) => { assert_eq!(accept_channel.temporary_channel_id, chan_id); @@ -571,6 +688,19 @@ mod tests { ); } + #[test] + fn open_channel_for_another_chain() { + let mut oc = open_channel(); + oc.chain_hash = [0xaa; CHAIN_HASH_SIZE]; + + assert_fail( + &accept_channel(), + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid open_channel: chain_hash aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa is not the chain hash", + ); + } + #[test] fn funding_satoshis_above_non_wumbo_limit_without_option_support_large_channel() { let mut oc = open_channel(); @@ -613,6 +743,19 @@ mod tests { ); } + #[test] + fn open_channel_channel_reserve_not_below_the_funding_amount() { + let mut oc = open_channel(); + oc.channel_reserve_satoshis = oc.funding_satoshis; + + assert_fail( + &accept_channel(), + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid open_channel: channel_reserve_satoshis 10000000 is not below funding_satoshis 10000000", + ); + } + #[test] fn open_channel_invalid_upfront_shutdown_script() { let mut oc = open_channel(); @@ -704,6 +847,32 @@ mod tests { ); } + #[test] + fn open_channel_feerate_below_the_minimum() { + let mut oc = open_channel(); + oc.feerate_per_kw = MIN_FEERATE_PER_KW - 1; + + assert_fail( + &accept_channel(), + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid open_channel: feerate_per_kw 252 is below the minimum of 253", + ); + } + + #[test] + fn open_channel_to_self_delay_above_the_maximum() { + let mut oc = open_channel(); + oc.to_self_delay = MAX_TO_SELF_DELAY + 1; + + assert_fail( + &accept_channel(), + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid open_channel: to_self_delay 2017 exceeds the maximum of 2016 blocks", + ); + } + #[test] fn open_channel_option_scid_alias_for_public_channel() { let mut oc = open_channel(); @@ -759,6 +928,19 @@ mod tests { ); } + #[test] + fn open_channel_dust_limit_above_the_maximum() { + let mut oc = open_channel(); + oc.dust_limit_satoshis = MAX_DUST_LIMIT_SATOSHIS + 1; + + assert_fail( + &accept_channel(), + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid open_channel: dust_limit_satoshis 10001 exceeds the maximum of 10000 sat", + ); + } + #[test] fn opener_cannot_afford_commitment_fee() { let mut oc = open_channel(); @@ -949,6 +1131,19 @@ mod tests { ); } + #[test] + fn accept_channel_max_accepted_htlcs_of_zero() { + let mut ac = accept_channel(); + ac.max_accepted_htlcs = 0; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + "invalid accept_channel: max_accepted_htlcs 0 leaves the channel unable to carry HTLCs", + ); + } + #[test] fn accept_channel_dust_limit_below_the_minimum() { let mut ac = accept_channel(); @@ -962,6 +1157,113 @@ mod tests { ); } + #[test] + fn accept_channel_dust_limit_above_the_maximum() { + let mut ac = accept_channel(); + ac.dust_limit_satoshis = MAX_DUST_LIMIT_SATOSHIS + 1; + ac.channel_reserve_satoshis = ac.dust_limit_satoshis; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + "invalid accept_channel: dust_limit_satoshis 10001 exceeds the maximum of 10000 sat", + ); + } + + #[test] + fn accept_channel_htlc_minimum_above_the_in_flight_limit() { + let mut ac = accept_channel(); + ac.htlc_minimum_msat = ac.max_htlc_value_in_flight_msat + 1; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + "invalid accept_channel: htlc_minimum_msat 100000001 exceeds max_htlc_value_in_flight_msat 100000000", + ); + } + + #[test] + fn accept_channel_htlc_minimum_above_the_funding_amount() { + let oc = open_channel(); + let mut ac = accept_channel(); + ac.htlc_minimum_msat = oc.funding_satoshis * 1000 + 1; + ac.max_htlc_value_in_flight_msat = ac.htlc_minimum_msat; + + assert_fail( + &ac, + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid accept_channel: htlc_minimum_msat 10000000001 exceeds the open_channel funding amount 10000000000 msat", + ); + } + + #[test] + fn accept_channel_to_self_delay_below_the_minimum() { + let mut ac = accept_channel(); + ac.to_self_delay = MIN_TO_SELF_DELAY - 1; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + "invalid accept_channel: to_self_delay 143 is below the minimum of 144 blocks", + ); + } + + #[test] + fn accept_channel_reuses_an_open_channel_pubkey() { + let oc = open_channel(); + let mut ac = accept_channel(); + ac.htlc_basepoint = oc.revocation_basepoint; + + assert_fail( + &ac, + Some(&pending_negotiation(oc)), + &sample_negotiated_features(), + "invalid accept_channel: accept_channel reuses a pubkey from the negotiation", + ); + } + + #[test] + fn accept_channel_reuses_its_own_pubkey() { + let mut ac = accept_channel(); + ac.htlc_basepoint = ac.funding_pubkey; + + assert_fail( + &ac, + Some(&pending_negotiation(open_channel())), + &sample_negotiated_features(), + "invalid accept_channel: accept_channel reuses a pubkey from the negotiation", + ); + } + + #[test] + fn accept_channel_reuses_per_commitment_point_from_earlier_negotiation() { + let ac = accept_channel(); + let per_commitment_points = HashSet::from([ac.first_per_commitment_point]); + + match AcceptChannelOracle.evaluate(&AcceptChannelContext { + accept_channel: &ac, + negotiation: Some(&pending_negotiation(open_channel())), + negotiated_features: &sample_negotiated_features(), + per_commitment_points: &per_commitment_points, + }) { + Err(Violation::InvalidAcceptChannel(chan_id, reason)) => { + assert_eq!(ac.temporary_channel_id, chan_id); + assert!( + reason.contains(&format!( + "invalid accept_channel: first_per_commitment_point {} was reused from an earlier negotiation", + ac.first_per_commitment_point, + )), + "unexpected failure reason: {reason}" + ); + } + _ => panic!("expected a per-commitment point reuse violation"), + } + } + #[test] fn accept_channel_initial_commitment_below_reserves() { let mut ac = accept_channel(); diff --git a/smite/src/oracles/channel_ready.rs b/smite/src/oracles/channel_ready.rs new file mode 100644 index 00000000..fbe48479 --- /dev/null +++ b/smite/src/oracles/channel_ready.rs @@ -0,0 +1,269 @@ +//! BOLT 2 `channel_ready` oracle, for the v1 outbound channel funding flow. + +use super::Oracle; +use crate::bolt::{ChannelReady, Features}; +use crate::channel_tx::ChannelState; +use crate::violation::Violation; + +use bitcoin::secp256k1::PublicKey; + +use std::collections::HashSet; + +/// 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>, + /// Features negotiated between the target node and Smite. + pub negotiated_features: &'a Features, + /// Per-commitment points revealed by either us or the target. + pub per_commitment_points: &'a HashSet, +} + +/// Checks whether a received `channel_ready` satisfies the BOLT 2 v1 channel +/// establishment requirements. +/// +/// # Deferred oracle checks +/// +/// - `short_channel_id` alias collisions: BOLT 2 requires aliases not to collide +/// with any of the target's real `short_channel_ids`. Checking this requires +/// fetching the `short_channel_id` for all channels we have with the target +/// over RPC. Bulk fetching adds RPC overhead that reduces fuzzing throughput, +/// while lazy lookups can still miss collisions. Until this can be checked +/// more efficiently, it is not worthwhile for this narrow surface. +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(), + )); + } + + // 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 an + // earlier negotiation. + if context + .per_commitment_points + .contains(&context.channel_ready.second_per_commitment_point) + { + return Err(Violation::InvalidChannelReady( + context.channel_ready.channel_id, + format!( + "second_per_commitment_point {} was reused from an earlier negotiation", + context.channel_ready.second_per_commitment_point, + ), + )); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bolt::{ChannelId, ChannelReadyTlvs, Features, ShortChannelId}; + use crate::channel_tx::{ + ChannelConfig, ChannelPartyConfig, CommitmentPartyState, CommitmentState, HolderIdentity, + Side, + }; + use bitcoin::OutPoint; + 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_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 key = pubkey(1); + let party = || ChannelPartyConfig { + funding_pubkey: key, + payment_basepoint: key, + revocation_basepoint: key, + delayed_payment_basepoint: key, + dust_limit_satoshis: 546, + to_self_delay: 144, + }; + + ChannelState::new( + ChannelConfig { + funding_outpoint: OutPoint::null(), + funding_satoshis: 10_000_000, + channel_type: Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), + opener: party(), + acceptor: party(), + minimum_depth: 6, + }, + HolderIdentity { + side: Side::Opener, + funding_privkey: secret_key(1), + }, + CommitmentState { + commitment_number: 0, + feerate_per_kw: 15_000, + opener: CommitmentPartyState { + per_commitment_point: key, + balance_msat: 7_000_000_000, + }, + acceptor: CommitmentPartyState { + per_commitment_point: key, + balance_msat: 3_000_000_000, + }, + }, + true, + false, + false, + ) + } + + #[track_caller] + fn assert_pass( + channel_ready: &ChannelReady, + channel: Option<&ChannelState>, + negotiated_features: &Features, + per_commitment_points: &HashSet, + ) { + if let Err(err) = ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready, + channel, + negotiated_features, + per_commitment_points, + }) { + panic!("expected pass, got: {err}"); + } + } + + #[track_caller] + fn assert_fail( + channel_ready: &ChannelReady, + channel: Option<&ChannelState>, + negotiated_features: &Features, + per_commitment_points: &HashSet, + expected: &str, + ) { + match ChannelReadyOracle.evaluate(&ChannelReadyContext { + channel_ready, + channel, + negotiated_features, + per_commitment_points, + }) { + 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()), + &Features::new(), + &HashSet::new(), + ); + } + + #[test] + fn unknown_channel_id_fails() { + assert_fail( + &channel_ready(), + None, + &Features::new(), + &HashSet::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, + &HashSet::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(), + &HashSet::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, + &HashSet::new(), + "option_scid_alias negotiated but short_channel_id alias is missing", + ); + } + + #[test] + fn channel_ready_reuses_per_commitment_point_from_earlier_negotiation() { + let cr = channel_ready(); + let per_commitment_points = HashSet::from([cr.second_per_commitment_point]); + + assert_fail( + &cr, + Some(&channel_state()), + &Features::new(), + &per_commitment_points, + &format!( + "second_per_commitment_point {} was reused from an earlier negotiation", + cr.second_per_commitment_point, + ), + ); + } +} diff --git a/smite/src/oracles/funding_signed.rs b/smite/src/oracles/funding_signed.rs new file mode 100644 index 00000000..55550cd0 --- /dev/null +++ b/smite/src/oracles/funding_signed.rs @@ -0,0 +1,233 @@ +//! BOLT 2 `funding_signed` oracle, for the v1 outbound channel funding flow. + +use super::Oracle; +use crate::bolt::FundingSigned; +use crate::channel_tx::ChannelState; +use crate::violation::Violation; + +/// Context for `FundingSignedOracle` +pub struct FundingSignedContext<'a> { + /// The `funding_signed` received from the peer. + pub funding_signed: &'a FundingSigned, + /// The channel the `funding_signed` belongs to, identified by its + /// `channel_id`, or `None` if no matching `funding_created` was sent. + pub channel: Option<&'a ChannelState>, +} + +/// Checks whether the `funding_created` answered by a `funding_signed` satisfied +/// the BOLT 2 v1 channel establishment requirements for acceptance, and whether +/// the `funding_signed` itself satisfies them. +pub struct FundingSignedOracle; + +impl Oracle> for FundingSignedOracle { + fn evaluate(&self, context: &FundingSignedContext<'_>) -> Result<(), Violation> { + // Check that the `funding_signed` answers a known `funding_created`. + let Some(channel) = context.channel else { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "unknown channel_id: no funding_created was sent for this channel".to_string(), + )); + }; + + // Check that this is the first `funding_signed` for this channel. + // + // NOTE: Besides a genuine resend, this also catches a valid + // `funding_signed` for a new channel whose channel_id collides with an + // existing one due to the XOR relationship between channel_id and the + // funding outpoint. BOLT 2 does not specify how to handle this, but + // LND, LDK, and Eclair all reject such channels. Since channel_id is + // reused across messages, accepting a collision could cause + // cross-channel state contamination and lead to more serious bugs. Such + // collisions are also extremely unlikely to occur naturally, so the + // target should reject them. + // see: https://github.com/ElementsProject/lightning/issues/9274#issuecomment-5316110622 + if channel.funding_signed_received { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "duplicate funding_signed: channel already funded".to_string(), + )); + } + + // Check whether we sent a valid signature during `funding_created`. + if channel.sent_invalid_signature { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "accepted invalid funding_created: signature is not valid".to_string(), + )); + } + + // Check that the `funding_signed` itself is valid. + // + // NOTE: If the colliding channel never received its own + // `funding_signed`, a channel_id collision (see above) surfaces here as + // an invalid signature instead. + if !channel.config.verify_counterparty_signature( + &channel.commitment, + &channel.holder, + &context.funding_signed.signature, + ) { + return Err(Violation::InvalidFundingSigned( + context.funding_signed.channel_id, + "invalid funding_signed: signature is not valid".to_string(), + )); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::bolt::{COMPACT_SIGNATURE_SIZE, ChannelId, Features}; + use crate::channel_tx::{ChannelConfig, ChannelPartyConfig, HolderIdentity, Side}; + use bitcoin::OutPoint; + use bitcoin::secp256k1::ecdsa::Signature; + 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) + } + + /// Valid `funding_signed` message for testing. + fn funding_signed(channel: &ChannelState) -> FundingSigned { + let acceptor = HolderIdentity { + side: Side::Acceptor, + funding_privkey: secret_key(2), + }; + FundingSigned { + channel_id: ChannelId::v1_from_funding_outpoint(channel.config.funding_outpoint), + signature: channel + .config + .sign_counterparty_commitment(&channel.commitment, &acceptor), + } + } + + #[track_caller] + fn assert_pass(funding_signed: &FundingSigned, channel: Option<&ChannelState>) { + if let Err(err) = FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed, + channel, + }) { + panic!("expected pass, got: {err}"); + } + } + + #[track_caller] + fn assert_fail(funding_signed: &FundingSigned, channel: Option<&ChannelState>, expected: &str) { + match FundingSignedOracle.evaluate(&FundingSignedContext { + funding_signed, + channel, + }) { + Err(Violation::InvalidFundingSigned(chan_id, reason)) => { + assert_eq!(funding_signed.channel_id, chan_id); + assert!( + reason.contains(expected), + "unexpected failure reason: {reason}" + ); + } + _ => panic!("expected failure: {expected}"), + } + } + + #[test] + fn conforming_funding_signed_passes() { + let channel = channel_state(); + + assert_pass(&funding_signed(&channel), Some(&channel)); + } + + #[test] + fn funding_signed_for_unknown_channel_id() { + assert_fail( + &funding_signed(&channel_state()), + None, + "unknown channel_id: no funding_created was sent for this channel", + ); + } + + #[test] + fn duplicate_funding_signed() { + let mut channel = channel_state(); + channel.funding_signed_received = true; + + assert_fail( + &funding_signed(&channel), + Some(&channel), + "duplicate funding_signed: channel already funded", + ); + } + + #[test] + fn funding_created_with_invalid_signature() { + let mut channel = channel_state(); + channel.sent_invalid_signature = true; + + assert_fail( + &funding_signed(&channel), + Some(&channel), + "accepted invalid funding_created: signature is not valid", + ); + } + + #[test] + fn funding_signed_with_invalid_signature() { + let channel = channel_state(); + let mut fs = funding_signed(&channel); + fs.signature = Signature::from_compact(&[0u8; COMPACT_SIGNATURE_SIZE]) + .expect("zero bytes parse as a signature"); + + assert_fail( + &fs, + Some(&channel), + "invalid funding_signed: signature is not valid", + ); + } +} diff --git a/smite/src/violation.rs b/smite/src/violation.rs index 515c9b52..3f0b9499 100644 --- a/smite/src/violation.rs +++ b/smite/src/violation.rs @@ -36,13 +36,22 @@ pub enum Violation { #[error("invalid accept_channel for temporary_channel_id {0}: {1}")] InvalidAcceptChannel(TemporaryChannelId, String), - /// The target sent a `funding_signed` or `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 `funding_signed` broke a BOLT 2 requirement, as judged by + /// [`crate::oracles::FundingSignedOracle`]. The reason names the breached + /// requirement, one of: + /// - it names a `channel_id` we sent no `funding_created` for, + /// - it names a `channel_id` that already received a `funding_signed`, + /// - it answers a `funding_created` with an invalid signature, or + /// - its signature is not valid for the holder's commitment transaction. + #[error("invalid funding_signed for channel_id {0}: {1}")] + InvalidFundingSigned(ChannelId, String), - /// The target's `funding_signed` signature failed to verify against the - /// holder's initial commitment transaction. - #[error("invalid counterparty signature for channel_id {0}")] - InvalidCounterpartySignature(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, + /// - it omits the `short_channel_id` alias `option_scid_alias` requires, or + /// - it reuses a per-commitment point from an earlier negotiation. + #[error("invalid channel_ready for channel_id {0}: {1}")] + InvalidChannelReady(ChannelId, String), }