Merge pull request #2822 from TheBlueMatt/2024-01-pm-dyn-ref
authorElias Rohrer <dev@tnull.de>
Thu, 11 Jan 2024 17:41:01 +0000 (18:41 +0100)
committerGitHub <noreply@github.com>
Thu, 11 Jan 2024 17:41:01 +0000 (18:41 +0100)
Drop `PeerManager` type bound on `UtxoLookup` entirely

42 files changed:
.github/workflows/build.yml
ci/check-cfg-flags.py
ci/ci-tests.sh
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
fuzz/src/onion_message.rs
lightning-background-processor/src/lib.rs
lightning-block-sync/src/convert.rs
lightning-invoice/src/lib.rs
lightning-invoice/src/payment.rs
lightning-invoice/src/sync.rs [deleted file]
lightning-invoice/src/utils.rs
lightning-persister/src/fs_store.rs
lightning-transaction-sync/Cargo.toml
lightning-transaction-sync/src/electrum.rs
lightning-transaction-sync/src/esplora.rs
lightning/src/blinded_path/message.rs
lightning/src/blinded_path/utils.rs
lightning/src/events/mod.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/channel.rs
lightning/src/ln/channel_keys.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/features.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/msgs.rs
lightning/src/ln/onion_payment.rs
lightning/src/ln/onion_utils.rs
lightning/src/ln/outbound_payment.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/peer_handler.rs
lightning/src/ln/reload_tests.rs
lightning/src/onion_message/functional_tests.rs
lightning/src/onion_message/messenger.rs
lightning/src/onion_message/mod.rs
lightning/src/onion_message/offers.rs
lightning/src/routing/router.rs
lightning/src/routing/scoring.rs
lightning/src/sync/nostd_sync.rs
lightning/src/util/crypto.rs
lightning/src/util/test_utils.rs

index 65be3faef8b5578e0e44b84a6e3651be360af01f..ad5e1fc517bf4e5a706cbc90070ef407407e9cb5 100644 (file)
@@ -39,6 +39,9 @@ jobs:
         run: |
           sudo apt-get -y install shellcheck
           shellcheck ci/ci-tests.sh
+      - 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_MINIMIZE_DISK_USAGE=1 ./ci/ci-tests.sh
index 02b598cd447d26a5577883cf17382cd5a2685ea3..277ae1077baad7052fe379f1f8cb5cb1e1ff6f41 100755 (executable)
@@ -39,6 +39,8 @@ def check_feature(feature):
         pass
     elif feature == "electrum":
         pass
+    elif feature == "time":
+        pass
     elif feature == "_test_utils":
         pass
     elif feature == "_test_vectors":
index ca8e20f07885b2ccd84c7967b5eb89fedd68f46a..3eccc48798dc6e4b5f3fccd0b0c29003be23b0c8 100755 (executable)
@@ -94,14 +94,14 @@ if [[ "$HOST_PLATFORM" != *windows* ]]; then
 
        DOWNLOAD_ELECTRS_AND_BITCOIND
 
-       RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-blocking
-       RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-blocking
-       RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-async
-       RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-async
-       RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features esplora-async-https
-       RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features esplora-async-https
-       RUSTFLAGS="--cfg no_download" cargo test --verbose --color always --features electrum
-       RUSTFLAGS="--cfg no_download" cargo check --verbose --color always --features electrum
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-blocking
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-blocking
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-async
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-async
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features esplora-async-https
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features esplora-async-https
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo test --verbose --color always --features electrum
+       RUSTFLAGS="$RUSTFLAGS --cfg no_download" cargo check --verbose --color always --features electrum
 
        popd
 fi
@@ -125,7 +125,7 @@ popd
 echo -e "\n\nBuilding with all Log-Limiting features"
 pushd lightning
 grep '^max_level_' Cargo.toml | awk '{ print $1 }'| while read -r FEATURE; do
-       cargo check --verbose --color always --features "$FEATURE"
+       RUSTFLAGS="$RUSTFLAGS -A unused_variables -A unused_macros -A unused_imports -A dead_code" cargo check --verbose --color always --features "$FEATURE"
 done
 popd
 
@@ -138,9 +138,9 @@ done
 
 for DIR in lightning lightning-invoice lightning-rapid-gossip-sync; do
        # check if there is a conflict between no-std and the c_bindings cfg
-       RUSTFLAGS="--cfg=c_bindings" cargo test -p $DIR --verbose --color always --no-default-features --features=no-std
+       RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test -p $DIR --verbose --color always --no-default-features --features=no-std
 done
-RUSTFLAGS="--cfg=c_bindings" cargo test --verbose --color always
+RUSTFLAGS="$RUSTFLAGS --cfg=c_bindings" cargo test --verbose --color always
 
 # Note that outbound_commitment_test only runs in this mode because of hardcoded signature values
 pushd lightning
@@ -174,5 +174,5 @@ if [ -f "$(which arm-none-eabi-gcc)" ]; then
 fi
 
 echo -e "\n\nTest cfg-flag builds"
-RUSTFLAGS="$RUSTFLAGS --cfg=taproot" cargo test --verbose --color always -p lightning
-RUSTFLAGS="$RUSTFLAGS --cfg=async_signing" cargo test --verbose --color always -p lightning
+RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
+RUSTFLAGS="--cfg=async_signing" cargo test --verbose --color always -p lightning
index 7a32434b86fff16dfcd58e76e9495095c6c4098f..89ff07fe698268a9f8fff51f1b298c124b46e73b 100644 (file)
@@ -48,7 +48,7 @@ use lightning::ln::script::ShutdownScript;
 use lightning::ln::functional_test_utils::*;
 use lightning::offers::invoice::{BlindedPayInfo, UnsignedBolt12Invoice};
 use lightning::offers::invoice_request::UnsignedInvoiceRequest;
-use lightning::onion_message::{Destination, MessageRouter, OnionMessagePath};
+use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath};
 use lightning::util::test_channel_signer::{TestChannelSigner, EnforcementState};
 use lightning::util::errors::APIError;
 use lightning::util::logger::Logger;
@@ -1289,6 +1289,94 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out, anchors: bool) {
                        },
                        0x89 => { fee_est_c.ret_val.store(253, atomic::Ordering::Release); nodes[2].maybe_update_chan_fees(); },
 
