Skip to content

Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into from
Apr 24, 2023
Merged

Don't remove nodes if there's no channel_update for a temp failure #2220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dab242
Fix unused `Secp256k1` context in monitor_tests
TheBlueMatt Apr 23, 2023
3dcd490
Remove unused import in functional_test_utils
TheBlueMatt Apr 24, 2023
67ad6c4
Don't remove nodes if there's no channel_update for a temp failure
TheBlueMatt Apr 23, 2023
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/router.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
},
4 => {
let short_channel_id = slice_to_be64(get_slice!(8));
net_graph.channel_failed(short_channel_id, false);
net_graph.channel_failed_permanent(short_channel_id);
},
_ if node_pks.is_empty() => {},
_ => {
Expand Down
1 change: 0 additions & 1 deletion lightning/src/ln/functional_test_utils.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/monitor_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
if anchors {
assert!(cfg!(anchors));
}
let secp = Secp256k1::new();
let mut chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
}) = events.pop().unwrap() {
let secp = Secp256k1::new();
assert_eq!(htlc_descriptors.len(), 1);
let descriptor = htlc_descriptors.pop().unwrap();
assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down
51 changes: 36 additions & 15 deletions lightning/src/ln/onion_utils.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
} else {
log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
}
if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
if update_opt.is_ok() || update_slice.is_empty() {
// if channel_update should NOT have caused the failure:
// MAY treat the channel_update as invalid.
let is_chan_update_invalid = match error_code & 0xff {
7 => false,
11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
12 => amt_to_forward
.checked_mul(chan_update.contents.fee_proportional_millionths as u64)
11 => update_opt.is_ok() &&
amt_to_forward >
update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
12 => update_opt.is_ok() && amt_to_forward
.checked_mul(update_opt.as_ref().unwrap()
.contents.fee_proportional_millionths as u64)
.map(|prop_fee| prop_fee / 1_000_000)
.and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
.and_then(|prop_fee| prop_fee.checked_add(
update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
.map(|fee_msats| route_hop.fee_msat >= fee_msats)
.unwrap_or(false),
13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
13 => update_opt.is_ok() &&
route_hop.cltv_expiry_delta as u16 >=
update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
14 => false, // expiry_too_soon; always valid?
20 => chan_update.contents.flags & 2 == 0,
20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
_ => false, // unknown error code; take channel_update as valid
};
if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
is_permanent: true,
});
} else {
// Make sure the ChannelUpdate contains the expected
// short channel id.
if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
short_channel_id = Some(failing_route_hop.short_channel_id);
if let Ok(chan_update) = update_opt {
// Make sure the ChannelUpdate contains the expected
// short channel id.
if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
short_channel_id = Some(failing_route_hop.short_channel_id);
} else {
log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
}
network_update = Some(NetworkUpdate::ChannelUpdateMessage {
msg: chan_update,
})
} else {
log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
network_update = Some(NetworkUpdate::ChannelFailure {
short_channel_id: route_hop.short_channel_id,
is_permanent: false,
});
}
network_update = Some(NetworkUpdate::ChannelUpdateMessage {
msg: chan_update,
})
};
} else {
// If the channel_update had a non-zero length (i.e. was
// present) but we couldn't read it, treat it as a total
// node failure.
log_info!(logger,
"Failed to read a channel_update of len {} in an onion",
update_slice.len());
}
}
}
Expand Down
56 changes: 22 additions & 34 deletions lightning/src/routing/gossip.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
msg: ChannelUpdate,
},
/// An error indicating that a channel failed to route a payment, which should be applied via
/// [`NetworkGraph::channel_failed`].
/// [`NetworkGraph::channel_failed_permanent`] if permanent.
ChannelFailure {
/// The short channel id of the closed channel.
short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
let _ = self.update_channel(msg);
},
NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
let action = if is_permanent { "Removing" } else { "Disabling" };
log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
self.channel_failed(short_channel_id, is_permanent);
if is_permanent {
log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
self.channel_failed_permanent(short_channel_id);
}
},
NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
Ok(())
}

