Skip to content

Commit 06fc27d

Browse files
TheBlueMattTheBlueMatt
committed
De-Option<> current_local_signed_commitment_tx in ChannelMonitor
Since we now are always initialised with an initial local commitment transaction available now, we might as well take advantage of it and stop using an Option<> where we don't need to.
1 parent 7631dfb commit 06fc27d

File tree

2 files changed

+71
-77
lines changed

2 files changed

+71
-77
lines changed

‎lightning/src/ln/channel.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,10 +1515,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
15151515
&their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
15161516
self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
15171517
self.get_commitment_transaction_number_obscure_factor(),
1518+
local_initial_commitment_tx.clone(),
15181519
self.logger.clone());
15191520

15201521
channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
1521-
channel_monitor.provide_latest_local_commitment_tx_info(local_initial_commitment_tx.clone(), Vec::new()).unwrap();
15221522
channel_monitor
15231523
} }
15241524
}
@@ -1574,16 +1574,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
15741574
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
15751575
macro_rules! create_monitor {
15761576
() => { {
1577+
let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
15771578
let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
15781579
&self.shutdown_pubkey, self.our_to_self_delay,
15791580
&self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
15801581
&their_pubkeys.htlc_basepoint, &their_pubkeys.delayed_payment_basepoint,
15811582
self.their_to_self_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
15821583
self.get_commitment_transaction_number_obscure_factor(),
1584+
local_commitment_tx,
15831585
self.logger.clone());
15841586

15851587
channel_monitor.provide_latest_remote_commitment_tx_info(&remote_initial_commitment_tx, Vec::new(), self.cur_remote_commitment_transaction_number, self.their_cur_commitment_point.unwrap());
1586-
channel_monitor.provide_latest_local_commitment_tx_info(LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), &msg.signature, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()), Vec::new()).unwrap();
15871588

15881589
channel_monitor
15891590
} }

‎lightning/src/ln/channelmonitor.rs‎

Lines changed: 68 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ pub struct ChannelMonitor<ChanSigner: ChannelKeys> {
738738
// various monitors for one channel being out of sync, and us broadcasting a local
739739
// transaction for which we have deleted claim information on some watchtowers.
740740
prev_local_signed_commitment_tx: Option<LocalSignedTx>,
741-
current_local_signed_commitment_tx: Option<LocalSignedTx>,
741+
current_local_commitment_tx: LocalSignedTx,
742742

743743
// Used just for ChannelManager to make sure it has the latest channel data during
744744
// deserialization
@@ -809,7 +809,7 @@ impl<ChanSigner: ChannelKeys> PartialEq for ChannelMonitor<ChanSigner> {
809809
self.prev_local_signed_commitment_tx != other.prev_local_signed_commitment_tx ||
810810
self.current_remote_commitment_number != other.current_remote_commitment_number ||
811811
self.current_local_commitment_number != other.current_local_commitment_number ||
812-
self.current_local_signed_commitment_tx != other.current_local_signed_commitment_tx ||
812+
self.current_local_commitment_tx != other.current_local_commitment_tx ||
813813
self.payment_preimages != other.payment_preimages ||
814814
self.pending_htlcs_updated != other.pending_htlcs_updated ||
815815
self.pending_events.len() != other.pending_events.len() || // We trust events to round-trip properly
@@ -963,12 +963,7 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
963963
writer.write_all(&[0; 1])?;
964964
}
965965

966-
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
967-
writer.write_all(&[1; 1])?;
968-
serialize_local_tx!(cur_local_tx);
969-
} else {
970-
writer.write_all(&[0; 1])?;
971-
}
966+
serialize_local_tx!(self.current_local_commitment_tx);
972967

973968
writer.write_all(&byte_utils::be48_to_array(self.current_remote_commitment_number))?;
974969
writer.write_all(&byte_utils::be48_to_array(self.current_local_commitment_number))?;
@@ -1031,12 +1026,34 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
10311026
their_htlc_base_key: &PublicKey, their_delayed_payment_base_key: &PublicKey,
10321027
their_to_self_delay: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
10331028
commitment_transaction_number_obscure_factor: u64,
1029+
initial_local_commitment_tx: LocalCommitmentTransaction,
10341030
logger: Arc<Logger>) -> ChannelMonitor<ChanSigner> {
10351031

10361032
assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
10371033
let our_channel_close_key_hash = Hash160::hash(&shutdown_pubkey.serialize());
10381034
let shutdown_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&our_channel_close_key_hash[..]).into_script();
10391035

