Merge pull request #2589 from ErikDeSmedt/reexport_route_hint_hop
authorElias Rohrer <dev@tnull.de>
Fri, 22 Sep 2023 07:09:41 +0000 (09:09 +0200)
committerGitHub <noreply@github.com>
Fri, 22 Sep 2023 07:09:41 +0000 (09:09 +0200)
Reexport RouteHintHop

31 files changed:
bench/benches/bench.rs
fuzz/src/chanmon_consistency.rs
lightning-background-processor/Cargo.toml
lightning-block-sync/Cargo.toml
lightning-custom-message/Cargo.toml
lightning-invoice/Cargo.toml
lightning-net-tokio/Cargo.toml
lightning-persister/Cargo.toml
lightning-persister/src/fs_store.rs
lightning-rapid-gossip-sync/Cargo.toml
lightning-transaction-sync/Cargo.toml
lightning/Cargo.toml
lightning/src/chain/chainmonitor.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/mod.rs
lightning/src/chain/package.rs
lightning/src/events/bump_transaction.rs
lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/monitor_tests.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/reload_tests.rs
lightning/src/ln/reorg_tests.rs
lightning/src/ln/shutdown_tests.rs
lightning/src/routing/router.rs
lightning/src/routing/scoring.rs
lightning/src/util/persist.rs
lightning/src/util/test_utils.rs

index bc4bd010822ddc24d84c3d761f7da63d8e7fad99..eaa3fcec50c85188b2350ef39a08f6dd01ae86ae 100644 (file)
@@ -13,6 +13,9 @@ criterion_group!(benches,
        lightning::routing::router::benches::generate_routes_with_probabilistic_scorer,
        lightning::routing::router::benches::generate_mpp_routes_with_probabilistic_scorer,
        lightning::routing::router::benches::generate_large_mpp_routes_with_probabilistic_scorer,
+       lightning::routing::router::benches::generate_routes_with_nonlinear_probabilistic_scorer,
+       lightning::routing::router::benches::generate_mpp_routes_with_nonlinear_probabilistic_scorer,
+       lightning::routing::router::benches::generate_large_mpp_routes_with_nonlinear_probabilistic_scorer,
        lightning::sign::benches::bench_get_secure_random_bytes,
        lightning::ln::channelmanager::bench::bench_sends,
        lightning_persister::fs_store::bench::bench_sends,
index b6d41fb99140cd2cb012ffbaf08c190efdd443fd..6a2007165d71773cbb3d39ea5ea0dd2187aca6bb 100644 (file)
@@ -138,7 +138,7 @@ impl TestChainMonitor {
        }
 }
 impl chain::Watch<TestChannelSigner> for TestChainMonitor {
-       fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> chain::ChannelMonitorUpdateStatus {
+       fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> Result<chain::ChannelMonitorUpdateStatus, ()> {
                let mut ser = VecWriter(Vec::new());
                monitor.write(&mut ser).unwrap();
                if let Some(_) = self.latest_monitors.lock().unwrap().insert(funding_txo, (monitor.get_latest_update_id(), ser.0)) {
@@ -500,7 +500,7 @@ pub fn do_test<Out: Output>(data: &[u8], underlying_out: Out) {
                        let res = (<(BlockHash, ChanMan)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, chain_monitor.clone());
                        for (funding_txo, mon) in monitors.drain() {
                                assert_eq!(chain_monitor.chain_monitor.watch_channel(funding_txo, mon),
-                                       ChannelMonitorUpdateStatus::Completed);
+                                       Ok(ChannelMonitorUpdateStatus::Completed));
                        }
                        res
                } }
index bb7399ddde46e05b787b029af941a7934182eebc..26de46cb8a884e0005a9accae2d4619ff4ee6b7c 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-background-processor"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Valentine Wallace <vwallace@protonmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -22,11 +22,11 @@ default = ["std"]
 
 [dependencies]
 bitcoin = { version = "0.29.0", default-features = false }
-lightning = { version = "0.0.116", path = "../lightning", default-features = false }
-lightning-rapid-gossip-sync = { version = "0.0.116", path = "../lightning-rapid-gossip-sync", default-features = false }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", default-features = false }
+lightning-rapid-gossip-sync = { version = "0.0.117-alpha1", path = "../lightning-rapid-gossip-sync", default-features = false }
 
 [dev-dependencies]
 tokio = { version = "1.14", features = [ "macros", "rt", "rt-multi-thread", "sync", "time" ] }
-lightning = { version = "0.0.116", path = "../lightning", features = ["_test_utils"] }
-lightning-invoice = { version = "0.24.0", path = "../lightning-invoice" }
-lightning-persister = { version = "0.0.116", path = "../lightning-persister" }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["_test_utils"] }
+lightning-invoice = { version = "0.25.0-alpha1", path = "../lightning-invoice" }
+lightning-persister = { version = "0.0.117-alpha1", path = "../lightning-persister" }
index 619bbb425c096b006f4b41724f5f133b9c75d83e..79cf2c9b7dbfb02b60719b119685f3eb1e81f875 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-block-sync"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Jeffrey Czyz", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -19,11 +19,11 @@ rpc-client = [ "serde_json", "chunked_transfer" ]
 
 [dependencies]
 bitcoin = "0.29.0"
-lightning = { version = "0.0.116", path = "../lightning" }
+lightning = { version = "0.0.117-alpha1", path = "../lightning" }
 tokio = { version = "1.0", features = [ "io-util", "net", "time" ], optional = true }
 serde_json = { version = "1.0", optional = true }
 chunked_transfer = { version = "1.4", optional = true }
 
 [dev-dependencies]
-lightning = { version = "0.0.116", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["_test_utils"] }
 tokio = { version = "1.14", features = [ "macros", "rt" ] }
index ad0f1835f97d1bd14e177afe4b03251ebeef3d60..8fc7034f9669192a3e11d5ebe1f6f30cf85b31a9 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-custom-message"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Jeffrey Czyz"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -15,4 +15,4 @@ rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
 bitcoin = "0.29.0"
-lightning = { version = "0.0.116", path = "../lightning" }
+lightning = { version = "0.0.117-alpha1", path = "../lightning" }
index 72e0d8e04ea15838e748e2ac8f1c916fb04da018..6774920cb92fcfb282b884939e68d0163a8b734f 100644 (file)
@@ -1,7 +1,7 @@
 [package]
 name = "lightning-invoice"
 description = "Data structures to parse and serialize BOLT11 lightning invoices"
-version = "0.24.0"
+version = "0.25.0-alpha1"
 authors = ["Sebastian Geisler <sgeisler@wh2.tu-dresden.de>"]
 documentation = "https://docs.rs/lightning-invoice/"
 license = "MIT OR Apache-2.0"
@@ -21,7 +21,7 @@ std = ["bitcoin_hashes/std", "num-traits/std", "lightning/std", "bech32/std"]
 
 [dependencies]
 bech32 = { version = "0.9.0", default-features = false }
-lightning = { version = "0.0.116", path = "../lightning", default-features = false }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", default-features = false }
 secp256k1 = { version = "0.24.0", default-features = false, features = ["recovery", "alloc"] }
 num-traits = { version = "0.2.8", default-features = false }
 bitcoin_hashes = { version = "0.11", default-features = false }
@@ -30,6 +30,6 @@ serde = { version = "1.0.118", optional = true }
 bitcoin = { version = "0.29.0", default-features = false }
 
 [dev-dependencies]
-lightning = { version = "0.0.116", path = "../lightning", default-features = false, features = ["_test_utils"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", default-features = false, features = ["_test_utils"] }
 hex = "0.4"
 serde_json = { version = "1"}
index d2ac6e5474543db1392926dc8dfebf07715b0d86..f429931360030ea6cc2fbd97c53fca8dd1941b19 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-net-tokio"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"
@@ -16,9 +16,9 @@ rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
 bitcoin = "0.29.0"
-lightning = { version = "0.0.116", path = "../lightning" }
+lightning = { version = "0.0.117-alpha1", path = "../lightning" }
 tokio = { version = "1.0", features = [ "rt", "sync", "net", "time" ] }
 
 [dev-dependencies]
 tokio = { version = "1.14", features = [ "macros", "rt", "rt-multi-thread", "sync", "net", "time" ] }
-lightning = { version = "0.0.116", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["_test_utils"] }
index 361ab0c57a1d13fd9ee39543534c0bcbbbac4ccf..206815301fbaf4517dfe0d9fcbaebd9abc38d09f 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-persister"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Valentine Wallace", "Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -15,7 +15,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
 bitcoin = "0.29.0"
-lightning = { version = "0.0.116", path = "../lightning" }
+lightning = { version = "0.0.117-alpha1", path = "../lightning" }
 
 [target.'cfg(windows)'.dependencies]
 windows-sys = { version = "0.48.0", default-features = false, features = ["Win32_Storage_FileSystem", "Win32_Foundation"] }
@@ -24,5 +24,5 @@ windows-sys = { version = "0.48.0", default-features = false, features = ["Win32
 criterion = { version = "0.4", optional = true, default-features = false }
 
 [dev-dependencies]
-lightning = { version = "0.0.116", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["_test_utils"] }
 bitcoin = { version = "0.29.0", default-features = false }
index 81f9709c45467d4e6725ef83f11ba10a33734b68..42b28018fe9202ce4435a41dc87907433b156e7d 100644 (file)
@@ -436,7 +436,7 @@ mod tests {
        }
 
        // Test that if the store's path to channel data is read-only, writing a
-       // monitor to it results in the store returning a PermanentFailure.
+       // monitor to it results in the store returning an InProgress.
        // Windows ignores the read-only flag for folders, so this test is Unix-only.
        #[cfg(not(target_os = "windows"))]
        #[test]
@@ -470,7 +470,7 @@ mod tests {
                        index: 0
                };
                match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
-                       ChannelMonitorUpdateStatus::PermanentFailure => {},
+                       ChannelMonitorUpdateStatus::UnrecoverableError => {},
                        _ => panic!("unexpected result from persisting new channel")
                }
 
@@ -507,7 +507,7 @@ mod tests {
                        index: 0
                };
                match store.persist_new_channel(test_txo, &added_monitors[0].1, update_id.2) {
-                       ChannelMonitorUpdateStatus::PermanentFailure => {},
+                       ChannelMonitorUpdateStatus::UnrecoverableError => {},
                        _ => panic!("unexpected result from persisting new channel")
                }
 
index a98bf7ee6cb5f76fe2f2e98064f4ff74a104bd00..fc4bd36bf4c649e6056c3e329451dee974983532 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-rapid-gossip-sync"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Arik Sosman <git@arik.io>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -15,11 +15,11 @@ no-std = ["lightning/no-std"]
 std = ["lightning/std"]
 
 [dependencies]
-lightning = { version = "0.0.116", path = "../lightning", default-features = false }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", default-features = false }
 bitcoin = { version = "0.29.0", default-features = false }
 
 [target.'cfg(ldk_bench)'.dependencies]
 criterion = { version = "0.4", optional = true, default-features = false }
 
 [dev-dependencies]
-lightning = { version = "0.0.116", path = "../lightning", features = ["_test_utils"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["_test_utils"] }
index 4dce05202a63fea3584684c1cf904717a5936f3b..aad057f86c15ed1c35100a1be20f03d156e5391c 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning-transaction-sync"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Elias Rohrer"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning"
@@ -21,7 +21,7 @@ esplora-blocking = ["esplora-client/blocking"]
 async-interface = []
 
 [dependencies]
-lightning = { version = "0.0.116", path = "../lightning", default-features = false }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", default-features = false }
 bitcoin = { version = "0.29.0", default-features = false }
 bdk-macros = "0.6"
 futures = { version = "0.3", optional = true }
@@ -29,7 +29,7 @@ esplora-client = { version = "0.4", default-features = false, optional = true }
 reqwest = { version = "0.11", optional = true, default-features = false, features = ["json"] }
 
 [dev-dependencies]
-lightning = { version = "0.0.116", path = "../lightning", features = ["std"] }
+lightning = { version = "0.0.117-alpha1", path = "../lightning", features = ["std"] }
 electrsd = { version = "0.22.0", features = ["legacy", "esplora_a33e97e1", "bitcoind_23_0"] }
 electrum-client = "0.12.0"
 tokio = { version = "1.14.0", features = ["full"] }
index ec3ccf763c6085426befd2bc1869df9dfb866127..773467b580e5c531e4cbfc3e0f6de6ae4f101f65 100644 (file)
@@ -1,6 +1,6 @@
 [package]
 name = "lightning"
-version = "0.0.116"
+version = "0.0.117-alpha1"
 authors = ["Matt Corallo"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/lightningdevkit/rust-lightning/"
index b3b62a2e8706f96a1ddf82146b7260e63bc91796..b6909cb3e416890895367532455edb127fdc038c 100644 (file)
@@ -44,7 +44,7 @@ use crate::prelude::*;
 use crate::sync::{RwLock, RwLockReadGuard, Mutex, MutexGuard};
 use core::iter::FromIterator;
 use core::ops::Deref;
-use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
+use core::sync::atomic::{AtomicUsize, Ordering};
 use bitcoin::secp256k1::PublicKey;
 
 #[derive(Clone, Copy, Hash, PartialEq, Eq)]
@@ -78,27 +78,48 @@ impl MonitorUpdateId {
 /// `Persist` defines behavior for persisting channel monitors: this could mean
 /// writing once to disk, and/or uploading to one or more backup services.
 ///
-/// Each method can return three possible values:
-///  * If persistence (including any relevant `fsync()` calls) happens immediately, the
-///    implementation should return [`ChannelMonitorUpdateStatus::Completed`], indicating normal
-///    channel operation should continue.
-///  * If persistence happens asynchronously, implementations should first ensure the
-///    [`ChannelMonitor`] or [`ChannelMonitorUpdate`] are written durably to disk, and then return
-///    [`ChannelMonitorUpdateStatus::InProgress`] while the update continues in the background.
-///    Once the update completes, [`ChainMonitor::channel_monitor_updated`] should be called with
-///    the corresponding [`MonitorUpdateId`].
+/// Persistence can happen in one of two ways - synchronously completing before the trait method
+/// calls return or asynchronously in the background.
 ///
-///    Note that unlike the direct [`chain::Watch`] interface,
-///    [`ChainMonitor::channel_monitor_updated`] must be called once for *each* update which occurs.
+/// # For those implementing synchronous persistence
 ///
-///  * If persistence fails for some reason, implementations should return
-///    [`ChannelMonitorUpdateStatus::PermanentFailure`], in which case the channel will likely be
-///    closed without broadcasting the latest state. See
-///    [`ChannelMonitorUpdateStatus::PermanentFailure`] for more details.
+///  * If persistence completes fully (including any relevant `fsync()` calls), the implementation
+///    should return [`ChannelMonitorUpdateStatus::Completed`], indicating normal channel operation
+///    should continue.
 ///
-/// Third-party watchtowers may be built as a part of an implementation of this trait, with the
-/// advantage that you can control whether to resume channel operation depending on if an update
-/// has been persisted to a watchtower. For this, you may find the following methods useful:
+///  * If persistence fails for some reason, implementations should consider returning
+///    [`ChannelMonitorUpdateStatus::InProgress`] and retry all pending persistence operations in
+///    the background with [`ChainMonitor::list_pending_monitor_updates`] and
+///    [`ChainMonitor::get_monitor`].
+///
+///    Once a full [`ChannelMonitor`] has been persisted, all pending updates for that channel can
+///    be marked as complete via [`ChainMonitor::channel_monitor_updated`].
+///
+///    If at some point no further progress can be made towards persisting the pending updates, the
+///    node should simply shut down.
+///
+///  * If the persistence has failed and cannot be retried further (e.g. because of some timeout),
+///    [`ChannelMonitorUpdateStatus::UnrecoverableError`] can be used, though this will result in
+///    an immediate panic and future operations in LDK generally failing.
+///
+/// # For those implementing asynchronous persistence
+///
+///  All calls should generally spawn a background task and immediately return
+///  [`ChannelMonitorUpdateStatus::InProgress`]. Once the update completes,
+///  [`ChainMonitor::channel_monitor_updated`] should be called with the corresponding
+///  [`MonitorUpdateId`].
+///
+///  Note that unlike the direct [`chain::Watch`] interface,
+///  [`ChainMonitor::channel_monitor_updated`] must be called once for *each* update which occurs.
+///
+///  If at some point no further progress can be made towards persisting a pending update, the node
+///  should simply shut down.
+///
+/// # Using remote watchtowers
+///
+/// Watchtowers may be updated as a part of an implementation of this trait, utilizing the async
+/// update process described above while the watchtower is being updated. The following methods are
+/// provided for bulding transactions for a watchtower:
 /// [`ChannelMonitor::initial_counterparty_commitment_tx`],
 /// [`ChannelMonitor::counterparty_commitment_txs_from_update`],
 /// [`ChannelMonitor::sign_to_local_justice_tx`], [`TrustedCommitmentTransaction::revokeable_output_index`],
@@ -180,12 +201,6 @@ struct MonitorHolder<ChannelSigner: WriteableEcdsaChannelSigner> {
        /// the ChannelManager re-adding the same payment entry, before the same block is replayed,
        /// resulting in a duplicate PaymentSent event.
        pending_monitor_updates: Mutex<Vec<MonitorUpdateId>>,
-       /// When the user returns a PermanentFailure error from an update_persisted_channel call during
-       /// block processing, we inform the ChannelManager that the channel should be closed
-       /// asynchronously. In order to ensure no further changes happen before the ChannelManager has
-       /// processed the closure event, we set this to true and return PermanentFailure for any other
-       /// chain::Watch events.
-       channel_perm_failed: AtomicBool,
        /// The last block height at which no [`UpdateOrigin::ChainSync`] monitor updates were present
        /// in `pending_monitor_updates`.
        /// If it's been more than [`LATENCY_GRACE_PERIOD_BLOCKS`] since we started waiting on a chain
@@ -286,11 +301,20 @@ where C::Target: chain::Filter,
        where
                FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
        {
+               let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
                let funding_outpoints: HashSet<OutPoint> = HashSet::from_iter(self.monitors.read().unwrap().keys().cloned());
                for funding_outpoint in funding_outpoints.iter() {
                        let monitor_lock = self.monitors.read().unwrap();
                        if let Some(monitor_state) = monitor_lock.get(funding_outpoint) {
-                               self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state);
+                               if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state).is_err() {
+                                       // Take the monitors lock for writing so that we poison it and any future
+                                       // operations going forward fail immediately.
+                                       core::mem::drop(monitor_state);
+                                       core::mem::drop(monitor_lock);
+                                       let _poison = self.monitors.write().unwrap();
+                                       log_error!(self.logger, "{}", err_str);
+                                       panic!("{}", err_str);
+                               }
                        }
                }
 
@@ -298,7 +322,10 @@ where C::Target: chain::Filter,
                let monitor_states = self.monitors.write().unwrap();
                for (funding_outpoint, monitor_state) in monitor_states.iter() {
                        if !funding_outpoints.contains(funding_outpoint) {
-                               self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state);
+                               if self.update_monitor_with_chain_data(header, best_height, txdata, &process, funding_outpoint, &monitor_state).is_err() {
+                                       log_error!(self.logger, "{}", err_str);
+                                       panic!("{}", err_str);
+                               }
                        }
                }
 
@@ -313,7 +340,10 @@ where C::Target: chain::Filter,
                }
        }
 
-       fn update_monitor_with_chain_data<FN>(&self, header: &BlockHeader, best_height: Option<u32>, txdata: &TransactionData, process: FN, funding_outpoint: &OutPoint, monitor_state: &MonitorHolder<ChannelSigner>) where FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs> {
+       fn update_monitor_with_chain_data<FN>(
+               &self, header: &BlockHeader, best_height: Option<u32>, txdata: &TransactionData,
+               process: FN, funding_outpoint: &OutPoint, monitor_state: &MonitorHolder<ChannelSigner>
+       ) -> Result<(), ()> where FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs> {
                let monitor = &monitor_state.monitor;
                let mut txn_outputs;
                {
@@ -335,15 +365,13 @@ where C::Target: chain::Filter,
                        match self.persister.update_persisted_channel(*funding_outpoint, None, monitor, update_id) {
                                ChannelMonitorUpdateStatus::Completed =>
                                        log_trace!(self.logger, "Finished syncing Channel Monitor for channel {}", log_funding_info!(monitor)),
-                               ChannelMonitorUpdateStatus::PermanentFailure => {
-                                       monitor_state.channel_perm_failed.store(true, Ordering::Release);
-                                       self.pending_monitor_events.lock().unwrap().push((*funding_outpoint, vec![MonitorEvent::UpdateFailed(*funding_outpoint)], monitor.get_counterparty_node_id()));
-                                       self.event_notifier.notify();
-                               }
                                ChannelMonitorUpdateStatus::InProgress => {
                                        log_debug!(self.logger, "Channel Monitor sync for channel {} in progress, holding events until completion!", log_funding_info!(monitor));
                                        pending_monitor_updates.push(update_id);
-                               }
+                               },
+                               ChannelMonitorUpdateStatus::UnrecoverableError => {
+                                       return Err(());
+                               },
                        }
                }
 
@@ -363,6 +391,7 @@ where C::Target: chain::Filter,
                                }
                        }
                }
+               Ok(())
        }
 
        /// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
@@ -491,9 +520,8 @@ where C::Target: chain::Filter,
                                // `MonitorEvent`s from the monitor back to the `ChannelManager` until they
                                // complete.
                                let monitor_is_pending_updates = monitor_data.has_pending_offchain_updates(&pending_monitor_updates);
-                               if monitor_is_pending_updates || monitor_data.channel_perm_failed.load(Ordering::Acquire) {
-                                       // If there are still monitor updates pending (or an old monitor update
-                                       // finished after a later one perm-failed), we cannot yet construct an
+                               if monitor_is_pending_updates {
+                                       // If there are still monitor updates pending, we cannot yet construct a
                                        // Completed event.
                                        return Ok(());
                                }
@@ -667,18 +695,12 @@ where C::Target: chain::Filter,
            L::Target: Logger,
            P::Target: Persist<ChannelSigner>,
 {
-       /// Adds the monitor that watches the channel referred to by the given outpoint.
-       ///
-       /// Calls back to [`chain::Filter`] with the funding transaction and outputs to watch.
-       ///
-       /// Note that we persist the given `ChannelMonitor` while holding the `ChainMonitor`
-       /// monitors lock.
-       fn watch_channel(&self, funding_outpoint: OutPoint, monitor: ChannelMonitor<ChannelSigner>) -> ChannelMonitorUpdateStatus {
+       fn watch_channel(&self, funding_outpoint: OutPoint, monitor: ChannelMonitor<ChannelSigner>) -> Result<ChannelMonitorUpdateStatus, ()> {
                let mut monitors = self.monitors.write().unwrap();
                let entry = match monitors.entry(funding_outpoint) {
                        hash_map::Entry::Occupied(_) => {
                                log_error!(self.logger, "Failed to add new channel data: channel monitor for given outpoint is already present");
-                               return ChannelMonitorUpdateStatus::PermanentFailure
+                               return Err(());
                        },
                        hash_map::Entry::Vacant(e) => e,
                };
@@ -691,13 +713,14 @@ where C::Target: chain::Filter,
                                log_info!(self.logger, "Persistence of new ChannelMonitor for channel {} in progress", log_funding_info!(monitor));
                                pending_monitor_updates.push(update_id);
                        },
-                       ChannelMonitorUpdateStatus::PermanentFailure => {
-                               log_error!(self.logger, "Persistence of new ChannelMonitor for channel {} failed", log_funding_info!(monitor));
-                               return persist_res;
-                       },
                        ChannelMonitorUpdateStatus::Completed => {
                                log_info!(self.logger, "Persistence of new ChannelMonitor for channel {} completed", log_funding_info!(monitor));
-                       }
+                       },
+                       ChannelMonitorUpdateStatus::UnrecoverableError => {
+                               let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
+                               log_error!(self.logger, "{}", err_str);
+                               panic!("{}", err_str);
+                       },
                }
                if let Some(ref chain_source) = self.chain_source {
                        monitor.load_outputs_to_watch(chain_source);
@@ -705,28 +728,25 @@ where C::Target: chain::Filter,
                entry.insert(MonitorHolder {
                        monitor,
                        pending_monitor_updates: Mutex::new(pending_monitor_updates),
-                       channel_perm_failed: AtomicBool::new(false),
                        last_chain_persist_height: AtomicUsize::new(self.highest_chain_height.load(Ordering::Acquire)),
                });
-               persist_res
+               Ok(persist_res)
        }
 
-       /// Note that we persist the given `ChannelMonitor` update while holding the
-       /// `ChainMonitor` monitors lock.
        fn update_channel(&self, funding_txo: OutPoint, update: &ChannelMonitorUpdate) -> ChannelMonitorUpdateStatus {
                // Update the monitor that watches the channel referred to by the given outpoint.
                let monitors = self.monitors.read().unwrap();
-               match monitors.get(&funding_txo) {
+               let ret = match monitors.get(&funding_txo) {
                        None => {
                                log_error!(self.logger, "Failed to update channel monitor: no such monitor registered");
 
                                // We should never ever trigger this from within ChannelManager. Technically a
                                // user could use this object with some proxying in between which makes this
                                // possible, but in tests and fuzzing, this should be a panic.
-                               #[cfg(any(test, fuzzing))]
+                               #[cfg(debug_assertions)]
                                panic!("ChannelManager generated a channel update for a channel that was not yet registered!");
-                               #[cfg(not(any(test, fuzzing)))]
-                               ChannelMonitorUpdateStatus::PermanentFailure
+                               #[cfg(not(debug_assertions))]
+                               ChannelMonitorUpdateStatus::InProgress
                        },
                        Some(monitor_state) => {
                                let monitor = &monitor_state.monitor;
@@ -745,23 +765,28 @@ where C::Target: chain::Filter,
                                                pending_monitor_updates.push(update_id);
                                                log_debug!(self.logger, "Persistence of ChannelMonitorUpdate for channel {} in progress", log_funding_info!(monitor));
                                        },
-                                       ChannelMonitorUpdateStatus::PermanentFailure => {
-                                               monitor_state.channel_perm_failed.store(true, Ordering::Release);
-                                               log_error!(self.logger, "Persistence of ChannelMonitorUpdate for channel {} failed", log_funding_info!(monitor));
-                                       },
                                        ChannelMonitorUpdateStatus::Completed => {
                                                log_debug!(self.logger, "Persistence of ChannelMonitorUpdate for channel {} completed", log_funding_info!(monitor));
                                        },
+                                       ChannelMonitorUpdateStatus::UnrecoverableError => { /* we'll panic in a moment */ },
                                }
                                if update_res.is_err() {
-                                       ChannelMonitorUpdateStatus::PermanentFailure
-                               } else if monitor_state.channel_perm_failed.load(Ordering::Acquire) {
-                                       ChannelMonitorUpdateStatus::PermanentFailure
+                                       ChannelMonitorUpdateStatus::InProgress
                                } else {
                                        persist_res
                                }
                        }
+               };
+               if let ChannelMonitorUpdateStatus::UnrecoverableError = ret {
+                       // Take the monitors lock for writing so that we poison it and any future
+                       // operations going forward fail immediately.
+                       core::mem::drop(monitors);
+                       let _poison = self.monitors.write().unwrap();
+                       let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
+                       log_error!(self.logger, "{}", err_str);
+                       panic!("{}", err_str);
                }
+               ret
        }
 
        fn release_pending_monitor_events(&self) -> Vec<(OutPoint, Vec<MonitorEvent>, Option<PublicKey>)> {
@@ -774,17 +799,6 @@ where C::Target: chain::Filter,
                        {
                                log_debug!(self.logger, "A Channel Monitor sync is still in progress, refusing to provide monitor events!");
                        } else {
-                               if monitor_state.channel_perm_failed.load(Ordering::Acquire) {
-                                       // If a `UpdateOrigin::ChainSync` persistence failed with `PermanantFailure`,
-                                       // we don't really know if the latest `ChannelMonitor` state is on disk or not.
-                                       // We're supposed to hold monitor updates until the latest state is on disk to
-                                       // avoid duplicate events, but the user told us persistence is screw-y and may
-                                       // not complete. We can't hold events forever because we may learn some payment
-                                       // preimage, so instead we just log and hope the user complied with the
-                                       // `PermanentFailure` requirements of having at least the local-disk copy
-                                       // updated.
-                                       log_info!(self.logger, "A Channel Monitor sync returned PermanentFailure. Returning monitor events but duplicate events may appear after reload!");
-                               }
                                if is_pending_monitor_update {
                                        log_error!(self.logger, "A ChannelMonitor sync took longer than {} blocks to complete.", LATENCY_GRACE_PERIOD_BLOCKS);
                                        log_error!(self.logger, "   To avoid funds-loss, we are allowing monitor updates to be released.");
@@ -831,12 +845,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L
 
 #[cfg(test)]
 mod tests {
-       use crate::{check_added_monitors, check_closed_broadcast, check_closed_event};
+       use crate::check_added_monitors;
        use crate::{expect_payment_claimed, expect_payment_path_successful, get_event_msg};
        use crate::{get_htlc_update_msgs, get_local_commitment_txn, get_revoke_commit_msgs, get_route_and_payment_hash, unwrap_send_err};
        use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Watch};
        use crate::chain::channelmonitor::LATENCY_GRACE_PERIOD_BLOCKS;
-       use crate::events::{Event, ClosureReason, MessageSendEvent, MessageSendEventsProvider};
+       use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider};
        use crate::ln::channelmanager::{PaymentSendFailure, PaymentId, RecipientOnionFields};
        use crate::ln::functional_test_utils::*;
        use crate::ln::msgs::ChannelMessageHandler;
@@ -988,12 +1002,8 @@ mod tests {
                chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
                unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, second_payment_hash,
                                RecipientOnionFields::secret_only(second_payment_secret), PaymentId(second_payment_hash.0)
-                       ), true, APIError::ChannelUnavailable { ref err },
-                       assert!(err.contains("ChannelMonitor storage failure")));
-               check_added_monitors!(nodes[0], 2); // After the failure we generate a close-channel monitor update
-               check_closed_broadcast!(nodes[0], true);
-               check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() },
-                       [nodes[1].node.get_our_node_id()], 100000);
+                       ), false, APIError::MonitorUpdateInProgress, {});
+               check_added_monitors!(nodes[0], 1);
 
                // However, as the ChainMonitor is still waiting for the original persistence to complete,
                // it won't yet release the MonitorEvents.
