Skip to content

Commit 26bf274

Browse files
Add would_broadcast_at_height functionality to Channel
In service to the larger refactor of removing the Channel's reference to its ChannelMonitor.
1 parent d735a24 commit 26bf274

File tree

3 files changed

+180
-38
lines changed

3 files changed

+180
-38
lines changed

lightning/src/ln/channel.rs

Lines changed: 156 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use ln::chan_utils;
2424
use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
2525
use chain::transaction::OutPoint;
2626
use chain::keysinterface::{ChannelKeys, KeysInterface};
27-
use util::transaction_utils;
27+
use util::{byte_utils, transaction_utils};
2828
use util::ser::{Readable, Writeable, Writer};
2929
use util::logger::Logger;
3030
use util::errors::APIError;
@@ -34,6 +34,7 @@ use std;
3434
use std::default::Default;
3535
use std::{cmp,mem,fmt};
3636
use std::ops::Deref;
37+
use std::collections::HashMap;
3738
use bitcoin::hashes::hex::ToHex;
3839

3940
#[cfg(test)]
@@ -100,10 +101,7 @@ enum InboundHTLCState {
100101
/// Note that we have to keep an eye on the HTLC until we've received a broadcastable
101102
/// commitment transaction without it as otherwise we'll have to force-close the channel to
102103
/// claim it before the timeout (obviously doesn't apply to revoked HTLCs that we can't claim
103-
/// anyway). That said, ChannelMonitor does this for us (see
104-
/// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
105-
/// local state before then, once we're sure that the next commitment_signed and
106-
/// ChannelMonitor::provide_latest_local_commitment_tx_info will not include this HTLC.
104+
/// anyway).
107105
LocalRemoved(InboundHTLCRemovalReason),
108106
}
109107

@@ -284,6 +282,8 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
284282
pending_inbound_htlcs: Vec<InboundHTLCOutput>,
285283
pending_outbound_htlcs: Vec<OutboundHTLCOutput>,
286284
holding_cell_htlc_updates: Vec<HTLCUpdateAwaitingACK>,
285+
payment_preimages: HashMap<PaymentHash, PaymentPreimage>,
286+
pending_drops: Vec<InboundHTLCOutput>,
287287

288288
/// When resending CS/RAA messages on channel monitor restoration or on reconnect, we always
289289
/// need to ensure we resend them in the order we originally generated them. Note that because
@@ -501,6 +501,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
501501
pending_inbound_htlcs: Vec::new(),
502502
pending_outbound_htlcs: Vec::new(),
503503
holding_cell_htlc_updates: Vec::new(),
504+
payment_preimages: HashMap::new(),
505+
pending_drops: Vec::new(),
504506
pending_update_fee: None,
505507
holding_cell_update_fee: None,
506508
next_local_htlc_id: 0,
@@ -729,6 +731,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
729731
pending_inbound_htlcs: Vec::new(),
730732
pending_outbound_htlcs: Vec::new(),
731733
holding_cell_htlc_updates: Vec::new(),
734+
payment_preimages: HashMap::new(),
735+
pending_drops: Vec::new(),
732736
pending_update_fee: None,
733737
holding_cell_update_fee: None,
734738
next_local_htlc_id: 0,
@@ -786,6 +790,70 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
786790
Ok(chan)
787791
}
788792