1036+
let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), their_to_self_delay, logger.clone());
1037+
1038+
let local_tx_sequence = initial_local_commitment_tx.without_valid_witness().input[0].sequence as u64;
1039+
let local_tx_locktime = initial_local_commitment_tx.without_valid_witness().lock_time as u64;
1040+
let local_commitment_tx = LocalSignedTx {
1041+
txid: initial_local_commitment_tx.txid(),
1042+
revocation_key: initial_local_commitment_tx.local_keys.revocation_key,
1043+
a_htlc_key: initial_local_commitment_tx.local_keys.a_htlc_key,
1044+
b_htlc_key: initial_local_commitment_tx.local_keys.b_htlc_key,
1045+
delayed_payment_key: initial_local_commitment_tx.local_keys.a_delayed_payment_key,
1046+
per_commitment_point: initial_local_commitment_tx.local_keys.per_commitment_point,
1047+
feerate_per_kw: initial_local_commitment_tx.feerate_per_kw,
1048+
htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
1049+
};
1050+
// Returning a monitor error before updating tracking points means in case of using
1051+
// a concurrent watchtower implementation for same channel, if this one doesn't
1052+
// reject update as we do, you MAY have the latest local valid commitment tx onchain
1053+
// for which you want to spend outputs. We're NOT robust again this scenario right
1054+
// now but we should consider it later.
1055+
onchain_tx_handler.provide_latest_local_tx(initial_local_commitment_tx).unwrap();
1056+
10401057
ChannelMonitor {
10411058
latest_update_id: 0,
10421059
commitment_transaction_number_obscure_factor,
@@ -1046,14 +1063,14 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
10461063
broadcasted_remote_payment_script: None,
10471064
shutdown_script,
10481065

1049-
keys: keys.clone(),
1066+
keys,
10501067
funding_info,
10511068
current_remote_commitment_txid: None,
10521069
prev_remote_commitment_txid: None,
10531070

10541071
their_htlc_base_key: their_htlc_base_key.clone(),
10551072
their_delayed_payment_base_key: their_delayed_payment_base_key.clone(),
1056-
funding_redeemscript: funding_redeemscript.clone(),
1073+
funding_redeemscript,
10571074
channel_value_satoshis: channel_value_satoshis,
10581075
their_cur_revocation_points: None,
10591076

@@ -1066,9 +1083,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
10661083
remote_hash_commitment_number: HashMap::new(),
10671084

10681085
prev_local_signed_commitment_tx: None,
1069-
current_local_signed_commitment_tx: None,
1086+
current_local_commitment_tx: local_commitment_tx,
10701087
current_remote_commitment_number: 1 << 48,
1071-
current_local_commitment_number: 0xffff_ffff_ffff,
1088+
current_local_commitment_number: 0xffff_ffff_ffff - ((((local_tx_sequence & 0xffffff) << 3*8) | (local_tx_locktime as u64 & 0xffffff)) ^ commitment_transaction_number_obscure_factor),
10721089

10731090
payment_preimages: HashMap::new(),
10741091
pending_htlcs_updated: Vec::new(),
@@ -1077,7 +1094,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
10771094
onchain_events_waiting_threshold_conf: HashMap::new(),
10781095
outputs_to_watch: HashMap::new(),
10791096

1080-
onchain_tx_handler: OnchainTxHandler::new(destination_script.clone(), keys, their_to_self_delay, logger.clone()),
1097+
onchain_tx_handler,
10811098

10821099
lockdown_from_offchain: false,
10831100

@@ -1104,13 +1121,13 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
11041121
}
11051122

