From 6df9129ace609bfb5c7f08ae0f41175126d05b1b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 30 Jul 2020 13:19:11 -0400 Subject: [PATCH] Use ln OutPoints not bitcoin ones in SpendableOutputDescriptors Lightning OutPoints only have 16 bits to express the output index instead of Bitcoin's 32 bits, implying that some outputs are possibly not expressible as lightning OutPoints. However, such OutPoints can never be hit within the lightning protocol, and must be on-chain spam sent by a third party wishing to donate us money. Still, in order to do so, the third party would need to fill nearly an entire block with garbage, so this case should be relatively safe. A new comment in channelmonitor explains the reasoning a bit further. --- lightning/src/chain/keysinterface.rs | 3 ++- lightning/src/ln/channelmonitor.rs | 22 ++++++++++++++++++---- lightning/src/ln/functional_tests.rs | 6 +++--- lightning/src/util/macro_logger.rs | 6 +++--- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index dbe0a8621..c23d23b1a 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -11,7 +11,7 @@ //! spendable on-chain outputs which the user owns and is responsible for using just as any other //! on-chain output which is theirs. -use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut}; +use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::blockdata::script::{Script, Builder}; use bitcoin::blockdata::opcodes; use bitcoin::network::constants::Network; @@ -31,6 +31,7 @@ use bitcoin::secp256k1; use util::byte_utils; use util::ser::{Writeable, Writer, Readable}; +use chain::transaction::OutPoint; use ln::chan_utils; use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys}; use ln::msgs::UnsignedChannelAnnouncement; diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index c743f647c..570982fb7 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -2201,16 +2201,30 @@ impl ChannelMonitor { fn is_paying_spendable_output(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger { let mut spendable_output = None; for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us + if i > ::std::u16::MAX as usize { + // While it is possible that an output exists on chain which is greater than the + // 2^16th output in a given transaction, this is only possible if the output is not + // in a lightning transaction and was instead placed there by some third party who + // wishes to give us money for no reason. + // Namely, any lightning transactions which we pre-sign will never have anywhere + // near 2^16 outputs both because such transactions must have ~2^16 outputs who's + // scripts are not longer than one byte in length and because they are inherently + // non-standard due to their size. + // Thus, it is completely safe to ignore such outputs, and while it may result in + // us ignoring non-lightning fund to us, that is only possible if someone fills + // nearly a full block with garbage just to hit this case. + continue; + } if outp.script_pubkey == self.destination_script { spendable_output = Some(SpendableOutputDescriptor::StaticOutput { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); break; } else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script { if broadcasted_local_revokable_script.0 == outp.script_pubkey { spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, per_commitment_point: broadcasted_local_revokable_script.1, to_self_delay: self.on_local_tx_csv, output: outp.clone(), @@ -2221,14 +2235,14 @@ impl ChannelMonitor { } } else if self.remote_payment_script == outp.script_pubkey { spendable_output = Some(SpendableOutputDescriptor::StaticOutputRemotePayment { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), key_derivation_params: self.keys.key_derivation_params(), }); break; } else if outp.script_pubkey == self.shutdown_script { spendable_output = Some(SpendableOutputDescriptor::StaticOutput { - outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, + outpoint: OutPoint { txid: tx.txid(), index: i as u16 }, output: outp.clone(), }); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 4b5453351..f44a8d59e 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4672,7 +4672,7 @@ macro_rules! check_spendable_outputs { match *outp { SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, ref output, ref key_derivation_params } => { let input = TxIn { - previous_output: outpoint.clone(), + previous_output: outpoint.into_bitcoin_outpoint(), script_sig: Script::new(), sequence: 0, witness: Vec::new(), @@ -4700,7 +4700,7 @@ macro_rules! check_spendable_outputs { }, SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref remote_revocation_pubkey } => { let input = TxIn { - previous_output: outpoint.clone(), + previous_output: outpoint.into_bitcoin_outpoint(), script_sig: Script::new(), sequence: *to_self_delay as u32, witness: Vec::new(), @@ -4733,7 +4733,7 @@ macro_rules! check_spendable_outputs { SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => { let secp_ctx = Secp256k1::new(); let input = TxIn { - previous_output: outpoint.clone(), + previous_output: outpoint.into_bitcoin_outpoint(), script_sig: Script::new(), sequence: 0, witness: Vec::new(), diff --git a/lightning/src/util/macro_logger.rs b/lightning/src/util/macro_logger.rs index 85484acea..e565c19cc 100644 --- a/lightning/src/util/macro_logger.rs +++ b/lightning/src/util/macro_logger.rs @@ -136,13 +136,13 @@ impl<'a> std::fmt::Display for DebugSpendable<'a> { fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> { match self.0 { &SpendableOutputDescriptor::StaticOutput { ref outpoint, .. } => { - write!(f, "StaticOutput {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + write!(f, "StaticOutput {}:{} marked for spending", outpoint.txid, outpoint.index)?; } &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, .. } => { - write!(f, "DynamicOutputP2WSH {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + write!(f, "DynamicOutputP2WSH {}:{} marked for spending", outpoint.txid, outpoint.index)?; } &SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, .. } => { - write!(f, "DynamicOutputP2WPKH {}:{} marked for spending", outpoint.txid, outpoint.vout)?; + write!(f, "DynamicOutputP2WPKH {}:{} marked for spending", outpoint.txid, outpoint.index)?; } } Ok(()) -- 2.39.5