@@ -1022,7 +1032,8 @@ mod tests {
        }
 
        #[test]
-       fn update_during_chainsync_fails_channel() {
+       #[cfg(feature = "std")]
+       fn update_during_chainsync_poisons_channel() {
                let chanmon_cfgs = create_chanmon_cfgs(2);
                let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
                let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
@@ -1030,18 +1041,15 @@ mod tests {
                create_announced_chan_between_nodes(&nodes, 0, 1);
 
                chanmon_cfgs[0].persister.chain_sync_monitor_persistences.lock().unwrap().clear();
-               chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-
-               connect_blocks(&nodes[0], 1);
-               // Before processing events, the ChannelManager will still think the Channel is open and
-               // there won't be any ChannelMonitorUpdates
-               assert_eq!(nodes[0].node.list_channels().len(), 1);
-               check_added_monitors!(nodes[0], 0);
-               // ... however once we get events once, the channel will close, creating a channel-closed
-               // ChannelMonitorUpdate.
-               check_closed_broadcast!(nodes[0], true);
-               check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "Failed to persist ChannelMonitor update during chain sync".to_string() },
-                       [nodes[1].node.get_our_node_id()], 100000);
-               check_added_monitors!(nodes[0], 1);
+               chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::UnrecoverableError);
+
+               assert!(std::panic::catch_unwind(|| {
+                       // Returning an UnrecoverableError should always panic immediately
+                       connect_blocks(&nodes[0], 1);
+               }).is_err());
+               assert!(std::panic::catch_unwind(|| {
+                       // ...and also poison our locks causing later use to panic as well
+                       core::mem::drop(nodes);
+               }).is_err());
        }
 }
index 8c92ab47cc00b6611075e7a65d72cf4653365664..3f9c83bb54393290c40b9a6784bcce7dd48c53da 100644 (file)
@@ -134,7 +134,7 @@ pub enum MonitorEvent {
        HTLCEvent(HTLCUpdate),
 
        /// A monitor event that the Channel's commitment transaction was confirmed.
-       CommitmentTxConfirmed(OutPoint),
+       HolderForceClosed(OutPoint),
 
        /// Indicates a [`ChannelMonitor`] update has completed. See
        /// [`ChannelMonitorUpdateStatus::InProgress`] for more information on how this is used.
@@ -150,24 +150,18 @@ pub enum MonitorEvent {
                /// same [`ChannelMonitor`] have been applied and persisted.
                monitor_update_id: u64,
        },
-
-       /// Indicates a [`ChannelMonitor`] update has failed. See
-       /// [`ChannelMonitorUpdateStatus::PermanentFailure`] for more information on how this is used.
-       ///
-       /// [`ChannelMonitorUpdateStatus::PermanentFailure`]: super::ChannelMonitorUpdateStatus::PermanentFailure
-       UpdateFailed(OutPoint),
 }
 impl_writeable_tlv_based_enum_upgradable!(MonitorEvent,
-       // Note that Completed and UpdateFailed are currently never serialized to disk as they are
-       // generated only in ChainMonitor
+       // Note that Completed is currently never serialized to disk as it is generated only in
+       // ChainMonitor.
        (0, Completed) => {
                (0, funding_txo, required),
                (2, monitor_update_id, required),
        },
 ;
        (2, HTLCEvent),
-       (4, CommitmentTxConfirmed),
-       (6, UpdateFailed),
+       (4, HolderForceClosed),
+       // 6 was `UpdateFailed` until LDK 0.0.117
 );
 
 /// Simple structure sent back by `chain::Watch` when an HTLC from a forward channel is detected on
@@ -1037,7 +1031,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
 
                writer.write_all(&(self.pending_monitor_events.iter().filter(|ev| match ev {
                        MonitorEvent::HTLCEvent(_) => true,
-                       MonitorEvent::CommitmentTxConfirmed(_) => true,
+                       MonitorEvent::HolderForceClosed(_) => true,
                        _ => false,
                }).count() as u64).to_be_bytes())?;
                for event in self.pending_monitor_events.iter() {
@@ -1046,7 +1040,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
                                        0u8.write(writer)?;
                                        upd.write(writer)?;
                                },
-                               MonitorEvent::CommitmentTxConfirmed(_) => 1u8.write(writer)?,
+                               MonitorEvent::HolderForceClosed(_) => 1u8.write(writer)?,
                                _ => {}, // Covered in the TLV writes below
                        }
                }
@@ -1488,21 +1482,20 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                self.inner.lock().unwrap().counterparty_node_id
        }
 
-       /// Used by ChannelManager deserialization to broadcast the latest holder state if its copy of
-       /// the Channel was out-of-date.
+       /// Used by [`ChannelManager`] deserialization to broadcast the latest holder state if its copy
+       /// of the channel state was out-of-date.
        ///
        /// You may also use this to broadcast the latest local commitment transaction, either because
-       /// a monitor update failed with [`ChannelMonitorUpdateStatus::PermanentFailure`] or because we've
-       /// fallen behind (i.e. we've received proof that our counterparty side knows a revocation
-       /// secret we gave them that they shouldn't know).
+       /// a monitor update failed or because we've fallen behind (i.e. we've received proof that our
+       /// counterparty side knows a revocation secret we gave them that they shouldn't know).
        ///
        /// Broadcasting these transactions in the second case is UNSAFE, as they allow counterparty
        /// side to punish you. Nevertheless you may want to broadcast them if counterparty doesn't
        /// close channel with their commitment transaction after a substantial amount of time. Best
        /// may be to contact the other node operator out-of-band to coordinate other options available
-       /// to you. In any-case, the choice is up to you.
+       /// to you.
        ///
-       /// [`ChannelMonitorUpdateStatus::PermanentFailure`]: super::ChannelMonitorUpdateStatus::PermanentFailure
+       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
        pub fn get_latest_holder_commitment_txn<L: Deref>(&self, logger: &L) -> Vec<Transaction>
        where L::Target: Logger {
                self.inner.lock().unwrap().get_latest_holder_commitment_txn(logger)
@@ -2533,7 +2526,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        txs.push(tx);
                }
                broadcaster.broadcast_transactions(&txs);
-               self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
+               self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
        }
 
        pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: F, logger: &L) -> Result<(), ()>
@@ -2599,6 +2592,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => {
                                        log_trace!(logger, "Updating ChannelMonitor with commitment secret");
                                        if let Err(e) = self.provide_secret(*idx, *secret) {
+                                               debug_assert!(false, "Latest counterparty commitment secret was invalid");
                                                log_error!(logger, "Providing latest counterparty commitment secret failed/was refused:");
                                                log_error!(logger, "    {}", e);
                                                ret = Err(());
@@ -2645,7 +2639,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                log_error!(logger, "    in channel monitor for channel {}!", &self.funding_info.0.to_channel_id());
                                                log_error!(logger, "    Read the docs for ChannelMonitor::get_latest_holder_commitment_txn and take manual action!");
                                        } else {
-                                               // If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
+                                               // If we generated a MonitorEvent::HolderForceClosed, the ChannelManager
                                                // will still give us a ChannelForceClosed event with !should_broadcast, but we
                                                // shouldn't print the scary warning above.
                                                log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction.");
@@ -3490,7 +3484,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone(), self.channel_value_satoshis, self.onchain_tx_handler.channel_type_features().clone());
                        let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), self.best_block.height());
                        claimable_outpoints.push(commitment_package);
-                       self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
+                       self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
                        // Although we aren't signing the transaction directly here, the transaction will be signed
                        // in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject
                        // new channel updates.
@@ -4254,7 +4248,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                for _ in 0..pending_monitor_events_len {
                        let ev = match <u8 as Readable>::read(reader)? {
                                0 => MonitorEvent::HTLCEvent(Readable::read(reader)?),
-                               1 => MonitorEvent::CommitmentTxConfirmed(funding_info.0),
+                               1 => MonitorEvent::HolderForceClosed(funding_info.0),
                                _ => return Err(DecodeError::InvalidValue)
                        };
                        pending_monitor_events.as_mut().unwrap().push(ev);
@@ -4413,13 +4407,12 @@ mod tests {
        use crate::chain::chaininterface::LowerBoundedFeeEstimator;
 
        use super::ChannelMonitorUpdateStep;
-       use crate::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err};
+       use crate::{check_added_monitors, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err};
        use crate::chain::{BestBlock, Confirm};
        use crate::chain::channelmonitor::ChannelMonitor;
        use crate::chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT};
        use crate::chain::transaction::OutPoint;
        use crate::sign::InMemorySigner;
-       use crate::events::ClosureReason;
        use crate::ln::{PaymentPreimage, PaymentHash};
        use crate::ln::chan_utils;
        use crate::ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
@@ -4485,18 +4478,14 @@ mod tests {
                let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000);
                unwrap_send_err!(nodes[1].node.send_payment_with_route(&route, payment_hash,
                                RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
-                       ), true, APIError::ChannelUnavailable { ref err },
-                       assert!(err.contains("ChannelMonitor storage failure")));
-               check_added_monitors!(nodes[1], 2); // After the failure we generate a close-channel monitor update
-               check_closed_broadcast!(nodes[1], true);
-               check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() }, 
-                       [nodes[0].node.get_our_node_id()], 100000);
+                       ), false, APIError::MonitorUpdateInProgress, {});
+               check_added_monitors!(nodes[1], 1);
 
                // Build a new ChannelMonitorUpdate which contains both the failing commitment tx update
                // and provides the claim preimages for the two pending HTLCs. The first update generates
                // an error, but the point of this test is to ensure the later updates are still applied.
                let monitor_updates = nodes[1].chain_monitor.monitor_updates.lock().unwrap();
-               let mut replay_update = monitor_updates.get(&channel.2).unwrap().iter().rev().skip(1).next().unwrap().clone();
+               let mut replay_update = monitor_updates.get(&channel.2).unwrap().iter().rev().next().unwrap().clone();
                assert_eq!(replay_update.updates.len(), 1);
                if let ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } = replay_update.updates[0] {
                } else { panic!(); }
index 236b10a7b19d1288b5d74f784d066c2a8bacc169..89e0b155cf68b2db3f24c1a0b23f1e01aacb67b1 100644 (file)
@@ -176,6 +176,25 @@ pub trait Confirm {
 }
 
 /// An enum representing the status of a channel monitor update persistence.
