Skip to content

Commit 6b65f43

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 debb3ab commit 6b65f43

File tree

3 files changed

+71
-24
lines changed

3 files changed

+71
-24
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: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,9 @@ enum OnchainEvent {
586586
HTLCUpdate {
587587
htlc_update: (HTLCSource, PaymentHash),
588588
},
589+
MaturingOutput {
590+
descriptor: SpendableOutputDescriptor,
591+
},
589592
}
590593

591594
const SERIALIZATION_VERSION: u8 = 1;
@@ -1040,6 +1043,10 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
10401043
htlc_update.0.write(writer)?;
10411044
htlc_update.1.write(writer)?;
10421045
},
1046+
OnchainEvent::MaturingOutput { ref descriptor } => {
1047+
1u8.write(writer)?;
1048+
descriptor.write(writer)?;
1049+
},
10431050
}
10441051
}
10451052
}
@@ -1531,6 +1538,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
15311538
OnchainEvent::HTLCUpdate { ref htlc_update } => {
15321539
return htlc_update.0 != **source
15331540
},
1541+
_ => true
15341542
}
15351543
});
15361544
e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1595,6 +1603,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
15951603
OnchainEvent::HTLCUpdate { ref htlc_update } => {
15961604
return htlc_update.0 != **source
15971605
},
1606+
_ => true
15981607
}
15991608
});
16001609
e.push(OnchainEvent::HTLCUpdate { htlc_update: ((**source).clone(), htlc.payment_hash.clone())});
@@ -1778,6 +1787,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17781787
OnchainEvent::HTLCUpdate { ref htlc_update } => {
17791788
return htlc_update.0 != $source
17801789
},
1790+
_ => true
17811791
}
17821792
});
17831793
e.push(OnchainEvent::HTLCUpdate { htlc_update: ($source, $payment_hash)});
@@ -1928,7 +1938,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
19281938

19291939
log_trace!(self, "Block {} at height {} connected with {} txn matched", block_hash, height, txn_matched.len());
19301940
let mut watch_outputs = Vec::new();
1931-
let mut spendable_outputs = Vec::new();
19321941
let mut claimable_outpoints = Vec::new();
19331942
for tx in txn_matched {
19341943
if tx.input.len() == 1 {
@@ -1978,9 +1987,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
19781987
// we call is_resolving_htlc_output here outside of the tx.input.len() == 1 check.
19791988
self.is_resolving_htlc_output(&tx, height);
19801989

1981-
if let Some(spendable_output) = self.is_mine(&tx) {
1982-
spendable_outputs.push(spendable_output);
1983-
}
1990+
self.is_mine(&tx, height);
19841991
}
19851992
let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx {
19861993
self.would_broadcast_at_height(height)
@@ -2025,6 +2032,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20252032
source: htlc_update.0,
20262033
});
20272034
},
2035+
OnchainEvent::MaturingOutput { descriptor } => {
2036+
log_trace!(self, "Descriptor {} has got enough confirmations to be passed upstream", log_spendable!(descriptor));
2037+
self.pending_events.push(events::Event::SpendableOutputs {
2038+
outputs: vec![descriptor]
2039+
});
2040+
}
20282041
}
20292042
}
20302043
}
@@ -2035,16 +2048,6 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20352048
self.outputs_to_watch.insert(txid.clone(), output_scripts.iter().map(|o| o.script_pubkey.clone()).collect());
20362049
}
20372050

2038-
for spend in spendable_outputs.iter() {
2039-
log_trace!(self, "{}", log_spendable!(spend));
2040-
}
2041-
2042-
if spendable_outputs.len() > 0 {
2043-
self.pending_events.push(events::Event::SpendableOutputs {
2044-
outputs: spendable_outputs,
2045-
});
2046-
}
2047-
20482051
watch_outputs
20492052
}
20502053

@@ -2254,6 +2257,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22542257
OnchainEvent::HTLCUpdate { ref htlc_update } => {
22552258
return htlc_update.0 != source
22562259
},
2260+
_ => true
22572261
}
22582262
});
22592263
e.push(OnchainEvent::HTLCUpdate { htlc_update: (source, payment_hash)});
@@ -2268,16 +2272,17 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22682272
}
22692273

22702274
/// Check if any transaction broadcasted is paying fund back to some address we can assume to own
2271-
fn is_mine(&self, tx: &Transaction) -> Option<SpendableOutputDescriptor> {
2275+
fn is_mine(&mut self, tx: &Transaction, height: u32) {
2276+
let mut spendable_output = None;
22722277
for (i, outp) in tx.output.iter().enumerate() { // There is max one spendable output for any channel tx, including ones generated by us
22732278
if outp.script_pubkey == self.destination_script {
2274-
return Some(SpendableOutputDescriptor::StaticOutput {
2279+
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
22752280
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
22762281
output: outp.clone(),
22772282
});
22782283
} else if let Some(ref revokeable_script) = self.revokeable_script {
22792284
if revokeable_script.0 == outp.script_pubkey {
2280-
return Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
2285+
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH {
22812286
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
22822287
key: revokeable_script.1,
22832288
witness_script: revokeable_script.2.clone(),
@@ -2287,7 +2292,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22872292
}
22882293
} else if let Some(ref payment_keys) = self.payment_keys {
22892294
if payment_keys.0 == outp.script_pubkey {
2290-
return Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
2295+
spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
22912296
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
22922297
key: payment_keys.1,
22932298
output: outp.clone(),
@@ -2299,7 +2304,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
22992304
let our_channel_close_key_hash = Hash160::hash(&shutdown_pubkey.serialize());
23002305
let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script();
23012306
if shutdown_script == outp.script_pubkey {
2302-
return Some(SpendableOutputDescriptor::StaticOutput {
2307+
spendable_output = Some(SpendableOutputDescriptor::StaticOutput {
23032308
outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 },
23042309
output: outp.clone(),
23052310
});
@@ -2309,7 +2314,18 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
23092314
}
23102315
}
23112316
}
2312-
None
2317+
if let Some(spendable_output) = spendable_output {
2318+
log_trace!(self, "Maturing {} until {}", log_spendable!(spendable_output), height + ANTI_REORG_DELAY - 1);
2319+
match self.onchain_events_waiting_threshold_conf.entry(height + ANTI_REORG_DELAY - 1) {
2320+
hash_map::Entry::Occupied(mut entry) => {
2321+
let e = entry.get_mut();
2322+
e.push(OnchainEvent::MaturingOutput { descriptor: spendable_output });
2323+
}
2324+
hash_map::Entry::Vacant(entry) => {
2325+
entry.insert(vec![OnchainEvent::MaturingOutput { descriptor: spendable_output }]);
2326+
}
2327+
}
2328+
}
23132329
}
23142330
}
23152331

@@ -2561,6 +2577,12 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
25612577
htlc_update: (htlc_source, hash)
25622578
}
25632579
},
2580+
1 => {
2581+
let descriptor = Readable::read(reader)?;
2582+
OnchainEvent::MaturingOutput {
2583+
descriptor
2584+
}
2585+
},
25642586
_ => return Err(DecodeError::InvalidValue),
25652587
};
25662588
events.push(ev);