11061123
if !self.payment_preimages.is_empty() {
1107-
let local_signed_commitment_tx = self.current_local_signed_commitment_tx.as_ref().expect("Channel needs at least an initial commitment tx !");
1124+
let cur_local_signed_commitment_tx = &self.current_local_commitment_tx;
11081125
let prev_local_signed_commitment_tx = self.prev_local_signed_commitment_tx.as_ref();
11091126
let min_idx = self.get_min_seen_secret();
11101127
let remote_hash_commitment_number = &mut self.remote_hash_commitment_number;
11111128

11121129
self.payment_preimages.retain(|&k, _| {
1113-
for &(ref htlc, _, _) in &local_signed_commitment_tx.htlc_outputs {
1130+
for &(ref htlc, _, _) in cur_local_signed_commitment_tx.htlc_outputs.iter() {
11141131
if k == htlc.payment_hash {
11151132
return true
11161133
}
@@ -1199,7 +1216,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
11991216
let txid = commitment_tx.txid();
12001217
let sequence = commitment_tx.without_valid_witness().input[0].sequence as u64;
12011218
let locktime = commitment_tx.without_valid_witness().lock_time as u64;
1202-
let new_local_signed_commitment_tx = LocalSignedTx {
1219+
let mut new_local_commitment_tx = LocalSignedTx {
12031220
txid,
12041221
revocation_key: commitment_tx.local_keys.revocation_key,
12051222
a_htlc_key: commitment_tx.local_keys.a_htlc_key,
@@ -1218,8 +1235,8 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
12181235
return Err(MonitorUpdateError("Local commitment signed has already been signed, no further update of LOCAL commitment transaction is allowed"));
12191236
}
12201237
self.current_local_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
1221-
self.prev_local_signed_commitment_tx = self.current_local_signed_commitment_tx.take();
1222-
self.current_local_signed_commitment_tx = Some(new_local_signed_commitment_tx);
1238+
mem::swap(&mut new_local_commitment_tx, &mut self.current_local_commitment_tx);
1239+
self.prev_local_signed_commitment_tx = Some(new_local_commitment_tx);
12231240
Ok(())
12241241
}
12251242

@@ -1676,15 +1693,12 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16761693
// HTLCs set may differ between last and previous local commitment txn, in case of one them hitting chain, ensure we cancel all HTLCs backward
16771694
let mut is_local_tx = false;
16781695

1679-
if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx {
1680-
if local_tx.txid == commitment_txid {
1681-
is_local_tx = true;
1682-
log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim");
1683-
let mut res = self.broadcast_by_local_state(tx, local_tx);
1684-
append_onchain_update!(res);
1685-
}
1686-
}
1687-
if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx {
1696+
if self.current_local_commitment_tx.txid == commitment_txid {
1697+
is_local_tx = true;
1698+
log_trace!(self, "Got latest local commitment tx broadcast, searching for available HTLCs to claim");
1699+
let mut res = self.broadcast_by_local_state(tx, &self.current_local_commitment_tx);
1700+
append_onchain_update!(res);
1701+
} else if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx {
16881702
if local_tx.txid == commitment_txid {
16891703
is_local_tx = true;
16901704
log_trace!(self, "Got previous local commitment tx broadcast, searching for available HTLCs to claim");
@@ -1706,9 +1720,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17061720
}
17071721

17081722
if is_local_tx {
1709-
if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx {
1710-
fail_dust_htlcs_after_threshold_conf!(local_tx);
1711-
}
1723+
fail_dust_htlcs_after_threshold_conf!(self.current_local_commitment_tx);
17121724
if let &Some(ref local_tx) = &self.prev_local_signed_commitment_tx {
17131725
fail_dust_htlcs_after_threshold_conf!(local_tx);
17141726
}
@@ -1731,18 +1743,16 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17311743
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() {
17321744
let txid = commitment_tx.txid();
17331745
let mut res = vec![commitment_tx];
1734-
if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx {
1735-
for htlc in local_tx.htlc_outputs.iter() {
1736-
if let Some(htlc_index) = htlc.0.transaction_output_index {
1737-
let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None };
1738-
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) {
1739-
res.push(htlc_tx);
1740-
}
1746+
for htlc in self.current_local_commitment_tx.htlc_outputs.iter() {
1747+
if let Some(htlc_index) = htlc.0.transaction_output_index {
1748+
let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None };
1749+
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) {
1750+
res.push(htlc_tx);
17411751
}
17421752
}
1743-
// We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
1744-
// The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation.
17451753
}
1754+
// We throw away the generated waiting_first_conf data as we aren't (yet) confirmed and we don't actually know what the caller wants to do.
1755+
// The data will be re-generated and tracked in check_spend_local_transaction if we get a confirmation.
17461756
return res
17471757
}
17481758
Vec::new()
@@ -1757,13 +1767,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17571767
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_copy_local_tx() {
17581768
let txid = commitment_tx.txid();
17591769
let mut res = vec![commitment_tx];
1760-
if let &Some(ref local_tx) = &self.current_local_signed_commitment_tx {
1761-
for htlc in local_tx.htlc_outputs.iter() {
1762-
if let Some(htlc_index) = htlc.0.transaction_output_index {
1763-
let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None };
1764-
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) {
1765-
res.push(htlc_tx);
1766-
}
1770+
for htlc in self.current_local_commitment_tx.htlc_outputs.iter() {
1771+
if let Some(htlc_index) = htlc.0.transaction_output_index {
1772+
let preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.0.payment_hash) { Some(*preimage) } else { None };
1773+
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(txid, htlc_index, preimage) {
1774+
res.push(htlc_tx);
17671775
}
17681776
}
17691777
}
@@ -1832,21 +1840,17 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
18321840

18331841
self.is_paying_spendable_output(&tx, height);
18341842
}
1835-
let should_broadcast = if let Some(_) = self.current_local_signed_commitment_tx {
1836-
self.would_broadcast_at_height(height)
1837-
} else { false };
1843+
let should_broadcast = self.would_broadcast_at_height(height);
18381844
if should_broadcast {
18391845
claimable_outpoints.push(ClaimRequest { absolute_timelock: height, aggregable: false, outpoint: BitcoinOutPoint { txid: self.funding_info.0.txid.clone(), vout: self.funding_info.0.index as u32 }, witness_data: InputMaterial::Funding { channel_value: self.channel_value_satoshis }});
18401846
}
1841-
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
1842-
if should_broadcast {
1843-
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() {
1844-
let (mut new_outpoints, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, cur_local_tx);
1845-
if !new_outputs.is_empty() {
1846-
watch_outputs.push((cur_local_tx.txid.clone(), new_outputs));
1847-
}
1848-
claimable_outpoints.append(&mut new_outpoints);
1847+
if should_broadcast {
1848+
if let Some(commitment_tx) = self.onchain_tx_handler.get_fully_signed_local_tx() {
1849+
let (mut new_outpoints, new_outputs, _) = self.broadcast_by_local_state(&commitment_tx, &self.current_local_commitment_tx);
1850+
if !new_outputs.is_empty() {
1851+
watch_outputs.push((self.current_local_commitment_tx.txid.clone(), new_outputs));
18491852
}
1853+
claimable_outpoints.append(&mut new_outpoints);
18501854
}
18511855
}
18521856
if let Some(events) = self.onchain_events_waiting_threshold_conf.remove(&height) {
@@ -1942,9 +1946,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
19421946
}
19431947
}
19441948

1945-
if let Some(ref cur_local_tx) = self.current_local_signed_commitment_tx {
1946-
scan_commitment!(cur_local_tx.htlc_outputs.iter().map(|&(ref a, _, _)| a), true);
1947-
}
1949+
scan_commitment!(self.current_local_commitment_tx.htlc_outputs.iter().map(|&(ref a, _, _)| a), true);
19481950

19491951
if let Some(ref txid) = self.current_remote_commitment_txid {
19501952
if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
@@ -2035,11 +2037,9 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20352037
}
20362038
}
20372039

2038-
if let Some(ref current_local_signed_commitment_tx) = self.current_local_signed_commitment_tx {
2039-
if input.previous_output.txid == current_local_signed_commitment_tx.txid {
2040-
scan_commitment!(current_local_signed_commitment_tx.htlc_outputs.iter().map(|&(ref a, _, ref b)| (a, b.as_ref())),
2041-
"our latest local commitment tx", true);
2042-
}
2040+
if input.previous_output.txid == self.current_local_commitment_tx.txid {
2041+
scan_commitment!(self.current_local_commitment_tx.htlc_outputs.iter().map(|&(ref a, _, ref b)| (a, b.as_ref())),
2042+
"our latest local commitment tx", true);
20432043
}
20442044
if let Some(ref prev_local_signed_commitment_tx) = self.prev_local_signed_commitment_tx {
20452045
if input.previous_output.txid == prev_local_signed_commitment_tx.txid {
@@ -2324,14 +2324,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
23242324
},
23252325
_ => return Err(DecodeError::InvalidValue),
23262326
};
2327-
2328-
let current_local_signed_commitment_tx = match <u8 as Readable>::read(reader)? {
2329-
0 => None,
2330-
1 => {
2331-
Some(read_local_tx!())
2332-
},
2333-
_ => return Err(DecodeError::InvalidValue),
2334-
};
2327+
let current_local_commitment_tx = read_local_tx!();
23352328

23362329
let current_remote_commitment_number = <U48 as Readable>::read(reader)?.0;
23372330
let current_local_commitment_number = <U48 as Readable>::read(reader)?.0;
@@ -2436,7 +2429,7 @@ impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for (Sha256dH
24362429
remote_hash_commitment_number,
24372430

24382431
prev_local_signed_commitment_tx,
2439-
current_local_signed_commitment_tx,
2432+
current_local_commitment_tx,
24402433
current_remote_commitment_number,
24412434
current_local_commitment_number,
24422435

@@ -2555,7 +2548,7 @@ mod tests {
25552548
(OutPoint { txid: Sha256dHash::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
25562549
&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
25572550
&PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
2558-
10, Script::new(), 46, 0, logger.clone());
2551+
10, Script::new(), 46, 0, LocalCommitmentTransaction::dummy(), logger.clone());
25592552

25602553
monitor.provide_latest_local_commitment_tx_info(LocalCommitmentTransaction::dummy(), preimages_to_local_htlcs!(preimages[0..10])).unwrap();
25612554
monitor.provide_latest_remote_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key);

0 commit comments

Comments
 (0)