+///
+/// These are generally used as the return value for an implementation of [`Persist`] which is used
+/// as the storage layer for a [`ChainMonitor`]. See the docs on [`Persist`] for a high-level
+/// explanation of how to handle different cases.
+///
+/// While `UnrecoverableError` is provided as a failure variant, it is not truly "handled" on the
+/// calling side, and generally results in an immediate panic. For those who prefer to avoid
+/// panics, `InProgress` can be used and you can retry the update operation in the background or
+/// shut down cleanly.
+///
+/// Note that channels should generally *not* be force-closed after a persistence failure.
+/// Force-closing with the latest [`ChannelMonitorUpdate`] applied may result in a transaction
+/// being broadcast which can only be spent by the latest [`ChannelMonitor`]! Thus, if the
+/// latest [`ChannelMonitor`] is not durably persisted anywhere and exists only in memory, naively
+/// calling [`ChannelManager::force_close_broadcasting_latest_txn`] *may result in loss of funds*!
+///
+/// [`Persist`]: chainmonitor::Persist
+/// [`ChainMonitor`]: chainmonitor::ChainMonitor
+/// [`ChannelManager::force_close_broadcasting_latest_txn`]: crate::ln::channelmanager::ChannelManager::force_close_broadcasting_latest_txn
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum ChannelMonitorUpdateStatus {
        /// The update has been durably persisted and all copies of the relevant [`ChannelMonitor`]
@@ -184,17 +203,13 @@ pub enum ChannelMonitorUpdateStatus {
        /// This includes performing any `fsync()` calls required to ensure the update is guaranteed to
        /// be available on restart even if the application crashes.
        Completed,
-       /// Used to indicate a temporary failure (eg connection to a watchtower or remote backup of
-       /// our state failed, but is expected to succeed at some point in the future).
-       ///
-       /// Such a failure will "freeze" a channel, preventing us from revoking old states or
-       /// submitting new commitment transactions to the counterparty. Once the update(s) which failed
-       /// have been successfully applied, a [`MonitorEvent::Completed`] can be used to restore the
-       /// channel to an operational state.
+       /// Indicates that the update will happen asynchronously in the background or that a transient
+       /// failure occurred which is being retried in the background and will eventually complete.
        ///
-       /// Note that a given [`ChannelManager`] will *never* re-generate a [`ChannelMonitorUpdate`].
-       /// If you return this error you must ensure that it is written to disk safely before writing
-       /// the latest [`ChannelManager`] state, or you should return [`PermanentFailure`] instead.
+       /// This will "freeze" a channel, preventing us from revoking old states or submitting a new
+       /// commitment transaction to the counterparty. Once the update(s) which are `InProgress` have
+       /// been completed, a [`MonitorEvent::Completed`] can be used to restore the channel to an
+       /// operational state.
        ///
        /// Even when a channel has been "frozen", updates to the [`ChannelMonitor`] can continue to
        /// occur (e.g. if an inbound HTLC which we forwarded was claimed upstream, resulting in us
@@ -204,74 +219,40 @@ pub enum ChannelMonitorUpdateStatus {
        /// until a [`MonitorEvent::Completed`] is provided, even if you return no error on a later
        /// monitor update for the same channel.
        ///
-       /// For deployments where a copy of ChannelMonitors and other local state are backed up in a
-       /// remote location (with local copies persisted immediately), it is anticipated that all
+       /// For deployments where a copy of [`ChannelMonitor`]s and other local state are backed up in
+       /// remote location (with local copies persisted immediately), it is anticipated that all
        /// updates will return [`InProgress`] until the remote copies could be updated.
        ///
-       /// [`PermanentFailure`]: ChannelMonitorUpdateStatus::PermanentFailure
+       /// Note that while fully asynchronous persistence of [`ChannelMonitor`] data is generally
+       /// reliable, this feature is considered beta, and a handful of edge-cases remain. Until the
+       /// remaining cases are fixed, in rare cases, *using this feature may lead to funds loss*.
+       ///
        /// [`InProgress`]: ChannelMonitorUpdateStatus::InProgress
-       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
        InProgress,
-       /// Used to indicate no further channel monitor updates will be allowed (likely a disk failure
-       /// or a remote copy of this [`ChannelMonitor`] is no longer reachable and thus not updatable).
-       ///
-       /// When this is returned, [`ChannelManager`] will force-close the channel but *not* broadcast
-       /// our current commitment transaction. This avoids a dangerous case where a local disk failure
-       /// (e.g. the Linux-default remounting of the disk as read-only) causes [`PermanentFailure`]s
-       /// for all monitor updates. If we were to broadcast our latest commitment transaction and then
-       /// restart, we could end up reading a previous [`ChannelMonitor`] and [`ChannelManager`],
-       /// revoking our now-broadcasted state before seeing it confirm and losing all our funds.
-       ///
-       /// Note that this is somewhat of a tradeoff - if the disk is really gone and we may have lost
-       /// the data permanently, we really should broadcast immediately. If the data can be recovered
-       /// with manual intervention, we'd rather close the channel, rejecting future updates to it,
-       /// and broadcast the latest state only if we have HTLCs to claim which are timing out (which
-       /// we do as long as blocks are connected).
+       /// Indicates that an update has failed and will not complete at any point in the future.
        ///
-       /// In order to broadcast the latest local commitment transaction, you'll need to call
-       /// [`ChannelMonitor::get_latest_holder_commitment_txn`] and broadcast the resulting
-       /// transactions once you've safely ensured no further channel updates can be generated by your
-       /// [`ChannelManager`].
+       /// Currently returning this variant will cause LDK to immediately panic to encourage immediate
+       /// shutdown. In the future this may be updated to disconnect peers and refuse to continue
+       /// normal operation without a panic.
        ///
-       /// Note that at least one final [`ChannelMonitorUpdate`] may still be provided, which must
-       /// still be processed by a running [`ChannelMonitor`]. This final update will mark the
-       /// [`ChannelMonitor`] as finalized, ensuring no further updates (e.g. revocation of the latest
-       /// commitment transaction) are allowed.
+       /// Applications which wish to perform an orderly shutdown after failure should consider
+       /// returning [`InProgress`] instead and simply shut down without ever marking the update
+       /// complete.
        ///
-       /// Note that even if you return a [`PermanentFailure`] due to unavailability of secondary
-       /// [`ChannelMonitor`] copies, you should still make an attempt to store the update where
-       /// possible to ensure you can claim HTLC outputs on the latest commitment transaction
-       /// broadcasted later.
-       ///
-       /// In case of distributed watchtowers deployment, the new version must be written to disk, as
-       /// state may have been stored but rejected due to a block forcing a commitment broadcast. This
-       /// storage is used to claim outputs of rejected state confirmed onchain by another watchtower,
-       /// lagging behind on block processing.
-       ///
-       /// [`PermanentFailure`]: ChannelMonitorUpdateStatus::PermanentFailure
-       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
-       PermanentFailure,
+       /// [`InProgress`]: ChannelMonitorUpdateStatus::InProgress
+       UnrecoverableError,
 }
 
 /// The `Watch` trait defines behavior for watching on-chain activity pertaining to channels as
 /// blocks are connected and disconnected.
 ///
 /// Each channel is associated with a [`ChannelMonitor`]. Implementations of this trait are
-/// responsible for maintaining a set of monitors such that they can be updated accordingly as
-/// channel state changes and HTLCs are resolved. See method documentation for specific
-/// requirements.
-///
-/// Implementations **must** ensure that updates are successfully applied and persisted upon method
-/// completion. If an update fails with a [`PermanentFailure`], then it must immediately shut down
-/// without taking any further action such as persisting the current state.
+/// responsible for maintaining a set of monitors such that they can be updated as channel state
+/// changes. On each update, *all copies* of a [`ChannelMonitor`] must be updated and the update
+/// persisted to disk to ensure that the latest [`ChannelMonitor`] state can be reloaded if the
+/// application crashes.
 ///
-/// If an implementation maintains multiple instances of a channel's monitor (e.g., by storing
-/// backup copies), then it must ensure that updates are applied across all instances. Otherwise, it
-/// could result in a revoked transaction being broadcast, allowing the counterparty to claim all
-/// funds in the channel. See [`ChannelMonitorUpdateStatus`] for more details about how to handle
-/// multiple instances.
-///
-/// [`PermanentFailure`]: ChannelMonitorUpdateStatus::PermanentFailure
+/// See method documentation and [`ChannelMonitorUpdateStatus`] for specific requirements.
 pub trait Watch<ChannelSigner: WriteableEcdsaChannelSigner> {
        /// Watches a channel identified by `funding_txo` using `monitor`.
        ///
@@ -279,20 +260,32 @@ pub trait Watch<ChannelSigner: WriteableEcdsaChannelSigner> {
        /// with any spends of outputs returned by [`get_outputs_to_watch`]. In practice, this means
        /// calling [`block_connected`] and [`block_disconnected`] on the monitor.
        ///
-       /// Note: this interface MUST error with [`ChannelMonitorUpdateStatus::PermanentFailure`] if
-       /// the given `funding_txo` has previously been registered via `watch_channel`.
+       /// A return of `Err(())` indicates that the channel should immediately be force-closed without
+       /// broadcasting the funding transaction.
+       ///
+       /// If the given `funding_txo` has previously been registered via `watch_channel`, `Err(())`
+       /// must be returned.
        ///
        /// [`get_outputs_to_watch`]: channelmonitor::ChannelMonitor::get_outputs_to_watch
        /// [`block_connected`]: channelmonitor::ChannelMonitor::block_connected
        /// [`block_disconnected`]: channelmonitor::ChannelMonitor::block_disconnected
-       fn watch_channel(&self, funding_txo: OutPoint, monitor: ChannelMonitor<ChannelSigner>) -> ChannelMonitorUpdateStatus;
+       fn watch_channel(&self, funding_txo: OutPoint, monitor: ChannelMonitor<ChannelSigner>) -> Result<ChannelMonitorUpdateStatus, ()>;
 
        /// Updates a channel identified by `funding_txo` by applying `update` to its monitor.
        ///
-       /// Implementations must call [`update_monitor`] with the given update. See
-       /// [`ChannelMonitorUpdateStatus`] for invariants around returning an error.
+       /// Implementations must call [`ChannelMonitor::update_monitor`] with the given update. This
+       /// may fail (returning an `Err(())`), in which case this should return
+       /// [`ChannelMonitorUpdateStatus::InProgress`] (and the update should never complete). This
+       /// generally implies the channel has been closed (either by the funding outpoint being spent
+       /// on-chain or the [`ChannelMonitor`] having decided to do so and broadcasted a transaction),
+       /// and the [`ChannelManager`] state will be updated once it sees the funding spend on-chain.
        ///
-       /// [`update_monitor`]: channelmonitor::ChannelMonitor::update_monitor
+       /// In general, persistence failures should be retried after returning
+       /// [`ChannelMonitorUpdateStatus::InProgress`] and eventually complete. If a failure truly
+       /// cannot be retried, the node should shut down immediately after returning
+       /// [`ChannelMonitorUpdateStatus::UnrecoverableError`], see its documentation for more info.
+       ///
+       /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager
        fn update_channel(&self, funding_txo: OutPoint, update: &ChannelMonitorUpdate) -> ChannelMonitorUpdateStatus;
 
        /// Returns any monitor events since the last call. Subsequent calls must only return new
index b66a2f70d3369f4aff6e012025b3e995d6918a9e..382ffac3e8e886c303207798fafe528c78406b0d 100644 (file)
@@ -976,14 +976,23 @@ impl PackageTemplate {
        ) -> u32 where F::Target: FeeEstimator {
                let feerate_estimate = fee_estimator.bounded_sat_per_1000_weight(conf_target);
                if self.feerate_previous != 0 {
-                       // If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee...
+                       // Use the new fee estimate if it's higher than the one previously used.
                        if feerate_estimate as u64 > self.feerate_previous {
                                feerate_estimate
                        } else if !force_feerate_bump {
                                self.feerate_previous.try_into().unwrap_or(u32::max_value())
                        } else {
-                               // ...else just increase the previous feerate by 25% (because that's a nice number)
-                               (self.feerate_previous + (self.feerate_previous / 4)).try_into().unwrap_or(u32::max_value())
+                               // Our fee estimate has decreased, but our transaction remains unconfirmed after
+                               // using our previous fee estimate. This may point to an unreliable fee estimator,
+                               // so we choose to bump our previous feerate by 25%, making sure we don't use a
+                               // lower feerate or overpay by a large margin by limiting it to 5x the new fee
+                               // estimate.
+                               let previous_feerate = self.feerate_previous.try_into().unwrap_or(u32::max_value());
+                               let mut new_feerate = previous_feerate.saturating_add(previous_feerate / 4);
+                               if new_feerate > feerate_estimate * 5 {
+                                       new_feerate = cmp::max(feerate_estimate * 5, previous_feerate);
+                               }
+                               new_feerate
                        }
                } else {
                        feerate_estimate
index 2d44275abb5aa2288009c681e26df14e959a2ce1..35e9da60544ddc680535983767117e9faf732afd 100644 (file)
@@ -14,7 +14,7 @@
 use alloc::collections::BTreeMap;
 use core::ops::Deref;
 
-use crate::chain::chaininterface::{BroadcasterInterface, compute_feerate_sat_per_1000_weight, fee_for_weight, FEERATE_FLOOR_SATS_PER_KW};
+use crate::chain::chaininterface::{BroadcasterInterface, fee_for_weight};
 use crate::chain::ClaimId;
 use crate::io_extras::sink;
 use crate::ln::channel::ANCHOR_OUTPUT_VALUE_SATOSHI;
@@ -542,7 +542,7 @@ where
        fn select_confirmed_utxos_internal(
                &self, utxos: &[Utxo], claim_id: ClaimId, force_conflicting_utxo_spend: bool,
                tolerate_high_network_feerates: bool, target_feerate_sat_per_1000_weight: u32,
-               preexisting_tx_weight: u64, target_amount_sat: u64,
+               preexisting_tx_weight: u64, input_amount_sat: u64, target_amount_sat: u64,
        ) -> Result<CoinSelection, ()> {
                let mut locked_utxos = self.locked_utxos.lock().unwrap();
                let mut eligible_utxos = utxos.iter().filter_map(|utxo| {
@@ -569,7 +569,7 @@ where
                }).collect::<Vec<_>>();
                eligible_utxos.sort_unstable_by_key(|(utxo, _)| utxo.output.value);
 
-               let mut selected_amount = 0;
+               let mut selected_amount = input_amount_sat;
                let mut total_fees = fee_for_weight(target_feerate_sat_per_1000_weight, preexisting_tx_weight);
                let mut selected_utxos = Vec::new();
                for (utxo, fee_to_spend_utxo) in eligible_utxos {
@@ -632,13 +632,14 @@ where
 
                let preexisting_tx_weight = 2 /* segwit marker & flag */ + total_input_weight +
                        ((BASE_TX_SIZE + total_output_size) * WITNESS_SCALE_FACTOR as u64);
+               let input_amount_sat: u64 = must_spend.iter().map(|input| input.previous_utxo.value).sum();
                let target_amount_sat = must_pay_to.iter().map(|output| output.value).sum();
                let do_coin_selection = |force_conflicting_utxo_spend: bool, tolerate_high_network_feerates: bool| {
                        log_debug!(self.logger, "Attempting coin selection targeting {} sat/kW (force_conflicting_utxo_spend = {}, tolerate_high_network_feerates = {})",
                                target_feerate_sat_per_1000_weight, force_conflicting_utxo_spend, tolerate_high_network_feerates);
                        self.select_confirmed_utxos_internal(
                                &utxos, claim_id, force_conflicting_utxo_spend, tolerate_high_network_feerates,
-                               target_feerate_sat_per_1000_weight, preexisting_tx_weight, target_amount_sat,
+                               target_feerate_sat_per_1000_weight, preexisting_tx_weight, input_amount_sat, target_amount_sat,
                        )
                };
                do_coin_selection(false, false)
@@ -724,27 +725,22 @@ where
                commitment_tx: &Transaction, commitment_tx_fee_sat: u64, anchor_descriptor: &AnchorDescriptor,
        ) -> Result<(), ()> {
                // Our commitment transaction already has fees allocated to it, so we should take them into
-               // account. We compute its feerate and subtract it from the package target, using the result
-               // as the target feerate for our anchor transaction. Unfortunately, this results in users
-               // overpaying by a small margin since we don't yet know the anchor transaction size, and
-               // avoiding the small overpayment only makes our API even more complex.
-               let commitment_tx_sat_per_1000_weight: u32 = compute_feerate_sat_per_1000_weight(
-                       commitment_tx_fee_sat, commitment_tx.weight() as u64,
-               );
-               let anchor_target_feerate_sat_per_1000_weight = core::cmp::max(
-                       package_target_feerate_sat_per_1000_weight - commitment_tx_sat_per_1000_weight,
-                       FEERATE_FLOOR_SATS_PER_KW,
-               );
-
-               log_debug!(self.logger, "Peforming coin selection for anchor transaction targeting {} sat/kW",
-                       anchor_target_feerate_sat_per_1000_weight);
+               // account. We do so by pretending the commitment tranasction's fee and weight are part of
+               // the anchor input.
+               let mut anchor_utxo = anchor_descriptor.previous_utxo();
+               anchor_utxo.value += commitment_tx_fee_sat;
                let must_spend = vec![Input {
                        outpoint: anchor_descriptor.outpoint,
-                       previous_utxo: anchor_descriptor.previous_utxo(),
+                       previous_utxo: anchor_utxo,
                        satisfaction_weight: commitment_tx.weight() as u64 + ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT,
                }];
+               #[cfg(debug_assertions)]
+               let must_spend_amount = must_spend.iter().map(|input| input.previous_utxo.value).sum::<u64>();
+
+               log_debug!(self.logger, "Peforming coin selection for commitment package (commitment and anchor transaction) targeting {} sat/kW",
+                       package_target_feerate_sat_per_1000_weight);
                let coin_selection = self.utxo_source.select_confirmed_utxos(
-                       claim_id, must_spend, &[], anchor_target_feerate_sat_per_1000_weight,
+                       claim_id, must_spend, &[], package_target_feerate_sat_per_1000_weight,
                )?;
 
                let mut anchor_tx = Transaction {
@@ -753,10 +749,13 @@ where
                        input: vec![anchor_descriptor.unsigned_tx_input()],
                        output: vec![],
                };
+
                #[cfg(debug_assertions)]
-               let total_satisfaction_weight =
-                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>() +
-                               ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT;
+               let total_satisfaction_weight = ANCHOR_INPUT_WITNESS_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>();
+               #[cfg(debug_assertions)]
+               let total_input_amount = must_spend_amount +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum::<u64>();
 
                self.process_coin_selection(&mut anchor_tx, coin_selection);
                let anchor_txid = anchor_tx.txid();
@@ -779,6 +778,16 @@ where
                        // never underestimate.
                        assert!(expected_signed_tx_weight >= signed_tx_weight &&
                                expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+
+                       let expected_package_fee = fee_for_weight(package_target_feerate_sat_per_1000_weight,
+                               signed_tx_weight + commitment_tx.weight() as u64);
+                       let package_fee = total_input_amount -
+                               anchor_tx.output.iter().map(|output| output.value).sum::<u64>();
+                       // Our fee should be within a 5% error margin of the expected fee based on the
+                       // feerate and transaction weight and we should never pay less than required.
+                       let fee_error_margin = expected_package_fee * 5 / 100;
+                       assert!(package_fee >= expected_package_fee &&
+                               package_fee - fee_error_margin <= expected_package_fee);
                }
 
                log_info!(self.logger, "Broadcasting anchor transaction {} to bump channel close with txid {}",
@@ -818,16 +827,24 @@ where
 
                log_debug!(self.logger, "Peforming coin selection for HTLC transaction targeting {} sat/kW",
                        target_feerate_sat_per_1000_weight);
+
                #[cfg(debug_assertions)]
                let must_spend_satisfaction_weight =
                        must_spend.iter().map(|input| input.satisfaction_weight).sum::<u64>();
+               #[cfg(debug_assertions)]
+               let must_spend_amount = must_spend.iter().map(|input| input.previous_utxo.value).sum::<u64>();
+
                let coin_selection = self.utxo_source.select_confirmed_utxos(
                        claim_id, must_spend, &htlc_tx.output, target_feerate_sat_per_1000_weight,
                )?;
+
                #[cfg(debug_assertions)]
-               let total_satisfaction_weight =
-                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>() +
-                               must_spend_satisfaction_weight;
+               let total_satisfaction_weight = must_spend_satisfaction_weight +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum::<u64>();
+               #[cfg(debug_assertions)]
+               let total_input_amount = must_spend_amount +
+                       coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum::<u64>();
+
                self.process_coin_selection(&mut htlc_tx, coin_selection);
 
                #[cfg(debug_assertions)]
@@ -852,6 +869,15 @@ where
                        // never underestimate.
                        assert!(expected_signed_tx_weight >= signed_tx_weight &&
                                expected_signed_tx_weight - (expected_signed_tx_weight / 100) <= signed_tx_weight);
+
+                       let expected_signed_tx_fee = fee_for_weight(target_feerate_sat_per_1000_weight, signed_tx_weight);
+                       let signed_tx_fee = total_input_amount -
+                               htlc_tx.output.iter().map(|output| output.value).sum::<u64>();
+                       // Our fee should be within a 5% error margin of the expected fee based on the
+                       // feerate and transaction weight and we should never pay less than required.
+                       let fee_error_margin = expected_signed_tx_fee * 5 / 100;
+                       assert!(signed_tx_fee >= expected_signed_tx_fee &&
+                               signed_tx_fee - fee_error_margin <= expected_signed_tx_fee);
                }
 
                log_info!(self.logger, "Broadcasting {}", log_tx!(htlc_tx));
index 33f4bbc8c26346a75af2b77880d2d7f8d0e06fa7..dc9bbd3e4048c902116b875a1a995e5186302266 100644 (file)
@@ -37,43 +37,6 @@ use bitcoin::hashes::Hash;
 use crate::prelude::*;
 use crate::sync::{Arc, Mutex};
 
-#[test]
-fn test_simple_monitor_permanent_update_fail() {
-       // Test that we handle a simple permanent monitor update failure
-       let chanmon_cfgs = create_chanmon_cfgs(2);
-       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-       create_announced_chan_between_nodes(&nodes, 0, 1);
-
-       let (route, payment_hash_1, _, payment_secret_1) = get_route_and_payment_hash!(&nodes[0], nodes[1], 1000000);
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-       unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, payment_hash_1,
-                       RecipientOnionFields::secret_only(payment_secret_1), PaymentId(payment_hash_1.0)
-               ), true, APIError::ChannelUnavailable {..}, {});
-       check_added_monitors!(nodes[0], 2);
-
-       let events_1 = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events_1.len(), 2);
-       match events_1[0] {
-               MessageSendEvent::BroadcastChannelUpdate { .. } => {},
-               _ => panic!("Unexpected event"),
-       };
-       match events_1[1] {
-               MessageSendEvent::HandleError { node_id, .. } => assert_eq!(node_id, nodes[1].node.get_our_node_id()),
-               _ => panic!("Unexpected event"),
-       };
-
-       assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
-
-       // TODO: Once we hit the chain with the failure transaction we should check that we get a
-       // PaymentPathFailed event
-
-       assert_eq!(nodes[0].node.list_channels().len(), 0);
-       check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() },
-               [nodes[1].node.get_our_node_id()], 100000);
-}
-
 #[test]
 fn test_monitor_and_persister_update_fail() {
        // Test that if both updating the `ChannelMonitor` and persisting the updated
@@ -117,14 +80,11 @@ fn test_monitor_and_persister_update_fail() {
                        new_monitor
                };
                let chain_mon = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
-               assert_eq!(chain_mon.watch_channel(outpoint, new_monitor), ChannelMonitorUpdateStatus::Completed);
+               assert_eq!(chain_mon.watch_channel(outpoint, new_monitor), Ok(ChannelMonitorUpdateStatus::Completed));
                chain_mon
        };
        chain_mon.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, Vec::new()), 200);
 
-       // Set the persister's return value to be a InProgress.
-       persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
-
        // Try to update ChannelMonitor
        nodes[1].node.claim_funds(preimage);
        expect_payment_claimed!(nodes[1], payment_hash, 9_000_000);
@@ -133,17 +93,21 @@ fn test_monitor_and_persister_update_fail() {
        let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
        assert_eq!(updates.update_fulfill_htlcs.len(), 1);
        nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
+
        {
                let mut node_0_per_peer_lock;
                let mut node_0_peer_state_lock;
                if let ChannelPhase::Funded(ref mut channel) = get_channel_ref!(nodes[0], nodes[1], node_0_per_peer_lock, node_0_peer_state_lock, chan.2) {
                        if let Ok(Some(update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
-                               // Check that even though the persister is returning a InProgress,
-                               // because the update is bogus, ultimately the error that's returned
-                               // should be a PermanentFailure.
-                               if let ChannelMonitorUpdateStatus::PermanentFailure = chain_mon.chain_monitor.update_channel(outpoint, &update) {} else { panic!("Expected monitor error to be permanent"); }
-                               logger.assert_log_regex("lightning::chain::chainmonitor", regex::Regex::new("Persistence of ChannelMonitorUpdate for channel [0-9a-f]* in progress").unwrap(), 1);
-                               assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
+                               // Check that the persister returns InProgress (and will never actually complete)
+                               // as the monitor update errors.
+                               if let ChannelMonitorUpdateStatus::InProgress = chain_mon.chain_monitor.update_channel(outpoint, &update) {} else { panic!("Expected monitor paused"); }
+                               logger.assert_log_regex("lightning::chain::chainmonitor", regex::Regex::new("Failed to update ChannelMonitor for channel [0-9a-f]*.").unwrap(), 1);
+
+                               // Apply the monitor update to the original ChainMonitor, ensuring the
+                               // ChannelManager and ChannelMonitor aren't out of sync.
+                               assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update),
+                                       ChannelMonitorUpdateStatus::Completed);
                        } else { assert!(false); }
                } else {
                        assert!(false);
@@ -151,8 +115,7 @@ fn test_monitor_and_persister_update_fail() {
        }
 
        check_added_monitors!(nodes[0], 1);
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
+       expect_payment_sent(&nodes[0], preimage, None, false, false);
 }
 
 fn do_test_simple_monitor_temporary_update_fail(disconnect: bool) {
@@ -2675,68 +2638,6 @@ fn test_temporary_error_during_shutdown() {
        check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000);
 }
 
-#[test]
-fn test_permanent_error_during_sending_shutdown() {
-       // Test that permanent failures when updating the monitor's shutdown script result in a force
-       // close when initiating a cooperative close.
-       let mut config = test_default_channel_config();
-       config.channel_handshake_config.commit_upfront_shutdown_pubkey = false;
-
-       let chanmon_cfgs = create_chanmon_cfgs(2);
-       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config), None]);
-       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
-       let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-
-       assert!(nodes[0].node.close_channel(&channel_id, &nodes[1].node.get_our_node_id()).is_ok());
-
-       // We always send the `shutdown` response when initiating a shutdown, even if we immediately
-       // close the channel thereafter.
-       let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(msg_events.len(), 3);
-       if let MessageSendEvent::SendShutdown { .. } = msg_events[0] {} else { panic!(); }
-       if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg_events[1] {} else { panic!(); }
-       if let MessageSendEvent::HandleError { .. } =  msg_events[2] {} else { panic!(); }
-
-       check_added_monitors!(nodes[0], 2);
-       check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() },
-               [nodes[1].node.get_our_node_id()], 100000);
-}
-
-#[test]
-fn test_permanent_error_during_handling_shutdown() {
-       // Test that permanent failures when updating the monitor's shutdown script result in a force
-       // close when handling a cooperative close.
-       let mut config = test_default_channel_config();
-       config.channel_handshake_config.commit_upfront_shutdown_pubkey = false;
-
-       let chanmon_cfgs = create_chanmon_cfgs(2);
-       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
-       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(config)]);
-       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
-       let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
-       chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-
-       assert!(nodes[0].node.close_channel(&channel_id, &nodes[1].node.get_our_node_id()).is_ok());
-       let shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
-       nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &shutdown);
-
-       // We always send the `shutdown` response when receiving a shutdown, even if we immediately
-       // close the channel thereafter.
-       let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
-       assert_eq!(msg_events.len(), 3);
-       if let MessageSendEvent::SendShutdown { .. } = msg_events[0] {} else { panic!(); }
-       if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg_events[1] {} else { panic!(); }
-       if let MessageSendEvent::HandleError { .. } =  msg_events[2] {} else { panic!(); }
-
-       check_added_monitors!(nodes[1], 2);
-       check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() },
-               [nodes[0].node.get_our_node_id()], 100000);
-}
-
 #[test]
 fn double_temp_error() {
        // Test that it's OK to have multiple `ChainMonitor::update_channel` calls fail in a row.
index 0cadbd41a29d21ccb99de1e0d7bf84319e33971c..e553e8e534055ae9cf3a8d64e0acc83aa04e30a6 100644 (file)
@@ -923,12 +923,14 @@ where
 /// called [`funding_transaction_generated`] for outbound channels) being closed.
 ///
 /// Note that you can be a bit lazier about writing out `ChannelManager` than you can be with
-/// [`ChannelMonitor`]. With [`ChannelMonitor`] you MUST write each monitor update out to disk before
-/// returning from [`chain::Watch::watch_channel`]/[`update_channel`], with ChannelManagers, writing updates
-/// happens out-of-band (and will prevent any other `ChannelManager` operations from occurring during
-/// the serialization process). If the deserialized version is out-of-date compared to the
-/// [`ChannelMonitor`] passed by reference to [`read`], those channels will be force-closed based on the
-/// `ChannelMonitor` state and no funds will be lost (mod on-chain transaction fees).
+/// [`ChannelMonitor`]. With [`ChannelMonitor`] you MUST durably write each
+/// [`ChannelMonitorUpdate`] before returning from
+/// [`chain::Watch::watch_channel`]/[`update_channel`] or before completing async writes. With
+/// `ChannelManager`s, writing updates happens out-of-band (and will prevent any other
+/// `ChannelManager` operations from occurring during the serialization process). If the
+/// deserialized version is out-of-date compared to the [`ChannelMonitor`] passed by reference to
+/// [`read`], those channels will be force-closed based on the `ChannelMonitor` state and no funds
+/// will be lost (modulo on-chain transaction fees).
 ///
 /// Note that the deserializer is only implemented for `(`[`BlockHash`]`, `[`ChannelManager`]`)`, which
 /// tells you the last block hash which was connected. You should get the best block tip before using the manager.
@@ -2040,56 +2042,30 @@ macro_rules! handle_monitor_update_completion {
 }
 
 macro_rules! handle_new_monitor_update {
-       ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, _internal, $remove: expr, $completed: expr) => { {
-               // update_maps_on_chan_removal needs to be able to take id_to_peer, so make sure we can in
-               // any case so that it won't deadlock.
-               debug_assert_ne!($self.id_to_peer.held_by_thread(), LockHeldState::HeldByThread);
+       ($self: ident, $update_res: expr, $chan: expr, _internal, $completed: expr) => { {
                debug_assert!($self.background_events_processed_since_startup.load(Ordering::Acquire));
                match $update_res {
+                       ChannelMonitorUpdateStatus::UnrecoverableError => {
+                               let err_str = "ChannelMonitor[Update] persistence failed unrecoverably. This indicates we cannot continue normal operation and must shut down.";
+                               log_error!($self.logger, "{}", err_str);
+                               panic!("{}", err_str);
+                       },
                        ChannelMonitorUpdateStatus::InProgress => {
                                log_debug!($self.logger, "ChannelMonitor update for {} in flight, holding messages until the update completes.",
                                        &$chan.context.channel_id());
-                               Ok(false)
-                       },
-                       ChannelMonitorUpdateStatus::PermanentFailure => {
-                               log_error!($self.logger, "Closing channel {} due to monitor update ChannelMonitorUpdateStatus::PermanentFailure",
-                                       &$chan.context.channel_id());
-                               update_maps_on_chan_removal!($self, &$chan.context);
-                               let res = Err(MsgHandleErrInternal::from_finish_shutdown(
-                                       "ChannelMonitor storage failure".to_owned(), $chan.context.channel_id(),
-                                       $chan.context.get_user_id(), $chan.context.force_shutdown(false),
-                                       $self.get_channel_update_for_broadcast(&$chan).ok(), $chan.context.get_value_satoshis()));
-                               $remove;
-                               res
+                               false
                        },
                        ChannelMonitorUpdateStatus::Completed => {
                                $completed;
-                               Ok(true)
+                               true
                        },
                }
        } };
-       ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, MANUALLY_REMOVING_INITIAL_MONITOR, $remove: expr) => {
-               handle_new_monitor_update!($self, $update_res, $peer_state_lock, $peer_state,
-                       $per_peer_state_lock, $chan, _internal, $remove,
+       ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, INITIAL_MONITOR) => {
+               handle_new_monitor_update!($self, $update_res, $chan, _internal,
                        handle_monitor_update_completion!($self, $peer_state_lock, $peer_state, $per_peer_state_lock, $chan))
        };
-       ($self: ident, $update_res: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan_entry: expr, INITIAL_MONITOR) => {
-               if let ChannelPhase::Funded(chan) = $chan_entry.get_mut() {
-                       handle_new_monitor_update!($self, $update_res, $peer_state_lock, $peer_state,
-                               $per_peer_state_lock, chan, MANUALLY_REMOVING_INITIAL_MONITOR, { $chan_entry.remove() })
-               } else {
-                       // We're not supposed to handle monitor updates for unfunded channels (they have no monitors to
-                       // update).
-                       debug_assert!(false);
-                       let channel_id = *$chan_entry.key();
-                       let (_, err) = convert_chan_phase_err!($self, ChannelError::Close(
-                               "Cannot update monitor for unfunded channels as they don't have monitors yet".into()),
-                               $chan_entry.get_mut(), &channel_id);
-                       $chan_entry.remove();
-                       Err(err)
-               }
-       };
-       ($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr, MANUALLY_REMOVING, $remove: expr) => { {
+       ($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan: expr) => { {
                let in_flight_updates = $peer_state.in_flight_monitor_updates.entry($funding_txo)
                        .or_insert_with(Vec::new);
                // During startup, we push monitor updates as background events through to here in
@@ -2101,8 +2077,7 @@ macro_rules! handle_new_monitor_update {
                                in_flight_updates.len() - 1
                        });
                let update_res = $self.chain_monitor.update_channel($funding_txo, &in_flight_updates[idx]);
-               handle_new_monitor_update!($self, update_res, $peer_state_lock, $peer_state,
-                       $per_peer_state_lock, $chan, _internal, $remove,
+               handle_new_monitor_update!($self, update_res, $chan, _internal,
                        {
                                let _ = in_flight_updates.remove(idx);
                                if in_flight_updates.is_empty() && $chan.blocked_monitor_updates_pending() == 0 {
@@ -2110,22 +2085,6 @@ macro_rules! handle_new_monitor_update {
                                }
                        })
        } };
-       ($self: ident, $funding_txo: expr, $update: expr, $peer_state_lock: expr, $peer_state: expr, $per_peer_state_lock: expr, $chan_entry: expr) => {
-               if let ChannelPhase::Funded(chan) = $chan_entry.get_mut() {
-                       handle_new_monitor_update!($self, $funding_txo, $update, $peer_state_lock, $peer_state,
-                               $per_peer_state_lock, chan, MANUALLY_REMOVING, { $chan_entry.remove() })
-               } else {
-                       // We're not supposed to handle monitor updates for unfunded channels (they have no monitors to
-                       // update).
-                       debug_assert!(false);
-                       let channel_id = *$chan_entry.key();
-                       let (_, err) = convert_chan_phase_err!($self, ChannelError::Close(
-                               "Cannot update monitor for unfunded channels as they don't have monitors yet".into()),
-                               $chan_entry.get_mut(), &channel_id);
-                       $chan_entry.remove();
-                       Err(err)
-               }
-       }
 }
 
 macro_rules! process_events_body {
@@ -2538,61 +2497,64 @@ where
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 
                let mut failed_htlcs: Vec<(HTLCSource, PaymentHash)>;
-               let result: Result<(), _> = loop {
-                       {
-                               let per_peer_state = self.per_peer_state.read().unwrap();
+               loop {
+                       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) })?;
+                       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) })?;
 
-                               let mut peer_state_lock = peer_state_mutex.lock().unwrap();
-                               let peer_state = &mut *peer_state_lock;
+                       let mut peer_state_lock = peer_state_mutex.lock().unwrap();
+                       let peer_state = &mut *peer_state_lock;
 
-                               match peer_state.channel_by_id.entry(channel_id.clone()) {
-                                       hash_map::Entry::Occupied(mut chan_phase_entry) => {
-                                               if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() {
-                                                       let funding_txo_opt = chan.context.get_funding_txo();
-                                                       let their_features = &peer_state.latest_features;
-                                                       let (shutdown_msg, mut monitor_update_opt, htlcs) =
-                                                               chan.get_shutdown(&self.signer_provider, their_features, target_feerate_sats_per_1000_weight, override_shutdown_script)?;
-                                                       failed_htlcs = htlcs;
+                       match peer_state.channel_by_id.entry(channel_id.clone()) {
+                               hash_map::Entry::Occupied(mut chan_phase_entry) => {
+                                       if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() {
+                                               let funding_txo_opt = chan.context.get_funding_txo();
+                                               let their_features = &peer_state.latest_features;
+                                               let (shutdown_msg, mut monitor_update_opt, htlcs) =
+                                                       chan.get_shutdown(&self.signer_provider, their_features, target_feerate_sats_per_1000_weight, override_shutdown_script)?;
+                                               failed_htlcs = htlcs;
+
+                                               // We can send the `shutdown` message before updating the `ChannelMonitor`
+                                               // here as we don't need the monitor update to complete until we send a
+                                               // `shutdown_signed`, which we'll delay if we're pending a monitor update.
+                                               peer_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
+                                                       node_id: *counterparty_node_id,
+                                                       msg: shutdown_msg,
+                                               });
 
-                                                       // We can send the `shutdown` message before updating the `ChannelMonitor`
-                                                       // here as we don't need the monitor update to complete until we send a
-                                                       // `shutdown_signed`, which we'll delay if we're pending a monitor update.
-                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
-                                                               node_id: *counterparty_node_id,
-                                                               msg: shutdown_msg,
-                                                       });
+                                               debug_assert!(monitor_update_opt.is_none() || !chan.is_shutdown(),
+                                                       "We can't both complete shutdown and generate a monitor update");
 
-                                                       // Update the monitor with the shutdown script if necessary.
-                                                       if let Some(monitor_update) = monitor_update_opt.take() {
-                                                               break handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
-                                                                       peer_state_lock, peer_state, per_peer_state, chan_phase_entry).map(|_| ());
-                                                       }
+                                               // Update the monitor with the shutdown script if necessary.
+                                               if let Some(monitor_update) = monitor_update_opt.take() {
+                                                       handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
+                                                               peer_state_lock, peer_state, per_peer_state, chan);
+                                                       break;
+                                               }
 
-                                                       if chan.is_shutdown() {
-                                                               if let ChannelPhase::Funded(chan) = remove_channel_phase!(self, chan_phase_entry) {
-                                                                       if let Ok(channel_update) = self.get_channel_update_for_broadcast(&chan) {
-                                                                               peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
-                                                                                       msg: channel_update
-                                                                               });
-                                                                       }
-                                                                       self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
+                                               if chan.is_shutdown() {
+                                                       if let ChannelPhase::Funded(chan) = remove_channel_phase!(self, chan_phase_entry) {
+                                                               if let Ok(channel_update) = self.get_channel_update_for_broadcast(&chan) {
+                                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
+                                                                               msg: channel_update
+                                                                       });
                                                                }
+                                                               self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
                                                        }
-                                                       break Ok(());
                                                }
-                                       },
-                                       hash_map::Entry::Vacant(_) => (),
-                               }
+                                               break;
+                                       }
+                               },
+                               hash_map::Entry::Vacant(_) => {
+                                       // If we reach this point, it means that the channel_id either refers to an unfunded channel or
+                                       // it does not exist for this peer. Either way, we can attempt to force-close it.
+                                       //
+                                       // An appropriate error will be returned for non-existence of the channel if that's the case.
+                                       return self.force_close_channel_with_peer(&channel_id, counterparty_node_id, None, false).map(|_| ())
+                               },
                        }