/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
/// If permanent, removes a channel from the local storage.
/// May cause the removal of nodes too, if this was their last channel.
/// If not permanent, makes channels unavailable for routing.
pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
/// Marks a channel in the graph as failed permanently.
///
/// The channel and any node for which this was their last channel are removed from the graph.
pub fn channel_failed_permanent(&self, short_channel_id: u64) {
#[cfg(feature = "std")]
let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
#[cfg(not(feature = "std"))]
let current_time_unix = None;

self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
}

/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
/// If permanent, removes a channel from the local storage.
/// May cause the removal of nodes too, if this was their last channel.
/// If not permanent, makes channels unavailable for routing.
fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
/// Marks a channel in the graph as failed permanently.
///
/// The channel and any node for which this was their last channel are removed from the graph.
fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
let mut channels = self.channels.write().unwrap();
if is_permanent {
if let Some(chan) = channels.remove(&short_channel_id) {
let mut nodes = self.nodes.write().unwrap();
self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
}
} else {
if let Some(chan) = channels.get_mut(&short_channel_id) {
if let Some(one_to_two) = chan.one_to_two.as_mut() {
one_to_two.enabled = false;
}
if let Some(two_to_one) = chan.two_to_one.as_mut() {
two_to_one.enabled = false;
}
}
if let Some(chan) = channels.remove(&short_channel_id) {
let mut nodes = self.nodes.write().unwrap();
self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
}
}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
}

// Non-permanent closing just disables a channel
// Non-permanent failure doesn't touch the channel at all
{
match network_graph.read_only().channels().get(&short_channel_id) {
None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
match network_graph.read_only().channels().get(&short_channel_id) {
None => panic!(),
Some(channel_info) => {
assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
}
};
}
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

// Mark the channel as permanently failed. This will also remove the two nodes
// and all of the entries will be tracked as removed.
network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

// Should not remove from tracking if insufficient time has passed
network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

// Mark the channel as permanently failed. This will also remove the two nodes
// and all of the entries will be tracked as removed.
network_graph.channel_failed(short_channel_id, true);
network_graph.channel_failed_permanent(short_channel_id);

// The first time we call the following, the channel will have a removal time assigned.
network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down
, '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" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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('^' + ".*" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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('^' + ".*" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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('^' + ".*" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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('^' + ".*" + '
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down

, '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); }
})();
})();
Don't remove nodes if there's no channel_update for a temp failure by TheBlueMatt · Pull Request #2220 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't remove nodes if there's no channel_update for a temp failure#2220

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 3 commits into

from

Apr 24, 2023
 Merged


Don't remove nodes if there's no channel_update for a temp failure
#2220
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


3 commits


Select commit
Hold shift + click to select a range
3dab242

Fix unused `Secp256k1` context in monitor_tests


TheBlueMatt Apr 23, 2023
3dcd490

Remove unused import in functional_test_utils


TheBlueMatt Apr 24, 2023
67ad6c4

Don't remove nodes if there's no channel_update for a temp failure