+                       0xf0 => {
+                               let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.get(0) {
+                                       monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[0].process_monitor_events();
+                       }
+                       0xf1 => {
+                               let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.get(1) {
+                                       monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[0].process_monitor_events();
+                       }
+                       0xf2 => {
+                               let pending_updates = monitor_a.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.last() {
+                                       monitor_a.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[0].process_monitor_events();
+                       }
+
+                       0xf4 => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.get(0) {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+                       0xf5 => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.get(1) {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+                       0xf6 => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_1_funding).unwrap();
+                               if let Some(id) = pending_updates.last() {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_1_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+
+                       0xf8 => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.get(0) {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+                       0xf9 => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.get(1) {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+                       0xfa => {
+                               let pending_updates = monitor_b.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.last() {
+                                       monitor_b.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[1].process_monitor_events();
+                       }
+
+                       0xfc => {
+                               let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.get(0) {
+                                       monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[2].process_monitor_events();
+                       }
+                       0xfd => {
+                               let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.get(1) {
+                                       monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[2].process_monitor_events();
+                       }
+                       0xfe => {
+                               let pending_updates = monitor_c.chain_monitor.list_pending_monitor_updates().remove(&chan_2_funding).unwrap();
+                               if let Some(id) = pending_updates.last() {
+                                       monitor_c.chain_monitor.channel_monitor_updated(chan_2_funding, *id).unwrap();
+                               }
+                               nodes[2].process_monitor_events();
+                       }
+
                        0xff => {
                                // Test that no channel is in a stuck state where neither party can send funds even
                                // after we resolve all pending events.
index a2ce98cf4d22c9bebaf40ff895b98f88d7b8da2e..1f5ceb2123517a5b14559ee5e82adc085fe576b7 100644 (file)
@@ -45,7 +45,7 @@ use lightning::ln::script::ShutdownScript;
 use lightning::ln::functional_test_utils::*;
 use lightning::offers::invoice::{BlindedPayInfo, UnsignedBolt12Invoice};
 use lightning::offers::invoice_request::UnsignedInvoiceRequest;
-use lightning::onion_message::{Destination, MessageRouter, OnionMessagePath};
+use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath};
 use lightning::routing::gossip::{P2PGossipSync, NetworkGraph};
 use lightning::routing::utxo::UtxoLookup;
 use lightning::routing::router::{InFlightHtlcs, PaymentParameters, Route, RouteParameters, Router};
index d2d60cfcf640593e2e38d264eac6eeb0c2abafad..4ad770fc20c8baa4e2a7a2cd1da8df3ff814a818 100644 (file)
@@ -16,7 +16,9 @@ use lightning::sign::{Recipient, KeyMaterial, EntropySource, NodeSigner, SignerP
 use lightning::util::test_channel_signer::TestChannelSigner;
 use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, Writeable, Writer};
-use lightning::onion_message::{CustomOnionMessageHandler, Destination, MessageRouter, OffersMessage, OffersMessageHandler, OnionMessageContents, OnionMessagePath, OnionMessenger, PendingOnionMessage};
+use lightning::onion_message::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage};
+use lightning::onion_message::offers::{OffersMessage, OffersMessageHandler};
+use lightning::onion_message::packet::OnionMessageContents;
 
 use crate::utils::test_logger;
 
index a12ec9c0f3b8b85a868e9f2e3fa05f0abc460b3d..0f2c67538d65de59acb37da608ca0f48da78f227 100644 (file)
@@ -27,7 +27,10 @@ use lightning::chain::chainmonitor::{ChainMonitor, Persist};
 use lightning::sign::{EntropySource, NodeSigner, SignerProvider};
 use lightning::events::{Event, PathFailure};
 #[cfg(feature = "std")]
-use lightning::events::{EventHandler, EventsProvider};
+use lightning::events::EventHandler;
+#[cfg(any(feature = "std", feature = "futures"))]
+use lightning::events::EventsProvider;
+
 use lightning::ln::channelmanager::ChannelManager;
 use lightning::ln::msgs::OnionMessageHandler;
 use lightning::ln::peer_handler::APeerManager;
@@ -727,8 +730,6 @@ async fn process_onion_message_handler_events_async<
 where
        PM::Target: APeerManager + Send + Sync,
 {
-       use lightning::events::EventsProvider;
-
        let events = core::cell::RefCell::new(Vec::new());
        peer_manager.onion_message_handler().process_pending_events(&|e| events.borrow_mut().push(e));
 
index 62b0d6e47cbbd303e19fde760b57d22339423009..ed811d2cc0c3f629e16a450937c58d8287f35689 100644 (file)
@@ -247,10 +247,12 @@ impl TryInto<BlockHash> for JsonResponse {
 /// The REST `getutxos` endpoint retuns a whole pile of data we don't care about and one bit we do
 /// - whether the `hit bitmap` field had any entries. Thus we condense the result down into only
 /// that.
+#[cfg(feature = "rest-client")]
 pub(crate) struct GetUtxosResponse {
        pub(crate) hit_bitmap_nonempty: bool
 }
 
+#[cfg(feature = "rest-client")]
 impl TryInto<GetUtxosResponse> for JsonResponse {
        type Error = std::io::Error;
 
index 690bf57640862daae0adb55019ab48d90a2384a0..df0412bfc5d3e0d3876b177200527e01fd272678 100644 (file)
@@ -77,6 +77,7 @@ mod de;
 mod ser;
 mod tb;
 
+#[allow(unused_imports)]
 mod prelude {
        #[cfg(feature = "hashbrown")]
        extern crate hashbrown;
@@ -92,10 +93,6 @@ mod prelude {
 
 use crate::prelude::*;
 
-/// Sync compat for std/no_std
-#[cfg(not(feature = "std"))]
-mod sync;
-
 /// Errors that indicate what is wrong with the invoice. They have some granularity for debug
 /// reasons, but should generally result in an "invalid BOLT11 invoice" message for the user.
 #[allow(missing_docs)]
@@ -2049,7 +2046,7 @@ mod test {
                use lightning::routing::router::RouteHintHop;
                use secp256k1::Secp256k1;
                use secp256k1::{SecretKey, PublicKey};
-               use std::time::{UNIX_EPOCH, Duration};
+               use std::time::Duration;
 
                let secp_ctx = Secp256k1::new();
 
@@ -2138,7 +2135,7 @@ mod test {
                assert_eq!(invoice.currency(), Currency::BitcoinTestnet);
                #[cfg(feature = "std")]
                assert_eq!(
-                       invoice.timestamp().duration_since(UNIX_EPOCH).unwrap().as_secs(),
+                       invoice.timestamp().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs(),
                        1234567
                );
                assert_eq!(invoice.payee_pub_key(), Some(&public_key));
index b8f564ef573bf9b37dba86350bf73df745f42b0a..8196fa9eb89a81d7df74300484ee96cddee5588f 100644 (file)
@@ -85,14 +85,12 @@ mod tests {
        use super::*;
        use crate::{InvoiceBuilder, Currency};
        use bitcoin::hashes::sha256::Hash as Sha256;
-       use lightning::events::Event;
-       use lightning::ln::channelmanager::{Retry, PaymentId};
-       use lightning::ln::msgs::ChannelMessageHandler;
        use lightning::ln::PaymentSecret;
-       use lightning::ln::functional_test_utils::*;
        use lightning::routing::router::Payee;
        use secp256k1::{SecretKey, PublicKey, Secp256k1};
-       use std::time::{SystemTime, Duration};
+       use core::time::Duration;
+       #[cfg(feature = "std")]
+       use std::time::SystemTime;
 
        fn duration_since_epoch() -> Duration {
                #[cfg(feature = "std")]
@@ -171,6 +169,10 @@ mod tests {
        #[test]
        #[cfg(feature = "std")]
        fn payment_metadata_end_to_end() {
+               use lightning::events::Event;
+               use lightning::ln::channelmanager::{Retry, PaymentId};
+               use lightning::ln::msgs::ChannelMessageHandler;
+               use lightning::ln::functional_test_utils::*;
                // Test that a payment metadata read from an invoice passed to `pay_invoice` makes it all
                // the way out through the `PaymentClaimable` event.
                let chanmon_cfgs = create_chanmon_cfgs(2);
diff --git a/lightning-invoice/src/sync.rs b/lightning-invoice/src/sync.rs
deleted file mode 100644 (file)
index fae923f..0000000
+++ /dev/null
@@ -1,37 +0,0 @@
-use core::cell::{RefCell, RefMut};
-use core::ops::{Deref, DerefMut};
-
-pub type LockResult<Guard> = Result<Guard, ()>;
-
-pub struct Mutex<T: ?Sized> {
-       inner: RefCell<T>
-}
-
-#[must_use = "if unused the Mutex will immediately unlock"]
-pub struct MutexGuard<'a, T: ?Sized + 'a> {
-       lock: RefMut<'a, T>,
-}
-
-impl<T: ?Sized> Deref for MutexGuard<'_, T> {
-       type Target = T;
-
-       fn deref(&self) -> &T {
-               &self.lock.deref()
-       }
-}
-
-impl<T: ?Sized> DerefMut for MutexGuard<'_, T> {
-       fn deref_mut(&mut self) -> &mut T {
-               self.lock.deref_mut()
-       }
-}
-
-impl<T> Mutex<T> {
-       pub fn new(inner: T) -> Mutex<T> {
-               Mutex { inner: RefCell::new(inner) }
-       }
-
-       pub fn lock<'a>(&'a self) -> LockResult<MutexGuard<'a, T>> {
-               Ok(MutexGuard { lock: self.inner.borrow_mut() })
-       }
-}
index 3a337a9a4b964dea101ee04ea5f6346b4b68fe27..5e8b72467e5da655cd9f77876ac692b948498833 100644 (file)
@@ -816,14 +816,15 @@ impl<'a, 'b, L: Deref> WithChannelDetails<'a, 'b, L> where L::Target: Logger {
 
 #[cfg(test)]
 mod test {
-       use core::cell::RefCell;
        use core::time::Duration;
        use crate::{Currency, Description, Bolt11InvoiceDescription, SignOrCreationError, CreationError};
        use bitcoin::hashes::{Hash, sha256};
        use bitcoin::hashes::sha256::Hash as Sha256;
        use lightning::sign::PhantomKeysManager;
-       use lightning::events::{MessageSendEvent, MessageSendEventsProvider, Event, EventsProvider};
-       use lightning::ln::{PaymentPreimage, PaymentHash};
+       use lightning::events::{MessageSendEvent, MessageSendEventsProvider};
+       use lightning::ln::PaymentHash;
+       #[cfg(feature = "std")]
+       use lightning::ln::PaymentPreimage;
        use lightning::ln::channelmanager::{PhantomRouteHints, MIN_FINAL_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields, Retry};
        use lightning::ln::functional_test_utils::*;
        use lightning::ln::msgs::ChannelMessageHandler;
@@ -1294,6 +1295,9 @@ mod test {
 
        #[cfg(feature = "std")]
        fn do_test_multi_node_receive(user_generated_pmt_hash: bool) {
+               use lightning::events::{Event, EventsProvider};
+               use core::cell::RefCell;
+
                let mut chanmon_cfgs = create_chanmon_cfgs(3);
                let seed_1 = [42u8; 32];
                let seed_2 = [43u8; 32];
index 118cf9af7ba0e9554ca23514534007ca8c824e0a..b5c6526207df007f10f33df3deac57629015323e 100644 (file)
@@ -381,11 +381,6 @@ mod tests {
        use lightning::util::persist::read_channel_monitors;
        use std::fs;
        use std::str::FromStr;
-       #[cfg(target_os = "windows")]
-       use {
-               lightning::get_event_msg,
-               lightning::ln::msgs::ChannelMessageHandler,
-       };
 
        impl Drop for FilesystemStore {
                fn drop(&mut self) {
index 8cf3e53c416ba1d9295a3a0a49f19969faf7d0ec..a2630400fb33bb690dea8941495ce5493ed7a8d3 100644 (file)
@@ -14,7 +14,8 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [features]
-default = []
+default = ["time"]
+time = []
 esplora-async = ["async-interface", "esplora-client/async", "futures"]
 esplora-async-https = ["esplora-async", "esplora-client/async-https-rustls"]
 esplora-blocking = ["esplora-client/blocking"]
index 07e11338905370d9385dc7d5259053b624055ea4..d0c8afef77e3161ec191f41a0ceae4aa5eb8efe0 100644 (file)
@@ -86,6 +86,7 @@ where
                let mut sync_state = self.sync_state.lock().unwrap();
 
                log_trace!(self.logger, "Starting transaction sync.");
+               #[cfg(feature = "time")]
                let start_time = Instant::now();
                let mut num_confirmed = 0;
                let mut num_unconfirmed = 0;
@@ -210,10 +211,15 @@ where
                                sync_state.pending_sync = false;
                        }
                }
+               #[cfg(feature = "time")]
                log_debug!(self.logger,
                        "Finished transaction sync at tip {} in {}ms: {} confirmed, {} unconfirmed.",
                        tip_header.block_hash(), start_time.elapsed().as_millis(), num_confirmed,
                        num_unconfirmed);
+               #[cfg(not(feature = "time"))]
+               log_debug!(self.logger,
+                       "Finished transaction sync at tip {}: {} confirmed, {} unconfirmed.",
+                       tip_header.block_hash(), num_confirmed, num_unconfirmed);
                Ok(())
        }
 
index 953f8b0718c3526cff701142c17a0a5d7e32e339..eb52faf33648cfb173985b13c29ecb9d754fdb87 100644 (file)
@@ -14,7 +14,6 @@ use esplora_client::r#async::AsyncClient;
 #[cfg(not(feature = "async-interface"))]
 use esplora_client::blocking::BlockingClient;
 
-use std::time::Instant;
 use std::collections::HashSet;
 use core::ops::Deref;
 
@@ -91,7 +90,8 @@ where
                let mut sync_state = self.sync_state.lock().await;
 
                log_trace!(self.logger, "Starting transaction sync.");
-               let start_time = Instant::now();
+               #[cfg(feature = "time")]
+               let start_time = std::time::Instant::now();
                let mut num_confirmed = 0;
                let mut num_unconfirmed = 0;
 
@@ -227,8 +227,12 @@ where
                                sync_state.pending_sync = false;
                        }
                }
+               #[cfg(feature = "time")]
                log_debug!(self.logger, "Finished transaction sync at tip {} in {}ms: {} confirmed, {} unconfirmed.",
                                tip_hash, start_time.elapsed().as_millis(), num_confirmed, num_unconfirmed);
+               #[cfg(not(feature = "time"))]
+               log_debug!(self.logger, "Finished transaction sync at tip {}: {} confirmed, {} unconfirmed.",
+                               tip_hash, num_confirmed, num_unconfirmed);
                Ok(())
        }
 
index d2e81444ef6307a2cac41082c444f18568af7a85..2631a05f660c4e886a6db37d9e8fa3b6e76f6708 100644 (file)
@@ -5,7 +5,7 @@ use crate::blinded_path::utils;
 use crate::io;
 use crate::io::Cursor;
 use crate::ln::onion_utils;
-use crate::onion_message::ControlTlvs;
+use crate::onion_message::packet::ControlTlvs;
 use crate::prelude::*;
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::chacha20poly1305rfc::ChaChaPolyReadAdapter;
index 2e691898dc26cc187504fee5e774d4f8ce9a4af7..7ddb39c1b68fbce0a93fca0637e604b70b393b90 100644 (file)
@@ -18,7 +18,7 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
 use super::{BlindedHop, BlindedPath};
 use crate::ln::msgs::DecodeError;
 use crate::ln::onion_utils;
-use crate::onion_message::Destination;
+use crate::onion_message::messenger::Destination;
 use crate::util::chacha20poly1305rfc::ChaChaPolyWriteAdapter;
 use crate::util::ser::{Readable, Writeable};
 
index 76e5f25c0e5f6eef87b2ff48102dd19acc239231..dede02a37cb9c5e6349dfb27e7504cc3b1fb83fc 100644 (file)
@@ -540,8 +540,8 @@ pub enum Event {
        /// replies. Handlers should connect to the node otherwise any buffered messages may be lost.
        ///
        /// [`OnionMessage`]: msgs::OnionMessage
-       /// [`MessageRouter`]: crate::onion_message::MessageRouter
-       /// [`Destination`]: crate::onion_message::Destination
+       /// [`MessageRouter`]: crate::onion_message::messenger::MessageRouter
+       /// [`Destination`]: crate::onion_message::messenger::Destination
        /// [`OnionMessageHandler`]: crate::ln::msgs::OnionMessageHandler
        ConnectionNeeded {
                /// The node id for the node needing a connection.
index 3552748b31c0955bfe35e8302fcfae71ffd267b8..672e3aa862e460e94f26451ebff4ff39293eb4e9 100644 (file)
@@ -485,9 +485,11 @@ impl TxCreationKeys {
 }
 
 /// The maximum length of a script returned by get_revokeable_redeemscript.
-// Calculated as 6 bytes of opcodes, 1 byte push plus 2 bytes for contest_delay, and two public
-// keys of 33 bytes (+ 1 push).
-pub const REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH: usize = 6 + 3 + 34*2;
+// Calculated as 6 bytes of opcodes, 1 byte push plus 3 bytes for contest_delay, and two public
+// keys of 33 bytes (+ 1 push). Generally, pushes are only 2 bytes (for values below 0x7fff, i.e.
+// around 7 months), however, a 7 month contest delay shouldn't result in being unable to reclaim
+// on-chain funds.
+pub const REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH: usize = 6 + 4 + 34*2;
 
 /// A script either spendable by the revocation
 /// key or the broadcaster_delayed_payment_key and satisfying the relative-locktime OP_CSV constrain.
index 588995656ec4790cedd6bcb5ac7edc6bc190ae23..b3657701a4a4b876c80694bd2496e3662a78937a 100644 (file)
@@ -732,8 +732,8 @@ struct CommitmentStats<'a> {
        total_fee_sat: u64, // the total fee included in the transaction
        num_nondust_htlcs: usize,  // the number of HTLC outputs (dust HTLCs *non*-included)
        htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
-       local_balance_msat: u64, // local balance before fees but considering dust limits
-       remote_balance_msat: u64, // remote balance before fees but considering dust limits
+       local_balance_msat: u64, // local balance before fees *not* considering dust limits
+       remote_balance_msat: u64, // remote balance before fees *not* considering dust limits
        outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
        inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
 }
@@ -814,6 +814,7 @@ pub(super) struct ReestablishResponses {
 /// The result of a shutdown that should be handled.
 #[must_use]
 pub(crate) struct ShutdownResult {
+       pub(crate) closure_reason: ClosureReason,
        /// A channel monitor update to apply.
        pub(crate) monitor_update: Option<(PublicKey, OutPoint, ChannelMonitorUpdate)>,
        /// A list of dropped outbound HTLCs that can safely be failed backwards immediately.
@@ -822,7 +823,10 @@ pub(crate) struct ShutdownResult {
        /// propagated to the remainder of the batch.
        pub(crate) unbroadcasted_batch_funding_txid: Option<Txid>,
        pub(crate) channel_id: ChannelId,
+       pub(crate) user_channel_id: u128,
+       pub(crate) channel_capacity_satoshis: u64,
        pub(crate) counterparty_node_id: PublicKey,
+       pub(crate) unbroadcasted_funding_tx: Option<Transaction>,
 }
 
 /// If the majority of the channels funds are to the fundee and the initiator holds only just
@@ -1728,13 +1732,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        }
                }
 
-               let mut value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
+               let value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset;
                assert!(value_to_self_msat >= 0);
                // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie
                // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to
                // "violate" their reserve value by couting those against it. Thus, we have to convert
                // everything to i64 before subtracting as otherwise we can overflow.
-               let mut value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
+               let value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset;
                assert!(value_to_remote_msat >= 0);
 
                #[cfg(debug_assertions)]
@@ -1800,10 +1804,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
                htlcs_included.append(&mut included_dust_htlcs);
 
-               // For the stats, trimmed-to-0 the value in msats accordingly
-               value_to_self_msat = if (value_to_self_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_self_msat };
-               value_to_remote_msat = if (value_to_remote_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_remote_msat };
-
                CommitmentStats {
                        tx,
                        feerate_per_kw,
@@ -1837,8 +1837,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
        /// will sign and send to our counterparty.
        /// If an Err is returned, it is a ChannelError::Close (for get_funding_created)
        fn build_remote_transaction_keys(&self) -> TxCreationKeys {
-               //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
-               //may see payments to it!
                let revocation_basepoint = &self.get_holder_pubkeys().revocation_basepoint;
                let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
                let counterparty_pubkeys = self.get_counterparty_pubkeys();
@@ -1876,7 +1874,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                if let Some(feerate) = outbound_feerate_update {
                        feerate_per_kw = cmp::max(feerate_per_kw, feerate);
                }
-               cmp::max(2530, feerate_per_kw * 1250 / 1000)
+               let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000);
+               cmp::max(2530, feerate_plus_quarter.unwrap_or(u32::max_value()))
        }
 
        /// Get forwarding information for the counterparty.
@@ -2316,15 +2315,17 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                res
        }
 
-       fn if_unbroadcasted_funding<F, O>(&self, f: F) -> Option<O>
-               where F: Fn() -> Option<O> {
+       fn if_unbroadcasted_funding<F, O>(&self, f: F) -> Option<O> where F: Fn() -> Option<O> {
                match self.channel_state {
                        ChannelState::FundingNegotiated => f(),
-                       ChannelState::AwaitingChannelReady(flags) => if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) {
-                               f()
-                       } else {
-                               None
-                       },
+                       ChannelState::AwaitingChannelReady(flags) =>
+                               if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) ||
+                                       flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into())
+                               {
+                                       f()
+                               } else {
+                                       None
+                               },
                        _ => None,
                }
        }
@@ -2359,7 +2360,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
        /// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
        /// Also returns the list of payment_hashes for channels which we can safely fail backwards
        /// immediately (others we will have to allow to time out).
-       pub fn force_shutdown(&mut self, should_broadcast: bool) -> ShutdownResult {
+       pub fn force_shutdown(&mut self, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult {
                // Note that we MUST only generate a monitor update that indicates force-closure - we're
                // called during initialization prior to the chain_monitor in the encompassing ChannelManager
                // being fully configured in some cases. Thus, its likely any monitor events we generate will
@@ -2400,15 +2401,20 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        } else { None }
                } else { None };
                let unbroadcasted_batch_funding_txid = self.unbroadcasted_batch_funding_txid();
+               let unbroadcasted_funding_tx = self.unbroadcasted_funding();
 
                self.channel_state = ChannelState::ShutdownComplete;
                self.update_time_counter += 1;
                ShutdownResult {
+                       closure_reason,
                        monitor_update,
                        dropped_outbound_htlcs,
                        unbroadcasted_batch_funding_txid,
                        channel_id: self.channel_id,
+                       user_channel_id: self.user_id,
+                       channel_capacity_satoshis: self.channel_value_satoshis,
                        counterparty_node_id: self.counterparty_node_id,
+                       unbroadcasted_funding_tx,
                }
        }
 
@@ -2549,26 +2555,24 @@ impl FailHTLCContents for msgs::OnionErrorPacket {
                HTLCUpdateAwaitingACK::FailHTLC { htlc_id, err_packet: self }
        }
 }
-impl FailHTLCContents for (u16, [u8; 32]) {
-       type Message = msgs::UpdateFailMalformedHTLC; // (failure_code, sha256_of_onion)
+impl FailHTLCContents for ([u8; 32], u16) {
+       type Message = msgs::UpdateFailMalformedHTLC;
        fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
                msgs::UpdateFailMalformedHTLC {
                        htlc_id,
                        channel_id,
-                       failure_code: self.0,
-                       sha256_of_onion: self.1
+                       sha256_of_onion: self.0,
+                       failure_code: self.1
                }
        }
        fn to_inbound_htlc_state(self) -> InboundHTLCState {
-               InboundHTLCState::LocalRemoved(
-                       InboundHTLCRemovalReason::FailMalformed((self.1, self.0))
-               )
+               InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(self))
        }
        fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK {
                HTLCUpdateAwaitingACK::FailMalformedHTLC {
                        htlc_id,
-                       failure_code: self.0,
-                       sha256_of_onion: self.1
+                       sha256_of_onion: self.0,
+                       failure_code: self.1
                }
        }
 }
@@ -2895,7 +2899,7 @@ impl<SP: Deref> Channel<SP> where
        pub fn queue_fail_malformed_htlc<L: Deref>(
                &mut self, htlc_id_arg: u64, failure_code: u16, sha256_of_onion: [u8; 32], logger: &L
        ) -> Result<(), ChannelError> where L::Target: Logger {
-               self.fail_htlc(htlc_id_arg, (failure_code, sha256_of_onion), true, logger)
+               self.fail_htlc(htlc_id_arg, (sha256_of_onion, failure_code), true, logger)
                        .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
        }
 
@@ -2908,7 +2912,7 @@ impl<SP: Deref> Channel<SP> where
        /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be
        /// [`ChannelError::Ignore`].
        fn fail_htlc<L: Deref, E: FailHTLCContents + Clone>(
-               &mut self, htlc_id_arg: u64, err_packet: E, mut force_holding_cell: bool,
+               &mut self, htlc_id_arg: u64, err_contents: E, mut force_holding_cell: bool,
                logger: &L
        ) -> Result<Option<E::Message>, ChannelError> where L::Target: Logger {
                if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
@@ -2975,7 +2979,7 @@ impl<SP: Deref> Channel<SP> where
                                }
                        }
                        log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, &self.context.channel_id());
-                       self.context.holding_cell_htlc_updates.push(err_packet.to_htlc_update_awaiting_ack(htlc_id_arg));
+                       self.context.holding_cell_htlc_updates.push(err_contents.to_htlc_update_awaiting_ack(htlc_id_arg));
                        return Ok(None);
                }
 
@@ -2983,10 +2987,10 @@ impl<SP: Deref> Channel<SP> where
                        E::Message::name(), &self.context.channel_id());
                {
                        let htlc = &mut self.context.pending_inbound_htlcs[pending_idx];
-                       htlc.state = err_packet.clone().to_inbound_htlc_state();
+                       htlc.state = err_contents.clone().to_inbound_htlc_state();
                }
 
-               Ok(Some(err_packet.to_message(htlc_id_arg, self.context.channel_id())))
+               Ok(Some(err_contents.to_message(htlc_id_arg, self.context.channel_id())))
        }
 
        // Message handlers:
@@ -3599,7 +3603,7 @@ impl<SP: Deref> Channel<SP> where
                                // the limit. In case it's less rare than I anticipate, we may want to revisit
                                // handling this case better and maybe fulfilling some of the HTLCs while attempting
                                // to rebalance channels.
-                               match &htlc_update {
+                               let fail_htlc_res = match &htlc_update {
                                        &HTLCUpdateAwaitingACK::AddHTLC {
                                                amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet,
                                                skimmed_fee_msat, blinding_point, ..
@@ -3627,6 +3631,7 @@ impl<SP: Deref> Channel<SP> where
                                                                }
                                                        }
                                                }
+                                               None
                                        },
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
                                                // If an HTLC claim was previously added to the holding cell (via
@@ -3640,40 +3645,33 @@ impl<SP: Deref> Channel<SP> where
                                                        { monitor_update } else { unreachable!() };
                                                update_fulfill_count += 1;
                                                monitor_update.updates.append(&mut additional_monitor_update.updates);
+                                               None
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
-                                               match self.fail_htlc(htlc_id, err_packet.clone(), false, logger) {
-                                                       Ok(update_fail_msg_option) => {
-                                                               // If an HTLC failure was previously added to the holding cell (via
-                                                               // `queue_fail_htlc`) then generating the fail message itself must
-                                                               // not fail - we should never end up in a state where we double-fail
-                                                               // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
-                                                               // for a full revocation before failing.
-                                                               debug_assert!(update_fail_msg_option.is_some());
-                                                               update_fail_count += 1;
-                                                       },
-                                                       Err(e) => {
-                                                               if let ChannelError::Ignore(_) = e {}
-                                                               else {
-                                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
-                                                               }
-                                                       }
-                                               }
+                                               Some(self.fail_htlc(htlc_id, err_packet.clone(), false, logger)
+                                                .map(|fail_msg_opt| fail_msg_opt.map(|_| ())))
                                        },
                                        &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
-                                               match self.fail_htlc(htlc_id, (failure_code, sha256_of_onion), false, logger) {
-                                                       Ok(update_fail_malformed_opt) => {
-                                                               debug_assert!(update_fail_malformed_opt.is_some()); // See above comment
-                                                               update_fail_count += 1;
-                                                       },
-                                                       Err(e) => {
-                                                               if let ChannelError::Ignore(_) = e {}
-                                                               else {
-                                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
-                                                               }
-                                                       }
-                                               }
-                                       },
+                                               Some(self.fail_htlc(htlc_id, (sha256_of_onion, failure_code), false, logger)
+                                                .map(|fail_msg_opt| fail_msg_opt.map(|_| ())))
+                                       }
+                               };
+                               if let Some(res) = fail_htlc_res {
+                                       match res {
+                                               Ok(fail_msg_opt) => {
+                                                       // If an HTLC failure was previously added to the holding cell (via
+                                                       // `queue_fail_{malformed_}htlc`) then generating the fail message itself must
+                                                       // not fail - we should never end up in a state where we double-fail
+                                                       // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
+                                                       // for a full revocation before failing.
+                                                       debug_assert!(fail_msg_opt.is_some());
+                                                       update_fail_count += 1;
+                                               },
+                                               Err(ChannelError::Ignore(_)) => {},
+                                               Err(_) => {
+                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
+                                               },
+                                       }
                                }
                        }
                        if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() {
@@ -4936,11 +4934,15 @@ impl<SP: Deref> Channel<SP> where
                if let Some((last_fee, sig)) = self.context.last_sent_closing_fee {
                        if last_fee == msg.fee_satoshis {
                                let shutdown_result = ShutdownResult {
+                                       closure_reason: ClosureReason::CooperativeClosure,
                                        monitor_update: None,
                                        dropped_outbound_htlcs: Vec::new(),
                                        unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(),
                                        channel_id: self.context.channel_id,
+                                       user_channel_id: self.context.user_id,
+                                       channel_capacity_satoshis: self.context.channel_value_satoshis,
                                        counterparty_node_id: self.context.counterparty_node_id,
+                                       unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
                                };
                                let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
                                self.context.channel_state = ChannelState::ShutdownComplete;
@@ -4966,11 +4968,15 @@ impl<SP: Deref> Channel<SP> where
                                                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
                                                let (signed_tx, shutdown_result) = if $new_fee == msg.fee_satoshis {
                                                        let shutdown_result = ShutdownResult {
+                                                               closure_reason: ClosureReason::CooperativeClosure,
                                                                monitor_update: None,
                                                                dropped_outbound_htlcs: Vec::new(),
                                                                unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(),
                                                                channel_id: self.context.channel_id,
+                                                               user_channel_id: self.context.user_id,
+                                                               channel_capacity_satoshis: self.context.channel_value_satoshis,
                                                                counterparty_node_id: self.context.counterparty_node_id,
+                                                               unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
                                                        };
                                                        self.context.channel_state = ChannelState::ShutdownComplete;
                                                        self.context.update_time_counter += 1;
@@ -6848,6 +6854,41 @@ pub(super) struct InboundV1Channel<SP: Deref> where SP::Target: SignerProvider {
        pub unfunded_context: UnfundedChannelContext,
 }
 
+/// Fetches the [`ChannelTypeFeatures`] that will be used for a channel built from a given
+/// [`msgs::OpenChannel`].
+pub(super) fn channel_type_from_open_channel(
+       msg: &msgs::OpenChannel, their_features: &InitFeatures,
+       our_supported_features: &ChannelTypeFeatures
+) -> Result<ChannelTypeFeatures, ChannelError> {
+       if let Some(channel_type) = &msg.channel_type {
+               if channel_type.supports_any_optional_bits() {
+                       return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
+               }
+
+               // We only support the channel types defined by the `ChannelManager` in
+               // `provided_channel_type_features`. The channel type must always support
+               // `static_remote_key`.
+               if !channel_type.requires_static_remote_key() {
+                       return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
+               }
+               // Make sure we support all of the features behind the channel type.
+               if !channel_type.is_subset(our_supported_features) {
+                       return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
+               }
+               let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
+               if channel_type.requires_scid_privacy() && announced_channel {
+                       return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
+               }
+               Ok(channel_type.clone())
+       } else {
+               let channel_type = ChannelTypeFeatures::from_init(&their_features);
+               if channel_type != ChannelTypeFeatures::only_static_remote_key() {
+                       return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
+               }
+               Ok(channel_type)
+       }
+}
+
 impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
        /// Creates a new channel from a remote sides' request for one.
        /// Assumes chain_hash has already been checked and corresponds with what we expect!
@@ -6866,32 +6907,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
                // First check the channel type is known, failing before we do anything else if we don't
                // support this channel type.
-               let channel_type = if let Some(channel_type) = &msg.channel_type {
-                       if channel_type.supports_any_optional_bits() {
-                               return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
-                       }
-
-                       // We only support the channel types defined by the `ChannelManager` in
-                       // `provided_channel_type_features`. The channel type must always support
-                       // `static_remote_key`.
-                       if !channel_type.requires_static_remote_key() {
-                               return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
-                       }
-                       // Make sure we support all of the features behind the channel type.
-                       if !channel_type.is_subset(our_supported_features) {
-                               return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
-                       }
-                       if channel_type.requires_scid_privacy() && announced_channel {
-                               return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
-                       }
-                       channel_type.clone()
-               } else {
-                       let channel_type = ChannelTypeFeatures::from_init(&their_features);
-                       if channel_type != ChannelTypeFeatures::only_static_remote_key() {
-                               return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
-                       }
-                       channel_type
-               };
+               let channel_type = channel_type_from_open_channel(msg, their_features, our_supported_features)?;
 
                let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.funding_satoshis, user_id);
                let holder_signer = signer_provider.derive_channel_signer(msg.funding_satoshis, channel_keys_id);
index f737dd23407727786d669b5ca1cbc50776cc1251..b577dc60008583537c4d9c1cebc8b2f6e5f48e77 100644 (file)
@@ -25,126 +25,125 @@ use bitcoin::secp256k1::PublicKey;
 use bitcoin::hashes::sha256::Hash as Sha256;
 
 macro_rules! doc_comment {
-    ($x:expr, $($tt:tt)*) => {
-        #[doc = $x]
-        $($tt)*
-    };
+       ($x:expr, $($tt:tt)*) => {
+               #[doc = $x]
+               $($tt)*
+       };
 }
 macro_rules! basepoint_impl {
-    ($BasepointT:ty) => {
-        impl $BasepointT {
-            /// Get inner Public Key
-            pub fn to_public_key(&self) -> PublicKey {
-                self.0
-            }
-        }
-        
-        impl From<PublicKey> for $BasepointT {
-            fn from(value: PublicKey) -> Self {
-                Self(value)
-            }
-        }
-        
-    }
+       ($BasepointT:ty) => {
+               impl $BasepointT {
+                       /// Get inner Public Key
+                       pub fn to_public_key(&self) -> PublicKey {
+                               self.0
+                       }
+               }
+
+               impl From<PublicKey> for $BasepointT {
+                       fn from(value: PublicKey) -> Self {
+                               Self(value)
+                       }
+               }
+
+       }
 }
 macro_rules! key_impl {
-    ($BasepointT:ty, $KeyName:expr) => {
-        doc_comment! {
-            concat!("Generate ", $KeyName, " using per_commitment_point"),
-            pub fn from_basepoint<T: secp256k1::Signing>(
-                secp_ctx: &Secp256k1<T>,
-                basepoint: &$BasepointT,
-                per_commitment_point: &PublicKey,
-            ) -> Self {
-                Self(derive_public_key(secp_ctx, per_commitment_point, &basepoint.0))
-            }
-        }
-        
-        doc_comment! {
-            concat!("Generate ", $KeyName, " from privkey"),
-            pub fn from_secret_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, sk: &SecretKey) -> Self {
-                Self(PublicKey::from_secret_key(&secp_ctx, &sk))
-            }
-        }
-        
-        /// Get inner Public Key
-        pub fn to_public_key(&self) -> PublicKey {
-            self.0
-        }
-    }
+       ($BasepointT:ty, $KeyName:expr) => {
+               doc_comment! {
+                       concat!("Derive a public ", $KeyName, " using one node's `per_commitment_point` and its countersignatory's `basepoint`"),
+                       pub fn from_basepoint<T: secp256k1::Signing>(
+                               secp_ctx: &Secp256k1<T>,
+                               countersignatory_basepoint: &$BasepointT,
+                               per_commitment_point: &PublicKey,
+                       ) -> Self {
+                               Self(derive_public_key(secp_ctx, per_commitment_point, &countersignatory_basepoint.0))
+                       }
+               }
+
+               doc_comment! {
+                       concat!("Build a ", $KeyName, " directly from an already-derived private key"),
+                       pub fn from_secret_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, sk: &SecretKey) -> Self {
+                               Self(PublicKey::from_secret_key(&secp_ctx, &sk))
+                       }
+               }
+
+               /// Get inner Public Key
+               pub fn to_public_key(&self) -> PublicKey {
+                       self.0
+               }
+       }
 }
 macro_rules! key_read_write {
-    ($SelfT:ty) => {
-        impl Writeable for $SelfT {
-            fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-                self.0.serialize().write(w)
-            }
-        }
-        
-        impl Readable for $SelfT {
-            fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
-                let key: PublicKey = Readable::read(r)?;
-                Ok(Self(key))
-            }
-        }
-    }
+       ($SelfT:ty) => {
+               impl Writeable for $SelfT {
+                       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+                               self.0.serialize().write(w)
+                       }
+               }
+
+               impl Readable for $SelfT {
+                       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+                               let key: PublicKey = Readable::read(r)?;
+                               Ok(Self(key))
+                       }
+               }
+       }
 }
 
 
 
-/// Master key used in conjunction with per_commitment_point to generate [`local_delayedpubkey`](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
-/// A watcher can be given a [DelayedPaymentBasepoint] to generate per commitment [DelayedPaymentKey] to create justice transactions.
+/// Base key used in conjunction with a `per_commitment_point` to generate a [`DelayedPaymentKey`].
+///
+/// The delayed payment key is used to pay the commitment state broadcaster their
+/// non-HTLC-encumbered funds after a delay to give their counterparty a chance to punish if the
+/// state broadcasted was previously revoked.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct DelayedPaymentBasepoint(pub PublicKey);
 basepoint_impl!(DelayedPaymentBasepoint);
 key_read_write!(DelayedPaymentBasepoint);
 
-/// [delayedpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
-/// To allow a counterparty to contest a channel state published by a node, Lightning protocol sets delays for some of the outputs, before can be spend.
-/// For example a commitment transaction has to_local output encumbered by a delay, negotiated at the channel establishment flow.
-/// To spend from such output a node has to generate a script using, among others, a local delayed payment key.
+
+/// A derived key built from a [`DelayedPaymentBasepoint`] and `per_commitment_point`.
+///
+/// The delayed payment key is used to pay the commitment state broadcaster their
+/// non-HTLC-encumbered funds after a delay. This delay gives their counterparty a chance to
+/// punish and claim all the channel funds if the state broadcasted was previously revoked.
+///
+/// [See the BOLT specs]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
+/// for more information on key derivation details.
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct DelayedPaymentKey(pub PublicKey);
 
 impl DelayedPaymentKey {
-    key_impl!(DelayedPaymentBasepoint, "delayedpubkey");
+       key_impl!(DelayedPaymentBasepoint, "delayedpubkey");
 }
 key_read_write!(DelayedPaymentKey);
 
