Skip to content

Commit 6df9129

Browse files
committed
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.
1 parent de8c5dc commit 6df9129

File tree

4 files changed

+26
-11
lines changed

4 files changed

+26
-11
lines changed

lightning/src/chain/keysinterface.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//! spendable on-chain outputs which the user owns and is responsible for using just as any other
1212
//! on-chain output which is theirs.
1313
14-
use bitcoin::blockdata::transaction::{Transaction, OutPoint, TxOut};
14+
use bitcoin::blockdata::transaction::{Transaction, TxOut};
1515
use bitcoin::blockdata::script::{Script, Builder};
1616
use bitcoin::blockdata::opcodes;
1717
use bitcoin::network::constants::Network;
@@ -31,6 +31,7 @@ use bitcoin::secp256k1;
3131
use util::byte_utils;
3232
use util::ser::{Writeable, Writer, Readable};
3333

34+
use chain::transaction::OutPoint;
3435
use ln::chan_utils;
3536
use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, LocalCommitmentTransaction, PreCalculatedTxCreationKeys};
3637
use ln::msgs::UnsignedChannelAnnouncement;

lightning/src/ln/channelmonitor.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2201,16 +2201,30 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22012201
fn is_paying_spendable_output<L: Deref>(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger {
22022202
let mut spendable_output = None;
22032203
for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us
2204+
if i > ::std::u16::MAX as usize {
2205+
// While it is possible that an output exists on chain which is greater than the
2206+
// 2^16th output in a given transaction, this is only possible if the output is not
2207+
// in a lightning transaction and was instead placed there by some third party who
2208+
// wishes to give us money for no reason.
2209+
// Namely, any lightning transactions which we pre-sign will never have anywhere
2210+
// near 2^16 outputs both because such transactions must have ~2^16 outputs who's
2211+
// scripts are not longer than one byte in length and because they are inherently
2212+
// non-standard due to their size.
2213+
// Thus, it is completely safe to ignore such outputs, and while it may result in
2214+
// us ignoring non-lightning fund to us, that is only possible if someone fills
2215+
// nearly a full block with garbage just to hit this case.
2216+
continue;
2217+
}
22042218
if outp.script_pubkey == self.destination_script {
22052219
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
2206-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2220+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22072221
output: outp.clone(),
22082222
});
22092223
break;
22102224
} else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script {
22112225
if broadcasted_local_revokable_script.0 == outp.script_pubkey {
22122226
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
2213-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2227+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22142228
per_commitment_point: broadcasted_local_revokable_script.1,
22152229
to_self_delay: self.on_local_tx_csv,
22162230
output: outp.clone(),
@@ -2221,14 +2235,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22212235
}
22222236
} else if self.remote_payment_script == outp.script_pubkey {
22232237
spendable_output = Some(SpendableOutputDescriptor::StaticOutputRemotePayment {
2224-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2238+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22252239
output: outp.clone(),
22262240
key_derivation_params: self.keys.key_derivation_params(),
22272241
});
22282242
break;
22292243
} else if outp.script_pubkey == self.shutdown_script {
22302244
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
2231-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2245+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22322246
output: outp.clone(),
22332247
});
22342248
}

lightning/src/ln/functional_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4672,7 +4672,7 @@ macro_rules! check_spendable_outputs {
46724672
match *outp {
46734673
SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, ref output, ref key_derivation_params } => {
46744674
let input = TxIn {
4675-
previous_output: outpoint.clone(),
4675+
previous_output: outpoint.into_bitcoin_outpoint(),
46764676
script_sig: Script::new(),
46774677
sequence: 0,
46784678
witness: Vec::new(),
@@ -4700,7 +4700,7 @@ macro_rules! check_spendable_outputs {
47004700
},
47014701
SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref remote_revocation_pubkey } => {
47024702
let input = TxIn {
4703-
previous_output: outpoint.clone(),
4703+
previous_output: outpoint.into_bitcoin_outpoint(),
47044704
script_sig: Script::new(),
47054705
sequence: *to_self_delay as u32,
47064706
witness: Vec::new(),
@@ -4733,7 +4733,7 @@ macro_rules! check_spendable_outputs {
47334733
SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
47344734
let secp_ctx = Secp256k1::new();
47354735
let input = TxIn {
4736-
previous_output: outpoint.clone(),
4736+
previous_output: outpoint.into_bitcoin_outpoint(),
47374737
script_sig: Script::new(),
47384738
sequence: 0,
47394739
witness: Vec::new(),

lightning/src/util/macro_logger.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,13 @@ impl<'a> std::fmt::Display for DebugSpendable<'a> {
136136
fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
137137
match self.0 {
138138
&SpendableOutputDescriptor::StaticOutput { ref outpoint, .. } => {
139-
write!(f, "StaticOutput {}:{} marked for spending", outpoint.txid, outpoint.vout)?;
139+
write!(f, "StaticOutput {}:{} marked for spending", outpoint.txid, outpoint.index)?;
140140
}
141141
&SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, .. } => {
142-
write!(f, "DynamicOutputP2WSH {}:{} marked for spending", outpoint.txid, outpoint.vout)?;
142+
write!(f, "DynamicOutputP2WSH {}:{} marked for spending", outpoint.txid, outpoint.index)?;
143143
}
144144
&SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, .. } => {
145-
write!(f, "DynamicOutputP2WPKH {}:{} marked for spending", outpoint.txid, outpoint.vout)?;
145+
write!(f, "DynamicOutputP2WPKH {}:{} marked for spending", outpoint.txid, outpoint.index)?;
146146
}
147147
}
148148
Ok(())

0 commit comments

Comments
 (0)