From 68d27bc7f3b08c176ba8548d8d1c1a21c7f3ec4e Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 15:13:34 +0000 Subject: [PATCH 1/7] Set `holder_commitment_point` to `Available` on upgrade When we upgrade from LDK 0.0.123 or prior, we need to intialize `holder_commitment_point` with commitment point(s). In 1f7f3a366c9e62cff5a5025724b5b508255a89d7 we changed the point(s) which we fetch from both the current and next per-commitment-point (setting the value to `HolderCommitmentPoint::Available` on upgrade) to only fetching the current per-commitment-point (setting the value to `HolderCommitmentPoint::PendingNext` on upgrade). In `commitment_signed` handling, we expect the next per-commitment-point to be available (allowing us to `advance()` the `holder_commitment_point`), as it was included in the `revoke_and_ack` we most recently sent to our peer, so must've been available at that time. Sadly, these two interact negatively with each other - on upgrade, assuming the channel is at a steady state and there are no pending updates, we'll not make the next per-commitment-point available but if we receive a `commitment_signed` from our peer we'll assume it is. As a result, in debug mode, we'll hit an assertion failure, and in production mode we'll force-close the channel. Instead, here, we fix the upgrade logic to always upgrade directly to `HolderCommitmentPoint::Available`, making the next per-commitment-point available immediately. We also attempt to resolve the next per-commitment-point in `get_channel_reestablish`, allowing any channels which were upgraded to LDK 0.0.124 and are in this broken state to avoid the force-closure, as long as they don't receive a `commitment_signed` in the interim. --- lightning/src/ln/channel.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 641a61c3140..f21788e8bd2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7153,6 +7153,12 @@ impl Channel where pub fn get_channel_reestablish(&mut self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger { assert!(self.context.channel_state.is_peer_disconnected()); assert_ne!(self.context.cur_counterparty_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER); + // This is generally the first function which gets called on any given channel once we're + // up and running normally. Thus, we take this opportunity to attempt to resolve the + // `holder_commitment_point` to get any keys which we are currently missing. + self.context.holder_commitment_point.try_resolve_pending( + &self.context.holder_signer, &self.context.secp_ctx, logger, + ); // Prior to static_remotekey, my_current_per_commitment_point was critical to claiming // current to_remote balances. However, it no longer has any use, and thus is now simply // set to a dummy (but valid, as required by the spec) public key. @@ -9464,8 +9470,10 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch // TODO(async_signing): remove this expect with the Uninitialized variant let current = holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number, &secp_ctx) .expect("Must be able to derive the current commitment point upon channel restoration"); - HolderCommitmentPoint::PendingNext { - transaction_number: cur_holder_commitment_transaction_number, current, + let next = holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number - 1, &secp_ctx) + .expect("Must be able to derive the next commitment point upon channel restoration"); + HolderCommitmentPoint::Available { + transaction_number: cur_holder_commitment_transaction_number, current, next, } }, }; From acfc6d9364519706d7636ab95b908d120c27bd2a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 16:54:56 +0000 Subject: [PATCH 2/7] Don't bump the `next_node_counter` when using a removed counter If we manage to pull a `node_counter` from `removed_node_counters` for reuse, `add_channel_between_nodes` would `unwrap_or` with the `next_node_counter`-incremented value. This visually looks right, except `unwrap_or` is always called, causing us to always increment `next_node_counter` even if we don't use it. This will result in the `node_counter`s always growing any time we add a new node to our graph, leading to somewhat larger memory usage when routing and a debug assertion failure in `test_node_counter_consistency`. The fix is trivial, this is what `unwrap_or_else` is for. --- lightning/src/routing/gossip.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index f1bc73110bf..bb14adefe07 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -1898,7 +1898,9 @@ impl NetworkGraph where L::Target: Logger { IndexedMapEntry::Vacant(node_entry) => { let mut removed_node_counters = self.removed_node_counters.lock().unwrap(); **chan_info_node_counter = removed_node_counters.pop() - .unwrap_or(self.next_node_counter.fetch_add(1, Ordering::Relaxed) as u32); + .unwrap_or_else(|| { + self.next_node_counter.fetch_add(1, Ordering::Relaxed) as u32 + }); node_entry.insert(NodeInfo { channels: vec!(short_channel_id), announcement_info: None, From a675d48cd4bb2c697ffb3be3c9f6103dd5ded5f1 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Thu, 10 Oct 2024 17:16:18 +0200 Subject: [PATCH 3/7] Fix `synchronize_listeners` calling default implementation Previously, the `ChainListenerSet` `Listen` implementation wouldn't forward to the listeners `block_connected` implementation outside of tests. This would result in the default implementation of `Listen::block_connected` being used and the listeners implementation never being called. --- lightning-block-sync/src/init.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/lightning-block-sync/src/init.rs b/lightning-block-sync/src/init.rs index 623f5404229..ef7e4b5f917 100644 --- a/lightning-block-sync/src/init.rs +++ b/lightning-block-sync/src/init.rs @@ -235,8 +235,6 @@ impl<'a, L: chain::Listen + ?Sized> chain::Listen for DynamicChainListener<'a, L struct ChainListenerSet<'a, L: chain::Listen + ?Sized>(Vec<(u32, &'a L)>); impl<'a, L: chain::Listen + ?Sized> chain::Listen for ChainListenerSet<'a, L> { - // Needed to differentiate test expectations. - #[cfg(test)] fn block_connected(&self, block: &bitcoin::Block, height: u32) { for (starting_height, chain_listener) in self.0.iter() { if height > *starting_height { From e48074afc9e600367694551e20b60457b0ba56f0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 17:11:04 +0000 Subject: [PATCH 4/7] Don't interpret decayed data as we've failed to send tiny values When we're calculating the success probability for min-/max-bucket pairs and are looking at the 0th' min-bucket, we only look at the highest max-bucket to decide the success probability. We ignore max-buckets which have a value below `BUCKET_FIXED_POINT_ONE` to only consider values which aren't substantially decayed. However, if all of our data is substantially decayed, this filter causes us to conclude that the highest max-bucket is bucket zero even though we really should then be looking at any bucket. We make this change here, looking at the highest non-zero max-bucket if no max-buckets have a value above `BUCKET_FIXED_POINT_ONE`. --- lightning/src/routing/scoring.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/lightning/src/routing/scoring.rs b/lightning/src/routing/scoring.rs index d40b1f22c40..61b0086ebc6 100644 --- a/lightning/src/routing/scoring.rs +++ b/lightning/src/routing/scoring.rs @@ -1818,15 +1818,27 @@ mod bucketed_history { // values, which will result in us thinking we have some nontrivial probability of // routing up to that amount. if min_liquidity_offset_history_buckets[0] != 0 { - let mut highest_max_bucket_with_points = 0; // The highest max-bucket with any data + // Track the highest max-buckets with any data at all, as well as the highest + // max-bucket with at least BUCKET_FIXED_POINT_ONE. + let mut highest_max_bucket_with_points = 0; + let mut highest_max_bucket_with_full_points = None; let mut total_max_points = 0; // Total points in max-buckets to consider for (max_idx, max_bucket) in max_liquidity_offset_history_buckets.iter().enumerate() { if *max_bucket >= BUCKET_FIXED_POINT_ONE { + highest_max_bucket_with_full_points = Some(cmp::max(highest_max_bucket_with_full_points.unwrap_or(0), max_idx)); + } + if *max_bucket != 0 { highest_max_bucket_with_points = cmp::max(highest_max_bucket_with_points, max_idx); } total_max_points += *max_bucket as u64; } - let max_bucket_end_pos = BUCKET_START_POS[32 - highest_max_bucket_with_points] - 1; + // Use the highest max-bucket with at least BUCKET_FIXED_POINT_ONE, but if none is + // available use the highest max-bucket with any non-zero value. This ensures that + // if we have substantially decayed data we don't end up thinking the highest + // max-bucket is zero even though we have no points in the 0th max-bucket and do + // have points elsewhere. + let selected_max = highest_max_bucket_with_full_points.unwrap_or(highest_max_bucket_with_points); + let max_bucket_end_pos = BUCKET_START_POS[32 - selected_max] - 1; if payment_pos < max_bucket_end_pos { let (numerator, denominator) = success_probability(payment_pos as u64, 0, max_bucket_end_pos as u64, POSITION_TICKS as u64 - 1, params, true); From a6834ebe448a1dd22c67c438ed5bdc584db08f99 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 18:08:04 +0000 Subject: [PATCH 5/7] Fix `docs.rs` for crates which depend on `lightning` w/o features --- lightning-background-processor/Cargo.toml | 1 + lightning-rapid-gossip-sync/Cargo.toml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/lightning-background-processor/Cargo.toml b/lightning-background-processor/Cargo.toml index 70c19c5c76d..ecea0370d4a 100644 --- a/lightning-background-processor/Cargo.toml +++ b/lightning-background-processor/Cargo.toml @@ -11,6 +11,7 @@ edition = "2021" [package.metadata.docs.rs] all-features = true +features = ["lightning/std"] rustdoc-args = ["--cfg", "docsrs"] [features] diff --git a/lightning-rapid-gossip-sync/Cargo.toml b/lightning-rapid-gossip-sync/Cargo.toml index ef28f294378..25db9de9919 100644 --- a/lightning-rapid-gossip-sync/Cargo.toml +++ b/lightning-rapid-gossip-sync/Cargo.toml @@ -9,6 +9,10 @@ description = """ Utility to process gossip routing data from Rapid Gossip Sync Server. """ +[package.metadata.docs.rs] +all-features = true +features = ["lightning/std"] + [features] default = ["std"] std = [] From f0f1b22722741d59f8aeb6dd359f291285893180 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 17:00:56 +0000 Subject: [PATCH 6/7] Bump crate versions to 0.0.125 --- lightning-background-processor/Cargo.toml | 10 +++++----- lightning-block-sync/Cargo.toml | 6 +++--- lightning-custom-message/Cargo.toml | 4 ++-- lightning-net-tokio/Cargo.toml | 6 +++--- lightning-persister/Cargo.toml | 6 +++--- lightning-rapid-gossip-sync/Cargo.toml | 6 +++--- lightning-transaction-sync/Cargo.toml | 6 +++--- lightning/Cargo.toml | 2 +- 8 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lightning-background-processor/Cargo.toml b/lightning-background-processor/Cargo.toml index ecea0370d4a..39443447a50 100644 --- a/lightning-background-processor/Cargo.toml +++ b/lightning-background-processor/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-background-processor" -version = "0.0.124" +version = "0.0.125" authors = ["Valentine Wallace "] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -22,14 +22,14 @@ default = ["std"] [dependencies] bitcoin = { version = "0.32.2", default-features = false } -lightning = { version = "0.0.124", path = "../lightning", default-features = false } -lightning-rapid-gossip-sync = { version = "0.0.124", path = "../lightning-rapid-gossip-sync", default-features = false } +lightning = { version = "0.0.125", path = "../lightning", default-features = false } +lightning-rapid-gossip-sync = { version = "0.0.125", path = "../lightning-rapid-gossip-sync", default-features = false } [dev-dependencies] tokio = { version = "1.35", features = [ "macros", "rt", "rt-multi-thread", "sync", "time" ] } -lightning = { version = "0.0.124", path = "../lightning", features = ["_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", features = ["_test_utils"] } lightning-invoice = { version = "0.32.0", path = "../lightning-invoice" } -lightning-persister = { version = "0.0.124", path = "../lightning-persister" } +lightning-persister = { version = "0.0.125", path = "../lightning-persister" } [lints] workspace = true diff --git a/lightning-block-sync/Cargo.toml b/lightning-block-sync/Cargo.toml index 38ec56a51b7..cefc970edf3 100644 --- a/lightning-block-sync/Cargo.toml +++ b/lightning-block-sync/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-block-sync" -version = "0.0.124" +version = "0.0.125" authors = ["Jeffrey Czyz", "Matt Corallo"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -19,13 +19,13 @@ rpc-client = [ "serde_json", "chunked_transfer" ] [dependencies] bitcoin = "0.32.2" -lightning = { version = "0.0.124", path = "../lightning" } +lightning = { version = "0.0.125", path = "../lightning" } tokio = { version = "1.35", features = [ "io-util", "net", "time", "rt" ], optional = true } serde_json = { version = "1.0", optional = true } chunked_transfer = { version = "1.4", optional = true } [dev-dependencies] -lightning = { version = "0.0.124", path = "../lightning", features = ["_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", features = ["_test_utils"] } tokio = { version = "1.35", features = [ "macros", "rt" ] } [lints] diff --git a/lightning-custom-message/Cargo.toml b/lightning-custom-message/Cargo.toml index d3eaf125520..b6d14517e1f 100644 --- a/lightning-custom-message/Cargo.toml +++ b/lightning-custom-message/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-custom-message" -version = "0.0.124" +version = "0.0.125" authors = ["Jeffrey Czyz"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -15,7 +15,7 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] bitcoin = "0.32.2" -lightning = { version = "0.0.124", path = "../lightning" } +lightning = { version = "0.0.125", path = "../lightning" } [lints] workspace = true diff --git a/lightning-net-tokio/Cargo.toml b/lightning-net-tokio/Cargo.toml index 9df6594b063..a9e7ca73521 100644 --- a/lightning-net-tokio/Cargo.toml +++ b/lightning-net-tokio/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-net-tokio" -version = "0.0.124" +version = "0.0.125" authors = ["Matt Corallo"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning/" @@ -16,12 +16,12 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] bitcoin = "0.32.2" -lightning = { version = "0.0.124", path = "../lightning" } +lightning = { version = "0.0.125", path = "../lightning" } tokio = { version = "1.35", features = [ "rt", "sync", "net", "time" ] } [dev-dependencies] tokio = { version = "1.35", features = [ "macros", "rt", "rt-multi-thread", "sync", "net", "time" ] } -lightning = { version = "0.0.124", path = "../lightning", features = ["_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", features = ["_test_utils"] } [lints] workspace = true diff --git a/lightning-persister/Cargo.toml b/lightning-persister/Cargo.toml index 4007f14cfc1..cfcb05975c1 100644 --- a/lightning-persister/Cargo.toml +++ b/lightning-persister/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-persister" -version = "0.0.124" +version = "0.0.125" authors = ["Valentine Wallace", "Matt Corallo"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -15,7 +15,7 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] bitcoin = "0.32.2" -lightning = { version = "0.0.124", path = "../lightning" } +lightning = { version = "0.0.125", path = "../lightning" } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.48.0", default-features = false, features = ["Win32_Storage_FileSystem", "Win32_Foundation"] } @@ -24,7 +24,7 @@ windows-sys = { version = "0.48.0", default-features = false, features = ["Win32 criterion = { version = "0.4", optional = true, default-features = false } [dev-dependencies] -lightning = { version = "0.0.124", path = "../lightning", features = ["_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", features = ["_test_utils"] } bitcoin = { version = "0.32.2", default-features = false } [lints] diff --git a/lightning-rapid-gossip-sync/Cargo.toml b/lightning-rapid-gossip-sync/Cargo.toml index 25db9de9919..cab32bb1898 100644 --- a/lightning-rapid-gossip-sync/Cargo.toml +++ b/lightning-rapid-gossip-sync/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-rapid-gossip-sync" -version = "0.0.124" +version = "0.0.125" authors = ["Arik Sosman "] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -18,14 +18,14 @@ default = ["std"] std = [] [dependencies] -lightning = { version = "0.0.124", path = "../lightning", default-features = false } +lightning = { version = "0.0.125", path = "../lightning", default-features = false } bitcoin = { version = "0.32.2", default-features = false } [target.'cfg(ldk_bench)'.dependencies] criterion = { version = "0.4", optional = true, default-features = false } [dev-dependencies] -lightning = { version = "0.0.124", path = "../lightning", features = ["_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", features = ["_test_utils"] } [lints] workspace = true diff --git a/lightning-transaction-sync/Cargo.toml b/lightning-transaction-sync/Cargo.toml index bca6e0a7c04..c7657a2ceab 100644 --- a/lightning-transaction-sync/Cargo.toml +++ b/lightning-transaction-sync/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning-transaction-sync" -version = "0.0.124" +version = "0.0.125" authors = ["Elias Rohrer"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning" @@ -23,7 +23,7 @@ electrum = ["electrum-client"] async-interface = [] [dependencies] -lightning = { version = "0.0.124", path = "../lightning", default-features = false, features = ["std"] } +lightning = { version = "0.0.125", path = "../lightning", default-features = false, features = ["std"] } bitcoin = { version = "0.32.2", default-features = false } bdk-macros = "0.6" futures = { version = "0.3", optional = true } @@ -31,7 +31,7 @@ esplora-client = { version = "0.9", default-features = false, optional = true } electrum-client = { version = "0.21.0", optional = true } [dev-dependencies] -lightning = { version = "0.0.124", path = "../lightning", default-features = false, features = ["std", "_test_utils"] } +lightning = { version = "0.0.125", path = "../lightning", default-features = false, features = ["std", "_test_utils"] } tokio = { version = "1.35.0", features = ["full"] } [target.'cfg(not(target_os = "windows"))'.dev-dependencies] diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index 8fa85f7a2fb..7e997c87a8a 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "lightning" -version = "0.0.124" +version = "0.0.125" authors = ["Matt Corallo"] license = "MIT OR Apache-2.0" repository = "https://github.com/lightningdevkit/rust-lightning/" From 2028c7825c958ff52b5b64f7cce408174b731ff8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 14 Oct 2024 17:28:25 +0000 Subject: [PATCH 7/7] Add CHANGELOG entry for 0.0.125 --- CHANGELOG.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 121755a1268..6df2b2c2133 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,26 @@ +# 0.0.125 - Oct 14, 2024 - "Delayed Beta Testing" + +## Bug Fixes + * On upgrade to 0.0.124, channels which were at a steady-state (i.e. for which + the counterparty has received our latest `revoke_and_ack` message) will + force-close upon receiving the next channel state update from our + counterparty. When built with debug assertions a debug assertion failure will + occur instead (#3362). + * Listeners in a `ChainListenerSet` will now have their `block_connected` + method called, when appropriate, rather than always having their + `filtered_block_connected` method called with full block data (#3354). + * Routefinding historical liquidity channel scores were made more consistent + for channels which have very little data which has been decayed (#3362). + * A debug assertion failure when adding nodes to the network graph after + removal of nodes from the network graph was fixed (#3362). + +In total, this release features 6 files changed, 32 insertions, 7 +deletions in 5 commits since 0.0.124 from 2 authors, in alphabetical order: + + * Elias Rohrer + * Matt Corallo + + # 0.0.124 - Sep 3, 2024 - "Papercutting Feature Requests" ## API Updates