Skip to content

Commit 830ceae

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 ffc724b commit 830ceae

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
@@ -2198,16 +2198,30 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
21982198
fn is_paying_spendable_output<L: Deref>(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger {
21992199
let mut spendable_output = None;
22002200
for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us
2201+
if i > ::std::u16::MAX as usize {
2202+
// While it is possible that an output exists on chain which is greater than the
2203+
// 2^16th output in a given transaction, this is only possible if the output is not
2204+
// in a lightning transaction and was instead placed there by some third party who
2205+
// wishes to give us money for no reason.
2206+
// Namely, any lightning transactions which we pre-sign will never have anywhere
2207+
// near 2^16 outputs both because such transactions must have ~2^16 outputs who's
2208+
// scripts are not longer than one byte in length and because they are inherently
2209+
// non-standard due to their size.
2210+
// Thus, it is completely safe to ignore such outputs, and while it may result in
2211+
// us ignoring non-lightning fund to us, that is only possible if someone fills
2212+
// nearly a full block with garbage just to hit this case.
2213+
continue;
2214+
}
22012215
if outp.script_pubkey == self.destination_script {
22022216
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
2203-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2217+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22042218
output: outp.clone(),
22052219
});
22062220
break;
22072221
} else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script {
22082222
if broadcasted_local_revokable_script.0 == outp.script_pubkey {
22092223
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
2210-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2224+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22112225
per_commitment_point: broadcasted_local_revokable_script.1,
22122226
to_self_delay: self.on_local_tx_csv,
22132227
output: outp.clone(),
@@ -2218,14 +2232,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22182232
}
22192233
} else if self.remote_payment_script == outp.script_pubkey {
22202234
spendable_output = Some(SpendableOutputDescriptor::StaticOutputRemotePayment {
2221-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2235+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22222236
output: outp.clone(),
22232237
key_derivation_params: self.keys.key_derivation_params(),
22242238
});
22252239
break;
22262240
} else if outp.script_pubkey == self.shutdown_script {
22272241
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
2228-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2242+
outpoint: OutPoint { txid: tx.txid(), index: i as u16 },
22292243
output: outp.clone(),
22302244
});
22312245
}

lightning/src/ln/functional_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4655,7 +4655,7 @@ macro_rules! check_spendable_outputs {
46554655
match *outp {
46564656
SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, ref output, ref key_derivation_params } => {
46574657
let input = TxIn {
4658-
previous_output: outpoint.clone(),
4658+
previous_output: outpoint.into_bitcoin_outpoint(),
46594659
script_sig: Script::new(),
46604660
sequence: 0,
46614661
witness: Vec::new(),
@@ -4683,7 +4683,7 @@ macro_rules! check_spendable_outputs {
46834683
},
46844684
SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref remote_revocation_pubkey } => {
46854685
let input = TxIn {
4686-
previous_output: outpoint.clone(),
4686+
previous_output: outpoint.into_bitcoin_outpoint(),
46874687
script_sig: Script::new(),
46884688
sequence: *to_self_delay as u32,
46894689
witness: Vec::new(),
@@ -4716,7 +4716,7 @@ macro_rules! check_spendable_outputs {
47164716
SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
47174717
let secp_ctx = Secp256k1::new();
47184718
let input = TxIn {
4719-
previous_output: outpoint.clone(),
4719+
previous_output: outpoint.into_bitcoin_outpoint(),
47204720
script_sig: Script::new(),
47214721
sequence: 0,
47224722
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)