-/// Master key used in conjunction with per_commitment_point to generate a [localpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
-/// Also used to generate a commitment number in a commitment transaction or as a Payment Key for a remote node (not us) in an anchor output if `option_static_remotekey` is enabled.
-/// Shared by both nodes in a channel establishment message flow.
-#[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
-pub struct PaymentBasepoint(pub PublicKey);
-basepoint_impl!(PaymentBasepoint);
-key_read_write!(PaymentBasepoint);
-
-
-/// [localpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of a payment basepoint,
-/// that enables a secure hash-lock for off-chain payments without risk of funds getting stuck or stolen. A payment key is normally shared with a counterparty so that it can generate 
-/// a commitment transaction's to_remote ouput, which our node can claim in case the counterparty force closes the channel.
-#[derive(PartialEq, Eq, Clone, Copy, Debug)]
-pub struct PaymentKey(pub PublicKey);
-
-impl PaymentKey {
-    key_impl!(PaymentBasepoint, "localpubkey");
-}
-key_read_write!(PaymentKey);
-
-/// Master key used in conjunction with per_commitment_point to generate [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation) for the latest state of a channel.
+/// Base key used in conjunction with a `per_commitment_point` to generate an [`HtlcKey`].
+///
+/// HTLC keys are used to ensure only the recipient of an HTLC can claim it on-chain with the HTLC
+/// preimage and that only the sender of an HTLC can claim it on-chain after it has timed out.
+/// Thus, both channel counterparties' HTLC keys will appears in each HTLC output's script.
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct HtlcBasepoint(pub PublicKey);
 basepoint_impl!(HtlcBasepoint);
 key_read_write!(HtlcBasepoint);
 
-
-/// [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of an htlc basepoint,
-/// that enables secure routing of payments in onion scheme without a risk of them getting stuck or diverted. It is used to claim the funds in successful or timed out htlc outputs.
+/// A derived key built from a [`HtlcBasepoint`] and `per_commitment_point`.
+///
+/// HTLC keys are used to ensure only the recipient of an HTLC can claim it on-chain with the HTLC
+/// preimage and that only the sender of an HTLC can claim it on-chain after it has timed out.
+/// Thus, both channel counterparties' HTLC keys will appears in each HTLC output's script.
+///
+/// [See the BOLT specs]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation)
+/// for more information on key derivation details.
 #[derive(PartialEq, Eq, Clone, Copy, Debug)]
 pub struct HtlcKey(pub PublicKey);
 
 impl HtlcKey {
-    key_impl!(HtlcBasepoint, "htlcpubkey");
+       key_impl!(HtlcBasepoint, "htlcpubkey");
 }
 key_read_write!(HtlcKey);
 
@@ -156,7 +155,6 @@ fn derive_public_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, per_commitm
        sha.input(&per_commitment_point.serialize());
        sha.input(&base_point.serialize());
        let res = Sha256::from_engine(sha).to_byte_array();
-    
 
        let hashkey = PublicKey::from_secret_key(&secp_ctx,
                &SecretKey::from_slice(&res).expect("Hashes should always be valid keys unless SHA-256 is broken"));
@@ -172,68 +170,71 @@ basepoint_impl!(RevocationBasepoint);
 key_read_write!(RevocationBasepoint);
 
 
-/// [htlcpubkey](https://github.com/lightning/bolts/blob/master/03-transactions.md#localpubkey-local_htlcpubkey-remote_htlcpubkey-local_delayedpubkey-and-remote_delayedpubkey-derivation) is a child key of a revocation basepoint,
-/// that enables a node to create a justice transaction punishing a counterparty for an attempt to steal funds. Used to in generation of commitment and htlc outputs.
+/// The revocation key is used to allow a channel party to revoke their state - giving their
+/// counterparty the required material to claim all of their funds if they broadcast that state.
+///
+/// Each commitment transaction has a revocation key based on the basepoint and
+/// per_commitment_point which is used in both commitment and HTLC transactions.
+///
+/// See [the BOLT spec for derivation details]
+/// (https://github.com/lightning/bolts/blob/master/03-transactions.md#revocationpubkey-derivation)
 #[derive(PartialEq, Eq, Clone, Copy, Debug, Hash)]
 pub struct RevocationKey(pub PublicKey);
 
 impl RevocationKey {
-    /// Derives a per-commitment-transaction revocation public key from its constituent parts. This is
-    /// the public equivalend of derive_private_revocation_key - using only public keys to derive a
-    /// public key instead of private keys.
-    ///
-    /// Only the cheating participant owns a valid witness to propagate a revoked
-    /// commitment transaction, thus per_commitment_point always come from cheater
-    /// and revocation_base_point always come from punisher, which is the broadcaster
-    /// of the transaction spending with this key knowledge.
-    ///
-    /// Note that this is infallible iff we trust that at least one of the two input keys are randomly
-    /// generated (ie our own).
-    pub fn from_basepoint<T: secp256k1::Verification>(
-        secp_ctx: &Secp256k1<T>,
-        basepoint: &RevocationBasepoint,
-        per_commitment_point: &PublicKey,
-    ) -> Self {
-        let rev_append_commit_hash_key = {
-            let mut sha = Sha256::engine();
-            sha.input(&basepoint.to_public_key().serialize());
-            sha.input(&per_commitment_point.serialize());
-    
-            Sha256::from_engine(sha).to_byte_array()
-        };
-        let commit_append_rev_hash_key = {
-            let mut sha = Sha256::engine();
-            sha.input(&per_commitment_point.serialize());
-            sha.input(&basepoint.to_public_key().serialize());
-    
-            Sha256::from_engine(sha).to_byte_array()
-        };
-    
-        let countersignatory_contrib = basepoint.to_public_key().mul_tweak(&secp_ctx, &Scalar::from_be_bytes(rev_append_commit_hash_key).unwrap())
-            .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
-        let broadcaster_contrib = (&per_commitment_point).mul_tweak(&secp_ctx, &Scalar::from_be_bytes(commit_append_rev_hash_key).unwrap())
-            .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
-        let pk = countersignatory_contrib.combine(&broadcaster_contrib)
-            .expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak commits to the key.");
-        Self(pk)
-    }
-
-    /// Get inner Public Key
-    pub fn to_public_key(&self) -> PublicKey {
-        self.0
-    }
+       /// Derives a per-commitment-transaction revocation public key from one party's per-commitment
+       /// point and the other party's [`RevocationBasepoint`]. This is the public equivalent of
+       /// [`chan_utils::derive_private_revocation_key`] - using only public keys to derive a public
+       /// key instead of private keys.
+       ///
+       /// Note that this is infallible iff we trust that at least one of the two input keys are randomly
+       /// generated (ie our own).
+       ///
+       /// [`chan_utils::derive_private_revocation_key`]: crate::ln::chan_utils::derive_private_revocation_key
+       pub fn from_basepoint<T: secp256k1::Verification>(
+               secp_ctx: &Secp256k1<T>,
+               countersignatory_basepoint: &RevocationBasepoint,
+               per_commitment_point: &PublicKey,
+       ) -> Self {
+               let rev_append_commit_hash_key = {
+                       let mut sha = Sha256::engine();
+                       sha.input(&countersignatory_basepoint.to_public_key().serialize());
+                       sha.input(&per_commitment_point.serialize());
+
+                       Sha256::from_engine(sha).to_byte_array()
+               };
+               let commit_append_rev_hash_key = {
+                       let mut sha = Sha256::engine();
+                       sha.input(&per_commitment_point.serialize());
+                       sha.input(&countersignatory_basepoint.to_public_key().serialize());
+
+                       Sha256::from_engine(sha).to_byte_array()
+               };
+
+               let countersignatory_contrib = countersignatory_basepoint.to_public_key().mul_tweak(&secp_ctx, &Scalar::from_be_bytes(rev_append_commit_hash_key).unwrap())
+                       .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
+               let broadcaster_contrib = (&per_commitment_point).mul_tweak(&secp_ctx, &Scalar::from_be_bytes(commit_append_rev_hash_key).unwrap())
+                       .expect("Multiplying a valid public key by a hash is expected to never fail per secp256k1 docs");
+               let pk = countersignatory_contrib.combine(&broadcaster_contrib)
+                       .expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak commits to the key.");
+               Self(pk)
+       }
+
+       /// Get inner Public Key
+       pub fn to_public_key(&self) -> PublicKey {
+               self.0
+       }
 }
 key_read_write!(RevocationKey);
 
 
-
 #[cfg(test)]
 mod test {
-    use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey};
-    use bitcoin::hashes::hex::FromHex;
-    use super::derive_public_key;
+       use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey};
+       use bitcoin::hashes::hex::FromHex;
+       use super::derive_public_key;
 
-    #[test]
+       #[test]
        fn test_key_derivation() {
                // Test vectors from BOLT 3 Appendix E:
                let secp_ctx = Secp256k1::new();
@@ -248,6 +249,6 @@ mod test {
                assert_eq!(per_commitment_point.serialize()[..], <Vec<u8>>::from_hex("025f7117a78150fe2ef97db7cfc83bd57b2e2c0d0dd25eaf467a4a1c2a45ce1486").unwrap()[..]);
 
                assert_eq!(derive_public_key(&secp_ctx, &per_commitment_point, &base_point).serialize()[..],
-                               <Vec<u8>>::from_hex("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]);
+                       <Vec<u8>>::from_hex("0235f2dbfaa89b57ec7b055afe29849ef7ddfeb1cefdb9ebdc43f5494984db29e5").unwrap()[..]);
        }
 }
index b2b28cdf365cce7ed07eda808089525c039ad015..9f3a3f425fb8728789551bdf7dec011200266c17 100644 (file)
@@ -43,14 +43,12 @@ use crate::events::{Event, EventHandler, EventsProvider, MessageSendEvent, Messa
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
 use crate::ln::{inbound_payment, ChannelId, PaymentHash, PaymentPreimage, PaymentSecret};
-use crate::ln::channel::{Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
+use crate::ln::channel::{self, Channel, ChannelPhase, ChannelContext, ChannelError, ChannelUpdateStatus, ShutdownResult, UnfundedChannelContext, UpdateFulfillCommitFetch, OutboundV1Channel, InboundV1Channel, WithChannelContext};
 use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
 #[cfg(any(feature = "_test_utils", test))]
 use crate::ln::features::Bolt11InvoiceFeatures;
-use crate::routing::gossip::NetworkGraph;
-use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
-use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters};
-use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundOnionErr, NextPacketDetails};
+use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
+use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails};
 use crate::ln::msgs;
 use crate::ln::onion_utils;
 use crate::ln::onion_utils::{HTLCFailReason, INVALID_ONION_BLINDING};
@@ -65,8 +63,9 @@ use crate::offers::merkle::SignError;
 use crate::offers::offer::{DerivedMetadata, Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
-use crate::onion_message::{Destination, MessageRouter, OffersMessage, OffersMessageHandler, PendingOnionMessage, new_pending_onion_message};
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider};
+use crate::onion_message::messenger::{Destination, MessageRouter, PendingOnionMessage, new_pending_onion_message};
+use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
+use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::sign::ecdsa::WriteableEcdsaChannelSigner;
 use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate};
 use crate::util::wakers::{Future, Notifier};
@@ -75,6 +74,13 @@ use crate::util::string::UntrustedString;
 use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter};
 use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
+#[cfg(not(c_bindings))]
+use {
+       crate::routing::router::DefaultRouter,
+       crate::routing::gossip::NetworkGraph,
+       crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters},
+       crate::sign::KeysManager,
+};
 
 use alloc::collections::{btree_map, BTreeMap};
 
@@ -545,9 +551,8 @@ impl Into<u16> for FailureCode {
 
 struct MsgHandleErrInternal {
        err: msgs::LightningError,
-       chan_id: Option<(ChannelId, u128)>, // If Some a channel of ours has been closed
+       closes_channel: bool,
        shutdown_finish: Option<(ShutdownResult, Option<msgs::ChannelUpdate>)>,
-       channel_capacity: Option<u64>,
 }
 impl MsgHandleErrInternal {
        #[inline]
@@ -562,17 +567,16 @@ impl MsgHandleErrInternal {
                                        },
                                },
                        },
-                       chan_id: None,
+                       closes_channel: false,
                        shutdown_finish: None,
-                       channel_capacity: None,
                }
        }
        #[inline]
        fn from_no_close(err: msgs::LightningError) -> Self {
-               Self { err, chan_id: None, shutdown_finish: None, channel_capacity: None }
+               Self { err, closes_channel: false, shutdown_finish: None }
        }
        #[inline]
-       fn from_finish_shutdown(err: String, channel_id: ChannelId, user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>, channel_capacity: u64) -> Self {
+       fn from_finish_shutdown(err: String, channel_id: ChannelId, shutdown_res: ShutdownResult, channel_update: Option<msgs::ChannelUpdate>) -> Self {
                let err_msg = msgs::ErrorMessage { channel_id, data: err.clone() };
                let action = if shutdown_res.monitor_update.is_some() {
                        // We have a closing `ChannelMonitorUpdate`, which means the channel was funded and we
@@ -584,9 +588,8 @@ impl MsgHandleErrInternal {
                };
                Self {
                        err: LightningError { err, action },
-                       chan_id: Some((channel_id, user_channel_id)),
+                       closes_channel: true,
                        shutdown_finish: Some((shutdown_res, channel_update)),
-                       channel_capacity: Some(channel_capacity)
                }
        }
        #[inline]
@@ -617,14 +620,13 @@ impl MsgHandleErrInternal {
                                        },
                                },
                        },
-                       chan_id: None,
+                       closes_channel: false,
                        shutdown_finish: None,
-                       channel_capacity: None,
                }
        }
 
        fn closes_channel(&self) -> bool {
-               self.chan_id.is_some()
+               self.closes_channel
        }
 }
 
@@ -1956,30 +1958,27 @@ macro_rules! handle_error {
 
                match $internal {
                        Ok(msg) => Ok(msg),
-                       Err(MsgHandleErrInternal { err, chan_id, shutdown_finish, channel_capacity }) => {
+                       Err(MsgHandleErrInternal { err, shutdown_finish, .. }) => {
                                let mut msg_events = Vec::with_capacity(2);
 
                                if let Some((shutdown_res, update_option)) = shutdown_finish {
+                                       let counterparty_node_id = shutdown_res.counterparty_node_id;
+                                       let channel_id = shutdown_res.channel_id;
+                                       let logger = WithContext::from(
+                                               &$self.logger, Some(counterparty_node_id), Some(channel_id),
+                                       );
+                                       log_error!(logger, "Force-closing channel: {}", err.err);
+
                                        $self.finish_close_channel(shutdown_res);
                                        if let Some(update) = update_option {
                                                msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                        msg: update
                                                });
                                        }
-                                       if let Some((channel_id, user_channel_id)) = chan_id {
-                                               $self.pending_events.lock().unwrap().push_back((events::Event::ChannelClosed {
-                                                       channel_id, user_channel_id,
-                                                       reason: ClosureReason::ProcessingError { err: err.err.clone() },
-                                                       counterparty_node_id: Some($counterparty_node_id),
-                                                       channel_capacity_sats: channel_capacity,
-                                               }, None));
-                                       }
+                               } else {
+                                       log_error!($self.logger, "Got non-closing error: {}", err.err);
                                }
 
-                               let logger = WithContext::from(
-                                       &$self.logger, Some($counterparty_node_id), chan_id.map(|(chan_id, _)| chan_id)
-                               );
-                               log_error!(logger, "{}", err.err);
                                if let msgs::ErrorAction::IgnoreError = err.action {
                                } else {
                                        msg_events.push(events::MessageSendEvent::HandleError {
@@ -2039,12 +2038,11 @@ macro_rules! convert_chan_phase_err {
                                let logger = WithChannelContext::from(&$self.logger, &$channel.context);
                                log_error!(logger, "Closing channel {} due to close-required error: {}", $channel_id, msg);
                                update_maps_on_chan_removal!($self, $channel.context);
-                               let shutdown_res = $channel.context.force_shutdown(true);
-                               let user_id = $channel.context.get_user_id();
-                               let channel_capacity_satoshis = $channel.context.get_value_satoshis();
-
-                               (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, user_id,
-                                       shutdown_res, $channel_update, channel_capacity_satoshis))
+                               let reason = ClosureReason::ProcessingError { err: msg.clone() };
+                               let shutdown_res = $channel.context.force_shutdown(true, reason);
+                               let err =
+                                       MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $channel_update);
+                               (true, err)
                        },
                }
        };
@@ -2701,26 +2699,6 @@ where
                        .collect()
        }
 
-       /// Helper function that issues the channel close events
-       fn issue_channel_close_events(&self, context: &ChannelContext<SP>, closure_reason: ClosureReason) {
-               let mut pending_events_lock = self.pending_events.lock().unwrap();
-               match context.unbroadcasted_funding() {
-                       Some(transaction) => {
-                               pending_events_lock.push_back((events::Event::DiscardFunding {
-                                       channel_id: context.channel_id(), transaction
-                               }, None));
-                       },
-                       None => {},
-               }
-               pending_events_lock.push_back((events::Event::ChannelClosed {
-                       channel_id: context.channel_id(),
-                       user_channel_id: context.get_user_id(),
-                       reason: closure_reason,
-                       counterparty_node_id: Some(context.get_counterparty_node_id()),
-                       channel_capacity_sats: Some(context.get_value_satoshis()),
-               }, None));
-       }
-
        fn close_channel_internal(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, override_shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
@@ -2762,9 +2740,8 @@ where
                                                                peer_state_lock, peer_state, per_peer_state, chan);
                                                }
                                        } else {
-                                               self.issue_channel_close_events(chan_phase_entry.get().context(), ClosureReason::HolderForceClosed);
                                                let mut chan_phase = remove_channel_phase!(self, chan_phase_entry);
-                                               shutdown_result = Some(chan_phase.context_mut().force_shutdown(false));
+                                               shutdown_result = Some(chan_phase.context_mut().force_shutdown(false, ClosureReason::HolderForceClosed));
                                        }
                                },
                                hash_map::Entry::Vacant(_) => {
@@ -2861,7 +2838,9 @@ where
                let logger = WithContext::from(
                        &self.logger, Some(shutdown_res.counterparty_node_id), Some(shutdown_res.channel_id),
                );
-               log_debug!(logger, "Finishing closure of channel with {} HTLCs to fail", shutdown_res.dropped_outbound_htlcs.len());
+
+               log_debug!(logger, "Finishing closure of channel due to {} with {} HTLCs to fail",
+                       shutdown_res.closure_reason, shutdown_res.dropped_outbound_htlcs.len());
                for htlc_source in shutdown_res.dropped_outbound_htlcs.drain(..) {
                        let (source, payment_hash, counterparty_node_id, channel_id) = htlc_source;
                        let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
@@ -2886,8 +2865,7 @@ where
                                        let mut peer_state = peer_state_mutex.lock().unwrap();
                                        if let Some(mut chan) = peer_state.channel_by_id.remove(&channel_id) {
                                                update_maps_on_chan_removal!(self, &chan.context());
-                                               self.issue_channel_close_events(&chan.context(), ClosureReason::FundingBatchClosure);
-                                               shutdown_results.push(chan.context_mut().force_shutdown(false));
+                                               shutdown_results.push(chan.context_mut().force_shutdown(false, ClosureReason::FundingBatchClosure));
                                        }
                                }
                                has_uncompleted_channel = Some(has_uncompleted_channel.map_or(!state, |v| v || !state));
@@ -2897,6 +2875,23 @@ where
                                "Closing a batch where all channels have completed initial monitor update",
                        );
                }
+
+               {
+                       let mut pending_events = self.pending_events.lock().unwrap();
+                       pending_events.push_back((events::Event::ChannelClosed {
+                               channel_id: shutdown_res.channel_id,
+                               user_channel_id: shutdown_res.user_channel_id,
+                               reason: shutdown_res.closure_reason,
+                               counterparty_node_id: Some(shutdown_res.counterparty_node_id),
+                               channel_capacity_sats: Some(shutdown_res.channel_capacity_satoshis),
+                       }, None));
+
+                       if let Some(transaction) = shutdown_res.unbroadcasted_funding_tx {
+                               pending_events.push_back((events::Event::DiscardFunding {
+                                       channel_id: shutdown_res.channel_id, transaction
+                               }, None));
+                       }
+               }
                for shutdown_result in shutdown_results.drain(..) {
                        self.finish_close_channel(shutdown_result);
                }
@@ -2919,17 +2914,16 @@ where
                        let logger = WithContext::from(&self.logger, Some(*peer_node_id), Some(*channel_id));
                        if let hash_map::Entry::Occupied(chan_phase_entry) = peer_state.channel_by_id.entry(channel_id.clone()) {
                                log_error!(logger, "Force-closing channel {}", channel_id);
-                               self.issue_channel_close_events(&chan_phase_entry.get().context(), closure_reason);
                                let mut chan_phase = remove_channel_phase!(self, chan_phase_entry);
                                mem::drop(peer_state);
                                mem::drop(per_peer_state);
                                match chan_phase {
                                        ChannelPhase::Funded(mut chan) => {
-                                               self.finish_close_channel(chan.context.force_shutdown(broadcast));
+                                               self.finish_close_channel(chan.context.force_shutdown(broadcast, closure_reason));
                                                (self.get_channel_update_for_broadcast(&chan).ok(), chan.context.get_counterparty_node_id())
                                        },
                                        ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {
-                                               self.finish_close_channel(chan_phase.context_mut().force_shutdown(false));
+                                               self.finish_close_channel(chan_phase.context_mut().force_shutdown(false, closure_reason));
                                                // Unfunded channel has no update
                                                (None, chan_phase.context().get_counterparty_node_id())
                                        },
@@ -3240,14 +3234,14 @@ where
                                                // delay) once they've send us a commitment_signed!
                                                PendingHTLCStatus::Forward(info)
                                        },
-                                       Err(InboundOnionErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
+                                       Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
                                }
                        },
                        onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
                                match create_fwd_pending_htlc_info(msg, next_hop_data, next_hop_hmac,
                                        new_packet_bytes, shared_secret, next_packet_pubkey_opt) {
                                        Ok(info) => PendingHTLCStatus::Forward(info),
-                                       Err(InboundOnionErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
+                                       Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
                                }
                        }
                }
