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.

diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs
index dbe0a8621ce851aab3a55ce25fcfd74658ff45a7..c23d23b1a3ea183160e2cb613e1848380c3aeb16 100644 (file)
--- 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 c743f647ccdeb0849aa5abce2771c807788a3b4e..570982fb70c280de5637adedafccced787bcaacb 100644 (file)
--- a/lightning/src/ln/channelmonitor.rs
+++ b/lightning/src/ln/channelmonitor.rs
@@ -2201,16 +2201,30 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        fn is_paying_spendable_output<L: Deref>(&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<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                }
                        } 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 4b5453351b490c3b4cddb98d534eb8026c6095a7..f44a8d59e0981c75d2207163e0a7f8aaf317a89f 100644 (file)
--- 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 85484aceae31aa443c3ffdc380be2c1822f736e5..e565c19ccab84e50f447dde4a0bfc6ecb468104f 100644 (file)
--- 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(())