-                       // If we reach this point, it means that the channel_id either refers to an unfunded channel or
-                       // it does not exist for this peer. Either way, we can attempt to force-close it.
-                       //
-                       // An appropriate error will be returned for non-existence of the channel if that's the case.
-                       return self.force_close_channel_with_peer(&channel_id, counterparty_node_id, None, false).map(|_| ())
-               };
+               }
 
                for htlc_source in failed_htlcs.drain(..) {
                        let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
@@ -2600,7 +2562,6 @@ where
                        self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
                }
 
-               let _ = handle_error!(self, result, *counterparty_node_id);
                Ok(())
        }
 
@@ -3342,9 +3303,8 @@ where
                                                        }, onion_packet, None, &self.fee_estimator, &self.logger);
                                                match break_chan_phase_entry!(self, send_res, chan_phase_entry) {
                                                        Some(monitor_update) => {
-                                                               match handle_new_monitor_update!(self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, chan_phase_entry) {
-                                                                       Err(e) => break Err(e),
-                                                                       Ok(false) => {
+                                                               match handle_new_monitor_update!(self, funding_txo, monitor_update, peer_state_lock, peer_state, per_peer_state, chan) {
+                                                                       false => {
                                                                                // Note that MonitorUpdateInProgress here indicates (per function
                                                                                // docs) that we will resend the commitment update once monitor
                                                                                // updating completes. Therefore, we must return an error
@@ -3353,7 +3313,7 @@ where
                                                                                // MonitorUpdateInProgress, below.
                                                                                return Err(APIError::MonitorUpdateInProgress);
                                                                        },
-                                                                       Ok(true) => {},
+                                                                       true => {},
                                                                }
                                                        },
                                                        None => {},
@@ -4535,33 +4495,29 @@ where
                                },
                                BackgroundEvent::MonitorUpdateRegeneratedOnStartup { counterparty_node_id, funding_txo, update } => {
                                        let mut updated_chan = false;
-                                       let res = {
+                                       {
                                                let per_peer_state = self.per_peer_state.read().unwrap();
                                                if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
                                                        let mut peer_state_lock = peer_state_mutex.lock().unwrap();
                                                        let peer_state = &mut *peer_state_lock;
                                                        match peer_state.channel_by_id.entry(funding_txo.to_channel_id()) {
                                                                hash_map::Entry::Occupied(mut chan_phase) => {
-                                                                       updated_chan = true;
-                                                                       handle_new_monitor_update!(self, funding_txo, update.clone(),
-                                                                               peer_state_lock, peer_state, per_peer_state, chan_phase).map(|_| ())
+                                                                       if let ChannelPhase::Funded(chan) = chan_phase.get_mut() {
+                                                                               updated_chan = true;
+                                                                               handle_new_monitor_update!(self, funding_txo, update.clone(),
+                                                                                       peer_state_lock, peer_state, per_peer_state, chan);
+                                                                       } else {
+                                                                               debug_assert!(false, "We shouldn't have an update for a non-funded channel");
+                                                                       }
                                                                },
-                                                               hash_map::Entry::Vacant(_) => Ok(()),
+                                                               hash_map::Entry::Vacant(_) => {},
                                                        }
-                                               } else { Ok(()) }
-                                       };
+                                               }
+                                       }
                                        if !updated_chan {
                                                // TODO: Track this as in-flight even though the channel is closed.
                                                let _ = self.chain_monitor.update_channel(funding_txo, &update);
                                        }
-                                       // TODO: If this channel has since closed, we're likely providing a payment
-                                       // preimage update, which we must ensure is durable! We currently don't,
-                                       // however, ensure that.
-                                       if res.is_err() {
-                                               log_error!(self.logger,
-                                                       "Failed to provide ChannelMonitorUpdate to closed channel! This likely lost us a payment preimage!");
-                                       }
-                                       let _ = handle_error!(self, res, counterparty_node_id);
                                },
                                BackgroundEvent::MonitorUpdatesComplete { counterparty_node_id, channel_id } => {
                                        let per_peer_state = self.per_peer_state.read().unwrap();
@@ -5284,15 +5240,8 @@ where
                                                                peer_state.monitor_update_blocked_actions.entry(chan_id).or_insert(Vec::new()).push(action);
                                                        }
                                                        if !during_init {
-                                                               let res = handle_new_monitor_update!(self, prev_hop.outpoint, monitor_update, peer_state_lock,
-                                                                       peer_state, per_peer_state, chan_phase_entry);
-                                                               if let Err(e) = res {
-                                                                       // TODO: This is a *critical* error - we probably updated the outbound edge
-                                                                       // of the HTLC's monitor with a preimage. We should retry this monitor
-                                                                       // update over and over again until morale improves.
-                                                                       log_error!(self.logger, "Failed to update channel monitor with preimage {:?}", payment_preimage);
-                                                                       return Err((counterparty_node_id, e));
-                                                               }
+                                                               handle_new_monitor_update!(self, prev_hop.outpoint, monitor_update, peer_state_lock,
+                                                                       peer_state, per_peer_state, chan);
                                                        } else {
                                                                // If we're running during init we cannot update a monitor directly -
                                                                // they probably haven't actually been loaded yet. Instead, push the
@@ -5919,47 +5868,42 @@ where
                                Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id".to_owned(), funding_msg.channel_id))
                        },
                        hash_map::Entry::Vacant(e) => {
-                               match self.id_to_peer.lock().unwrap().entry(chan.context.channel_id()) {
+                               let mut id_to_peer_lock = self.id_to_peer.lock().unwrap();
+                               match id_to_peer_lock.entry(chan.context.channel_id()) {
                                        hash_map::Entry::Occupied(_) => {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close(
                                                        "The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
                                                        funding_msg.channel_id))
                                        },
                                        hash_map::Entry::Vacant(i_e) => {
-                                               i_e.insert(chan.context.get_counterparty_node_id());
-                                       }
-                               }
-
-                               // There's no problem signing a counterparty's funding transaction if our monitor
-                               // hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
-                               // accepted payment from yet. We do, however, need to wait to send our channel_ready
-                               // until we have persisted our monitor.
-                               let new_channel_id = funding_msg.channel_id;
-                               peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
-                                       node_id: counterparty_node_id.clone(),
-                                       msg: funding_msg,
-                               });
+                                               let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
+                                               if let Ok(persist_state) = monitor_res {
+                                                       i_e.insert(chan.context.get_counterparty_node_id());
+                                                       mem::drop(id_to_peer_lock);
+
+                                                       // There's no problem signing a counterparty's funding transaction if our monitor
+                                                       // hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
+                                                       // accepted payment from yet. We do, however, need to wait to send our channel_ready
+                                                       // until we have persisted our monitor.
+                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
+                                                               node_id: counterparty_node_id.clone(),
+                                                               msg: funding_msg,
+                                                       });
 
-                               let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
-
-                               if let ChannelPhase::Funded(chan) = e.insert(ChannelPhase::Funded(chan)) {
-                                       let mut res = handle_new_monitor_update!(self, monitor_res, peer_state_lock, peer_state,
-                                               per_peer_state, chan, MANUALLY_REMOVING_INITIAL_MONITOR,
-                                               { peer_state.channel_by_id.remove(&new_channel_id) });
-
-                                       // Note that we reply with the new channel_id in error messages if we gave up on the
-                                       // channel, not the temporary_channel_id. This is compatible with ourselves, but the
-                                       // spec is somewhat ambiguous here. Not a huge deal since we'll send error messages for
-                                       // any messages referencing a previously-closed channel anyway.
-                                       // We do not propagate the monitor update to the user as it would be for a monitor
-                                       // that we didn't manage to store (and that we don't care about - we don't respond
-                                       // with the funding_signed so the channel can never go on chain).
-                                       if let Err(MsgHandleErrInternal { shutdown_finish: Some((res, _)), .. }) = &mut res {
-                                               res.0 = None;
+                                                       if let ChannelPhase::Funded(chan) = e.insert(ChannelPhase::Funded(chan)) {
+                                                               handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
+                                                                       per_peer_state, chan, INITIAL_MONITOR);
+                                                       } else {
+                                                               unreachable!("This must be a funded channel as we just inserted it.");
+                                                       }
+                                                       Ok(())
+                                               } else {
+                                                       log_error!(self.logger, "Persisting initial ChannelMonitor failed, implying the funding outpoint was duplicated");
+                                                       return Err(MsgHandleErrInternal::send_err_msg_no_close(
+                                                               "The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
+                                                               funding_msg.channel_id));
+                                               }
                                        }
-                                       res.map(|_| ())
-                               } else {
-                                       unreachable!("This must be a funded channel as we just inserted it.");
                                }
                        }
                }
@@ -5982,17 +5926,12 @@ where
                                        ChannelPhase::Funded(ref mut chan) => {
                                                let monitor = try_chan_phase_entry!(self,
                                                        chan.funding_signed(&msg, best_block, &self.signer_provider, &self.logger), chan_phase_entry);
-                                               let update_res = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor);
-                                               let mut res = handle_new_monitor_update!(self, update_res, peer_state_lock, peer_state, per_peer_state, chan_phase_entry, INITIAL_MONITOR);
-                                               if let Err(MsgHandleErrInternal { ref mut shutdown_finish, .. }) = res {
-                                                       // 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.
-                                                       if let Some((ref mut shutdown_finish, _)) = shutdown_finish {
-                                                               shutdown_finish.0.take();
-                                                       }
+                                               if let Ok(persist_status) = self.chain_monitor.watch_channel(chan.context.get_funding_txo().unwrap(), monitor) {
+                                                       handle_new_monitor_update!(self, persist_status, peer_state_lock, peer_state, per_peer_state, chan, INITIAL_MONITOR);
+                                                       Ok(())
+                                               } else {
+                                                       try_chan_phase_entry!(self, Err(ChannelError::Close("Channel funding outpoint was a duplicate".to_owned())), chan_phase_entry)
                                                }
-                                               res.map(|_| ())
                                        },
                                        _ => {
                                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id));
