diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index 28fb7bc6..60412054 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -251,6 +251,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 +280,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 +438,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 +477,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 +514,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)) } @@ -526,7 +540,11 @@ impl Executor { 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)?; + recv_channel_ready( + &mut self.conn, + &mut self.channel_states, + &mut self.per_commitment_points, + )?; log::debug!("[{:?}] RecvChannelReady: received", start.elapsed()); } None @@ -905,6 +923,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 +935,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); } } @@ -1189,7 +1211,8 @@ 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. +/// per-commitment point on the channel it identifies, and added to +/// `per_commitment_points` as revealed by the target. /// /// # Errors /// @@ -1199,6 +1222,7 @@ fn recv_bolt( fn recv_channel_ready( conn: &mut impl Connection, channel_states: &mut HashMap, + per_commitment_points: &mut HashSet, ) -> Result<(), ExecuteError> { let cr: ChannelReady = recv_bolt(conn, RECV_CHANNEL_READY_TIMEOUT)?; @@ -1207,6 +1231,8 @@ fn recv_channel_ready( .ok_or(Violation::UnknownChannel(cr.channel_id))?; *state.next_counterparty_per_commitment_point_mut() = Some(cr.second_per_commitment_point); + per_commitment_points.insert(cr.second_per_commitment_point); + Ok(()) } @@ -1262,8 +1288,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 +1311,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 +1324,14 @@ 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); } /// 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..d8c40ab7 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 -- @@ -1089,6 +1162,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] @@ -1161,6 +1235,7 @@ fn execute_recv_channel_ready_invalid_funding_outpoint_is_noop() { let state = fx.channel_state(&funding_channel_id()); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); + assert!(fx.per_commitment_points().is_empty()); } #[test] @@ -1180,6 +1255,7 @@ fn execute_recv_channel_ready_below_minimum_depth_is_noop() { let state = fx.channel_state(&funding_channel_id()); 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 +1277,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] @@ -1228,6 +1305,7 @@ fn execute_recv_channel_ready_funding_mined_prematurely_is_noop() { assert!(state.was_funding_mined_prematurely); assert!(state.next_counterparty_per_commitment_point().is_none()); assert_eq!(fx.queued_len(), 1); + assert!(fx.per_commitment_points().is_empty()); } // -- extract_field tests -- @@ -1286,7 +1364,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/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();