TheBlueMatt Apr 23, 2023

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/router.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -227,7 +227,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 },
 4 => {
 let short_channel_id = slice_to_be64(get_slice!(8));
 net_graph.channel_failed(short_channel_id, false);
 net_graph.channel_failed_permanent(short_channel_id);
 },
 _ if node_pks.is_empty() => {},
 _ => {
Expand Down

1 change: 0 additions & 1 deletion
lightning/src/ln/functional_test_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,6 @@ use crate::util::config::UserConfig;
use crate::util::ser::{ReadableArgs, Writeable};

use bitcoin::blockdata::block::{Block, BlockHeader};
use bitcoin::blockdata::constants::genesis_block;
use bitcoin::blockdata::transaction::{Transaction, TxOut};
use bitcoin::network::constants::Network;

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/monitor_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1784,7 +1784,6 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
	if anchors {
 assert!(cfg!(anchors));
	}
	let secp = Secp256k1::new();
	let mut chanmon_cfgs = create_chanmon_cfgs(2);
	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
	let mut config = test_default_channel_config();
Expand DownExpand Up@@ -1838,6 +1837,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
 feerate = if let Event::BumpTransaction(BumpTransactionEvent::HTLCResolution {
 target_feerate_sat_per_1000_weight, mut htlc_descriptors, tx_lock_time,
 }) = events.pop().unwrap() {
 let secp = Secp256k1::new();
 assert_eq!(htlc_descriptors.len(), 1);
 let descriptor = htlc_descriptors.pop().unwrap();
 assert_eq!(descriptor.commitment_txid, commitment_txn[0].txid());
Expand Down

51 changes: 36 additions & 15 deletions
lightning/src/ln/onion_utils.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -493,21 +493,28 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 } else {
 log_trace!(logger, "Failure provided features a channel update without type prefix. Deprecated, but allowing for now.");
 }
 if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) {
 let update_opt = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice));
 if update_opt.is_ok() || update_slice.is_empty() {
 // if channel_update should NOT have caused the failure:
 // MAY treat the channel_update as invalid.
 let is_chan_update_invalid = match error_code & 0xff {
 7 => false,
 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat,
 12 => amt_to_forward
 .checked_mul(chan_update.contents.fee_proportional_millionths as u64)
 11 => update_opt.is_ok() &&
 amt_to_forward >
 update_opt.as_ref().unwrap().contents.htlc_minimum_msat,
 12 => update_opt.is_ok() && amt_to_forward
 .checked_mul(update_opt.as_ref().unwrap()
 .contents.fee_proportional_millionths as u64)
 .map(|prop_fee| prop_fee / 1_000_000)
 .and_then(|prop_fee| prop_fee.checked_add(chan_update.contents.fee_base_msat as u64))
 .and_then(|prop_fee| prop_fee.checked_add(
 update_opt.as_ref().unwrap().contents.fee_base_msat as u64))
 .map(|fee_msats| route_hop.fee_msat >= fee_msats)
 .unwrap_or(false),
 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta,
 13 => update_opt.is_ok() &&
 route_hop.cltv_expiry_delta as u16 >=
 update_opt.as_ref().unwrap().contents.cltv_expiry_delta,
 14 => false, // expiry_too_soon; always valid?
 20 => chan_update.contents.flags & 2 == 0,
 20 => update_opt.as_ref().unwrap().contents.flags & 2 == 0,
 _ => false, // unknown error code; take channel_update as valid
 };
 if is_chan_update_invalid {
Expand All@@ -518,17 +525,31 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
 is_permanent: true,
 });
 } else {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 if let Ok(chan_update) = update_opt {
 // Make sure the ChannelUpdate contains the expected
 // short channel id.
 if failing_route_hop.short_channel_id == chan_update.contents.short_channel_id {
 short_channel_id = Some(failing_route_hop.short_channel_id);
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 } else {
 log_info!(logger, "Node provided a channel_update for which it was not authoritative, ignoring.");
 network_update = Some(NetworkUpdate::ChannelFailure {
 short_channel_id: route_hop.short_channel_id,
 is_permanent: false,
 });
 }
 network_update = Some(NetworkUpdate::ChannelUpdateMessage {
 msg: chan_update,
 })
 };
 } else {
 // If the channel_update had a non-zero length (i.e. was
 // present) but we couldn't read it, treat it as a total
 // node failure.
 log_info!(logger,
 "Failed to read a channel_update of len {} in an onion",
 update_slice.len());
 }
 }
 }
Expand Down