793+
pub(super) fn monitor_would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {
794+
let remote_keys = self.build_remote_transaction_keys().unwrap();
795+
let (_tx, _size, remote_htlc_outputs) = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true, self.feerate_per_kw, logger);
796+
797+
let curr_remote_outputs: Vec<&HTLCOutputInCommitment> = remote_htlc_outputs
798+
.iter().map(|&(ref a, _)| a).collect();
799+
800+
macro_rules! add_htlc_output {
801+
($htlc: expr, $offered: expr, $list: expr) => {
802+
$list.push(HTLCOutputInCommitment{
803+
offered: $offered,
804+
amount_msat: $htlc.amount_msat,
805+
cltv_expiry: $htlc.cltv_expiry,
806+
payment_hash: $htlc.payment_hash,
807+
transaction_output_index: None
808+
});
809+
}
810+
}
811+
812+
let mut prev_remote_htlc_outputs = Vec::new();
813+
let mut local_outputs = Vec::new();
814+
let awaiting_raa = (self.channel_state & ChannelState::AwaitingRemoteRevoke as u32) != 0;
815+
for ref htlc in self.pending_inbound_htlcs.iter() {
816+
match htlc.state {
817+
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) => add_htlc_output!(htlc, false, local_outputs),
818+
InboundHTLCState::Committed => add_htlc_output!(htlc, false, local_outputs),
819+
InboundHTLCState::LocalRemoved(_) => {
820+
add_htlc_output!(htlc, false, local_outputs);
821+
if awaiting_raa {
822+
add_htlc_output!(htlc, true, prev_remote_htlc_outputs)
823+
}
824+
},
825+
_ => {},
826+
}
827+
}
828+
for ref htlc in self.pending_outbound_htlcs.iter() {
829+
match htlc.state {
830+
OutboundHTLCState::LocalAnnounced(_) => add_htlc_output!(htlc, true, local_outputs),
831+
OutboundHTLCState::Committed => add_htlc_output!(htlc, true, local_outputs),
832+
OutboundHTLCState::RemoteRemoved(_) => {
833+
add_htlc_output!(htlc, true, local_outputs);
834+
if awaiting_raa {
835+
add_htlc_output!(htlc, false, prev_remote_htlc_outputs)
836+
}
837+
},
838+
OutboundHTLCState::AwaitingRemoteRevokeToRemove(_) => add_htlc_output!(htlc, true, local_outputs),
839+
OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => {
840+
if awaiting_raa {
841+
add_htlc_output!(htlc, false, prev_remote_htlc_outputs)
842+
}
843+
},
844+
}
845+
}
846+
847+
for ref htlc in self.pending_drops.iter() {
848+
add_htlc_output!(htlc, false, local_outputs);
849+
}
850+
851+
let local_htlc_outputs = local_outputs.iter().collect();
852+
let prev_remote_outputs = prev_remote_htlc_outputs.iter().collect();
853+
let remote_outputs = [curr_remote_outputs, prev_remote_outputs].concat();
854+
ChannelMonitor::<ChanSigner>::would_broadcast_at_height_given_htlcs(local_htlc_outputs, remote_outputs, height, &self.payment_preimages, logger)
855+
}
856+
789857
// Utilities to build transactions:
790858

791859
fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
@@ -1207,6 +1275,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
12071275
// We have to put the payment_preimage in the channel_monitor right away here to ensure we
12081276
// can claim it even if the channel hits the chain before we see their next commitment.
12091277
self.latest_monitor_update_id += 1;
1278+
self.payment_preimages.insert(payment_hash_calc, payment_preimage_arg.clone());
12101279
let monitor_update = ChannelMonitorUpdate {
12111280
update_id: self.latest_monitor_update_id,
12121281
updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
@@ -2001,6 +2070,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
20012070
return Err((None, ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), local_commitment_tx.1))));
20022071
}
20032072

2073+
// A LocalRemoved HTLC need to be monitored for expiration until we receive a
2074+
// broadcastable commitment tx without said HTLC. Now that we've confirmed that
2075+
// this commitment signed message provides said commitment tx, we can drop the
2076+
// LocalRemoved HTLCs we were previously watching for.
2077+
self.pending_drops.clear();
2078+
20042079
// TODO: Merge these two, sadly they are currently both required to be passed separately to
20052080
// ChannelMonitor:
20062081
let mut htlcs_without_source = Vec::with_capacity(local_commitment_tx.2.len());
@@ -2309,17 +2384,34 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
23092384
// Take references explicitly so that we can hold multiple references to self.
23102385
let pending_inbound_htlcs: &mut Vec<_> = &mut self.pending_inbound_htlcs;
23112386
let pending_outbound_htlcs: &mut Vec<_> = &mut self.pending_outbound_htlcs;
2312-
2313-
// We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
2314-
pending_inbound_htlcs.retain(|htlc| {
2315-
if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
2316-
log_trace!(logger, " ...removing inbound LocalRemoved {}", log_bytes!(htlc.payment_hash.0));
2317-
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
2318-
value_to_self_msat_diff += htlc.amount_msat as i64;
2387+
let pending_drops: &mut Vec<_> = &mut self.pending_drops;
2388+
2389+
// LocalRemoved HTLCs are saved in pending_drops so we can properly
2390+
// calculate whether to broadcast a commitment transaction due to an
2391+
// expiring HTLC or whether the ChannelMonitor will take care of it for
2392+
// us.
2393+
let mut inbounds = Vec::new();
2394+
for htlc in pending_inbound_htlcs.drain(..) {
2395+
match htlc.state {
2396+
InboundHTLCState::LocalRemoved(_) => {
2397+
log_trace!(logger, " ...removing inbound LocalRemoved {}", log_bytes!(htlc.payment_hash.0));
2398+
pending_drops.push(htlc);
2399+
},
2400+
_ => inbounds.push(htlc),
2401+
}
2402+
}
2403+
for htlc in pending_drops.iter() {
2404+
match htlc.state {
2405+
InboundHTLCState::LocalRemoved(ref reason) => {
2406+
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
2407+
value_to_self_msat_diff += htlc.amount_msat as i64;
2408+
}
23192409
}
2320-
false
2321-
} else { true }
2322-
});
2410+
_ => unreachable!(),
2411+
};
2412+
}
2413+
mem::swap(pending_inbound_htlcs, &mut inbounds);
2414+
// We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
23232415
pending_outbound_htlcs.retain(|htlc| {
23242416
if let &OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) = &htlc.state {
23252417
log_trace!(logger, " ...removing outbound AwaitingRemovedRemoteRevoke {}", log_bytes!(htlc.payment_hash.0));
@@ -3098,14 +3190,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
30983190
self.user_id
30993191
}
31003192

3101-
/// May only be called after funding has been initiated (ie is_funding_initiated() is true)
3102-
pub fn channel_monitor(&mut self) -> &mut ChannelMonitor<ChanSigner> {
3103-
if self.channel_state < ChannelState::FundingSent as u32 {
3104-
panic!("Can't get a channel monitor until funding has been created");
3105-
}
3106-
self.channel_monitor.as_mut().unwrap()
3107-
}
3108-
31093193
/// Guaranteed to be Some after both FundingLocked messages have been exchanged (and, thus,
31103194
/// is_usable() returns true).
31113195
/// Allowed in any state (including after shutdown)
@@ -4101,6 +4185,21 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
41014185
}
41024186
}
41034187

