Skip to content

Commit f0bcb7d

Browse files
TheBlueMattTheBlueMatt
committed
log_trace HTLC transitions in RAA handling (best reviewed with -b)
1 parent b2252fb commit f0bcb7d

File tree

3 files changed

+77
-60
lines changed

3 files changed

+77
-60
lines changed

‎src/ln/channel.rs

Lines changed: 73 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use chain::transaction::OutPoint;
2525
use chain::keysinterface::{ChannelKeys, KeysInterface};
2626
use util::transaction_utils;
2727
use util::ser::{Readable, ReadableArgs, Writeable, Writer, WriterWriteAdaptor};
28-
use util::logger::Logger;
28+
use util::logger::{Logger, LogHolder};
2929
use util::errors::APIError;
3030
use util::config::{UserConfig,ChannelConfig};
3131

@@ -1992,74 +1992,89 @@ impl Channel {
19921992
self.monitor_pending_order = None;
19931993
}
19941994

1995+
log_trace!(self, "Updating HTLCs on receipt of RAA...");
19951996
let mut to_forward_infos = Vec::new();
19961997
let mut revoked_htlcs = Vec::new();
19971998
let mut update_fail_htlcs = Vec::new();
19981999
let mut update_fail_malformed_htlcs = Vec::new();
19992000
let mut require_commitment = false;
20002001
let mut value_to_self_msat_diff: i64 = 0;
2001-
// We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
2002-
self.pending_inbound_htlcs.retain(|htlc| {
2003-
if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
2004-
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
2005-
value_to_self_msat_diff += htlc.amount_msat as i64;
2006-
}
2007-
false
2008-
} else { true }
2009-
});
2010-
self.pending_outbound_htlcs.retain(|htlc| {
2011-
if let OutboundHTLCState::AwaitingRemovedRemoteRevoke = htlc.state {
2012-
if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
2013-
revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
2014-
} else {
2015-
// They fulfilled, so we sent them money
2016-
value_to_self_msat_diff -= htlc.amount_msat as i64;
2017-
}
2018-
false
2019-
} else { true }
2020-
});
2021-
for htlc in self.pending_inbound_htlcs.iter_mut() {
2022-
let swap = if let &InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) = &htlc.state {
2023-
true
2024-
} else if let &InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) = &htlc.state {
2025-
true
2026-
} else { false };
2027-
if swap {
2028-
let mut state = InboundHTLCState::Committed;
2029-
mem::swap(&mut state, &mut htlc.state);
2030-
2031-
if let InboundHTLCState::AwaitingRemoteRevokeToAnnounce(forward_info) = state {
2032-
htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info);
2033-
require_commitment = true;
2034-
} else if let InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info) = state {
2035-
match forward_info {
2036-
PendingHTLCStatus::Fail(fail_msg) => {
2037-
require_commitment = true;
2038-
match fail_msg {
2039-
HTLCFailureMsg::Relay(msg) => {
2040-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
2041-
update_fail_htlcs.push(msg)
2042-
},
2043-
HTLCFailureMsg::Malformed(msg) => {
2044-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed((msg.sha256_of_onion, msg.failure_code)));
2045-
update_fail_malformed_htlcs.push(msg)
2046-
},
2002+
2003+
{
2004+
// Take references explicitly so that we can hold multiple references to self.
2005+
let pending_inbound_htlcs: &mut Vec<_> = &mut self.pending_inbound_htlcs;
2006+
let pending_outbound_htlcs: &mut Vec<_> = &mut self.pending_outbound_htlcs;
2007+
let logger = LogHolder { logger: &self.logger };
2008+
2009+
// We really shouldnt have two passes here, but retain gives a non-mutable ref (Rust bug)
2010+
pending_inbound_htlcs.retain(|htlc| {
2011+
if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
2012+
log_trace!(logger, " ...removing inbound LocalRemoved {}", log_bytes!(htlc.payment_hash.0));
2013+
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
2014+
value_to_self_msat_diff += htlc.amount_msat as i64;
2015+
}
2016+
false
2017+
} else { true }
2018+
});
2019+
pending_outbound_htlcs.retain(|htlc| {
2020+
if let OutboundHTLCState::AwaitingRemovedRemoteRevoke = htlc.state {
2021+
log_trace!(logger, " ...removing outbound AwaitingRemovedRemoteRevoke {}", log_bytes!(htlc.payment_hash.0));
2022+
if let Some(reason) = htlc.fail_reason.clone() { // We really want take() here, but, again, non-mut ref :(
2023+
revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
2024+
} else {
2025+
// They fulfilled, so we sent them money
2026+
value_to_self_msat_diff -= htlc.amount_msat as i64;
2027+
}
2028+
false
2029+
} else { true }
2030+
});
2031+
for htlc in pending_inbound_htlcs.iter_mut() {
2032+
let swap = if let &InboundHTLCState::AwaitingRemoteRevokeToAnnounce(_) = &htlc.state {
2033+
log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce {} to Committed", log_bytes!(htlc.payment_hash.0));
2034+
true
2035+
} else if let &InboundHTLCState::AwaitingAnnouncedRemoteRevoke(_) = &htlc.state {
2036+
log_trace!(logger, " ...promoting inbound AwaitingAnnouncedRemoteRevoke {} to Committed", log_bytes!(htlc.payment_hash.0));
2037+
true
2038+
} else { false };
2039+
if swap {
2040+
let mut state = InboundHTLCState::Committed;
2041+
mem::swap(&mut state, &mut htlc.state);
2042+
2043+
if let InboundHTLCState::AwaitingRemoteRevokeToAnnounce(forward_info) = state {
2044+
htlc.state = InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info);
2045+
require_commitment = true;
2046+
} else if let InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info) = state {
2047+
match forward_info {
2048+
PendingHTLCStatus::Fail(fail_msg) => {
2049+
require_commitment = true;
2050+
match fail_msg {
2051+
HTLCFailureMsg::Relay(msg) => {
2052+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
2053+
update_fail_htlcs.push(msg)
2054+
},
2055+
HTLCFailureMsg::Malformed(msg) => {
2056+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed((msg.sha256_of_onion, msg.failure_code)));
2057+
update_fail_malformed_htlcs.push(msg)
2058+
},
2059+
}
2060+
},
2061+
PendingHTLCStatus::Forward(forward_info) => {
2062+
to_forward_infos.push((forward_info, htlc.htlc_id));
2063+
htlc.state = InboundHTLCState::Committed;
20472064
}
2048-
},
2049-
PendingHTLCStatus::Forward(forward_info) => {
2050-
to_forward_infos.push((forward_info, htlc.htlc_id));
2051-
htlc.state = InboundHTLCState::Committed;
20522065
}
20532066
}
20542067
}
20552068
}
2056-
}
2057-
for htlc in self.pending_outbound_htlcs.iter_mut() {
2058-
if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
2059-
htlc.state = OutboundHTLCState::Committed;
2060-
} else if let OutboundHTLCState::AwaitingRemoteRevokeToRemove = htlc.state {
2061-
htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
2062-
require_commitment = true;
2069+
for htlc in pending_outbound_htlcs.iter_mut() {
2070+
if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
2071+
log_trace!(logger, " ...promoting outbound LocalAnnounced {} to Committed", log_bytes!(htlc.payment_hash.0));
2072+
htlc.state = OutboundHTLCState::Committed;
2073+
} else if let OutboundHTLCState::AwaitingRemoteRevokeToRemove = htlc.state {
2074+
log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
2075+
htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke;
2076+
require_commitment = true;
2077+
}
20632078
}
20642079
}
20652080
self.value_to_self_msat = (self.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;

‎src/ln/onion_utils.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use util::{byte_utils, internal_traits};
55
use util::chacha20::ChaCha20;
66
use util::errors::{self, APIError};
77
use util::ser::{Readable, Writeable};
8-
use util::logger::Logger;
8+
use util::logger::{Logger, LogHolder};
99

1010
use bitcoin_hashes::{Hash, HashEngine};
1111
use bitcoin_hashes::cmp::fixed_time_eq;
@@ -265,7 +265,6 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type:
265265
encrypt_failure_packet(shared_secret, &failure_packet.encode()[..])
266266
}
267267

268-
struct LogHolder<'a> { logger: &'a Arc<Logger> }
269268
/// Process failure we got back from upstream on a payment we sent (implying htlc_source is an
270269
/// OutboundRoute).
271270
/// Returns update, a boolean indicating that the payment itself failed, and the error code.

‎src/util/logger.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
1717
use std::cmp;
1818
use std::fmt;
19+
use std::sync::Arc;
1920

2021
static LOG_LEVEL_NAMES: [&'static str; 6] = ["OFF", "ERROR", "WARN", "INFO", "DEBUG", "TRACE"];
2122

@@ -127,6 +128,8 @@ pub trait Logger: Sync + Send {
127128
fn log(&self, record: &Record);
128129
}
129130

131+
pub(crate) struct LogHolder<'a> { pub(crate) logger: &'a Arc<Logger> }
132+
130133
#[cfg(test)]
131134
mod tests {
132135
use util::logger::{Logger, Level};

0 commit comments

Comments
 (0)