From a7ed97a08a1dde426186edb9a207f80121bf421b Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 21 Mar 2023 21:12:53 +0200 Subject: [PATCH 01/28] Add `InteractiveTxConstructor` state machine` --- lightning/src/ln/interactivetxs.rs | 263 +++++++++++++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 lightning/src/ln/interactivetxs.rs diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs new file mode 100644 index 00000000000..a24ed98ca9e --- /dev/null +++ b/lightning/src/ln/interactivetxs.rs @@ -0,0 +1,263 @@ +// This file is Copyright its original authors, visible in version control +// history. +// +// This file is licensed under the Apache License, Version 2.0 or the MIT license +// , at your option. +// You may not use this file except in accordance with one or both of these +// licenses. + +use std::collections::{HashMap, HashSet}; + +use bitcoin::{psbt::Psbt, TxIn, Sequence, Transaction, TxOut, OutPoint}; + +use super::msgs::TxAddInput; + +/// The number of received `tx_add_input` messages during a negotiation at which point the +/// negotiation MUST be failed. +const MAX_RECEIVED_TX_ADD_INPUT_COUNT: u16 = 4096; + +/// The number of received `tx_add_output` messages during a negotiation at which point the +/// negotiation MUST be failed. +const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; + +type SerialId = u64; +trait SerialIdExt { + fn is_valid_for_initiator(&self) -> bool; +} +impl SerialIdExt for SerialId { + fn is_valid_for_initiator(&self) -> bool { self % 2 == 0 } +} + +pub(crate) enum InteractiveTxConstructionError { + InputsNotConfirmed, + ReceivedTooManyTxAddInputs, + ReceivedTooManyTxAddOutputs, + IncorrectInputSequenceValue, + IncorrectSerialIdParity, + SerialIdUnknown, + DuplicateSerialId, + PrevTxOutInvalid, +} + +// States +// TODO: ASCII state machine +pub(crate) trait AcceptingChanges {} + +/// We are currently in the process of negotiating the transaction. +pub(crate) struct Negotiating; +/// We have sent a `tx_complete` message and are awaiting the counterparty's. +pub(crate) struct OurTxComplete; +/// We have received a `tx_complete` message and the counterparty is awaiting ours. +pub(crate) struct TheirTxComplete; +/// We have exchanged consecutive `tx_complete` messages with the counterparty and the transaction +/// negotiation is complete. +pub(crate) struct NegotiationComplete; +/// We have sent a `tx_signatures` message and the counterparty is awaiting ours. +pub(crate) struct OurTxSignatures; +/// We have received a `tx_signatures` message from the counterparty +pub(crate) struct TheirTxSignatures; +/// The negotiation has failed and cannot be continued. +pub(crate) struct NegotiationFailed { + error: InteractiveTxConstructionError, +} + +// TODO: Add RBF negotiation + +impl AcceptingChanges for Negotiating {} +impl AcceptingChanges for OurTxComplete {} + +struct NegotiationContext { + channel_id: [u8; 32], + require_confirmed_inputs: bool, + holder_is_initiator: bool, + received_tx_add_input_count: u16, + received_tx_add_output_count: u16, + inputs: HashMap, + prevtx_outpoints: HashSet, + outputs: HashMap, + base_tx: Transaction, +} + +pub(crate) struct InteractiveTxConstructor { + inner: Box, + state: S, +} + +impl InteractiveTxConstructor { + fn new(channel_id: [u8; 32], require_confirmed_inputs: bool, is_initiator: bool, base_tx: Transaction) -> Self { + Self { + inner: Box::new(NegotiationContext { + channel_id, + require_confirmed_inputs, + holder_is_initiator: is_initiator, + received_tx_add_input_count: 0, + received_tx_add_output_count: 0, + inputs: HashMap::new(), + prevtx_outpoints: HashSet::new(), + outputs: HashMap::new(), + base_tx, + }), + state: Negotiating, + } + } +} + +impl InteractiveTxConstructor + where S: AcceptingChanges { + fn fail_negotiation(self, error: InteractiveTxConstructionError) -> + Result, InteractiveTxConstructor> { + Err(InteractiveTxConstructor { inner: self.inner, state: NegotiationFailed { error } }) + } + + fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> + Result, InteractiveTxConstructor> { + // - TODO: MUST fail the negotiation if: + // - `prevtx` is not a valid transaction + if !self.is_valid_counterparty_serial_id(serial_id) { + // The receiving node: + // - MUST fail the negotiation if: + // - the `serial_id` has the wrong parity + return self.fail_negotiation(InteractiveTxConstructionError::IncorrectSerialIdParity); + } + + if msg.sequence >= 0xFFFFFFFE { + // The receiving node: + // - MUST fail the negotiation if: + // - `sequence` is set to `0xFFFFFFFE` or `0xFFFFFFFF` + return self.fail_negotiation(InteractiveTxConstructionError::IncorrectInputSequenceValue); + } + + if self.inner.require_confirmed_inputs && !confirmed { + return self.fail_negotiation(InteractiveTxConstructionError::InputsNotConfirmed); + } + + if let Some(tx_out) = msg.prevtx.output.get(msg.prevtx_out as usize) { + if !tx_out.script_pubkey.is_witness_program() { + // The receiving node: + // - MUST fail the negotiation if: + // - the `scriptPubKey` is not a witness program + return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + } else if !self.inner.prevtx_outpoints.insert(OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }) { + // The receiving node: + // - MUST fail the negotiation if: + // - the `prevtx` and `prevtx_vout` are identical to a previously added + // (and not removed) input's + return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + } + } else { + // The receiving node: + // - MUST fail the negotiation if: + // - `prevtx_vout` is greater or equal to the number of outputs on `prevtx` + return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + } + + self.inner.received_tx_add_input_count += 1; + if self.inner.received_tx_add_input_count > MAX_RECEIVED_TX_ADD_INPUT_COUNT { + // The receiving node: + // - MUST fail the negotiation if: + // - if has received 4096 `tx_add_input` messages during this negotiation + return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddInputs); + } + + if let None = self.inner.inputs.insert(serial_id, TxIn { + previous_output: OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }, + sequence: Sequence(msg.sequence), + ..Default::default() + }) { + Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + } else { + // The receiving node: + // - MUST fail the negotiation if: + // - the `serial_id` is already included in the transaction + self.fail_negotiation(InteractiveTxConstructionError::DuplicateSerialId) + } + } + + fn receive_tx_remove_input(mut self, serial_id: SerialId) -> + Result, InteractiveTxConstructor> { + if !self.is_valid_counterparty_serial_id(serial_id) { + return self.fail_negotiation(InteractiveTxConstructionError::IncorrectSerialIdParity); + } + + if let Some(input) = self.inner.inputs.remove(&serial_id) { + self.inner.prevtx_outpoints.remove(&input.previous_output); + Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + } else { + // The receiving node: + // - MUST fail the negotiation if: + // - the input or output identified by the `serial_id` was not added by the sender + // - the `serial_id` does not correspond to a currently added input + self.fail_negotiation(InteractiveTxConstructionError::SerialIdUnknown) + } + } + + fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> + Result, InteractiveTxConstructor> { + self.inner.received_tx_add_output_count += 1; + if self.inner.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { + // The receiving node: + // - MUST fail the negotiation if: + // - if has received 4096 `tx_add_output` messages during this negotiation + return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddOutputs); + } + + if let None = self.inner.outputs.insert(serial_id, output) { + Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + } else { + // The receiving node: + // - MUST fail the negotiation if: + // - the `serial_id` is already included in the transaction + self.fail_negotiation(InteractiveTxConstructionError::DuplicateSerialId) + } + } + + + fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxConstructor { + self.inner.inputs.insert(serial_id, input); + InteractiveTxConstructor { inner: self.inner, state: Negotiating {} } + } + + pub(crate) fn send_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxConstructor { + self.inner.outputs.insert(serial_id, output); + InteractiveTxConstructor { inner: self.inner, state: Negotiating {} } + } + + pub(crate) fn send_tx_abort(mut self) -> InteractiveTxConstructor { + todo!(); + } + + pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { + todo!(); + } + + fn is_valid_counterparty_serial_id(&self, serial_id: SerialId) -> bool { + // A received `SerialId`'s parity must match the role of the counterparty. + self.inner.holder_is_initiator == !serial_id.is_valid_for_initiator() + } +} + +impl InteractiveTxConstructor { + fn send_tx_complete(self) -> InteractiveTxConstructor { + InteractiveTxConstructor { + inner: self.inner, + state: NegotiationComplete {} + } + } +} + +impl InteractiveTxConstructor { + fn receive_tx_complete(self) -> InteractiveTxConstructor { + InteractiveTxConstructor { + inner: self.inner, + state: NegotiationComplete {} + } + } +} + +impl InteractiveTxConstructor { + fn get_psbt(&self) -> Result { + // Build PSBT from inputs & outputs + todo!(); + } +} From 8536621437db6fcc4a9552f55d6a73494a773e1c Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Fri, 21 Apr 2023 07:38:31 -0700 Subject: [PATCH 02/28] Check add_output sats amount < 2.1 quadrillion sats --- lightning/src/ln/interactivetxs.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a24ed98ca9e..fae031e30ca 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -20,6 +20,7 @@ const MAX_RECEIVED_TX_ADD_INPUT_COUNT: u16 = 4096; /// The number of received `tx_add_output` messages during a negotiation at which point the /// negotiation MUST be failed. const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; +const MAX_MONEY: u64 = 2_100_000_000_000_000; type SerialId = u64; trait SerialIdExt { @@ -38,6 +39,7 @@ pub(crate) enum InteractiveTxConstructionError { SerialIdUnknown, DuplicateSerialId, PrevTxOutInvalid, + ExceedMaxiumSatsAllowed, } // States @@ -202,6 +204,13 @@ impl InteractiveTxConstructor return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddOutputs); } + if output.value > MAX_MONEY { + // The receiving node: + // - MUST fail the negotiation if: + // - the sats amount is greater than 2,100,000,000,000,000 (MAX_MONEY) + return self.fail_negotiation(InteractiveTxConstructionError::ExceedMaxiumSatsAllowed); + } + if let None = self.inner.outputs.insert(serial_id, output) { Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) } else { From 8cf4ee7d68dad866ffe3e07e62de816a0296d2d0 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Fri, 21 Apr 2023 07:39:05 -0700 Subject: [PATCH 03/28] Add test module for API usage experiment --- lightning/src/ln/interactivetxs.rs | 195 ++++++++++++++++++++++++----- lightning/src/ln/mod.rs | 1 + 2 files changed, 162 insertions(+), 34 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index fae031e30ca..2b61b5e7629 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -9,7 +9,8 @@ use std::collections::{HashMap, HashSet}; -use bitcoin::{psbt::Psbt, TxIn, Sequence, Transaction, TxOut, OutPoint}; +use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; +use crate::ln::interactivetxs::ChannelMode::Indeterminate; use super::msgs::TxAddInput; @@ -79,27 +80,35 @@ struct NegotiationContext { prevtx_outpoints: HashSet, outputs: HashMap, base_tx: Transaction, + did_send_tx_signatures: bool, } pub(crate) struct InteractiveTxConstructor { - inner: Box, + context: NegotiationContext, state: S, } impl InteractiveTxConstructor { - fn new(channel_id: [u8; 32], require_confirmed_inputs: bool, is_initiator: bool, base_tx: Transaction) -> Self { + fn new( + channel_id: [u8; 32], + require_confirmed_inputs: bool, + is_initiator: bool, + base_tx: Transaction, + did_send_tx_signatures: bool, + ) -> Self { Self { - inner: Box::new(NegotiationContext { + context: NegotiationContext { channel_id, require_confirmed_inputs, + base_tx, + did_send_tx_signatures, holder_is_initiator: is_initiator, received_tx_add_input_count: 0, received_tx_add_output_count: 0, inputs: HashMap::new(), prevtx_outpoints: HashSet::new(), outputs: HashMap::new(), - base_tx, - }), + }, state: Negotiating, } } @@ -109,7 +118,7 @@ impl InteractiveTxConstructor where S: AcceptingChanges { fn fail_negotiation(self, error: InteractiveTxConstructionError) -> Result, InteractiveTxConstructor> { - Err(InteractiveTxConstructor { inner: self.inner, state: NegotiationFailed { error } }) + Err(InteractiveTxConstructor { context: self.context, state: NegotiationFailed { error } }) } fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> @@ -130,7 +139,7 @@ impl InteractiveTxConstructor return self.fail_negotiation(InteractiveTxConstructionError::IncorrectInputSequenceValue); } - if self.inner.require_confirmed_inputs && !confirmed { + if self.context.require_confirmed_inputs && !confirmed { return self.fail_negotiation(InteractiveTxConstructionError::InputsNotConfirmed); } @@ -140,7 +149,7 @@ impl InteractiveTxConstructor // - MUST fail the negotiation if: // - the `scriptPubKey` is not a witness program return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); - } else if !self.inner.prevtx_outpoints.insert(OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }) { + } else if !self.context.prevtx_outpoints.insert(OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }) { // The receiving node: // - MUST fail the negotiation if: // - the `prevtx` and `prevtx_vout` are identical to a previously added @@ -154,20 +163,20 @@ impl InteractiveTxConstructor return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); } - self.inner.received_tx_add_input_count += 1; - if self.inner.received_tx_add_input_count > MAX_RECEIVED_TX_ADD_INPUT_COUNT { + self.context.received_tx_add_input_count += 1; + if self.context.received_tx_add_input_count > MAX_RECEIVED_TX_ADD_INPUT_COUNT { // The receiving node: // - MUST fail the negotiation if: // - if has received 4096 `tx_add_input` messages during this negotiation return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddInputs); } - if let None = self.inner.inputs.insert(serial_id, TxIn { + if let None = self.context.inputs.insert(serial_id, TxIn { previous_output: OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }, sequence: Sequence(msg.sequence), ..Default::default() }) { - Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -182,9 +191,9 @@ impl InteractiveTxConstructor return self.fail_negotiation(InteractiveTxConstructionError::IncorrectSerialIdParity); } - if let Some(input) = self.inner.inputs.remove(&serial_id) { - self.inner.prevtx_outpoints.remove(&input.previous_output); - Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + if let Some(input) = self.context.inputs.remove(&serial_id) { + self.context.prevtx_outpoints.remove(&input.previous_output); + Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -196,8 +205,9 @@ impl InteractiveTxConstructor fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> Result, InteractiveTxConstructor> { - self.inner.received_tx_add_output_count += 1; - if self.inner.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { + // TODO: the sats amount is less than the dust_limit + self.context.received_tx_add_output_count += 1; + if self.context.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { // The receiving node: // - MUST fail the negotiation if: // - if has received 4096 `tx_add_output` messages during this negotiation @@ -211,8 +221,8 @@ impl InteractiveTxConstructor return self.fail_negotiation(InteractiveTxConstructionError::ExceedMaxiumSatsAllowed); } - if let None = self.inner.outputs.insert(serial_id, output) { - Ok(InteractiveTxConstructor { inner: self.inner, state: Negotiating {} }) + if let None = self.context.outputs.insert(serial_id, output) { + Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -221,35 +231,37 @@ impl InteractiveTxConstructor } } + pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { + todo!(); + } fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxConstructor { - self.inner.inputs.insert(serial_id, input); - InteractiveTxConstructor { inner: self.inner, state: Negotiating {} } + self.context.inputs.insert(serial_id, input); + InteractiveTxConstructor { context: self.context, state: Negotiating {} } } pub(crate) fn send_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxConstructor { - self.inner.outputs.insert(serial_id, output); - InteractiveTxConstructor { inner: self.inner, state: Negotiating {} } + self.context.outputs.insert(serial_id, output); + InteractiveTxConstructor { context: self.context, state: Negotiating {} } } pub(crate) fn send_tx_abort(mut self) -> InteractiveTxConstructor { - todo!(); - } - - pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { + // A sending node: + // - MUST NOT have already transmitted tx_signatures + // - SHOULD forget the current negotiation and reset their state. todo!(); } fn is_valid_counterparty_serial_id(&self, serial_id: SerialId) -> bool { // A received `SerialId`'s parity must match the role of the counterparty. - self.inner.holder_is_initiator == !serial_id.is_valid_for_initiator() + self.context.holder_is_initiator == !serial_id.is_valid_for_initiator() } } impl InteractiveTxConstructor { fn send_tx_complete(self) -> InteractiveTxConstructor { InteractiveTxConstructor { - inner: self.inner, + context: self.context, state: NegotiationComplete {} } } @@ -258,15 +270,130 @@ impl InteractiveTxConstructor { impl InteractiveTxConstructor { fn receive_tx_complete(self) -> InteractiveTxConstructor { InteractiveTxConstructor { - inner: self.inner, + context: self.context, state: NegotiationComplete {} } } } impl InteractiveTxConstructor { - fn get_psbt(&self) -> Result { - // Build PSBT from inputs & outputs - todo!(); + fn get_psbt(&self) -> Result { + // Build transaction from inputs & outputs in `NegotiationContext`. + return Ok(Transaction { + version: self.context.base_tx.version, + lock_time: self.context.base_tx.lock_time, + input: self.context.inputs.values().cloned().collect(), + output: self.context.outputs.values().cloned().collect(), + }) + } +} + +enum ChannelMode { + Negotiating(InteractiveTxConstructor), + OurTxComplete(InteractiveTxConstructor), + TheirTxComplete(InteractiveTxConstructor), + NegotiationComplete(InteractiveTxConstructor), + OurTxSignatures(InteractiveTxConstructor), + TheirTxSignatures(InteractiveTxConstructor), + NegotiationFailed(InteractiveTxConstructor), + Indeterminate, +} + +impl Default for ChannelMode { + fn default() -> Self { Indeterminate } +} + +#[cfg(test)] +mod tests { + use core::str::FromStr; + use std::collections::HashMap; + use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationFailed}; + use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor}; + use crate::ln::msgs::TransactionU16LenLimited; + use bitcoin::consensus::encode; + use bitcoin::{Address, PackedLockTime, Script, Sequence, Transaction, Txid, TxIn, TxOut, Witness}; + use bitcoin::hashes::hex::FromHex; + use crate::chain::transaction::OutPoint; + use crate::ln::msgs::TxAddInput; + + struct DummyChannel { + mode: ChannelMode, + } + + impl DummyChannel { + fn new() -> Self { + let tx: Transaction = encode::deserialize(&hex::decode("020000000001010e0ade\ + f48412e4361325ac1c6e36411299ab09d4f083b9d8ddb55fbc06e1b0c00000000000feffffff0220a107000\ + 0000000220020f81d95e040bd0a493e38bae27bff52fe2bb58b93b293eb579c01c31b05c5af1dc072cfee54\ + a3000016001434b1d6211af5551905dc2642d05f5b04d25a8fe80247304402207f570e3f0de50546aad25a8\ + 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ + dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ + b76bd00000000").unwrap()[..]).unwrap(); + Self { + mode: Negotiating(InteractiveTxConstructor::new( + [0; 32], + true, + true, + tx, + false, + )) + } + } + + fn handle_add_tx_input(&mut self) { + // We use mem::take here because we want to update `self.mode` based on its value and + // avoid cloning `ChannelMode`. + // By moving the value out of the struct, we can now safely modify it in this scope. + let mut mode = core::mem::take(&mut self.mode); + self.mode = if let Negotiating(constructor) = mode { + match constructor.receive_tx_add_input( + 1234, + get_sample_tx_add_input(), + true + ) { + Ok(c) => Negotiating(c), + Err(c) => NegotiationFailed(c), + } + } else { + mode + } + } + } + + // Fixtures + fn get_sample_tx_add_input() -> TxAddInput { + let prevtx = TransactionU16LenLimited::new( + Transaction { + version: 2, + lock_time: PackedLockTime(0), + input: vec![TxIn { + previous_output: OutPoint { txid: Txid::from_hex("305bab643ee297b8b6b76b320792c8223d55082122cb606bf89382146ced9c77").unwrap(), index: 2 }.into_bitcoin_outpoint(), + script_sig: Script::new(), + sequence: Sequence(0xfffffffd), + witness: Witness::from_vec(vec![ + hex::decode("304402206af85b7dd67450ad12c979302fac49dfacbc6a8620f49c5da2b5721cf9565ca502207002b32fed9ce1bf095f57aeb10c36928ac60b12e723d97d2964a54640ceefa701").unwrap(), + hex::decode("0301ab7dc16488303549bfcdd80f6ae5ee4c20bf97ab5410bbd6b1bfa85dcd6944").unwrap()]), + }], + output: vec![ + TxOut { + value: 12704566, + script_pubkey: Address::from_str("bc1qzlffunw52jav8vwdu5x3jfk6sr8u22rmq3xzw2").unwrap().script_pubkey(), + }, + TxOut { + value: 245148, + script_pubkey: Address::from_str("bc1qxmk834g5marzm227dgqvynd23y2nvt2ztwcw2z").unwrap().script_pubkey(), + }, + ], + } + ).unwrap(); + + return TxAddInput { + channel_id: [2; 32], + serial_id: 4886718345, + prevtx, + prevtx_out: 305419896, + sequence: 305419896, + }; } } + diff --git a/lightning/src/ln/mod.rs b/lightning/src/ln/mod.rs index 3a0efd38690..142f76b08d7 100644 --- a/lightning/src/ln/mod.rs +++ b/lightning/src/ln/mod.rs @@ -66,6 +66,7 @@ mod monitor_tests; #[cfg(test)] #[allow(unused_mut)] mod shutdown_tests; +mod interactivetxs; pub use self::peer_channel_encryptor::LN_MAX_MSG_LEN; From af5133e9ae3cfe118fed2e5857865ee7d2b99a73 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Sun, 7 May 2023 20:51:46 +0200 Subject: [PATCH 04/28] Add ascii diagram & remove tx_signatures --- lightning/src/ln/interactivetxs.rs | 102 ++++++++++++++++++----------- 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 2b61b5e7629..bc769b061f5 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -31,7 +31,8 @@ impl SerialIdExt for SerialId { fn is_valid_for_initiator(&self) -> bool { self % 2 == 0 } } -pub(crate) enum InteractiveTxConstructionError { +pub(crate) enum AbortReason { + CounterpartyAborted, InputsNotConfirmed, ReceivedTooManyTxAddInputs, ReceivedTooManyTxAddOutputs, @@ -40,11 +41,41 @@ pub(crate) enum InteractiveTxConstructionError { SerialIdUnknown, DuplicateSerialId, PrevTxOutInvalid, - ExceedMaxiumSatsAllowed, + ExceededMaximumSatsAllowed, } -// States -// TODO: ASCII state machine +// Interactive Transaction Construction negotiation +// from the perspective of a holder +// +// AcceptingChanges +// ┌──────────────────────────────┐ +// │ │ +// │ ┌────────────────┐ │ +// │ │(sent/received) │ │ +// │ │tx_add_input │ │ +// │ │tx_add_output │ │ +// │ │tx_remove_input │ │ +// │ │tx_remove_output│ │ +// │ └───┐ ┌────┘ │ +// │ │ ▼ │ +// ────────────┼──────────►┌───┴───────────┐ │ received_tx_complete ┌─────────────────────┐ +// accept_channel2 │ │ ├──┼───────────────────┐ sent_tx_complete │ │ +// or splice_ack │ ┌─────┤ Negotiating │ │ ▼ ┌───────────────►│ NegotiationComplete │◄──┐ +// or tx_ack_rbf │ │ │ │ │ ┌─────────────────┐ │ │ │ │ +// (sent or received) │ │ ┌──►└───────────────┘ │ │ │ │ └─────────────────────┘ │ +// │ │ │ │ TheirTxComplete ├─┘ │ +// sent_tx_complete │ │ received_tx_add_* │ │ │ ┌────────────────────┐ │ +// │ │ received_tx_remove_* │ └─────────────────┘ │ │ │ +// │ │ │ │ ┌──►│ NegotiationAborted │ │ +// │ │ └───┬───────────────┐ │ (sent/received)_tx_abort │ │ │ │ +// │ │ │ │ ├────────────────────────────────────────────┘ └────────────────────┘ │ +// │ └────►│ OurTxComplete │ │ │ +// │ │ ├──┼──┐ │ +// │ └───────────────┘ │ └──────────────────────────────────────────────────────────────────────┘ +// │ │ received_tx_complete +// │ │ +// └──────────────────────────────┘ +// pub(crate) trait AcceptingChanges {} /// We are currently in the process of negotiating the transaction. @@ -56,16 +87,8 @@ pub(crate) struct TheirTxComplete; /// We have exchanged consecutive `tx_complete` messages with the counterparty and the transaction /// negotiation is complete. pub(crate) struct NegotiationComplete; -/// We have sent a `tx_signatures` message and the counterparty is awaiting ours. -pub(crate) struct OurTxSignatures; -/// We have received a `tx_signatures` message from the counterparty -pub(crate) struct TheirTxSignatures; /// The negotiation has failed and cannot be continued. -pub(crate) struct NegotiationFailed { - error: InteractiveTxConstructionError, -} - -// TODO: Add RBF negotiation +pub(crate) struct NegotiationAborted(AbortReason); impl AcceptingChanges for Negotiating {} impl AcceptingChanges for OurTxComplete {} @@ -116,31 +139,32 @@ impl InteractiveTxConstructor { impl InteractiveTxConstructor where S: AcceptingChanges { - fn fail_negotiation(self, error: InteractiveTxConstructionError) -> - Result, InteractiveTxConstructor> { - Err(InteractiveTxConstructor { context: self.context, state: NegotiationFailed { error } }) + fn abort_negotiation(self, reason: AbortReason) -> + Result, InteractiveTxConstructor> { + + Err(InteractiveTxConstructor { context: self.context, state: NegotiationAborted(reason) }) } fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxConstructor> { // - TODO: MUST fail the negotiation if: // - `prevtx` is not a valid transaction if !self.is_valid_counterparty_serial_id(serial_id) { // The receiving node: // - MUST fail the negotiation if: // - the `serial_id` has the wrong parity - return self.fail_negotiation(InteractiveTxConstructionError::IncorrectSerialIdParity); + return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } if msg.sequence >= 0xFFFFFFFE { // The receiving node: // - MUST fail the negotiation if: // - `sequence` is set to `0xFFFFFFFE` or `0xFFFFFFFF` - return self.fail_negotiation(InteractiveTxConstructionError::IncorrectInputSequenceValue); + return self.abort_negotiation(AbortReason::IncorrectInputSequenceValue); } if self.context.require_confirmed_inputs && !confirmed { - return self.fail_negotiation(InteractiveTxConstructionError::InputsNotConfirmed); + return self.abort_negotiation(AbortReason::InputsNotConfirmed); } if let Some(tx_out) = msg.prevtx.output.get(msg.prevtx_out as usize) { @@ -148,19 +172,19 @@ impl InteractiveTxConstructor // The receiving node: // - MUST fail the negotiation if: // - the `scriptPubKey` is not a witness program - return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + return self.abort_negotiation(AbortReason::PrevTxOutInvalid); } else if !self.context.prevtx_outpoints.insert(OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }) { // The receiving node: // - MUST fail the negotiation if: // - the `prevtx` and `prevtx_vout` are identical to a previously added // (and not removed) input's - return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + return self.abort_negotiation(AbortReason::PrevTxOutInvalid); } } else { // The receiving node: // - MUST fail the negotiation if: // - `prevtx_vout` is greater or equal to the number of outputs on `prevtx` - return self.fail_negotiation(InteractiveTxConstructionError::PrevTxOutInvalid); + return self.abort_negotiation(AbortReason::PrevTxOutInvalid); } self.context.received_tx_add_input_count += 1; @@ -168,7 +192,7 @@ impl InteractiveTxConstructor // The receiving node: // - MUST fail the negotiation if: // - if has received 4096 `tx_add_input` messages during this negotiation - return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddInputs); + return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); } if let None = self.context.inputs.insert(serial_id, TxIn { @@ -181,14 +205,14 @@ impl InteractiveTxConstructor // The receiving node: // - MUST fail the negotiation if: // - the `serial_id` is already included in the transaction - self.fail_negotiation(InteractiveTxConstructionError::DuplicateSerialId) + self.abort_negotiation(AbortReason::DuplicateSerialId) } } fn receive_tx_remove_input(mut self, serial_id: SerialId) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxConstructor> { if !self.is_valid_counterparty_serial_id(serial_id) { - return self.fail_negotiation(InteractiveTxConstructionError::IncorrectSerialIdParity); + return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } if let Some(input) = self.context.inputs.remove(&serial_id) { @@ -199,26 +223,26 @@ impl InteractiveTxConstructor // - MUST fail the negotiation if: // - the input or output identified by the `serial_id` was not added by the sender // - the `serial_id` does not correspond to a currently added input - self.fail_negotiation(InteractiveTxConstructionError::SerialIdUnknown) + self.abort_negotiation(AbortReason::SerialIdUnknown) } } fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxConstructor> { // TODO: the sats amount is less than the dust_limit self.context.received_tx_add_output_count += 1; if self.context.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { // The receiving node: // - MUST fail the negotiation if: // - if has received 4096 `tx_add_output` messages during this negotiation - return self.fail_negotiation(InteractiveTxConstructionError::ReceivedTooManyTxAddOutputs); + return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); } if output.value > MAX_MONEY { // The receiving node: // - MUST fail the negotiation if: // - the sats amount is greater than 2,100,000,000,000,000 (MAX_MONEY) - return self.fail_negotiation(InteractiveTxConstructionError::ExceedMaxiumSatsAllowed); + return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); } if let None = self.context.outputs.insert(serial_id, output) { @@ -227,11 +251,11 @@ impl InteractiveTxConstructor // The receiving node: // - MUST fail the negotiation if: // - the `serial_id` is already included in the transaction - self.fail_negotiation(InteractiveTxConstructionError::DuplicateSerialId) + self.abort_negotiation(AbortReason::DuplicateSerialId) } } - pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { + pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { todo!(); } @@ -245,7 +269,7 @@ impl InteractiveTxConstructor InteractiveTxConstructor { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_abort(mut self) -> InteractiveTxConstructor { + pub(crate) fn send_tx_abort(mut self) -> InteractiveTxConstructor { // A sending node: // - MUST NOT have already transmitted tx_signatures // - SHOULD forget the current negotiation and reset their state. @@ -277,7 +301,7 @@ impl InteractiveTxConstructor { } impl InteractiveTxConstructor { - fn get_psbt(&self) -> Result { + fn get_psbt(&self) -> Result { // Build transaction from inputs & outputs in `NegotiationContext`. return Ok(Transaction { version: self.context.base_tx.version, @@ -293,9 +317,7 @@ enum ChannelMode { OurTxComplete(InteractiveTxConstructor), TheirTxComplete(InteractiveTxConstructor), NegotiationComplete(InteractiveTxConstructor), - OurTxSignatures(InteractiveTxConstructor), - TheirTxSignatures(InteractiveTxConstructor), - NegotiationFailed(InteractiveTxConstructor), + NegotiationAborted(InteractiveTxConstructor), Indeterminate, } @@ -307,7 +329,7 @@ impl Default for ChannelMode { mod tests { use core::str::FromStr; use std::collections::HashMap; - use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationFailed}; + use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor}; use crate::ln::msgs::TransactionU16LenLimited; use bitcoin::consensus::encode; @@ -352,7 +374,7 @@ mod tests { true ) { Ok(c) => Negotiating(c), - Err(c) => NegotiationFailed(c), + Err(c) => NegotiationAborted(c), } } else { mode From 7310736de62f707d8da8c0071744624f0632250d Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 7 May 2023 22:02:56 -0700 Subject: [PATCH 05/28] Update API usage --- lightning/src/ln/interactivetxs.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index bc769b061f5..d3c6aaae045 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -167,13 +167,20 @@ impl InteractiveTxConstructor return self.abort_negotiation(AbortReason::InputsNotConfirmed); } - if let Some(tx_out) = msg.prevtx.output.get(msg.prevtx_out as usize) { + let transaction = msg.prevtx.into_transaction(); + + if let Some(tx_out) = transaction.output.get(msg.prevtx_out as usize) { if !tx_out.script_pubkey.is_witness_program() { // The receiving node: // - MUST fail the negotiation if: // - the `scriptPubKey` is not a witness program return self.abort_negotiation(AbortReason::PrevTxOutInvalid); - } else if !self.context.prevtx_outpoints.insert(OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }) { + } else if !self.context.prevtx_outpoints.insert( + OutPoint { + txid: transaction.txid(), + vout: msg.prevtx_out + } + ) { // The receiving node: // - MUST fail the negotiation if: // - the `prevtx` and `prevtx_vout` are identical to a previously added @@ -196,7 +203,7 @@ impl InteractiveTxConstructor } if let None = self.context.inputs.insert(serial_id, TxIn { - previous_output: OutPoint { txid: msg.prevtx.txid(), vout: msg.prevtx_out }, + previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, sequence: Sequence(msg.sequence), ..Default::default() }) { From 487f2790acedd274aefab7cac1aecee2483464ff Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Fri, 14 Jul 2023 08:04:25 -0700 Subject: [PATCH 06/28] Wrap InteractiveTxStateMachine in constructor struct --- lightning/src/ln/interactivetxs.rs | 129 ++++++++++++++++++----------- 1 file changed, 80 insertions(+), 49 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index d3c6aaae045..5d00057e6b6 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -106,12 +106,12 @@ struct NegotiationContext { did_send_tx_signatures: bool, } -pub(crate) struct InteractiveTxConstructor { +pub(crate) struct InteractiveTxStateMachine { context: NegotiationContext, state: S, } -impl InteractiveTxConstructor { +impl InteractiveTxStateMachine { fn new( channel_id: [u8; 32], require_confirmed_inputs: bool, @@ -137,16 +137,16 @@ impl InteractiveTxConstructor { } } -impl InteractiveTxConstructor +impl InteractiveTxStateMachine where S: AcceptingChanges { fn abort_negotiation(self, reason: AbortReason) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxStateMachine> { - Err(InteractiveTxConstructor { context: self.context, state: NegotiationAborted(reason) }) + Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(reason) }) } fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxStateMachine> { // - TODO: MUST fail the negotiation if: // - `prevtx` is not a valid transaction if !self.is_valid_counterparty_serial_id(serial_id) { @@ -207,7 +207,7 @@ impl InteractiveTxConstructor sequence: Sequence(msg.sequence), ..Default::default() }) { - Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) + Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -217,14 +217,14 @@ impl InteractiveTxConstructor } fn receive_tx_remove_input(mut self, serial_id: SerialId) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxStateMachine> { if !self.is_valid_counterparty_serial_id(serial_id) { return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } if let Some(input) = self.context.inputs.remove(&serial_id) { self.context.prevtx_outpoints.remove(&input.previous_output); - Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) + Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -235,7 +235,7 @@ impl InteractiveTxConstructor } fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> - Result, InteractiveTxConstructor> { + Result, InteractiveTxStateMachine> { // TODO: the sats amount is less than the dust_limit self.context.received_tx_add_output_count += 1; if self.context.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { @@ -253,7 +253,7 @@ impl InteractiveTxConstructor } if let None = self.context.outputs.insert(serial_id, output) { - Ok(InteractiveTxConstructor { context: self.context, state: Negotiating {} }) + Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: // - MUST fail the negotiation if: @@ -262,21 +262,21 @@ impl InteractiveTxConstructor } } - pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxConstructor { + pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxStateMachine { todo!(); } - fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxConstructor { + fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { self.context.inputs.insert(serial_id, input); - InteractiveTxConstructor { context: self.context, state: Negotiating {} } + InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxConstructor { + pub(crate) fn send_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxStateMachine { self.context.outputs.insert(serial_id, output); - InteractiveTxConstructor { context: self.context, state: Negotiating {} } + InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_abort(mut self) -> InteractiveTxConstructor { + pub(crate) fn send_tx_abort(mut self) -> InteractiveTxStateMachine { // A sending node: // - MUST NOT have already transmitted tx_signatures // - SHOULD forget the current negotiation and reset their state. @@ -289,25 +289,25 @@ impl InteractiveTxConstructor } } -impl InteractiveTxConstructor { - fn send_tx_complete(self) -> InteractiveTxConstructor { - InteractiveTxConstructor { +impl InteractiveTxStateMachine { + fn send_tx_complete(self) -> InteractiveTxStateMachine { + InteractiveTxStateMachine { context: self.context, state: NegotiationComplete {} } } } -impl InteractiveTxConstructor { - fn receive_tx_complete(self) -> InteractiveTxConstructor { - InteractiveTxConstructor { +impl InteractiveTxStateMachine { + fn receive_tx_complete(self) -> InteractiveTxStateMachine { + InteractiveTxStateMachine { context: self.context, state: NegotiationComplete {} } } } -impl InteractiveTxConstructor { +impl InteractiveTxStateMachine { fn get_psbt(&self) -> Result { // Build transaction from inputs & outputs in `NegotiationContext`. return Ok(Transaction { @@ -319,12 +319,13 @@ impl InteractiveTxConstructor { } } + enum ChannelMode { - Negotiating(InteractiveTxConstructor), - OurTxComplete(InteractiveTxConstructor), - TheirTxComplete(InteractiveTxConstructor), - NegotiationComplete(InteractiveTxConstructor), - NegotiationAborted(InteractiveTxConstructor), + Negotiating(InteractiveTxStateMachine), + OurTxComplete(InteractiveTxStateMachine), + TheirTxComplete(InteractiveTxStateMachine), + NegotiationComplete(InteractiveTxStateMachine), + NegotiationAborted(InteractiveTxStateMachine), Indeterminate, } @@ -332,12 +333,56 @@ impl Default for ChannelMode { fn default() -> Self { Indeterminate } } +struct InteractiveTxConstructor { + mode: ChannelMode, +} + +impl InteractiveTxConstructor { + fn new( + channel_id: [u8; 32], + require_confirmed_inputs: bool, + is_initiator: bool, + base_tx: Transaction, + did_send_tx_signatures: bool, + ) -> Self { + let initial_state_machine = InteractiveTxStateMachine::new( + channel_id, + require_confirmed_inputs, + is_initiator, + base_tx, + did_send_tx_signatures + ); + Self { + mode: ChannelMode::Negotiating(initial_state_machine) + } + } + + fn add_tx_input(&mut self, transaction_input: TxAddInput) { + // We use mem::take here because we want to update `self.mode` based on its value and + // avoid cloning `ChannelMode`. + // By moving the value out of the struct, we can now safely modify it in this scope. + let mut mode = core::mem::take(&mut self.mode); + self.mode = if let ChannelMode::Negotiating(constructor) = mode { + match constructor.receive_tx_add_input( + 1234, + transaction_input, + true + ) { + Ok(c) => ChannelMode::Negotiating(c), + Err(c) => ChannelMode::NegotiationAborted(c), + } + } else { + mode + } + } +} + #[cfg(test)] mod tests { use core::str::FromStr; use std::collections::HashMap; use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; - use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor}; + use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine}; use crate::ln::msgs::TransactionU16LenLimited; use bitcoin::consensus::encode; use bitcoin::{Address, PackedLockTime, Script, Sequence, Transaction, Txid, TxIn, TxOut, Witness}; @@ -346,7 +391,7 @@ mod tests { use crate::ln::msgs::TxAddInput; struct DummyChannel { - mode: ChannelMode, + tx_constructor: InteractiveTxConstructor } impl DummyChannel { @@ -358,34 +403,20 @@ mod tests { 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); + Self { - mode: Negotiating(InteractiveTxConstructor::new( + tx_constructor: InteractiveTxConstructor::new( [0; 32], true, true, tx, false, - )) + ) } } fn handle_add_tx_input(&mut self) { - // We use mem::take here because we want to update `self.mode` based on its value and - // avoid cloning `ChannelMode`. - // By moving the value out of the struct, we can now safely modify it in this scope. - let mut mode = core::mem::take(&mut self.mode); - self.mode = if let Negotiating(constructor) = mode { - match constructor.receive_tx_add_input( - 1234, - get_sample_tx_add_input(), - true - ) { - Ok(c) => Negotiating(c), - Err(c) => NegotiationAborted(c), - } - } else { - mode - } + self.tx_constructor.add_tx_input(get_sample_tx_add_input()) } } From 46c5f1ade1c023bcdffa4c224eb0705db1beef96 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Fri, 14 Jul 2023 08:04:52 -0700 Subject: [PATCH 07/28] Add abort negotiation --- lightning/src/ln/interactivetxs.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 5d00057e6b6..3a6f4ef8776 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -375,6 +375,18 @@ impl InteractiveTxConstructor { mode } } + + fn abort_negotation(&mut self, reason: AbortReason) { + let mut mode = core::mem::take(&mut self.mode); + self.mode = if let ChannelMode::Negotiating(constructor) = mode { + match constructor.abort_negotiation(reason) { + Err(c) => ChannelMode::NegotiationAborted(c), + Ok(c) => ChannelMode::Negotiating(c), + } + } else { + mode + } + } } #[cfg(test)] From 2643fce5b2048d9a107aa0d0fa3cbf8d8e3a4749 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 12:57:53 -0700 Subject: [PATCH 08/28] Abstract mem::take to function --- lightning/src/ln/interactivetxs.rs | 35 ++++++++++++++++-------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3a6f4ef8776..3b50a2985a8 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -357,35 +357,38 @@ impl InteractiveTxConstructor { } } - fn add_tx_input(&mut self, transaction_input: TxAddInput) { + // Functions that handle the case where mode is [`ChannelMode::Negotiating`] + fn handle_negotiating(&mut self, f: F) + where F: FnOnce(InteractiveTxStateMachine) -> Result, InteractiveTxStateMachine> { // We use mem::take here because we want to update `self.mode` based on its value and // avoid cloning `ChannelMode`. // By moving the value out of the struct, we can now safely modify it in this scope. let mut mode = core::mem::take(&mut self.mode); self.mode = if let ChannelMode::Negotiating(constructor) = mode { - match constructor.receive_tx_add_input( - 1234, - transaction_input, - true - ) { + match f(constructor) { Ok(c) => ChannelMode::Negotiating(c), Err(c) => ChannelMode::NegotiationAborted(c), } } else { mode } + } fn abort_negotation(&mut self, reason: AbortReason) { - let mut mode = core::mem::take(&mut self.mode); - self.mode = if let ChannelMode::Negotiating(constructor) = mode { - match constructor.abort_negotiation(reason) { - Err(c) => ChannelMode::NegotiationAborted(c), - Ok(c) => ChannelMode::Negotiating(c), - } - } else { - mode - } + self.handle_negotiating(|state_machine| state_machine.abort_negotiation(reason)) + } + + fn add_tx_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { + self.handle_negotiating(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) + } + + fn remove_tx_input(&mut self, serial_id: SerialId) { + self.handle_negotiating(|state_machine| state_machine.receive_tx_remove_input(serial_id)) + } + + fn add_tx_output(&mut self, serial_id: SerialId, output: TxOut) { + self.handle_negotiating(|state_machine| state_machine.receive_tx_add_output(serial_id, output)) } } @@ -428,7 +431,7 @@ mod tests { } fn handle_add_tx_input(&mut self) { - self.tx_constructor.add_tx_input(get_sample_tx_add_input()) + self.tx_constructor.add_tx_input(1234, get_sample_tx_add_input(), true) } } From 6ffc4b3e1b478178951d343f29ac47a27f7a3bf8 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 16:45:13 -0700 Subject: [PATCH 09/28] Exhaustively handle adding/removing inputs and outputs --- lightning/src/ln/interactivetxs.rs | 97 +++++++++++++++++++++++------- 1 file changed, 76 insertions(+), 21 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3b50a2985a8..e88eca4e910 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -262,20 +262,39 @@ impl InteractiveTxStateMachine } } - pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxStateMachine { - todo!(); + fn receive_tx_remove_output(mut self, serial_id: SerialId) -> + Result, InteractiveTxStateMachine> { + if !self.is_valid_counterparty_serial_id(serial_id) { + return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); + } + + if let Some(output) = self.context.outputs.remove(&serial_id) { + Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) + } else { + self.abort_negotiation(AbortReason::SerialIdUnknown) + } } - fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { + pub(crate) fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { self.context.inputs.insert(serial_id, input); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxStateMachine { + pub(crate) fn send_tx_add_output(mut self, serial_id: SerialId, output: TxOut) -> InteractiveTxStateMachine { self.context.outputs.insert(serial_id, output); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } + pub(crate) fn send_tx_remove_input(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { + self.context.inputs.remove(&serial_id); + InteractiveTxStateMachine { context: self.context, state: Negotiating {} } + } + + pub(crate) fn send_tx_remove_output(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { + self.context.outputs.remove(&serial_id); + InteractiveTxStateMachine { context: self.context, state: Negotiating {} } + } + pub(crate) fn send_tx_abort(mut self) -> InteractiveTxStateMachine { // A sending node: // - MUST NOT have already transmitted tx_signatures @@ -283,6 +302,10 @@ impl InteractiveTxStateMachine todo!(); } + pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxStateMachine { + todo!(); + } + fn is_valid_counterparty_serial_id(&self, serial_id: SerialId) -> bool { // A received `SerialId`'s parity must match the role of the counterparty. self.context.holder_is_initiator == !serial_id.is_valid_for_initiator() @@ -358,7 +381,43 @@ impl InteractiveTxConstructor { } // Functions that handle the case where mode is [`ChannelMode::Negotiating`] - fn handle_negotiating(&mut self, f: F) + fn abort_negotation(&mut self, reason: AbortReason) { + self.handle_negotiating_receive(|state_machine| state_machine.abort_negotiation(reason)) + } + + fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { + self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) + } + + fn receive_tx_remove_input(&mut self, serial_id: SerialId) { + self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_remove_input(serial_id)) + } + + fn receive_tx_add_output(&mut self, serial_id: SerialId, output: TxOut) { + self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_output(serial_id, output)) + } + + fn receive_tx_remove_output(&mut self, serial_id: SerialId) { + self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_remove_output(serial_id)) + } + + fn send_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxIn) { + self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_input(serial_id, transaction_input)) + } + + fn send_tx_remove_input(&mut self, serial_id: SerialId) { + self.handle_negotiating_send(|state_machine| state_machine.send_tx_remove_input(serial_id)) + } + + fn send_tx_add_output(&mut self, serial_id: SerialId, transaction_output: TxOut) { + self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_output(serial_id, transaction_output)) + } + + fn send_tx_remove_output(&mut self, serial_id: SerialId) { + self.handle_negotiating_send(|state_machine| state_machine.send_tx_remove_output(serial_id)) + } + + fn handle_negotiating_receive(&mut self, f: F) where F: FnOnce(InteractiveTxStateMachine) -> Result, InteractiveTxStateMachine> { // We use mem::take here because we want to update `self.mode` based on its value and // avoid cloning `ChannelMode`. @@ -372,23 +431,19 @@ impl InteractiveTxConstructor { } else { mode } - } - fn abort_negotation(&mut self, reason: AbortReason) { - self.handle_negotiating(|state_machine| state_machine.abort_negotiation(reason)) - } - - fn add_tx_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { - self.handle_negotiating(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) - } - - fn remove_tx_input(&mut self, serial_id: SerialId) { - self.handle_negotiating(|state_machine| state_machine.receive_tx_remove_input(serial_id)) - } - - fn add_tx_output(&mut self, serial_id: SerialId, output: TxOut) { - self.handle_negotiating(|state_machine| state_machine.receive_tx_add_output(serial_id, output)) + fn handle_negotiating_send(&mut self, f: F) + where F: FnOnce(InteractiveTxStateMachine) -> InteractiveTxStateMachine { + // We use mem::take here because we want to update `self.mode` based on its value and + // avoid cloning `ChannelMode`. + // By moving the value out of the struct, we can now safely modify it in this scope. + let mut mode = core::mem::take(&mut self.mode); + self.mode = if let ChannelMode::Negotiating(constructor) = mode { + ChannelMode::Negotiating(f(constructor)) + } else { + mode + } } } @@ -431,7 +486,7 @@ mod tests { } fn handle_add_tx_input(&mut self) { - self.tx_constructor.add_tx_input(1234, get_sample_tx_add_input(), true) + self.tx_constructor.receive_tx_add_input(1234, get_sample_tx_add_input(), true) } } From 6779d74552dec7c8428f2404288158155878ed98 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 16:47:23 -0700 Subject: [PATCH 10/28] Added tx_complete handling to state machine --- lightning/src/ln/interactivetxs.rs | 82 ++++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index e88eca4e910..9231d8f0f79 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -21,6 +21,14 @@ const MAX_RECEIVED_TX_ADD_INPUT_COUNT: u16 = 4096; /// The number of received `tx_add_output` messages during a negotiation at which point the /// negotiation MUST be failed. const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; + +/// The number of inputs or outputs that the state machine can have, before it MUST fail the +/// negotiation. +const MAX_INPUTS_OUTPUTS_COUNT: usize = 252; + +/// Maximum weight of bitcoin transaction +const MAX_STANDARD_TX_WEIGHT: usize = 400_000; + const MAX_MONEY: u64 = 2_100_000_000_000_000; type SerialId = u64; @@ -42,6 +50,9 @@ pub(crate) enum AbortReason { DuplicateSerialId, PrevTxOutInvalid, ExceededMaximumSatsAllowed, + ExceededNumberOfInputsOrOutputs, + InvalidTransactionState, + TransactionTooLarge, } // Interactive Transaction Construction negotiation @@ -306,6 +317,49 @@ impl InteractiveTxStateMachine todo!(); } + fn is_current_transaction_state_able_to_complete(&self) -> Result<(), AbortReason> { + let tx_to_validate = Transaction { + version: self.context.base_tx.version, + lock_time: self.context.base_tx.lock_time, + input: self.context.inputs.values().cloned().collect(), + output: self.context.outputs.values().cloned().collect(), + }; + + // The receiving node: + // MUST fail the negotiation if: + + // TODO: Verify this is the correct way to do this. + // - the peer's total input satoshis is less than their outputs + let get_output = |outpoint: &OutPoint| { + if outpoint.txid == tx_to_validate.txid() { + return tx_to_validate.output.get(outpoint.vout as usize).cloned() + } else { + None + } + }; + if let Err(_) = tx_to_validate.verify(get_output) { + return Err(AbortReason::InvalidTransactionState) + }; + + // - there are more than 252 inputs + // - there are more than 252 outputs + if self.context.inputs.len() > MAX_INPUTS_OUTPUTS_COUNT || + self.context.outputs.len() > MAX_INPUTS_OUTPUTS_COUNT { + return Err(AbortReason::ExceededNumberOfInputsOrOutputs) + } + + if tx_to_validate.weight() > MAX_STANDARD_TX_WEIGHT { + return Err(AbortReason::TransactionTooLarge) + } + + // TODO: Need to figure out how to do this + // if is the non-initiator: + // - the initiator's fees do not cover the common fields (version, segwit marker + flag, + // input count, output count, locktime) + + return Ok(()) + } + fn is_valid_counterparty_serial_id(&self, serial_id: SerialId) -> bool { // A received `SerialId`'s parity must match the role of the counterparty. self.context.holder_is_initiator == !serial_id.is_valid_for_initiator() @@ -321,11 +375,33 @@ impl InteractiveTxStateMachine { } } -impl InteractiveTxStateMachine { - fn receive_tx_complete(self) -> InteractiveTxStateMachine { +impl InteractiveTxStateMachine { + fn receive_tx_complete(self) -> Result, InteractiveTxStateMachine> { + match self.is_current_transaction_state_able_to_complete() { + Err(e) => Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(e) }), + _ => Ok(InteractiveTxStateMachine { + context: self.context, + state: TheirTxComplete {} + }) + } + } + + fn send_tx_complete(self) -> InteractiveTxStateMachine { InteractiveTxStateMachine { context: self.context, - state: NegotiationComplete {} + state: OurTxComplete {} + } + } +} + +impl InteractiveTxStateMachine { + fn receive_tx_complete(self) -> Result, InteractiveTxStateMachine> { + match self.is_current_transaction_state_able_to_complete() { + Err(e) => Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(e) }), + _ => Ok(InteractiveTxStateMachine { + context: self.context, + state: NegotiationComplete {} + }) } } } From 2591320d1f9c217978039de2e2c6997d38a9c614 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 17:00:14 -0700 Subject: [PATCH 11/28] Add APIs for send/receive tx complete --- lightning/src/ln/interactivetxs.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 9231d8f0f79..3aa8e8527e8 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -493,6 +493,34 @@ impl InteractiveTxConstructor { self.handle_negotiating_send(|state_machine| state_machine.send_tx_remove_output(serial_id)) } + fn send_tx_complete(&mut self) { + let mut mode = core::mem::take(&mut self.mode); + self.mode = match mode { + ChannelMode::Negotiating(c) => { ChannelMode::OurTxComplete(c.send_tx_complete()) } + ChannelMode::TheirTxComplete(c) => { ChannelMode::NegotiationComplete(c.send_tx_complete()) } + _ => mode + } + } + + fn receive_tx_complete(&mut self) { + let mut mode = core::mem::take(&mut self.mode); + self.mode = match mode { + ChannelMode::Negotiating(c) => { + match c.receive_tx_complete() { + Ok(c) => ChannelMode::TheirTxComplete(c), + Err(c) => ChannelMode::NegotiationAborted(c) + } + } + ChannelMode::OurTxComplete(c) => { + match c.receive_tx_complete() { + Ok(c) => ChannelMode::NegotiationComplete(c), + Err(c) => ChannelMode::NegotiationAborted(c) + } + } + _ => mode + } + } + fn handle_negotiating_receive(&mut self, f: F) where F: FnOnce(InteractiveTxStateMachine) -> Result, InteractiveTxStateMachine> { // We use mem::take here because we want to update `self.mode` based on its value and From 26ce75f8f52f14fc35d2310a20ea867cde9764ff Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 17:13:19 -0700 Subject: [PATCH 12/28] Clean up comments --- lightning/src/ln/interactivetxs.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3aa8e8527e8..b9494b37260 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -87,6 +87,8 @@ pub(crate) enum AbortReason { // │ │ // └──────────────────────────────┘ // + +// Channel states that can receive `(send|receive)_tx_(add|remove)_(input|output)` pub(crate) trait AcceptingChanges {} /// We are currently in the process of negotiating the transaction. @@ -368,6 +370,8 @@ impl InteractiveTxStateMachine impl InteractiveTxStateMachine { fn send_tx_complete(self) -> InteractiveTxStateMachine { + // TODO: Should we validate before transitioning states? If so, do we want to abort negotiation + // if our current transaction state is invalid? InteractiveTxStateMachine { context: self.context, state: NegotiationComplete {} @@ -387,6 +391,8 @@ impl InteractiveTxStateMachine { } fn send_tx_complete(self) -> InteractiveTxStateMachine { + // TODO: Should we validate before transitioning states? If so, do we want to abort negotiation + // if our current transaction state is invalid? InteractiveTxStateMachine { context: self.context, state: OurTxComplete {} @@ -456,7 +462,6 @@ impl InteractiveTxConstructor { } } - // Functions that handle the case where mode is [`ChannelMode::Negotiating`] fn abort_negotation(&mut self, reason: AbortReason) { self.handle_negotiating_receive(|state_machine| state_machine.abort_negotiation(reason)) } From 1d789711bd2a18fcc4a67f97857caa4dfa5fbeb5 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 17:38:07 -0700 Subject: [PATCH 13/28] Tidy up visibility --- lightning/src/ln/interactivetxs.rs | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index b9494b37260..1de02a7f072 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -119,7 +119,7 @@ struct NegotiationContext { did_send_tx_signatures: bool, } -pub(crate) struct InteractiveTxStateMachine { +struct InteractiveTxStateMachine { context: NegotiationContext, state: S, } @@ -288,34 +288,34 @@ impl InteractiveTxStateMachine } } - pub(crate) fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { + fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { self.context.inputs.insert(serial_id, input); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_add_output(mut self, serial_id: SerialId, output: TxOut) -> InteractiveTxStateMachine { + fn send_tx_add_output(mut self, serial_id: SerialId, output: TxOut) -> InteractiveTxStateMachine { self.context.outputs.insert(serial_id, output); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_remove_input(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { + fn send_tx_remove_input(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { self.context.inputs.remove(&serial_id); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_remove_output(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { + fn send_tx_remove_output(mut self, serial_id: SerialId) -> InteractiveTxStateMachine { self.context.outputs.remove(&serial_id); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } - pub(crate) fn send_tx_abort(mut self) -> InteractiveTxStateMachine { + fn send_tx_abort(mut self) -> InteractiveTxStateMachine { // A sending node: // - MUST NOT have already transmitted tx_signatures // - SHOULD forget the current negotiation and reset their state. todo!(); } - pub(crate) fn receive_tx_abort(mut self) -> InteractiveTxStateMachine { + fn receive_tx_abort(mut self) -> InteractiveTxStateMachine { todo!(); } @@ -438,12 +438,12 @@ impl Default for ChannelMode { fn default() -> Self { Indeterminate } } -struct InteractiveTxConstructor { +pub(crate) struct InteractiveTxConstructor { mode: ChannelMode, } impl InteractiveTxConstructor { - fn new( + pub(crate) fn new( channel_id: [u8; 32], require_confirmed_inputs: bool, is_initiator: bool, @@ -462,43 +462,43 @@ impl InteractiveTxConstructor { } } - fn abort_negotation(&mut self, reason: AbortReason) { + pub(crate) fn abort_negotation(&mut self, reason: AbortReason) { self.handle_negotiating_receive(|state_machine| state_machine.abort_negotiation(reason)) } - fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { + pub(crate) fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) } - fn receive_tx_remove_input(&mut self, serial_id: SerialId) { + pub(crate) fn receive_tx_remove_input(&mut self, serial_id: SerialId) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_remove_input(serial_id)) } - fn receive_tx_add_output(&mut self, serial_id: SerialId, output: TxOut) { + pub(crate) fn receive_tx_add_output(&mut self, serial_id: SerialId, output: TxOut) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_output(serial_id, output)) } - fn receive_tx_remove_output(&mut self, serial_id: SerialId) { + pub(crate) fn receive_tx_remove_output(&mut self, serial_id: SerialId) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_remove_output(serial_id)) } - fn send_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxIn) { + pub(crate) fn send_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxIn) { self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_input(serial_id, transaction_input)) } - fn send_tx_remove_input(&mut self, serial_id: SerialId) { + pub(crate) fn send_tx_remove_input(&mut self, serial_id: SerialId) { self.handle_negotiating_send(|state_machine| state_machine.send_tx_remove_input(serial_id)) } - fn send_tx_add_output(&mut self, serial_id: SerialId, transaction_output: TxOut) { + pub(crate) fn send_tx_add_output(&mut self, serial_id: SerialId, transaction_output: TxOut) { self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_output(serial_id, transaction_output)) } - fn send_tx_remove_output(&mut self, serial_id: SerialId) { + pub(crate) fn send_tx_remove_output(&mut self, serial_id: SerialId) { self.handle_negotiating_send(|state_machine| state_machine.send_tx_remove_output(serial_id)) } - fn send_tx_complete(&mut self) { + pub(crate) fn send_tx_complete(&mut self) { let mut mode = core::mem::take(&mut self.mode); self.mode = match mode { ChannelMode::Negotiating(c) => { ChannelMode::OurTxComplete(c.send_tx_complete()) } @@ -507,7 +507,7 @@ impl InteractiveTxConstructor { } } - fn receive_tx_complete(&mut self) { + pub(crate) fn receive_tx_complete(&mut self) { let mut mode = core::mem::take(&mut self.mode); self.mode = match mode { ChannelMode::Negotiating(c) => { From 0dd26b97c34a93a7b77b4fa28ae424c06af5a76a Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Sun, 16 Jul 2023 17:38:18 -0700 Subject: [PATCH 14/28] [wip] add tests --- lightning/src/ln/interactivetxs.rs | 31 +++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 1de02a7f072..6e2a8290706 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -561,14 +561,43 @@ mod tests { use core::str::FromStr; use std::collections::HashMap; use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; - use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine}; + use crate::ln::interactivetxs::{AbortReason, ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine}; use crate::ln::msgs::TransactionU16LenLimited; use bitcoin::consensus::encode; use bitcoin::{Address, PackedLockTime, Script, Sequence, Transaction, Txid, TxIn, TxOut, Witness}; use bitcoin::hashes::hex::FromHex; use crate::chain::transaction::OutPoint; + use crate::ln::interactivetxs::AbortReason::IncorrectSerialIdParity; use crate::ln::msgs::TxAddInput; + #[test] + fn test_invalid_counterparty_serial_id_should_abort_negotiation() { + let tx: Transaction = encode::deserialize(&hex::decode("020000000001010e0ade\ + f48412e4361325ac1c6e36411299ab09d4f083b9d8ddb55fbc06e1b0c00000000000feffffff0220a107000\ + 0000000220020f81d95e040bd0a493e38bae27bff52fe2bb58b93b293eb579c01c31b05c5af1dc072cfee54\ + a3000016001434b1d6211af5551905dc2642d05f5b04d25a8fe80247304402207f570e3f0de50546aad25a8\ + 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ + dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ + b76bd00000000").unwrap()[..]).unwrap(); + + let mut constructor = InteractiveTxConstructor::new( + [0; 32], + true, + true, + tx, + false, + ); + + constructor.receive_tx_add_input( + 2, + get_sample_tx_add_input(), + false + ); + + assert!(matches!(constructor.mode, ChannelMode::NegotiationAborted { .. })) + } + + struct DummyChannel { tx_constructor: InteractiveTxConstructor } From 84035694e753896f1f249fe8191889116d2eaab8 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Fri, 4 Aug 2023 20:19:46 -0700 Subject: [PATCH 15/28] [wip] more things --- lightning/src/ln/interactivetxs.rs | 56 +++++++++++++++++++++--------- lightning/src/ln/msgs.rs | 2 +- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 6e2a8290706..028addf4c33 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -9,7 +9,8 @@ use std::collections::{HashMap, HashSet}; -use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; +use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint, Witness}; +use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use crate::ln::interactivetxs::ChannelMode::Indeterminate; use super::msgs::TxAddInput; @@ -112,11 +113,12 @@ struct NegotiationContext { holder_is_initiator: bool, received_tx_add_input_count: u16, received_tx_add_output_count: u16, - inputs: HashMap, + inputs: HashMap, prevtx_outpoints: HashSet, outputs: HashMap, base_tx: Transaction, did_send_tx_signatures: bool, + feerate_sat_per_kw: u32, } struct InteractiveTxStateMachine { @@ -127,6 +129,7 @@ struct InteractiveTxStateMachine { impl InteractiveTxStateMachine { fn new( channel_id: [u8; 32], + feerate_sat_per_kw: u32, require_confirmed_inputs: bool, is_initiator: bool, base_tx: Transaction, @@ -144,6 +147,7 @@ impl InteractiveTxStateMachine { inputs: HashMap::new(), prevtx_outpoints: HashSet::new(), outputs: HashMap::new(), + feerate_sat_per_kw: feerate_sat_per_kw, }, state: Negotiating, } @@ -160,7 +164,9 @@ impl InteractiveTxStateMachine fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> Result, InteractiveTxStateMachine> { - // - TODO: MUST fail the negotiation if: + // TODO: clean up this comment + // No need to explicitly fail negotiation since PeerManager will disconnect the peer if + // `prevtx` is invalid, implicitly ending negotiation. // - `prevtx` is not a valid transaction if !self.is_valid_counterparty_serial_id(serial_id) { // The receiving node: @@ -215,11 +221,17 @@ impl InteractiveTxStateMachine return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); } - if let None = self.context.inputs.insert(serial_id, TxIn { + let prev_out = if let Some(prev_out) = msg.prevtx.0.output.get(msg.prevtx_out as usize) { + prev_out.clone() + } else { + // TODO: New error + return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); + }; + if let None = self.context.inputs.insert(serial_id, (TxIn { previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, sequence: Sequence(msg.sequence), ..Default::default() - }) { + }, prev_out)) { Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: @@ -323,25 +335,20 @@ impl InteractiveTxStateMachine let tx_to_validate = Transaction { version: self.context.base_tx.version, lock_time: self.context.base_tx.lock_time, - input: self.context.inputs.values().cloned().collect(), + input: self.context.inputs.values().map(|(input, _)| input).cloned().collect(), output: self.context.outputs.values().cloned().collect(), }; // The receiving node: // MUST fail the negotiation if: - // TODO: Verify this is the correct way to do this. // - the peer's total input satoshis is less than their outputs - let get_output = |outpoint: &OutPoint| { - if outpoint.txid == tx_to_validate.txid() { - return tx_to_validate.output.get(outpoint.vout as usize).cloned() - } else { - None - } - }; - if let Err(_) = tx_to_validate.verify(get_output) { - return Err(AbortReason::InvalidTransactionState) - }; + let total_input_amount = self.context.inputs.values().map(|(_, prev_out)| prev_out.value).sum(); + let total_output_amount = tx_to_validate.output.iter().map(|output| output.value).sum(); + if total_input_amount < total_output_amount { + // TODO: New error + return Err(AbortReason::CounterpartyAborted); + } // - there are more than 252 inputs // - there are more than 252 outputs @@ -358,6 +365,21 @@ impl InteractiveTxStateMachine // if is the non-initiator: // - the initiator's fees do not cover the common fields (version, segwit marker + flag, // input count, output count, locktime) + if !self.context.holder_is_initiator { + let total_initiator_input_amount = self.context.inputs.iter().filter_map(|(serial_id, (input, prev_out))| + if (serial_id as SerialId).is_valid_for_initiator() { Some(prev_out.value) } else { None } + ).sum(); + let total_initiator_output_amount = self.context.outputs.iter().filter_map(|(serial_id, output)| + if (serial_id as SerialId).is_valid_for_initiator() { Some(output.value) } else { None } + ).sum(); + let initiator_fees_contributed = total_initiator_input_amount - total_initiator_output_amount; + let tx_common_fields_weight = (4 /* version */ + 4 /* locktime */ + 1 /* input count */ + 1 /* output count */) * WITNESS_SCALE_FACTOR as u64 + 2 /* segwit marker + flag */; + let tx_common_fields_fee = self.context.feerate_sat_per_kw as u64 * 1000 / tx_common_fields_weight; + if initiator_fees_contributed < tx_common_fields_weight { + // TODO: New error + return Err(AbortReason::CounterpartyAborted); + } + } return Ok(()) } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 27f8544e3b5..f6044cac28f 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -430,7 +430,7 @@ pub struct ChannelReady { /// /// Use [`TransactionU16LenLimited::into_transaction`] to convert into the contained `Transaction`. #[derive(Clone, Debug, PartialEq, Eq)] -pub struct TransactionU16LenLimited(Transaction); +pub struct TransactionU16LenLimited(pub Transaction); impl TransactionU16LenLimited { /// Constructs a new `TransactionU16LenLimited` from a `Transaction` only if it's consensus- From 5342b1a7d0e3bf07fe6d0534f596a1b812e3f3eb Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Mon, 7 Aug 2023 15:42:24 -0700 Subject: [PATCH 16/28] Make data structure for TxIn/TxOut tuple --- lightning/src/ln/interactivetxs.rs | 56 ++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 028addf4c33..526306dc8c2 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -107,13 +107,18 @@ pub(crate) struct NegotiationAborted(AbortReason); impl AcceptingChanges for Negotiating {} impl AcceptingChanges for OurTxComplete {} +struct TransactionInputWithPrevOutput { + input: TxIn, + prev_output: TxOut, +} + struct NegotiationContext { channel_id: [u8; 32], require_confirmed_inputs: bool, holder_is_initiator: bool, received_tx_add_input_count: u16, received_tx_add_output_count: u16, - inputs: HashMap, + inputs: HashMap, prevtx_outpoints: HashSet, outputs: HashMap, base_tx: Transaction, @@ -186,7 +191,7 @@ impl InteractiveTxStateMachine return self.abort_negotiation(AbortReason::InputsNotConfirmed); } - let transaction = msg.prevtx.into_transaction(); + let transaction = msg.prevtx.clone().into_transaction(); if let Some(tx_out) = transaction.output.get(msg.prevtx_out as usize) { if !tx_out.script_pubkey.is_witness_program() { @@ -227,11 +232,17 @@ impl InteractiveTxStateMachine // TODO: New error return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); }; - if let None = self.context.inputs.insert(serial_id, (TxIn { - previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, - sequence: Sequence(msg.sequence), - ..Default::default() - }, prev_out)) { + if let None = self.context.inputs.insert( + serial_id, + TransactionInputWithPrevOutput { + input: TxIn { + previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, + sequence: Sequence(msg.sequence), + ..Default::default() + }, + prev_output: prev_out + } + ) { Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: @@ -248,7 +259,7 @@ impl InteractiveTxStateMachine } if let Some(input) = self.context.inputs.remove(&serial_id) { - self.context.prevtx_outpoints.remove(&input.previous_output); + self.context.prevtx_outpoints.remove(&input.input.previous_output); Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { // The receiving node: @@ -300,8 +311,14 @@ impl InteractiveTxStateMachine } } - fn send_tx_add_input(mut self, serial_id: u64, input: TxIn) -> InteractiveTxStateMachine { - self.context.inputs.insert(serial_id, input); + fn send_tx_add_input(mut self, serial_id: u64, input: TxIn, prevout: TxOut) -> InteractiveTxStateMachine { + self.context.inputs.insert( + serial_id, + TransactionInputWithPrevOutput { + input: input, + prev_output: prevout + } + ); InteractiveTxStateMachine { context: self.context, state: Negotiating {} } } @@ -335,7 +352,7 @@ impl InteractiveTxStateMachine let tx_to_validate = Transaction { version: self.context.base_tx.version, lock_time: self.context.base_tx.lock_time, - input: self.context.inputs.values().map(|(input, _)| input).cloned().collect(), + input: self.context.inputs.values().map(|p| p.input.clone()).collect(), output: self.context.outputs.values().cloned().collect(), }; @@ -343,7 +360,7 @@ impl InteractiveTxStateMachine // MUST fail the negotiation if: // - the peer's total input satoshis is less than their outputs - let total_input_amount = self.context.inputs.values().map(|(_, prev_out)| prev_out.value).sum(); + let total_input_amount: u64 = self.context.inputs.values().map(|p| p.prev_output.value).sum(); let total_output_amount = tx_to_validate.output.iter().map(|output| output.value).sum(); if total_input_amount < total_output_amount { // TODO: New error @@ -366,11 +383,11 @@ impl InteractiveTxStateMachine // - the initiator's fees do not cover the common fields (version, segwit marker + flag, // input count, output count, locktime) if !self.context.holder_is_initiator { - let total_initiator_input_amount = self.context.inputs.iter().filter_map(|(serial_id, (input, prev_out))| - if (serial_id as SerialId).is_valid_for_initiator() { Some(prev_out.value) } else { None } + let total_initiator_input_amount: u64 = self.context.inputs.iter().filter_map(|(serial_id, input_with_prevout)| + if serial_id.is_valid_for_initiator() { Some(input_with_prevout.prev_output.value) } else { None } ).sum(); - let total_initiator_output_amount = self.context.outputs.iter().filter_map(|(serial_id, output)| - if (serial_id as SerialId).is_valid_for_initiator() { Some(output.value) } else { None } + let total_initiator_output_amount: u64 = self.context.outputs.iter().filter_map(|(serial_id, output)| + if serial_id.is_valid_for_initiator() { Some(output.value) } else { None } ).sum(); let initiator_fees_contributed = total_initiator_input_amount - total_initiator_output_amount; let tx_common_fields_weight = (4 /* version */ + 4 /* locktime */ + 1 /* input count */ + 1 /* output count */) * WITNESS_SCALE_FACTOR as u64 + 2 /* segwit marker + flag */; @@ -440,7 +457,7 @@ impl InteractiveTxStateMachine { return Ok(Transaction { version: self.context.base_tx.version, lock_time: self.context.base_tx.lock_time, - input: self.context.inputs.values().cloned().collect(), + input: self.context.inputs.values().map(|p| p.input.clone()).collect(), output: self.context.outputs.values().cloned().collect(), }) } @@ -474,6 +491,7 @@ impl InteractiveTxConstructor { ) -> Self { let initial_state_machine = InteractiveTxStateMachine::new( channel_id, + 4, require_confirmed_inputs, is_initiator, base_tx, @@ -504,8 +522,8 @@ impl InteractiveTxConstructor { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_remove_output(serial_id)) } - pub(crate) fn send_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxIn) { - self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_input(serial_id, transaction_input)) + pub(crate) fn send_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxIn, previous_output: TxOut) { + self.handle_negotiating_send(|state_machine| state_machine.send_tx_add_input(serial_id, transaction_input, previous_output)) } pub(crate) fn send_tx_remove_input(&mut self, serial_id: SerialId) { From 3ceb99ca0baa548ebfed54bb04bc1f0b095b85b6 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 7 Aug 2023 16:59:15 -0700 Subject: [PATCH 17/28] MOAR validation --- lightning/src/ln/interactivetxs.rs | 99 +++++++++++++++++++++++++----- 1 file changed, 84 insertions(+), 15 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 526306dc8c2..55b2b70a6a2 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -126,6 +126,32 @@ struct NegotiationContext { feerate_sat_per_kw: u32, } +impl NegotiationContext { + fn initiator_inputs_contributed(&self) -> impl Iterator { + self.inputs.iter() + .filter(|(serial_id, _)| serial_id.is_valid_for_initiator()) + .map(|(_, input_with_prevout)| input_with_prevout) + } + + fn non_initiator_inputs_contributed(&self) -> impl Iterator { + self.inputs.iter() + .filter(|(serial_id, _)| !serial_id.is_valid_for_initiator()) + .map(|(_, input_with_prevout)| input_with_prevout) + } + + fn initiator_outputs_contributed(&self) -> impl Iterator { + self.outputs.iter() + .filter(|(serial_id, _)| serial_id.is_valid_for_initiator()) + .map(|(_, output)| output) + } + + fn non_initiator_outputs_contributed(&self) -> impl Iterator { + self.outputs.iter() + .filter(|(serial_id, _)| !serial_id.is_valid_for_initiator()) + .map(|(_, output)| output) + } +} + struct InteractiveTxStateMachine { context: NegotiationContext, state: S, @@ -272,7 +298,18 @@ impl InteractiveTxStateMachine fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> Result, InteractiveTxStateMachine> { - // TODO: the sats amount is less than the dust_limit + // The receiving node: + // - MUST fail the negotiation if: + // - the serial_id has the wrong parity + if serial_id.is_valid_for_initiator() && self.context.holder_is_initiator { + // TODO: New error + return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); + } + if !serial_id.is_valid_for_initiator() && !self.context.holder_is_initiator { + // TODO: Same error as above + return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); + } + self.context.received_tx_add_output_count += 1; if self.context.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT { // The receiving node: @@ -281,6 +318,13 @@ impl InteractiveTxStateMachine return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); } + if output.value < output.script_pubkey.dust_value().to_sat() { + // The receiving node: + // - MUST fail the negotiation if: + // - the sats amount is less than the dust_limit + // TODO: New error + return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); + } if output.value > MAX_MONEY { // The receiving node: // - MUST fail the negotiation if: @@ -288,6 +332,16 @@ impl InteractiveTxStateMachine return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); } + // The receiving node: + // - MUST accept P2WSH, P2WPKH, P2TR scripts + // - MAY fail the negotiation if script is non-standard + if !output.script_pubkey.is_v0_p2wpkh() && !output.script_pubkey.is_v0_p2wsh() && + !output.script_pubkey.is_v1_p2tr() + { + // TODO: New error + return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); + } + if let None = self.context.outputs.insert(serial_id, output) { Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { @@ -303,7 +357,7 @@ impl InteractiveTxStateMachine if !self.is_valid_counterparty_serial_id(serial_id) { return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } - + if let Some(output) = self.context.outputs.remove(&serial_id) { Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { @@ -378,21 +432,36 @@ impl InteractiveTxStateMachine return Err(AbortReason::TransactionTooLarge) } - // TODO: Need to figure out how to do this - // if is the non-initiator: - // - the initiator's fees do not cover the common fields (version, segwit marker + flag, - // input count, output count, locktime) - if !self.context.holder_is_initiator { - let total_initiator_input_amount: u64 = self.context.inputs.iter().filter_map(|(serial_id, input_with_prevout)| - if serial_id.is_valid_for_initiator() { Some(input_with_prevout.prev_output.value) } else { None } - ).sum(); - let total_initiator_output_amount: u64 = self.context.outputs.iter().filter_map(|(serial_id, output)| - if serial_id.is_valid_for_initiator() { Some(output.value) } else { None } - ).sum(); - let initiator_fees_contributed = total_initiator_input_amount - total_initiator_output_amount; + // TODO: + // - Use existing rust-lightning/rust-bitcoin constants. + // - How do we enforce their fees cover the witness without knowing its expected length? + // - Read eclair's code to see if they do this? + const INPUT_WEIGHT: u64 = (32 + 4 + 4) * WITNESS_SCALE_FACTOR as u64; + const OUTPUT_WEIGHT: u64 = 8 * WITNESS_SCALE_FACTOR as u64; + + // - the peer's paid feerate does not meet or exceed the agreed feerate (based on the minimum fee). + if self.context.holder_is_initiator { + let non_initiator_fees_contributed: u64 = self.context.non_initiator_outputs_contributed().map(|output| output.value).sum::() - + self.context.non_initiator_inputs_contributed().map(|input| input.prev_output.value).sum::(); + let non_initiator_contribution_weight = self.context.non_initiator_inputs_contributed().count() as u64 * INPUT_WEIGHT + + self.context.non_initiator_outputs_contributed().count() as u64 * OUTPUT_WEIGHT; + let required_non_initiator_contribution_fee = self.context.feerate_sat_per_kw as u64 * 1000 / non_initiator_contribution_weight; + if non_initiator_fees_contributed < required_non_initiator_contribution_fee { + // TODO: New error + return Err(AbortReason::CounterpartyAborted); + } + } else { + // if is the non-initiator: + // - the initiator's fees do not cover the common fields (version, segwit marker + flag, + // input count, output count, locktime) + let initiator_fees_contributed: u64 = self.context.initiator_outputs_contributed().map(|output| output.value).sum::() - + self.context.initiator_inputs_contributed().map(|input| input.prev_output.value).sum::(); + let initiator_contribution_weight = self.context.initiator_inputs_contributed().count() as u64 * INPUT_WEIGHT + + self.context.initiator_outputs_contributed().count() as u64 * OUTPUT_WEIGHT; + let required_initiator_contribution_fee = self.context.feerate_sat_per_kw as u64 * 1000 / initiator_contribution_weight; let tx_common_fields_weight = (4 /* version */ + 4 /* locktime */ + 1 /* input count */ + 1 /* output count */) * WITNESS_SCALE_FACTOR as u64 + 2 /* segwit marker + flag */; let tx_common_fields_fee = self.context.feerate_sat_per_kw as u64 * 1000 / tx_common_fields_weight; - if initiator_fees_contributed < tx_common_fields_weight { + if initiator_fees_contributed < tx_common_fields_fee + required_initiator_contribution_fee { // TODO: New error return Err(AbortReason::CounterpartyAborted); } From 4b9f2374ff8365a7eca50fc5e269f66a3afbbdbb Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 8 Aug 2023 09:46:15 -0700 Subject: [PATCH 18/28] Minor cleanup --- lightning/src/ln/interactivetxs.rs | 126 ++++++++++------------------- 1 file changed, 44 insertions(+), 82 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 55b2b70a6a2..ae7d062260c 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -11,9 +11,10 @@ use std::collections::{HashMap, HashSet}; use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint, Witness}; use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; -use crate::ln::interactivetxs::ChannelMode::Indeterminate; +use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; -use super::msgs::TxAddInput; +use crate::ln::interactivetxs::ChannelMode::Indeterminate; +use crate::ln::msgs; /// The number of received `tx_add_input` messages during a negotiation at which point the /// negotiation MUST be failed. @@ -27,9 +28,6 @@ const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; /// negotiation. const MAX_INPUTS_OUTPUTS_COUNT: usize = 252; -/// Maximum weight of bitcoin transaction -const MAX_STANDARD_TX_WEIGHT: usize = 400_000; - const MAX_MONEY: u64 = 2_100_000_000_000_000; type SerialId = u64; @@ -107,7 +105,7 @@ pub(crate) struct NegotiationAborted(AbortReason); impl AcceptingChanges for Negotiating {} impl AcceptingChanges for OurTxComplete {} -struct TransactionInputWithPrevOutput { +struct TxInputWithPrevOutput { input: TxIn, prev_output: TxOut, } @@ -118,7 +116,7 @@ struct NegotiationContext { holder_is_initiator: bool, received_tx_add_input_count: u16, received_tx_add_output_count: u16, - inputs: HashMap, + inputs: HashMap, prevtx_outpoints: HashSet, outputs: HashMap, base_tx: Transaction, @@ -127,13 +125,13 @@ struct NegotiationContext { } impl NegotiationContext { - fn initiator_inputs_contributed(&self) -> impl Iterator { + fn initiator_inputs_contributed(&self) -> impl Iterator { self.inputs.iter() .filter(|(serial_id, _)| serial_id.is_valid_for_initiator()) .map(|(_, input_with_prevout)| input_with_prevout) } - fn non_initiator_inputs_contributed(&self) -> impl Iterator { + fn non_initiator_inputs_contributed(&self) -> impl Iterator { self.inputs.iter() .filter(|(serial_id, _)| !serial_id.is_valid_for_initiator()) .map(|(_, input_with_prevout)| input_with_prevout) @@ -157,14 +155,13 @@ struct InteractiveTxStateMachine { state: S, } +type InteractiveTxStateMachineResult = + Result, InteractiveTxStateMachine>; + impl InteractiveTxStateMachine { fn new( - channel_id: [u8; 32], - feerate_sat_per_kw: u32, - require_confirmed_inputs: bool, - is_initiator: bool, - base_tx: Transaction, - did_send_tx_signatures: bool, + channel_id: [u8; 32], feerate_sat_per_kw: u32, require_confirmed_inputs: bool, + is_initiator: bool, base_tx: Transaction, did_send_tx_signatures: bool, ) -> Self { Self { context: NegotiationContext { @@ -178,23 +175,19 @@ impl InteractiveTxStateMachine { inputs: HashMap::new(), prevtx_outpoints: HashSet::new(), outputs: HashMap::new(), - feerate_sat_per_kw: feerate_sat_per_kw, + feerate_sat_per_kw, }, state: Negotiating, } } } -impl InteractiveTxStateMachine - where S: AcceptingChanges { - fn abort_negotiation(self, reason: AbortReason) -> - Result, InteractiveTxStateMachine> { - +impl InteractiveTxStateMachine where S: AcceptingChanges { + fn abort_negotiation(self, reason: AbortReason) -> InteractiveTxStateMachineResult { Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(reason) }) } - fn receive_tx_add_input(mut self, serial_id: SerialId, msg: TxAddInput, confirmed: bool) -> - Result, InteractiveTxStateMachine> { + fn receive_tx_add_input(mut self, serial_id: SerialId, msg: &msgs::TxAddInput, confirmed: bool) -> InteractiveTxStateMachineResult { // TODO: clean up this comment // No need to explicitly fail negotiation since PeerManager will disconnect the peer if // `prevtx` is invalid, implicitly ending negotiation. @@ -260,7 +253,7 @@ impl InteractiveTxStateMachine }; if let None = self.context.inputs.insert( serial_id, - TransactionInputWithPrevOutput { + TxInputWithPrevOutput { input: TxIn { previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, sequence: Sequence(msg.sequence), @@ -278,8 +271,7 @@ impl InteractiveTxStateMachine } } - fn receive_tx_remove_input(mut self, serial_id: SerialId) -> - Result, InteractiveTxStateMachine> { + fn receive_tx_remove_input(mut self, serial_id: SerialId) -> InteractiveTxStateMachineResult { if !self.is_valid_counterparty_serial_id(serial_id) { return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } @@ -296,18 +288,12 @@ impl InteractiveTxStateMachine } } - fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> - Result, InteractiveTxStateMachine> { + fn receive_tx_add_output(mut self, serial_id: u64, output: TxOut) -> InteractiveTxStateMachineResult { // The receiving node: // - MUST fail the negotiation if: // - the serial_id has the wrong parity - if serial_id.is_valid_for_initiator() && self.context.holder_is_initiator { - // TODO: New error - return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); - } - if !serial_id.is_valid_for_initiator() && !self.context.holder_is_initiator { - // TODO: Same error as above - return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddOutputs); + if !self.is_valid_counterparty_serial_id(serial_id) { + return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } self.context.received_tx_add_output_count += 1; @@ -352,8 +338,7 @@ impl InteractiveTxStateMachine } } - fn receive_tx_remove_output(mut self, serial_id: SerialId) -> - Result, InteractiveTxStateMachine> { + fn receive_tx_remove_output(mut self, serial_id: SerialId) -> InteractiveTxStateMachineResult { if !self.is_valid_counterparty_serial_id(serial_id) { return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } @@ -368,7 +353,7 @@ impl InteractiveTxStateMachine fn send_tx_add_input(mut self, serial_id: u64, input: TxIn, prevout: TxOut) -> InteractiveTxStateMachine { self.context.inputs.insert( serial_id, - TransactionInputWithPrevOutput { + TxInputWithPrevOutput { input: input, prev_output: prevout } @@ -488,7 +473,7 @@ impl InteractiveTxStateMachine { } impl InteractiveTxStateMachine { - fn receive_tx_complete(self) -> Result, InteractiveTxStateMachine> { + fn receive_tx_complete(self) -> InteractiveTxStateMachineResult { match self.is_current_transaction_state_able_to_complete() { Err(e) => Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(e) }), _ => Ok(InteractiveTxStateMachine { @@ -509,7 +494,7 @@ impl InteractiveTxStateMachine { } impl InteractiveTxStateMachine { - fn receive_tx_complete(self) -> Result, InteractiveTxStateMachine> { + fn receive_tx_complete(self) -> InteractiveTxStateMachineResult { match self.is_current_transaction_state_able_to_complete() { Err(e) => Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(e) }), _ => Ok(InteractiveTxStateMachine { @@ -552,18 +537,11 @@ pub(crate) struct InteractiveTxConstructor { impl InteractiveTxConstructor { pub(crate) fn new( - channel_id: [u8; 32], - require_confirmed_inputs: bool, - is_initiator: bool, - base_tx: Transaction, - did_send_tx_signatures: bool, + channel_id: [u8; 32], feerate_sat_per_kw: u32, require_confirmed_inputs: bool, + is_initiator: bool, base_tx: Transaction, did_send_tx_signatures: bool, ) -> Self { let initial_state_machine = InteractiveTxStateMachine::new( - channel_id, - 4, - require_confirmed_inputs, - is_initiator, - base_tx, + channel_id, feerate_sat_per_kw, require_confirmed_inputs, is_initiator, base_tx, did_send_tx_signatures ); Self { @@ -575,7 +553,7 @@ impl InteractiveTxConstructor { self.handle_negotiating_receive(|state_machine| state_machine.abort_negotiation(reason)) } - pub(crate) fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: TxAddInput, confirmed: bool) { + pub(crate) fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: &msgs::TxAddInput, confirmed: bool) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) } @@ -617,7 +595,7 @@ impl InteractiveTxConstructor { } pub(crate) fn receive_tx_complete(&mut self) { - let mut mode = core::mem::take(&mut self.mode); + let mode = core::mem::take(&mut self.mode); self.mode = match mode { ChannelMode::Negotiating(c) => { match c.receive_tx_complete() { @@ -636,11 +614,13 @@ impl InteractiveTxConstructor { } fn handle_negotiating_receive(&mut self, f: F) - where F: FnOnce(InteractiveTxStateMachine) -> Result, InteractiveTxStateMachine> { + where + F: FnOnce(InteractiveTxStateMachine) -> InteractiveTxStateMachineResult + { // We use mem::take here because we want to update `self.mode` based on its value and // avoid cloning `ChannelMode`. // By moving the value out of the struct, we can now safely modify it in this scope. - let mut mode = core::mem::take(&mut self.mode); + let mode = core::mem::take(&mut self.mode); self.mode = if let ChannelMode::Negotiating(constructor) = mode { match f(constructor) { Ok(c) => ChannelMode::Negotiating(c), @@ -652,11 +632,13 @@ impl InteractiveTxConstructor { } fn handle_negotiating_send(&mut self, f: F) - where F: FnOnce(InteractiveTxStateMachine) -> InteractiveTxStateMachine { + where + F: FnOnce(InteractiveTxStateMachine) -> InteractiveTxStateMachine + { // We use mem::take here because we want to update `self.mode` based on its value and // avoid cloning `ChannelMode`. // By moving the value out of the struct, we can now safely modify it in this scope. - let mut mode = core::mem::take(&mut self.mode); + let mode = core::mem::take(&mut self.mode); self.mode = if let ChannelMode::Negotiating(constructor) = mode { ChannelMode::Negotiating(f(constructor)) } else { @@ -668,8 +650,8 @@ impl InteractiveTxConstructor { #[cfg(test)] mod tests { use core::str::FromStr; - use std::collections::HashMap; - use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; + use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW; +use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; use crate::ln::interactivetxs::{AbortReason, ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine}; use crate::ln::msgs::TransactionU16LenLimited; use bitcoin::consensus::encode; @@ -688,21 +670,8 @@ mod tests { 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); - - let mut constructor = InteractiveTxConstructor::new( - [0; 32], - true, - true, - tx, - false, - ); - - constructor.receive_tx_add_input( - 2, - get_sample_tx_add_input(), - false - ); - + let mut constructor = InteractiveTxConstructor::new([0; 32], FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false); + constructor.receive_tx_add_input(2, &get_sample_tx_add_input(), false); assert!(matches!(constructor.mode, ChannelMode::NegotiationAborted { .. })) } @@ -720,20 +689,13 @@ mod tests { 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); - Self { - tx_constructor: InteractiveTxConstructor::new( - [0; 32], - true, - true, - tx, - false, - ) + tx_constructor: InteractiveTxConstructor::new([0; 32], FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false) } } fn handle_add_tx_input(&mut self) { - self.tx_constructor.receive_tx_add_input(1234, get_sample_tx_add_input(), true) + self.tx_constructor.receive_tx_add_input(1234, &get_sample_tx_add_input(), true) } } From 9b4f2e563ae7fea5a31e63fbfbd8ba0112037fa7 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Tue, 8 Aug 2023 20:26:30 -0700 Subject: [PATCH 19/28] Add better errors --- lightning/src/ln/interactivetxs.rs | 33 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ae7d062260c..e74ba3963b3 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -52,6 +52,10 @@ pub(crate) enum AbortReason { ExceededNumberOfInputsOrOutputs, InvalidTransactionState, TransactionTooLarge, + ExceededDustLimit, + InvalidOutputScript, + InsufficientFees, + OutputsExceedInputs, } // Interactive Transaction Construction negotiation @@ -188,10 +192,11 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { } fn receive_tx_add_input(mut self, serial_id: SerialId, msg: &msgs::TxAddInput, confirmed: bool) -> InteractiveTxStateMachineResult { - // TODO: clean up this comment - // No need to explicitly fail negotiation since PeerManager will disconnect the peer if - // `prevtx` is invalid, implicitly ending negotiation. - // - `prevtx` is not a valid transaction + // The interactive-txs spec calls for us to fail negotiation if the `prevtx` we receive is + // invalid. However, we would not need to account for this explicit negotiation failure + // mode here since `PeerManager` would already disconnect the peer if the `prevtx` is + // invalid; implicitly ending the negotiation. + if !self.is_valid_counterparty_serial_id(serial_id) { // The receiving node: // - MUST fail the negotiation if: @@ -248,8 +253,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { let prev_out = if let Some(prev_out) = msg.prevtx.0.output.get(msg.prevtx_out as usize) { prev_out.clone() } else { - // TODO: New error - return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); + return self.abort_negotiation(AbortReason::PrevTxOutInvalid); }; if let None = self.context.inputs.insert( serial_id, @@ -308,8 +312,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { // The receiving node: // - MUST fail the negotiation if: // - the sats amount is less than the dust_limit - // TODO: New error - return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); + return self.abort_negotiation(AbortReason::ExceededDustLimit); } if output.value > MAX_MONEY { // The receiving node: @@ -324,8 +327,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { if !output.script_pubkey.is_v0_p2wpkh() && !output.script_pubkey.is_v0_p2wsh() && !output.script_pubkey.is_v1_p2tr() { - // TODO: New error - return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); + return self.abort_negotiation(AbortReason::InvalidOutputScript); } if let None = self.context.outputs.insert(serial_id, output) { @@ -402,8 +404,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { let total_input_amount: u64 = self.context.inputs.values().map(|p| p.prev_output.value).sum(); let total_output_amount = tx_to_validate.output.iter().map(|output| output.value).sum(); if total_input_amount < total_output_amount { - // TODO: New error - return Err(AbortReason::CounterpartyAborted); + return Err(AbortReason::OutputsExceedInputs); } // - there are more than 252 inputs @@ -413,7 +414,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { return Err(AbortReason::ExceededNumberOfInputsOrOutputs) } - if tx_to_validate.weight() > MAX_STANDARD_TX_WEIGHT { + if tx_to_validate.weight() as u32 > MAX_STANDARD_TX_WEIGHT { return Err(AbortReason::TransactionTooLarge) } @@ -432,8 +433,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { self.context.non_initiator_outputs_contributed().count() as u64 * OUTPUT_WEIGHT; let required_non_initiator_contribution_fee = self.context.feerate_sat_per_kw as u64 * 1000 / non_initiator_contribution_weight; if non_initiator_fees_contributed < required_non_initiator_contribution_fee { - // TODO: New error - return Err(AbortReason::CounterpartyAborted); + return Err(AbortReason::InsufficientFees); } } else { // if is the non-initiator: @@ -447,8 +447,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { let tx_common_fields_weight = (4 /* version */ + 4 /* locktime */ + 1 /* input count */ + 1 /* output count */) * WITNESS_SCALE_FACTOR as u64 + 2 /* segwit marker + flag */; let tx_common_fields_fee = self.context.feerate_sat_per_kw as u64 * 1000 / tx_common_fields_weight; if initiator_fees_contributed < tx_common_fields_fee + required_initiator_contribution_fee { - // TODO: New error - return Err(AbortReason::CounterpartyAborted); + return Err(AbortReason::InsufficientFees); } } From 94deca8447dec810b2fb5d824e6eff901a223f0d Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Tue, 8 Aug 2023 20:39:57 -0700 Subject: [PATCH 20/28] Use 21M from channel.rs --- lightning/src/ln/interactivetxs.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index e74ba3963b3..1415f761bda 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -12,6 +12,7 @@ use std::collections::{HashMap, HashSet}; use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint, Witness}; use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; +use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::interactivetxs::ChannelMode::Indeterminate; use crate::ln::msgs; @@ -28,8 +29,6 @@ const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; /// negotiation. const MAX_INPUTS_OUTPUTS_COUNT: usize = 252; -const MAX_MONEY: u64 = 2_100_000_000_000_000; - type SerialId = u64; trait SerialIdExt { fn is_valid_for_initiator(&self) -> bool; @@ -314,10 +313,10 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { // - the sats amount is less than the dust_limit return self.abort_negotiation(AbortReason::ExceededDustLimit); } - if output.value > MAX_MONEY { + if output.value > TOTAL_BITCOIN_SUPPLY_SATOSHIS { // The receiving node: // - MUST fail the negotiation if: - // - the sats amount is greater than 2,100,000,000,000,000 (MAX_MONEY) + // - the sats amount is greater than 2,100,000,000,000,000 (TOTAL_BITCOIN_SUPPLY_SATOSHIS) return self.abort_negotiation(AbortReason::ExceededMaximumSatsAllowed); } From f10a639bafc8bb1e8cc2a6eb4b454277918d05d6 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Tue, 8 Aug 2023 20:40:09 -0700 Subject: [PATCH 21/28] Add more TODOs --- lightning/src/ln/interactivetxs.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 1415f761bda..2a48c310d66 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -190,6 +190,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(reason) }) } + // TODO: change `serial_id` field in `TxAddInput` to be `SerialId` instead of `u64` fn receive_tx_add_input(mut self, serial_id: SerialId, msg: &msgs::TxAddInput, confirmed: bool) -> InteractiveTxStateMachineResult { // The interactive-txs spec calls for us to fail negotiation if the `prevtx` we receive is // invalid. However, we would not need to account for this explicit negotiation failure @@ -506,6 +507,7 @@ impl InteractiveTxStateMachine { impl InteractiveTxStateMachine { fn get_psbt(&self) -> Result { // Build transaction from inputs & outputs in `NegotiationContext`. + // TODO: Return Psbt type (?) return Ok(Transaction { version: self.context.base_tx.version, lock_time: self.context.base_tx.lock_time, @@ -533,6 +535,10 @@ pub(crate) struct InteractiveTxConstructor { mode: ChannelMode, } +// TODO: `InteractiveTxConstructor` methods should return an `Err` when the state machine itself +// errors out. There are two scenarios where that may occur: (1) Invalid data; causing negotiation +// to abort (2) Illegal state transition. Check spec to see if it dictates what needs to happen +// if a node receives an unexpected message. impl InteractiveTxConstructor { pub(crate) fn new( channel_id: [u8; 32], feerate_sat_per_kw: u32, require_confirmed_inputs: bool, From 4d72dfc6ffc04c3a1f2ba9b73ef9e3383adc7e09 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Wed, 16 Aug 2023 09:59:22 -0700 Subject: [PATCH 22/28] Change serial_id in messages to SerialId --- lightning/src/ln/interactivetxs.rs | 15 +++++++-------- lightning/src/ln/msgs.rs | 10 ++++++---- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 2a48c310d66..33cf52e7588 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -16,6 +16,7 @@ use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::interactivetxs::ChannelMode::Indeterminate; use crate::ln::msgs; +use crate::ln::msgs::SerialId; /// The number of received `tx_add_input` messages during a negotiation at which point the /// negotiation MUST be failed. @@ -29,7 +30,6 @@ const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096; /// negotiation. const MAX_INPUTS_OUTPUTS_COUNT: usize = 252; -type SerialId = u64; trait SerialIdExt { fn is_valid_for_initiator(&self) -> bool; } @@ -119,9 +119,9 @@ struct NegotiationContext { holder_is_initiator: bool, received_tx_add_input_count: u16, received_tx_add_output_count: u16, - inputs: HashMap, + inputs: HashMap, prevtx_outpoints: HashSet, - outputs: HashMap, + outputs: HashMap, base_tx: Transaction, did_send_tx_signatures: bool, feerate_sat_per_kw: u32, @@ -190,14 +190,13 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(reason) }) } - // TODO: change `serial_id` field in `TxAddInput` to be `SerialId` instead of `u64` - fn receive_tx_add_input(mut self, serial_id: SerialId, msg: &msgs::TxAddInput, confirmed: bool) -> InteractiveTxStateMachineResult { + fn receive_tx_add_input(mut self, msg: &msgs::TxAddInput, confirmed: bool) -> InteractiveTxStateMachineResult { // The interactive-txs spec calls for us to fail negotiation if the `prevtx` we receive is // invalid. However, we would not need to account for this explicit negotiation failure // mode here since `PeerManager` would already disconnect the peer if the `prevtx` is // invalid; implicitly ending the negotiation. - if !self.is_valid_counterparty_serial_id(serial_id) { + if !self.is_valid_counterparty_serial_id(msg.serial_id) { // The receiving node: // - MUST fail the negotiation if: // - the `serial_id` has the wrong parity @@ -256,7 +255,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { return self.abort_negotiation(AbortReason::PrevTxOutInvalid); }; if let None = self.context.inputs.insert( - serial_id, + msg.serial_id, TxInputWithPrevOutput { input: TxIn { previous_output: OutPoint { txid: transaction.txid(), vout: msg.prevtx_out }, @@ -558,7 +557,7 @@ impl InteractiveTxConstructor { } pub(crate) fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: &msgs::TxAddInput, confirmed: bool) { - self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(serial_id, transaction_input, confirmed)) + self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(transaction_input, confirmed)) } pub(crate) fn receive_tx_remove_input(&mut self, serial_id: SerialId) { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index f6044cac28f..9083430b568 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -464,6 +464,8 @@ impl Readable for TransactionU16LenLimited { } } +pub type SerialId = u64; + /// A tx_add_input message for adding an input during interactive transaction construction /// // TODO(dual_funding): Add spec link for `tx_add_input`. @@ -473,7 +475,7 @@ pub struct TxAddInput { pub channel_id: [u8; 32], /// A randomly chosen unique identifier for this input, which is even for initiators and odd for /// non-initiators. - pub serial_id: u64, + pub serial_id: SerialId, /// Serialized transaction that contains the output this input spends to verify that it is non /// malleable. pub prevtx: TransactionU16LenLimited, @@ -492,7 +494,7 @@ pub struct TxAddOutput { pub channel_id: [u8; 32], /// A randomly chosen unique identifier for this output, which is even for initiators and odd for /// non-initiators. - pub serial_id: u64, + pub serial_id: SerialId, /// The satoshi value of the output pub sats: u64, /// The scriptPubKey for the output @@ -507,7 +509,7 @@ pub struct TxRemoveInput { /// The channel ID pub channel_id: [u8; 32], /// The serial ID of the input to be removed - pub serial_id: u64, + pub serial_id: SerialId, } /// A tx_remove_output message for removing an output during interactive transaction construction. @@ -518,7 +520,7 @@ pub struct TxRemoveOutput { /// The channel ID pub channel_id: [u8; 32], /// The serial ID of the output to be removed - pub serial_id: u64, + pub serial_id: SerialId, } /// A tx_complete message signalling the conclusion of a peer's transaction contributions during From b82b77d97cb612175c20f0052a6032d3aa618d64 Mon Sep 17 00:00:00 2001 From: Jurvis Tan Date: Wed, 16 Aug 2023 10:02:13 -0700 Subject: [PATCH 23/28] Accept changes on TheirTxComplete --- lightning/src/ln/interactivetxs.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 33cf52e7588..1aa1bd9160c 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -107,6 +107,7 @@ pub(crate) struct NegotiationAborted(AbortReason); impl AcceptingChanges for Negotiating {} impl AcceptingChanges for OurTxComplete {} +impl AcceptingChanges for TheirTxComplete {} struct TxInputWithPrevOutput { input: TxIn, @@ -460,12 +461,13 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { } impl InteractiveTxStateMachine { - fn send_tx_complete(self) -> InteractiveTxStateMachine { - // TODO: Should we validate before transitioning states? If so, do we want to abort negotiation - // if our current transaction state is invalid? - InteractiveTxStateMachine { - context: self.context, - state: NegotiationComplete {} + fn send_tx_complete(self) -> InteractiveTxStateMachineResult { + match self.is_current_transaction_state_able_to_complete() { + Err(e) => Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(e) }), + _ => Ok(InteractiveTxStateMachine { + context: self.context, + state: NegotiationComplete {} + }) } } } @@ -592,7 +594,12 @@ impl InteractiveTxConstructor { let mut mode = core::mem::take(&mut self.mode); self.mode = match mode { ChannelMode::Negotiating(c) => { ChannelMode::OurTxComplete(c.send_tx_complete()) } - ChannelMode::TheirTxComplete(c) => { ChannelMode::NegotiationComplete(c.send_tx_complete()) } + ChannelMode::TheirTxComplete(c) => { + match c.send_tx_complete() { + Ok(c) => ChannelMode::NegotiationComplete(c), + Err(c) => ChannelMode::NegotiationAborted(c) + } + } _ => mode } } From 1358d6534796af29685949772d3a94e429563773 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sun, 27 Aug 2023 23:15:05 +0200 Subject: [PATCH 24/28] Adaptations, compile fixes --- lightning/src/ln/interactivetxs.rs | 21 +++++---- lightning/src/ln/msgs.rs | 76 +++++++++++++++--------------- lightning/src/util/ser.rs | 4 ++ 3 files changed, 53 insertions(+), 48 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 1aa1bd9160c..a2ab312967c 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet}; -use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint, Witness}; +use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; // Witness use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; @@ -250,7 +250,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { return self.abort_negotiation(AbortReason::ReceivedTooManyTxAddInputs); } - let prev_out = if let Some(prev_out) = msg.prevtx.0.output.get(msg.prevtx_out as usize) { + let prev_out = if let Some(prev_out) = msg.prevtx.inner().output.get(msg.prevtx_out as usize) { prev_out.clone() } else { return self.abort_negotiation(AbortReason::PrevTxOutInvalid); @@ -345,7 +345,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { return self.abort_negotiation(AbortReason::IncorrectSerialIdParity); } - if let Some(output) = self.context.outputs.remove(&serial_id) { + if let Some(_output) = self.context.outputs.remove(&serial_id) { Ok(InteractiveTxStateMachine { context: self.context, state: Negotiating {} }) } else { self.abort_negotiation(AbortReason::SerialIdUnknown) @@ -558,7 +558,7 @@ impl InteractiveTxConstructor { self.handle_negotiating_receive(|state_machine| state_machine.abort_negotiation(reason)) } - pub(crate) fn receive_tx_add_input(&mut self, serial_id: SerialId, transaction_input: &msgs::TxAddInput, confirmed: bool) { + pub(crate) fn receive_tx_add_input(&mut self, _serial_id: SerialId, transaction_input: &msgs::TxAddInput, confirmed: bool) { self.handle_negotiating_receive(|state_machine| state_machine.receive_tx_add_input(transaction_input, confirmed)) } @@ -591,7 +591,7 @@ impl InteractiveTxConstructor { } pub(crate) fn send_tx_complete(&mut self) { - let mut mode = core::mem::take(&mut self.mode); + let mode = core::mem::take(&mut self.mode); self.mode = match mode { ChannelMode::Negotiating(c) => { ChannelMode::OurTxComplete(c.send_tx_complete()) } ChannelMode::TheirTxComplete(c) => { @@ -661,14 +661,15 @@ impl InteractiveTxConstructor { mod tests { use core::str::FromStr; use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW; -use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; - use crate::ln::interactivetxs::{AbortReason, ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine}; - use crate::ln::msgs::TransactionU16LenLimited; + use crate::ln::ChannelId; + // use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; + use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor}; // AbortReason, InteractiveTxStateMachine + use crate::util::ser::TransactionU16LenLimited; use bitcoin::consensus::encode; use bitcoin::{Address, PackedLockTime, Script, Sequence, Transaction, Txid, TxIn, TxOut, Witness}; use bitcoin::hashes::hex::FromHex; use crate::chain::transaction::OutPoint; - use crate::ln::interactivetxs::AbortReason::IncorrectSerialIdParity; + // use crate::ln::interactivetxs::AbortReason::IncorrectSerialIdParity; use crate::ln::msgs::TxAddInput; #[test] @@ -737,7 +738,7 @@ use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; ).unwrap(); return TxAddInput { - channel_id: [2; 32], + channel_id: ChannelId::from_bytes([2; 32]), serial_id: 4886718345, prevtx, prevtx_out: 305419896, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 1301add8d5f..ce8a0fe9ff1 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -429,44 +429,44 @@ pub struct ChannelReady { pub short_channel_id_alias: Option, } -/// A wrapper for a `Transaction` which can only be constructed with [`TransactionU16LenLimited::new`] -/// if the `Transaction`'s consensus-serialized length is <= u16::MAX. -/// -/// Use [`TransactionU16LenLimited::into_transaction`] to convert into the contained `Transaction`. -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct TransactionU16LenLimited(pub Transaction); - -impl TransactionU16LenLimited { - /// Constructs a new `TransactionU16LenLimited` from a `Transaction` only if it's consensus- - /// serialized length is <= u16::MAX. - pub fn new(transaction: Transaction) -> Result { - if transaction.serialized_length() > (u16::MAX as usize) { - Err(()) - } else { - Ok(Self(transaction)) - } - } - - /// Consumes this `TransactionU16LenLimited` and returns its contained `Transaction`. - pub fn into_transaction(self) -> Transaction { - self.0 - } -} - -impl Writeable for TransactionU16LenLimited { - fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.0.serialized_length() as u16).write(w)?; - self.0.write(w) - } -} - -impl Readable for TransactionU16LenLimited { - fn read(r: &mut R) -> Result { - let len = ::read(r)?; - let mut tx_reader = FixedLengthReader::new(r, len as u64); - Ok(Self(Readable::read(&mut tx_reader)?)) - } -} +// /// A wrapper for a `Transaction` which can only be constructed with [`TransactionU16LenLimited::new`] +// /// if the `Transaction`'s consensus-serialized length is <= u16::MAX. +// /// +// /// Use [`TransactionU16LenLimited::into_transaction`] to convert into the contained `Transaction`. +// #[derive(Clone, Debug, PartialEq, Eq)] +// pub struct TransactionU16LenLimited(pub Transaction); + +// impl TransactionU16LenLimited { +// /// Constructs a new `TransactionU16LenLimited` from a `Transaction` only if it's consensus- +// /// serialized length is <= u16::MAX. +// pub fn new(transaction: Transaction) -> Result { +// if transaction.serialized_length() > (u16::MAX as usize) { +// Err(()) +// } else { +// Ok(Self(transaction)) +// } +// } + +// /// Consumes this `TransactionU16LenLimited` and returns its contained `Transaction`. +// pub fn into_transaction(self) -> Transaction { +// self.0 +// } +// } + +// impl Writeable for TransactionU16LenLimited { +// fn write(&self, w: &mut W) -> Result<(), io::Error> { +// (self.0.serialized_length() as u16).write(w)?; +// self.0.write(w) +// } +// } + +// impl Readable for TransactionU16LenLimited { +// fn read(r: &mut R) -> Result { +// let len = ::read(r)?; +// let mut tx_reader = FixedLengthReader::new(r, len as u64); +// Ok(Self(Readable::read(&mut tx_reader)?)) +// } +// } pub type SerialId = u64; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 1eb5e7424c8..5adc7a24098 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1392,6 +1392,10 @@ impl TransactionU16LenLimited { pub fn into_transaction(self) -> Transaction { self.0 } + + pub fn inner(&self) -> &Transaction { + &self.0 + } } impl Writeable for TransactionU16LenLimited { From 3f21de9a611eb9113b3cd3297321c10795905bf1 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sun, 27 Aug 2023 23:24:39 +0200 Subject: [PATCH 25/28] ChannelId adaptations --- lightning/src/ln/interactivetxs.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a2ab312967c..e0461d2cd6a 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -12,6 +12,7 @@ use std::collections::{HashMap, HashSet}; use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; // Witness use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; +use crate::ln::ChannelId; use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::interactivetxs::ChannelMode::Indeterminate; @@ -115,7 +116,7 @@ struct TxInputWithPrevOutput { } struct NegotiationContext { - channel_id: [u8; 32], + channel_id: ChannelId, require_confirmed_inputs: bool, holder_is_initiator: bool, received_tx_add_input_count: u16, @@ -164,7 +165,7 @@ type InteractiveTxStateMachineResult = impl InteractiveTxStateMachine { fn new( - channel_id: [u8; 32], feerate_sat_per_kw: u32, require_confirmed_inputs: bool, + channel_id: ChannelId, feerate_sat_per_kw: u32, require_confirmed_inputs: bool, is_initiator: bool, base_tx: Transaction, did_send_tx_signatures: bool, ) -> Self { Self { @@ -542,7 +543,7 @@ pub(crate) struct InteractiveTxConstructor { // if a node receives an unexpected message. impl InteractiveTxConstructor { pub(crate) fn new( - channel_id: [u8; 32], feerate_sat_per_kw: u32, require_confirmed_inputs: bool, + channel_id: ChannelId, feerate_sat_per_kw: u32, require_confirmed_inputs: bool, is_initiator: bool, base_tx: Transaction, did_send_tx_signatures: bool, ) -> Self { let initial_state_machine = InteractiveTxStateMachine::new( @@ -681,7 +682,7 @@ mod tests { 72e3df059d277e776dda4269fa0d2cc8c2ee6ec9a022054e7fae5ca94d47534c86705857c24ceea3ad51c69\ dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); - let mut constructor = InteractiveTxConstructor::new([0; 32], FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false); + let mut constructor = InteractiveTxConstructor::new(ChannelId::new_zero(), FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false); constructor.receive_tx_add_input(2, &get_sample_tx_add_input(), false); assert!(matches!(constructor.mode, ChannelMode::NegotiationAborted { .. })) } @@ -701,7 +702,7 @@ mod tests { dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); Self { - tx_constructor: InteractiveTxConstructor::new([0; 32], FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false) + tx_constructor: InteractiveTxConstructor::new(ChannelId::new_zero(), FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false) } } From de3f4b1291bc2050bc23a33a9f95538016255523 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 30 Aug 2023 07:29:28 +0200 Subject: [PATCH 26/28] Add unit tests; minor changes (fee substraction) --- lightning/src/ln/interactivetxs.rs | 285 ++++++++++++++++++++++++++--- 1 file changed, 255 insertions(+), 30 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index e0461d2cd6a..dad895c44a8 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -9,6 +9,7 @@ use std::collections::{HashMap, HashSet}; +use bitcoin::hashes::hex::ToHex; use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; // Witness use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; @@ -38,6 +39,7 @@ impl SerialIdExt for SerialId { fn is_valid_for_initiator(&self) -> bool { self % 2 == 0 } } +#[derive(Debug)] pub(crate) enum AbortReason { CounterpartyAborted, InputsNotConfirmed, @@ -189,6 +191,7 @@ impl InteractiveTxStateMachine { impl InteractiveTxStateMachine where S: AcceptingChanges { fn abort_negotiation(self, reason: AbortReason) -> InteractiveTxStateMachineResult { + // println!("ABORT {:?}", reason); // TODO remove Err(InteractiveTxStateMachine { context: self.context, state: NegotiationAborted(reason) }) } @@ -219,6 +222,7 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { let transaction = msg.prevtx.clone().into_transaction(); if let Some(tx_out) = transaction.output.get(msg.prevtx_out as usize) { + // println!("txout {} {}", tx_out.value, tx_out.script_pubkey.to_hex()); // TODO remove if !tx_out.script_pubkey.is_witness_program() { // The receiving node: // - MUST fail the negotiation if: @@ -428,8 +432,16 @@ impl InteractiveTxStateMachine where S: AcceptingChanges { // - the peer's paid feerate does not meet or exceed the agreed feerate (based on the minimum fee). if self.context.holder_is_initiator { - let non_initiator_fees_contributed: u64 = self.context.non_initiator_outputs_contributed().map(|output| output.value).sum::() - - self.context.non_initiator_inputs_contributed().map(|input| input.prev_output.value).sum::(); + // TODO + // panicked at 'attempt to subtract with overflow' + // FEES output_sum / input_sum 88888 12704566 + // println!("FEES output_sum / input_sum {} {}", + // self.context.non_initiator_outputs_contributed().map(|output| output.value).sum::(), + // self.context.non_initiator_inputs_contributed().map(|input| input.prev_output.value).sum::(), + // ); + let non_initiator_fees_contributed: u64 = + self.context.non_initiator_inputs_contributed().map(|input| input.prev_output.value).sum::() - + self.context.non_initiator_outputs_contributed().map(|output| output.value).sum::(); let non_initiator_contribution_weight = self.context.non_initiator_inputs_contributed().count() as u64 * INPUT_WEIGHT + self.context.non_initiator_outputs_contributed().count() as u64 * OUTPUT_WEIGHT; let required_non_initiator_contribution_fee = self.context.feerate_sat_per_kw as u64 * 1000 / non_initiator_contribution_weight; @@ -664,14 +676,183 @@ mod tests { use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW; use crate::ln::ChannelId; // use crate::ln::interactivetxs::ChannelMode::{Negotiating, NegotiationAborted}; - use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor}; // AbortReason, InteractiveTxStateMachine + use crate::ln::interactivetxs::{ChannelMode, InteractiveTxConstructor, InteractiveTxStateMachine, NegotiationAborted}; // AbortReason, InteractiveTxStateMachine use crate::util::ser::TransactionU16LenLimited; use bitcoin::consensus::encode; use bitcoin::{Address, PackedLockTime, Script, Sequence, Transaction, Txid, TxIn, TxOut, Witness}; + use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey}; + use bitcoin::hashes::Hash; + use bitcoin::hash_types::WPubkeyHash; use bitcoin::hashes::hex::FromHex; use crate::chain::transaction::OutPoint; // use crate::ln::interactivetxs::AbortReason::IncorrectSerialIdParity; - use crate::ln::msgs::TxAddInput; + use crate::ln::msgs::{TxAddInput, TxAddOutput}; + + #[test] + fn test_interact_tx_abort() { + let mut dc = DummyChannel::new(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.abort_negotation(super::AbortReason::OutputsExceedInputs); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::NegotiationAborted(InteractiveTxStateMachine{context: _, state: NegotiationAborted(super::AbortReason::OutputsExceedInputs)}))); + } + + #[test] + fn test_interact_tx_recv_add_input_success() { + let mut dc = DummyChannel::new(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + let msg = get_sample_tx_add_input(4886718345); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_recv_remove_input_success() { + let mut dc = DummyChannel::new(); + let msg = get_sample_tx_add_input(4886718345); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.receive_tx_remove_input(4886718345); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_recv_add_output_success() { + let mut dc = DummyChannel::new(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + let txout = get_sample_tx_output(); + dc.tx_constructor.receive_tx_add_output(4886718347, txout); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_recv_remove_output_success() { + let mut dc = DummyChannel::new(); + let txout = get_sample_tx_output(); + dc.tx_constructor.receive_tx_add_output(4886718345, txout); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.receive_tx_remove_output(4886718345); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_send_add_input_success() { + let mut dc = DummyChannel::new(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.send_tx_add_input(4886718345, get_sample_tx_in(), get_sample_tx_out()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_send_add_output_success() { + let mut dc = DummyChannel::new(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.send_tx_add_output(4886718345, get_sample_tx_out()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_send_remove_input_success() { + let mut dc = DummyChannel::new(); + dc.tx_constructor.send_tx_add_input(4886718345, get_sample_tx_in(), get_sample_tx_out()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.send_tx_remove_input(4886718345); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_send_remove_output_success() { + let mut dc = DummyChannel::new(); + dc.tx_constructor.send_tx_add_output(4886718345, get_sample_tx_out()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.send_tx_remove_output(4886718345); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + fn test_interact_tx_send_abort_success() { + let mut dc = DummyChannel::new(); + let msg = get_sample_tx_add_input(4886718345); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + // TODO + // dc.tx_constructor.send_tx_abort(); + } + + fn test_interact_tx_recv_abort_success() { + let mut dc = DummyChannel::new(); + dc.tx_constructor.send_tx_add_input(4886718345, get_sample_tx_in(), get_sample_tx_out()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + // TODO + // dc.tx_constructor.receive_tx_abort(); + } + + #[test] + fn test_interact_tx_recv_complete_negotiating_success() { + let mut dc = DummyChannel::new(); + let msg = get_sample_tx_add_input(4886718345); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + let txout = get_sample_tx_output(); + dc.tx_constructor.receive_tx_add_output(4886718347, txout); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.receive_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::TheirTxComplete(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_recv_complete_ourcomplete_success() { + let mut dc = DummyChannel::new(); + dc.tx_constructor.send_tx_add_input(4886718345, get_sample_tx_in(), get_sample_tx_out()); + dc.tx_constructor.send_tx_add_output(4886718347, get_sample_tx_out_2()); + dc.tx_constructor.send_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::OurTxComplete(_))); + let msg = get_sample_tx_add_input(4886718349); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + let txout = get_sample_tx_output(); + dc.tx_constructor.receive_tx_add_output(4886718351, txout); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::OurTxComplete(_))); + + dc.tx_constructor.receive_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::NegotiationComplete(InteractiveTxStateMachine{context: _, state: Negotiating}))); + } + + #[test] + fn test_interact_tx_send_complete_negotiating_success() { + let mut dc = DummyChannel::new(); + dc.tx_constructor.send_tx_add_input(4886718345, get_sample_tx_in(), get_sample_tx_out()); + dc.tx_constructor.send_tx_add_output(4886718347, get_sample_tx_out_2()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::Negotiating(_))); + + dc.tx_constructor.send_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::OurTxComplete(InteractiveTxStateMachine{context: _, state: _}))); + } + + #[test] + fn test_interact_tx_send_complete_theircomplete_success() { + let mut dc = DummyChannel::new(); + let msg = get_sample_tx_add_input(4886718345); + dc.tx_constructor.receive_tx_add_input(666, &msg, true); + let txout = get_sample_tx_output(); + dc.tx_constructor.receive_tx_add_output(4886718347, txout); + dc.tx_constructor.receive_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::TheirTxComplete(_))); + dc.tx_constructor.send_tx_add_input(4886718349, get_sample_tx_in(), get_sample_tx_out()); + dc.tx_constructor.send_tx_add_output(4886718351, get_sample_tx_out_2()); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::TheirTxComplete(_))); + + dc.tx_constructor.send_tx_complete(); + assert!(matches!(dc.tx_constructor.mode, ChannelMode::NegotiationComplete(InteractiveTxStateMachine{context: _, state: _}))); + } #[test] fn test_invalid_counterparty_serial_id_should_abort_negotiation() { @@ -683,7 +864,7 @@ mod tests { dd6051c5850304880fc43a012103cb11a1bacc223d98d91f1946c6752e358a5eb1a1c983b3e6fb15378f453\ b76bd00000000").unwrap()[..]).unwrap(); let mut constructor = InteractiveTxConstructor::new(ChannelId::new_zero(), FEERATE_FLOOR_SATS_PER_KW, true, true, tx, false); - constructor.receive_tx_add_input(2, &get_sample_tx_add_input(), false); + constructor.receive_tx_add_input(2, &get_sample_tx_add_input(4886718345), false); assert!(matches!(constructor.mode, ChannelMode::NegotiationAborted { .. })) } @@ -706,45 +887,89 @@ mod tests { } } - fn handle_add_tx_input(&mut self) { - self.tx_constructor.receive_tx_add_input(1234, &get_sample_tx_add_input(), true) - } + // fn handle_add_tx_input(&mut self) { + // self.tx_constructor.receive_tx_add_input(1234, &get_sample_tx_add_input(4886718345), true) + // } } // Fixtures - fn get_sample_tx_add_input() -> TxAddInput { + fn get_sample_channel_id() -> ChannelId { + ChannelId::v1_from_funding_txid(&[2; 32], 0) + } + + fn get_sample_tx_in_prev_outpoint() -> OutPoint { + OutPoint { + txid: Txid::from_hex("305bab643ee297b8b6b76b320792c8223d55082122cb606bf89382146ced9c77").unwrap(), + index: 2, + } + } + + fn get_sample_tx_in() -> TxIn { + TxIn { + previous_output: get_sample_tx_in_prev_outpoint().into_bitcoin_outpoint(), + script_sig: Script::new(), + sequence: Sequence(0xfffffffd), + witness: Witness::from_vec(vec![ + hex::decode("304402206af85b7dd67450ad12c979302fac49dfacbc6a8620f49c5da2b5721cf9565ca502207002b32fed9ce1bf095f57aeb10c36928ac60b12e723d97d2964a54640ceefa701").unwrap(), + hex::decode("0301ab7dc16488303549bfcdd80f6ae5ee4c20bf97ab5410bbd6b1bfa85dcd6944").unwrap()]), + } + } + + fn get_sample_tx_out() -> TxOut { + TxOut { + value: 12704566, + script_pubkey: Address::from_str("bc1qzlffunw52jav8vwdu5x3jfk6sr8u22rmq3xzw2").unwrap().script_pubkey(), + } + } + + fn get_sample_tx_out_2() -> TxOut { + TxOut { + value: 245148, + script_pubkey: Address::from_str("bc1qxmk834g5marzm227dgqvynd23y2nvt2ztwcw2z").unwrap().script_pubkey(), + } + } + + fn get_sample_tx_add_input(serial_id: u64) -> TxAddInput { let prevtx = TransactionU16LenLimited::new( Transaction { version: 2, lock_time: PackedLockTime(0), - input: vec![TxIn { - previous_output: OutPoint { txid: Txid::from_hex("305bab643ee297b8b6b76b320792c8223d55082122cb606bf89382146ced9c77").unwrap(), index: 2 }.into_bitcoin_outpoint(), - script_sig: Script::new(), - sequence: Sequence(0xfffffffd), - witness: Witness::from_vec(vec![ - hex::decode("304402206af85b7dd67450ad12c979302fac49dfacbc6a8620f49c5da2b5721cf9565ca502207002b32fed9ce1bf095f57aeb10c36928ac60b12e723d97d2964a54640ceefa701").unwrap(), - hex::decode("0301ab7dc16488303549bfcdd80f6ae5ee4c20bf97ab5410bbd6b1bfa85dcd6944").unwrap()]), - }], + input: vec![get_sample_tx_in()], output: vec![ - TxOut { - value: 12704566, - script_pubkey: Address::from_str("bc1qzlffunw52jav8vwdu5x3jfk6sr8u22rmq3xzw2").unwrap().script_pubkey(), - }, - TxOut { - value: 245148, - script_pubkey: Address::from_str("bc1qxmk834g5marzm227dgqvynd23y2nvt2ztwcw2z").unwrap().script_pubkey(), - }, + get_sample_tx_out(), + get_sample_tx_out_2(), ], } ).unwrap(); - return TxAddInput { - channel_id: ChannelId::from_bytes([2; 32]), - serial_id: 4886718345, + TxAddInput { + channel_id: get_sample_channel_id(), + serial_id, prevtx, - prevtx_out: 305419896, + prevtx_out: 0, sequence: 305419896, - }; + } + } + + fn get_sample_tx_output() -> TxOut { + let secret_key = SecretKey::from_slice(&[2; 32]).unwrap(); + let secp_ctx = Secp256k1::new(); + let pubkey = PublicKey::from_secret_key(&secp_ctx, &secret_key); + let script_pubkey = Script::new_v0_p2wpkh(&WPubkeyHash::hash(&pubkey.serialize())); + TxOut { + value: 88888, + script_pubkey, + } + } + + fn get_sample_tx_add_output(serial_id: u64) -> TxAddOutput { + let tx_out = get_sample_tx_output(); + TxAddOutput { + channel_id: get_sample_channel_id(), + serial_id, + sats: tx_out.value, + script: tx_out.script_pubkey, + } } } From d2f5fe43db3becf474e8b505f4ba647e489ba824 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 30 Aug 2023 22:38:29 +0200 Subject: [PATCH 27/28] Add missing docs (error in CI) --- lightning/src/ln/interactivetxs.rs | 2 ++ lightning/src/ln/msgs.rs | 1 + lightning/src/util/ser.rs | 1 + 3 files changed, 4 insertions(+) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index dad895c44a8..a4cd17ee237 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -670,6 +670,7 @@ impl InteractiveTxConstructor { } } +// TODO: put in different test file #[cfg(test)] mod tests { use core::str::FromStr; @@ -952,6 +953,7 @@ mod tests { } fn get_sample_tx_output() -> TxOut { + // TODO: better create from address let secret_key = SecretKey::from_slice(&[2; 32]).unwrap(); let secp_ctx = Secp256k1::new(); let pubkey = PublicKey::from_secret_key(&secp_ctx, &secret_key); diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index ce8a0fe9ff1..3334328ef16 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -468,6 +468,7 @@ pub struct ChannelReady { // } // } +/// Serial ID, 64-bit pub type SerialId = u64; /// A tx_add_input message for adding an input during interactive transaction construction diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 5adc7a24098..3fd760507f4 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1393,6 +1393,7 @@ impl TransactionU16LenLimited { self.0 } + /// Access inner transaction, as reference pub fn inner(&self) -> &Transaction { &self.0 } From deca874d57b317d5cc42c0ad1f005dc0c773bbff Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Thu, 31 Aug 2023 00:05:48 +0200 Subject: [PATCH 28/28] Remove 'use std', compile fix --- lightning/src/ln/interactivetxs.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a4cd17ee237..5211ed31124 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -7,9 +7,6 @@ // You may not use this file except in accordance with one or both of these // licenses. -use std::collections::{HashMap, HashSet}; - -use bitcoin::hashes::hex::ToHex; use bitcoin::{TxIn, Sequence, Transaction, TxOut, OutPoint}; // Witness use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::policy::MAX_STANDARD_TX_WEIGHT; @@ -20,6 +17,8 @@ use crate::ln::interactivetxs::ChannelMode::Indeterminate; use crate::ln::msgs; use crate::ln::msgs::SerialId; +use crate::prelude::*; + /// The number of received `tx_add_input` messages during a negotiation at which point the /// negotiation MUST be failed. const MAX_RECEIVED_TX_ADD_INPUT_COUNT: u16 = 4096;