‎lightning/src/ln/functional_tests.rs

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4012,6 +4012,8 @@ fn test_claim_sizeable_push_msat() {
40124012

40134013
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
40144014
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
4015+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
4016+
40154017
let spend_txn = check_spendable_outputs!(nodes[1], 1);
40164018
assert_eq!(spend_txn.len(), 1);
40174019
check_spends!(spend_txn[0], node_txn[0]);
@@ -4038,6 +4040,8 @@ fn test_claim_on_remote_sizeable_push_msat() {
40384040
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
40394041
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0);
40404042
check_closed_broadcast!(nodes[1], false);
4043+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
4044+
40414045
let spend_txn = check_spendable_outputs!(nodes[1], 1);
40424046
assert_eq!(spend_txn.len(), 2);
40434047
assert_eq!(spend_txn[0], spend_txn[1]);
@@ -4068,6 +4072,7 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() {
40684072
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
40694073
let header_1 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
40704074
nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1);
4075+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash());
40714076

40724077
let spend_txn = check_spendable_outputs!(nodes[1], 1);
40734078
assert_eq!(spend_txn.len(), 3);
@@ -4117,6 +4122,7 @@ fn test_static_spendable_outputs_preimage_tx() {
41174122

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

41214127
let spend_txn = check_spendable_outputs!(nodes[1], 1);
41224128
assert_eq!(spend_txn.len(), 1);
@@ -4151,6 +4157,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() {
41514157

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

41554162
let spend_txn = check_spendable_outputs!(nodes[1], 1);
41564163
assert_eq!(spend_txn.len(), 1);
@@ -4203,6 +4210,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
42034210

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

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

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

42554264
// Check A's ChannelMonitor was able to generate the right spendable output descriptor
42564265
let spend_txn = check_spendable_outputs!(nodes[0], 1);
@@ -4522,6 +4531,7 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
45224531

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

45264536
// Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
45274537
let spend_txn = check_spendable_outputs!(nodes[1], 1);
@@ -4793,7 +4803,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
47934803
// Create some initial channels
47944804
let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::supported(), InitFeatures::supported());
47954805

4796-
route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000).0;
4806+
let (_, our_payment_hash) = route_payment(&nodes[0], &vec!(&nodes[1])[..], 9000000);
47974807
let local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get_mut(&chan_1.2).unwrap().channel_monitor().get_latest_local_commitment_txn();
47984808
assert_eq!(local_txn[0].input.len(), 1);
47994809
check_spends!(local_txn[0], chan_1.3);
@@ -4813,6 +4823,15 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
48134823

48144824
let header_201 = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
48154825
nodes[0].block_notifier.block_connected(&Block { header: header_201, txdata: vec![htlc_timeout.clone()] }, 201);
4826+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash());
4827+
let events = nodes[0].node.get_and_clear_pending_events();
4828+
assert_eq!(events.len(), 1);
4829+
match events[0] {
4830+
Event::PaymentFailed { payment_hash, .. } => {
4831+
assert_eq!(payment_hash, our_payment_hash);
4832+
},
4833+
_ => panic!("Unexpected event"),
4834+
}
48164835

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

48374856
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
4838-
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
4857+
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
4858+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
4859+
48394860
let spend_txn = check_spendable_outputs!(nodes[0], 2);
48404861
assert_eq!(spend_txn.len(), 1);
48414862
check_spends!(spend_txn[0], closing_tx);
48424863

4843-
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 1);
4864+
nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0);
4865+
connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
4866+
48444867
let spend_txn = check_spendable_outputs!(nodes[1], 2);
48454868
assert_eq!(spend_txn.len(), 1);
48464869
check_spends!(spend_txn[0], closing_tx);
@@ -6714,7 +6737,8 @@ fn test_data_loss_protect() {
67146737
check_spends!(node_txn[0], chan.3);
67156738
assert_eq!(node_txn[0].output.len(), 2);
67166739
let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42};
6717-
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 1);
6740+
nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 0);
6741+
connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash());
67186742
let spend_txn = check_spendable_outputs!(nodes[0], 1);
67196743
assert_eq!(spend_txn.len(), 1);
67206744
check_spends!(spend_txn[0], node_txn[0]);

0 commit comments

Comments
 (0)