@@ -3750,7 +3744,7 @@ where
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                let funding_txo;
-               let (chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
+               let (mut chan, msg_opt) = match peer_state.channel_by_id.remove(temporary_channel_id) {
                        Some(ChannelPhase::UnfundedOutboundV1(mut chan)) => {
                                funding_txo = find_funding_output(&chan, &funding_transaction)?;
 
@@ -3758,10 +3752,9 @@ where
                                let funding_res = chan.get_funding_created(funding_transaction, funding_txo, is_batch_funding, &&logger)
                                        .map_err(|(mut chan, e)| if let ChannelError::Close(msg) = e {
                                                let channel_id = chan.context.channel_id();
-                                               let user_id = chan.context.get_user_id();
-                                               let shutdown_res = chan.context.force_shutdown(false);
-                                               let channel_capacity = chan.context.get_value_satoshis();
-                                               (chan, MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, user_id, shutdown_res, None, channel_capacity))
+                                               let reason = ClosureReason::ProcessingError { err: msg.clone() };
+                                               let shutdown_res = chan.context.force_shutdown(false, reason);
+                                               (chan, MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, shutdown_res, None))
                                        } else { unreachable!(); });
                                match funding_res {
                                        Ok(funding_msg) => (chan, funding_msg),
@@ -3801,8 +3794,20 @@ where
                        },
                        hash_map::Entry::Vacant(e) => {
                                let mut outpoint_to_peer = self.outpoint_to_peer.lock().unwrap();
-                               if outpoint_to_peer.insert(funding_txo, chan.context.get_counterparty_node_id()).is_some() {
-                                       panic!("outpoint_to_peer map already contained funding outpoint, which shouldn't be possible");
+                               match outpoint_to_peer.entry(funding_txo) {
+                                       hash_map::Entry::Vacant(e) => { e.insert(chan.context.get_counterparty_node_id()); },
+                                       hash_map::Entry::Occupied(o) => {
+                                               let err = format!(
+                                                       "An existing channel using outpoint {} is open with peer {}",
+                                                       funding_txo, o.get()
+                                               );
+                                               mem::drop(outpoint_to_peer);
+                                               mem::drop(peer_state_lock);
+                                               mem::drop(per_peer_state);
+                                               let reason = ClosureReason::ProcessingError { err: err.clone() };
+                                               self.finish_close_channel(chan.context.force_shutdown(true, reason));
+                                               return Err(APIError::ChannelUnavailable { err });
+                                       }
                                }
                                e.insert(ChannelPhase::UnfundedOutboundV1(chan));
                        }
@@ -3966,8 +3971,8 @@ where
                                                .and_then(|mut peer_state| peer_state.channel_by_id.remove(&channel_id))
                                                .map(|mut chan| {
                                                        update_maps_on_chan_removal!(self, &chan.context());
-                                                       self.issue_channel_close_events(&chan.context(), ClosureReason::ProcessingError { err: e.clone() });
-                                                       shutdown_results.push(chan.context_mut().force_shutdown(false));
+                                                       let closure_reason = ClosureReason::ProcessingError { err: e.clone() };
+                                                       shutdown_results.push(chan.context_mut().force_shutdown(false, closure_reason));
                                                });
                                }
                        }
@@ -4304,7 +4309,7 @@ where
                                                                                                                        current_height, self.default_configuration.accept_mpp_keysend)
                                                                                                                {
                                                                                                                        Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, vec![(info, prev_htlc_id)])),
-                                                                                                                       Err(InboundOnionErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
+                                                                                                                       Err(InboundHTLCErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
                                                                                                                }
                                                                                                        },
                                                                                                        _ => panic!(),
@@ -4346,7 +4351,7 @@ where
                                        if let Some(ChannelPhase::Funded(ref mut chan)) = peer_state.channel_by_id.get_mut(&forward_chan_id) {
                                                let logger = WithChannelContext::from(&self.logger, &chan.context);
                                                for forward_info in pending_forwards.drain(..) {
-                                                       match forward_info {
+                                                       let queue_fail_htlc_res = match forward_info {
                                                                HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
                                                                        prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
                                                                        forward_info: PendingHTLCInfo {
@@ -4392,40 +4397,35 @@ where
                                                                                ));
                                                                                continue;
                                                                        }
+                                                                       None
                                                                },
                                                                HTLCForwardInfo::AddHTLC { .. } => {
                                                                        panic!("short_channel_id != 0 should imply any pending_forward entries are of type Forward");
                                                                },
                                                                HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => {
                                                                        log_trace!(logger, "Failing HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id);
-                                                                       if let Err(e) = chan.queue_fail_htlc(
-                                                                               htlc_id, err_packet, &&logger
-                                                                       ) {
-                                                                               if let ChannelError::Ignore(msg) = e {
-                                                                                       log_trace!(logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg);
-                                                                               } else {
-                                                                                       panic!("Stated return value requirements in queue_fail_htlc() were not met");
-                                                                               }
-                                                                               // fail-backs are best-effort, we probably already have one
-                                                                               // pending, and if not that's OK, if not, the channel is on
-                                                                               // the chain and sending the HTLC-Timeout is their problem.
-                                                                               continue;
-                                                                       }
+                                                                       Some((chan.queue_fail_htlc(htlc_id, err_packet, &&logger), htlc_id))
                                                                },
                                                                HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
-                                                                       log_trace!(self.logger, "Failing malformed HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id);
-                                                                       if let Err(e) = chan.queue_fail_malformed_htlc(htlc_id, failure_code, sha256_of_onion, &self.logger) {
-                                                                               if let ChannelError::Ignore(msg) = e {
-                                                                                       log_trace!(self.logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg);
-                                                                               } else {
-                                                                                       panic!("Stated return value requirements in queue_fail_malformed_htlc() were not met");
-                                                                               }
-                                                                               // fail-backs are best-effort, we probably already have one
-                                                                               // pending, and if not that's OK, if not, the channel is on
-                                                                               // the chain and sending the HTLC-Timeout is their problem.
-                                                                               continue;
-                                                                       }
+                                                                       log_trace!(logger, "Failing malformed HTLC back to channel with short id {} (backward HTLC ID {}) after delay", short_chan_id, htlc_id);
+                                                                       let res = chan.queue_fail_malformed_htlc(
+                                                                               htlc_id, failure_code, sha256_of_onion, &&logger
+                                                                       );
+                                                                       Some((res, htlc_id))
                                                                },
+                                                       };
+                                                       if let Some((queue_fail_htlc_res, htlc_id)) = queue_fail_htlc_res {
+                                                               if let Err(e) = queue_fail_htlc_res {
+                                                                       if let ChannelError::Ignore(msg) = e {
+                                                                               log_trace!(logger, "Failed to fail HTLC with ID {} backwards to short_id {}: {}", htlc_id, short_chan_id, msg);
+                                                                       } else {
+                                                                               panic!("Stated return value requirements in queue_fail_{{malformed_}}htlc() were not met");
+                                                                       }
+                                                                       // fail-backs are best-effort, we probably already have one
+                                                                       // pending, and if not that's OK, if not, the channel is on
+                                                                       // the chain and sending the HTLC-Timeout is their problem.
+                                                                       continue;
+                                                               }
                                                        }
                                                }
                                        } else {
@@ -4889,8 +4889,7 @@ where
                                        log_error!(logger,
                                                "Force-closing pending channel with ID {} for not establishing in a timely manner", chan_id);
                                        update_maps_on_chan_removal!(self, &context);
-                                       self.issue_channel_close_events(&context, ClosureReason::HolderForceClosed);
-                                       shutdown_channels.push(context.force_shutdown(false));
+                                       shutdown_channels.push(context.force_shutdown(false, ClosureReason::HolderForceClosed));
                                        pending_msg_events.push(MessageSendEvent::HandleError {
                                                node_id: counterparty_node_id,
                                                action: msgs::ErrorAction::SendErrorMessage {
@@ -5997,13 +5996,20 @@ where
        }
 
        fn do_accept_inbound_channel(&self, temporary_channel_id: &ChannelId, counterparty_node_id: &PublicKey, accept_0conf: bool, user_channel_id: u128) -> Result<(), APIError> {
+
+               let logger = WithContext::from(&self.logger, Some(*counterparty_node_id), Some(*temporary_channel_id));
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                let peers_without_funded_channels =
                        self.peers_without_funded_channels(|peer| { peer.total_channel_count() > 0 });
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex = per_peer_state.get(counterparty_node_id)
-                       .ok_or_else(|| APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id) })?;
+               .ok_or_else(|| {
+                       let err_str = format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id);
+                       log_error!(logger, "{}", err_str);
+
+                       APIError::ChannelUnavailable { err: err_str }
+               })?;
                let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                let peer_state = &mut *peer_state_lock;
                let is_only_peer_channel = peer_state.total_channel_count() == 1;
@@ -6018,9 +6024,19 @@ where
                                InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider,
                                        counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features,
                                        &unaccepted_channel.open_channel_msg, user_channel_id, &self.default_configuration, best_block_height,
-                                       &self.logger, accept_0conf).map_err(|e| APIError::ChannelUnavailable { err: e.to_string() })
+                                       &self.logger, accept_0conf).map_err(|e| {
+                                               let err_str = e.to_string();
+                                               log_error!(logger, "{}", err_str);
+
+                                               APIError::ChannelUnavailable { err: err_str }
+                                       })
+                               }
+                       _ => {
+                               let err_str = "No such channel awaiting to be accepted.".to_owned();
+                               log_error!(logger, "{}", err_str);
+
+                               Err(APIError::APIMisuseError { err: err_str })
                        }
-                       _ => Err(APIError::APIMisuseError { err: "No such channel awaiting to be accepted.".to_owned() })
                }?;
 
                if accept_0conf {
@@ -6034,7 +6050,10 @@ where
                                }
                        };
                        peer_state.pending_msg_events.push(send_msg_err_event);
-                       return Err(APIError::APIMisuseError { err: "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned() });
+                       let err_str = "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned();
+                       log_error!(logger, "{}", err_str);
+
+                       return Err(APIError::APIMisuseError { err: err_str });
                } else {
                        // If this peer already has some channels, a new channel won't increase our number of peers
                        // with unfunded channels, so as long as we aren't over the maximum number of unfunded
@@ -6047,7 +6066,10 @@ where
                                        }
                                };
                                peer_state.pending_msg_events.push(send_msg_err_event);
-                               return Err(APIError::APIMisuseError { err: "Too many peers with unfunded channels, refusing to accept new ones".to_owned() });
+                               let err_str = "Too many peers with unfunded channels, refusing to accept new ones".to_owned();
+                               log_error!(logger, "{}", err_str);
+
+                               return Err(APIError::APIMisuseError { err: err_str });
                        }
                }
 
@@ -6170,13 +6192,18 @@ where
 
                // If we're doing manual acceptance checks on the channel, then defer creation until we're sure we want to accept.
                if self.default_configuration.manually_accept_inbound_channels {
+                       let channel_type = channel::channel_type_from_open_channel(
+                                       &msg, &peer_state.latest_features, &self.channel_type_features()
+                               ).map_err(|e|
+                                       MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id)
+                               )?;
                        let mut pending_events = self.pending_events.lock().unwrap();
                        pending_events.push_back((events::Event::OpenChannelRequest {
                                temporary_channel_id: msg.temporary_channel_id.clone(),
                                counterparty_node_id: counterparty_node_id.clone(),
                                funding_satoshis: msg.funding_satoshis,
                                push_msat: msg.push_msat,
-                               channel_type: msg.channel_type.clone().unwrap(),
+                               channel_type,
                        }, None));
                        peer_state.inbound_channel_request_by_id.insert(channel_id, InboundChannelRequest {
                                open_channel_msg: msg.clone(),
@@ -6376,7 +6403,7 @@ where
                                        let res =
                                                chan.funding_signed(&msg, best_block, &self.signer_provider, &&logger);
                                        match res {
-                                               Ok((chan, monitor)) => {
+                                               Ok((mut chan, monitor)) => {
                                                        if let Ok(persist_status) = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor) {
                                                                // We really should be able to insert here without doing a second
                                                                // lookup, but sadly rust stdlib doesn't currently allow keeping
@@ -6388,6 +6415,11 @@ where
                                                                Ok(())
                                                        } else {
                                                                let e = ChannelError::Close("Channel funding outpoint was a duplicate".to_owned());
+                                                               // We weren't able to watch the channel to begin with, so no
+                                                               // updates should be made on it. Previously, full_stack_target
+                                                               // found an (unreachable) panic when the monitor update contained
+                                                               // within `shutdown_finish` was applied.
+                                                               chan.unset_funding_info(msg.channel_id);
                                                                return Err(convert_chan_phase_err!(self, e, &mut ChannelPhase::Funded(chan), &msg.channel_id).1);
                                                        }
                                                },
@@ -6510,9 +6542,8 @@ where
                                                let context = phase.context_mut();
                                                let logger = WithChannelContext::from(&self.logger, context);
                                                log_error!(logger, "Immediately closing unfunded channel {} as peer asked to cooperatively shut it down (which is unnecessary)", &msg.channel_id);
-                                               self.issue_channel_close_events(&context, ClosureReason::CounterpartyCoopClosedUnfundedChannel);
                                                let mut chan = remove_channel_phase!(self, chan_phase_entry);
-                                               finish_shutdown = Some(chan.context_mut().force_shutdown(false));
+                                               finish_shutdown = Some(chan.context_mut().force_shutdown(false, ClosureReason::CounterpartyCoopClosedUnfundedChannel));
                                        },
                                }
                        } else {
@@ -6581,7 +6612,6 @@ where
                                        msg: update
                                });
                        }
-                       self.issue_channel_close_events(&chan.context, ClosureReason::CooperativeClosure);
                }
                mem::drop(per_peer_state);
                if let Some(shutdown_result) = shutdown_result {
@@ -7233,13 +7263,12 @@ where
                                                                let pending_msg_events = &mut peer_state.pending_msg_events;
                                                                if let hash_map::Entry::Occupied(chan_phase_entry) = peer_state.channel_by_id.entry(funding_outpoint.to_channel_id()) {
                                                                        if let ChannelPhase::Funded(mut chan) = remove_channel_phase!(self, chan_phase_entry) {
-                                                                               failed_channels.push(chan.context.force_shutdown(false));
+                                                                               failed_channels.push(chan.context.force_shutdown(false, ClosureReason::HolderForceClosed));
                                                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                                                msg: update
                                                                                        });
                                                                                }
-                                                                               self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
                                                                                pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                                                        node_id: chan.context.get_counterparty_node_id(),
                                                                                        action: msgs::ErrorAction::DisconnectPeer {
@@ -7426,8 +7455,6 @@ where
                                                                                        });
                                                                                }
 
-                                                                               self.issue_channel_close_events(&chan.context, ClosureReason::CooperativeClosure);
-
                                                                                log_info!(logger, "Broadcasting {}", log_tx!(tx));
                                                                                self.tx_broadcaster.broadcast_transactions(&[&tx]);
                                                                                update_maps_on_chan_removal!(self, &chan.context);
@@ -8440,14 +8467,13 @@ where
                                                                update_maps_on_chan_removal!(self, &channel.context);
                                                                // It looks like our counterparty went on-chain or funding transaction was
                                                                // reorged out of the main chain. Close the channel.
-                                                               failed_channels.push(channel.context.force_shutdown(true));
+                                                               let reason_message = format!("{}", reason);
+                                                               failed_channels.push(channel.context.force_shutdown(true, reason));
                                                                if let Ok(update) = self.get_channel_update_for_broadcast(&channel) {
                                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                                msg: update
                                                                        });
                                                                }
-                                                               let reason_message = format!("{}", reason);
-                                                               self.issue_channel_close_events(&channel.context, reason);
                                                                pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                                        node_id: channel.context.get_counterparty_node_id(),
                                                                        action: msgs::ErrorAction::DisconnectPeer {
@@ -8845,8 +8871,7 @@ where
                                        };
                                        // Clean up for removal.
                                        update_maps_on_chan_removal!(self, &context);
-                                       self.issue_channel_close_events(&context, ClosureReason::DisconnectedPeer);
-                                       failed_channels.push(context.force_shutdown(false));
+                                       failed_channels.push(context.force_shutdown(false, ClosureReason::DisconnectedPeer));
                                        false
                                });
                                // Note that we don't bother generating any events for pre-accept channels -