@@ -6059,7 +5998,7 @@ where
 
        fn internal_shutdown(&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown) -> Result<(), MsgHandleErrInternal> {
                let mut dropped_htlcs: Vec<(HTLCSource, PaymentHash)>;
-               let result: Result<(), _> = loop {
+               {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let peer_state_mutex = per_peer_state.get(counterparty_node_id)
                                .ok_or_else(|| {
@@ -6094,10 +6033,9 @@ where
                                                }
                                                // Update the monitor with the shutdown script if necessary.
                                                if let Some(monitor_update) = monitor_update_opt {
-                                                       break handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
-                                                               peer_state_lock, peer_state, per_peer_state, chan_phase_entry).map(|_| ());
+                                                       handle_new_monitor_update!(self, funding_txo_opt.unwrap(), monitor_update,
+                                                               peer_state_lock, peer_state, per_peer_state, chan);
                                                }
-                                               break Ok(());
                                        },
                                        ChannelPhase::UnfundedInboundV1(_) | ChannelPhase::UnfundedOutboundV1(_) => {
                                                let context = phase.context_mut();
@@ -6111,14 +6049,14 @@ where
                        } else {
                                return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id))
                        }
-               };
+               }
                for htlc_source in dropped_htlcs.drain(..) {
                        let receiver = HTLCDestination::NextHopChannel { node_id: Some(counterparty_node_id.clone()), channel_id: msg.channel_id };
                        let reason = HTLCFailReason::from_failure_code(0x4000 | 8);
                        self.fail_htlc_backwards_internal(&htlc_source.0, &htlc_source.1, &reason, receiver);
                }
 
-               result
+               Ok(())
        }
 
        fn internal_closing_signed(&self, counterparty_node_id: &PublicKey, msg: &msgs::ClosingSigned) -> Result<(), MsgHandleErrInternal> {
@@ -6348,8 +6286,9 @@ where
                                        let monitor_update_opt = try_chan_phase_entry!(self, chan.commitment_signed(&msg, &self.logger), chan_phase_entry);
                                        if let Some(monitor_update) = monitor_update_opt {
                                                handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
-                                                       peer_state, per_peer_state, chan_phase_entry).map(|_| ())
-                                       } else { Ok(()) }
+                                                       peer_state, per_peer_state, chan);
+                                       }
+                                       Ok(())
                                } else {
                                        return try_chan_phase_entry!(self, Err(ChannelError::Close(
                                                "Got a commitment_signed message for an unfunded channel!".into())), chan_phase_entry);
@@ -6498,7 +6437,7 @@ where
        }
 
        fn internal_revoke_and_ack(&self, counterparty_node_id: &PublicKey, msg: &msgs::RevokeAndACK) -> Result<(), MsgHandleErrInternal> {
-               let (htlcs_to_fail, res) = {
+               let htlcs_to_fail = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let mut peer_state_lock = per_peer_state.get(counterparty_node_id)
                                .ok_or_else(|| {
@@ -6517,13 +6456,13 @@ where
                                                } else { false };
                                                let (htlcs_to_fail, monitor_update_opt) = try_chan_phase_entry!(self,
                                                        chan.revoke_and_ack(&msg, &self.fee_estimator, &self.logger, mon_update_blocked), chan_phase_entry);
-                                               let res = if let Some(monitor_update) = monitor_update_opt {
+                                               if let Some(monitor_update) = monitor_update_opt {
                                                        let funding_txo = funding_txo_opt
                                                                .expect("Funding outpoint must have been set for RAA handling to succeed");
                                                        handle_new_monitor_update!(self, funding_txo, monitor_update,
-                                                               peer_state_lock, peer_state, per_peer_state, chan_phase_entry).map(|_| ())
-                                               } else { Ok(()) };
-                                               (htlcs_to_fail, res)
+                                                               peer_state_lock, peer_state, per_peer_state, chan);
+                                               }
+                                               htlcs_to_fail
                                        } else {
                                                return try_chan_phase_entry!(self, Err(ChannelError::Close(
                                                        "Got a revoke_and_ack message for an unfunded channel!".into())), chan_phase_entry);
@@ -6533,7 +6472,7 @@ where
                        }
                };
                self.fail_holding_cell_htlcs(htlcs_to_fail, msg.channel_id, counterparty_node_id);
-               res
+               Ok(())
        }
 
        fn internal_update_fee(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFee) -> Result<(), MsgHandleErrInternal> {
@@ -6729,8 +6668,7 @@ where
                                                        self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
                                                }
                                        },
-                                       MonitorEvent::CommitmentTxConfirmed(funding_outpoint) |
-                                       MonitorEvent::UpdateFailed(funding_outpoint) => {
+                                       MonitorEvent::HolderForceClosed(funding_outpoint) => {
                                                let counterparty_node_id_opt = match counterparty_node_id {
                                                        Some(cp_id) => Some(cp_id),
                                                        None => {
@@ -6754,12 +6692,7 @@ where
                                                                                                msg: update
                                                                                        });
                                                                                }
-                                                                               let reason = if let MonitorEvent::UpdateFailed(_) = monitor_event {
-                                                                                       ClosureReason::ProcessingError { err: "Failed to persist ChannelMonitor update during chain sync".to_string() }
-                                                                               } else {
-                                                                                       ClosureReason::CommitmentTxConfirmed
-                                                                               };
-                                                                               self.issue_channel_close_events(&chan.context, reason);
+                                                                               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::SendErrorMessage {
@@ -6800,7 +6733,6 @@ where
        fn check_free_holding_cells(&self) -> bool {
                let mut has_monitor_update = false;
                let mut failed_htlcs = Vec::new();
-               let mut handle_errors = Vec::new();
 
                // Walk our list of channels and find any that need to update. Note that when we do find an
                // update, if it includes actions that must be taken afterwards, we have to drop the
@@ -6825,13 +6757,8 @@ where
                                                if let Some(monitor_update) = monitor_opt {
                                                        has_monitor_update = true;
 
-                                                       let channel_id: ChannelId = *channel_id;
-                                                       let res = handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
-                                                               peer_state_lock, peer_state, per_peer_state, chan, MANUALLY_REMOVING,
-                                                               peer_state.channel_by_id.remove(&channel_id));
-                                                       if res.is_err() {
-                                                               handle_errors.push((counterparty_node_id, res));
-                                                       }
+                                                       handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update,
+                                                               peer_state_lock, peer_state, per_peer_state, chan);
                                                        continue 'peer_loop;
                                                }
                                        }
@@ -6841,15 +6768,11 @@ where
                        break 'peer_loop;
                }
 
-               let has_update = has_monitor_update || !failed_htlcs.is_empty() || !handle_errors.is_empty();
+               let has_update = has_monitor_update || !failed_htlcs.is_empty();
                for (failures, channel_id, counterparty_node_id) in failed_htlcs.drain(..) {
                        self.fail_holding_cell_htlcs(failures, channel_id, &counterparty_node_id);
                }
 
-               for (counterparty_node_id, err) in handle_errors.drain(..) {
-                       let _ = handle_error!(self, err, counterparty_node_id);
-               }
-
                has_update
        }
 
@@ -7144,7 +7067,6 @@ where
        /// operation. It will double-check that nothing *else* is also blocking the same channel from
        /// making progress and then let any blocked [`ChannelMonitorUpdate`]s fly.
        fn handle_monitor_update_release(&self, counterparty_node_id: PublicKey, channel_funding_outpoint: OutPoint, mut completed_blocker: Option<RAAMonitorUpdateBlockingAction>) {
-               let mut errors = Vec::new();
                loop {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        if let Some(peer_state_mtx) = per_peer_state.get(&counterparty_node_id) {
@@ -7176,11 +7098,8 @@ where
                                                if let Some((monitor_update, further_update_exists)) = chan.unblock_next_blocked_monitor_update() {
                                                        log_debug!(self.logger, "Unlocking monitor updating for channel {} and updating monitor",
                                                                channel_funding_outpoint.to_channel_id());
-                                                       if let Err(e) = handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,
-                                                               peer_state_lck, peer_state, per_peer_state, chan_phase_entry)
-                                                       {
-                                                               errors.push((e, counterparty_node_id));
-                                                       }
+                                                       handle_new_monitor_update!(self, channel_funding_outpoint, monitor_update,
+                                                               peer_state_lck, peer_state, per_peer_state, chan);
                                                        if further_update_exists {
                                                                // If there are more `ChannelMonitorUpdate`s to process, restart at the
                                                                // top of the loop.
@@ -7199,10 +7118,6 @@ where
                        }
                        break;
                }
-               for (err, counterparty_node_id) in errors {
-                       let res = Err::<(), _>(err);
-                       let _ = handle_error!(self, res, counterparty_node_id);
-               }
        }
 
        fn handle_post_event_actions(&self, actions: Vec<EventCompletionAction>) {
@@ -10190,7 +10105,7 @@ mod tests {
                        TEST_FINAL_CLTV, false), 100_000);
                let route = find_route(
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
-                       None, nodes[0].logger, &scorer, &(), &random_seed_bytes
+                       None, nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
                ).unwrap();
                nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
                        RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
@@ -10224,7 +10139,7 @@ mod tests {
                let payment_preimage = PaymentPreimage([42; 32]);
                let route = find_route(
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
-                       None, nodes[0].logger, &scorer, &(), &random_seed_bytes
+                       None, nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
                ).unwrap();
                let payment_hash = nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
                        RecipientOnionFields::spontaneous_empty(), PaymentId(payment_preimage.0)).unwrap();
@@ -10281,7 +10196,7 @@ mod tests {
                );
                let route = find_route(
                        &nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph,
-                       None, nodes[0].logger, &scorer, &(), &random_seed_bytes
+                       None, nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
                ).unwrap();
                let payment_id_2 = PaymentId([45; 32]);
                nodes[0].node.send_spontaneous_payment(&route, Some(payment_preimage),
@@ -10332,7 +10247,7 @@ mod tests {
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-                       nodes[0].logger, &scorer, &(), &random_seed_bytes
+                       nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
                ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);
@@ -10377,7 +10292,7 @@ mod tests {
                let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
                let route = find_route(
                        &payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
-                       nodes[0].logger, &scorer, &(), &random_seed_bytes
+                       nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
                ).unwrap();
 
                let test_preimage = PaymentPreimage([42; 32]);
index f6684485dba0d69964686b145b9fd3b383c854cc..82fa06f5c80b8a5494b9b546d4e5ccb0d4287999 100644 (file)
@@ -422,6 +422,10 @@ pub struct Node<'chan_man, 'node_cfg: 'chan_man, 'chan_mon_cfg: 'node_cfg> {
                &'chan_mon_cfg test_utils::TestLogger,
        >,
 }
+#[cfg(feature = "std")]
+impl<'a, 'b, 'c> std::panic::UnwindSafe for Node<'a, 'b, 'c> {}
+#[cfg(feature = "std")]
+impl<'a, 'b, 'c> std::panic::RefUnwindSafe for Node<'a, 'b, 'c> {}
 impl<'a, 'b, 'c> Node<'a, 'b, 'c> {
        pub fn best_block_hash(&self) -> BlockHash {
                self.blocks.lock().unwrap().last().unwrap().0.block_hash()
@@ -578,7 +582,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                        let chain_source = test_utils::TestChainSource::new(Network::Testnet);
                        let chain_monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &broadcaster, &self.logger, &feeest, &persister, &self.keys_manager);
                        for deserialized_monitor in deserialized_monitors.drain(..) {
-                               if chain_monitor.watch_channel(deserialized_monitor.get_funding_txo().0, deserialized_monitor) != ChannelMonitorUpdateStatus::Completed {
+                               if chain_monitor.watch_channel(deserialized_monitor.get_funding_txo().0, deserialized_monitor) != Ok(ChannelMonitorUpdateStatus::Completed) {
                                        panic!();
                                }
                        }
@@ -977,7 +981,7 @@ pub fn _reload_node<'a, 'b, 'c>(node: &'a Node<'a, 'b, 'c>, default_config: User
 
        for monitor in monitors_read.drain(..) {
                assert_eq!(node.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor),
-                       ChannelMonitorUpdateStatus::Completed);
+                       Ok(ChannelMonitorUpdateStatus::Completed));
                check_added_monitors!(node, 1);
        }
 
@@ -1854,7 +1858,7 @@ pub fn get_route(send_node: &Node, route_params: &RouteParameters) -> Result<Rou
        router::get_route(
                &send_node.node.get_our_node_id(), route_params, &send_node.network_graph.read_only(),
                Some(&send_node.node.list_usable_channels().iter().collect::<Vec<_>>()),
-               send_node.logger, &scorer, &(), &random_seed_bytes
+               send_node.logger, &scorer, &Default::default(), &random_seed_bytes
        )
 }
 
@@ -2506,7 +2510,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
        let keys_manager = test_utils::TestKeysInterface::new(&seed, Network::Testnet);
        let random_seed_bytes = keys_manager.get_secure_random_bytes();
        let route = router::get_route(&origin_node.node.get_our_node_id(), &route_params, &network_graph,
-               None, origin_node.logger, &scorer, &(), &random_seed_bytes).unwrap();
+               None, origin_node.logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        assert_eq!(route.paths.len(), 1);
        assert_eq!(route.paths[0].hops.len(), expected_route.len());
        for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
index b252a352c48df981c2a2d102ba188c9943942143..1066362a4c4c101791a0c12a051e36c7986c2741 100644 (file)
@@ -2408,6 +2408,7 @@ fn channel_monitor_network_test() {
                }
                check_added_monitors!(nodes[4], 1);
                test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
+               check_closed_event!(nodes[4], 1, ClosureReason::HolderForceClosed, [nodes[3].node.get_our_node_id()], 100000);
 
                mine_transaction(&nodes[4], &node_txn[0]);
                check_preimage_claim(&nodes[4], &node_txn);
@@ -2419,9 +2420,8 @@ fn channel_monitor_network_test() {
        assert_eq!(nodes[4].node.list_channels().len(), 0);
 
        assert_eq!(nodes[3].chain_monitor.chain_monitor.watch_channel(OutPoint { txid: chan_3.3.txid(), index: 0 }, chan_3_mon),
-               ChannelMonitorUpdateStatus::Completed);
-       check_closed_event!(nodes[3], 1, ClosureReason::CommitmentTxConfirmed, [nodes[4].node.get_our_node_id()], 100000);
-       check_closed_event!(nodes[4], 1, ClosureReason::CommitmentTxConfirmed, [nodes[3].node.get_our_node_id()], 100000);
+               Ok(ChannelMonitorUpdateStatus::Completed));
+       check_closed_event!(nodes[3], 1, ClosureReason::HolderForceClosed, [nodes[4].node.get_our_node_id()], 100000);
 }
 
 #[test]
@@ -5660,7 +5660,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
        test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS });
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
-       check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
 }
 
 fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
@@ -5691,7 +5691,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
        test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
-       check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 100000);
+       check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
 }
 
 fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no_close: bool) {
@@ -5737,7 +5737,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
                test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
                check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
-               check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 100000);
+               check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
        } else {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
        }
@@ -7231,7 +7231,7 @@ fn test_check_htlc_underpaying() {
                TEST_FINAL_CLTV).with_bolt11_features(nodes[1].node.invoice_features()).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, 10_000);
        let route = get_route(&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(),
-               None, nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
+               None, nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        let (_, our_payment_hash, _) = get_payment_preimage_hash!(nodes[0]);
        let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash(our_payment_hash, Some(100_000), 7200, None).unwrap();
        nodes[0].node.send_payment_with_route(&route, our_payment_hash,
@@ -7489,12 +7489,12 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, 3_000_000);
        let route = get_route(&nodes[0].node.get_our_node_id(), &route_params, &nodes[0].network_graph.read_only(), None,
-               nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
+               nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        let payment_preimage = send_along_route(&nodes[0], route, &[&nodes[1]], 3_000_000).0;
        let payment_params = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(), 50).with_bolt11_features(nodes[0].node.invoice_features()).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, 3_000_000);
        let route = get_route(&nodes[1].node.get_our_node_id(), &route_params, &nodes[1].network_graph.read_only(), None,
-               nodes[0].logger, &scorer, &(), &random_seed_bytes).unwrap();
+               nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        send_along_route(&nodes[1], route, &[&nodes[0]], 3_000_000);
 
        let revoked_local_txn = get_local_commitment_txn!(nodes[1], chan.2);
@@ -8453,7 +8453,7 @@ fn test_update_err_monitor_lockdown() {
                        new_monitor
                };
                let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &chanmon_cfgs[0].tx_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
-               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), ChannelMonitorUpdateStatus::Completed);
+               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), Ok(ChannelMonitorUpdateStatus::Completed));
                watchtower
        };
        let block = create_dummy_block(BlockHash::all_zeros(), 42, Vec::new());
@@ -8475,7 +8475,7 @@ fn test_update_err_monitor_lockdown() {
                let mut node_0_peer_state_lock;
                if let ChannelPhase::Funded(ref mut channel) = get_channel_ref!(nodes[0], nodes[1], node_0_per_peer_lock, node_0_peer_state_lock, chan_1.2) {
                        if let Ok(Some(update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
-                               assert_eq!(watchtower.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::PermanentFailure);
+                               assert_eq!(watchtower.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::InProgress);
                                assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                        } else { assert!(false); }
                } else {
@@ -8526,7 +8526,7 @@ fn test_concurrent_monitor_claim() {
                        new_monitor
                };
                let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &alice_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
-               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), ChannelMonitorUpdateStatus::Completed);
+               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), Ok(ChannelMonitorUpdateStatus::Completed));
                watchtower
        };
        let block = create_dummy_block(BlockHash::all_zeros(), 42, Vec::new());
@@ -8557,7 +8557,7 @@ fn test_concurrent_monitor_claim() {
                        new_monitor
                };
                let watchtower = test_utils::TestChainMonitor::new(Some(&chain_source), &bob_broadcaster, &logger, &chanmon_cfgs[0].fee_estimator, &persister, &node_cfgs[0].keys_manager);
-               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), ChannelMonitorUpdateStatus::Completed);
+               assert_eq!(watchtower.watch_channel(outpoint, new_monitor), Ok(ChannelMonitorUpdateStatus::Completed));
                watchtower
        };
        watchtower_bob.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, Vec::new()), HTLC_TIMEOUT_BROADCAST - 1);
