Skip to content

Commit b7407b2

Browse files
author
Antoine Riard
committed
Implement reorg-safety for SpendableOutputDescriptor detection
We delay SpendableOutputDescriptor until reaching ANTI_REORG_DELAY to avoid misleading user wallet in case of reorg and alternative settlement on a channel output. Fix tests in consequence.
1 parent aa84673 commit b7407b2

File tree

3 files changed

+81
-30
lines changed

3 files changed

+81
-30
lines changed

‎lightning/src/chain/keysinterface.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use ln::msgs::DecodeError;
3737
/// spend on-chain. The information needed to do this is provided in this enum, including the
3838
/// outpoint describing which txid and output index is available, the full output which exists at
3939
/// that txid/index, and any keys or other information required to sign.
40+
#[derive(Clone, PartialEq)]
4041
pub enum SpendableOutputDescriptor {
4142
/// An output to a script which was provided via KeysInterface, thus you should already know
4243
/// how to spend it. No keys are provided as rust-lightning was never given any keys - only the

‎lightning/src/ln/channelmonitor.rs

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -594,6 +594,9 @@ enum OnchainEvent {
594594
HTLCUpdate {
595595
htlc_update: (HTLCSource, PaymentHash),
596596
},
597+
MaturingOutput {
598+
descriptor: SpendableOutputDescriptor,
599+
},
597600
}
598601

599602
const SERIALIZATION_VERSION: u8 = 1;
@@ -1065,6 +1068,10 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
10651068
htlc_update.0.write(writer)?;
10661069
htlc_update.1.write(writer)?;
10671070
},
1071+
OnchainEvent::MaturingOutput { ref descriptor } => {
1072+
1u8.write(writer)?;
1073+
descriptor.write(writer)?;
1074+
},
10681075
}
10691076
}
10701077
}
@@ -1561,6 +1568,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
15611568
OnchainEvent::HTLCUpdate { ref htlc_update } => {
15621569
return htlc_update.0 != **source
15631570
},
1571+
_ => true
15641572
}
15651573
});
15661574
e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1625,6 +1633,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16251633
OnchainEvent::HTLCUpdate { ref htlc_update } => {
16261634
return htlc_update.0 != **source
16271635
},
1636+
_ => true
16281637
}
16291638
});
16301639
e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1808,6 +1817,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
18081817
OnchainEvent::HTLCUpdate { ref htlc_update } => {
18091818
return htlc_update.0 != $source
18101819
},
1820+
_ => true
18111821
}
18121822
});
18131823
e.push(OnchainEvent::HTLCUpdate { htlc_update: ($source, $payment_hash)});
@@ -1961,7 +1971,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
19611971

19621972
log_trace!(self, "Block {} at height {} connected with {} txn matched", block_hash, height, txn_matched.len());
19631973
let mut watch_outputs = Vec::new();
1964-
let mut spendable_outputs = Vec::new();
19651974
let mut claimable_outpoints = Vec::new();
19661975
for tx in txn_matched {
19671976
if tx.input.len() == 1 {
@@ -2011,9 +2020,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20112020
// we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check.
20122021
self.is_resolving_htlc_output(&tx, height);
20132022

2014-
if let Some(spendable_output) = self.is_paying_spendable_output(&tx) {
2015-
spendable_outputs.push(spendable_output);
2016-
}
2023+
self.is_paying_spendable_output(&tx, height);
20172024
}
20182025
let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx {
20192026
self.would_broadcast_at_height(height)
@@ -2058,6 +2065,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20582065
source: htlc_update.0,
20592066
});
20602067
},
2068+
OnchainEvent::MaturingOutput { descriptor } => {
2069+
log_trace!(self, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));
2070+
self.pending_events.push(events::Event::SpendableOutputs {
2071+
outputs: vec![descriptor]
2072+
});
2073+
}
20612074
}
20622075
}
20632076
}
@@ -2068,16 +2081,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20682081
self.outputs_to_watch.insert(txid.clone(), output_scripts.iter().map(|o| o.script_pubkey.clone()).collect());
20692082
}
20702083

2071-
for spend in spendable_outputs.iter() {
2072-
log_trace!(self, "Announcing spendable output to user: {}", log_spendable!(spend));
2073-
}
2074-
2075-
if spendable_outputs.len() > 0 {
2076-
self.pending_events.push(events::Event::SpendableOutputs {
2077-
outputs: spendable_outputs,
2078-
});
2079-
}
2080-
20812084
watch_outputs
20822085
}
20832086

