diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs index bf6f6b90..5454f998 100644 --- a/smite-scenarios/src/executor.rs +++ b/smite-scenarios/src/executor.rs @@ -846,7 +846,7 @@ fn build_funding_created( minimum_depth: accept_channel.minimum_depth, }; - let state = config.new_initial_commitment( + let commitments = config.new_initial_commitments( open_channel.push_msat, open_channel.feerate_per_kw, open_channel.first_per_commitment_point, @@ -856,7 +856,7 @@ fn build_funding_created( side: Side::Opener, funding_privkey: opener_funding_privkey, }; - let signature = config.sign_counterparty_commitment(&state, &holder); + let signature = config.sign_counterparty_commitment(&commitments, &holder); // 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 @@ -889,7 +889,7 @@ fn build_funding_created( ChannelState::new( config, holder, - state, + commitments, is_funding_outpoint_valid, mined_txids.contains(&funding_outpoint.txid), sent_invalid_signature, @@ -926,12 +926,12 @@ fn build_channel_ready( // Record the holder's next per-commitment point from the first locally-sent // `channel_ready`'s `second_per_commitment_point`. We only do so when the - // channel is tracked, the commitment number is still 0, and the point is not - // 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. + // channel is tracked, the holder's commitment number is still 0, and the + // point is not 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. if let Some(state) = channel_states.get_mut(&channel_id) - && state.commitment.commitment_number == 0 + && state.holder_commitment_state().commitment_number == 0 { let next_point = state.next_holder_per_commitment_point_mut(); if next_point.is_none() { @@ -1226,18 +1226,19 @@ fn recv_channel_ready( /// 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`, 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`). +/// A `channel_ready` is expected when a tracked channel's counterparty +/// commitment 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`, 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, ) -> bool { channel_states.values().any(|state| { - state.commitment.commitment_number == 0 + state.counterparty_commitment_state().commitment_number == 0 && state.next_counterparty_per_commitment_point().is_none() && state.is_funding_outpoint_valid && !state.was_funding_mined_prematurely diff --git a/smite-scenarios/src/executor/tests.rs b/smite-scenarios/src/executor/tests.rs index 3f030aa4..6d548b9b 100644 --- a/smite-scenarios/src/executor/tests.rs +++ b/smite-scenarios/src/executor/tests.rs @@ -788,7 +788,7 @@ fn execute_send_funding_created_and_recv_funding_signed() { assert!( state .config - .verify_counterparty_signature(&state.commitment, &holder, &fc.signature) + .verify_counterparty_signature(&state.commitments, &holder, &fc.signature) ); let pending = fx.negotiation(&TemporaryChannelId::new([0xbb; 32])); diff --git a/smite/src/channel_tx.rs b/smite/src/channel_tx.rs index 3e646154..fc8b89fe 100644 --- a/smite/src/channel_tx.rs +++ b/smite/src/channel_tx.rs @@ -7,7 +7,7 @@ mod commitment; mod funding; pub use commitment::{ - ChannelConfig, ChannelPartyConfig, ChannelState, CommitmentCost, CommitmentError, - CommitmentPartyState, CommitmentState, HolderIdentity, Side, + ChannelCommitments, ChannelConfig, ChannelPartyConfig, ChannelState, CommitmentCost, + CommitmentError, CommitmentState, HolderIdentity, Side, }; pub use funding::{FundingTransaction, InsufficientFunds, build_funding_transaction}; diff --git a/smite/src/channel_tx/commitment.rs b/smite/src/channel_tx/commitment.rs index c5f8802b..abbdce20 100644 --- a/smite/src/channel_tx/commitment.rs +++ b/smite/src/channel_tx/commitment.rs @@ -86,32 +86,41 @@ pub struct ChannelConfig { pub minimum_depth: u32, } -/// Per-party parameters used in a commitment transaction. -pub struct CommitmentPartyState { - /// Per-commitment point used to derive all commitment-specific keys. - pub per_commitment_point: PublicKey, - - /// Amount allocated to this party in millisatoshis. - /// Represents the balance before subtraction of fees, and anchors outputs. - /// In-flight HTLCs are represented as separate outputs in the commitment - /// transaction, so those values are already deducted from these balance values. - pub balance_msat: u64, -} - -/// Parameters for building a commitment transaction. +/// State of `local_side`'s commitment transaction. +/// +/// Each side has its own commitment state, so the commitment number, fee rate, +/// and balances may differ until both sides have caught up. pub struct CommitmentState { + /// Side whose commitment transaction this state is for. + pub local_side: Side, /// The commitment transaction number. pub commitment_number: u64, /// Fee rate for the commitment transaction. pub feerate_per_kw: u32, - /// Parameters for the channel opener. - pub opener: CommitmentPartyState, - /// Parameters for the channel acceptor. - pub acceptor: CommitmentPartyState, + /// Per-commitment point of `local_side`, used to derive commitment-specific + /// keys. + pub per_commitment_point: PublicKey, + /// Amount allocated to the opener in millisatoshis. Represents the balance + /// before subtraction of fees, and anchors outputs. In-flight HTLCs are + /// represented as separate outputs in the commitment transaction, so those + /// values are already deducted from these balance values. + pub opener_balance_msat: u64, + /// Amount allocated to the acceptor in millisatoshis. In-flight HTLCs are + /// represented as separate outputs in the commitment transaction, so those + /// values are already deducted from these balance values. + pub acceptor_balance_msat: u64, // TODO: When adding HTLC support, store pending HTLCs (offered/received) for both sides // to correctly compute balances and construct HTLC outputs in the commitment transaction. } +/// State of both sides' commitments. +pub struct ChannelCommitments { + /// State of the opener's commitment. + opener: CommitmentState, + /// State of the acceptor's commitment. + acceptor: CommitmentState, +} + /// Costs associated with a commitment transaction, including transaction fee /// and anchor outputs. pub struct CommitmentCost { @@ -123,8 +132,6 @@ pub struct CommitmentCost { /// Per-commitment keys used when constructing a commitment transaction. struct TxCreationKeys { - /// Side whose commitment transaction these keys are for. - local_side: Side, /// Local delayed payment pubkey. local_delayedpubkey: PublicKey, /// Revocation pubkey for this commitment. @@ -132,7 +139,7 @@ struct TxCreationKeys { } /// State of a single channel, including its static configuration, holder -/// identity, and current commitment state. +/// identity, and the state of each side's commitment. #[allow(clippy::struct_excessive_bools)] // Independent flags, not a state machine pub struct ChannelState { /// Channel configuration established at channel creation and unchanged @@ -141,9 +148,9 @@ pub struct ChannelState { /// Holder-specific identity data (channel side and funding secret) used to /// sign commitment transactions and verify the counterparty's signatures. pub holder: HolderIdentity, - /// Current commitment state, updated as commitments are exchanged and + /// State of each side's commitment, updated as commitments are exchanged and /// revoked. - pub commitment: CommitmentState, + pub commitments: ChannelCommitments, /// Opener's next per-commitment point used to build its next commitment, /// revealed by `channel_ready` and then each `revoke_and_ack`. `None` until /// known. @@ -195,7 +202,7 @@ impl ChannelState { pub fn new( config: ChannelConfig, holder: HolderIdentity, - commitment: CommitmentState, + commitments: ChannelCommitments, is_funding_outpoint_valid: bool, was_funding_mined_prematurely: bool, sent_invalid_signature: bool, @@ -203,7 +210,7 @@ impl ChannelState { Self { config, holder, - commitment, + commitments, opener_next_per_commitment_point: None, acceptor_next_per_commitment_point: None, is_funding_outpoint_valid, @@ -213,6 +220,18 @@ impl ChannelState { } } + /// Returns the state of the holder's commitment. + #[must_use] + pub fn holder_commitment_state(&self) -> &CommitmentState { + self.commitments.state(self.holder.side) + } + + /// Returns the state of the counterparty's commitment. + #[must_use] + pub fn counterparty_commitment_state(&self) -> &CommitmentState { + self.commitments.state(self.holder.counterparty_side()) + } + /// Returns the holder's next per-commitment point. #[must_use] pub fn next_holder_per_commitment_point(&self) -> &Option { @@ -233,7 +252,7 @@ impl ChannelState { /// Returns the counterparty's next per-commitment point. #[must_use] pub fn next_counterparty_per_commitment_point(&self) -> &Option { - match self.holder.side.other() { + match self.holder.counterparty_side() { Side::Opener => &self.opener_next_per_commitment_point, Side::Acceptor => &self.acceptor_next_per_commitment_point, } @@ -242,7 +261,7 @@ impl ChannelState { /// Returns a mutable reference to the counterparty's next per-commitment /// point. pub fn next_counterparty_per_commitment_point_mut(&mut self) -> &mut Option { - match self.holder.side.other() { + match self.holder.counterparty_side() { Side::Opener => &mut self.opener_next_per_commitment_point, Side::Acceptor => &mut self.acceptor_next_per_commitment_point, } @@ -258,7 +277,9 @@ impl ChannelConfig { } } - /// Constructs the initial commitment state after channel funding. + /// Constructs the initial commitment states for both sides after channel + /// funding. Both sides start with the same commitment state, except for the + /// per-commitment point and `local_side`. /// /// # Errors /// @@ -267,13 +288,13 @@ impl ChannelConfig { /// converting to millisatoshis. /// - [`CommitmentError::PushExceedsFunding`] if `push_msat` exceeds the total /// funding amount in millisatoshis. - pub fn new_initial_commitment( + pub fn new_initial_commitments( &self, push_msat: u64, feerate_per_kw: u32, opener_per_commitment_point: PublicKey, acceptor_per_commitment_point: PublicKey, - ) -> Result { + ) -> Result { let funding_msat = self .funding_satoshis .checked_mul(1000) @@ -283,17 +304,18 @@ impl ChannelConfig { .ok_or(CommitmentError::PushExceedsFunding)?; let to_acceptor_balance_msat = push_msat; - Ok(CommitmentState { + let new_state = |local_side, per_commitment_point| CommitmentState { + local_side, commitment_number: 0, feerate_per_kw, - opener: CommitmentPartyState { - per_commitment_point: opener_per_commitment_point, - balance_msat: to_opener_balance_msat, - }, - acceptor: CommitmentPartyState { - per_commitment_point: acceptor_per_commitment_point, - balance_msat: to_acceptor_balance_msat, - }, + per_commitment_point, + opener_balance_msat: to_opener_balance_msat, + acceptor_balance_msat: to_acceptor_balance_msat, + }; + + Ok(ChannelCommitments { + opener: new_state(Side::Opener, opener_per_commitment_point), + acceptor: new_state(Side::Acceptor, acceptor_per_commitment_point), }) } @@ -301,10 +323,11 @@ impl ChannelConfig { #[must_use] pub fn sign_counterparty_commitment( &self, - state: &CommitmentState, + commitments: &ChannelCommitments, holder: &HolderIdentity, ) -> Signature { - let commitment = self.build_commitment_tx(state, holder.counterparty_side()); + let state = commitments.state(holder.counterparty_side()); + let commitment = self.build_commitment_tx(state); self.sign_commitment_tx(&commitment, &holder.funding_privkey) } @@ -313,11 +336,12 @@ impl ChannelConfig { #[must_use] pub fn verify_counterparty_signature( &self, - state: &CommitmentState, + commitments: &ChannelCommitments, holder: &HolderIdentity, commitment_sig: &Signature, ) -> bool { - let commitment = self.build_commitment_tx(state, holder.side); + let state = commitments.state(holder.side); + let commitment = self.build_commitment_tx(state); self.verify_commitment_sig( &commitment, &self.party(holder.counterparty_side()).funding_pubkey, @@ -330,19 +354,17 @@ impl ChannelConfig { #[cfg(test)] fn sign_holder_commitment( &self, - state: &CommitmentState, + commitments: &ChannelCommitments, holder: &HolderIdentity, ) -> Signature { - let commitment = self.build_commitment_tx(state, holder.side); + let state = commitments.state(holder.side); + let commitment = self.build_commitment_tx(state); self.sign_commitment_tx(&commitment, &holder.funding_privkey) } /// Builds the commitment transaction. The commitment format (legacy or /// anchor) is determined by the `channel_type`. - /// - /// `local_side` selects whose commitment is built: the opener's or - /// the acceptor's. - fn build_commitment_tx(&self, state: &CommitmentState, local_side: Side) -> Transaction { + fn build_commitment_tx(&self, state: &CommitmentState) -> Transaction { // Obscured commitment number. let obscuring_factor = compute_obscuring_factor( &self.opener.payment_basepoint, @@ -363,7 +385,7 @@ impl ChannelConfig { .expect("commitment_number cannot be more than 48 bits"); // Build the commitment transaction - let keys = TxCreationKeys::derive(self, state, local_side); + let keys = TxCreationKeys::derive(self, state); let outputs = self.build_commitment_outputs(state, &keys); // Witness is not included in the BIP 143 sighash, so we leave it empty. @@ -405,22 +427,22 @@ impl ChannelConfig { /// Builds the lexicographically sorted commitment outputs. /// - /// Outputs are built for the commitment side the keys were derived for: + /// Outputs are built for the commitment side specified by `state`: /// the opener or the acceptor. fn build_commitment_outputs( &self, state: &CommitmentState, keys: &TxCreationKeys, ) -> Vec { - let local_side = keys.local_side; + let local_side = state.local_side; let anchor = self.channel_type.supports_feature(Features::OPTION_ANCHORS); let mut outputs: Vec = Vec::new(); // Fee and balances. let commitment_cost = CommitmentCost::new(state.feerate_per_kw, &self.channel_type); let opener_balance = - (state.opener.balance_msat / 1000).saturating_sub(commitment_cost.total_sat()); - let acceptor_balance = state.acceptor.balance_msat / 1000; + (state.opener_balance_msat / 1000).saturating_sub(commitment_cost.total_sat()); + let acceptor_balance = state.acceptor_balance_msat / 1000; // Map opener/acceptor to local/remote for this commitment side. let (to_local_value, to_remote_value) = match local_side { @@ -503,17 +525,14 @@ impl ChannelConfig { } } -impl CommitmentState { - /// Returns the parameters for the given commitment side. - fn party(&self, side: Side) -> &CommitmentPartyState { +impl ChannelCommitments { + /// Returns the state of the given side's commitment. + fn state(&self, side: Side) -> &CommitmentState { match side { Side::Opener => &self.opener, Side::Acceptor => &self.acceptor, } } - - // TODO: When adding HTLC support, add `get_next_commitment_state` to build the next - // commitment state based on the previous state and the HTLCs claimed by both sides. } impl CommitmentCost { @@ -534,14 +553,14 @@ impl CommitmentCost { } impl TxCreationKeys { - /// Derives the per-commitment keys for the `local_side`. - fn derive(config: &ChannelConfig, state: &CommitmentState, local_side: Side) -> Self { + /// Derives the per-commitment keys for the given commitment state. + fn derive(config: &ChannelConfig, state: &CommitmentState) -> Self { + let local_side = state.local_side; + let per_commitment_point = state.per_commitment_point; let local = config.party(local_side); let remote = config.party(local_side.other()); - let per_commitment_point = state.party(local_side).per_commitment_point; Self { - local_side, local_delayedpubkey: derive_pubkey( &local.delayed_payment_basepoint, &per_commitment_point, diff --git a/smite/src/channel_tx/commitment/tests.rs b/smite/src/channel_tx/commitment/tests.rs index b114519f..d714a4c7 100644 --- a/smite/src/channel_tx/commitment/tests.rs +++ b/smite/src/channel_tx/commitment/tests.rs @@ -107,7 +107,7 @@ fn new_initial_from_funding_msat_overflow() { u64::MAX, Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), ); - let result = chan_config.new_initial_commitment(0, 15_000, sample_key, sample_key); + let result = chan_config.new_initial_commitments(0, 15_000, sample_key, sample_key); assert!(matches!(result, Err(CommitmentError::FundingMsatOverflow))); } @@ -118,7 +118,7 @@ fn new_initial_from_funding_push_exceeds_funding() { 1_000, Features::from_bits(&[Features::OPTION_STATIC_REMOTEKEY]), ); - let result = chan_config.new_initial_commitment(2_000_000, 15_000, sample_key, sample_key); + let result = chan_config.new_initial_commitments(2_000_000, 15_000, sample_key, sample_key); assert!(matches!(result, Err(CommitmentError::PushExceedsFunding))); } diff --git a/smite/src/channel_tx/commitment/tests/harness.rs b/smite/src/channel_tx/commitment/tests/harness.rs index 759acab7..5d6fd64b 100644 --- a/smite/src/channel_tx/commitment/tests/harness.rs +++ b/smite/src/channel_tx/commitment/tests/harness.rs @@ -97,19 +97,20 @@ impl TestVectorFile { } } - /// Builds the commitment state for a vector. - fn build_commitment_state(&self, vector: &CommitmentVector) -> CommitmentState { - CommitmentState { + /// Builds the channel commitment states for both sides from a commitment vector. + fn build_channel_commitments(&self, vector: &CommitmentVector) -> ChannelCommitments { + let new_state = |local_side, per_commitment_point| CommitmentState { + local_side, commitment_number: self.commitment_number, feerate_per_kw: vector.feerate_per_kw, - opener: CommitmentPartyState { - per_commitment_point: self.opener.per_commitment_point, - balance_msat: vector.to_opener_msat, - }, - acceptor: CommitmentPartyState { - per_commitment_point: self.acceptor.per_commitment_point, - balance_msat: vector.to_acceptor_msat, - }, + per_commitment_point, + opener_balance_msat: vector.to_opener_msat, + acceptor_balance_msat: vector.to_acceptor_msat, + }; + + ChannelCommitments { + opener: new_state(Side::Opener, self.opener.per_commitment_point), + acceptor: new_state(Side::Acceptor, self.acceptor.per_commitment_point), } } @@ -129,14 +130,13 @@ impl TestVectorFile { /// Checks one vector, returning a message per failed assertion. fn check_vector(&self, vector: &CommitmentVector) -> Vec { let channel_config = self.build_channel_config(vector); - let commitment_state = self.build_commitment_state(vector); + let commitments = self.build_channel_commitments(vector); let opener_holder = self.build_holder_identity(Side::Opener); let acceptor_holder = self.build_holder_identity(Side::Acceptor); let mut failures = Vec::new(); // Opener signs own commitment. - let local_signature = - channel_config.sign_holder_commitment(&commitment_state, &opener_holder); + let local_signature = channel_config.sign_holder_commitment(&commitments, &opener_holder); if local_signature != vector.local_signature { failures.push(format!( "{}: local signature mismatch\n expected: {}\n actual: {}", @@ -146,7 +146,7 @@ impl TestVectorFile { // Acceptor signs opener's commitment, then the opener verifies it. let remote_signature = - channel_config.sign_counterparty_commitment(&commitment_state, &acceptor_holder); + channel_config.sign_counterparty_commitment(&commitments, &acceptor_holder); if remote_signature != vector.remote_signature { failures.push(format!( "{}: remote signature mismatch\n expected: {}\n actual: {}", @@ -154,7 +154,7 @@ impl TestVectorFile { )); } if !channel_config.verify_counterparty_signature( - &commitment_state, + &commitments, &opener_holder, &vector.remote_signature, ) { @@ -163,9 +163,9 @@ impl TestVectorFile { // Opener signs the acceptor's commitment, then the acceptor verifies it. let acceptor_commit_sig = - channel_config.sign_counterparty_commitment(&commitment_state, &opener_holder); + channel_config.sign_counterparty_commitment(&commitments, &opener_holder); if !channel_config.verify_counterparty_signature( - &commitment_state, + &commitments, &acceptor_holder, &acceptor_commit_sig, ) { diff --git a/smite/src/oracles/funding_signed.rs b/smite/src/oracles/funding_signed.rs index 55550cd0..c1d3fa4f 100644 --- a/smite/src/oracles/funding_signed.rs +++ b/smite/src/oracles/funding_signed.rs @@ -62,7 +62,7 @@ impl Oracle> for FundingSignedOracle { // `funding_signed`, a channel_id collision (see above) surfaces here as // an invalid signature instead. if !channel.config.verify_counterparty_signature( - &channel.commitment, + &channel.commitments, &channel.holder, &context.funding_signed.signature, ) { @@ -125,15 +125,15 @@ mod tests { }, minimum_depth: 8, }; - let commitment = config - .new_initial_commitment(3_000_000_000, 15_000, pkey1, pkey2) - .expect("valid initial commitment"); + let commitments = config + .new_initial_commitments(3_000_000_000, 15_000, pkey1, pkey2) + .expect("valid initial commitments"); let holder = HolderIdentity { side: Side::Opener, funding_privkey: secret_key(1), }; - ChannelState::new(config, holder, commitment, true, false, false) + ChannelState::new(config, holder, commitments, true, false, false) } /// Valid `funding_signed` message for testing. @@ -146,7 +146,7 @@ mod tests { channel_id: ChannelId::v1_from_funding_outpoint(channel.config.funding_outpoint), signature: channel .config - .sign_counterparty_commitment(&channel.commitment, &acceptor), + .sign_counterparty_commitment(&channel.commitments, &acceptor), } }