@@ -8577,7 +8577,7 @@ fn test_concurrent_monitor_claim() {
                if let ChannelPhase::Funded(ref mut channel) = get_channel_ref!(nodes[0], nodes[1], node_0_per_peer_lock, node_0_peer_state_lock, chan_1.2) {
                        if let Ok(Some(update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
                                // Watchtower Alice should already have seen the block and reject the update
-                               assert_eq!(watchtower_alice.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::PermanentFailure);
+                               assert_eq!(watchtower_alice.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::InProgress);
                                assert_eq!(watchtower_bob.chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                                assert_eq!(nodes[0].chain_monitor.update_channel(outpoint, &update), ChannelMonitorUpdateStatus::Completed);
                        } else { assert!(false); }
@@ -8603,7 +8603,7 @@ fn test_concurrent_monitor_claim() {
        let height = HTLC_TIMEOUT_BROADCAST + 1;
        connect_blocks(&nodes[0], height - nodes[0].best_block_info().1);
        check_closed_broadcast(&nodes[0], 1, true);
-       check_closed_event!(&nodes[0], 1, ClosureReason::CommitmentTxConfirmed, false,
+       check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false,
                [nodes[1].node.get_our_node_id()], 100000);
        watchtower_alice.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, vec![bob_state_y.clone()]), height);
        check_added_monitors(&nodes[0], 1);
index ece24794f63f593ecba133660fa73a466fc547a7..cb78cda714f83d4edec2b64f44364fc88f55864d 100644 (file)
@@ -1046,14 +1046,14 @@ fn do_test_revoked_counterparty_commitment_balances(confirm_htlc_spend_first: bo
        assert!(failed_payments.is_empty());
        if let Event::PendingHTLCsForwardable { .. } = events[0] {} else { panic!(); }
        match &events[1] {
-               Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => {},
+               Event::ChannelClosed { reason: ClosureReason::HolderForceClosed, .. } => {},
                _ => panic!(),
        }
 
        connect_blocks(&nodes[1], htlc_cltv_timeout + 1 - 10);
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
-       check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 1000000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
 
        // Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only
        // lists the two on-chain timeout-able HTLCs as claimable balances.
index 3731c31c5b42c372663f29e31d30b6f6207ea811..52ae2908022c7894300b4a42824de38f3ab5e39e 100644 (file)
@@ -1052,7 +1052,7 @@ macro_rules! get_phantom_route {
                (get_route(
                        &$nodes[0].node.get_our_node_id(), &route_params, &network_graph,
                        Some(&$nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()),
-                       $nodes[0].logger, &scorer, &(), &[0u8; 32]
+                       $nodes[0].logger, &scorer, &Default::default(), &[0u8; 32]
                ).unwrap(), phantom_route_hint.phantom_scid)
        }
 }}
index 3def4e3629bf2fb98d227d9a95e5b1faf6493bc9..19f55f4f2d30ea64b62db1bca27263e8ff70f186 100644 (file)
@@ -267,7 +267,7 @@ fn do_test_keysend_payments(public_node: bool, with_retry: bool) {
        let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
        let route = find_route(
                &payer_pubkey, &route_params, &network_graph, first_hops,
-               nodes[0].logger, &scorer, &(), &random_seed_bytes
+               nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
        ).unwrap();
 
        {
@@ -320,7 +320,7 @@ fn test_mpp_keysend() {
        let scorer = test_utils::TestScorer::new();
        let random_seed_bytes = chanmon_cfgs[0].keys_manager.get_secure_random_bytes();
        let route = find_route(&payer_pubkey, &route_params, &network_graph, None, nodes[0].logger,
-               &scorer, &(), &random_seed_bytes).unwrap();
+               &scorer, &Default::default(), &random_seed_bytes).unwrap();
 
        let payment_preimage = PaymentPreimage([42; 32]);
        let payment_secret = PaymentSecret(payment_preimage.0);
@@ -1106,7 +1106,7 @@ fn get_ldk_payment_preimage() {
        let route = get_route( &nodes[0].node.get_our_node_id(), &route_params,
                &nodes[0].network_graph.read_only(),
                Some(&nodes[0].node.list_usable_channels().iter().collect::<Vec<_>>()), nodes[0].logger,
-               &scorer, &(), &random_seed_bytes).unwrap();
+               &scorer, &Default::default(), &random_seed_bytes).unwrap();
        nodes[0].node.send_payment_with_route(&route, payment_hash,
                RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)).unwrap();
        check_added_monitors!(nodes[0], 1);
@@ -1750,9 +1750,9 @@ fn do_test_intercepted_payment(test: InterceptTest) {
                ]).unwrap()
                .with_bolt11_features(nodes[2].node.invoice_features()).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat,);
-       let route = get_route( &nodes[0].node.get_our_node_id(), &route_params,
-               &nodes[0].network_graph.read_only(), None, nodes[0].logger, &scorer, &(),
-               &random_seed_bytes,).unwrap();
+       let route = get_route(&nodes[0].node.get_our_node_id(), &route_params,
+               &nodes[0].network_graph.read_only(), None, nodes[0].logger, &scorer, &Default::default(),
+               &random_seed_bytes).unwrap();
 
        let (payment_hash, payment_secret) = nodes[2].node.create_inbound_payment(Some(amt_msat), 60 * 60, None).unwrap();
        nodes[0].node.send_payment_with_route(&route, payment_hash,
@@ -2258,12 +2258,14 @@ fn auto_retry_partial_failure() {
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
+       // Open three channels, the first has plenty of liquidity, the second and third have ~no
+       // available liquidity, causing any outbound payments routed over it to fail immediately.
        let chan_1_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
-       let chan_2_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
-       let chan_3_id = create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
+       let chan_2_id = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 989_000_000).0.contents.short_channel_id;
+       let chan_3_id = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 989_000_000).0.contents.short_channel_id;
 
        // Marshall data to send the payment
-       let amt_msat = 20_000;
+       let amt_msat = 10_000_000;
        let (_, payment_hash, payment_preimage, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
        #[cfg(feature = "std")]
        let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
@@ -2278,16 +2280,6 @@ fn auto_retry_partial_failure() {
                .with_bolt11_features(invoice_features).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat);
 
-       // Ensure the first monitor update (for the initial send path1 over chan_1) succeeds, but the
-       // second (for the initial send path2 over chan_2) fails.
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-       // Ensure third monitor update (for the retry1's path1 over chan_1) succeeds, but the fourth (for
-       // the retry1's path2 over chan_3) fails, and monitor updates succeed after that.
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
-
        // Configure the initial send, retry1 and retry2's paths.
        let send_route = Route {
                paths: vec![
@@ -2364,32 +2356,23 @@ fn auto_retry_partial_failure() {
        // Send a payment that will partially fail on send, then partially fail on retry, then succeed.
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
                PaymentId(payment_hash.0), route_params, Retry::Attempts(3)).unwrap();
-       let closed_chan_events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(closed_chan_events.len(), 4);
-       match closed_chan_events[0] {
-               Event::ChannelClosed { .. } => {},
-               _ => panic!("Unexpected event"),
-       }
-       match closed_chan_events[1] {
+       let payment_failed_events = nodes[0].node.get_and_clear_pending_events();
+       assert_eq!(payment_failed_events.len(), 2);
+       match payment_failed_events[0] {
                Event::PaymentPathFailed { .. } => {},
                _ => panic!("Unexpected event"),
        }
-       match closed_chan_events[2] {
-               Event::ChannelClosed { .. } => {},
-               _ => panic!("Unexpected event"),
-       }
-       match closed_chan_events[3] {
+       match payment_failed_events[1] {
                Event::PaymentPathFailed { .. } => {},
                _ => panic!("Unexpected event"),
        }
 
        // Pass the first part of the payment along the path.
-       check_added_monitors!(nodes[0], 5); // three outbound channel updates succeeded, two permanently failed
+       check_added_monitors!(nodes[0], 1); // only one HTLC actually made it out
        let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
 
-       // First message is the first update_add, remaining messages are broadcasting channel updates and
-       // errors for the permfailed channels
-       assert_eq!(msg_events.len(), 5);
+       // Only one HTLC/channel update actually made it out
+       assert_eq!(msg_events.len(), 1);
        let mut payment_event = SendEvent::from_event(msg_events.remove(0));
 
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
@@ -2478,12 +2461,13 @@ fn auto_retry_zero_attempts_send_error() {
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
        let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
-       create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
-       create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
+       // Open a single channel that does not have sufficient liquidity for the payment we want to
+       // send.
+       let chan_id  = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 989_000_000).0.contents.short_channel_id;
 
        // Marshall data to send the payment
-       let amt_msat = 20_000;
-       let (_, payment_hash, _, payment_secret) = get_route_and_payment_hash!(&nodes[0], nodes[1], amt_msat);
+       let amt_msat = 10_000_000;
+       let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[1], Some(amt_msat), None);
        #[cfg(feature = "std")]
        let payment_expiry_secs = SystemTime::UNIX_EPOCH.elapsed().unwrap().as_secs() + 60 * 60;
        #[cfg(not(feature = "std"))]
@@ -2497,16 +2481,31 @@ fn auto_retry_zero_attempts_send_error() {
                .with_bolt11_features(invoice_features).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params, amt_msat);
 
-       chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::PermanentFailure);
+       // Override the route search to return a route, rather than failing at the route-finding step.
+       let send_route = Route {
+               paths: vec![
+                       Path { hops: vec![RouteHop {
+                               pubkey: nodes[1].node.get_our_node_id(),
+                               node_features: nodes[1].node.node_features(),
+                               short_channel_id: chan_id,
+                               channel_features: nodes[1].node.channel_features(),
+                               fee_msat: amt_msat,
+                               cltv_expiry_delta: 100,
+                               maybe_announced_channel: true,
+                       }], blinded_tail: None },
+               ],
+               route_params: Some(route_params.clone()),
+       };
+       nodes[0].router.expect_find_route(route_params.clone(), Ok(send_route));
+
        nodes[0].node.send_payment(payment_hash, RecipientOnionFields::secret_only(payment_secret),
                PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap();
-       assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 2); // channel close messages
+       assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
        let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 3);
-       if let Event::ChannelClosed { .. } = events[0] { } else { panic!(); }
-       if let Event::PaymentPathFailed { .. } = events[1] { } else { panic!(); }
-       if let Event::PaymentFailed { .. } = events[2] { } else { panic!(); }
-       check_added_monitors!(nodes[0], 2);
+       assert_eq!(events.len(), 2);
+       if let Event::PaymentPathFailed { .. } = events[0] { } else { panic!(); }
+       if let Event::PaymentFailed { .. } = events[1] { } else { panic!(); }
+       check_added_monitors!(nodes[0], 0);
 }
 
 #[test]
index c3a428ae0147d85ceb8ddf2bef58cfc75a9ff34c..fa08fba99fabda640739f714dd9eb6818d2ab374 100644 (file)
@@ -448,7 +448,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 
        for monitor in node_0_monitors.drain(..) {
                assert_eq!(nodes[0].chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor),
-                       ChannelMonitorUpdateStatus::Completed);
+                       Ok(ChannelMonitorUpdateStatus::Completed));
                check_added_monitors!(nodes[0], 1);
        }
        nodes[0].node = &nodes_0_deserialized;
index 7c57b553068ada64d561f805a53013569254d68b..cb3471763d4b7e2075761ed466b718cd64d6797d 100644 (file)
@@ -462,7 +462,7 @@ fn test_set_outpoints_partial_claiming() {
        // Connect blocks on node B
        connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        check_closed_broadcast!(nodes[1], true);
-       check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 1000000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
        check_added_monitors!(nodes[1], 1);
        // Verify node B broadcast 2 HTLC-timeout txn
        let partial_claim_tx = {
index f105edc19c05be35d1dceb1480681b35651172f7..01f2837abc5c691ce0c32296dbe16d3748226dd5 100644 (file)
@@ -315,12 +315,12 @@ fn updates_shutdown_wait() {
        let payment_params_1 = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), TEST_FINAL_CLTV).with_bolt11_features(nodes[1].node.invoice_features()).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params_1, 100_000);
        let route_1 = get_route(&nodes[0].node.get_our_node_id(), &route_params,
-               &nodes[0].network_graph.read_only(), None, &logger, &scorer, &(), &random_seed_bytes).unwrap();
+               &nodes[0].network_graph.read_only(), None, &logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        let payment_params_2 = PaymentParameters::from_node_id(nodes[0].node.get_our_node_id(),
                TEST_FINAL_CLTV).with_bolt11_features(nodes[0].node.invoice_features()).unwrap();
        let route_params = RouteParameters::from_payment_params_and_value(payment_params_2, 100_000);
        let route_2 = get_route(&nodes[1].node.get_our_node_id(), &route_params,
-               &nodes[1].network_graph.read_only(), None, &logger, &scorer, &(), &random_seed_bytes).unwrap();
+               &nodes[1].network_graph.read_only(), None, &logger, &scorer, &Default::default(), &random_seed_bytes).unwrap();
        unwrap_send_err!(nodes[0].node.send_payment_with_route(&route_1, payment_hash,
                        RecipientOnionFields::secret_only(payment_secret), PaymentId(payment_hash.0)
                ), true, APIError::ChannelUnavailable {..}, {});
index 1758cbbf9c3eaae24151050f488c2524f43109da..6e5c00e7665a4c04ccab6df38de89bef9ad7e5ba 100644 (file)
@@ -2325,8 +2325,9 @@ where L::Target: Logger {
                                        // Decrease the available liquidity of a hop in the middle of the path.
                                        let victim_candidate = &payment_path.hops[(payment_path.hops.len()) / 2].0.candidate;
                                        let exhausted = u64::max_value();
-                                       log_trace!(logger, "Disabling route candidate {} for future path building iterations to
-                                               avoid duplicates.", LoggedCandidateHop(victim_candidate));
+                                       log_trace!(logger,
+                                               "Disabling route candidate {} for future path building iterations to avoid duplicates.",
+                                               LoggedCandidateHop(victim_candidate));
                                        *used_liquidities.entry(victim_candidate.id(false)).or_default() = exhausted;
                                        *used_liquidities.entry(victim_candidate.id(true)).or_default() = exhausted;
                                }
@@ -2704,7 +2705,7 @@ fn build_route_from_hops_internal<L: Deref>(
 
        let scorer = HopScorer { our_node_id, hop_ids };
 
-       get_route(our_node_pubkey, route_params, network_graph, None, logger, &scorer, &(), random_seed_bytes)
+       get_route(our_node_pubkey, route_params, network_graph, None, logger, &scorer, &Default::default(), random_seed_bytes)
 }
 
 #[cfg(test)]
@@ -2800,14 +2801,14 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 0);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes) {
+                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "Cannot send a payment of 0 msat");
                } else { panic!(); }
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -2841,12 +2842,12 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
                        &route_params, &network_graph.read_only(), Some(&our_chans.iter().collect::<Vec<_>>()),
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes) {
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "First hop cannot have our_node_pubkey as a destination.");
                } else { panic!(); }
  
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
        }
 
@@ -2957,8 +2958,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, 199_999_999);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes) {
+                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
 
@@ -2978,7 +2979,7 @@ mod tests {
 
                // A payment above the minimum should pass
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
        }
 
@@ -3063,7 +3064,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 60_000);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                // Overpay fees to hit htlc_minimum_msat.
                let overpaid_fees = route.paths[0].hops[0].fee_msat + route.paths[1].hops[0].fee_msat;
                // TODO: this could be better balanced to overpay 10k and not 15k.
@@ -3109,7 +3110,7 @@ mod tests {
                });
 
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                // Fine to overpay for htlc_minimum_msat if it allows us to save fee.
                assert_eq!(route.paths.len(), 1);
                assert_eq!(route.paths[0].hops[0].short_channel_id, 12);
@@ -3118,7 +3119,7 @@ mod tests {
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 50_000);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                // Not fine to overpay for htlc_minimum_msat if it requires paying more than fee on
                // the other channel.
                assert_eq!(route.paths.len(), 1);
@@ -3165,8 +3166,8 @@ mod tests {
                // If all the channels require some features we don't understand, route should fail
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes) {
+                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
 
@@ -3174,8 +3175,8 @@ mod tests {
                let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(),
                        InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &route_params, &network_graph.read_only(),
-                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[7]);
@@ -3212,8 +3213,8 @@ mod tests {
                // If all nodes require some features we don't understand, route should fail
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes) {
+                       &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!(); }
 
@@ -3221,8 +3222,8 @@ mod tests {
                let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(),
                        InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &route_params, &network_graph.read_only(),
-                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[7]);
@@ -3256,7 +3257,7 @@ mod tests {
                let payment_params = PaymentParameters::from_node_id(nodes[0], 42);
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 3);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3286,8 +3287,8 @@ mod tests {
                let our_chans = vec![get_channel_details(Some(42), nodes[7].clone(),
                        InitFeatures::from_le_bytes(vec![0b11]), 250_000_000)];
                let route = get_route(&our_id, &route_params, &network_graph.read_only(),
-                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[7]);
@@ -3413,8 +3414,8 @@ mod tests {
                                .with_route_hints(invalid_last_hops).unwrap();
                        let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                               &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer, &(),
-                               &random_seed_bytes) {
+                               &route_params, &network_graph.read_only(), None, Arc::clone(&logger), &scorer,
+                               &Default::default(), &random_seed_bytes) {
                                        assert_eq!(err, "Route hint cannot have the payee as the source.");
                        } else { panic!(); }
                }
@@ -3423,7 +3424,7 @@ mod tests {
                        .with_route_hints(last_hops_multi_private_channels(&nodes)).unwrap();
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 5);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3500,7 +3501,7 @@ mod tests {
                // Test handling of an empty RouteHint passed in Invoice.
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 5);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3608,7 +3609,7 @@ mod tests {
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 4);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3682,7 +3683,7 @@ mod tests {
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &[42u8; 32]).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &[42u8; 32]).unwrap();
                assert_eq!(route.paths[0].hops.len(), 4);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3766,7 +3767,7 @@ mod tests {
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 5);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3822,8 +3823,8 @@ mod tests {
                        .with_route_hints(last_hops.clone()).unwrap();
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(),
-                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 2);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[3]);
@@ -3848,7 +3849,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 4);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3884,7 +3885,7 @@ mod tests {
                // ...but still use 8 for larger payments as 6 has a variable feerate
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 2000);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths[0].hops.len(), 5);
 
                assert_eq!(route.paths[0].hops[0].pubkey, nodes[1]);
@@ -3951,7 +3952,7 @@ mod tests {
                let network_graph = NetworkGraph::new(Network::Testnet, &logger);
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, route_val);
                let route = get_route(&source_node_id, &route_params, &network_graph.read_only(),
-                               Some(&our_chans.iter().collect::<Vec<_>>()), &logger, &scorer, &(),
+                               Some(&our_chans.iter().collect::<Vec<_>>()), &logger, &scorer, &Default::default(),
                                &random_seed_bytes);
                route
        }
@@ -4077,7 +4078,7 @@ mod tests {
                                payment_params.clone(), 250_000_001);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None,
-                                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes) {
+                                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4087,7 +4088,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 250_000_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(),&random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.hops.len(), 2);
@@ -4120,7 +4121,7 @@ mod tests {
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(),
                                        Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
-                                       &(), &random_seed_bytes) {
+                                       &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4130,8 +4131,8 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 200_000_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(),
-                               Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                               &random_seed_bytes).unwrap();
+                               Some(&our_chans.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                               &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.hops.len(), 2);
@@ -4174,7 +4175,7 @@ mod tests {
                                payment_params.clone(), 15_001);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4184,7 +4185,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 15_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.hops.len(), 2);
@@ -4251,7 +4252,7 @@ mod tests {
                                payment_params.clone(), 15_001);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4261,7 +4262,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 15_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.hops.len(), 2);
@@ -4289,7 +4290,7 @@ mod tests {
                                payment_params.clone(), 10_001);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4299,7 +4300,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 10_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let path = route.paths.last().unwrap();
                        assert_eq!(path.hops.len(), 2);
@@ -4407,7 +4408,7 @@ mod tests {
                                payment_params.clone(), 60_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4417,7 +4418,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 49_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4433,7 +4434,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 50_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4484,7 +4485,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 50_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4653,7 +4654,7 @@ mod tests {
                                payment_params.clone(), 300_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                &our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes) {
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
                                        assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4665,7 +4666,7 @@ mod tests {
                                zero_payment_params, 100);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                &our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes) {
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
                                        assert_eq!(err, "Can't find a route with no paths allowed.");
                        } else { panic!(); }
                }
@@ -4679,7 +4680,7 @@ mod tests {
                                fail_payment_params, 250_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                &our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes) {
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes) {
                                        assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4690,7 +4691,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 250_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4710,7 +4711,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 290_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -4884,7 +4885,7 @@ mod tests {
                                payment_params.clone(), 350_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -4895,7 +4896,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 300_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
 
                        let mut total_amount_paid_msat = 0;
@@ -5059,7 +5060,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 180_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 2);
 
                        let mut total_value_transferred_msat = 0;
@@ -5233,7 +5234,7 @@ mod tests {
                                payment_params.clone(), 210_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -5242,7 +5243,7 @@ mod tests {
                        // Now, attempt to route 200 sats (exact amount we can route).
                        let route_params = RouteParameters::from_payment_params_and_value(payment_params, 200_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 2);
 
                        let mut total_amount_paid_msat = 0;
@@ -5345,7 +5346,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, 100_000);
                let mut route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                route.paths.sort_by_key(|path| path.hops[0].short_channel_id);
                // Paths are manually ordered ordered by SCID, so:
@@ -5467,7 +5468,7 @@ mod tests {
                                payment_params.clone(), 150_000);
                        if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(
                                        &our_id, &route_params, &network_graph.read_only(), None, Arc::clone(&logger),
-                                       &scorer, &(), &random_seed_bytes) {
+                                       &scorer, &Default::default(), &random_seed_bytes) {
                                                assert_eq!(err, "Failed to find a sufficient route to the given destination");
                        } else { panic!(); }
                }
@@ -5478,7 +5479,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 125_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 3);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -5494,7 +5495,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 90_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 2);
                        let mut total_amount_paid_msat = 0;
                        for path in &route.paths {
@@ -5636,7 +5637,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 10_000);
                        let route = get_route(&our_id, &route_params, &network.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].hops.len(), 3);
 
@@ -5710,7 +5711,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 90_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].hops.len(), 2);
 
@@ -5779,7 +5780,7 @@ mod tests {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params, 90_000);
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].hops.len(), 2);
 
@@ -5823,7 +5824,7 @@ mod tests {
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), Some(&[
                                &get_channel_details(Some(3), nodes[0], channelmanager::provided_init_features(&config), 200_000),
                                &get_channel_details(Some(2), nodes[0], channelmanager::provided_init_features(&config), 10_000),
-                       ]), Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       ]), Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].hops.len(), 1);
 
@@ -5837,7 +5838,7 @@ mod tests {
                        let route = get_route(&our_id, &route_params, &network_graph.read_only(), Some(&[
                                &get_channel_details(Some(3), nodes[0], channelmanager::provided_init_features(&config), 50_000),
                                &get_channel_details(Some(2), nodes[0], channelmanager::provided_init_features(&config), 50_000),
-                       ]), Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       ]), Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 2);
                        assert_eq!(route.paths[0].hops.len(), 1);
                        assert_eq!(route.paths[1].hops.len(), 1);
@@ -5871,7 +5872,7 @@ mod tests {
                                &get_channel_details(Some(8), nodes[0], channelmanager::provided_init_features(&config), 50_000),
                                &get_channel_details(Some(9), nodes[0], channelmanager::provided_init_features(&config), 50_000),
                                &get_channel_details(Some(4), nodes[0], channelmanager::provided_init_features(&config), 1_000_000),
-                       ]), Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       ]), Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                        assert_eq!(route.paths.len(), 1);
                        assert_eq!(route.paths[0].hops.len(), 1);
 
@@ -5894,7 +5895,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 100);
                let route = get_route( &our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
 
                assert_eq!(route.get_total_fees(), 100);
@@ -5907,7 +5908,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, 100);
                let route = get_route( &our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
 
                assert_eq!(route.get_total_fees(), 300);
@@ -5960,7 +5961,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, 100);
                let route = get_route( &our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
 
                assert_eq!(route.get_total_fees(), 100);
@@ -5970,7 +5971,7 @@ mod tests {
                // A different path to nodes[6] exists if channel 6 cannot be routed over.
                let scorer = BadChannelScorer { short_channel_id: 6 };
                let route = get_route( &our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
 
                assert_eq!(route.get_total_fees(), 300);
@@ -5980,7 +5981,7 @@ mod tests {
                // A path to nodes[6] does not exist if nodes[2] cannot be routed through.
                let scorer = BadNodeScorer { node_id: NodeId::from_pubkey(&nodes[2]) };
                match get_route( &our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes) {
+                       &scorer, &Default::default(), &random_seed_bytes) {
                                Err(LightningError { err, .. } ) => {
                                        assert_eq!(err, "Failed to find a path to the given destination");
                                },
@@ -6076,7 +6077,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        feasible_payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
                assert_ne!(path.len(), 0);
 
@@ -6087,7 +6088,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        fail_payment_params, 100);
                match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer,
-                       &(), &random_seed_bytes)
+                       &Default::default(), &random_seed_bytes)
                {
                        Err(LightningError { err, .. } ) => {
                                assert_eq!(err, "Failed to find a path to the given destination");
@@ -6115,12 +6116,12 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 100);
                assert!(get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).is_ok());
+                       &scorer, &Default::default(), &random_seed_bytes).is_ok());
                loop {
                        let route_params = RouteParameters::from_payment_params_and_value(
                                payment_params.clone(), 100);
                        if let Ok(route) = get_route(&our_id, &route_params, &network_graph, None,
-                               Arc::clone(&logger), &scorer, &(), &random_seed_bytes)
+                               Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes)
                        {
                                for chan in route.paths[0].hops.iter() {
                                        assert!(!payment_params.previously_failed_channels.contains(&chan.short_channel_id));
@@ -6147,7 +6148,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        feasible_payment_params, 100);
                let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                let path = route.paths[0].hops.iter().map(|hop| hop.short_channel_id).collect::<Vec<_>>();
                assert!(path.len() == MAX_PATH_LENGTH_ESTIMATE.into());
 
@@ -6156,7 +6157,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        fail_payment_params, 100);
                match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer,
-                       &(), &random_seed_bytes)
+                       &Default::default(), &random_seed_bytes)
                {
                        Err(LightningError { err, .. } ) => {
                                assert_eq!(err, "Failed to find a path to the given destination");
@@ -6178,7 +6179,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 100);
                let route = get_route(&our_id, &route_params, &network_graph.read_only(), None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 1);
 
                let cltv_expiry_deltas_before = route.paths[0].hops.iter().map(|h| h.cltv_expiry_delta).collect::<Vec<u32>>();
@@ -6215,7 +6216,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), 100);
                let mut route = get_route(&our_id, &route_params, &network_graph, None,
-                       Arc::clone(&logger), &scorer, &(), &random_seed_bytes).unwrap();
+                       Arc::clone(&logger), &scorer, &Default::default(), &random_seed_bytes).unwrap();
                add_random_cltv_offset(&mut route, &payment_params, &network_graph, &random_seed_bytes);
 
                let mut path_plausibility = vec![];