@@ -8979,13 +9004,7 @@ where
                                let pending_msg_events = &mut peer_state.pending_msg_events;
 
                                peer_state.channel_by_id.iter_mut().filter_map(|(_, phase)|
-                                       if let ChannelPhase::Funded(chan) = phase { Some(chan) } else {
-                                               // Since unfunded channel maps are cleared upon disconnecting a peer, and they're not persisted
-                                               // (so won't be recovered after a crash), they shouldn't exist here and we would never need to
-                                               // worry about closing and removing them.
-                                               debug_assert!(false);
-                                               None
-                                       }
+                                       if let ChannelPhase::Funded(chan) = phase { Some(chan) } else { None }
                                ).for_each(|chan| {
                                        let logger = WithChannelContext::from(&self.logger, &chan.context);
                                        pending_msg_events.push(events::MessageSendEvent::SendChannelReestablish {
@@ -10292,7 +10311,7 @@ where
                                                log_error!(logger, " The ChannelMonitor for channel {} is at counterparty commitment transaction number {} but the ChannelManager is at counterparty commitment transaction number {}.",
                                                        &channel.context.channel_id(), monitor.get_cur_counterparty_commitment_number(), channel.get_cur_counterparty_commitment_transaction_number());
                                        }
-                                       let mut shutdown_result = channel.context.force_shutdown(true);
+                                       let mut shutdown_result = channel.context.force_shutdown(true, ClosureReason::OutdatedChannelManager);
                                        if shutdown_result.unbroadcasted_batch_funding_txid.is_some() {
                                                return Err(DecodeError::InvalidValue);
                                        }
@@ -10354,7 +10373,7 @@ where
                                // If we were persisted and shut down while the initial ChannelMonitor persistence
                                // was in-progress, we never broadcasted the funding transaction and can still
                                // safely discard the channel.
-                               let _ = channel.context.force_shutdown(false);
+                               let _ = channel.context.force_shutdown(false, ClosureReason::DisconnectedPeer);
                                channel_closures.push_back((events::Event::ChannelClosed {
                                        channel_id: channel.context.channel_id(),
                                        user_channel_id: channel.context.get_user_id(),
@@ -12100,8 +12119,8 @@ mod tests {
                let sender_intended_amt_msat = 100;
                let extra_fee_msat = 10;
                let hop_data = msgs::InboundOnionPayload::Receive {
-                       amt_msat: 100,
-                       outgoing_cltv_value: 42,
+                       sender_intended_htlc_amt_msat: 100,
+                       cltv_expiry_height: 42,
                        payment_metadata: None,
                        keysend_preimage: None,
                        payment_data: Some(msgs::FinalOnionHopData {
@@ -12112,7 +12131,7 @@ mod tests {
                // Check that if the amount we received + the penultimate hop extra fee is less than the sender
                // intended amount, we fail the payment.
                let current_height: u32 = node[0].node.best_block.read().unwrap().height();
-               if let Err(crate::ln::channelmanager::InboundOnionErr { err_code, .. }) =
+               if let Err(crate::ln::channelmanager::InboundHTLCErr { err_code, .. }) =
                        create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
                                sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat),
                                current_height, node[0].node.default_configuration.accept_mpp_keysend)
@@ -12122,8 +12141,8 @@ mod tests {
 
                // If amt_received + extra_fee is equal to the sender intended amount, we're fine.
                let hop_data = msgs::InboundOnionPayload::Receive { // This is the same payload as above, InboundOnionPayload doesn't implement Clone
-                       amt_msat: 100,
-                       outgoing_cltv_value: 42,
+                       sender_intended_htlc_amt_msat: 100,
+                       cltv_expiry_height: 42,
                        payment_metadata: None,
                        keysend_preimage: None,
                        payment_data: Some(msgs::FinalOnionHopData {
@@ -12146,8 +12165,8 @@ mod tests {
 
                let current_height: u32 = node[0].node.best_block.read().unwrap().height();
                let result = create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive {
-                       amt_msat: 100,
-                       outgoing_cltv_value: 22,
+                       sender_intended_htlc_amt_msat: 100,
+                       cltv_expiry_height: 22,
                        payment_metadata: None,
                        keysend_preimage: None,
                        payment_data: Some(msgs::FinalOnionHopData {
index df5c0abf25f2a27f8d81589447435b61a15589ef..2e732b17aa8bc8a88f923942ce8e47f0f3807b82 100644 (file)
@@ -469,12 +469,24 @@ impl<T: sealed::Context> Clone for Features<T> {
 }
 impl<T: sealed::Context> Hash for Features<T> {
        fn hash<H: Hasher>(&self, hasher: &mut H) {
-               self.flags.hash(hasher);
+               let mut nonzero_flags = &self.flags[..];
+               while nonzero_flags.last() == Some(&0) {
+                       nonzero_flags = &nonzero_flags[..nonzero_flags.len() - 1];
+               }
+               nonzero_flags.hash(hasher);
        }
 }
 impl<T: sealed::Context> PartialEq for Features<T> {
        fn eq(&self, o: &Self) -> bool {
-               self.flags.eq(&o.flags)
+               let mut o_iter = o.flags.iter();
+               let mut self_iter = self.flags.iter();
+               loop {
+                       match (o_iter.next(), self_iter.next()) {
+                               (Some(o), Some(us)) => if o != us { return false },
+                               (Some(b), None) | (None, Some(b)) => if *b != 0 { return false },
+                               (None, None) => return true,
+                       }
+               }
        }
 }
 impl<T: sealed::Context> PartialOrd for Features<T> {
@@ -1215,4 +1227,26 @@ mod tests {
                assert!(!converted_features.supports_any_optional_bits());
                assert!(converted_features.requires_static_remote_key());
        }
+
+       #[test]
+       #[cfg(feature = "std")]
+       fn test_excess_zero_bytes_ignored() {
+               // Checks that `Hash` and `PartialEq` ignore excess zero bytes, which may appear due to
+               // feature conversion or because a peer serialized their feature poorly.
+               use std::collections::hash_map::DefaultHasher;
+               use std::hash::{Hash, Hasher};
+
+               let mut zerod_features = InitFeatures::empty();
+               zerod_features.flags = vec![0];
+               let empty_features = InitFeatures::empty();
+               assert!(empty_features.flags.is_empty());
+
+               assert_eq!(zerod_features, empty_features);
+
+               let mut zerod_hash = DefaultHasher::new();
+               zerod_features.hash(&mut zerod_hash);
+               let mut empty_hash = DefaultHasher::new();
+               empty_features.hash(&mut empty_hash);
+               assert_eq!(zerod_hash.finish(), empty_hash.finish());
+       }
 }
index a2d9631716c4c23098135a8f95ee37a1fc5a6949..2adfe2f9379c04c6352a709ff2471232f87d6141 100644 (file)
@@ -1218,7 +1218,7 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>(initiator: &'a Node<'b, 'c, 'd>, r
        (tx, as_channel_ready.channel_id)
 }
 
-pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> Transaction {
+pub fn exchange_open_accept_chan<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> ChannelId {
        let create_chan_id = node_a.node.create_channel(node_b.node.get_our_node_id(), channel_value, push_msat, 42, None, None).unwrap();
        let open_channel_msg = get_event_msg!(node_a, MessageSendEvent::SendOpenChannel, node_b.node.get_our_node_id());
        assert_eq!(open_channel_msg.temporary_channel_id, create_chan_id);
@@ -1238,6 +1238,11 @@ pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, '
        node_a.node.handle_accept_channel(&node_b.node.get_our_node_id(), &accept_channel_msg);
        assert_ne!(node_b.node.list_channels().iter().find(|channel| channel.channel_id == create_chan_id).unwrap().user_channel_id, 0);
 
+       create_chan_id
+}
+
+pub fn create_chan_between_nodes_with_value_init<'a, 'b, 'c>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>, channel_value: u64, push_msat: u64) -> Transaction {
+       let create_chan_id = exchange_open_accept_chan(node_a, node_b, channel_value, push_msat);
        sign_funding_transaction(node_a, node_b, channel_value, create_chan_id)
 }
 
index 2ad53faa8b2e129874cbec1391156ab58aabc6e5..6f79b59774e6a728d43905fa44378b97d0fbda14 100644 (file)
@@ -768,7 +768,7 @@ fn test_update_fee_that_funder_cannot_afford() {
        //check to see if the funder, who sent the update_fee request, can afford the new fee (funder_balance >= fee+channel_reserve)
        //Should produce and error.
        nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commit_signed_msg);
-       nodes[1].logger.assert_log("lightning::ln::channelmanager", "Funding remote cannot afford proposed new fee".to_string(), 1);
+       nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Funding remote cannot afford proposed new fee", 3);
        check_added_monitors!(nodes[1], 1);
        check_closed_broadcast!(nodes[1], true);
        check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: String::from("Funding remote cannot afford proposed new fee") },
@@ -1617,7 +1617,7 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() {
 
        nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &msg);
        // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd.
-       nodes[0].logger.assert_log("lightning::ln::channelmanager", "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_string(), 1);
+       nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value", 3);
        assert_eq!(nodes[0].node.list_channels().len(), 0);
        let err_msg = check_closed_broadcast!(nodes[0], true).unwrap();
        assert_eq!(err_msg.data, "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value");
@@ -1796,7 +1796,7 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() {
 
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
        // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd.
-       nodes[1].logger.assert_log("lightning::ln::channelmanager", "Remote HTLC add would put them under remote reserve value".to_string(), 1);
+       nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Remote HTLC add would put them under remote reserve value", 3);
        assert_eq!(nodes[1].node.list_channels().len(), 1);
        let err_msg = check_closed_broadcast!(nodes[1], true).unwrap();
        assert_eq!(err_msg.data, "Remote HTLC add would put them under remote reserve value");
@@ -3328,22 +3328,18 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
 
        let events = nodes[1].node.get_and_clear_pending_events();
        assert_eq!(events.len(), if deliver_bs_raa { 3 + nodes.len() - 1 } else { 4 + nodes.len() });
-       match events[0] {
-               Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => { },
-               _ => panic!("Unexepected event"),
-       }
-       match events[1] {
-               Event::PaymentPathFailed { ref payment_hash, .. } => {
-                       assert_eq!(*payment_hash, fourth_payment_hash);
-               },
-               _ => panic!("Unexpected event"),
-       }
-       match events[2] {
-               Event::PaymentFailed { ref payment_hash, .. } => {
-                       assert_eq!(*payment_hash, fourth_payment_hash);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       assert!(events.iter().any(|ev| matches!(
+               ev,
+               Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. }
+       )));
+       assert!(events.iter().any(|ev| matches!(
+               ev,
+               Event::PaymentPathFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash
+       )));
+       assert!(events.iter().any(|ev| matches!(
+               ev,
+               Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash
+       )));
 
        nodes[1].node.process_pending_htlc_forwards();
        check_added_monitors!(nodes[1], 1);
@@ -6299,7 +6295,7 @@ fn test_update_add_htlc_bolt2_receiver_zero_value_msat() {
        updates.update_add_htlcs[0].amount_msat = 0;
 
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
-       nodes[1].logger.assert_log("lightning::ln::channelmanager", "Remote side tried to send a 0-msat HTLC".to_string(), 1);
+       nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Remote side tried to send a 0-msat HTLC", 3);
        check_closed_broadcast!(nodes[1], true).unwrap();
        check_added_monitors!(nodes[1], 1);
        check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Remote side tried to send a 0-msat HTLC".to_string() },
@@ -8216,8 +8212,10 @@ fn test_onion_value_mpp_set_calculation() {
                                RecipientOnionFields::secret_only(our_payment_secret), height + 1, &None).unwrap();
                        // Edit amt_to_forward to simulate the sender having set
                        // the final amount and the routing node taking less fee
-                       if let msgs::OutboundOnionPayload::Receive { ref mut amt_msat, .. } = onion_payloads[1] {
-                               *amt_msat = 99_000;
+                       if let msgs::OutboundOnionPayload::Receive {
+                               ref mut sender_intended_htlc_amt_msat, ..
+                       } = onion_payloads[1] {
+                               *sender_intended_htlc_amt_msat = 99_000;
                        } else { panic!() }
                        let new_onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash).unwrap();
                        payment_event.msgs[0].onion_routing_packet = new_onion_packet;
@@ -8981,6 +8979,101 @@ fn test_duplicate_temporary_channel_id_from_different_peers() {
        }
 }
 
+#[test]
+fn test_peer_funding_sidechannel() {
+       // Test that if a peer somehow learns which txid we'll use for our channel funding before we
+       // receive `funding_transaction_generated` the peer cannot cause us to crash. We'd previously
+       // assumed that LDK would receive `funding_transaction_generated` prior to our peer learning
+       // the txid and panicked if the peer tried to open a redundant channel to us with the same
+       // funding outpoint.
+       //
+       // While this assumption is generally safe, some users may have out-of-band protocols where
+       // they notify their LSP about a funding outpoint first, or this may be violated in the future
+       // with collaborative transaction construction protocols, i.e. dual-funding.
+       let chanmon_cfgs = create_chanmon_cfgs(3);
+       let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+       let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+       let temp_chan_id_ab = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0);
+       let temp_chan_id_ca = exchange_open_accept_chan(&nodes[2], &nodes[0], 1_000_000, 0);
+
+       let (_, tx, funding_output) =
+               create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
+
+       let cs_funding_events = nodes[2].node.get_and_clear_pending_events();
+       assert_eq!(cs_funding_events.len(), 1);
+       match cs_funding_events[0] {
+               Event::FundingGenerationReady { .. } => {}
+               _ => panic!("Unexpected event {:?}", cs_funding_events),
+       }
+
+       nodes[2].node.funding_transaction_generated_unchecked(&temp_chan_id_ca, &nodes[0].node.get_our_node_id(), tx.clone(), funding_output.index).unwrap();
+       let funding_created_msg = get_event_msg!(nodes[2], MessageSendEvent::SendFundingCreated, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_funding_created(&nodes[2].node.get_our_node_id(), &funding_created_msg);
+       get_event_msg!(nodes[0], MessageSendEvent::SendFundingSigned, nodes[2].node.get_our_node_id());
+       expect_channel_pending_event(&nodes[0], &nodes[2].node.get_our_node_id());
+       check_added_monitors!(nodes[0], 1);
+
+       let res = nodes[0].node.funding_transaction_generated(&temp_chan_id_ab, &nodes[1].node.get_our_node_id(), tx.clone());
+       let err_msg = format!("{:?}", res.unwrap_err());
+       assert!(err_msg.contains("An existing channel using outpoint "));
+       assert!(err_msg.contains(" is open with peer"));
+       // Even though the last funding_transaction_generated errored, it still generated a
+       // SendFundingCreated. However, when the peer responds with a funding_signed it will send the
+       // appropriate error message.
+       let as_funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &as_funding_created);
+       check_added_monitors!(nodes[1], 1);
+       expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
+       let reason = ClosureReason::ProcessingError { err: format!("An existing channel using outpoint {} is open with peer {}", funding_output, nodes[2].node.get_our_node_id()), };
+       check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(funding_output.to_channel_id(), true, reason)]);
+
+       let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed);
+       get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id());
+}
+
+#[test]
+fn test_duplicate_conflicting_funding_from_second_peer() {
+       // Test that if a user tries to fund a channel with a funding outpoint they'd previously used
+       // we don't try to remove the previous ChannelMonitor. This is largely a test to ensure we
+       // don't regress in the fuzzer, as such funding getting passed our outpoint-matches checks
+       // implies the user (and our counterparty) has reused cryptographic keys across channels, which
+       // we require the user not do.
+       let chanmon_cfgs = create_chanmon_cfgs(4);
+       let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
+       let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]);
+       let nodes = create_network(4, &node_cfgs, &node_chanmgrs);
+
+       let temp_chan_id = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0);
+
+       let (_, tx, funding_output) =
+               create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42);
+
+       // Now that we have a funding outpoint, create a dummy `ChannelMonitor` and insert it into
+       // nodes[0]'s ChainMonitor so that the initial `ChannelMonitor` write fails.
+       let dummy_chan_id = create_chan_between_nodes(&nodes[2], &nodes[3]).3;
+       let dummy_monitor = get_monitor!(nodes[2], dummy_chan_id).clone();
+       nodes[0].chain_monitor.chain_monitor.watch_channel(funding_output, dummy_monitor).unwrap();
+
+       nodes[0].node.funding_transaction_generated(&temp_chan_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap();
+
+       let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg);
+       let funding_signed_msg = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id());
+       check_added_monitors!(nodes[1], 1);
+       expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id());
+
+       nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed_msg);
+       // At this point, the channel should be closed, after having generated one monitor write (the
+       // watch_channel call which failed), but zero monitor updates.
+       check_added_monitors!(nodes[0], 1);
+       get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id());
+       let err_reason = ClosureReason::ProcessingError { err: "Channel funding outpoint was a duplicate".to_owned() };
+       check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(funding_signed_msg.channel_id, true, err_reason)]);
+}
+
 #[test]
 fn test_duplicate_funding_err_in_funding() {
        // Test that if we have a live channel with one peer, then another peer comes along and tries
@@ -9131,16 +9224,16 @@ fn test_duplicate_chan_id() {
                                chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap()
                        },
                        _ => panic!("Unexpected ChannelPhase variant"),
-               }
+               }.unwrap()
        };
        check_added_monitors!(nodes[0], 0);
-       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created.unwrap());
+       nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created);
        // At this point we'll look up if the channel_id is present and immediately fail the channel
        // without trying to persist the `ChannelMonitor`.
        check_added_monitors!(nodes[1], 0);
 
        check_closed_events(&nodes[1], &[
-               ExpectedCloseEvent::from_id_reason(channel_id, false, ClosureReason::ProcessingError {
+               ExpectedCloseEvent::from_id_reason(funding_created.temporary_channel_id, false, ClosureReason::ProcessingError {
                        err: "Already had channel with the new channel_id".to_owned()
                })
        ]);
index 2e56e2fc2c0c95d2e3cb3380d1f8b50138404ead..1794aefe7ef80bda8629d4e0c08aae95afac57df 100644 (file)
@@ -695,7 +695,7 @@ pub struct OnionMessage {
        /// Used in decrypting the onion packet's payload.
        pub blinding_point: PublicKey,
        /// The full onion packet including hop data, pubkey, and hmac
-       pub onion_routing_packet: onion_message::Packet,
+       pub onion_routing_packet: onion_message::packet::Packet,
 }
 
 /// An [`update_fulfill_htlc`] message to be sent to or received from a peer.
@@ -1706,8 +1706,8 @@ mod fuzzy_internal_msgs {
                        payment_metadata: Option<Vec<u8>>,
                        keysend_preimage: Option<PaymentPreimage>,
                        custom_tlvs: Vec<(u64, Vec<u8>)>,
-                       amt_msat: u64,
-                       outgoing_cltv_value: u32,
+                       sender_intended_htlc_amt_msat: u64,
+                       cltv_expiry_height: u32,
                },
                BlindedForward {
                        short_channel_id: u64,
@@ -1717,9 +1717,9 @@ mod fuzzy_internal_msgs {
                        intro_node_blinding_point: PublicKey,
                },
                BlindedReceive {
-                       amt_msat: u64,
+                       sender_intended_htlc_amt_msat: u64,
                        total_msat: u64,
-                       outgoing_cltv_value: u32,
+                       cltv_expiry_height: u32,
                        payment_secret: PaymentSecret,
                        payment_constraints: PaymentConstraints,
                        intro_node_blinding_point: Option<PublicKey>,
@@ -1738,17 +1738,17 @@ mod fuzzy_internal_msgs {
                        payment_metadata: Option<Vec<u8>>,
                        keysend_preimage: Option<PaymentPreimage>,
                        custom_tlvs: Vec<(u64, Vec<u8>)>,
-                       amt_msat: u64,
-                       outgoing_cltv_value: u32,
+                       sender_intended_htlc_amt_msat: u64,
+                       cltv_expiry_height: u32,
                },
                BlindedForward {
                        encrypted_tlvs: Vec<u8>,
                        intro_node_blinding_point: Option<PublicKey>,
                },
                BlindedReceive {
-                       amt_msat: u64,
+                       sender_intended_htlc_amt_msat: u64,
                        total_msat: u64,
-                       outgoing_cltv_value: u32,
+                       cltv_expiry_height: u32,
                        encrypted_tlvs: Vec<u8>,
                        intro_node_blinding_point: Option<PublicKey>, // Set if the introduction node of the blinded path is the final node
                }
@@ -2245,7 +2245,8 @@ impl Readable for OnionMessage {
                let blinding_point: PublicKey = Readable::read(r)?;
                let len: u16 = Readable::read(r)?;
                let mut packet_reader = FixedLengthReader::new(r, len as u64);
-               let onion_routing_packet: onion_message::Packet = <onion_message::Packet as LengthReadable>::read(&mut packet_reader)?;
+               let onion_routing_packet: onion_message::packet::Packet =
+                       <onion_message::packet::Packet as LengthReadable>::read(&mut packet_reader)?;
                Ok(Self {
                        blinding_point,
                        onion_routing_packet,
@@ -2289,8 +2290,8 @@ impl Writeable for OutboundOnionPayload {
                                });
                        },
                        Self::Receive {
-                               ref payment_data, ref payment_metadata, ref keysend_preimage, amt_msat,
-                               outgoing_cltv_value, ref custom_tlvs,
+                               ref payment_data, ref payment_metadata, ref keysend_preimage, sender_intended_htlc_amt_msat,
+                               cltv_expiry_height, ref custom_tlvs,
                        } => {
                                // We need to update [`ln::outbound_payment::RecipientOnionFields::with_custom_tlvs`]
                                // to reject any reserved types in the experimental range if new ones are ever
@@ -2299,8 +2300,8 @@ impl Writeable for OutboundOnionPayload {
                                let mut custom_tlvs: Vec<&(u64, Vec<u8>)> = custom_tlvs.iter().chain(keysend_tlv.iter()).collect();
                                custom_tlvs.sort_unstable_by_key(|(typ, _)| *typ);
                                _encode_varint_length_prefixed_tlv!(w, {
-                                       (2, HighZeroBytesDroppedBigSize(*amt_msat), required),
-                                       (4, HighZeroBytesDroppedBigSize(*outgoing_cltv_value), required),
+                                       (2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required),
+                                       (4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required),
                                        (8, payment_data, option),
                                        (16, payment_metadata.as_ref().map(|m| WithoutLength(m)), option)
                                }, custom_tlvs.iter());
@@ -2312,12 +2313,12 @@ impl Writeable for OutboundOnionPayload {
                                });
                        },
                        Self::BlindedReceive {
-                               amt_msat, total_msat, outgoing_cltv_value, encrypted_tlvs,
+                               sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, encrypted_tlvs,
                                intro_node_blinding_point,
                        } => {
                                _encode_varint_length_prefixed_tlv!(w, {
-                                       (2, HighZeroBytesDroppedBigSize(*amt_msat), required),
-                                       (4, HighZeroBytesDroppedBigSize(*outgoing_cltv_value), required),
+                                       (2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required),
+                                       (4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required),
                                        (10, *encrypted_tlvs, required_vec),
                                        (12, intro_node_blinding_point, option),
                                        (18, HighZeroBytesDroppedBigSize(*total_msat), required)
@@ -2401,9 +2402,9 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, &NS)> for InboundOnionPayload w
                                })} => {
                                        if total_msat.unwrap_or(0) > MAX_VALUE_MSAT { return Err(DecodeError::InvalidValue) }
                                        Ok(Self::BlindedReceive {
-                                               amt_msat: amt.ok_or(DecodeError::InvalidValue)?,
+                                               sender_intended_htlc_amt_msat: amt.ok_or(DecodeError::InvalidValue)?,
                                                total_msat: total_msat.ok_or(DecodeError::InvalidValue)?,
-                                               outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?,
+                                               cltv_expiry_height: cltv_value.ok_or(DecodeError::InvalidValue)?,
                                                payment_secret,
                                                payment_constraints,
                                                intro_node_blinding_point,
@@ -2432,8 +2433,8 @@ impl<NS: Deref> ReadableArgs<(Option<PublicKey>, &NS)> for InboundOnionPayload w
                                payment_data,
                                payment_metadata: payment_metadata.map(|w| w.0),
                                keysend_preimage,
-                               amt_msat: amt.ok_or(DecodeError::InvalidValue)?,
-                               outgoing_cltv_value: cltv_value.ok_or(DecodeError::InvalidValue)?,
+                               sender_intended_htlc_amt_msat: amt.ok_or(DecodeError::InvalidValue)?,
+                               cltv_expiry_height: cltv_value.ok_or(DecodeError::InvalidValue)?,
                                custom_tlvs,
                        })
                }
@@ -4019,8 +4020,8 @@ mod tests {
                        payment_data: None,
                        payment_metadata: None,
                        keysend_preimage: None,
-                       amt_msat: 0x0badf00d01020304,
-                       outgoing_cltv_value: 0xffffffff,
+                       sender_intended_htlc_amt_msat: 0x0badf00d01020304,
+                       cltv_expiry_height: 0xffffffff,
                        custom_tlvs: vec![],
                };
                let encoded_value = outbound_msg.encode();
@@ -4030,10 +4031,10 @@ mod tests {
                let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet);
                let inbound_msg = ReadableArgs::read(&mut Cursor::new(&target_value[..]), (None, &&node_signer)).unwrap();
                if let msgs::InboundOnionPayload::Receive {
-                       payment_data: None, amt_msat, outgoing_cltv_value, ..
+                       payment_data: None, sender_intended_htlc_amt_msat, cltv_expiry_height, ..
                } = inbound_msg {
-                       assert_eq!(amt_msat, 0x0badf00d01020304);
-                       assert_eq!(outgoing_cltv_value, 0xffffffff);
+                       assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304);
+                       assert_eq!(cltv_expiry_height, 0xffffffff);
                } else { panic!(); }
        }
 
@@ -4047,8 +4048,8 @@ mod tests {
                        }),
                        payment_metadata: None,
                        keysend_preimage: None,
-                       amt_msat: 0x0badf00d01020304,
-                       outgoing_cltv_value: 0xffffffff,
+                       sender_intended_htlc_amt_msat: 0x0badf00d01020304,
+                       cltv_expiry_height: 0xffffffff,
                        custom_tlvs: vec![],
                };
                let encoded_value = outbound_msg.encode();
@@ -4062,14 +4063,14 @@ mod tests {
                                payment_secret,
                                total_msat: 0x1badca1f
                        }),
-                       amt_msat, outgoing_cltv_value,
+                       sender_intended_htlc_amt_msat, cltv_expiry_height,
                        payment_metadata: None,
                        keysend_preimage: None,
                        custom_tlvs,
                } = inbound_msg  {
                        assert_eq!(payment_secret, expected_payment_secret);
-                       assert_eq!(amt_msat, 0x0badf00d01020304);
-                       assert_eq!(outgoing_cltv_value, 0xffffffff);
+                       assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304);
+                       assert_eq!(cltv_expiry_height, 0xffffffff);
                        assert_eq!(custom_tlvs, vec![]);
                } else { panic!(); }
        }
@@ -4087,8 +4088,8 @@ mod tests {
                        payment_metadata: None,
                        keysend_preimage: None,
                        custom_tlvs: bad_type_range_tlvs,
-                       amt_msat: 0x0badf00d01020304,
-                       outgoing_cltv_value: 0xffffffff,
+                       sender_intended_htlc_amt_msat: 0x0badf00d01020304,
+                       cltv_expiry_height: 0xffffffff,
                };
                let encoded_value = msg.encode();
                let node_signer = test_utils::TestKeysInterface::new(&[42; 32], Network::Testnet);
@@ -4119,8 +4120,8 @@ mod tests {
                        payment_metadata: None,
                        keysend_preimage: None,
                        custom_tlvs: expected_custom_tlvs.clone(),
-                       amt_msat: 0x0badf00d01020304,
-                       outgoing_cltv_value: 0xffffffff,
+                       sender_intended_htlc_amt_msat: 0x0badf00d01020304,
+                       cltv_expiry_height: 0xffffffff,
                };
                let encoded_value = msg.encode();
                let target_value = <Vec<u8>>::from_hex("2e02080badf00d010203040404ffffffffff0000000146c6616b021234ff0000000146c6616f084242424242424242").unwrap();
@@ -4132,12 +4133,12 @@ mod tests {
                        payment_metadata: None,
                        keysend_preimage: None,
                        custom_tlvs,
-                       amt_msat,
-                       outgoing_cltv_value,
+                       sender_intended_htlc_amt_msat,
+                       cltv_expiry_height: outgoing_cltv_value,
                        ..
                } = inbound_msg {
                        assert_eq!(custom_tlvs, expected_custom_tlvs);
-                       assert_eq!(amt_msat, 0x0badf00d01020304);
+                       assert_eq!(sender_intended_htlc_amt_msat, 0x0badf00d01020304);
                        assert_eq!(outgoing_cltv_value, 0xffffffff);
                } else { panic!(); }
        }
index 5966dce91039db227155264deb0557a2ed2012ff..c552bf13b8efd0fa80cd396d23e2f96da5c85eba 100644 (file)
@@ -25,7 +25,7 @@ use core::ops::Deref;
 
 /// Invalid inbound onion payment.
 #[derive(Debug)]