56 changes: 22 additions & 34 deletions
lightning/src/routing/gossip.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,7 @@ pub enum NetworkUpdate {
 msg: ChannelUpdate,
	},
	/// An error indicating that a channel failed to route a payment, which should be applied via
	/// [`NetworkGraph::channel_failed`].
	/// [`NetworkGraph::channel_failed_permanent`] if permanent.
	ChannelFailure {
 /// The short channel id of the closed channel.
 short_channel_id: u64,
Expand DownExpand Up@@ -352,9 +352,10 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 let _ = self.update_channel(msg);
 },
 NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
 let action = if is_permanent { "Removing" } else { "Disabling" };
 log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
 self.channel_failed(short_channel_id, is_permanent);
 if is_permanent {
 log_debug!(self.logger, "Removing channel graph entry for {} due to a payment failure.", short_channel_id);
 self.channel_failed_permanent(short_channel_id);
 }
 },
 NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 if is_permanent {
Expand DownExpand Up@@ -1632,40 +1633,27 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 Ok(())
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	pub fn channel_failed_permanent(&self, short_channel_id: u64) {
 #[cfg(feature = "std")]
 let current_time_unix = Some(SystemTime::now().duration_since(UNIX_EPOCH).expect("Time must be > 1970").as_secs());
 #[cfg(not(feature = "std"))]
 let current_time_unix = None;

 self.channel_failed_with_time(short_channel_id, is_permanent, current_time_unix)
 self.channel_failed_permanent_with_time(short_channel_id, current_time_unix)
	}

	/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
	/// If permanent, removes a channel from the local storage.
	/// May cause the removal of nodes too, if this was their last channel.
	/// If not permanent, makes channels unavailable for routing.
	fn channel_failed_with_time(&self, short_channel_id: u64, is_permanent: bool, current_time_unix: Option<u64>) {
	/// Marks a channel in the graph as failed permanently.
	///
	/// The channel and any node for which this was their last channel are removed from the graph.
	fn channel_failed_permanent_with_time(&self, short_channel_id: u64, current_time_unix: Option<u64>) {
 let mut channels = self.channels.write().unwrap();
 if is_permanent {
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
 } else {
 if let Some(chan) = channels.get_mut(&short_channel_id) {
 if let Some(one_to_two) = chan.one_to_two.as_mut() {
 one_to_two.enabled = false;
 }
 if let Some(two_to_one) = chan.two_to_one.as_mut() {
 two_to_one.enabled = false;
 }
 }
 if let Some(chan) = channels.remove(&short_channel_id) {
 let mut nodes = self.nodes.write().unwrap();
 self.removed_channels.lock().unwrap().insert(short_channel_id, current_time_unix);
 Self::remove_channel_in_nodes(&mut nodes, &chan, short_channel_id);
 }
	}

Expand DownExpand Up@@ -2450,7 +2438,7 @@ pub(crate) mod tests {
 assert!(network_graph.read_only().channels().get(&short_channel_id).unwrap().one_to_two.is_some());
 }

 // Non-permanent closing just disables a channel
 // Non-permanent failure doesn't touch the channel at all
 {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
Expand All@@ -2467,7 +2455,7 @@ pub(crate) mod tests {
 match network_graph.read_only().channels().get(&short_channel_id) {
 None => panic!(),
 Some(channel_info) => {
 assert!(!channel_info.one_to_two.as_ref().unwrap().enabled);
 assert!(channel_info.one_to_two.as_ref().unwrap().enabled);
 }
 };
 }
Expand DownExpand Up@@ -2601,7 +2589,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed_with_time(short_channel_id, true, Some(tracking_time));
 network_graph.channel_failed_permanent_with_time(short_channel_id, Some(tracking_time));

 // Should not remove from tracking if insufficient time has passed
 network_graph.remove_stale_channels_and_tracking_with_time(
Expand DownExpand Up@@ -2634,7 +2622,7 @@ pub(crate) mod tests {

 // Mark the channel as permanently failed. This will also remove the two nodes
 // and all of the entries will be tracked as removed.
 network_graph.channel_failed(short_channel_id, true);
 network_graph.channel_failed_permanent(short_channel_id);

 // The first time we call the following, the channel will have a removal time assigned.
 network_graph.remove_stale_channels_and_tracking_with_time(removal_time);
Expand Down