@@ -6485,7 +6486,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, max_htlc_msat + 1);
                if let Err(LightningError{err, action: ErrorAction::IgnoreError}) = get_route(&our_id,
-                       &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &(),
+                       &route_params, &netgraph, None, Arc::clone(&logger), &scorer, &Default::default(),
                        &random_seed_bytes)
                {
                        assert_eq!(err, "Failed to find a sufficient route to the given destination");
@@ -6500,7 +6501,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, max_htlc_msat + 1);
                let route = get_route(&our_id, &route_params, &netgraph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                assert!(route.paths[0].hops.last().unwrap().fee_msat <= max_htlc_msat);
                assert!(route.paths[1].hops.last().unwrap().fee_msat <= max_htlc_msat);
@@ -6556,8 +6557,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, amt_msat);
                let route = get_route(&our_node_id, &route_params, &network_graph.read_only(),
-                       Some(&first_hop.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&first_hop.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                assert!(route.paths[0].hops.last().unwrap().fee_msat <= max_htlc_msat);
                assert!(route.paths[1].hops.last().unwrap().fee_msat <= max_htlc_msat);
@@ -6570,8 +6571,8 @@ mod tests {
                        get_channel_details(Some(43), intermed_node_id, InitFeatures::from_le_bytes(vec![0b11]), amt_msat - 10),
                ];
                let route = get_route(&our_node_id, &route_params, &network_graph.read_only(),
-                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                assert!(route.paths[0].hops.last().unwrap().fee_msat <= max_htlc_msat);
                assert!(route.paths[1].hops.last().unwrap().fee_msat <= max_htlc_msat);
@@ -6602,8 +6603,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, amt_msat);
                let route = get_route(&our_node_id, &route_params, &network_graph.read_only(),
-                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                assert!(route.paths[0].hops.last().unwrap().fee_msat <= max_htlc_msat);
                assert!(route.paths[1].hops.last().unwrap().fee_msat <= max_htlc_msat);
@@ -6801,7 +6802,7 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, 1001);
                let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 1);
                assert_eq!(route.paths[0].hops.len(), 2);
 
@@ -6856,7 +6857,7 @@ mod tests {
                        (blinded_payinfo.clone(), invalid_blinded_path_2)]);
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 1001);
                match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes)
+                       &scorer, &Default::default(), &random_seed_bytes)
                {
                        Err(LightningError { err, .. }) => {
                                assert_eq!(err, "1-hop blinded paths must all have matching introduction node ids");
@@ -6868,7 +6869,7 @@ mod tests {
                let payment_params = PaymentParameters::blinded(vec![(blinded_payinfo.clone(), invalid_blinded_path.clone())]);
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 1001);
                match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer,
-                       &(), &random_seed_bytes)
+                       &Default::default(), &random_seed_bytes)
                {
                        Err(LightningError { err, .. }) => {
                                assert_eq!(err, "Cannot generate a route to blinded paths if we are the introduction node to all of them");
@@ -6881,7 +6882,7 @@ mod tests {
                let payment_params = PaymentParameters::blinded(vec![(blinded_payinfo, invalid_blinded_path)]);
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 1001);
                match get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger), &scorer,
-                       &(), &random_seed_bytes)
+                       &Default::default(), &random_seed_bytes)
                {
                        Err(LightningError { err, .. }) => {
                                assert_eq!(err, "0-hop blinded path provided");
@@ -6935,7 +6936,7 @@ mod tests {
 
                let route_params = RouteParameters::from_payment_params_and_value(payment_params, 100_000);
                let route = get_route(&our_id, &route_params, &network_graph, None, Arc::clone(&logger),
-                       &scorer, &(), &random_seed_bytes).unwrap();
+                       &scorer, &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
                let mut total_amount_paid_msat = 0;
                for path in route.paths.into_iter() {
@@ -7024,8 +7025,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params.clone(), amt_msat);
                if let Err(LightningError { err, .. }) = get_route(&nodes[0], &route_params, &netgraph,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes) {
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes) {
                                assert_eq!(err, "Failed to find a path to the given destination");
                } else { panic!("Expected error") }
 
@@ -7034,8 +7035,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, amt_minus_blinded_path_fee);
                let route = get_route(&nodes[0], &route_params, &netgraph,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.get_total_fees(), blinded_payinfo.fee_base_msat as u64);
                assert_eq!(route.get_total_amount(), amt_minus_blinded_path_fee);
        }
@@ -7103,8 +7104,8 @@ mod tests {
                let route_params = RouteParameters::from_payment_params_and_value(
                        payment_params, amt_msat);
                let route = get_route(&nodes[0], &route_params, &netgraph,
-                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer, &(),
-                       &random_seed_bytes).unwrap();
+                       Some(&first_hops.iter().collect::<Vec<_>>()), Arc::clone(&logger), &scorer,
+                       &Default::default(), &random_seed_bytes).unwrap();
                assert_eq!(route.get_total_fees(), blinded_payinfo.fee_base_msat as u64);
                assert_eq!(route.get_total_amount(), amt_msat);
        }
@@ -7319,15 +7320,15 @@ pub mod benches {
                let logger = TestLogger::new();
                let network_graph = bench_utils::read_network_graph(&logger).unwrap();
                let scorer = FixedPenaltyScorer::with_penalty(0);
-               generate_routes(bench, &network_graph, scorer, &(), Bolt11InvoiceFeatures::empty(), 0,
-                       "generate_routes_with_zero_penalty_scorer");
+               generate_routes(bench, &network_graph, scorer, &Default::default(),
+                       Bolt11InvoiceFeatures::empty(), 0, "generate_routes_with_zero_penalty_scorer");
        }
 
        pub fn generate_mpp_routes_with_zero_penalty_scorer(bench: &mut Criterion) {
                let logger = TestLogger::new();
                let network_graph = bench_utils::read_network_graph(&logger).unwrap();
                let scorer = FixedPenaltyScorer::with_penalty(0);
-               generate_routes(bench, &network_graph, scorer, &(),
+               generate_routes(bench, &network_graph, scorer, &Default::default(),
                        channelmanager::provided_invoice_features(&UserConfig::default()), 0,
                        "generate_mpp_routes_with_zero_penalty_scorer");
        }
@@ -7361,6 +7362,42 @@ pub mod benches {
                        "generate_large_mpp_routes_with_probabilistic_scorer");
        }
 
+       pub fn generate_routes_with_nonlinear_probabilistic_scorer(bench: &mut Criterion) {
+               let logger = TestLogger::new();
+               let network_graph = bench_utils::read_network_graph(&logger).unwrap();
+               let mut params = ProbabilisticScoringFeeParameters::default();
+               params.linear_success_probability = false;
+               let scorer = ProbabilisticScorer::new(
+                       ProbabilisticScoringDecayParameters::default(), &network_graph, &logger);
+               generate_routes(bench, &network_graph, scorer, &params,
+                       channelmanager::provided_invoice_features(&UserConfig::default()), 0,
+                       "generate_routes_with_nonlinear_probabilistic_scorer");
+       }
+
+       pub fn generate_mpp_routes_with_nonlinear_probabilistic_scorer(bench: &mut Criterion) {
+               let logger = TestLogger::new();
+               let network_graph = bench_utils::read_network_graph(&logger).unwrap();
+               let mut params = ProbabilisticScoringFeeParameters::default();
+               params.linear_success_probability = false;
+               let scorer = ProbabilisticScorer::new(
+                       ProbabilisticScoringDecayParameters::default(), &network_graph, &logger);
+               generate_routes(bench, &network_graph, scorer, &params,
+                       channelmanager::provided_invoice_features(&UserConfig::default()), 0,
+                       "generate_mpp_routes_with_nonlinear_probabilistic_scorer");
+       }
+
+       pub fn generate_large_mpp_routes_with_nonlinear_probabilistic_scorer(bench: &mut Criterion) {
+               let logger = TestLogger::new();
+               let network_graph = bench_utils::read_network_graph(&logger).unwrap();
+               let mut params = ProbabilisticScoringFeeParameters::default();
+               params.linear_success_probability = false;
+               let scorer = ProbabilisticScorer::new(
+                       ProbabilisticScoringDecayParameters::default(), &network_graph, &logger);
+               generate_routes(bench, &network_graph, scorer, &params,
+                       channelmanager::provided_invoice_features(&UserConfig::default()), 100_000_000,
+                       "generate_large_mpp_routes_with_nonlinear_probabilistic_scorer");
+       }
+
        fn generate_routes<S: ScoreLookUp + ScoreUpdate>(
                bench: &mut Criterion, graph: &NetworkGraph<&TestLogger>, mut scorer: S,
                score_params: &S::ScoreParams, features: Bolt11InvoiceFeatures, starting_amount: u64,
index 748edd31eaed679af2e6c5d767b9abd8139c25ec..c790f5df5c360b8873198703a4493d55ff664443 100644 (file)
@@ -454,12 +454,13 @@ pub struct ProbabilisticScoringFeeParameters {
        /// Default value: 500 msat
        pub base_penalty_msat: u64,
 
-       /// A multiplier used with the payment amount to calculate a fixed penalty applied to each
-       /// channel, in excess of the [`base_penalty_msat`].
+       /// A multiplier used with the total amount flowing over a channel to calculate a fixed penalty
+       /// applied to each channel, in excess of the [`base_penalty_msat`].
        ///
        /// The purpose of the amount penalty is to avoid having fees dominate the channel cost (i.e.,
        /// fees plus penalty) for large payments. The penalty is computed as the product of this
-       /// multiplier and `2^30`ths of the payment amount.
+       /// multiplier and `2^30`ths of the total amount flowing over a channel (i.e. the payment
+       /// amount plus the amount of any other HTLCs flowing we sent over the same channel).
        ///
        /// ie `base_penalty_amount_multiplier_msat * amount_msat / 2^30`
        ///
@@ -486,14 +487,14 @@ pub struct ProbabilisticScoringFeeParameters {
        /// [`liquidity_offset_half_life`]: ProbabilisticScoringDecayParameters::liquidity_offset_half_life
        pub liquidity_penalty_multiplier_msat: u64,
 
-       /// A multiplier used in conjunction with a payment amount and the negative `log10` of the
-       /// channel's success probability for the payment, as determined by our latest estimates of the
-       /// channel's liquidity, to determine the amount penalty.
+       /// A multiplier used in conjunction with the total amount flowing over a channel and the
+       /// negative `log10` of the channel's success probability for the payment, as determined by our
+       /// latest estimates of the channel's liquidity, to determine the amount penalty.
        ///
        /// The purpose of the amount penalty is to avoid having fees dominate the channel cost (i.e.,
        /// fees plus penalty) for large payments. The penalty is computed as the product of this
-       /// multiplier and `2^20`ths of the payment amount, weighted by the negative `log10` of the
-       /// success probability.
+       /// multiplier and `2^20`ths of the amount flowing over this channel, weighted by the negative
+       /// `log10` of the success probability.
        ///
        /// `-log10(success_probability) * liquidity_penalty_amount_multiplier_msat * amount_msat / 2^20`
        ///
@@ -522,13 +523,15 @@ pub struct ProbabilisticScoringFeeParameters {
        /// [`liquidity_penalty_multiplier_msat`]: Self::liquidity_penalty_multiplier_msat
        pub historical_liquidity_penalty_multiplier_msat: u64,
 
-       /// A multiplier used in conjunction with the payment amount and the negative `log10` of the
-       /// channel's success probability for the payment, as determined based on the history of our
-       /// estimates of the channel's available liquidity, to determine a penalty.
+       /// A multiplier used in conjunction with the total amount flowing over a channel and the
+       /// negative `log10` of the channel's success probability for the payment, as determined based
+       /// on the history of our estimates of the channel's available liquidity, to determine a
+       /// penalty.
        ///
        /// The purpose of the amount penalty is to avoid having fees dominate the channel cost for
-       /// large payments. The penalty is computed as the product of this multiplier and the `2^20`ths
-       /// of the payment amount, weighted by the negative `log10` of the success probability.
+       /// large payments. The penalty is computed as the product of this multiplier and `2^20`ths
+       /// of the amount flowing over this channel, weighted by the negative `log10` of the success
+       /// probability.
        ///
        /// This penalty is similar to [`liquidity_penalty_amount_multiplier_msat`], however, instead
        /// of using only our latest estimate for the current liquidity available in the channel, it
@@ -558,8 +561,9 @@ pub struct ProbabilisticScoringFeeParameters {
        /// Default value: 250 msat
        pub anti_probing_penalty_msat: u64,
 
-       /// This penalty is applied when the amount we're attempting to send over a channel exceeds our
-       /// current estimate of the channel's available liquidity.
+       /// This penalty is applied when the total amount flowing over a channel exceeds our current
+       /// estimate of the channel's available liquidity. The total amount is the amount of the
+       /// current HTLC plus any HTLCs which we've sent over the same channel.
        ///
        /// Note that in this case all other penalties, including the
        /// [`liquidity_penalty_multiplier_msat`] and [`liquidity_penalty_amount_multiplier_msat`]-based
@@ -576,6 +580,28 @@ pub struct ProbabilisticScoringFeeParameters {
        /// [`base_penalty_msat`]: Self::base_penalty_msat
        /// [`anti_probing_penalty_msat`]: Self::anti_probing_penalty_msat
        pub considered_impossible_penalty_msat: u64,
+
+       /// In order to calculate most of the scores above, we must first convert a lower and upper
+       /// bound on the available liquidity in a channel into the probability that we think a payment
+       /// will succeed. That probability is derived from a Probability Density Function for where we
+       /// think the liquidity in a channel likely lies, given such bounds.
+       ///
+       /// If this flag is set, that PDF is simply a constant - we assume that the actual available
+       /// liquidity in a channel is just as likely to be at any point between our lower and upper
+       /// bounds.
+       ///
+       /// If this flag is *not* set, that PDF is `(x - 0.5*capacity) ^ 2`. That is, we use an
+       /// exponential curve which expects the liquidity of a channel to lie "at the edges". This
+       /// matches experimental results - most routing nodes do not aggressively rebalance their
+       /// channels and flows in the network are often unbalanced, leaving liquidity usually
+       /// unavailable.
+       ///
+       /// Thus, for the "best" routes, leave this flag `false`. However, the flag does imply a number
+       /// of floating-point multiplications in the hottest routing code, which may lead to routing
+       /// performance degradation on some machines.
+       ///
+       /// Default value: false
+       pub linear_success_probability: bool,
 }
 
 impl Default for ProbabilisticScoringFeeParameters {
@@ -590,6 +616,7 @@ impl Default for ProbabilisticScoringFeeParameters {
                        considered_impossible_penalty_msat: 1_0000_0000_000,
                        historical_liquidity_penalty_multiplier_msat: 10_000,
                        historical_liquidity_penalty_amount_multiplier_msat: 64,
+                       linear_success_probability: false,
                }
        }
 }
@@ -643,6 +670,7 @@ impl ProbabilisticScoringFeeParameters {
                        manual_node_penalties: HashMap::new(),
                        anti_probing_penalty_msat: 0,
                        considered_impossible_penalty_msat: 0,
+                       linear_success_probability: true,
                }
        }
 }
@@ -733,7 +761,6 @@ struct DirectedChannelLiquidity<L: Deref<Target = u64>, BRT: Deref<Target = Hist
        min_liquidity_offset_msat: L,
        max_liquidity_offset_msat: L,
        liquidity_history: HistoricalMinMaxBuckets<BRT>,
-       inflight_htlc_msat: u64,
        capacity_msat: u64,
        last_updated: U,
        now: T,
@@ -771,7 +798,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
                                let log_direction = |source, target| {
                                        if let Some((directed_info, _)) = chan_debug.as_directed_to(target) {
                                                let amt = directed_info.effective_capacity().as_msat();
-                                               let dir_liq = liq.as_directed(source, target, 0, amt, self.decay_params);
+                                               let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
 
                                                let (min_buckets, max_buckets) = dir_liq.liquidity_history
                                                        .get_decayed_buckets(now, *dir_liq.last_updated,
@@ -825,7 +852,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
                        if let Some(liq) = self.channel_liquidities.get(&scid) {
                                if let Some((directed_info, source)) = chan.as_directed_to(target) {
                                        let amt = directed_info.effective_capacity().as_msat();
-                                       let dir_liq = liq.as_directed(source, target, 0, amt, self.decay_params);
+                                       let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
                                        return Some((dir_liq.min_liquidity_msat(), dir_liq.max_liquidity_msat()));
                                }
                        }
@@ -867,7 +894,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
                        if let Some(liq) = self.channel_liquidities.get(&scid) {
                                if let Some((directed_info, source)) = chan.as_directed_to(target) {
                                        let amt = directed_info.effective_capacity().as_msat();
-                                       let dir_liq = liq.as_directed(source, target, 0, amt, self.decay_params);
+                                       let dir_liq = liq.as_directed(source, target, amt, self.decay_params);
 
                                        let (min_buckets, mut max_buckets) =
                                                dir_liq.liquidity_history.get_decayed_buckets(
@@ -893,7 +920,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
        /// [`Self::historical_estimated_channel_liquidity_probabilities`] (but not those returned by
        /// [`Self::estimated_channel_liquidity_range`]).
        pub fn historical_estimated_payment_success_probability(
-               &self, scid: u64, target: &NodeId, amount_msat: u64)
+               &self, scid: u64, target: &NodeId, amount_msat: u64, params: &ProbabilisticScoringFeeParameters)
        -> Option<f64> {
                let graph = self.network_graph.read_only();
 
@@ -901,11 +928,12 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ProbabilisticScorerU
                        if let Some(liq) = self.channel_liquidities.get(&scid) {
                                if let Some((directed_info, source)) = chan.as_directed_to(target) {
                                        let capacity_msat = directed_info.effective_capacity().as_msat();
-                                       let dir_liq = liq.as_directed(source, target, 0, capacity_msat, self.decay_params);
+                                       let dir_liq = liq.as_directed(source, target, capacity_msat, self.decay_params);
 
                                        return dir_liq.liquidity_history.calculate_success_probability_times_billion(
                                                dir_liq.now, *dir_liq.last_updated,
-                                               self.decay_params.historical_no_updates_half_life, amount_msat, capacity_msat
+                                               self.decay_params.historical_no_updates_half_life, &params, amount_msat,
+                                               capacity_msat
                                        ).map(|p| p as f64 / (1024 * 1024 * 1024) as f64);
                                }
                        }
@@ -929,7 +957,7 @@ impl<T: Time> ChannelLiquidity<T> {
        /// Returns a view of the channel liquidity directed from `source` to `target` assuming
        /// `capacity_msat`.
        fn as_directed(
-               &self, source: &NodeId, target: &NodeId, inflight_htlc_msat: u64, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
+               &self, source: &NodeId, target: &NodeId, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
        ) -> DirectedChannelLiquidity<&u64, &HistoricalBucketRangeTracker, T, &T> {
                let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
                        if source < target {
@@ -947,7 +975,6 @@ impl<T: Time> ChannelLiquidity<T> {
                                min_liquidity_offset_history,
                                max_liquidity_offset_history,
                        },
-                       inflight_htlc_msat,
                        capacity_msat,
                        last_updated: &self.last_updated,
                        now: T::now(),
@@ -958,7 +985,7 @@ impl<T: Time> ChannelLiquidity<T> {
        /// Returns a mutable view of the channel liquidity directed from `source` to `target` assuming
        /// `capacity_msat`.
        fn as_directed_mut(
-               &mut self, source: &NodeId, target: &NodeId, inflight_htlc_msat: u64, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
+               &mut self, source: &NodeId, target: &NodeId, capacity_msat: u64, decay_params: ProbabilisticScoringDecayParameters
        ) -> DirectedChannelLiquidity<&mut u64, &mut HistoricalBucketRangeTracker, T, &mut T> {
                let (min_liquidity_offset_msat, max_liquidity_offset_msat, min_liquidity_offset_history, max_liquidity_offset_history) =
                        if source < target {
@@ -976,7 +1003,6 @@ impl<T: Time> ChannelLiquidity<T> {
                                min_liquidity_offset_history,
                                max_liquidity_offset_history,
                        },
-                       inflight_htlc_msat,
                        capacity_msat,
                        last_updated: &mut self.last_updated,
                        now: T::now(),
@@ -997,11 +1023,80 @@ const PRECISION_LOWER_BOUND_DENOMINATOR: u64 = approx::LOWER_BITS_BOUND;
 const AMOUNT_PENALTY_DIVISOR: u64 = 1 << 20;
 const BASE_AMOUNT_PENALTY_DIVISOR: u64 = 1 << 30;
 
+/// Raises three `f64`s to the 3rd power, without `powi` because it requires `std` (dunno why).
+#[inline(always)]
+fn three_f64_pow_3(a: f64, b: f64, c: f64) -> (f64, f64, f64) {
+       (a * a * a, b * b * b, c * c * c)
+}
+
+/// Given liquidity bounds, calculates the success probability (in the form of a numerator and
+/// denominator) of an HTLC. This is a key assumption in our scoring models.
+///
+/// Must not return a numerator or denominator greater than 2^31 for arguments less than 2^31.
+///
+/// min_zero_implies_no_successes signals that a `min_liquidity_msat` of 0 means we've not
+/// (recently) seen an HTLC successfully complete over this channel.
+#[inline(always)]
+fn success_probability(
+       amount_msat: u64, min_liquidity_msat: u64, max_liquidity_msat: u64, capacity_msat: u64,
+       params: &ProbabilisticScoringFeeParameters, min_zero_implies_no_successes: bool,
+) -> (u64, u64) {
+       debug_assert!(min_liquidity_msat <= amount_msat);
+       debug_assert!(amount_msat < max_liquidity_msat);
+       debug_assert!(max_liquidity_msat <= capacity_msat);
+
+       let (numerator, mut denominator) =
+               if params.linear_success_probability {
+                       (max_liquidity_msat - amount_msat,
+                               (max_liquidity_msat - min_liquidity_msat).saturating_add(1))
+               } else {
+                       let capacity = capacity_msat as f64;
+                       let min = (min_liquidity_msat as f64) / capacity;
+                       let max = (max_liquidity_msat as f64) / capacity;
+                       let amount = (amount_msat as f64) / capacity;
+
+                       // Assume the channel has a probability density function of (x - 0.5)^2 for values from
+                       // 0 to 1 (where 1 is the channel's full capacity). The success probability given some
+                       // liquidity bounds is thus the integral under the curve from the amount to maximum
+                       // estimated liquidity, divided by the same integral from the minimum to the maximum
+                       // estimated liquidity bounds.
+                       //
+                       // Because the integral from x to y is simply (y - 0.5)^3 - (x - 0.5)^3, we can
+                       // calculate the cumulative density function between the min/max bounds trivially. Note
+                       // that we don't bother to normalize the CDF to total to 1, as it will come out in the
+                       // division of num / den.
+                       let (max_pow, amt_pow, min_pow) = three_f64_pow_3(max - 0.5, amount - 0.5, min - 0.5);
+                       let num = max_pow - amt_pow;
+                       let den = max_pow - min_pow;
+
+                       // Because our numerator and denominator max out at 0.5^3 we need to multiply them by
+                       // quite a large factor to get something useful (ideally in the 2^30 range).
+                       const BILLIONISH: f64 = 1024.0 * 1024.0 * 1024.0;
+                       let numerator = (num * BILLIONISH) as u64 + 1;
+                       let denominator = (den * BILLIONISH) as u64 + 1;
+                       debug_assert!(numerator <= 1 << 30, "Got large numerator ({}) from float {}.", numerator, num);
+                       debug_assert!(denominator <= 1 << 30, "Got large denominator ({}) from float {}.", denominator, den);
+                       (numerator, denominator)
+               };
+
+       if min_zero_implies_no_successes && min_liquidity_msat == 0 &&
+               denominator < u64::max_value() / 21
+       {
+               // If we have no knowledge of the channel, scale probability down by ~75%
+               // Note that we prefer to increase the denominator rather than decrease the numerator as
+               // the denominator is more likely to be larger and thus provide greater precision. This is
+               // mostly an overoptimization but makes a large difference in tests.
+               denominator = denominator * 21 / 16
+       }
+
+       (numerator, denominator)
+}
+
 impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>, T: Time, U: Deref<Target = T>> DirectedChannelLiquidity< L, BRT, T, U> {
        /// Returns a liquidity penalty for routing the given HTLC `amount_msat` through the channel in
        /// this direction.
        fn penalty_msat(&self, amount_msat: u64, score_params: &ProbabilisticScoringFeeParameters) -> u64 {
-               let available_capacity = self.available_capacity();
+               let available_capacity = self.capacity_msat;
                let max_liquidity_msat = self.max_liquidity_msat();
                let min_liquidity_msat = core::cmp::min(self.min_liquidity_msat(), max_liquidity_msat);
 
@@ -1017,9 +1112,9 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
                                        score_params.liquidity_penalty_amount_multiplier_msat)
                                .saturating_add(score_params.considered_impossible_penalty_msat)
                } else {
-                       let numerator = (max_liquidity_msat - amount_msat).saturating_add(1);
-                       let denominator = (max_liquidity_msat - min_liquidity_msat).saturating_add(1);
-                       if amount_msat - min_liquidity_msat < denominator / PRECISION_LOWER_BOUND_DENOMINATOR {
+                       let (numerator, denominator) = success_probability(amount_msat,
+                               min_liquidity_msat, max_liquidity_msat, available_capacity, score_params, false);
+                       if denominator - numerator < denominator / PRECISION_LOWER_BOUND_DENOMINATOR {
                                // If the failure probability is < 1.5625% (as 1 - numerator/denominator < 1/64),
                                // don't bother trying to use the log approximation as it gets too noisy to be
                                // particularly helpful, instead just round down to 0.
@@ -1046,7 +1141,8 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
                   score_params.historical_liquidity_penalty_amount_multiplier_msat != 0 {
                        if let Some(cumulative_success_prob_times_billion) = self.liquidity_history
                                .calculate_success_probability_times_billion(self.now, *self.last_updated,
-                                       self.decay_params.historical_no_updates_half_life, amount_msat, self.capacity_msat)
+                                       self.decay_params.historical_no_updates_half_life, score_params, amount_msat,
+                                       self.capacity_msat)
                        {
                                let historical_negative_log10_times_2048 = approx::negative_log10_times_2048(cumulative_success_prob_times_billion + 1, 1024 * 1024 * 1024);
                                res = res.saturating_add(Self::combined_penalty_msat(amount_msat,
@@ -1056,9 +1152,8 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
                                // If we don't have any valid points (or, once decayed, we have less than a full
                                // point), redo the non-historical calculation with no liquidity bounds tracked and
                                // the historical penalty multipliers.
-                               let available_capacity = self.available_capacity();
-                               let numerator = available_capacity.saturating_sub(amount_msat).saturating_add(1);
-                               let denominator = available_capacity.saturating_add(1);
+                               let (numerator, denominator) = success_probability(amount_msat, 0,
+                                       available_capacity, available_capacity, score_params, true);
                                let negative_log10_times_2048 =
                                        approx::negative_log10_times_2048(numerator, denominator);
                                res = res.saturating_add(Self::combined_penalty_msat(amount_msat, negative_log10_times_2048,
@@ -1097,16 +1192,10 @@ impl<L: Deref<Target = u64>, BRT: Deref<Target = HistoricalBucketRangeTracker>,
        /// Returns the upper bound of the channel liquidity balance in this direction.
        #[inline(always)]
        fn max_liquidity_msat(&self) -> u64 {
-               self.available_capacity()
+               self.capacity_msat
                        .saturating_sub(self.decayed_offset_msat(*self.max_liquidity_offset_msat))
        }
 
-       /// Returns the capacity minus the in-flight HTLCs in this direction.
-       #[inline(always)]
-       fn available_capacity(&self) -> u64 {
-               self.capacity_msat.saturating_sub(self.inflight_htlc_msat)
-       }
-
        fn decayed_offset_msat(&self, offset_msat: u64) -> u64 {
                let half_life = self.decay_params.liquidity_offset_half_life.as_secs();
                if half_life != 0 {
@@ -1241,13 +1330,12 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ScoreLookUp for Prob
                        _ => {},
                }
 
-               let amount_msat = usage.amount_msat;
+               let amount_msat = usage.amount_msat.saturating_add(usage.inflight_htlc_msat);
                let capacity_msat = usage.effective_capacity.as_msat();
-               let inflight_htlc_msat = usage.inflight_htlc_msat;
                self.channel_liquidities
                        .get(&short_channel_id)
                        .unwrap_or(&ChannelLiquidity::new())
-                       .as_directed(source, target, inflight_htlc_msat, capacity_msat, self.decay_params)
+                       .as_directed(source, target, capacity_msat, self.decay_params)
                        .penalty_msat(amount_msat, score_params)
                        .saturating_add(anti_probing_penalty_msat)
                        .saturating_add(base_penalty_msat)
@@ -1277,13 +1365,13 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ScoreUpdate for Prob
                                        self.channel_liquidities
                                                .entry(hop.short_channel_id)
                                                .or_insert_with(ChannelLiquidity::new)
-                                               .as_directed_mut(source, &target, 0, capacity_msat, self.decay_params)
+                                               .as_directed_mut(source, &target, capacity_msat, self.decay_params)
                                                .failed_at_channel(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
                                } else {
                                        self.channel_liquidities
                                                .entry(hop.short_channel_id)
                                                .or_insert_with(ChannelLiquidity::new)
-                                               .as_directed_mut(source, &target, 0, capacity_msat, self.decay_params)
+                                               .as_directed_mut(source, &target, capacity_msat, self.decay_params)
                                                .failed_downstream(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
                                }
                        } else {
@@ -1311,7 +1399,7 @@ impl<G: Deref<Target = NetworkGraph<L>>, L: Deref, T: Time> ScoreUpdate for Prob
                                self.channel_liquidities
                                        .entry(hop.short_channel_id)
                                        .or_insert_with(ChannelLiquidity::new)
-                                       .as_directed_mut(source, &target, 0, capacity_msat, self.decay_params)
+                                       .as_directed_mut(source, &target, capacity_msat, self.decay_params)
                                        .successful(amount_msat, format_args!("SCID {}, towards {:?}", hop.short_channel_id, target), &self.logger);
                        } else {
                                log_debug!(self.logger, "Not able to learn for channel with SCID {} as we do not have graph info for it (likely a route-hint last-hop).",
@@ -1851,8 +1939,9 @@ mod bucketed_history {
 
                #[inline]
                pub(super) fn calculate_success_probability_times_billion<T: Time>(
-                       &self, now: T, last_updated: T, half_life: Duration, amount_msat: u64, capacity_msat: u64)
-               -> Option<u64> {
+                       &self, now: T, last_updated: T, half_life: Duration,
+                       params: &ProbabilisticScoringFeeParameters, amount_msat: u64, capacity_msat: u64
+               ) -> Option<u64> {
                        // If historical penalties are enabled, we try to calculate a probability of success
                        // given our historical distribution of min- and max-liquidity bounds in a channel.
                        // To do so, we walk the set of historical liquidity bucket (min, max) combinations
@@ -1887,14 +1976,13 @@ mod bucketed_history {
                                }
                                let max_bucket_end_pos = BUCKET_START_POS[32 - highest_max_bucket_with_points] - 1;
                                if payment_pos < max_bucket_end_pos {
+                                       let (numerator, denominator) = success_probability(payment_pos as u64, 0,
+                                               max_bucket_end_pos as u64, POSITION_TICKS as u64 - 1, params, true);
                                        let bucket_prob_times_billion =
                                                (self.min_liquidity_offset_history.buckets[0] as u64) * total_max_points
                                                        * 1024 * 1024 * 1024 / total_valid_points_tracked;
                                        cumulative_success_prob_times_billion += bucket_prob_times_billion *
-                                               ((max_bucket_end_pos - payment_pos) as u64) /
-                                               // Add an additional one in the divisor as the payment bucket has been
-                                               // rounded down.
-                                               (max_bucket_end_pos + 1) as u64;
+                                               numerator / denominator;
                                }
                        }
 
@@ -1912,11 +2000,11 @@ mod bucketed_history {
                                        } else if payment_pos < min_bucket_start_pos {
                                                cumulative_success_prob_times_billion += bucket_prob_times_billion;
                                        } else {
+                                               let (numerator, denominator) = success_probability(payment_pos as u64,
+                                                       min_bucket_start_pos as u64, max_bucket_end_pos as u64,
+                                                       POSITION_TICKS as u64 - 1, params, true);
                                                cumulative_success_prob_times_billion += bucket_prob_times_billion *
-                                                       ((max_bucket_end_pos - payment_pos) as u64) /
-                                                       // Add an additional one in the divisor as the payment bucket has been
-                                                       // rounded down.
-                                                       ((max_bucket_end_pos - min_bucket_start_pos + 1) as u64);
+                                                       numerator / denominator;
                                        }
                                }
                        }
@@ -2228,52 +2316,52 @@ mod tests {
                // Update minimum liquidity.
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 100);
                assert_eq!(liquidity.max_liquidity_msat(), 300);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 700);
                assert_eq!(liquidity.max_liquidity_msat(), 900);
 
                scorer.channel_liquidities.get_mut(&42).unwrap()
-                       .as_directed_mut(&source, &target, 0, 1_000, decay_params)
+                       .as_directed_mut(&source, &target, 1_000, decay_params)
                        .set_min_liquidity_msat(200);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 200);
                assert_eq!(liquidity.max_liquidity_msat(), 300);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 700);
                assert_eq!(liquidity.max_liquidity_msat(), 800);
 
                // Update maximum liquidity.
 
                let liquidity = scorer.channel_liquidities.get(&43).unwrap()
-                       .as_directed(&target, &recipient, 0, 1_000, decay_params);
+                       .as_directed(&target, &recipient, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 700);
                assert_eq!(liquidity.max_liquidity_msat(), 900);
 
                let liquidity = scorer.channel_liquidities.get(&43).unwrap()
-                       .as_directed(&recipient, &target, 0, 1_000, decay_params);
+                       .as_directed(&recipient, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 100);
                assert_eq!(liquidity.max_liquidity_msat(), 300);
 
                scorer.channel_liquidities.get_mut(&43).unwrap()
-                       .as_directed_mut(&target, &recipient, 0, 1_000, decay_params)
+                       .as_directed_mut(&target, &recipient, 1_000, decay_params)
                        .set_max_liquidity_msat(200);
 
                let liquidity = scorer.channel_liquidities.get(&43).unwrap()
-                       .as_directed(&target, &recipient, 0, 1_000, decay_params);
+                       .as_directed(&target, &recipient, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 0);
                assert_eq!(liquidity.max_liquidity_msat(), 200);
 
                let liquidity = scorer.channel_liquidities.get(&43).unwrap()
-                       .as_directed(&recipient, &target, 0, 1_000, decay_params);
+                       .as_directed(&recipient, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 800);
                assert_eq!(liquidity.max_liquidity_msat(), 1000);
        }
@@ -2297,42 +2385,42 @@ mod tests {
 
                // Check initial bounds.
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 400);
                assert_eq!(liquidity.max_liquidity_msat(), 800);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 200);
                assert_eq!(liquidity.max_liquidity_msat(), 600);
 
                // Reset from source to target.
                scorer.channel_liquidities.get_mut(&42).unwrap()
-                       .as_directed_mut(&source, &target, 0, 1_000, decay_params)
+                       .as_directed_mut(&source, &target, 1_000, decay_params)
                        .set_min_liquidity_msat(900);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 900);
                assert_eq!(liquidity.max_liquidity_msat(), 1_000);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 0);
                assert_eq!(liquidity.max_liquidity_msat(), 100);
 
                // Reset from target to source.
                scorer.channel_liquidities.get_mut(&42).unwrap()
-                       .as_directed_mut(&target, &source, 0, 1_000, decay_params)
+                       .as_directed_mut(&target, &source, 1_000, decay_params)
                        .set_min_liquidity_msat(400);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 0);
                assert_eq!(liquidity.max_liquidity_msat(), 600);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 400);
                assert_eq!(liquidity.max_liquidity_msat(), 1_000);
        }
@@ -2356,42 +2444,42 @@ mod tests {
 
                // Check initial bounds.
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 400);
                assert_eq!(liquidity.max_liquidity_msat(), 800);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 200);
                assert_eq!(liquidity.max_liquidity_msat(), 600);
 
                // Reset from source to target.
                scorer.channel_liquidities.get_mut(&42).unwrap()
-                       .as_directed_mut(&source, &target, 0, 1_000, decay_params)
+                       .as_directed_mut(&source, &target, 1_000, decay_params)
                        .set_max_liquidity_msat(300);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 0);
                assert_eq!(liquidity.max_liquidity_msat(), 300);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 700);
                assert_eq!(liquidity.max_liquidity_msat(), 1_000);
 
                // Reset from target to source.
                scorer.channel_liquidities.get_mut(&42).unwrap()
-                       .as_directed_mut(&target, &source, 0, 1_000, decay_params)
+                       .as_directed_mut(&target, &source, 1_000, decay_params)
                        .set_max_liquidity_msat(600);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 400);
                assert_eq!(liquidity.max_liquidity_msat(), 1_000);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&target, &source, 0, 1_000, decay_params);
+                       .as_directed(&target, &source, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 0);
                assert_eq!(liquidity.max_liquidity_msat(), 600);
        }
@@ -2720,7 +2808,7 @@ mod tests {
                let usage = ChannelUsage { amount_msat: 256, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 106);
                let usage = ChannelUsage { amount_msat: 768, ..usage };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 916);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 921);
                let usage = ChannelUsage { amount_msat: 896, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), u64::max_value());
 
@@ -2920,7 +3008,7 @@ mod tests {
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 300);
 
                SinceEpoch::advance(Duration::from_secs(10));
-               assert_eq!(deserialized_scorer.channel_penalty_msat(42, &source, &target, usage, &params), 365);
+               assert_eq!(deserialized_scorer.channel_penalty_msat(42, &source, &target, usage, &params), 370);
        }
 
        #[test]
@@ -2939,47 +3027,47 @@ mod tests {
                        inflight_htlc_msat: 0,
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 950_000_000, htlc_maximum_msat: 1_000 },
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4375);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 11497);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 2739);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 7408);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 2_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 2236);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 6151);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 3_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1983);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 5427);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 4_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1637);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4955);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 5_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1606);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4736);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 6_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1331);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4484);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 7_450_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1387);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4484);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 7_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1379);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4263);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 8_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1363);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4263);
                let usage = ChannelUsage {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 9_950_000_000, htlc_maximum_msat: 1_000 }, ..usage
                };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 1355);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 4044);
        }
 
        #[test]