@@ -2089,6 +2092,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20892092
if let Some(_) = self.onchain_events_waiting_threshold_conf.remove(&(height + ANTI_REORG_DELAY - 1)) {
20902093
//We may discard:
20912094
//- htlc update there as failure-trigger tx (revoked commitment tx, non-revoked commitment tx, HTLC-timeout tx) has been disconnected
2095+
//- maturing spendable output has transaction paying us has been disconnected
20922096
}
20932097

20942098
self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator);
@@ -2291,6 +2295,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22912295
OnchainEvent::HTLCUpdate { ref htlc_update } => {
22922296
return htlc_update.0 != source
22932297
},
2298+
_ => true
22942299
}
22952300
});
22962301
e.push(OnchainEvent::HTLCUpdate { htlc_update: (source, payment_hash)});
@@ -2305,39 +2310,54 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
23052310
}
23062311

23072312
/// Check if any transaction broadcasted is paying fund back to some address we can assume to own
2308-
fn is_paying_spendable_output(&self, tx: &Transaction) -> Option<SpendableOutputDescriptor> {
2313+
fn is_paying_spendable_output(&mut self, tx: &Transaction, height: u32) {
2314+
let mut spendable_output = None;
23092315
for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us
23102316
if outp.script_pubkey == self.destination_script {
2311-
return Some(SpendableOutputDescriptor::StaticOutput {
2317+
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
23122318
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
23132319
output: outp.clone(),
23142320
});
2321+
break;
23152322
} else if let Some(ref broadcasted_local_revokable_script) = self.broadcasted_local_revokable_script {
23162323
if broadcasted_local_revokable_script.0 == outp.script_pubkey {
2317-
return Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
2324+
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
23182325
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
23192326
key: broadcasted_local_revokable_script.1,
23202327
witness_script: broadcasted_local_revokable_script.2.clone(),
23212328
to_self_delay: self.their_to_self_delay.unwrap(),
23222329
output: outp.clone(),
23232330
});
2331+
break;
23242332
}
23252333
} else if let Some(ref broadcasted_remote_payment_script) = self.broadcasted_remote_payment_script {
23262334
if broadcasted_remote_payment_script.0 == outp.script_pubkey {
2327-
return Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
2335+
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
23282336
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
23292337
key: broadcasted_remote_payment_script.1,
23302338
output: outp.clone(),
23312339
});
2340+
break;
23322341
}
23332342
} else if outp.script_pubkey == self.shutdown_script {
2334-
return Some(SpendableOutputDescriptor::StaticOutput {
2335-
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2336-
output: outp.clone(),
2343+
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
2344+
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
2345+
output: outp.clone(),
23372346
});
23382347
}
23392348
}
2340-
None
2349+
if let Some(spendable_output) = spendable_output {
2350+
log_trace!(self, "Maturing {} until {}", log_spendable!(spendable_output), height + ANTI_REORG_DELAY - 1);
2351+
match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) {
2352+
hash_map::Entry::Occupied(mut entry) => {
2353+
let e = entry.get_mut();
2354+
e.push(OnchainEvent::MaturingOutput { descriptor: spendable_output });
2355+
}
2356+
hash_map::Entry::Vacant(entry) => {
2357+
entry.insert(vec![OnchainEvent::MaturingOutput { descriptor: spendable_output }]);
2358+
}
2359+
}
2360+
}
23412361
}
23422362
}
23432363

@@ -2588,6 +2608,12 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
25882608
htlc_update: (htlc_source, hash)
25892609
}
25902610
},
2611+
1 => {
2612+
let descriptor = Readable::read(reader)?;
2613+
OnchainEvent::MaturingOutput {
2614+
descriptor
2615+
}
2616+
},
25912617
_ => return Err(DecodeError::InvalidValue),
25922618
};
25932619
events.push(ev);

‎lightning/src/ln/functional_tests.rs

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4049,6 +4049,8 @@ fn test_claim_sizeable_push_msat() {
40494049

40504050
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
40514051
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
4052+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
4053+
40524054
let spend_txn = check_spendable_outputs!(nodes[1], 1);
40534055
assert_eq!(spend_txn.len(), 1);
40544056
check_spends!(spend_txn[0], node_txn[0]);
@@ -4077,6 +4079,8 @@ fn test_claim_on_remote_sizeable_push_msat() {
40774079
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
40784080
check_closed_broadcast!(nodes[1], false);
40794081
check_added_monitors!(nodes[1], 1);
4082+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
4083+
40804084
let spend_txn = check_spendable_outputs!(nodes[1], 1);
40814085
assert_eq!(spend_txn.len(), 2);
40824086
assert_eq!(spend_txn[0], spend_txn[1]);
@@ -4101,13 +4105,14 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
41014105

41024106
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
41034107
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4104-
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
4108+
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 0);
41054109
check_closed_broadcast!(nodes[1], false);
41064110
check_added_monitors!(nodes[1], 1);
41074111

41084112
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
41094113
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4110-
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 2);
4114+
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
4115+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
41114116