4188+
(self.pending_drops.len() as u64).write(writer)?;
4189+
for htlc in self.pending_drops.iter() {
4190+
htlc.htlc_id.write(writer)?;
4191+
htlc.amount_msat.write(writer)?;
4192+
htlc.cltv_expiry.write(writer)?;
4193+
htlc.payment_hash.write(writer)?;
4194+
match &htlc.state {
4195+
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
4196+
4u8.write(writer)?;
4197+
removal_reason.write(writer)?;
4198+
},
4199+
_ => unreachable!(),
4200+
}
4201+
}
4202+
41044203
(self.pending_outbound_htlcs.len() as u64).write(writer)?;
41054204
for htlc in self.pending_outbound_htlcs.iter() {
41064205
htlc.htlc_id.write(writer)?;
@@ -4131,6 +4230,11 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
41314230
}
41324231
}
41334232

4233+
writer.write_all(&byte_utils::be64_to_array(self.payment_preimages.len() as u64))?;
4234+
for payment_preimage in self.payment_preimages.values() {
4235+
writer.write_all(&payment_preimage.0[..])?;
4236+
}
4237+
41344238
(self.holding_cell_htlc_updates.len() as u64).write(writer)?;
41354239
for update in self.holding_cell_htlc_updates.iter() {
41364240
match update {
@@ -4228,6 +4332,8 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
42284332
}
42294333
}
42304334

4335+
const MAX_ALLOC_SIZE: usize = 64*1024;
4336+
42314337
impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
42324338
fn read<R : ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
42334339
let _ver: u8 = Readable::read(reader)?;
@@ -4272,6 +4378,21 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
42724378
});
42734379
}
42744380

4381+
let pending_drops_count: u64 = Readable::read(reader)?;
4382+
let mut pending_drops = Vec::with_capacity(cmp::min(pending_drops_count as usize, OUR_MAX_HTLCS as usize));
4383+
for _ in 0..pending_drops_count {
4384+
pending_drops.push(InboundHTLCOutput {
4385+
htlc_id: Readable::read(reader)?,
4386+
amount_msat: Readable::read(reader)?,
4387+
cltv_expiry: Readable::read(reader)?,
4388+
payment_hash: Readable::read(reader)?,
4389+
state: match <u8 as Readable>::read(reader)? {
4390+
4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
4391+
_ => return Err(DecodeError::InvalidValue),
4392+
},
4393+
});
4394+
}
4395+
42754396
let pending_outbound_htlc_count: u64 = Readable::read(reader)?;
42764397
let mut pending_outbound_htlcs = Vec::with_capacity(cmp::min(pending_outbound_htlc_count as usize, OUR_MAX_HTLCS as usize));
42774398
for _ in 0..pending_outbound_htlc_count {
@@ -4292,6 +4413,16 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
42924413
});
42934414
}
42944415