@@ -3144,29 +3232,29 @@ mod tests {
                };
 
                // With no historical data the normal liquidity penalty calculation is used.
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 47);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 168);
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        None);
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42, &params),
                        None);
 
                scorer.payment_path_failed(&payment_path_for_amount(1), 42);
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 2048);
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage_1, &params), 128);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage_1, &params), 249);
                // The "it failed" increment is 32, where the probability should lie several buckets into
                // the first octile.
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        Some(([32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
                                [0, 0, 0, 0, 0, 0, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])));
-               assert!(scorer.historical_estimated_payment_success_probability(42, &target, 1)
+               assert!(scorer.historical_estimated_payment_success_probability(42, &target, 1, &params)
                        .unwrap() > 0.35);
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 500),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 500, &params),
                        Some(0.0));
 
                // Even after we tell the scorer we definitely have enough available liquidity, it will
                // still remember that there was some failure in the past, and assign a non-0 penalty.
                scorer.payment_path_failed(&payment_path_for_amount(1000), 43);
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 32);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 105);
                // The first points should be decayed just slightly and the last bucket has a new point.
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        Some(([31, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 0, 0, 0, 0, 0],
@@ -3175,23 +3263,23 @@ mod tests {
                // The exact success probability is a bit complicated and involves integer rounding, so we
                // simply check bounds here.
                let five_hundred_prob =
-                       scorer.historical_estimated_payment_success_probability(42, &target, 500).unwrap();
-               assert!(five_hundred_prob > 0.66);
-               assert!(five_hundred_prob < 0.68);
+                       scorer.historical_estimated_payment_success_probability(42, &target, 500, &params).unwrap();
+               assert!(five_hundred_prob > 0.59);
+               assert!(five_hundred_prob < 0.60);
                let one_prob =
-                       scorer.historical_estimated_payment_success_probability(42, &target, 1).unwrap();
-               assert!(one_prob < 1.0);
-               assert!(one_prob > 0.95);
+                       scorer.historical_estimated_payment_success_probability(42, &target, 1, &params).unwrap();
+               assert!(one_prob < 0.85);
+               assert!(one_prob > 0.84);
 
                // Advance the time forward 16 half-lives (which the docs claim will ensure all data is
                // gone), and check that we're back to where we started.
                SinceEpoch::advance(Duration::from_secs(10 * 16));
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 47);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 168);
                // Once fully decayed we still have data, but its all-0s. In the future we may remove the
                // data entirely instead.
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        None);
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 1), None);
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 1, &params), None);
 
                let mut usage = ChannelUsage {
                        amount_msat: 100,
@@ -3199,7 +3287,7 @@ mod tests {
                        effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: 1_024 },
                };
                scorer.payment_path_failed(&payment_path_for_amount(1), 42);
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 2048);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 2050);
                usage.inflight_htlc_msat = 0;
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 866);
 
@@ -3313,7 +3401,7 @@ mod tests {
                scorer.payment_path_failed(&path, 43);
 
                let liquidity = scorer.channel_liquidities.get(&42).unwrap()
-                       .as_directed(&source, &target, 0, 1_000, decay_params);
+                       .as_directed(&source, &target, 1_000, decay_params);
                assert_eq!(liquidity.min_liquidity_msat(), 256);
                assert_eq!(liquidity.max_liquidity_msat(), 768);
        }
@@ -3350,12 +3438,12 @@ mod tests {
                        inflight_htlc_msat: 0,
                        effective_capacity: EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: capacity_msat },
                };
-               // With no historical data the normal liquidity penalty calculation is used, which in this
-               // case is diminuitively low.
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage, &params), 0);
+               // 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(42, &source, &target, usage, &params), 1269);
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        None);
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, 42, &params),
                        None);
 
                // Fail to pay once, and then check the buckets and penalty.
@@ -3370,14 +3458,14 @@ mod tests {
                        Some(([32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
                                [0, 32, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])));
                // The success probability estimate itself should be zero.
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat, &params),
                        Some(0.0));
 
                // Now test again with the amount in the bottom bucket.
                amount_msat /= 2;
                // The new amount is entirely within the only minimum bucket with score, so the probability
                // we assign is 1/2.
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat, &params),
                        Some(0.5));
 
                // ...but once we see a failure, we consider the payment to be substantially less likely,
@@ -3387,7 +3475,7 @@ mod tests {
                assert_eq!(scorer.historical_estimated_channel_liquidity_probabilities(42, &target),
                        Some(([63, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
                                [32, 31, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])));
-               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat),
+               assert_eq!(scorer.historical_estimated_payment_success_probability(42, &target, amount_msat, &params),
                        Some(0.0));
        }
 }
index 372a094a931bed6dcca159e4bab310cdd7863a4b..431c62c9fb83de88830319e6b87b2f7370ac7f90 100644 (file)
@@ -174,8 +174,8 @@ impl<'a, A: KVStore, M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Der
 impl<ChannelSigner: WriteableEcdsaChannelSigner, K: KVStore> Persist<ChannelSigner> for K {
        // TODO: We really need a way for the persister to inform the user that its time to crash/shut
        // down once these start returning failure.
-       // A PermanentFailure implies we should probably just shut down the node since we're
-       // force-closing channels without even broadcasting!
+       // Then we should return InProgress rather than UnrecoverableError, implying we should probably
+       // just shut down the node since we're not retrying persistence!
 
        fn persist_new_channel(&self, funding_txo: OutPoint, monitor: &ChannelMonitor<ChannelSigner>, _update_id: MonitorUpdateId) -> chain::ChannelMonitorUpdateStatus {
                let key = format!("{}_{}", funding_txo.txid.to_hex(), funding_txo.index);
@@ -185,7 +185,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner, K: KVStore> Persist<ChannelSign
                        &key, &monitor.encode())
                {
                        Ok(()) => chain::ChannelMonitorUpdateStatus::Completed,
-                       Err(_) => chain::ChannelMonitorUpdateStatus::PermanentFailure,
+                       Err(_) => chain::ChannelMonitorUpdateStatus::UnrecoverableError
                }
        }
 
@@ -197,7 +197,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner, K: KVStore> Persist<ChannelSign
                        &key, &monitor.encode())
                {
                        Ok(()) => chain::ChannelMonitorUpdateStatus::Completed,
-                       Err(_) => chain::ChannelMonitorUpdateStatus::PermanentFailure,
+                       Err(_) => chain::ChannelMonitorUpdateStatus::UnrecoverableError
                }
        }
 }
index 785b40befd472ba10b30fb618bd75eb9ab20c15a..d53cd39b119fac08d8ebb151204d9c4e53a2a4a3 100644 (file)
@@ -225,7 +225,7 @@ impl<'a> TestChainMonitor<'a> {
        }
 }
 impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
-       fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> chain::ChannelMonitorUpdateStatus {
+       fn watch_channel(&self, funding_txo: OutPoint, monitor: channelmonitor::ChannelMonitor<TestChannelSigner>) -> Result<chain::ChannelMonitorUpdateStatus, ()> {
                // At every point where we get a monitor update, we should be able to send a useful monitor
                // to a watchtower and disk...
                let mut w = TestVecWriter(Vec::new());