From 9b46225551dc3c0f2af8c1128e875e020ebf3717 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 10 Jan 2025 09:51:31 -0600 Subject: [PATCH 01/12] Support Bolt12Invoice::payment_paths in bindings Lack of bindings support was because the method used to return a slice of tuples, it seems. Now that it returns &[BlindedPaymentPath], bindings should be possible given that they can be generated for Bolt12Invoice::message_paths. --- lightning/src/offers/invoice_macros.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/lightning/src/offers/invoice_macros.rs b/lightning/src/offers/invoice_macros.rs index 4a540c16046..93e62d7adaf 100644 --- a/lightning/src/offers/invoice_macros.rs +++ b/lightning/src/offers/invoice_macros.rs @@ -109,9 +109,6 @@ macro_rules! invoice_accessors_common { ($self: ident, $contents: expr, $invoice /// Blinded paths provide recipient privacy by obfuscating its node id. Note, however, that this /// privacy is lost if a public node id is used for #[doc = concat!("[`", stringify!($invoice_type), "::signing_pubkey`].")] - /// - /// This is not exported to bindings users as slices with non-reference types cannot be ABI - /// matched in another language. pub fn payment_paths(&$self) -> &[BlindedPaymentPath] { $contents.payment_paths() } From c5a9c3c3c80e8e1ca8252f877db3d5b5e73c5b81 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 10 Jan 2025 17:17:15 +0000 Subject: [PATCH 02/12] Clean up fuzz test build to fix disk space usage fuzz CI failures --- .github/workflows/build.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 04cdc00a9f9..78fe093a1a2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -229,7 +229,10 @@ jobs: run: | cd fuzz && cargo update -p regex --precise "1.9.6" --verbose && cd .. - name: Sanity check fuzz targets on Rust ${{ env.TOOLCHAIN }} - run: cd fuzz && RUSTFLAGS="--cfg=fuzzing --cfg=secp256k1_fuzz --cfg=hashes_fuzz" cargo test --verbose --color always + run: | + cd fuzz + RUSTFLAGS="--cfg=fuzzing --cfg=secp256k1_fuzz --cfg=hashes_fuzz" cargo test --verbose --color always + cargo clean - name: Run fuzzers run: cd fuzz && ./ci-fuzz.sh && cd .. From 0a2575f535bd4b06935d36ea5a3a23f8e62a38c9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 28 Dec 2024 00:24:31 +0000 Subject: [PATCH 03/12] Fix max-value overflows in `set_max_path_length` When either the amount or the `max_total_cltv_expiry_delta` are set to max-value, `set_max_path_length` can trigger overflows in `build_onion_payloads_callback`, leading to debug-panics. --- lightning/src/ln/onion_utils.rs | 35 ++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 960209c0e0a..a3ffffa3bc2 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -239,7 +239,7 @@ where // the intended recipient). let value_msat = if cur_value_msat == 0 { hop.fee_msat } else { cur_value_msat }; let cltv = if cur_cltv == starting_htlc_offset { - hop.cltv_expiry_delta + starting_htlc_offset + hop.cltv_expiry_delta.saturating_add(starting_htlc_offset) } else { cur_cltv }; @@ -307,7 +307,7 @@ where if cur_value_msat >= 21000000 * 100000000 * 1000 { return Err(APIError::InvalidRoute { err: "Channel fees overflowed?".to_owned() }); } - cur_cltv += hop.cltv_expiry_delta as u32; + cur_cltv = cur_cltv.saturating_add(hop.cltv_expiry_delta as u32); if cur_cltv >= 500000000 { return Err(APIError::InvalidRoute { err: "Channel CLTV overflowed?".to_owned() }); } @@ -333,10 +333,10 @@ pub(crate) fn set_max_path_length( .saturating_add(PAYLOAD_HMAC_LEN); const OVERPAY_ESTIMATE_MULTIPLER: u64 = 3; - let final_value_msat_with_overpay_buffer = core::cmp::max( - route_params.final_value_msat.saturating_mul(OVERPAY_ESTIMATE_MULTIPLER), - MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY, - ); + let final_value_msat_with_overpay_buffer = route_params + .final_value_msat + .saturating_mul(OVERPAY_ESTIMATE_MULTIPLER) + .clamp(MIN_FINAL_VALUE_ESTIMATE_WITH_OVERPAY, 0x1000_0000); let blinded_tail_opt = route_params .payment_params @@ -351,13 +351,15 @@ pub(crate) fn set_max_path_length( excess_final_cltv_expiry_delta: 0, }); + let cltv_expiry_delta = + core::cmp::min(route_params.payment_params.max_total_cltv_expiry_delta, 0x1000_0000); let unblinded_route_hop = RouteHop { pubkey: PublicKey::from_slice(&[2; 33]).unwrap(), node_features: NodeFeatures::empty(), short_channel_id: 42, channel_features: ChannelFeatures::empty(), fee_msat: final_value_msat_with_overpay_buffer, - cltv_expiry_delta: route_params.payment_params.max_total_cltv_expiry_delta, + cltv_expiry_delta, maybe_announced_channel: false, }; let mut num_reserved_bytes: usize = 0; @@ -1280,7 +1282,7 @@ fn decode_next_hop, N: NextPacketBytes>( mod tests { use crate::io; use crate::ln::msgs; - use crate::routing::router::{Path, Route, RouteHop}; + use crate::routing::router::{Path, PaymentParameters, Route, RouteHop}; use crate::types::features::{ChannelFeatures, NodeFeatures}; use crate::types::payment::PaymentHash; use crate::util::ser::{VecWriter, Writeable, Writer}; @@ -1292,7 +1294,7 @@ mod tests { use bitcoin::secp256k1::Secp256k1; use bitcoin::secp256k1::{PublicKey, SecretKey}; - use super::OnionKeys; + use super::*; fn get_test_session_key() -> SecretKey { let hex = "4141414141414141414141414141414141414141414141414141414141414141"; @@ -1607,4 +1609,19 @@ mod tests { writer.write_all(&self.data[..]) } } + + #[test] + fn max_length_with_no_cltv_limit() { + // While users generally shouldn't do this, we shouldn't overflow when + // `max_total_cltv_expiry_delta` is `u32::MAX`. + let recipient = PublicKey::from_slice(&[2; 33]).unwrap(); + let mut route_params = RouteParameters { + payment_params: PaymentParameters::for_keysend(recipient, u32::MAX, true), + final_value_msat: u64::MAX, + max_total_routing_fee_msat: Some(u64::MAX), + }; + route_params.payment_params.max_total_cltv_expiry_delta = u32::MAX; + let recipient_onion = RecipientOnionFields::spontaneous_empty(); + set_max_path_length(&mut route_params, &recipient_onion, None, None, 42).unwrap(); + } } From 886177aa6bac88fd92d6e29f360cd77b9810bc99 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 11 Jan 2025 17:33:15 +0000 Subject: [PATCH 04/12] Drop spurious debug assertion in sweeping logic With the `Confirm` interface, transaction confirmations can come in at any time, so asserting that a confirmation is more recent than the last time we broadcasted a transaction can lead to spurious assertion failures. --- lightning/src/util/sweep.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs index 49c293054c0..b61306194df 100644 --- a/lightning/src/util/sweep.rs +++ b/lightning/src/util/sweep.rs @@ -173,7 +173,6 @@ impl OutputSpendStatus { latest_broadcast_height, .. } => { - debug_assert!(confirmation_height >= *latest_broadcast_height); *self = Self::PendingThresholdConfirmations { first_broadcast_hash: *first_broadcast_hash, latest_broadcast_height: *latest_broadcast_height, From 276d08245a8c590142c57bf03ff9e9fa4723b2a0 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Mon, 13 Jan 2025 10:44:16 +0100 Subject: [PATCH 05/12] Move `lightning-transaction-sync` tests to dedicated script .. and bump its MSRV to 1.75. Recently, `rustls` bumped their MSRV to 1.71. As we depend on them and don't want to continuously pin this security-critical dependency back, we have no choice left but to bump the MSRV for `lightning-transaction-sync` to a version >= 1.71, too. Here, we hence move the `lightning-transaction-sync` tests to a dedicated script and propose to introduce a secondary MSRV of 1.75. We chose this particular version, because: a) it's > 1 year old b) it provides a buffer to 1.71, i.e., if some crate bumped to a version > 1.71, there is a chance we don't immediately have to react again c) it stabilized `async fn`s in traits (see https://blog.rust-lang.org/2023/12/21/async-fn-rpit-in-traits.html), which might become handy for related (BDK) crates, which hopefully will adopt the same target. --- .github/workflows/build.yml | 27 ++++++++++++++++-- Cargo.toml | 2 +- ci/ci-tests.sh | 41 ++------------------------- ci/ci-tx-sync-tests.sh | 39 +++++++++++++++++++++++++ lightning-transaction-sync/Cargo.toml | 10 +++++-- msrv-no-dev-deps-check/Cargo.toml | 1 - no-std-check/Cargo.toml | 4 --- 7 files changed, 74 insertions(+), 50 deletions(-) create mode 100755 ci/ci-tx-sync-tests.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 78fe093a1a2..83ae38a1b9e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,7 +18,7 @@ jobs: fail-fast: false matrix: platform: [ ubuntu-latest, windows-latest, macos-latest ] - toolchain: [ stable, beta, 1.63.0 ] # 1.63.0 is the MSRV for all crates. + toolchain: [ stable, beta, 1.63.0 ] # 1.63.0 is the MSRV for all crates but `lightning-transaction-sync`. exclude: - platform: windows-latest toolchain: 1.63.0 @@ -44,6 +44,27 @@ jobs: - name: Set RUSTFLAGS to deny warnings if: "matrix.toolchain == '1.63.0'" run: echo "RUSTFLAGS=-D warnings" >> "$GITHUB_ENV" + - name: Run CI script + shell: bash # Default on Winblows is powershell + run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh + + build-tx-sync: + strategy: + fail-fast: false + matrix: + platform: [ ubuntu-latest, macos-latest ] + toolchain: [ stable, beta, 1.75.0 ] # 1.75.0 is the MSRV for `lightning-transaction-sync`. + runs-on: ${{ matrix.platform }} + steps: + - name: Checkout source code + uses: actions/checkout@v4 + - name: Install Rust ${{ matrix.toolchain }} toolchain + run: | + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y --profile=minimal --default-toolchain ${{ matrix.toolchain }} + rustup override set ${{ matrix.toolchain }} + - name: Set RUSTFLAGS to deny warnings + if: "matrix.toolchain == '1.75.0'" + run: echo "RUSTFLAGS=-D warnings" >> "$GITHUB_ENV" - name: Enable caching for bitcoind id: cache-bitcoind uses: actions/cache@v4 @@ -57,7 +78,7 @@ jobs: path: bin/electrs-${{ runner.os }}-${{ runner.arch }} key: electrs-${{ runner.os }}-${{ runner.arch }} - name: Download bitcoind/electrs - if: "matrix.platform != 'windows-latest' && (steps.cache-bitcoind.outputs.cache-hit != 'true' || steps.cache-electrs.outputs.cache-hit != 'true')" + if: "steps.cache-bitcoind.outputs.cache-hit != 'true' || steps.cache-electrs.outputs.cache-hit != 'true'" run: | source ./contrib/download_bitcoind_electrs.sh mkdir bin @@ -69,7 +90,7 @@ jobs: echo "ELECTRS_EXE=$( pwd )/bin/electrs-${{ runner.os }}-${{ runner.arch }}" >> "$GITHUB_ENV" - name: Run CI script shell: bash # Default on Winblows is powershell - run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh + run: CI_ENV=1 CI_MINIMIZE_DISK_USAGE=1 ./ci/ci-tx-sync-tests.sh coverage: strategy: diff --git a/Cargo.toml b/Cargo.toml index de0f39a3d25..dc3eb92c7e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ members = [ "lightning-background-processor", "lightning-rapid-gossip-sync", "lightning-custom-message", - "lightning-transaction-sync", "lightning-macros", "lightning-dns-resolver", "lightning-liquidity", @@ -21,6 +20,7 @@ members = [ ] exclude = [ + "lightning-transaction-sync", "no-std-check", "msrv-no-dev-deps-check", "bench", diff --git a/ci/ci-tests.sh b/ci/ci-tests.sh index b7e09246ae3..f4987569fda 100755 --- a/ci/ci-tests.sh +++ b/ci/ci-tests.sh @@ -2,7 +2,6 @@ set -eox pipefail RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }') -HOST_PLATFORM="$(rustc --version --verbose | grep "host:" | awk '{ print $2 }')" # Some crates require pinning to meet our MSRV even for our downstream users, # which we do here. @@ -11,19 +10,6 @@ function PIN_RELEASE_DEPS { # Starting with version 1.39.0, the `tokio` crate has an MSRV of rustc 1.70.0 [ "$RUSTC_MINOR_VERSION" -lt 70 ] && cargo update -p tokio --precise "1.38.1" --verbose - # Starting with version 0.7.12, the `tokio-util` crate has an MSRV of rustc 1.70.0 - [ "$RUSTC_MINOR_VERSION" -lt 70 ] && cargo update -p tokio-util --precise "0.7.11" --verbose - - # url 2.5.3 switched to idna 1.0.3 and ICU4X, which requires rustc 1.67 or newer. - # Here we opt to keep using unicode-rs by pinning idna_adapter as described here: https://docs.rs/crate/idna_adapter/1.2.0 - [ "$RUSTC_MINOR_VERSION" -lt 67 ] && cargo update -p idna_adapter --precise "1.1.0" --verbose - - # indexmap 2.6.0 upgraded to hashbrown 0.15, which unfortunately bumped their MSRV to rustc 1.65 with the 0.15.1 release (and 2.7.0 was released since). - [ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p indexmap@2.7.0 --precise "2.5.0" --verbose - - # Starting with version 0.23.20, the `rustls` crate has an MSRV of rustc 1.71.0 - [ "$RUSTC_MINOR_VERSION" -lt 71 ] && cargo update -p rustls@0.23.20 --precise "0.23.19" --verbose - return 0 # Don't fail the script if our rustc is higher than the last check } @@ -35,15 +21,12 @@ PIN_RELEASE_DEPS # pin the release dependencies in our main workspace # The addr2line v0.21 crate (a dependency of `backtrace` starting with 0.3.69) relies on rustc 1.65 [ "$RUSTC_MINOR_VERSION" -lt 65 ] && cargo update -p backtrace --precise "0.3.68" --verbose -# Starting with version 0.5.9 (there is no .6-.8), the `home` crate has an MSRV of rustc 1.70.0. -[ "$RUSTC_MINOR_VERSION" -lt 70 ] && cargo update -p home --precise "0.5.5" --verbose - # proptest 1.3.0 requires rustc 1.64.0 [ "$RUSTC_MINOR_VERSION" -lt 64 ] && cargo update -p proptest --precise "1.2.0" --verbose export RUST_BACKTRACE=1 -echo -e "\n\nChecking the full workspace." +echo -e "\n\nChecking the workspace, except lightning-transaction-sync." cargo check --verbose --color always # When the workspace members change, make sure to update the list here as well @@ -58,7 +41,6 @@ WORKSPACE_MEMBERS=( lightning-background-processor lightning-rapid-gossip-sync lightning-custom-message - lightning-transaction-sync lightning-macros lightning-dns-resolver lightning-liquidity @@ -83,25 +65,6 @@ cargo check -p lightning-block-sync --verbose --color always --features rpc-clie cargo test -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio cargo check -p lightning-block-sync --verbose --color always --features rpc-client,rest-client,tokio -if [[ "$HOST_PLATFORM" != *windows* ]]; then - echo -e "\n\nChecking Transaction Sync Clients with features." - cargo check -p lightning-transaction-sync --verbose --color always --features esplora-blocking - cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async - cargo check -p lightning-transaction-sync --verbose --color always --features esplora-async-https - cargo check -p lightning-transaction-sync --verbose --color always --features electrum - - if [ -z "$CI_ENV" ] && [[ -z "$BITCOIND_EXE" || -z "$ELECTRS_EXE" ]]; then - echo -e "\n\nSkipping testing Transaction Sync Clients due to BITCOIND_EXE or ELECTRS_EXE being unset." - cargo check -p lightning-transaction-sync --tests - else - echo -e "\n\nTesting Transaction Sync Clients with features." - cargo test -p lightning-transaction-sync --verbose --color always --features esplora-blocking - cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async - cargo test -p lightning-transaction-sync --verbose --color always --features esplora-async-https - cargo test -p lightning-transaction-sync --verbose --color always --features electrum - fi -fi - echo -e "\n\nTest futures builds" cargo test -p lightning-background-processor --verbose --color always --features futures cargo test -p lightning-background-processor --verbose --color always --features futures --no-default-features @@ -145,7 +108,7 @@ cargo test -p lightning-invoice --verbose --color always --no-default-features - echo -e "\n\nTesting no_std build on a downstream no-std crate" # check no-std compatibility across dependencies pushd no-std-check -cargo check --verbose --color always --features lightning-transaction-sync +cargo check --verbose --color always [ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean popd diff --git a/ci/ci-tx-sync-tests.sh b/ci/ci-tx-sync-tests.sh new file mode 100755 index 00000000000..3ca2fae6725 --- /dev/null +++ b/ci/ci-tx-sync-tests.sh @@ -0,0 +1,39 @@ +#!/bin/bash +set -eox pipefail + +RUSTC_MINOR_VERSION=$(rustc --version | awk '{ split($2,a,"."); print a[2] }') + +pushd lightning-transaction-sync + +# Some crates require pinning to meet our MSRV even for our downstream users, +# which we do here. +# Further crates which appear only as dev-dependencies are pinned further down. +function PIN_RELEASE_DEPS { + return 0 # Don't fail the script if our rustc is higher than the last check +} + +PIN_RELEASE_DEPS # pin the release dependencies + +# Starting with version 0.5.11, the `home` crate has an MSRV of rustc 1.81.0. +[ "$RUSTC_MINOR_VERSION" -lt 81 ] && cargo update -p home --precise "0.5.9" --verbose + +export RUST_BACKTRACE=1 + +echo -e "\n\nChecking Transaction Sync Clients with features." +cargo check --verbose --color always --features esplora-blocking +cargo check --verbose --color always --features esplora-async +cargo check --verbose --color always --features esplora-async-https +cargo check --verbose --color always --features electrum + +if [ -z "$CI_ENV" ] && [[ -z "$BITCOIND_EXE" || -z "$ELECTRS_EXE" ]]; then + echo -e "\n\nSkipping testing Transaction Sync Clients due to BITCOIND_EXE or ELECTRS_EXE being unset." + cargo check --tests +else + echo -e "\n\nTesting Transaction Sync Clients with features." + cargo test --verbose --color always --features esplora-blocking + cargo test --verbose --color always --features esplora-async + cargo test --verbose --color always --features esplora-async-https + cargo test --verbose --color always --features electrum +fi + +popd diff --git a/lightning-transaction-sync/Cargo.toml b/lightning-transaction-sync/Cargo.toml index 2e604894108..71922c92fbb 100644 --- a/lightning-transaction-sync/Cargo.toml +++ b/lightning-transaction-sync/Cargo.toml @@ -37,5 +37,11 @@ tokio = { version = "1.35.0", features = ["macros"] } [target.'cfg(not(target_os = "windows"))'.dev-dependencies] electrsd = { version = "0.28.0", default-features = false, features = ["legacy"] } -[lints] -workspace = true +[lints.rust.unexpected_cfgs] +level = "forbid" +# When adding a new cfg attribute, ensure that it is added to this list. +# +# Note that Cargo automatically declares corresponding cfgs for every feature +# defined in the member-level [features] tables as "expected". +check-cfg = [ +] diff --git a/msrv-no-dev-deps-check/Cargo.toml b/msrv-no-dev-deps-check/Cargo.toml index be594f6e5c4..3a4acc675e6 100644 --- a/msrv-no-dev-deps-check/Cargo.toml +++ b/msrv-no-dev-deps-check/Cargo.toml @@ -6,7 +6,6 @@ edition = "2021" [dependencies] lightning = { path = "../lightning" } lightning-block-sync = { path = "../lightning-block-sync", features = [ "rest-client", "rpc-client" ] } -lightning-transaction-sync = { path = "../lightning-transaction-sync", features = [ "esplora-async-https", "electrum" ] } lightning-invoice = { path = "../lightning-invoice" } lightning-net-tokio = { path = "../lightning-net-tokio" } lightning-persister = { path = "../lightning-persister" } diff --git a/no-std-check/Cargo.toml b/no-std-check/Cargo.toml index bc43e63404a..f97673414d9 100644 --- a/no-std-check/Cargo.toml +++ b/no-std-check/Cargo.toml @@ -11,7 +11,3 @@ lightning = { path = "../lightning", default-features = false } lightning-invoice = { path = "../lightning-invoice", default-features = false } lightning-rapid-gossip-sync = { path = "../lightning-rapid-gossip-sync", default-features = false } lightning-background-processor = { path = "../lightning-background-processor", features = ["futures"], default-features = false } - -# Obviously lightning-transaction-sync doesn't support no-std, but it should build -# even if lightning is built with no-std. -lightning-transaction-sync = { path = "../lightning-transaction-sync", optional = true } From de15b0f31d2db0a080eae0eb8d57ce908b45555c Mon Sep 17 00:00:00 2001 From: Matt Morehouse Date: Tue, 14 Jan 2025 15:40:07 -0600 Subject: [PATCH 06/12] Fix package splitting logic When scanning confirmed transactions for spends that conflict with our existing packages, we should continue scanning after detecting the first conflicting package since a transaction can conflict with multiple packages. This ensures that we remove *all* inputs from our packages that have already been spent by the counterparty so that valid claim transactions are generated. Fixes https://github.com/lightningdevkit/rust-lightning/issues/3537. --- lightning/src/chain/onchaintx.rs | 1 - lightning/src/ln/functional_tests.rs | 259 +++++++++++++++++++++++++++ 2 files changed, 259 insertions(+), 1 deletion(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index cf7f8d7bfe0..759668cfa9c 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -964,7 +964,6 @@ impl OnchainTxHandler { self.pending_claim_events.retain(|entry| entry.0 != *claim_id); } } - break; //No need to iterate further, either tx is our or their } else { panic!("Inconsistencies between pending_claim_requests map and claimable_outpoints map"); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index ac43efe4499..e52870cf19d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -18,6 +18,7 @@ use crate::chain::channelmonitor; use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use crate::chain::transaction::OutPoint; use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider}; +use crate::events::bump_transaction::WalletSource; use crate::events::{Event, FundingInfo, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentSecret, PaymentHash}; @@ -2774,6 +2775,264 @@ fn claim_htlc_outputs() { assert_eq!(nodes[1].node.list_channels().len(), 0); } +// Test that the HTLC package logic removes HTLCs from the package when they are claimed by the +// counterparty, even when the counterparty claims HTLCs from multiple packages in a single +// transaction. +// +// This is a regression test for https://github.com/lightningdevkit/rust-lightning/issues/3537. +#[test] +fn test_multiple_package_conflicts() { + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let mut user_cfg = test_default_channel_config(); + + // Anchor channels are required so that multiple HTLC-Successes can be aggregated into a single + // transaction. + user_cfg.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + user_cfg.manually_accept_inbound_channels = true; + + let node_chanmgrs = + create_node_chanmgrs(3, &node_cfgs, &[Some(user_cfg), Some(user_cfg), Some(user_cfg)]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + // Since we're using anchor channels, make sure each node has a UTXO for paying fees. + let coinbase_tx = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { ..Default::default() }], + output: vec![ + TxOut { + value: Amount::ONE_BTC, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + TxOut { + value: Amount::ONE_BTC, + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }, + TxOut { + value: Amount::ONE_BTC, + script_pubkey: nodes[2].wallet_source.get_change_script().unwrap(), + }, + ], + }; + nodes[0].wallet_source.add_utxo( + bitcoin::OutPoint { txid: coinbase_tx.compute_txid(), vout: 0 }, + coinbase_tx.output[0].value, + ); + nodes[1].wallet_source.add_utxo( + bitcoin::OutPoint { txid: coinbase_tx.compute_txid(), vout: 1 }, + coinbase_tx.output[1].value, + ); + nodes[2].wallet_source.add_utxo( + bitcoin::OutPoint { txid: coinbase_tx.compute_txid(), vout: 2 }, + coinbase_tx.output[2].value, + ); + + // Create the network. + // 0 -- 1 -- 2 + // + // Payments will be routed from node 0 to node 2. Node 2 will force close and spend HTLCs from + // two of node 1's packages. We will then verify that node 1 correctly removes the conflicting + // HTLC spends from its packages. + const CHAN_CAPACITY: u64 = 10_000_000; + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, CHAN_CAPACITY, 0); + let (_, _, cid_1_2, funding_tx_1_2) = + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, CHAN_CAPACITY, 0); + + // Ensure all nodes are at the same initial height. + let node_max_height = nodes.iter().map(|node| node.best_block_info().1).max().unwrap(); + for node in &nodes { + let blocks_to_mine = node_max_height - node.best_block_info().1; + if blocks_to_mine > 0 { + connect_blocks(node, blocks_to_mine); + } + } + + // Route HTLC 1. + let (preimage_1, payment_hash_1, ..) = + route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000); + + // Route HTLCs 2 and 3, with CLTVs 1 higher than HTLC 1. The higher CLTVs will cause these + // HTLCs to be included in a different package than HTLC 1. + connect_blocks(&nodes[0], 1); + connect_blocks(&nodes[1], 1); + connect_blocks(&nodes[2], 1); + let (preimage_2, payment_hash_2, ..) = + route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000); + route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 900_000_000); + + // Mine blocks until HTLC 1 times out in 1 block and HTLCs 2 and 3 time out in 2 blocks. + connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); + + // Node 2 force closes, causing node 1 to group the HTLCs into the following packages: + // Package 1: HTLC 1 + // Package 2: HTLCs 2 and 3 + let node2_commit_tx = get_local_commitment_txn!(nodes[2], cid_1_2); + assert_eq!(node2_commit_tx.len(), 1); + let node2_commit_tx = &node2_commit_tx[0]; + check_spends!(node2_commit_tx, funding_tx_1_2); + mine_transaction(&nodes[1], node2_commit_tx); + check_closed_event( + &nodes[1], + 1, + ClosureReason::CommitmentTxConfirmed, + false, + &[nodes[2].node.get_our_node_id()], + CHAN_CAPACITY, + ); + check_closed_broadcast!(nodes[1], true); + check_added_monitors(&nodes[1], 1); + + // Node 1 should immediately claim package 1 but has to wait a block to claim package 2. + let timeout_tx = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(timeout_tx.len(), 1); + check_spends!(timeout_tx[0], node2_commit_tx); + assert_eq!(timeout_tx[0].input.len(), 1); + + // After one block, node 1 should also attempt to claim package 2. + connect_blocks(&nodes[1], 1); + let timeout_tx = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(timeout_tx.len(), 1); + check_spends!(timeout_tx[0], node2_commit_tx); + assert_eq!(timeout_tx[0].input.len(), 2); + + // Force node 2 to broadcast an aggregated HTLC-Success transaction spending HTLCs 1 and 2. + // This will conflict with both of node 1's HTLC packages. + { + let broadcaster = &node_cfgs[2].tx_broadcaster; + let fee_estimator = &LowerBoundedFeeEstimator::new(node_cfgs[2].fee_estimator); + let logger = &node_cfgs[2].logger; + let monitor = get_monitor!(nodes[2], cid_1_2); + monitor.provide_payment_preimage_unsafe_legacy( + &payment_hash_1, + &preimage_1, + broadcaster, + fee_estimator, + logger, + ); + monitor.provide_payment_preimage_unsafe_legacy( + &payment_hash_2, + &preimage_2, + broadcaster, + fee_estimator, + logger, + ); + } + mine_transaction(&nodes[2], node2_commit_tx); + check_closed_event( + &nodes[2], + 1, + ClosureReason::CommitmentTxConfirmed, + false, + &[nodes[1].node.get_our_node_id()], + CHAN_CAPACITY, + ); + check_closed_broadcast!(nodes[2], true); + check_added_monitors(&nodes[2], 1); + + let process_bump_event = |node: &Node| { + let events = node.chain_monitor.chain_monitor.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + let bump_event = match &events[0] { + Event::BumpTransaction(bump_event) => bump_event, + _ => panic!("Unexepected event"), + }; + node.bump_tx_handler.handle_event(bump_event); + + let mut tx = node.tx_broadcaster.txn_broadcast(); + assert_eq!(tx.len(), 1); + tx.pop().unwrap() + }; + + let conflict_tx = process_bump_event(&nodes[2]); + assert_eq!(conflict_tx.input.len(), 3); + assert_eq!(conflict_tx.input[0].previous_output.txid, node2_commit_tx.compute_txid()); + assert_eq!(conflict_tx.input[1].previous_output.txid, node2_commit_tx.compute_txid()); + assert_eq!(conflict_tx.input[2].previous_output.txid, coinbase_tx.compute_txid()); + + // Mine node 2's aggregated HTLC-Success transaction on node 1, causing the package splitting + // logic to run. Package 2 should get split so that only HTLC 3 gets claimed. + mine_transaction(&nodes[1], &conflict_tx); + + // Check that node 1 only attempts to claim HTLC 3 now. There should be no conflicting spends + // in the newly broadcasted transaction. + let broadcasted_txs = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(broadcasted_txs.len(), 1); + let txins = &broadcasted_txs[0].input; + assert_eq!(txins.len(), 1); + assert_eq!(txins[0].previous_output.txid, node2_commit_tx.compute_txid()); + for conflict_in in &conflict_tx.input { + assert_ne!(txins[0].previous_output, conflict_in.previous_output); + } + + // Node 1 should also extract the preimages from the mined transaction and claim them upstream. + // + // Because two update_fulfill_htlc messages are created at once, the commitment_signed_dance + // macro doesn't work properly and we must process the first update_fulfill_htlc manually. + let updates = get_htlc_update_msgs(&nodes[1], &nodes[0].node.get_our_node_id()); + assert_eq!(updates.update_fulfill_htlcs.len(), 1); + nodes[0].node.handle_update_fulfill_htlc( + nodes[1].node.get_our_node_id(), + &updates.update_fulfill_htlcs[0], + ); + nodes[0] + .node + .handle_commitment_signed(nodes[1].node.get_our_node_id(), &updates.commitment_signed); + check_added_monitors(&nodes[0], 1); + + let (revoke_ack, commit_signed) = + get_revoke_commit_msgs(&nodes[0], &nodes[1].node.get_our_node_id()); + nodes[1].node.handle_revoke_and_ack(nodes[0].node.get_our_node_id(), &revoke_ack); + nodes[1].node.handle_commitment_signed(nodes[0].node.get_our_node_id(), &commit_signed); + check_added_monitors(&nodes[1], 4); + + let events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 2); + let revoke_ack = match &events[1] { + MessageSendEvent::SendRevokeAndACK { node_id: _, msg } => msg, + _ => panic!("Unexpected event"), + }; + nodes[0].node.handle_revoke_and_ack(nodes[1].node.get_our_node_id(), revoke_ack); + expect_payment_sent!(nodes[0], preimage_1); + + let updates = match &events[0] { + MessageSendEvent::UpdateHTLCs { node_id: _, updates } => updates, + _ => panic!("Unexpected event"), + }; + assert_eq!(updates.update_fulfill_htlcs.len(), 1); + nodes[0].node.handle_update_fulfill_htlc( + nodes[1].node.get_our_node_id(), + &updates.update_fulfill_htlcs[0], + ); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); + expect_payment_sent!(nodes[0], preimage_2); + + let mut events = nodes[1].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + expect_payment_forwarded( + events.pop().unwrap(), + &nodes[1], + &nodes[0], + &nodes[2], + Some(1000), + None, + false, + true, + false, + ); + expect_payment_forwarded( + events.pop().unwrap(), + &nodes[1], + &nodes[0], + &nodes[2], + Some(1000), + None, + false, + true, + false, + ); +} + #[test] fn test_htlc_on_chain_success() { // Test that in case of a unilateral close onchain, we detect the state of output and pass From e290d44e16c26da6f41137a14a2fe0c74a6934d0 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Fri, 10 Jan 2025 07:57:05 +0200 Subject: [PATCH 07/12] ci: silence unnecessary_map_or lint as solution requires MSRV >= 1.70 Rust 1.84.0 was recently released along with some new clippy lints, one of which is `unnecessary_map_or`. Unfortunately this lint suggests using `Option::is_some_and` as a fix, but this is only available in Rust version >= 1.70, while we still have an MSRV of 1.63. So we silence that lint for now. We'd still like our lint CI to use stable Rust so that we can benefit from new lint checks which may be helpful and don't require an MSRV bump, but sometimes new lints (like in this case) do. See: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_or https://doc.rust-lang.org/std/option/enum.Option.html#method.is_some_and --- ci/check-lint.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/check-lint.sh b/ci/check-lint.sh index b7f1bb909f3..b66a856797a 100755 --- a/ci/check-lint.sh +++ b/ci/check-lint.sh @@ -93,4 +93,5 @@ RUSTFLAGS='-D warnings' cargo clippy -- \ -A clippy::unnecessary_to_owned \ -A clippy::unnecessary_unwrap \ -A clippy::unused_unit \ - -A clippy::useless_conversion + -A clippy::useless_conversion \ + -A clippy::unnecessary_map_or `# to be removed once we hit MSRV 1.70` From 82022509d0ff280115d4d18baad130b80bf48b79 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 14 Jan 2025 11:15:21 +0100 Subject: [PATCH 08/12] `liquidity`: Allow setting `process_events` callback in `c_bindings` To trigger message processing, we previously had the user set a callback to `PeerManager::process_events` via an `Fn()` callback. This is however not supported by `c_bindings`. Here, we therefore introduce as `ProcessMesssagesCallback` trait that can be used via `LiquidityManager::set_process_msgs_callback_fn`, which is exposed in `c_bindings`. --- lightning-liquidity/src/manager.rs | 69 ++---------------------- lightning-liquidity/src/message_queue.rs | 38 ++++++++----- 2 files changed, 29 insertions(+), 78 deletions(-) diff --git a/lightning-liquidity/src/manager.rs b/lightning-liquidity/src/manager.rs index 1e467c302de..a4c13033370 100644 --- a/lightning-liquidity/src/manager.rs +++ b/lightning-liquidity/src/manager.rs @@ -7,7 +7,7 @@ use crate::lsps0::ser::{ LSPS_MESSAGE_TYPE_ID, }; use crate::lsps0::service::LSPS0ServiceHandler; -use crate::message_queue::MessageQueue; +use crate::message_queue::{MessageQueue, ProcessMessagesCallback}; use crate::lsps1::client::{LSPS1ClientConfig, LSPS1ClientHandler}; use crate::lsps1::msgs::LSPS1Message; @@ -17,7 +17,7 @@ use crate::lsps1::service::{LSPS1ServiceConfig, LSPS1ServiceHandler}; use crate::lsps2::client::{LSPS2ClientConfig, LSPS2ClientHandler}; use crate::lsps2::msgs::LSPS2Message; use crate::lsps2::service::{LSPS2ServiceConfig, LSPS2ServiceHandler}; -use crate::prelude::{new_hash_map, new_hash_set, HashMap, HashSet, ToString, Vec}; +use crate::prelude::{new_hash_map, new_hash_set, Box, HashMap, HashSet, ToString, Vec}; use crate::sync::{Arc, Mutex, RwLock}; use lightning::chain::{self, BestBlock, Confirm, Filter, Listen}; @@ -315,69 +315,8 @@ where { /// ``` /// /// [`PeerManager::process_events`]: lightning::ln::peer_handler::PeerManager::process_events - #[cfg(feature = "std")] - pub fn set_process_msgs_callback(&self, callback: impl Fn() + Send + Sync + 'static) { - self.pending_messages.set_process_msgs_callback(callback) - } - - /// Allows to set a callback that will be called after new messages are pushed to the message - /// queue. - /// - /// Usually, you'll want to use this to call [`PeerManager::process_events`] to clear the - /// message queue. For example: - /// - /// ``` - /// # use lightning::io; - /// # use lightning_liquidity::LiquidityManager; - /// # use std::sync::{Arc, RwLock}; - /// # use std::sync::atomic::{AtomicBool, Ordering}; - /// # use std::time::SystemTime; - /// # struct MyStore {} - /// # impl lightning::util::persist::KVStore for MyStore { - /// # fn read(&self, primary_namespace: &str, secondary_namespace: &str, key: &str) -> io::Result> { Ok(Vec::new()) } - /// # fn write(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, buf: &[u8]) -> io::Result<()> { Ok(()) } - /// # fn remove(&self, primary_namespace: &str, secondary_namespace: &str, key: &str, lazy: bool) -> io::Result<()> { Ok(()) } - /// # fn list(&self, primary_namespace: &str, secondary_namespace: &str) -> io::Result> { Ok(Vec::new()) } - /// # } - /// # struct MyEntropySource {} - /// # impl lightning::sign::EntropySource for MyEntropySource { - /// # fn get_secure_random_bytes(&self) -> [u8; 32] { [0u8; 32] } - /// # } - /// # struct MyEventHandler {} - /// # impl MyEventHandler { - /// # async fn handle_event(&self, _: lightning::events::Event) {} - /// # } - /// # #[derive(Eq, PartialEq, Clone, Hash)] - /// # struct MySocketDescriptor {} - /// # impl lightning::ln::peer_handler::SocketDescriptor for MySocketDescriptor { - /// # fn send_data(&mut self, _data: &[u8], _resume_read: bool) -> usize { 0 } - /// # fn disconnect_socket(&mut self) {} - /// # } - /// # type MyBroadcaster = dyn lightning::chain::chaininterface::BroadcasterInterface; - /// # type MyFeeEstimator = dyn lightning::chain::chaininterface::FeeEstimator; - /// # type MyNodeSigner = dyn lightning::sign::NodeSigner; - /// # type MyUtxoLookup = dyn lightning::routing::utxo::UtxoLookup; - /// # type MyFilter = dyn lightning::chain::Filter; - /// # type MyLogger = dyn lightning::util::logger::Logger; - /// # type MyChainMonitor = lightning::chain::chainmonitor::ChainMonitor, Arc, Arc, Arc, Arc>; - /// # type MyPeerManager = lightning::ln::peer_handler::SimpleArcPeerManager, MyLogger>; - /// # type MyNetworkGraph = lightning::routing::gossip::NetworkGraph>; - /// # type MyGossipSync = lightning::routing::gossip::P2PGossipSync, Arc, Arc>; - /// # type MyChannelManager = lightning::ln::channelmanager::SimpleArcChannelManager; - /// # type MyScorer = RwLock, Arc>>; - /// # type MyLiquidityManager = LiquidityManager, Arc, Arc>; - /// # fn setup_background_processing(my_persister: Arc, my_event_handler: Arc, my_chain_monitor: Arc, my_channel_manager: Arc, my_logger: Arc, my_peer_manager: Arc, my_liquidity_manager: Arc) { - /// let process_msgs_pm = Arc::clone(&my_peer_manager); - /// let process_msgs_callback = move || process_msgs_pm.process_events(); - /// - /// my_liquidity_manager.set_process_msgs_callback(process_msgs_callback); - /// # } - /// ``` - /// - /// [`PeerManager::process_events`]: lightning::ln::peer_handler::PeerManager::process_events - #[cfg(not(feature = "std"))] - pub fn set_process_msgs_callback(&self, callback: impl Fn() + 'static) { - self.pending_messages.set_process_msgs_callback(callback) + pub fn set_process_msgs_callback(&self, callback: F) { + self.pending_messages.set_process_msgs_callback(Box::new(callback)); } /// Blocks the current thread until next event is ready and returns it. diff --git a/lightning-liquidity/src/message_queue.rs b/lightning-liquidity/src/message_queue.rs index 89dab8a318e..7b61a87bcd4 100644 --- a/lightning-liquidity/src/message_queue.rs +++ b/lightning-liquidity/src/message_queue.rs @@ -11,10 +11,7 @@ use bitcoin::secp256k1::PublicKey; /// [`LiquidityManager`]: crate::LiquidityManager pub struct MessageQueue { queue: Mutex>, - #[cfg(feature = "std")] - process_msgs_callback: RwLock>>, - #[cfg(not(feature = "std"))] - process_msgs_callback: RwLock>>, + process_msgs_callback: RwLock>>, } impl MessageQueue { @@ -24,14 +21,8 @@ impl MessageQueue { Self { queue, process_msgs_callback } } - #[cfg(feature = "std")] - pub(crate) fn set_process_msgs_callback(&self, callback: impl Fn() + Send + Sync + 'static) { - *self.process_msgs_callback.write().unwrap() = Some(Box::new(callback)); - } - - #[cfg(not(feature = "std"))] - pub(crate) fn set_process_msgs_callback(&self, callback: impl Fn() + 'static) { - *self.process_msgs_callback.write().unwrap() = Some(Box::new(callback)); + pub(crate) fn set_process_msgs_callback(&self, callback: Box) { + *self.process_msgs_callback.write().unwrap() = Some(callback); } pub(crate) fn get_and_clear_pending_msgs(&self) -> Vec<(PublicKey, LSPSMessage)> { @@ -45,7 +36,28 @@ impl MessageQueue { } if let Some(process_msgs_callback) = self.process_msgs_callback.read().unwrap().as_ref() { - (process_msgs_callback)() + process_msgs_callback.call() } } } + +macro_rules! define_callback { ($($bounds: path),*) => { +/// A callback which will be called to trigger network message processing. +/// +/// Usually, this should call [`PeerManager::process_events`]. +/// +/// [`PeerManager::process_events`]: lightning::ln::peer_handler::PeerManager::process_events +pub trait ProcessMessagesCallback : $($bounds +)* { + /// The method which is called. + fn call(&self); +} + +impl ProcessMessagesCallback for F { + fn call(&self) { (self)(); } +} +} } + +#[cfg(feature = "std")] +define_callback!(Send, Sync); +#[cfg(not(feature = "std"))] +define_callback!(); From 3386c4b9925c93826f28dd080841403e88910a8d Mon Sep 17 00:00:00 2001 From: Ian Slane Date: Thu, 8 Aug 2024 07:50:03 -0600 Subject: [PATCH 09/12] Validate amount_msats against invreq amount Add a check to ensure that the amount_msats in an invoice matches the amount_msats specified in the invoice_request or offer (or refund). Reject the invoice as invalid if there is a mismatch between these amounts. Otherwise, an invoice may be paid with an amount greater than the requested amount. Co-authored-by: Ian Slane Co-authored-by: Jeffrey Czyz --- lightning/src/ln/offers_tests.rs | 12 ++-- lightning/src/offers/invoice.rs | 77 ++++++++++++++++++++++++- lightning/src/offers/invoice_macros.rs | 8 +++ lightning/src/offers/invoice_request.rs | 56 +++++++++++++++++- lightning/src/offers/parse.rs | 2 +- 5 files changed, 144 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 6455a60b139..35a4c61713c 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -566,7 +566,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() { human_readable_name: None, }, }); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), david_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id)); @@ -727,7 +727,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() { human_readable_name: None, }, }); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), bob_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id)); @@ -1116,7 +1116,7 @@ fn creates_and_pays_for_offer_with_retry() { human_readable_name: None, }, }); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), bob_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(bob_id)); let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); @@ -1411,7 +1411,7 @@ fn fails_authentication_when_handling_invoice_request() { alice.onion_messenger.handle_onion_message(david_id, &onion_message); let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), david_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id)); @@ -1441,7 +1441,7 @@ fn fails_authentication_when_handling_invoice_request() { alice.onion_messenger.handle_onion_message(bob_id, &onion_message); let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), david_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id)); @@ -1543,7 +1543,7 @@ fn fails_authentication_when_handling_invoice_for_offer() { alice.onion_messenger.handle_onion_message(bob_id, &onion_message); let (invoice_request, reply_path) = extract_invoice_request(alice, &onion_message); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(10_000_000)); assert_ne!(invoice_request.payer_signing_pubkey(), david_id); assert_eq!(reply_path.introduction_node(), &IntroductionNode::NodeId(charlie_id)); diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 136ea2625de..d6a0392dac2 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -342,7 +342,7 @@ macro_rules! invoice_builder_methods { ( pub(crate) fn amount_msats( invoice_request: &InvoiceRequest ) -> Result { - match invoice_request.amount_msats() { + match invoice_request.contents.inner.amount_msats() { Some(amount_msats) => Ok(amount_msats), None => match invoice_request.contents.inner.offer.amount() { Some(Amount::Bitcoin { amount_msats }) => { @@ -1531,6 +1531,11 @@ impl TryFrom for InvoiceContents { experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream, ) )?; + + if amount_msats != refund.amount_msats() { + return Err(Bolt12SemanticError::InvalidAmount); + } + Ok(InvoiceContents::ForRefund { refund, fields }) } else { let invoice_request = InvoiceRequestContents::try_from( @@ -1539,6 +1544,13 @@ impl TryFrom for InvoiceContents { experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream, ) )?; + + if let Some(requested_amount_msats) = invoice_request.amount_msats() { + if amount_msats != requested_amount_msats { + return Err(Bolt12SemanticError::InvalidAmount); + } + } + Ok(InvoiceContents::ForOffer { invoice_request, fields }) } } @@ -2707,6 +2719,69 @@ mod tests { } } + #[test] + fn fails_parsing_invoice_with_wrong_amount() { + let expanded_key = ExpandedKey::new([42; 32]); + let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); + let secp_ctx = Secp256k1::new(); + let payment_id = PaymentId([1; 32]); + + let invoice = OfferBuilder::new(recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() + .build_and_sign().unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap() + .amount_msats_unchecked(2000) + .build().unwrap() + .sign(recipient_sign).unwrap(); + + let mut buffer = Vec::new(); + invoice.write(&mut buffer).unwrap(); + + match Bolt12Invoice::try_from(buffer) { + Ok(_) => panic!("expected error"), + Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)), + } + + let invoice = OfferBuilder::new(recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() + .amount_msats(1000).unwrap() + .build_and_sign().unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap() + .amount_msats_unchecked(2000) + .build().unwrap() + .sign(recipient_sign).unwrap(); + + let mut buffer = Vec::new(); + invoice.write(&mut buffer).unwrap(); + + match Bolt12Invoice::try_from(buffer) { + Ok(_) => panic!("expected error"), + Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)), + } + + let invoice = RefundBuilder::new(vec![1; 32], payer_pubkey(), 1000).unwrap() + .build().unwrap() + .respond_using_derived_keys_no_std( + payment_paths(), payment_hash(), now(), &expanded_key, &entropy + ) + .unwrap() + .amount_msats_unchecked(2000) + .build_and_sign(&secp_ctx).unwrap(); + + let mut buffer = Vec::new(); + invoice.write(&mut buffer).unwrap(); + + match Bolt12Invoice::try_from(buffer) { + Ok(_) => panic!("expected error"), + Err(e) => assert_eq!(e, Bolt12ParseError::InvalidSemantics(Bolt12SemanticError::InvalidAmount)), + } + } + #[test] fn fails_parsing_invoice_without_signature() { let expanded_key = ExpandedKey::new([42; 32]); diff --git a/lightning/src/offers/invoice_macros.rs b/lightning/src/offers/invoice_macros.rs index 93e62d7adaf..dd75fe62504 100644 --- a/lightning/src/offers/invoice_macros.rs +++ b/lightning/src/offers/invoice_macros.rs @@ -87,6 +87,14 @@ macro_rules! invoice_builder_methods_test { ( $self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr $(, $self_mut: tt)? ) => { + #[cfg_attr(c_bindings, allow(dead_code))] + pub(crate) fn amount_msats_unchecked( + $($self_mut)* $self: $self_type, amount_msats: u64, + ) -> $return_type { + $invoice_fields.amount_msats = amount_msats; + $return_value + } + #[cfg_attr(c_bindings, allow(dead_code))] pub(crate) fn features_unchecked( $($self_mut)* $self: $self_type, features: Bolt12InvoiceFeatures diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 74bbdb8a0bf..1a3eb0b5e5a 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -79,7 +79,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE}; use crate::offers::nonce::Nonce; -use crate::offers::offer::{EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; +use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef}; use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError}; use crate::offers::payer::{PayerContents, PayerTlvStream, PayerTlvStreamRef}; use crate::offers::signer::{Metadata, MetadataMaterial}; @@ -974,7 +974,15 @@ impl InvoiceRequestContents { } pub(super) fn amount_msats(&self) -> Option { - self.inner.amount_msats + self.inner + .amount_msats() + .or_else(|| match self.inner.offer.amount() { + Some(Amount::Bitcoin { amount_msats }) => { + Some(amount_msats.saturating_mul(self.quantity().unwrap_or(1))) + }, + Some(Amount::Currency { .. }) => None, + None => { debug_assert!(false); None}, + }) } pub(super) fn features(&self) -> &InvoiceRequestFeatures { @@ -1015,6 +1023,10 @@ impl InvoiceRequestContentsWithoutPayerSigningPubkey { self.chain.unwrap_or_else(|| self.offer.implied_chain()) } + pub(super) fn amount_msats(&self) -> Option { + self.amount_msats + } + pub(super) fn as_tlv_stream(&self) -> PartialInvoiceRequestTlvStreamRef { let payer = PayerTlvStreamRef { metadata: self.payer.0.as_bytes(), @@ -1381,7 +1393,7 @@ mod tests { assert_eq!(invoice_request.supported_quantity(), Quantity::One); assert_eq!(invoice_request.issuer_signing_pubkey(), Some(recipient_pubkey())); assert_eq!(invoice_request.chain(), ChainHash::using_genesis_block(Network::Bitcoin)); - assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(invoice_request.amount_msats(), Some(1000)); assert_eq!(invoice_request.invoice_request_features(), &InvoiceRequestFeatures::empty()); assert_eq!(invoice_request.quantity(), None); assert_eq!(invoice_request.payer_note(), None); @@ -1748,6 +1760,44 @@ mod tests { } } + #[test] + fn builds_invoice_request_without_amount() { + let expanded_key = ExpandedKey::new([42; 32]); + let entropy = FixedEntropy {}; + let nonce = Nonce::from_entropy_source(&entropy); + let secp_ctx = Secp256k1::new(); + let payment_id = PaymentId([1; 32]); + + let invoice_request = OfferBuilder::new(recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() + .build_and_sign().unwrap(); + let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert_eq!(invoice_request.amount_msats(), Some(1000)); + assert_eq!(tlv_stream.amount, None); + + let invoice_request = OfferBuilder::new(recipient_pubkey()) + .amount_msats(1000) + .supported_quantity(Quantity::Unbounded) + .build().unwrap() + .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() + .quantity(2).unwrap() + .build_and_sign().unwrap(); + let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert_eq!(invoice_request.amount_msats(), Some(2000)); + assert_eq!(tlv_stream.amount, None); + + let invoice_request = OfferBuilder::new(recipient_pubkey()) + .amount(Amount::Currency { iso4217_code: *b"USD", amount: 10 }) + .build_unchecked() + .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() + .build_unchecked_and_sign(); + let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert_eq!(invoice_request.amount_msats(), None); + assert_eq!(tlv_stream.amount, None); + } + #[test] fn builds_invoice_request_with_features() { let expanded_key = ExpandedKey::new([42; 32]); diff --git a/lightning/src/offers/parse.rs b/lightning/src/offers/parse.rs index f3c481a9f95..6b72c6b1682 100644 --- a/lightning/src/offers/parse.rs +++ b/lightning/src/offers/parse.rs @@ -147,7 +147,7 @@ pub enum Bolt12SemanticError { UnexpectedChain, /// An amount was expected but was missing. MissingAmount, - /// The amount exceeded the total bitcoin supply. + /// The amount exceeded the total bitcoin supply or didn't match an expected amount. InvalidAmount, /// An amount was provided but was not sufficient in value. InsufficientAmount, From 3c356abbd782754bcc97bc552f037fd843f36162 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 15 Jan 2025 09:56:04 -0600 Subject: [PATCH 10/12] Add InvoiceRequest::has_amount_msats When InvoiceRequest::amount_msats returns Some, it may have been inferred from the Offer::amount and InvoiceRequest::quantity. Add a method to InvoiceRequest for determining if the amount was explicitly set. --- lightning/src/offers/invoice_request.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index 1a3eb0b5e5a..957884f69d0 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -665,6 +665,15 @@ macro_rules! invoice_request_accessors { ($self: ident, $contents: expr) => { $contents.amount_msats() } + /// Returns whether an amount was set in the request; otherwise, if [`amount_msats`] is `Some` + /// then it was inferred from the [`Offer::amount`] and [`quantity`]. + /// + /// [`amount_msats`]: Self::amount_msats + /// [`quantity`]: Self::quantity + pub fn has_amount_msats(&$self) -> bool { + $contents.has_amount_msats() + } + /// Features pertaining to requesting an invoice. pub fn invoice_request_features(&$self) -> &InvoiceRequestFeatures { &$contents.features() @@ -985,6 +994,10 @@ impl InvoiceRequestContents { }) } + pub(super) fn has_amount_msats(&self) -> bool { + self.inner.amount_msats().is_some() + } + pub(super) fn features(&self) -> &InvoiceRequestFeatures { &self.inner.features } @@ -1669,6 +1682,7 @@ mod tests { .amount_msats(1000).unwrap() .build_and_sign().unwrap(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), Some(1000)); assert_eq!(tlv_stream.amount, Some(1000)); @@ -1680,6 +1694,7 @@ mod tests { .amount_msats(1000).unwrap() .build_and_sign().unwrap(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), Some(1000)); assert_eq!(tlv_stream.amount, Some(1000)); @@ -1690,6 +1705,7 @@ mod tests { .amount_msats(1001).unwrap() .build_and_sign().unwrap(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), Some(1001)); assert_eq!(tlv_stream.amount, Some(1001)); @@ -1774,6 +1790,7 @@ mod tests { .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() .build_and_sign().unwrap(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(!invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), Some(1000)); assert_eq!(tlv_stream.amount, None); @@ -1785,6 +1802,7 @@ mod tests { .quantity(2).unwrap() .build_and_sign().unwrap(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(!invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), Some(2000)); assert_eq!(tlv_stream.amount, None); @@ -1794,6 +1812,7 @@ mod tests { .request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap() .build_unchecked_and_sign(); let (_, _, tlv_stream, _, _, _) = invoice_request.as_tlv_stream(); + assert!(!invoice_request.has_amount_msats()); assert_eq!(invoice_request.amount_msats(), None); assert_eq!(tlv_stream.amount, None); } From be1a3abc194c749303f733d8f42d88d6ae4b84c5 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 13 Jan 2025 22:00:43 +0000 Subject: [PATCH 11/12] Add draft 0.1 release notes --- CHANGELOG.md | 174 ++++++++++++++++++ .../3137-channel-negotiation-type.txt | 7 - .../3243-remove-balance_msat.txt | 1 - pending_changelog/3259-no-downgrade.txt | 4 - pending_changelog/3322-a.txt | 6 - pending_changelog/3322-b.txt | 7 - ...e-counterparty-id-in-payment-forwarded.txt | 7 - .../3383-deprecate-old-inbounds.txt | 6 - ...3435-authenticate-payment-receive-tlvs.txt | 9 - .../3439-remove-accept-mpp-keysend-cfg.txt | 3 - pending_changelog/matt-no-upgrade-skip.txt | 6 - .../matt-persist-preimage-on-upgrade.txt | 8 - 12 files changed, 174 insertions(+), 64 deletions(-) delete mode 100644 pending_changelog/3137-channel-negotiation-type.txt delete mode 100644 pending_changelog/3243-remove-balance_msat.txt delete mode 100644 pending_changelog/3259-no-downgrade.txt delete mode 100644 pending_changelog/3322-a.txt delete mode 100644 pending_changelog/3322-b.txt delete mode 100644 pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt delete mode 100644 pending_changelog/3383-deprecate-old-inbounds.txt delete mode 100644 pending_changelog/3435-authenticate-payment-receive-tlvs.txt delete mode 100644 pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt delete mode 100644 pending_changelog/matt-no-upgrade-skip.txt delete mode 100644 pending_changelog/matt-persist-preimage-on-upgrade.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 6df2b2c2133..3400ab642b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,177 @@ +# 0.1 - Jan XXX, 2025 - XXX + +## API Updates + * The `lightning-liquidity` crate has been moved into the `rust-lightning` + git tree, enabling support for both sides of the LSPS channel open + negotiation protocols (#3436). + * Since its last alpha release, `lightning-liquidity` has also gained support + for acting as an LSPS1 client (#3436). + * This release includes support for BIP 353 Human Readable Names resolution. + With the `dnssec` feature enabled, simply call `ChannelManager`'s + `pay_for_offer_from_human_readable_name` with a list of lightning nodes that + have the `dns_resolver` feature flag set (e.g. those running LDK with the + new `lightning_dns_resolver::OMDomainResolver` set up to resolve DNS queries + for others) and a Human Readable Name (#3346, #3179, #3283). + * Asynchronous `ChannelMonitorUpdate` persistence (i.e. the use of + `ChannelMonitorUpdateStatus::InProgress`) is now considered beta-quality. + There are no known issues with it, though the likelihood of unknown issues + is high (#3414). + * `ChannelManager`'s `send_payment_with_route` and `send_spontaneous_payment` + were removed. Use `send_payment` and `send_spontaneous_payment_with_retry` + (now renamed `send_spontaneous_payment`) instead (#3430). + * `ChannelMonitor`s no longer need to be re-persisted after deserializing the + `ChannelManager` before beginning normal operation. As such, + `ChannelManagerReadArgs::channel_monitors` no longer requires mutable + references (#3322). See the Backwards Compatibility section for more info. + * Additional information is now stored in `ChannelMonitorUpdate`s which may + increase the average size of `ChannelMonitorUpdate`s when claiming inbound + payments substantially. The expected maximum size of `ChannelMonitorUpdate`s + shouldn't change materially (#3322). + * Redundant `Event::PaymentClaimed`s will be generated more frequently on + startup compared to previous versions. + `Event::PaymentClaim{able,ed}::payment_id` has been added to allow for more + robust handling of redundant events on payments with duplicate + `PaymentHash`es (#3303, #3322). + * `ChannelMonitorUpdate::update_id`s no longer have a magic value (of + `u64::MAX`) for updates after a channel has been closed. They are now + always monotonically increasing (#3355). + * The MSRV of `lightning-transaction-sync` has been increased to rustc 1.75 due + to its HTTP client dependencies (#3528). + * The default `ProbabilisticScoringFeeParameters` values now recommend specific + ratios between different penalties, and default penalties now allow for + higher fees in order to reduce payment latency (#3495). + * On-chain state resolution now more aggressively batches claims into single + transactions, reducing on-chain fee costs when resolving multiple HTLCs for a + single channel force-closure. This also reduces the on-chain reserve + requirements for nodes using anchor channels (#3340). + * A `MigratableKVStore` trait was added (and implemented for + `FilesystemStore`), enabling easy migration between `KVStore`s (#3481). + * `InvoiceRequest::amount_msats` now returns the `offer`-implied amount if a + Bitcoin-denominated amount was set in the `offer` and no amount was set + directly in the `invoice_request` (#3535). + * `Event::OpenChannelRequest::push_msat` has been replaced with an enum in + preparation for the dual-funding protocol coming in a future release (#3137). + * `GossipVerifier` now requires a `P2PGossipSync` which holds a reference to + the `GossipVerifier` via an `Arc` (#3432). + * The `max_level_*` features were removed as the performance gain compared to + doing the limiting at runtime was negligible (#3431). + * `ChannelManager::create_bolt11_invoice` was added, deprecating the + `lightning::ln::invoice_utils` module (#3389). + * The `bech32` dependency has been upgraded to 0.11 across crates (#3270). + * Support for creating BOLT 12 `invoice_request`s with a static signing key + rather than an ephemeral one has been removed (#3264). + * The `Router` trait no longer extends the `MessageRouter` trait, creating an + extra argument to `ChannelManager` construction (#3326). + * The deprecated `AvailableBalances::balance_msat` has been removed in favor of + `ChannelMonitor::get_claimable_balances` (#3243). + * Deprecated re-exports of `Payment{Hash,Preimage,Secret}` and `features` were + removed (#3359). + * `bolt11_payment::*_from_zero_amount_invoice` methods were renamed + `*_from_variable_amount_invoice` (#3397) + * Offer `signing_pubkey` (and related struct names) have been renamed + `issuer_signing_pubkey` (#3218). + * `Event::PaymentForwarded::{prev,next}_node_id` were added (#3458). + * `Event::ChannelClosed::last_local_balance_msat` was added (#3235). + * `RoutingMessageHandler::handle_*` now all have a `node_id` argument (#3291). + * `lightning::util::persist::MonitorName` has been exposed (#3376). + * `ProbabilisticScorer::live_estimated_payment_success_probability` was added + (#3420) + * `EcdsaChannelSigner::sign_splicing_funding_input` was added to support an + eventual splicing feature (#3316). + * `{Payment,Offer}Id` now support lowercase-hex formatting (#3377). + +## Bug Fixes + * Fixed a rare case where a BOLT 12 payment may be made duplicatively if the + node crashes while processing a BOLT 12 `invoice` message (#3313). + * Fixed a bug where a malicious sender could cause a payment `Event` to be + generated with an `OfferId` using a payment with a lower amount than the + corresponding BOLT 12 offer would have required. The amount in the + `Event::Payment{Claimable,Claimed}` were still correct (#3435). + * The `ProbabilisticScorer` model and associated default scoring parameters + were tweaked to be more predictive of real-world results (#3368, #3495). + * `ProbabilisticScoringFeeParameters::base_penalty_amount_multiplier_msat` no + longer includes any pending HTLCs we already have through channels in the + graph, avoiding over-penalizing them in comparison to other channels (#3356). + * A `ChannelMonitor` will no longer be archived if a `MonitorEvent` containing + a preimage for another channel is pending. This fixes an issue where a + payment preimage needed for another channel claim is lost if events go + un-processed for 4038 blocks (#3450). + * `std` builds no longer send the full gossip state to peers that do not + request it (#3390). + * `lightning-block-sync` listeners now receive `block_connected` calls, rather + than always receiving `filtered_block_connected` calls (#3354). + * Fixed a bug where some transactions were broadcasted one block before their + locktime made them candidates for inclusion in the mempool (though they would + be automatically re-broadcasted later, #3453). + * `ChainMonitor` now persists `ChannelMonitor`s when their `Balance` set first + goes empty, making `ChannelMonitor` pruning more reliable on nodes that are + only online briefly (e.g. mobile nodes, #3442). + * BOLT 12 invoice requests now better handle intermittent internet connectivity + (e.g. on mobile devices with app interruptions, #3010). + * Broadcast-gossip `MessageSendEvent`s from the `ChannelMessageHandler` are now + delivered to peers even if the peer is behind in processing relayed gossip. + This ensures our own gossip propagates well even if we have very limited + upload bandwidth (#3142). + * Fixed a bug where calling `OutputSweeper::transactions_confirmed` with + transactions from anything but the latest block may have triggered a spurious + assertion in debug mode (#3524). + +## Performance Improvements + * LDK now verifies `channel_update` gossip messages without holding a lock, + allowing additional parallelism during gossip sync (#3310). + * LDK now checks if it already has certain gossip messages before verifying the + message signatures, reducing CPU usage during gossip sync after the first + startup (#3305). + +## Node Compatibility + * LDK now handles fields in the experimental range of BOLT 12 messages (#3237). + +## Backwards Compatibility + * Nodes with pending forwarded HTLCs or unclaimed payments cannot be + upgraded directly from 0.0.123 or earlier to 0.1. Instead, they must + first either resolve all pending HTLCs (including those pending + resolution on-chain), or run 0.0.124 or 0.0.125 and resolve any HTLCs that + were originally forwarded or received running 0.0.123 or earlier (#3355). + * `ChannelMonitor`s not being re-persisted after deserializing the + `ChannelManager` only applies to upgraded nodes *after* a startup with the + old semantics completes at least once. In other words, you must deserialize + the `ChannelManager` with an upgraded LDK, persist the `ChannelMonitor`s as + you would on pre-0.1 versions of LDK, then continue to normal startup once, + and for startups thereafter you can take advantage of the new semantics + avoiding redundant persistence on startup (#3322). + * Pending inbound payments paying a BOLT 12 `invoice` issued prior to upgrade + to LDK 0.1 will fail. Issued BOLT 12 `offer`s remain payable (#3435). + * `UserConfig::accept_mpp_keysend` was removed, thus the presence of pending + inbound MPP keysend payments will prevent downgrade to LDK 0.0.115 and + earlier (#3439). + * Inbound payments initialized using the removed + `ChannelManager::create_inbound_payment{,_for_hash}_legacy` API will no + longer be accepted by LDK 0.1 (#3383). + * Downgrading to prior versions of LDK after using `ChannelManager`'s + `unsafe_manual_funding_transaction_generated` may cause `ChannelManager` + deserialization to fail (#3259). + * `ChannelDetails` serialized with LDK 0.1+ read with versions prior to 0.1 + will have `balance_msat` equal to `next_outbound_htlc_limit_msat` (#3243). + +## Security +0.1 fixes a funds-theft vulnerability when paying BOLT 12 offers as well as a +funds-lockup denial-of-service issue for anchor channels. + * When paying a BOLT 12 offer, if the recipient responds to our + `invoice_request` with an `invoice` which had an amount different from the + amount we intended to pay (either from the `offer` or the `amount_msats` + passed to `ChannelManager::pay_for_offer`), LDK would pay the amount from the + `invoice`. As a result, a malicious recipient could cause us to overpay the + amount we intended to pay (#3535). + * Fixed a bug where a counterparty can cause funds of ours to be locked up + by broadcasting a revoked commitment transaction and following HTLC + transactions in specific formats when using an anchor channel. The funds can + be recovered by upgrading to 0.1 and replaying the counterparty's broadcasted + transactions (using `Confirm::transactions_confirmed`) (#3537). Thanks to + Matt Morehouse for reporting and fixing this issue. + * Various denial-of-service issues in the formerly-alpha `lightning-liquidity` + crate have been addressed (#3436, #3493). + + # 0.0.125 - Oct 14, 2024 - "Delayed Beta Testing" ## Bug Fixes diff --git a/pending_changelog/3137-channel-negotiation-type.txt b/pending_changelog/3137-channel-negotiation-type.txt deleted file mode 100644 index 8eafa4e072b..00000000000 --- a/pending_changelog/3137-channel-negotiation-type.txt +++ /dev/null @@ -1,7 +0,0 @@ -# API Updates - * `Event::OpenChannelRequest::push_msat` has been replaced by the field `channel_negotiation_type` to - differentiate between an inbound request for a dual-funded (V2) or non-dual-funded (V1) channel to be - opened, with value being either of the enum variants `InboundChannelFunds::DualFunded` and - `InboundChannelFunds::PushMsat(u64)` corresponding to V2 and V1 channel open requests respectively. - This is in preparation for supporting accepting dual-funded channels, which will be available in a later release. - diff --git a/pending_changelog/3243-remove-balance_msat.txt b/pending_changelog/3243-remove-balance_msat.txt deleted file mode 100644 index 6378bd79054..00000000000 --- a/pending_changelog/3243-remove-balance_msat.txt +++ /dev/null @@ -1 +0,0 @@ -* The `AvailableBalances::balance_msat` field has been removed in favor of `ChainMonitor::get_claimable_balances`. `ChannelDetails` serialized with versions of LDK >= 0.0.125 will have their `balance_msat` field set to `next_outbound_htlc_limit_msat` when read by versions of LDK prior to 0.0.125 (#3243). diff --git a/pending_changelog/3259-no-downgrade.txt b/pending_changelog/3259-no-downgrade.txt deleted file mode 100644 index ed4193da480..00000000000 --- a/pending_changelog/3259-no-downgrade.txt +++ /dev/null @@ -1,4 +0,0 @@ -# Backwards Compatibility - * Downgrading after using `ChannelManager`'s - `unsafe_manual_funding_transaction_generated` may cause deserialization of - `ChannelManager` to fail with an `Err` (#3259). diff --git a/pending_changelog/3322-a.txt b/pending_changelog/3322-a.txt deleted file mode 100644 index 83849926a4e..00000000000 --- a/pending_changelog/3322-a.txt +++ /dev/null @@ -1,6 +0,0 @@ -API Changes -=========== - -Additional information is now stored in `ChannelMonitorUpdate`s which may increase the size of -`ChannelMonitorUpdate`s claiming inbound payments substantially. The expected maximum size of -`ChannelMonitorUpdate`s shouldn't change materially. diff --git a/pending_changelog/3322-b.txt b/pending_changelog/3322-b.txt deleted file mode 100644 index c8bb0c64bd9..00000000000 --- a/pending_changelog/3322-b.txt +++ /dev/null @@ -1,7 +0,0 @@ -API Updates -=========== - -As a part of adding robustness against several unlikely scenarios, redundant `PaymentClaimed` -`Event`s will be generated more frequently on startup for payments received on LDK 0.1 and -newer. A new `Event::PaymentClaimed::payment_id` field may be used to better differentiate -between redundant payments. diff --git a/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt b/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt deleted file mode 100644 index 8c2b1e338a6..00000000000 --- a/pending_changelog/3358-include-counterparty-id-in-payment-forwarded.txt +++ /dev/null @@ -1,7 +0,0 @@ -API Updates -=========== - -To improve clarity and uniqueness in identifying forwarded payments, the `PaymentForwarded` -event now includes counterparty node IDs alongside `ChannelIds`. This change resolves -potential ambiguity in cases like v1 0conf channel opens, where `ChannelIds` alone may not -be unique. \ No newline at end of file diff --git a/pending_changelog/3383-deprecate-old-inbounds.txt b/pending_changelog/3383-deprecate-old-inbounds.txt deleted file mode 100644 index 654cbcb5078..00000000000 --- a/pending_changelog/3383-deprecate-old-inbounds.txt +++ /dev/null @@ -1,6 +0,0 @@ -# Backwards Compatibility -* Pending inbound payments added in versions 0.0.116 or earlier using the - `create_inbound_payment{,_for_hash}_legacy` API will be ignored on `ChannelManager` - deserialization and fail to be received - - diff --git a/pending_changelog/3435-authenticate-payment-receive-tlvs.txt b/pending_changelog/3435-authenticate-payment-receive-tlvs.txt deleted file mode 100644 index 714bd00d8ce..00000000000 --- a/pending_changelog/3435-authenticate-payment-receive-tlvs.txt +++ /dev/null @@ -1,9 +0,0 @@ -## API Updates - * Payment `ReceiveTlvs` now contains an `authentication` field. It should be - set to `None` and then filled in with a nonce and the HMAC produced by - `ReceiveTlvs::hmac_for_offer_payment` when passing in the nonce (#3435). - -## Backwards Compatibility - * `ReceiveTlvs` for payments over `BlindedPaymentPath`s are now authenticated. - Any inbound payments for a preexisting `Bolt12Invoice` will therefore fail - (#3435). diff --git a/pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt b/pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt deleted file mode 100644 index f52d123d2ac..00000000000 --- a/pending_changelog/3439-remove-accept-mpp-keysend-cfg.txt +++ /dev/null @@ -1,3 +0,0 @@ -# Backwards Compatibility -* The presence of pending inbound MPP keysend payments breaks downgrades to LDK versions 0.0.115 and - earlier. diff --git a/pending_changelog/matt-no-upgrade-skip.txt b/pending_changelog/matt-no-upgrade-skip.txt deleted file mode 100644 index f5fcb8c5f25..00000000000 --- a/pending_changelog/matt-no-upgrade-skip.txt +++ /dev/null @@ -1,6 +0,0 @@ -## Backwards Compatibility - * Nodes with pending forwarded HTLCs or unclaimed payments cannot be - upgraded directly from 0.0.123 or earlier to 0.1. Instead, they must - first either resolve all pending HTLCs (including those pending - resolution on-chain), or run 0.0.124 and resolve any HTLCs that were - originally forwarded or received running 0.0.123 or earlier. diff --git a/pending_changelog/matt-persist-preimage-on-upgrade.txt b/pending_changelog/matt-persist-preimage-on-upgrade.txt deleted file mode 100644 index fc53469c6f6..00000000000 --- a/pending_changelog/matt-persist-preimage-on-upgrade.txt +++ /dev/null @@ -1,8 +0,0 @@ -# Backwards Compatibility - * The `ChannelManager` deserialization semantics no longer require that - `ChannelMonitor`s be re-persisted after `(BlockHash, ChannelManager)::read` - is called prior to normal node operation. This applies to upgraded nodes - only *after* a startup with the old semantics completes at least once. IOW, - you must deserialize the `ChannelManager` with upgraded LDK, persist the - `ChannelMonitor`s then continue to normal startup once, and thereafter you - may skip the `ChannelMonitor` persistence step. From a016cc92dd094466cfcc0614651799e51d4163bc Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 15 Jan 2025 14:12:36 -0500 Subject: [PATCH 12/12] Fix build for cfg(async_payments) Static invoices don't have an amount_msats field. --- lightning/src/offers/invoice.rs | 26 ++++++++++++++++++++++---- lightning/src/offers/invoice_macros.rs | 12 ++---------- lightning/src/offers/static_invoice.rs | 4 ++-- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index d6a0392dac2..75095e058e7 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -120,7 +120,7 @@ use crate::ln::inbound_payment::{ExpandedKey, IV_LEN}; use crate::ln::msgs::DecodeError; use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_methods_common}; #[cfg(test)] -use crate::offers::invoice_macros::invoice_builder_methods_test; +use crate::offers::invoice_macros::invoice_builder_methods_test_common; use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef}; use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE}; use crate::offers::nonce::Nonce; @@ -380,6 +380,20 @@ macro_rules! invoice_builder_methods { ( } } } +#[cfg(test)] +macro_rules! invoice_builder_methods_test { ( + $self: ident, $self_type: ty, $return_type: ty, $return_value: expr + $(, $self_mut: tt)? +) => { + #[cfg_attr(c_bindings, allow(dead_code))] + pub(crate) fn amount_msats_unchecked( + $($self_mut)* $self: $self_type, amount_msats: u64, + ) -> $return_type { + $self.invoice.fields_mut().amount_msats = amount_msats; + $return_value + } +} } + impl<'a> InvoiceBuilder<'a, ExplicitSigningPubkey> { invoice_explicit_signing_pubkey_builder_methods!(self, Self); } @@ -393,7 +407,9 @@ impl<'a, S: SigningPubkeyStrategy> InvoiceBuilder<'a, S> { invoice_builder_methods_common!(self, Self, self.invoice.fields_mut(), Self, self, Bolt12Invoice, mut); #[cfg(test)] - invoice_builder_methods_test!(self, Self, self.invoice.fields_mut(), Self, self, mut); + invoice_builder_methods_test!(self, Self, Self, self, mut); + #[cfg(test)] + invoice_builder_methods_test_common!(self, Self, self.invoice.fields_mut(), Self, self, mut); } #[cfg(all(c_bindings, not(test)))] @@ -408,7 +424,8 @@ impl<'a> InvoiceWithExplicitSigningPubkeyBuilder<'a> { invoice_explicit_signing_pubkey_builder_methods!(self, &mut Self); invoice_builder_methods!(self, &mut Self, &mut Self, self, ExplicitSigningPubkey); invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, Bolt12Invoice); - invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self); + invoice_builder_methods_test!(self, &mut Self, &mut Self, self); + invoice_builder_methods_test_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self); } #[cfg(all(c_bindings, not(test)))] @@ -423,7 +440,8 @@ impl<'a> InvoiceWithDerivedSigningPubkeyBuilder<'a> { invoice_derived_signing_pubkey_builder_methods!(self, &mut Self); invoice_builder_methods!(self, &mut Self, &mut Self, self, DerivedSigningPubkey); invoice_builder_methods_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self, Bolt12Invoice); - invoice_builder_methods_test!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self); + invoice_builder_methods_test!(self, &mut Self, &mut Self, self); + invoice_builder_methods_test_common!(self, &mut Self, self.invoice.fields_mut(), &mut Self, self); } #[cfg(c_bindings)] diff --git a/lightning/src/offers/invoice_macros.rs b/lightning/src/offers/invoice_macros.rs index dd75fe62504..2b276a37d29 100644 --- a/lightning/src/offers/invoice_macros.rs +++ b/lightning/src/offers/invoice_macros.rs @@ -83,18 +83,10 @@ macro_rules! invoice_builder_methods_common { ( } } #[cfg(test)] -macro_rules! invoice_builder_methods_test { ( +macro_rules! invoice_builder_methods_test_common { ( $self: ident, $self_type: ty, $invoice_fields: expr, $return_type: ty, $return_value: expr $(, $self_mut: tt)? ) => { - #[cfg_attr(c_bindings, allow(dead_code))] - pub(crate) fn amount_msats_unchecked( - $($self_mut)* $self: $self_type, amount_msats: u64, - ) -> $return_type { - $invoice_fields.amount_msats = amount_msats; - $return_value - } - #[cfg_attr(c_bindings, allow(dead_code))] pub(crate) fn features_unchecked( $($self_mut)* $self: $self_type, features: Bolt12InvoiceFeatures @@ -154,4 +146,4 @@ macro_rules! invoice_accessors_common { ($self: ident, $contents: expr, $invoice pub(super) use invoice_accessors_common; pub(super) use invoice_builder_methods_common; #[cfg(test)] -pub(super) use invoice_builder_methods_test; +pub(super) use invoice_builder_methods_test_common; diff --git a/lightning/src/offers/static_invoice.rs b/lightning/src/offers/static_invoice.rs index 39c17eb3bcc..411ba3ff272 100644 --- a/lightning/src/offers/static_invoice.rs +++ b/lightning/src/offers/static_invoice.rs @@ -20,7 +20,7 @@ use crate::offers::invoice::{ InvoiceTlvStream, InvoiceTlvStreamRef, }; #[cfg(test)] -use crate::offers::invoice_macros::invoice_builder_methods_test; +use crate::offers::invoice_macros::invoice_builder_methods_test_common; use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_methods_common}; use crate::offers::invoice_request::InvoiceRequest; use crate::offers::merkle::{ @@ -174,7 +174,7 @@ impl<'a> StaticInvoiceBuilder<'a> { invoice_builder_methods_common!(self, Self, self.invoice, Self, self, StaticInvoice, mut); #[cfg(test)] - invoice_builder_methods_test!(self, Self, self.invoice, Self, self, mut); + invoice_builder_methods_test_common!(self, Self, self.invoice, Self, self, mut); } /// A semantically valid [`StaticInvoice`] that hasn't been signed.