-pub struct InboundOnionErr {
+pub struct InboundHTLCErr {
        /// BOLT 4 error code.
        pub err_code: u16,
        /// Data attached to this error.
@@ -63,7 +63,7 @@ pub(super) fn create_fwd_pending_htlc_info(
        msg: &msgs::UpdateAddHTLC, hop_data: msgs::InboundOnionPayload, hop_hmac: [u8; 32],
        new_packet_bytes: [u8; onion_utils::ONION_DATA_LEN], shared_secret: [u8; 32],
        next_packet_pubkey_opt: Option<Result<PublicKey, secp256k1::Error>>
-) -> Result<PendingHTLCInfo, InboundOnionErr> {
+) -> Result<PendingHTLCInfo, InboundHTLCErr> {
        debug_assert!(next_packet_pubkey_opt.is_some());
        let outgoing_packet = msgs::OnionPacket {
                version: 0,
@@ -85,7 +85,7 @@ pub(super) fn create_fwd_pending_htlc_info(
                        ).map_err(|()| {
                                // We should be returning malformed here if `msg.blinding_point` is set, but this is
                                // unreachable right now since we checked it in `decode_update_add_htlc_onion`.
-                               InboundOnionErr {
+                               InboundHTLCErr {
                                        msg: "Underflow calculating outbound amount or cltv value for blinded forward",
                                        err_code: INVALID_ONION_BLINDING,
                                        err_data: vec![0; 32],
@@ -94,7 +94,7 @@ pub(super) fn create_fwd_pending_htlc_info(
                        (short_channel_id, amt_to_forward, outgoing_cltv_value, Some(intro_node_blinding_point))
                },
                msgs::InboundOnionPayload::Receive { .. } | msgs::InboundOnionPayload::BlindedReceive { .. } =>
-                       return Err(InboundOnionErr {
+                       return Err(InboundHTLCErr {
                                msg: "Final Node OnionHopData provided for us as an intermediary node",
                                err_code: 0x4000 | 22,
                                err_data: Vec::new(),
@@ -120,41 +120,44 @@ pub(super) fn create_recv_pending_htlc_info(
        hop_data: msgs::InboundOnionPayload, shared_secret: [u8; 32], payment_hash: PaymentHash,
        amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool,
        counterparty_skimmed_fee_msat: Option<u64>, current_height: u32, accept_mpp_keysend: bool,
-) -> Result<PendingHTLCInfo, InboundOnionErr> {
+) -> Result<PendingHTLCInfo, InboundHTLCErr> {
        let (
-               payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, outgoing_cltv_value,
+               payment_data, keysend_preimage, custom_tlvs, onion_amt_msat, onion_cltv_expiry,
                payment_metadata, requires_blinded_error
        ) = match hop_data {
                msgs::InboundOnionPayload::Receive {
-                       payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata, ..
+                       payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
+                       cltv_expiry_height, payment_metadata, ..
                } =>
-                       (payment_data, keysend_preimage, custom_tlvs, amt_msat, outgoing_cltv_value, payment_metadata,
-                        false),
+                       (payment_data, keysend_preimage, custom_tlvs, sender_intended_htlc_amt_msat,
+                        cltv_expiry_height, payment_metadata, false),
                msgs::InboundOnionPayload::BlindedReceive {
-                       amt_msat, total_msat, outgoing_cltv_value, payment_secret, intro_node_blinding_point,
-                       payment_constraints, ..
+                       sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, payment_secret,
+                       intro_node_blinding_point, payment_constraints, ..
                } => {
-                       check_blinded_payment_constraints(amt_msat, cltv_expiry, &payment_constraints)
+                       check_blinded_payment_constraints(
+                               sender_intended_htlc_amt_msat, cltv_expiry, &payment_constraints
+                       )
                                .map_err(|()| {
-                                       InboundOnionErr {
+                                       InboundHTLCErr {
                                                err_code: INVALID_ONION_BLINDING,
                                                err_data: vec![0; 32],
                                                msg: "Amount or cltv_expiry violated blinded payment constraints",
                                        }
                                })?;
                        let payment_data = msgs::FinalOnionHopData { payment_secret, total_msat };
-                       (Some(payment_data), None, Vec::new(), amt_msat, outgoing_cltv_value, None,
-                        intro_node_blinding_point.is_none())
+                       (Some(payment_data), None, Vec::new(), sender_intended_htlc_amt_msat, cltv_expiry_height,
+                        None, intro_node_blinding_point.is_none())
                }
                msgs::InboundOnionPayload::Forward { .. } => {
-                       return Err(InboundOnionErr {
+                       return Err(InboundHTLCErr {
                                err_code: 0x4000|22,
                                err_data: Vec::new(),
                                msg: "Got non final data with an HMAC of 0",
                        })
                },
                msgs::InboundOnionPayload::BlindedForward { .. } => {
-                       return Err(InboundOnionErr {
+                       return Err(InboundHTLCErr {
                                err_code: INVALID_ONION_BLINDING,
                                err_data: vec![0; 32],
                                msg: "Got blinded non final data with an HMAC of 0",
@@ -162,8 +165,8 @@ pub(super) fn create_recv_pending_htlc_info(
                }
        };
        // final_incorrect_cltv_expiry
-       if outgoing_cltv_value > cltv_expiry {
-               return Err(InboundOnionErr {
+       if onion_cltv_expiry > cltv_expiry {
+               return Err(InboundHTLCErr {
                        msg: "Upstream node set CLTV to less than the CLTV set by the sender",
                        err_code: 18,
                        err_data: cltv_expiry.to_be_bytes().to_vec()
@@ -180,7 +183,7 @@ pub(super) fn create_recv_pending_htlc_info(
                let mut err_data = Vec::with_capacity(12);
                err_data.extend_from_slice(&amt_msat.to_be_bytes());
                err_data.extend_from_slice(&current_height.to_be_bytes());
-               return Err(InboundOnionErr {
+               return Err(InboundHTLCErr {
                        err_code: 0x4000 | 15, err_data,
                        msg: "The final CLTV expiry is too soon to handle",
                });
@@ -189,7 +192,7 @@ pub(super) fn create_recv_pending_htlc_info(
                (allow_underpay && onion_amt_msat >
                 amt_msat.saturating_add(counterparty_skimmed_fee_msat.unwrap_or(0)))
        {
-               return Err(InboundOnionErr {
+               return Err(InboundHTLCErr {
                        err_code: 19,
                        err_data: amt_msat.to_be_bytes().to_vec(),
                        msg: "Upstream node sent less than we were supposed to receive in payment",
@@ -204,14 +207,14 @@ pub(super) fn create_recv_pending_htlc_info(
                // time discrepancies due to a hash collision with X.
                let hashed_preimage = PaymentHash(Sha256::hash(&payment_preimage.0).to_byte_array());
                if hashed_preimage != payment_hash {
-                       return Err(InboundOnionErr {
+                       return Err(InboundHTLCErr {
                                err_code: 0x4000|22,
                                err_data: Vec::new(),
                                msg: "Payment preimage didn't match payment hash",
                        });
                }
                if !accept_mpp_keysend && payment_data.is_some() {
-                       return Err(InboundOnionErr {
+                       return Err(InboundHTLCErr {
                                err_code: 0x4000|22,
                                err_data: Vec::new(),
                                msg: "We don't support MPP keysend payments",
@@ -221,20 +224,20 @@ pub(super) fn create_recv_pending_htlc_info(
                        payment_data,
                        payment_preimage,
                        payment_metadata,
-                       incoming_cltv_expiry: outgoing_cltv_value,
+                       incoming_cltv_expiry: onion_cltv_expiry,
                        custom_tlvs,
                }
        } else if let Some(data) = payment_data {
                PendingHTLCRouting::Receive {
                        payment_data: data,
                        payment_metadata,
-                       incoming_cltv_expiry: outgoing_cltv_value,
+                       incoming_cltv_expiry: onion_cltv_expiry,
                        phantom_shared_secret,
                        custom_tlvs,
                        requires_blinded_error,
                }
        } else {
-               return Err(InboundOnionErr {
+               return Err(InboundHTLCErr {
                        err_code: 0x4000|0x2000|3,
                        err_data: Vec::new(),
                        msg: "We require payment_secrets",
@@ -246,7 +249,7 @@ pub(super) fn create_recv_pending_htlc_info(
                incoming_shared_secret: shared_secret,
                incoming_amt_msat: Some(amt_msat),
                outgoing_amt_msat: onion_amt_msat,
-               outgoing_cltv_value,
+               outgoing_cltv_value: onion_cltv_expiry,
                skimmed_fee_msat: counterparty_skimmed_fee_msat,
        })
 }
@@ -263,7 +266,7 @@ pub(super) fn create_recv_pending_htlc_info(
 pub fn peel_payment_onion<NS: Deref, L: Deref, T: secp256k1::Verification>(
        msg: &msgs::UpdateAddHTLC, node_signer: &NS, logger: &L, secp_ctx: &Secp256k1<T>,
        cur_height: u32, accept_mpp_keysend: bool, allow_skimmed_fees: bool,
-) -> Result<PendingHTLCInfo, InboundOnionErr>
+) -> Result<PendingHTLCInfo, InboundHTLCErr>
 where
        NS::Target: NodeSigner,
        L::Target: Logger,
@@ -276,7 +279,7 @@ where
                        HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason.data),
                };
                let msg = "Failed to decode update add htlc onion";
-               InboundOnionErr { msg, err_code, err_data }
+               InboundHTLCErr { msg, err_code, err_data }
        })?;
        Ok(match hop {
                onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
@@ -285,7 +288,7 @@ where
                        } = match next_packet_details_opt {
                                Some(next_packet_details) => next_packet_details,
                                // Forward should always include the next hop details
-                               None => return Err(InboundOnionErr {
+                               None => return Err(InboundHTLCErr {
                                        msg: "Failed to decode update add htlc onion",
                                        err_code: 0x4000 | 22,
                                        err_data: Vec::new(),
@@ -295,7 +298,7 @@ where
                        if let Err((err_msg, code)) = check_incoming_htlc_cltv(
                                cur_height, outgoing_cltv_value, msg.cltv_expiry
                        ) {
-                               return Err(InboundOnionErr {
+                               return Err(InboundHTLCErr {
                                        msg: err_msg,
                                        err_code: code,
                                        err_data: Vec::new(),
index 99e3e965a9a9aab167928198db79b82054c98066..ac0bb6189c6d750e92b4fdf5f6d061cca46d2cb2 100644 (file)
@@ -190,9 +190,9 @@ pub(super) fn build_onion_payloads(path: &Path, total_msat: u64, mut recipient_o
                                                cur_value_msat += final_value_msat;
                                                cur_cltv += excess_final_cltv_expiry_delta;
                                                res.push(msgs::OutboundOnionPayload::BlindedReceive {
-                                                       amt_msat: *final_value_msat,
+                                                       sender_intended_htlc_amt_msat: *final_value_msat,
                                                        total_msat,
-                                                       outgoing_cltv_value: cltv,
+                                                       cltv_expiry_height: cltv,
                                                        encrypted_tlvs: blinded_hop.encrypted_payload.clone(),
                                                        intro_node_blinding_point: blinding_point.take(),
                                                });
@@ -214,8 +214,8 @@ pub(super) fn build_onion_payloads(path: &Path, total_msat: u64, mut recipient_o
                                        payment_metadata: recipient_onion.payment_metadata.take(),
                                        keysend_preimage: *keysend_preimage,
                                        custom_tlvs: recipient_onion.custom_tlvs.clone(),
-                                       amt_msat: value_msat,
-                                       outgoing_cltv_value: cltv,
+                                       sender_intended_htlc_amt_msat: value_msat,
+                                       cltv_expiry_height: cltv,
                                });
                        }
                } else {
index e1748dca7f2ac11978035b2b44c4b66f8a2973d0..f5396046177fe40dbb02295bd6419b8208f41052 100644 (file)
@@ -243,7 +243,7 @@ impl PendingOutboundPayment {
                if insert_res {
                        if let PendingOutboundPayment::Retryable {
                                ref mut pending_amt_msat, ref mut pending_fee_msat,
-                               ref mut remaining_max_total_routing_fee_msat, .. 
+                               ref mut remaining_max_total_routing_fee_msat, ..
                        } = self {
                                        *pending_amt_msat += path.final_value_msat();
                                        let path_fee_msat = path.fee_msat();
index 6af0e63c98bf421bade346f89f1fff303f48e764..73cdf59bbb699fb6966ad964ac99d6f00f3efeb9 100644 (file)
@@ -43,10 +43,9 @@ use crate::ln::functional_test_utils;
 use crate::ln::functional_test_utils::*;
 use crate::routing::gossip::NodeId;
 #[cfg(feature = "std")]
-use {
-       crate::util::time::tests::SinceEpoch,
-       std::time::{SystemTime, Instant, Duration}
-};
+use std::time::{SystemTime, Instant, Duration};
+#[cfg(not(feature = "no-std"))]
+use crate::util::time::tests::SinceEpoch;
 
 #[test]
 fn mpp_failure() {
index a4e0042414f8996f95c08deb72d7a578322fff97..6ffffec4dd0bd94febc085ed998051c7090da5eb 100644 (file)
@@ -18,7 +18,7 @@
 use bitcoin::blockdata::constants::ChainHash;
 use bitcoin::secp256k1::{self, Secp256k1, SecretKey, PublicKey};
 
-use crate::sign::{KeysManager, NodeSigner, Recipient};
+use crate::sign::{NodeSigner, Recipient};
 use crate::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider};
 use crate::ln::ChannelId;
 use crate::ln::features::{InitFeatures, NodeFeatures};
@@ -31,9 +31,11 @@ use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor, NextNoiseStep, Mes
 use crate::ln::wire;
 use crate::ln::wire::{Encode, Type};
 #[cfg(not(c_bindings))]
-use crate::onion_message::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
-use crate::onion_message::{CustomOnionMessageHandler, OffersMessage, OffersMessageHandler, OnionMessageContents, PendingOnionMessage};
-use crate::routing::gossip::{NetworkGraph, P2PGossipSync, NodeId, NodeAlias};
+use crate::onion_message::messenger::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
+use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage};
+use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
+use crate::onion_message::packet::OnionMessageContents;
+use crate::routing::gossip::{NodeId, NodeAlias};
 use crate::util::atomic_counter::AtomicCounter;
 use crate::util::logger::{Logger, WithContext};
 use crate::util::string::PrintableString;
@@ -41,12 +43,19 @@ use crate::util::string::PrintableString;
 use crate::prelude::*;
 use crate::io;
 use alloc::collections::VecDeque;
-use crate::sync::{Arc, Mutex, MutexGuard, FairRwLock};
+use crate::sync::{Mutex, MutexGuard, FairRwLock};
 use core::sync::atomic::{AtomicBool, AtomicU32, AtomicI32, Ordering};
 use core::{cmp, hash, fmt, mem};
 use core::ops::Deref;
 use core::convert::Infallible;
-#[cfg(feature = "std")] use std::error;
+#[cfg(feature = "std")]
+use std::error;
+#[cfg(not(c_bindings))]
+use {
+       crate::routing::gossip::{NetworkGraph, P2PGossipSync},
+       crate::sign::KeysManager,
+       crate::sync::Arc,
+};
 
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::sha256::HashEngine as Sha256Engine;
@@ -377,7 +386,7 @@ pub struct MessageHandler<CM: Deref, RM: Deref, OM: Deref, CustomM: Deref> where
        /// A message handler which handles onion messages. This should generally be an
        /// [`OnionMessenger`], but can also be an [`IgnoringMessageHandler`].
        ///
-       /// [`OnionMessenger`]: crate::onion_message::OnionMessenger
+       /// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger
        pub onion_message_handler: OM,
 
        /// A message handler which handles custom messages. The only LDK-provided implementation is
index b3d52b78f2b5a5a3921c66873325fde418601b2b..1ac290383a48761fa1e892bbc01fc73685aad983 100644 (file)
@@ -15,16 +15,14 @@ use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, ChannelMonitor};
 use crate::sign::EntropySource;
 use crate::chain::transaction::OutPoint;
 use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
-use crate::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, Retry, RecipientOnionFields};
+use crate::ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, PaymentId, RecipientOnionFields};
 use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
-use crate::routing::router::{RouteParameters, PaymentParameters};
 use crate::util::test_channel_signer::TestChannelSigner;
 use crate::util::test_utils;
 use crate::util::errors::APIError;
 use crate::util::ser::{Writeable, ReadableArgs};
 use crate::util::config::UserConfig;
-use crate::util::string::UntrustedString;
 
 use bitcoin::hash_types::BlockHash;
 
@@ -496,6 +494,9 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 
 #[cfg(feature = "std")]
 fn do_test_data_loss_protect(reconnect_panicing: bool, substantially_old: bool, not_stale: bool) {
+       use crate::routing::router::{RouteParameters, PaymentParameters};
+       use crate::ln::channelmanager::Retry;
+       use crate::util::string::UntrustedString;
        // When we get a data_loss_protect proving we're behind, we immediately panic as the
        // chain::Watch API requirements have been violated (e.g. the user restored from a backup). The
        // panic message informs the user they should force-close without broadcasting, which is tested
index b0031a6c3f4e05cd41823ceb9fa785ab3fb91722..23d27b18943c7514af3224e16904268c08df999e 100644 (file)
@@ -16,7 +16,9 @@ use crate::ln::msgs::{self, DecodeError, OnionMessageHandler, SocketAddress};
 use crate::sign::{EntropySource, NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::{CustomOnionMessageHandler, Destination, MessageRouter, OffersMessage, OffersMessageHandler, OnionMessageContents, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
+use super::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
+use super::offers::{OffersMessage, OffersMessageHandler};
+use super::packet::{OnionMessageContents, Packet};
 
 use bitcoin::network::constants::Network;
 use bitcoin::hashes::hex::FromHex;
@@ -571,8 +573,8 @@ fn spec_test_vector() {
        let sender_to_alice_packet_bytes_len = sender_to_alice_packet_bytes.len() as u64;
        let mut reader = io::Cursor::new(sender_to_alice_packet_bytes);
        let mut packet_reader = FixedLengthReader::new(&mut reader, sender_to_alice_packet_bytes_len);
-       let sender_to_alice_packet: super::Packet =
-               <super::Packet as LengthReadable>::read(&mut packet_reader).unwrap();
+       let sender_to_alice_packet: Packet =
+               <Packet as LengthReadable>::read(&mut packet_reader).unwrap();
        let secp_ctx = Secp256k1::new();
        let sender_to_alice_om = msgs::OnionMessage {
                blinding_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&<Vec<u8>>::from_hex("6363636363636363636363636363636363636363636363636363636363636363").unwrap()).unwrap()),
index 7743270e257b039301b4eaa9cdab681e242590b3..5dc3bb422e67efffc880465ad5ab3769ce48fc7a 100644 (file)
@@ -7,8 +7,8 @@
 // You may not use this file except in accordance with one or both of these
 // licenses.
 
-//! LDK sends, receives, and forwards onion messages via the [`OnionMessenger`]. See its docs for
-//! more information.
+//! LDK sends, receives, and forwards onion messages via this [`OnionMessenger`], which lives here,
+//! as well as various types, traits, and utilities that it uses.
 
 use bitcoin::hashes::{Hash, HashEngine};
 use bitcoin::hashes::hmac::{Hmac, HmacEngine};
@@ -19,15 +19,14 @@ use crate::blinded_path::BlindedPath;
 use crate::blinded_path::message::{advance_path_by_one, ForwardTlvs, ReceiveTlvs};
 use crate::blinded_path::utils;
 use crate::events::{Event, EventHandler, EventsProvider};
-use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient};
+use crate::sign::{EntropySource, NodeSigner, Recipient};
 #[cfg(not(c_bindings))]
 use crate::ln::channelmanager::{SimpleArcChannelManager, SimpleRefChannelManager};
 use crate::ln::features::{InitFeatures, NodeFeatures};
 use crate::ln::msgs::{self, OnionMessage, OnionMessageHandler, SocketAddress};
 use crate::ln::onion_utils;
-use crate::ln::peer_handler::IgnoringMessageHandler;
 use crate::routing::gossip::{NetworkGraph, NodeId};
-pub use super::packet::OnionMessageContents;
+use super::packet::OnionMessageContents;
 use super::packet::ParsedOnionMessageContents;
 use super::offers::OffersMessageHandler;
 use super::packet::{BIG_PACKET_HOP_DATA_LEN, ForwardControlTlvs, Packet, Payload, ReceiveControlTlvs, SMALL_PACKET_HOP_DATA_LEN};
@@ -37,9 +36,16 @@ use crate::util::ser::Writeable;
 use core::fmt;
 use core::ops::Deref;
 use crate::io;
-use crate::sync::{Arc, Mutex};
+use crate::sync::Mutex;
 use crate::prelude::*;
 
+#[cfg(not(c_bindings))]
+use {
+       crate::sign::KeysManager,
+       crate::ln::peer_handler::IgnoringMessageHandler,
+       crate::sync::Arc,
+};
+
 pub(super) const MAX_TIMER_TICKS: usize = 2;
 
 /// A sender, receiver and forwarder of [`OnionMessage`]s.
@@ -68,7 +74,8 @@ pub(super) const MAX_TIMER_TICKS: usize = 2;
 /// # use lightning::blinded_path::BlindedPath;
 /// # use lightning::sign::{EntropySource, KeysManager};
 /// # use lightning::ln::peer_handler::IgnoringMessageHandler;
-/// # use lightning::onion_message::{OnionMessageContents, Destination, MessageRouter, OnionMessagePath, OnionMessenger};
+/// # use lightning::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath, OnionMessenger};
+/// # use lightning::onion_message::packet::OnionMessageContents;
 /// # use lightning::util::logger::{Logger, Record};
 /// # use lightning::util::ser::{Writeable, Writer};
 /// # use lightning::io;
@@ -258,7 +265,7 @@ pub struct PendingOnionMessage<T: OnionMessageContents> {
 ///
 /// These are obtained when released from [`OnionMessenger`]'s handlers after which they are
 /// enqueued for sending.
-pub type PendingOnionMessage<T: OnionMessageContents> = (T, Destination, Option<BlindedPath>);
+pub type PendingOnionMessage<T> = (T, Destination, Option<BlindedPath>);
 
 pub(crate) fn new_pending_onion_message<T: OnionMessageContents>(
        contents: T, destination: Destination, reply_path: Option<BlindedPath>
index d106a542fd368156f0a4f7959c37919d8e1821ec..05a8b7d6fbd033922fb140ea97d326287aec8757 100644 (file)
 //!
 //! [offers]: <https://github.com/lightning/bolts/pull/798>
 //! [blinded paths]: crate::blinded_path::BlindedPath
+//! [`OnionMessenger`]: self::messenger::OnionMessenger
 
-mod messenger;
-mod offers;
-mod packet;
+pub mod messenger;
+pub mod offers;
+pub mod packet;
 #[cfg(test)]
 mod functional_tests;
-
-// Re-export structs so they can be imported with just the `onion_message::` module prefix.
-pub use self::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, MessageRouter, OnionMessageContents, OnionMessagePath, OnionMessenger, PeeledOnion, PendingOnionMessage, SendError};
-pub use self::messenger::{create_onion_message, peel_onion_message};
-#[cfg(not(c_bindings))]
-pub use self::messenger::{SimpleArcOnionMessenger, SimpleRefOnionMessenger};
-pub use self::offers::{OffersMessage, OffersMessageHandler};
-pub use self::packet::{Packet, ParsedOnionMessageContents};
-pub(crate) use self::packet::ControlTlvs;
-pub(crate) use self::messenger::new_pending_onion_message;
index fb6ff746717ed1daeb731032ee2fd0145428b53f..6672b5dfcdd0f3e33025c4bfe0654d6cfd2b0e56 100644 (file)
@@ -17,10 +17,11 @@ use crate::offers::invoice_error::InvoiceError;
 use crate::offers::invoice_request::InvoiceRequest;
 use crate::offers::invoice::Bolt12Invoice;
 use crate::offers::parse::Bolt12ParseError;
-use crate::onion_message::OnionMessageContents;
-use crate::onion_message::messenger::PendingOnionMessage;
+use crate::onion_message::packet::OnionMessageContents;
 use crate::util::logger::Logger;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
+#[cfg(not(c_bindings))]
+use crate::onion_message::messenger::PendingOnionMessage;
 
 use crate::prelude::*;
 
@@ -38,7 +39,7 @@ pub trait OffersMessageHandler {
        ///
        /// The returned [`OffersMessage`], if any, is enqueued to be sent by [`OnionMessenger`].
        ///
-       /// [`OnionMessenger`]: crate::onion_message::OnionMessenger
+       /// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger
        fn handle_message(&self, message: OffersMessage) -> Option<OffersMessage>;
 
        /// Releases any [`OffersMessage`]s that need to be sent.
@@ -53,7 +54,7 @@ pub trait OffersMessageHandler {
        /// Typically, this is used for messages initiating a payment flow rather than in response to
        /// another message. The latter should use the return value of [`Self::handle_message`].
        #[cfg(c_bindings)]
-       fn release_pending_messages(&self) -> Vec<(OffersMessage, crate::onion_message::Destination, Option<crate::blinded_path::BlindedPath>)> { vec![] }
+       fn release_pending_messages(&self) -> Vec<(OffersMessage, crate::onion_message::messenger::Destination, Option<crate::blinded_path::BlindedPath>)> { vec![] }
 }
 
 /// Possible BOLT 12 Offers messages sent and received via an [`OnionMessage`].
index 2359c837201e1738e8d301a5d39fb8f75f4417dc..277661862e3361d72a3b3b44975a928422a87db9 100644 (file)
@@ -20,7 +20,7 @@ use crate::ln::channelmanager::{ChannelDetails, PaymentId};
 use crate::ln::features::{BlindedHopFeatures, Bolt11InvoiceFeatures, Bolt12InvoiceFeatures, ChannelFeatures, NodeFeatures};
 use crate::ln::msgs::{DecodeError, ErrorAction, LightningError, MAX_VALUE_MSAT};
 use crate::offers::invoice::{BlindedPayInfo, Bolt12Invoice};
-use crate::onion_message::{DefaultMessageRouter, Destination, MessageRouter, OnionMessagePath};
+use crate::onion_message::messenger::{DefaultMessageRouter, Destination, MessageRouter, OnionMessagePath};
 use crate::routing::gossip::{DirectedChannelInfo, EffectiveCapacity, ReadOnlyNetworkGraph, NetworkGraph, NodeId, RoutingFees};
 use crate::routing::scoring::{ChannelUsage, LockableScore, ScoreLookUp};
 use crate::sign::EntropySource;
@@ -1119,6 +1119,68 @@ const _GRAPH_NODE_SMALL: usize = 64 - core::mem::size_of::<RouteGraphNode>();
 #[cfg(any(ldk_bench, not(any(test, fuzzing))))]
 const _GRAPH_NODE_FIXED_SIZE: usize = core::mem::size_of::<RouteGraphNode>() - 64;
 
+/// A [`CandidateRouteHop::FirstHop`] entry.
+#[derive(Clone, Debug)]
+pub struct FirstHopCandidate<'a> {
+       /// Channel details of the first hop
+       ///
+       /// [`ChannelDetails::get_outbound_payment_scid`] MUST be `Some` (indicating the channel
+       /// has been funded and is able to pay), and accessor methods may panic otherwise.
+       ///
+       /// [`find_route`] validates this prior to constructing a [`CandidateRouteHop`].
+       pub details: &'a ChannelDetails,
+       /// The node id of the payer, which is also the source side of this candidate route hop.
+       pub payer_node_id: &'a NodeId,
+}
+
+/// A [`CandidateRouteHop::PublicHop`] entry.
+#[derive(Clone, Debug)]
+pub struct PublicHopCandidate<'a> {
+       /// Information about the channel, including potentially its capacity and
+       /// direction-specific information.
+       pub info: DirectedChannelInfo<'a>,
+       /// The short channel ID of the channel, i.e. the identifier by which we refer to this
+       /// channel.
+       pub short_channel_id: u64,
+}
+
+/// A [`CandidateRouteHop::PrivateHop`] entry.
+#[derive(Clone, Debug)]
+pub struct PrivateHopCandidate<'a> {
+       /// Information about the private hop communicated via BOLT 11.
+       pub hint: &'a RouteHintHop,
+       /// Node id of the next hop in BOLT 11 route hint.
+       pub target_node_id: &'a NodeId
+}
+
+/// A [`CandidateRouteHop::Blinded`] entry.
+#[derive(Clone, Debug)]
+pub struct BlindedPathCandidate<'a> {
+       /// Information about the blinded path including the fee, HTLC amount limits, and
+       /// cryptographic material required to build an HTLC through the given path.
+       pub hint: &'a (BlindedPayInfo, BlindedPath),
+       /// Index of the hint in the original list of blinded hints.
+       ///
+       /// This is used to cheaply uniquely identify this blinded path, even though we don't have
+       /// a short channel ID for this hop.
+       hint_idx: usize,
+}
+
+/// A [`CandidateRouteHop::OneHopBlinded`] entry.
+#[derive(Clone, Debug)]
+pub struct OneHopBlindedPathCandidate<'a> {
+       /// Information about the blinded path including the fee, HTLC amount limits, and
+       /// cryptographic material required to build an HTLC terminating with the given path.
+       ///
+       /// Note that the [`BlindedPayInfo`] is ignored here.
+       pub hint: &'a (BlindedPayInfo, BlindedPath),
+       /// Index of the hint in the original list of blinded hints.
+       ///
+       /// This is used to cheaply uniquely identify this blinded path, even though we don't have
+       /// a short channel ID for this hop.
+       hint_idx: usize,
+}
+
 /// A wrapper around the various hop representations.
 ///
 /// Can be used to examine the properties of a hop,
@@ -1126,36 +1188,14 @@ const _GRAPH_NODE_FIXED_SIZE: usize = core::mem::size_of::<RouteGraphNode>() - 6
 #[derive(Clone, Debug)]
 pub enum CandidateRouteHop<'a> {
        /// A hop from the payer, where the outbound liquidity is known.
-       FirstHop {
-               /// Channel details of the first hop
-               ///
-               /// [`ChannelDetails::get_outbound_payment_scid`] MUST be `Some` (indicating the channel
-               /// has been funded and is able to pay), and accessor methods may panic otherwise.
-               ///
-               /// [`find_route`] validates this prior to constructing a [`CandidateRouteHop`].
-               details: &'a ChannelDetails,
-               /// The node id of the payer, which is also the source side of this candidate route hop.
-               payer_node_id: &'a NodeId,
-       },
+       FirstHop(FirstHopCandidate<'a>),
        /// A hop found in the [`ReadOnlyNetworkGraph`].
-       PublicHop {
-               /// Information about the channel, including potentially its capacity and
-               /// direction-specific information.
-               info: DirectedChannelInfo<'a>,
-               /// The short channel ID of the channel, i.e. the identifier by which we refer to this
-               /// channel.
-               short_channel_id: u64,
-       },
+       PublicHop(PublicHopCandidate<'a>),
        /// A private hop communicated by the payee, generally via a BOLT 11 invoice.
        ///
        /// Because BOLT 11 route hints can take multiple hops to get to the destination, this may not
        /// terminate at the payee.
-       PrivateHop {
-               /// Information about the private hop communicated via BOLT 11.
-               hint: &'a RouteHintHop,
-               /// Node id of the next hop in BOLT 11 route hint.
-               target_node_id: &'a NodeId
-       },
+       PrivateHop(PrivateHopCandidate<'a>),
        /// A blinded path which starts with an introduction point and ultimately terminates with the
        /// payee.
        ///
@@ -1164,16 +1204,7 @@ pub enum CandidateRouteHop<'a> {
        ///
        /// Because blinded paths are "all or nothing", and we cannot use just one part of a blinded
        /// path, the full path is treated as a single [`CandidateRouteHop`].
-       Blinded {
-               /// Information about the blinded path including the fee, HTLC amount limits, and
-               /// cryptographic material required to build an HTLC through the given path.
-               hint: &'a (BlindedPayInfo, BlindedPath),
-               /// Index of the hint in the original list of blinded hints.
-               ///
-               /// This is used to cheaply uniquely identify this blinded path, even though we don't have
-               /// a short channel ID for this hop.
-               hint_idx: usize,
-       },
+       Blinded(BlindedPathCandidate<'a>),
        /// Similar to [`Self::Blinded`], but the path here only has one hop.
        ///
        /// While we treat this similarly to [`CandidateRouteHop::Blinded`] in many respects (e.g.
@@ -1185,18 +1216,7 @@ pub enum CandidateRouteHop<'a> {
        ///
        /// This primarily exists to track that we need to included a blinded path at the end of our
        /// [`Route`], even though it doesn't actually add an additional hop in the payment.
-       OneHopBlinded {
-               /// Information about the blinded path including the fee, HTLC amount limits, and
-               /// cryptographic material required to build an HTLC terminating with the given path.
-               ///
-               /// Note that the [`BlindedPayInfo`] is ignored here.
-               hint: &'a (BlindedPayInfo, BlindedPath),
-               /// Index of the hint in the original list of blinded hints.
-               ///
-               /// This is used to cheaply uniquely identify this blinded path, even though we don't have
-               /// a short channel ID for this hop.
-               hint_idx: usize,
-       },
+       OneHopBlinded(OneHopBlindedPathCandidate<'a>),
 }
 
 impl<'a> CandidateRouteHop<'a> {
@@ -1218,11 +1238,11 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        fn short_channel_id(&self) -> Option<u64> {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => details.get_outbound_payment_scid(),
-                       CandidateRouteHop::PublicHop { short_channel_id, .. } => Some(*short_channel_id),
-                       CandidateRouteHop::PrivateHop { hint, .. } => Some(hint.short_channel_id),
-                       CandidateRouteHop::Blinded { .. } => None,
-                       CandidateRouteHop::OneHopBlinded { .. } => None,
+                       CandidateRouteHop::FirstHop(hop) => hop.details.get_outbound_payment_scid(),
+                       CandidateRouteHop::PublicHop(hop) => Some(hop.short_channel_id),
+                       CandidateRouteHop::PrivateHop(hop) => Some(hop.hint.short_channel_id),
+                       CandidateRouteHop::Blinded(_) => None,
+                       CandidateRouteHop::OneHopBlinded(_) => None,
                }
        }
 
@@ -1234,22 +1254,22 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn globally_unique_short_channel_id(&self) -> Option<u64> {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => if details.is_public { details.short_channel_id } else { None },
-                       CandidateRouteHop::PublicHop { short_channel_id, .. } => Some(*short_channel_id),
-                       CandidateRouteHop::PrivateHop { .. } => None,
-                       CandidateRouteHop::Blinded { .. } => None,
-                       CandidateRouteHop::OneHopBlinded { .. } => None,
+                       CandidateRouteHop::FirstHop(hop) => if hop.details.is_public { hop.details.short_channel_id } else { None },
+                       CandidateRouteHop::PublicHop(hop) => Some(hop.short_channel_id),
+                       CandidateRouteHop::PrivateHop(_) => None,
+                       CandidateRouteHop::Blinded(_) => None,
+                       CandidateRouteHop::OneHopBlinded(_) => None,
                }
        }
 
        // NOTE: This may alloc memory so avoid calling it in a hot code path.
        fn features(&self) -> ChannelFeatures {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => details.counterparty.features.to_context(),
-                       CandidateRouteHop::PublicHop { info, .. } => info.channel().features.clone(),
-                       CandidateRouteHop::PrivateHop { .. } => ChannelFeatures::empty(),
-                       CandidateRouteHop::Blinded { .. } => ChannelFeatures::empty(),
-                       CandidateRouteHop::OneHopBlinded { .. } => ChannelFeatures::empty(),
+                       CandidateRouteHop::FirstHop(hop) => hop.details.counterparty.features.to_context(),
+                       CandidateRouteHop::PublicHop(hop) => hop.info.channel().features.clone(),
+                       CandidateRouteHop::PrivateHop(_) => ChannelFeatures::empty(),
+                       CandidateRouteHop::Blinded(_) => ChannelFeatures::empty(),
+                       CandidateRouteHop::OneHopBlinded(_) => ChannelFeatures::empty(),
                }
        }
 
@@ -1261,11 +1281,11 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn cltv_expiry_delta(&self) -> u32 {
                match self {
-                       CandidateRouteHop::FirstHop { .. } => 0,
-                       CandidateRouteHop::PublicHop { info, .. } => info.direction().cltv_expiry_delta as u32,
-                       CandidateRouteHop::PrivateHop { hint, .. } => hint.cltv_expiry_delta as u32,
-                       CandidateRouteHop::Blinded { hint, .. } => hint.0.cltv_expiry_delta as u32,
-                       CandidateRouteHop::OneHopBlinded { .. } => 0,
+                       CandidateRouteHop::FirstHop(_) => 0,
+                       CandidateRouteHop::PublicHop(hop) => hop.info.direction().cltv_expiry_delta as u32,
+                       CandidateRouteHop::PrivateHop(hop) => hop.hint.cltv_expiry_delta as u32,
+                       CandidateRouteHop::Blinded(hop) => hop.hint.0.cltv_expiry_delta as u32,
+                       CandidateRouteHop::OneHopBlinded(_) => 0,
                }
        }
 
@@ -1273,10 +1293,10 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn htlc_minimum_msat(&self) -> u64 {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => details.next_outbound_htlc_minimum_msat,
-                       CandidateRouteHop::PublicHop { info, .. } => info.direction().htlc_minimum_msat,
-                       CandidateRouteHop::PrivateHop { hint, .. } => hint.htlc_minimum_msat.unwrap_or(0),
-                       CandidateRouteHop::Blinded { hint, .. } => hint.0.htlc_minimum_msat,
+                       CandidateRouteHop::FirstHop(hop) => hop.details.next_outbound_htlc_minimum_msat,
+                       CandidateRouteHop::PublicHop(hop) => hop.info.direction().htlc_minimum_msat,
+                       CandidateRouteHop::PrivateHop(hop) => hop.hint.htlc_minimum_msat.unwrap_or(0),
+                       CandidateRouteHop::Blinded(hop) => hop.hint.0.htlc_minimum_msat,
                        CandidateRouteHop::OneHopBlinded { .. } => 0,
                }
        }
@@ -1285,18 +1305,18 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn fees(&self) -> RoutingFees {
                match self {
-                       CandidateRouteHop::FirstHop { .. } => RoutingFees {
+                       CandidateRouteHop::FirstHop(_) => RoutingFees {
                                base_msat: 0, proportional_millionths: 0,
                        },
-                       CandidateRouteHop::PublicHop { info, .. } => info.direction().fees,
-                       CandidateRouteHop::PrivateHop { hint, .. } => hint.fees,
-                       CandidateRouteHop::Blinded { hint, .. } => {
+                       CandidateRouteHop::PublicHop(hop) => hop.info.direction().fees,
+                       CandidateRouteHop::PrivateHop(hop) => hop.hint.fees,
+                       CandidateRouteHop::Blinded(hop) => {
                                RoutingFees {
-                                       base_msat: hint.0.fee_base_msat,
-                                       proportional_millionths: hint.0.fee_proportional_millionths
+                                       base_msat: hop.hint.0.fee_base_msat,
+                                       proportional_millionths: hop.hint.0.fee_proportional_millionths
                                }
                        },
-                       CandidateRouteHop::OneHopBlinded { .. } =>
+                       CandidateRouteHop::OneHopBlinded(_) =>
                                RoutingFees { base_msat: 0, proportional_millionths: 0 },
                }
        }
@@ -1307,17 +1327,17 @@ impl<'a> CandidateRouteHop<'a> {
        /// cached!
        fn effective_capacity(&self) -> EffectiveCapacity {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => EffectiveCapacity::ExactLiquidity {
-                               liquidity_msat: details.next_outbound_htlc_limit_msat,
+                       CandidateRouteHop::FirstHop(hop) => EffectiveCapacity::ExactLiquidity {
+                               liquidity_msat: hop.details.next_outbound_htlc_limit_msat,
                        },
-                       CandidateRouteHop::PublicHop { info, .. } => info.effective_capacity(),
-                       CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: Some(max), .. }, .. } =>
+                       CandidateRouteHop::PublicHop(hop) => hop.info.effective_capacity(),
+                       CandidateRouteHop::PrivateHop(PrivateHopCandidate { hint: RouteHintHop { htlc_maximum_msat: Some(max), .. }, .. }) =>
                                EffectiveCapacity::HintMaxHTLC { amount_msat: *max },
-                       CandidateRouteHop::PrivateHop { hint: RouteHintHop { htlc_maximum_msat: None, .. }, .. } =>
+                       CandidateRouteHop::PrivateHop(PrivateHopCandidate { hint: RouteHintHop { htlc_maximum_msat: None, .. }, .. }) =>
                                EffectiveCapacity::Infinite,
-                       CandidateRouteHop::Blinded { hint, .. } =>
-                               EffectiveCapacity::HintMaxHTLC { amount_msat: hint.0.htlc_maximum_msat },
-                       CandidateRouteHop::OneHopBlinded { .. } => EffectiveCapacity::Infinite,
+                       CandidateRouteHop::Blinded(hop) =>
+                               EffectiveCapacity::HintMaxHTLC { amount_msat: hop.hint.0.htlc_maximum_msat },
+                       CandidateRouteHop::OneHopBlinded(_) => EffectiveCapacity::Infinite,
                }
        }
 
@@ -1327,14 +1347,14 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        fn id(&self) -> CandidateHopId {
                match self {
-                       CandidateRouteHop::Blinded { hint_idx, .. } => CandidateHopId::Blinded(*hint_idx),
-                       CandidateRouteHop::OneHopBlinded { hint_idx, .. } => CandidateHopId::Blinded(*hint_idx),
+                       CandidateRouteHop::Blinded(hop) => CandidateHopId::Blinded(hop.hint_idx),
+                       CandidateRouteHop::OneHopBlinded(hop) => CandidateHopId::Blinded(hop.hint_idx),
                        _ => CandidateHopId::Clear((self.short_channel_id().unwrap(), self.source() < self.target().unwrap())),
                }
        }
        fn blinded_path(&self) -> Option<&'a BlindedPath> {
                match self {
-                       CandidateRouteHop::Blinded { hint, .. } | CandidateRouteHop::OneHopBlinded { hint, .. } => {
+                       CandidateRouteHop::Blinded(BlindedPathCandidate { hint, .. }) | CandidateRouteHop::OneHopBlinded(OneHopBlindedPathCandidate { hint, .. }) => {
                                Some(&hint.1)
                        },
                        _ => None,
@@ -1348,11 +1368,11 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn source(&self) -> NodeId {
                match self {
-                       CandidateRouteHop::FirstHop { payer_node_id, .. } => **payer_node_id,
-                       CandidateRouteHop::PublicHop { info, .. } => *info.source(),
-                       CandidateRouteHop::PrivateHop { hint, .. } => hint.src_node_id.into(),
-                       CandidateRouteHop::Blinded { hint, .. } => hint.1.introduction_node_id.into(),
-                       CandidateRouteHop::OneHopBlinded { hint, .. } => hint.1.introduction_node_id.into(),
+                       CandidateRouteHop::FirstHop(hop) => *hop.payer_node_id,
+                       CandidateRouteHop::PublicHop(hop) => *hop.info.source(),
+                       CandidateRouteHop::PrivateHop(hop) => hop.hint.src_node_id.into(),
+                       CandidateRouteHop::Blinded(hop) => hop.hint.1.introduction_node_id.into(),
+                       CandidateRouteHop::OneHopBlinded(hop) => hop.hint.1.introduction_node_id.into(),
                }
        }
        /// Returns the target node id of this hop, if known.
@@ -1367,11 +1387,11 @@ impl<'a> CandidateRouteHop<'a> {
        #[inline]
        pub fn target(&self) -> Option<NodeId> {
                match self {
-                       CandidateRouteHop::FirstHop { details, .. } => Some(details.counterparty.node_id.into()),
-                       CandidateRouteHop::PublicHop { info, .. } => Some(*info.target()),
-                       CandidateRouteHop::PrivateHop { target_node_id, .. } => Some(**target_node_id),
-                       CandidateRouteHop::Blinded { .. } => None,
-                       CandidateRouteHop::OneHopBlinded { .. } => None,
+                       CandidateRouteHop::FirstHop(hop) => Some(hop.details.counterparty.node_id.into()),
+                       CandidateRouteHop::PublicHop(hop) => Some(*hop.info.target()),
+                       CandidateRouteHop::PrivateHop(hop) => Some(*hop.target_node_id),
+                       CandidateRouteHop::Blinded(_) => None,
+                       CandidateRouteHop::OneHopBlinded(_) => None,
                }
        }
 }
@@ -1666,17 +1686,17 @@ struct LoggedCandidateHop<'a>(&'a CandidateRouteHop<'a>);
 impl<'a> fmt::Display for LoggedCandidateHop<'a> {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
                match self.0 {
-                       CandidateRouteHop::Blinded { hint, .. } | CandidateRouteHop::OneHopBlinded { hint, .. } => {
+                       CandidateRouteHop::Blinded(BlindedPathCandidate { hint, .. }) | CandidateRouteHop::OneHopBlinded(OneHopBlindedPathCandidate { hint, .. }) => {
                                "blinded route hint with introduction node id ".fmt(f)?;
                                hint.1.introduction_node_id.fmt(f)?;
                                " and blinding point ".fmt(f)?;
                                hint.1.blinding_point.fmt(f)
                        },
-                       CandidateRouteHop::FirstHop { .. } => {
+                       CandidateRouteHop::FirstHop(_) => {
                                "first hop with SCID ".fmt(f)?;
                                self.0.short_channel_id().unwrap().fmt(f)
                        },
-                       CandidateRouteHop::PrivateHop { .. } => {
+                       CandidateRouteHop::PrivateHop(_) => {
                                "route hint with SCID ".fmt(f)?;
                                self.0.short_channel_id().unwrap().fmt(f)
                        },
@@ -2095,10 +2115,10 @@ where L::Target: Logger {
                                                |scid| payment_params.previously_failed_channels.contains(&scid));
 
                                        let (should_log_candidate, first_hop_details) = match $candidate {
-                                               CandidateRouteHop::FirstHop { details, .. } => (true, Some(details)),
-                                               CandidateRouteHop::PrivateHop { .. } => (true, None),
-                                               CandidateRouteHop::Blinded { .. } => (true, None),
-                                               CandidateRouteHop::OneHopBlinded { .. } => (true, None),
+                                               CandidateRouteHop::FirstHop(hop) => (true, Some(hop.details)),
+                                               CandidateRouteHop::PrivateHop(_) => (true, None),
+                                               CandidateRouteHop::Blinded(_) => (true, None),
+                                               CandidateRouteHop::OneHopBlinded(_) => (true, None),
                                                _ => (false, None),
                                        };
 
@@ -2365,9 +2385,9 @@ where L::Target: Logger {
                        if !skip_node {
                                if let Some(first_channels) = first_hop_targets.get(&$node_id) {
                                        for details in first_channels {
-                                               let candidate = CandidateRouteHop::FirstHop {
+                                               let candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                                        details, payer_node_id: &our_node_id,
-                                               };
+                                               });
                                                add_entry!(&candidate, fee_to_target_msat,
                                                        $next_hops_value_contribution,
                                                        next_hops_path_htlc_minimum_msat, next_hops_path_penalty_msat,
@@ -2388,10 +2408,10 @@ where L::Target: Logger {
                                                        if let Some((directed_channel, source)) = chan.as_directed_to(&$node_id) {
                                                                if first_hops.is_none() || *source != our_node_id {
                                                                        if directed_channel.direction().enabled {
-                                                                               let candidate = CandidateRouteHop::PublicHop {
+                                                                               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                                                                        info: directed_channel,
                                                                                        short_channel_id: *chan_id,
-                                                                               };
+                                                                               });
                                                                                add_entry!(&candidate,
                                                                                        fee_to_target_msat,
                                                                                        $next_hops_value_contribution,
@@ -2422,9 +2442,9 @@ where L::Target: Logger {
                // place where it could be added.
                payee_node_id_opt.map(|payee| first_hop_targets.get(&payee).map(|first_channels| {
                        for details in first_channels {
-                               let candidate = CandidateRouteHop::FirstHop {
+                               let candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                        details, payer_node_id: &our_node_id,
-                               };
+                               });
                                let added = add_entry!(&candidate, 0, path_value_msat,
                                                                        0, 0u64, 0, 0).is_some();
                                log_trace!(logger, "{} direct route to payee via {}",
@@ -2459,8 +2479,8 @@ where L::Target: Logger {
                                network_nodes.get(&intro_node_id).is_some();
                        if !have_intro_node_in_graph || our_node_id == intro_node_id { continue }
                        let candidate = if hint.1.blinded_hops.len() == 1 {
-                               CandidateRouteHop::OneHopBlinded { hint, hint_idx }
-                       } else { CandidateRouteHop::Blinded { hint, hint_idx } };
+                               CandidateRouteHop::OneHopBlinded(OneHopBlindedPathCandidate { hint, hint_idx })
+                       } else { CandidateRouteHop::Blinded(BlindedPathCandidate { hint, hint_idx }) };
                        let mut path_contribution_msat = path_value_msat;
                        if let Some(hop_used_msat) = add_entry!(&candidate,
                                0, path_contribution_msat, 0, 0_u64, 0, 0)
@@ -2471,9 +2491,9 @@ where L::Target: Logger {
                                sort_first_hop_channels(first_channels, &used_liquidities, recommended_value_msat,
                                        our_node_pubkey);
                                for details in first_channels {
-                                       let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                       let first_hop_candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                                details, payer_node_id: &our_node_id,
-                                       };
+                                       });
                                        let blinded_path_fee = match compute_fees(path_contribution_msat, candidate.fees()) {
                                                Some(fee) => fee,
                                                None => continue
@@ -2525,11 +2545,11 @@ where L::Target: Logger {
                                        let candidate = network_channels
                                                .get(&hop.short_channel_id)
                                                .and_then(|channel| channel.as_directed_to(&target))
-                                               .map(|(info, _)| CandidateRouteHop::PublicHop {
+                                               .map(|(info, _)| CandidateRouteHop::PublicHop(PublicHopCandidate {
                                                        info,
                                                        short_channel_id: hop.short_channel_id,
-                                               })
-                                               .unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop, target_node_id: target });
+                                               }))
+                                               .unwrap_or_else(|| CandidateRouteHop::PrivateHop(PrivateHopCandidate { hint: hop, target_node_id: target }));
 
                                        if let Some(hop_used_msat) = add_entry!(&candidate,
                                                aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
@@ -2569,9 +2589,9 @@ where L::Target: Logger {
                                                sort_first_hop_channels(first_channels, &used_liquidities,
                                                        recommended_value_msat, our_node_pubkey);
                                                for details in first_channels {
-                                                       let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                                       let first_hop_candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                                                details, payer_node_id: &our_node_id,
-                                                       };
+                                                       });
                                                        add_entry!(&first_hop_candidate,
                                                                aggregate_next_hops_fee_msat, aggregate_path_contribution_msat,
                                                                aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat,
@@ -2616,9 +2636,9 @@ where L::Target: Logger {
                                                        sort_first_hop_channels(first_channels, &used_liquidities,
                                                                recommended_value_msat, our_node_pubkey);
                                                        for details in first_channels {
-                                                               let first_hop_candidate = CandidateRouteHop::FirstHop {
+                                                               let first_hop_candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                                                        details, payer_node_id: &our_node_id,
-                                                               };
+                                                               });
                                                                add_entry!(&first_hop_candidate,
                                                                        aggregate_next_hops_fee_msat,
                                                                        aggregate_path_contribution_msat,
@@ -2661,7 +2681,7 @@ where L::Target: Logger {
                                        let target = ordered_hops.last().unwrap().0.candidate.target().unwrap_or(maybe_dummy_payee_node_id);
                                        if let Some(first_channels) = first_hop_targets.get(&target) {
                                                for details in first_channels {
-                                                       if let CandidateRouteHop::FirstHop { details: last_hop_details, .. }
+                                                       if let CandidateRouteHop::FirstHop(FirstHopCandidate { details: last_hop_details, .. })
                                                                = ordered_hops.last().unwrap().0.candidate
                                                        {
                                                                if details.get_outbound_payment_scid() == last_hop_details.get_outbound_payment_scid() {
@@ -2928,12 +2948,12 @@ where L::Target: Logger {
                        .filter(|(h, _)| h.candidate.short_channel_id().is_some())
                {
                        let target = hop.candidate.target().expect("target is defined when short_channel_id is defined");
-                       let maybe_announced_channel = if let CandidateRouteHop::PublicHop { .. } = hop.candidate {
+                       let maybe_announced_channel = if let CandidateRouteHop::PublicHop(_) = hop.candidate {
                                // If we sourced the hop from the graph we're sure the target node is announced.
                                true
-                       } else if let CandidateRouteHop::FirstHop { details, .. } = hop.candidate {
+                       } else if let CandidateRouteHop::FirstHop(first_hop) = &hop.candidate {
                                // If this is a first hop we also know if it's announced.
-                               details.is_public
+                               first_hop.details.is_public
                        } else {
                                // If we sourced it any other way, we double-check the network graph to see if
                                // there are announced channels between the endpoints. If so, the hop might be
@@ -3163,7 +3183,7 @@ mod tests {
        use crate::routing::utxo::UtxoResult;
        use crate::routing::router::{get_route, build_route_from_hops_internal, add_random_cltv_offset, default_node_features,
                BlindedTail, InFlightHtlcs, Path, PaymentParameters, Route, RouteHint, RouteHintHop, RouteHop, RoutingFees,
-               DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE, RouteParameters, CandidateRouteHop};
+               DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE, RouteParameters, CandidateRouteHop, PublicHopCandidate};
        use crate::routing::scoring::{ChannelUsage, FixedPenaltyScorer, ScoreLookUp, ProbabilisticScorer, ProbabilisticScoringFeeParameters, ProbabilisticScoringDecayParameters};
        use crate::routing::test_utils::{add_channel, add_or_update_node, build_graph, build_line_graph, id_to_feature_flags, get_nodes, update_channel};
        use crate::chain::transaction::OutPoint;
@@ -6977,10 +6997,10 @@ mod tests {
                let channels = network_graph.channels();
                let channel = channels.get(&5).unwrap();
                let info = channel.as_directed_from(&NodeId::from_pubkey(&nodes[3])).unwrap();
-               let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop {
+               let candidate: CandidateRouteHop = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info: info.0,
                        short_channel_id: 5,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &scorer_params), 456);
 
                // Then check we can get a normal route
index 6ace8681ac025a08423bff54c4dae67b829fab5f..646405c6287ac150d88199890137020e0350079f 100644 (file)
 
 use crate::ln::msgs::DecodeError;
 use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId};
-use crate::routing::router::{Path, CandidateRouteHop};
+use crate::routing::router::{Path, CandidateRouteHop, PublicHopCandidate};
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
 use crate::util::logger::Logger;
 
 use crate::prelude::*;
 use core::{cmp, fmt};
-use core::cell::{RefCell, RefMut, Ref};
 use core::convert::TryInto;
 use core::ops::{Deref, DerefMut};
 use core::time::Duration;
 use crate::io::{self, Read};
-use crate::sync::{Mutex, MutexGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};
+use crate::sync::{RwLock, RwLockReadGuard, RwLockWriteGuard};
+#[cfg(not(c_bindings))]
+use {
+       core::cell::{RefCell, RefMut, Ref},
+       crate::sync::{Mutex, MutexGuard},
+};
 
 /// We define Score ever-so-slightly differently based on whether we are being built for C bindings
 /// or not. For users, `LockableScore` must somehow be writeable to disk. For Rust users, this is
@@ -1320,7 +1324,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref> ScoreLookUp for Probabilistic
                &self, candidate: &CandidateRouteHop, usage: ChannelUsage, score_params: &ProbabilisticScoringFeeParameters
        ) -> u64 {
                let (scid, target) = match candidate {
-                       CandidateRouteHop::PublicHop { info, short_channel_id } => {
+                       CandidateRouteHop::PublicHop(PublicHopCandidate { info, short_channel_id }) => {
                                (short_channel_id, info.target())
                        },
                        _ => return 0,
@@ -2155,7 +2159,7 @@ mod tests {
        use crate::ln::channelmanager;
        use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, UnsignedChannelAnnouncement, UnsignedChannelUpdate};
        use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId};
-       use crate::routing::router::{BlindedTail, Path, RouteHop, CandidateRouteHop};
+       use crate::routing::router::{BlindedTail, Path, RouteHop, CandidateRouteHop, PublicHopCandidate};
        use crate::routing::scoring::{ChannelUsage, ScoreLookUp, ScoreUpdate};
        use crate::util::ser::{ReadableArgs, Writeable};
        use crate::util::test_utils::{self, TestLogger};
@@ -2531,10 +2535,10 @@ mod tests {
                let network_graph = network_graph.read_only();
                let channel = network_graph.channel(42).unwrap();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
                let usage = ChannelUsage { amount_msat: 10_240, ..usage };
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
@@ -2594,10 +2598,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
                let usage = ChannelUsage { amount_msat: 50, ..usage };
                assert_ne!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
@@ -2625,10 +2629,10 @@ mod tests {
                let successful_path = payment_path_for_amount(200);
                let channel = &network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 41,
-               };
+               });
 
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 301);
 
@@ -2658,10 +2662,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
                let usage = ChannelUsage { amount_msat: 500, ..usage };
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 301);
@@ -2698,10 +2702,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
                let usage = ChannelUsage { amount_msat: 500, ..usage };
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 301);
@@ -2764,50 +2768,50 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_a).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
                // Note that a default liquidity bound is used for B -> C as no channel exists
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_b).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 43,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
                let channel = network_graph.read_only().channel(44).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_c).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 44,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
 
                scorer.payment_path_failed(&Path { hops: path, blinded_tail: None }, 43, Duration::ZERO);
 
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_a).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 80);
                // Note that a default liquidity bound is used for B -> C as no channel exists
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_b).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 43,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
                let channel = network_graph.read_only().channel(44).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&node_c).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 44,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 128);
        }
 
@@ -2830,20 +2834,20 @@ mod tests {
                let channel_42 = network_graph.get(&42).unwrap();
                let channel_43 = network_graph.get(&43).unwrap();
                let (info, _) = channel_42.as_directed_from(&source).unwrap();
-               let candidate_41 = CandidateRouteHop::PublicHop {
+               let candidate_41 = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 41,
-               };
+               });
                let (info, target) = channel_42.as_directed_from(&source).unwrap();
-               let candidate_42 = CandidateRouteHop::PublicHop {
+               let candidate_42 = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                let (info, _) = channel_43.as_directed_from(&target).unwrap();
-               let candidate_43 = CandidateRouteHop::PublicHop {
+               let candidate_43 = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 43,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate_41, usage, &params), 128);
                assert_eq!(scorer.channel_penalty_msat(&candidate_42, usage, &params), 128);
                assert_eq!(scorer.channel_penalty_msat(&candidate_43, usage, &params), 128);
@@ -2878,10 +2882,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
                let usage = ChannelUsage { amount_msat: 1_023, ..usage };
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 2_000);
@@ -2967,10 +2971,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
 
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 300);
 
@@ -3022,10 +3026,10 @@ mod tests {
                scorer.payment_path_failed(&payment_path_for_amount(500), 42, Duration::ZERO);
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), u64::max_value());
 
                scorer.time_passed(Duration::from_secs(10));
@@ -3066,10 +3070,10 @@ mod tests {
                scorer.payment_path_failed(&payment_path_for_amount(500), 42, Duration::ZERO);
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), u64::max_value());
 
                if decay_before_reload {
@@ -3118,10 +3122,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 11497);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
@@ -3183,10 +3187,10 @@ mod tests {
                let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger);
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 58);
 
                let params = ProbabilisticScoringFeeParameters {
@@ -3225,10 +3229,10 @@ mod tests {
                let scorer = ProbabilisticScorer::new(ProbabilisticScoringDecayParameters::default(), &network_graph, &logger);
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 300);
 
                let params = ProbabilisticScoringFeeParameters {
@@ -3257,10 +3261,10 @@ mod tests {
                let decay_params = ProbabilisticScoringDecayParameters::zero_penalty();
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                let scorer = ProbabilisticScorer::new(decay_params, &network_graph, &logger);
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 80_000);
        }
@@ -3284,10 +3288,10 @@ mod tests {
                let network_graph = network_graph.read_only();
                let channel = network_graph.channel(42).unwrap();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_ne!(scorer.channel_penalty_msat(&candidate, usage, &params), u64::max_value());
 
                let usage = ChannelUsage { inflight_htlc_msat: 251, ..usage };
@@ -3311,10 +3315,10 @@ mod tests {
                let network_graph = network_graph.read_only();
                let channel = network_graph.channel(42).unwrap();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), base_penalty_msat);
 
                let usage = ChannelUsage { amount_msat: 1_000, ..usage };
@@ -3356,10 +3360,10 @@ mod tests {
                        let network_graph = network_graph.read_only();
                        let channel = network_graph.channel(42).unwrap();
                        let (info, _) = channel.as_directed_from(&source).unwrap();
-                       let candidate = CandidateRouteHop::PublicHop {
+                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                info,
                                short_channel_id: 42,
-                       };
+                       });
 
                        // With no historical data the normal liquidity penalty calculation is used.
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 168);
@@ -3374,10 +3378,10 @@ mod tests {
                        let network_graph = network_graph.read_only();
                        let channel = network_graph.channel(42).unwrap();
                        let (info, _) = channel.as_directed_from(&source).unwrap();
-                       let candidate = CandidateRouteHop::PublicHop {
+                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                info,
                                short_channel_id: 42,
-                       };
+                       });
 
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 2048);
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage_1, &params), 249);
@@ -3399,10 +3403,10 @@ mod tests {
                        let network_graph = network_graph.read_only();
                        let channel = network_graph.channel(42).unwrap();
                        let (info, _) = channel.as_directed_from(&source).unwrap();
-                       let candidate = CandidateRouteHop::PublicHop {
+                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                info,
                                short_channel_id: 42,
-                       };
+                       });
 
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 105);
                }
@@ -3429,10 +3433,10 @@ mod tests {
                        let network_graph = network_graph.read_only();
                        let channel = network_graph.channel(42).unwrap();
                        let (info, _) = channel.as_directed_from(&source).unwrap();
-                       let candidate = CandidateRouteHop::PublicHop {
+                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                info,
                                short_channel_id: 42,
-                       };
+                       });
 
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 168);
                }
@@ -3452,10 +3456,10 @@ mod tests {
                        let network_graph = network_graph.read_only();
                        let channel = network_graph.channel(42).unwrap();
                        let (info, _) = channel.as_directed_from(&source).unwrap();
-                       let candidate = CandidateRouteHop::PublicHop {
+                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                info,
                                short_channel_id: 42,
-                       };
+                       });
 
                        assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 2050);
 
@@ -3505,10 +3509,10 @@ mod tests {
                let network_graph = network_graph.read_only();
                let channel = network_graph.channel(42).unwrap();
                let (info, _) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 0);
 
                // Check we receive anti-probing penalty for htlc_maximum_msat == channel_capacity.
@@ -3555,10 +3559,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, target) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 300);
 
                let mut path = payment_path_for_amount(768);
@@ -3624,10 +3628,10 @@ mod tests {
                };
                let channel = network_graph.read_only().channel(42).unwrap().to_owned();
                let (info, target) = channel.as_directed_from(&source).unwrap();
-               let candidate = CandidateRouteHop::PublicHop {
+               let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                        info,
                        short_channel_id: 42,
-               };
+               });
                // With no historical data the normal liquidity penalty calculation is used, which results
                // in a success probability of ~75%.
                assert_eq!(scorer.channel_penalty_msat(&candidate, usage, &params), 1269);
index 27cfb9b8f782c4bafabecd5180a5d0d2256105c3..0f92bd6caa3ad7c914b304db24d5fa69a8456b85 100644 (file)
@@ -37,10 +37,6 @@ impl<T> Mutex<T> {
                Ok(MutexGuard { lock: self.inner.borrow_mut() })
        }
 
-       pub fn try_lock<'a>(&'a self) -> LockResult<MutexGuard<'a, T>> {
-               Ok(MutexGuard { lock: self.inner.borrow_mut() })
-       }
-
        pub fn into_inner(self) -> LockResult<T> {
                Ok(self.inner.into_inner())
        }
index bb5903b2c8d166783c8b6a275441282e9e7d1b27..98963c7c2bd4fd823c614e25b50dd1785d8dbab2 100644 (file)
@@ -64,6 +64,7 @@ pub fn sign<C: Signing>(ctx: &Secp256k1<C>, msg: &Message, sk: &SecretKey) -> Si
 }
 
 #[inline]
+#[allow(unused_variables)]
 pub fn sign_with_aux_rand<C: Signing, ES: Deref>(
        ctx: &Secp256k1<C>, msg: &Message, sk: &SecretKey, entropy_source: &ES
 ) -> Signature where ES::Target: EntropySource {
index ba56edf3584f017ab0aa623e19aad6d2ec07f2e2..1c45c4a4c5d3f3d36120c6ce5fffb515c5032cb3 100644 (file)
@@ -19,7 +19,7 @@ use crate::chain::chainmonitor::{MonitorUpdateId, UpdateOrigin};
 use crate::chain::channelmonitor;
 use crate::chain::channelmonitor::MonitorEvent;
 use crate::chain::transaction::OutPoint;
-use crate::routing::router::CandidateRouteHop;
+use crate::routing::router::{CandidateRouteHop, FirstHopCandidate, PublicHopCandidate, PrivateHopCandidate};
 use crate::sign;
 use crate::events;
 use crate::events::bump_transaction::{WalletSource, Utxo};
@@ -32,7 +32,7 @@ use crate::ln::msgs::LightningError;
 use crate::ln::script::ShutdownScript;
 use crate::offers::invoice::{BlindedPayInfo, UnsignedBolt12Invoice};
 use crate::offers::invoice_request::UnsignedInvoiceRequest;
-use crate::onion_message::{Destination, MessageRouter, OnionMessagePath};
+use crate::onion_message::messenger::{Destination, MessageRouter, OnionMessagePath};
 use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId, RoutingFees};
 use crate::routing::utxo::{UtxoLookup, UtxoLookupError, UtxoResult};
 use crate::routing::router::{find_route, InFlightHtlcs, Path, Route, RouteParameters, RouteHintHop, Router, ScorerAccountingForInFlightHtlcs};
@@ -146,10 +146,10 @@ impl<'a> Router for TestRouter<'a> {
                                                        if let Some(first_hops) = first_hops {
                                                                if let Some(idx) = first_hops.iter().position(|h| h.get_outbound_payment_scid() == Some(hop.short_channel_id)) {
                                                                        let node_id = NodeId::from_pubkey(payer);
-                                                                       let candidate = CandidateRouteHop::FirstHop {
+                                                                       let candidate = CandidateRouteHop::FirstHop(FirstHopCandidate {
                                                                                details: first_hops[idx],
                                                                                payer_node_id: &node_id,
-                                                                       };
+                                                                       });
                                                                        scorer.channel_penalty_msat(&candidate, usage, &());
                                                                        continue;
                                                                }
@@ -158,10 +158,10 @@ impl<'a> Router for TestRouter<'a> {
                                                let network_graph = self.network_graph.read_only();
                                                if let Some(channel) = network_graph.channel(hop.short_channel_id) {
                                                        let (directed, _) = channel.as_directed_to(&NodeId::from_pubkey(&hop.pubkey)).unwrap();
-                                                       let candidate = CandidateRouteHop::PublicHop {
+                                                       let candidate = CandidateRouteHop::PublicHop(PublicHopCandidate {
                                                                info: directed,
                                                                short_channel_id: hop.short_channel_id,
-                                                       };
+                                                       });
                                                        scorer.channel_penalty_msat(&candidate, usage, &());
                                                } else {
                                                        let target_node_id = NodeId::from_pubkey(&hop.pubkey);
@@ -173,10 +173,10 @@ impl<'a> Router for TestRouter<'a> {
                                                                htlc_minimum_msat: None,
                                                                htlc_maximum_msat: None,
                                                        };
-                                                       let candidate = CandidateRouteHop::PrivateHop {
+                                                       let candidate = CandidateRouteHop::PrivateHop(PrivateHopCandidate {
                                                                hint: &route_hint,
                                                                target_node_id: &target_node_id,
-                                                       };
+                                                       });
                                                        scorer.channel_penalty_msat(&candidate, usage, &());
                                                }
                                                prev_hop_node = &hop.pubkey;