Skip to content

Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheBlueMatt merged 5 commits into from
Mar 20, 2020
Merged

Generate latest local commitment transactions via monitor avoiding Channel's copy #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
32d62ad
Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs
TheBlueMatt Mar 19, 2020
82d40ee
Deduplicate HTLC preimage events from channelmonitor.
TheBlueMatt Mar 19, 2020
03b5da1
Broadcast final local txn via ChannelMonitorUpdate
TheBlueMatt Mar 18, 2020
e1c1ac7
Fetch latest local commitment txn via a macro in tests
TheBlueMatt Mar 19, 2020
feca83a
Fetch latest commitment txn via monitor, not channel in test
TheBlueMatt Mar 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fuzz/src/chanmon_consistency.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
};
let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
deserialized_monitor.update_monitor(update.clone()).unwrap();
deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
let mut ser = VecWriter(Vec::new());
deserialized_monitor.write_for_disk(&mut ser).unwrap();
map_entry.insert((update.update_id, ser.0));
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/chanmon_update_fail_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
check_added_monitors!(nodes[0], 1);
check_added_monitors!(nodes[0], 2);

let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

// ...and make sure we can force-close a frozen channel
nodes[0].node.force_close_channel(&channel_id);
check_added_monitors!(nodes[0], 0);
check_added_monitors!(nodes[0], 1);
check_closed_broadcast!(nodes[0], false);

// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down
35 changes: 24 additions & 11 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
/// Ok(_) if debug assertions are turned on and preconditions are met.
/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
/// return Ok(_) if debug assertions are turned on or preconditions are met.
///
/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
/// but then have a reorg which settles on an HTLC-failure on chain.
fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
// Either ChannelFunded got set (which means it won't be unset) or there is no way any
// caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
} else {
log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
}
debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
return Ok((None, None));
},
_ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
match pending_update {
&HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
if htlc_id_arg == htlc_id {
// Make sure we don't leave latest_monitor_update_id incremented here:
self.latest_monitor_update_id -= 1;

@ariardariardMar 19, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b

Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?

@TheBlueMattTheBlueMattMar 19, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.

debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
return Ok((None, None));
}
},
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
// TODO: We may actually be able to switch to a fulfill here, though its
// rare enough it may not be worth the complexity burden.
debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
return Ok((None, Some(monitor_update)));
}
},
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
/// Ok(_) if debug assertions are turned on and preconditions are met.
/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
/// return Ok(_) if debug assertions are turned on or preconditions are met.
///
/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
/// but then have a reorg which settles on an HTLC-failure on chain.
pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
match htlc.state {
InboundHTLCState::Committed => {},
InboundHTLCState::LocalRemoved(_) => {
debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
return Ok(None);
},
_ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
match pending_update {
&HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
if htlc_id_arg == htlc_id {
debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
}
},
&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
if htlc_id_arg == htlc_id {
debug_assert!(false, "Tried to fail an HTLC that was already failed");
return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
}
},
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
/// Also returns the list of payment_hashes for channels which we can safely fail backwards
/// immediately (others we will have to allow to time out).
pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

// We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

self.channel_state = ChannelState::ShutdownComplete as u32;
self.update_time_counter += 1;
if self.channel_monitor.is_some() {
(self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
} else {
// We aren't even signed funding yet, so can't broadcast anything
(Vec::new(), dropped_outbound_htlcs)
}
self.latest_monitor_update_id += 1;
(self.funding_txo.clone(), ChannelMonitorUpdate {
update_id: self.latest_monitor_update_id,
updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
}, dropped_outbound_htlcs)
}
}

Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Generate latest local commitment transactions via monitor avoiding Channel's copy by TheBlueMatt · Pull Request #551 · lightningdevkit/rust-lightning · GitHub
Skip to content
Generate latest local commitment transactions via monitor avoiding Channel's copy#551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

TheBlueMatt
merged 5 commits into

from

Mar 20, 2020
 Merged


Generate latest local commitment transactions via monitor avoiding Channel's copy
#551
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


5 commits


Select commit
Hold shift + click to select a range
32d62ad

Remove bogus mon_update_id += 1 fulfilling already-fulfilled HTLCs


TheBlueMatt Mar 19, 2020
82d40ee

Deduplicate HTLC preimage events from channelmonitor.


TheBlueMatt Mar 19, 2020
03b5da1

Broadcast final local txn via ChannelMonitorUpdate


TheBlueMatt Mar 18, 2020
e1c1ac7

Fetch latest local commitment txn via a macro in tests


TheBlueMatt Mar 19, 2020
feca83a

Fetch latest commitment txn via monitor, not channel in test


TheBlueMatt Mar 19, 2020

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