4416+
let payment_preimages_len: u64 = Readable::read(reader)?;
4417+
let mut payment_preimages = HashMap::with_capacity(cmp::min(payment_preimages_len as usize, MAX_ALLOC_SIZE / 32));
4418+
for _ in 0..payment_preimages_len {
4419+
let preimage: PaymentPreimage = Readable::read(reader)?;
4420+
let hash = PaymentHash(Sha256::hash(&preimage.0[..]).into_inner());
4421+
if let Some(_) = payment_preimages.insert(hash, preimage) {
4422+
return Err(DecodeError::InvalidValue);
4423+
}
4424+
}
4425+
42954426
let holding_cell_htlc_update_count: u64 = Readable::read(reader)?;
42964427
let mut holding_cell_htlc_updates = Vec::with_capacity(cmp::min(holding_cell_htlc_update_count as usize, OUR_MAX_HTLCS as usize*2));
42974428
for _ in 0..holding_cell_htlc_update_count {
@@ -4408,6 +4539,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
44084539
pending_inbound_htlcs,
44094540
pending_outbound_htlcs,
44104541
holding_cell_htlc_updates,
4542+
payment_preimages,
4543+
pending_drops,
44114544

44124545
resend_order,
44134546

lightning/src/ln/channelmanager.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3044,7 +3044,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
30443044
}
30453045
}
30463046
}
3047-
if channel.is_funding_initiated() && channel.channel_monitor().would_broadcast_at_height(height, &self.logger) {
3047+
if channel.is_funding_initiated() && channel.monitor_would_broadcast_at_height(height, &self.logger) {
30483048
if let Some(short_id) = channel.get_short_channel_id() {
30493049
short_to_id.remove(&short_id);
30503050
}

lightning/src/ln/channelmonitor.rs

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,7 +1986,26 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
19861986
self.last_block_hash = block_hash.clone();
19871987
}
19881988

1989-
pub(super) fn would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {
1989+
fn would_broadcast_at_height<L: Deref>(&self, height: u32, logger: &L) -> bool where L::Target: Logger {
1990+
let local_outputs: Vec<&HTLCOutputInCommitment> = self.current_local_commitment_tx.htlc_outputs
1991+
.iter().map(|&(ref a, _, _)| a).collect();
1992+
let mut prev_remote_outputs = Vec::new();
1993+
if let Some(ref txid) = self.prev_remote_commitment_txid {
1994+
if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
1995+
prev_remote_outputs = htlc_outputs.iter().map(|&(ref a, _)| a).collect();
1996+
}
1997+
}
1998+
let mut curr_remote_outputs = Vec::new();
1999+
if let Some(ref txid) = self.current_remote_commitment_txid {
2000+
if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
2001+
curr_remote_outputs = htlc_outputs.iter().map(|&(ref a, _)| a).collect()
2002+
}
2003+
}
2004+
let remote_outputs = [curr_remote_outputs, prev_remote_outputs].concat();
2005+
ChannelMonitor::<ChanSigner>::would_broadcast_at_height_given_htlcs(local_outputs, remote_outputs, height, &self.payment_preimages, logger)
2006+
}
2007+
2008+
pub(super) fn would_broadcast_at_height_given_htlcs<L: Deref>(local_htlc_outputs: Vec<&HTLCOutputInCommitment>, remote_htlc_outputs: Vec<&HTLCOutputInCommitment>, height: u32, preimages: &HashMap<PaymentHash, PaymentPreimage>, logger: &L) -> bool where L::Target: Logger {
19902009
// We need to consider all HTLCs which are:
19912010
// * in any unrevoked remote commitment transaction, as they could broadcast said
19922011
// transactions and we'd end up in a race, or
@@ -2025,26 +2044,16 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
20252044
// with CHECK_CLTV_EXPIRY_SANITY_2.
20262045
let htlc_outbound = $local_tx == htlc.offered;
20272046
if ( htlc_outbound && htlc.cltv_expiry + LATENCY_GRACE_PERIOD_BLOCKS <= height) ||
2028-
(!htlc_outbound && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && self.payment_preimages.contains_key(&htlc.payment_hash)) {
2047+
(!htlc_outbound && htlc.cltv_expiry <= height + CLTV_CLAIM_BUFFER && preimages.contains_key(&htlc.payment_hash)) {
20292048
log_info!(logger, "Force-closing channel due to {} HTLC timeout, HTLC expiry is {}", if htlc_outbound { "outbound" } else { "inbound "}, htlc.cltv_expiry);
20302049
return true;
20312050
}
20322051
}
20332052
}
20342053
}
20352054

2036-
scan_commitment!(self.current_local_commitment_tx.htlc_outputs.iter().map(|&(ref a, _, _)| a), true);
2037-
2038-
if let Some(ref txid) = self.current_remote_commitment_txid {
2039-
if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
2040-
scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false);
2041-
}
2042-
}
2043-
if let Some(ref txid) = self.prev_remote_commitment_txid {
2044-
if let Some(ref htlc_outputs) = self.remote_claimable_outpoints.get(txid) {
2045-
scan_commitment!(htlc_outputs.iter().map(|&(ref a, _)| a), false);
2046-
}
2047-
}
2055+
scan_commitment!(local_htlc_outputs, true);
2056+
scan_commitment!(remote_htlc_outputs, false);
20482057

20492058
false
20502059
}

0 commit comments

Comments
 (0)