41124117
let spend_txn = check_spendable_outputs!(nodes[1], 1);
41134118
assert_eq!(spend_txn.len(), 3);
@@ -4158,6 +4163,7 @@ fn test_static_spendable_outputs_preimage_tx() {
41584163

41594164
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
41604165
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
4166+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
41614167

41624168
let spend_txn = check_spendable_outputs!(nodes[1], 1);
41634169
assert_eq!(spend_txn.len(), 1);
@@ -4182,7 +4188,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
41824188
claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 3_000_000);
41834189

41844190
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4185-
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
4191+
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 0);
41864192
check_closed_broadcast!(nodes[1], false);
41874193
check_added_monitors!(nodes[1], 1);
41884194

@@ -4192,7 +4198,8 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
41924198
check_spends!(node_txn[0], revoked_local_txn[0]);
41934199

41944200
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4195-
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 2);
4201+
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
4202+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
41964203

41974204
let spend_txn = check_spendable_outputs!(nodes[1], 1);
41984205
assert_eq!(spend_txn.len(), 1);
@@ -4247,6 +4254,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
42474254

42484255
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
42494256
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1);
4257+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
42504258

42514259
// Check B's ChannelMonitor was able to generate the right spendable output descriptor
42524260
let spend_txn = check_spendable_outputs!(nodes[1], 1);
@@ -4297,6 +4305,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() {
42974305

42984306
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
42994307
nodes[0].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone(), node_txn[2].clone()] }, 1);
4308+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
43004309

43014310
// Check A's ChannelMonitor was able to generate the right spendable output descriptor
43024311
let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4573,6 +4582,7 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
45734582

45744583
let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
45754584
nodes[1].block_notifier.block_connected(&Block { header: header_201, txdata: node_txn.clone() }, 201);
4585+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
45764586

45774587
// Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
45784588
let spend_txn = check_spendable_outputs!(nodes[1], 1);
@@ -4844,7 +4854,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
48444854
// Create some initial channels
48454855
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
48464856

4847-
route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000).0;
4857+
let (_, our_payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000);
48484858
let local_txn = get_local_commitment_txn!(nodes[0], chan_1.2);
48494859
assert_eq!(local_txn[0].input.len(), 1);
48504860
check_spends!(local_txn[0], chan_1.3);
@@ -4865,6 +4875,15 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
48654875

48664876
let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
48674877
nodes[0].block_notifier.block_connected(&Block { header: header_201, txdata: vec![htlc_timeout.clone()] }, 201);
4878+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
4879+
let events = nodes[0].node.get_and_clear_pending_events();
4880+
assert_eq!(events.len(), 1);
4881+
match events[0] {
4882+
Event::PaymentFailed { payment_hash, .. } => {
4883+
assert_eq!(payment_hash, our_payment_hash);
4884+
},
4885+
_ => panic!("Unexpected event"),
4886+
}
48684887

48694888
// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
48704889
let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4887,12 +4906,16 @@ fn test_static_output_closing_tx() {
48874906
let closing_tx = close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true).2;
48884907

48894908
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4890-
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
4909+
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
4910+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
4911+
48914912
let spend_txn = check_spendable_outputs!(nodes[0], 2);
48924913
assert_eq!(spend_txn.len(), 1);
48934914
check_spends!(spend_txn[0], closing_tx);
48944915

4895-
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
4916+
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
4917+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
4918+
48964919
let spend_txn = check_spendable_outputs!(nodes[1], 2);
48974920
assert_eq!(spend_txn.len(), 1);
48984921
check_spends!(spend_txn[0], closing_tx);
@@ -6744,7 +6767,8 @@ fn test_data_loss_protect() {
67446767
check_spends!(node_txn[0], chan.3);
67456768
assert_eq!(node_txn[0].output.len(), 2);
67466769
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
6747-
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 1);
6770+
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 0);
6771+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
67486772
let spend_txn = check_spendable_outputs!(nodes[0], 1);
67496773
assert_eq!(spend_txn.len(), 1);
67506774
check_spends!(spend_txn[0], node_txn[0]);

0 commit comments

Comments
 (0)