2 changes: 1 addition & 1 deletion
fuzz/src/chanmon_consistency.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -116,7 +116,7 @@ impl channelmonitor::ManyChannelMonitor<EnforcingChannelKeys> for TestChannelMon
 };
 let mut deserialized_monitor = <(Sha256d, channelmonitor::ChannelMonitor<EnforcingChannelKeys>)>::
 read(&mut Cursor::new(&map_entry.get().1), Arc::clone(&self.logger)).unwrap().1;
 deserialized_monitor.update_monitor(update.clone()).unwrap();
 deserialized_monitor.update_monitor(update.clone(), &&TestBroadcaster {}).unwrap();
 let mut ser = VecWriter(Vec::new());
 deserialized_monitor.write_for_disk(&mut ser).unwrap();
 map_entry.insert((update.update_id, ser.0));
Expand Down

4 changes: 2 additions & 2 deletions
lightning/src/ln/chanmon_update_fail_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,7 @@ fn test_simple_monitor_permanent_update_fail() {

	*nodes[0].chan_monitor.update_ret.lock().unwrap() = Err(ChannelMonitorUpdateErr::PermanentFailure);
	if let Err(APIError::ChannelUnavailable {..}) = nodes[0].node.send_payment(route, payment_hash_1) {} else { panic!(); }
	check_added_monitors!(nodes[0], 1);
	check_added_monitors!(nodes[0], 2);

	let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
	assert_eq!(events_1.len(), 2);
Expand DownExpand Up@@ -120,7 +120,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {

	// ...and make sure we can force-close a frozen channel
	nodes[0].node.force_close_channel(&channel_id);
	check_added_monitors!(nodes[0], 0);
	check_added_monitors!(nodes[0], 1);
	check_closed_broadcast!(nodes[0], false);

	// TODO: Once we hit the chain with the failure transaction we should check that we get a
Expand Down

35 changes: 24 additions & 11 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1138,8 +1138,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> {
 // Either ChannelFunded got set (which means it won't be unset) or there is no way any
 // caller thought we could have something claimed (cause we wouldn't have accepted in an
Expand DownExpand Up@@ -1167,6 +1170,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 } else {
 log_warn!(self, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
 }
 debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
 return Ok((None, None));
 },
 _ => {
Expand DownExpand Up@@ -1200,6 +1204,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 // Make sure we don't leave latest_monitor_update_id incremented here:
 self.latest_monitor_update_id -= 1;
@ariardariardMar 19, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c95f91b
Hitting this case would indicate a programming error right ? If so I think function description doesn't match anymore what we really do given we don't return IgnoreError anymore here. You should add a debug_assert too?
@TheBlueMattTheBlueMattMar 19, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. I fixed the underlying duplicate events, added the debug_assertions, and added a comment noting that it is possible to hit them in some reorg cases.
 debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
 return Ok((None, None));
 }
 },
Expand All@@ -1208,6 +1215,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_warn!(self, "Have preimage and want to fulfill HTLC with pending failure against channel {}", log_bytes!(self.channel_id()));
 // TODO: We may actually be able to switch to a fulfill here, though its
 // rare enough it may not be worth the complexity burden.
 debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
 return Ok((None, Some(monitor_update)));
 }
 },
Expand DownExpand Up@@ -1259,8 +1267,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	}

	/// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
	/// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
	/// Ok(_) if debug assertions are turned on and preconditions are met.
	/// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
	/// return Ok(_) if debug assertions are turned on or preconditions are met.
	///
	/// Note that it is still possible to hit these assertions in case we find a preimage on-chain
	/// but then have a reorg which settles on an HTLC-failure on chain.
	pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> {
 if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 panic!("Was asked to fail an HTLC when channel was not in an operational state");
Expand All@@ -1277,6 +1288,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match htlc.state {
 InboundHTLCState::Committed => {},
 InboundHTLCState::LocalRemoved(_) => {
 debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
 return Ok(None);
 },
 _ => {
Expand All@@ -1297,11 +1309,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 match pending_update {
 &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
 &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
 if htlc_id_arg == htlc_id {
 debug_assert!(false, "Tried to fail an HTLC that was already failed");
 return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID"));
 }
 },
Expand DownExpand Up@@ -3760,7 +3774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
	/// Also returns the list of payment_hashes for channels which we can safely fail backwards
	/// immediately (others we will have to allow to time out).
	pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
	pub fn force_shutdown(&mut self, should_broadcast: bool) -> (Option<OutPoint>, ChannelMonitorUpdate, Vec<(HTLCSource, PaymentHash)>) {
 assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

 // We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All@@ -3783,12 +3797,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 self.channel_state = ChannelState::ShutdownComplete as u32;
 self.update_time_counter += 1;
 if self.channel_monitor.is_some() {
 (self.channel_monitor.as_mut().unwrap().get_latest_local_commitment_txn(), dropped_outbound_htlcs)
 } else {
 // We aren't even signed funding yet, so can't broadcast anything
 (Vec::new(), dropped_outbound_htlcs)
 }
 self.latest_monitor_update_id += 1;
 (self.funding_txo.clone(), ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
 }, dropped_outbound_htlcs)
	}
}

Expand Down
Loading