Merge pull request #1611 from TheBlueMatt/2022-07-lower-bounded-estimator-nit
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 25 Jul 2022 21:11:07 +0000 (21:11 +0000)
committerGitHub <noreply@github.com>
Mon, 25 Jul 2022 21:11:07 +0000 (21:11 +0000)
Use a separate (non-trait) fee-estimation fn in LowerBoundedEstimator

24 files changed:
.github/dependabot.yml [deleted file]
.github/workflows/build.yml
lightning-background-processor/src/lib.rs
lightning-block-sync/src/poll.rs
lightning-net-tokio/src/lib.rs
lightning-rapid-gossip-sync/src/lib.rs
lightning-rapid-gossip-sync/src/processing.rs
lightning/src/chain/chaininterface.rs
lightning/src/chain/channelmonitor.rs
lightning/src/debug_sync.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/msgs.rs
lightning/src/ln/onion_route_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/ln/peer_handler.rs
lightning/src/ln/priv_short_conf_tests.rs
lightning/src/ln/reorg_tests.rs
lightning/src/routing/gossip.rs
lightning/src/routing/router.rs
lightning/src/routing/scoring.rs
lightning/src/util/test_utils.rs

diff --git a/.github/dependabot.yml b/.github/dependabot.yml
deleted file mode 100644 (file)
index c8c6daa..0000000
+++ /dev/null
@@ -1,36 +0,0 @@
-# https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates
-
-version: 2
-updates:
-  - package-ecosystem: "cargo" 
-    directory: "/" 
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning-background-processor" 
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning-block-sync" 
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning-invoice" 
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning-net-tokio"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning-persister"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "cargo" 
-    directory: "/lightning"
-    schedule:
-      interval: "daily"
-  - package-ecosystem: "github-actions" 
-    directory: "/"
-    schedule:
-      interval: "daily"
index 68cf376235f7edcb1e812da6027da9e608e97ed2..cfb1b9c847420dc0cb4ee0edb9188033a738e26f 100644 (file)
@@ -232,14 +232,14 @@ jobs:
           EXPECTED_ROUTING_GRAPH_SNAPSHOT_SHASUM: 05a5361278f68ee2afd086cc04a1f927a63924be451f3221d380533acfacc303
       - name: Fetch rapid graph sync reference input
         run: |
-          curl --verbose -L -o lightning-rapid-gossip-sync/res/full_graph.lngossip https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin
+          curl --verbose -L -o lightning-rapid-gossip-sync/res/full_graph.lngossip https://bitcoin.ninja/ldk-compressed_graph-285cb27df79-2022-07-21.bin
           echo "Sha sum: $(sha256sum lightning-rapid-gossip-sync/res/full_graph.lngossip | awk '{ print $1 }')"
           if [ "$(sha256sum lightning-rapid-gossip-sync/res/full_graph.lngossip | awk '{ print $1 }')" != "${EXPECTED_RAPID_GOSSIP_SHASUM}" ]; then
             echo "Bad hash"
             exit 1
           fi
         env:
-          EXPECTED_RAPID_GOSSIP_SHASUM: 9637b91cea9d64320cf48fc0787c70fe69fc062f90d3512e207044110cadfd7b
+          EXPECTED_RAPID_GOSSIP_SHASUM: e0f5d11641c11896d7af3a2246d3d6c3f1720b7d2d17aab321ecce82e6b7deb8
       - name: Test with Network Graph on Rust ${{ matrix.toolchain }}
         run: |
           cd lightning
index 11dca44bbab3d7b5c91585835ddc02ccf4baa393..484439b3907b364dddc3dc2c13bb6c078be2ad0c 100644 (file)
@@ -137,6 +137,55 @@ where A::Target: chain::Access, L::Target: Logger {
        }
 }
 
+/// (C-not exported) as the bindings concretize everything and have constructors for us
+impl<P: Deref<Target = P2PGossipSync<G, A, L>>, G: Deref<Target = NetworkGraph<L>>, A: Deref, L: Deref>
+       GossipSync<P, &RapidGossipSync<G, L>, G, A, L>
+where
+       A::Target: chain::Access,
+       L::Target: Logger,
+{
+       /// Initializes a new [`GossipSync::P2P`] variant.
+       pub fn p2p(gossip_sync: P) -> Self {
+               GossipSync::P2P(gossip_sync)
+       }
+}
+
+/// (C-not exported) as the bindings concretize everything and have constructors for us
+impl<'a, R: Deref<Target = RapidGossipSync<G, L>>, G: Deref<Target = NetworkGraph<L>>, L: Deref>
+       GossipSync<
+               &P2PGossipSync<G, &'a (dyn chain::Access + Send + Sync), L>,
+               R,
+               G,
+               &'a (dyn chain::Access + Send + Sync),
+               L,
+       >
+where
+       L::Target: Logger,
+{
+       /// Initializes a new [`GossipSync::Rapid`] variant.
+       pub fn rapid(gossip_sync: R) -> Self {
+               GossipSync::Rapid(gossip_sync)
+       }
+}
+
+/// (C-not exported) as the bindings concretize everything and have constructors for us
+impl<'a, L: Deref>
+       GossipSync<
+               &P2PGossipSync<&'a NetworkGraph<L>, &'a (dyn chain::Access + Send + Sync), L>,
+               &RapidGossipSync<&'a NetworkGraph<L>, L>,
+               &'a NetworkGraph<L>,
+               &'a (dyn chain::Access + Send + Sync),
+               L,
+       >
+where
+       L::Target: Logger,
+{
+       /// Initializes a new [`GossipSync::None`] variant.
+       pub fn none() -> Self {
+               GossipSync::None
+       }
+}
+
 /// Decorates an [`EventHandler`] with common functionality provided by standard [`EventHandler`]s.
 struct DecoratingEventHandler<
        'a,
index 6e30d2e86d20d268529bd37d942492db810d84b6..4c6cb0c0600725e9cf3552045865e8b5be472199 100644 (file)
@@ -59,12 +59,11 @@ impl Validate for BlockHeaderData {
        type T = ValidatedBlockHeader;
 
        fn validate(self, block_hash: BlockHash) -> BlockSourceResult<Self::T> {
-               self.header
+               let pow_valid_block_hash = self.header
                        .validate_pow(&self.header.target())
                        .or_else(|e| Err(BlockSourceError::persistent(e)))?;
 
-               // TODO: Use the result of validate_pow instead of recomputing the block hash once upstream.
-               if self.header.block_hash() != block_hash {
+               if pow_valid_block_hash != block_hash {
                        return Err(BlockSourceError::persistent("invalid block hash"));
                }
 
@@ -76,12 +75,11 @@ impl Validate for Block {
        type T = ValidatedBlock;
 
        fn validate(self, block_hash: BlockHash) -> BlockSourceResult<Self::T> {
-               self.header
+               let pow_valid_block_hash = self.header
                        .validate_pow(&self.header.target())
                        .or_else(|e| Err(BlockSourceError::persistent(e)))?;
 
-               // TODO: Use the result of validate_pow instead of recomputing the block hash once upstream.
-               if self.block_hash() != block_hash {
+               if pow_valid_block_hash != block_hash {
                        return Err(BlockSourceError::persistent("invalid block hash"));
                }
 
index 2ac10762b04eaf23f602bc3d8dcc987438542cae..645a7434e4536534138c5dd6150fe52bad55ff0c 100644 (file)
@@ -84,6 +84,7 @@ use lightning::ln::peer_handler::CustomMessageHandler;
 use lightning::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, NetAddress};
 use lightning::util::logger::Logger;
 
+use std::ops::Deref;
 use std::task;
 use std::net::SocketAddr;
 use std::net::TcpStream as StdTcpStream;
@@ -120,11 +121,16 @@ struct Connection {
        id: u64,
 }
 impl Connection {
-       async fn poll_event_process<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, Arc<CMH>, Arc<RMH>, Arc<L>, Arc<UMH>>>, mut event_receiver: mpsc::Receiver<()>) where
-                       CMH: ChannelMessageHandler + 'static + Send + Sync,
-                       RMH: RoutingMessageHandler + 'static + Send + Sync,
-                       L: Logger + 'static + ?Sized + Send + Sync,
-                       UMH: CustomMessageHandler + 'static + Send + Sync {
+       async fn poll_event_process<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, CMH, RMH, L, UMH>>, mut event_receiver: mpsc::Receiver<()>) where
+                       CMH: Deref + 'static + Send + Sync,
+                       RMH: Deref + 'static + Send + Sync,
+                       L: Deref + 'static + Send + Sync,
+                       UMH: Deref + 'static + Send + Sync,
+                       CMH::Target: ChannelMessageHandler + Send + Sync,
+                       RMH::Target: RoutingMessageHandler + Send + Sync,
+                       L::Target: Logger + Send + Sync,
+                       UMH::Target: CustomMessageHandler + Send + Sync,
+    {
                loop {
                        if event_receiver.recv().await.is_none() {
                                return;
@@ -133,11 +139,16 @@ impl Connection {
                }
        }
 
-       async fn schedule_read<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, Arc<CMH>, Arc<RMH>, Arc<L>, Arc<UMH>>>, us: Arc<Mutex<Self>>, mut reader: io::ReadHalf<TcpStream>, mut read_wake_receiver: mpsc::Receiver<()>, mut write_avail_receiver: mpsc::Receiver<()>) where
-                       CMH: ChannelMessageHandler + 'static + Send + Sync,
-                       RMH: RoutingMessageHandler + 'static + Send + Sync,
-                       L: Logger + 'static + ?Sized + Send + Sync,
-                       UMH: CustomMessageHandler + 'static + Send + Sync {
+       async fn schedule_read<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, CMH, RMH, L, UMH>>, us: Arc<Mutex<Self>>, mut reader: io::ReadHalf<TcpStream>, mut read_wake_receiver: mpsc::Receiver<()>, mut write_avail_receiver: mpsc::Receiver<()>) where
+                       CMH: Deref + 'static + Send + Sync,
+                       RMH: Deref + 'static + Send + Sync,
+                       L: Deref + 'static + Send + Sync,
+                       UMH: Deref + 'static + Send + Sync,
+                       CMH::Target: ChannelMessageHandler + 'static + Send + Sync,
+                       RMH::Target: RoutingMessageHandler + 'static + Send + Sync,
+                       L::Target: Logger + 'static + Send + Sync,
+                       UMH::Target: CustomMessageHandler + 'static + Send + Sync,
+        {
                // Create a waker to wake up poll_event_process, above
                let (event_waker, event_receiver) = mpsc::channel(1);
                tokio::spawn(Self::poll_event_process(Arc::clone(&peer_manager), event_receiver));
@@ -255,11 +266,16 @@ fn get_addr_from_stream(stream: &StdTcpStream) -> Option<NetAddress> {
 /// The returned future will complete when the peer is disconnected and associated handling
 /// futures are freed, though, because all processing futures are spawned with tokio::spawn, you do
 /// not need to poll the provided future in order to make progress.
-pub fn setup_inbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, Arc<CMH>, Arc<RMH>, Arc<L>, Arc<UMH>>>, stream: StdTcpStream) -> impl std::future::Future<Output=()> where
-               CMH: ChannelMessageHandler + 'static + Send + Sync,
-               RMH: RoutingMessageHandler + 'static + Send + Sync,
-               L: Logger + 'static + ?Sized + Send + Sync,
-               UMH: CustomMessageHandler + 'static + Send + Sync {
+pub fn setup_inbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, CMH, RMH, L, UMH>>, stream: StdTcpStream) -> impl std::future::Future<Output=()> where
+               CMH: Deref + 'static + Send + Sync,
+               RMH: Deref + 'static + Send + Sync,
+               L: Deref + 'static + Send + Sync,
+               UMH: Deref + 'static + Send + Sync,
+               CMH::Target: ChannelMessageHandler + Send + Sync,
+               RMH::Target: RoutingMessageHandler + Send + Sync,
+               L::Target: Logger + Send + Sync,
+               UMH::Target: CustomMessageHandler + Send + Sync,
+{
        let remote_addr = get_addr_from_stream(&stream);
        let (reader, write_receiver, read_receiver, us) = Connection::new(stream);
        #[cfg(debug_assertions)]
@@ -297,11 +313,16 @@ pub fn setup_inbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManag
 /// The returned future will complete when the peer is disconnected and associated handling
 /// futures are freed, though, because all processing futures are spawned with tokio::spawn, you do
 /// not need to poll the provided future in order to make progress.
-pub fn setup_outbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, Arc<CMH>, Arc<RMH>, Arc<L>, Arc<UMH>>>, their_node_id: PublicKey, stream: StdTcpStream) -> impl std::future::Future<Output=()> where
-               CMH: ChannelMessageHandler + 'static + Send + Sync,
-               RMH: RoutingMessageHandler + 'static + Send + Sync,
-               L: Logger + 'static + ?Sized + Send + Sync,
-               UMH: CustomMessageHandler + 'static + Send + Sync {
+pub fn setup_outbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, CMH, RMH, L, UMH>>, their_node_id: PublicKey, stream: StdTcpStream) -> impl std::future::Future<Output=()> where
+               CMH: Deref + 'static + Send + Sync,
+               RMH: Deref + 'static + Send + Sync,
+               L: Deref + 'static + Send + Sync,
+               UMH: Deref + 'static + Send + Sync,
+               CMH::Target: ChannelMessageHandler + Send + Sync,
+               RMH::Target: RoutingMessageHandler + Send + Sync,
+               L::Target: Logger + Send + Sync,
+               UMH::Target: CustomMessageHandler + Send + Sync,
+{
        let remote_addr = get_addr_from_stream(&stream);
        let (reader, mut write_receiver, read_receiver, us) = Connection::new(stream);
        #[cfg(debug_assertions)]
@@ -368,11 +389,16 @@ pub fn setup_outbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerMana
 /// disconnected and associated handling futures are freed, though, because all processing in said
 /// futures are spawned with tokio::spawn, you do not need to poll the second future in order to
 /// make progress.
-pub async fn connect_outbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, Arc<CMH>, Arc<RMH>, Arc<L>, Arc<UMH>>>, their_node_id: PublicKey, addr: SocketAddr) -> Option<impl std::future::Future<Output=()>> where
-               CMH: ChannelMessageHandler + 'static + Send + Sync,
-               RMH: RoutingMessageHandler + 'static + Send + Sync,
-               L: Logger + 'static + ?Sized + Send + Sync,
-               UMH: CustomMessageHandler + 'static + Send + Sync {
+pub async fn connect_outbound<CMH, RMH, L, UMH>(peer_manager: Arc<peer_handler::PeerManager<SocketDescriptor, CMH, RMH, L, UMH>>, their_node_id: PublicKey, addr: SocketAddr) -> Option<impl std::future::Future<Output=()>> where
+               CMH: Deref + 'static + Send + Sync,
+               RMH: Deref + 'static + Send + Sync,
+               L: Deref + 'static + Send + Sync,
+               UMH: Deref + 'static + Send + Sync,
+               CMH::Target: ChannelMessageHandler + Send + Sync,
+               RMH::Target: RoutingMessageHandler + Send + Sync,
+               L::Target: Logger + Send + Sync,
+               UMH::Target: CustomMessageHandler + Send + Sync,
+{
        if let Ok(Ok(stream)) = time::timeout(Duration::from_secs(10), async { TcpStream::connect(&addr).await.map(|s| s.into_std().unwrap()) }).await {
                Some(setup_outbound(peer_manager, their_node_id, stream))
        } else { None }
index 7880e11a60ea45c824dfceb3e75d6ee9d48610c8..6e9280f86a3f85909c8c25e49fc254bcc3a06819 100644 (file)
@@ -240,7 +240,7 @@ mod tests {
                let sync_result = rapid_sync
                        .sync_network_graph_with_file_path("./res/full_graph.lngossip");
                if let Err(crate::error::GraphSyncError::DecodeError(DecodeError::Io(io_error))) = &sync_result {
-                       let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-bc08df7542-2022-05-05.bin\n\n{:?}", io_error);
+                       let error_string = format!("Input file lightning-rapid-gossip-sync/res/full_graph.lngossip is missing! Download it from https://bitcoin.ninja/ldk-compressed_graph-285cb27df79-2022-07-21.bin\n\n{:?}", io_error);
                        #[cfg(not(require_route_graph_test))]
                        {
                                println!("{}", error_string);
index 09693a76d6f3b4868424d863ddd97399802974f9..818e19fe4b42fa6f12a31597b8ca4798eee2b5cb 100644 (file)
@@ -8,7 +8,7 @@ use bitcoin::BlockHash;
 use bitcoin::secp256k1::PublicKey;
 
 use lightning::ln::msgs::{
-       DecodeError, ErrorAction, LightningError, OptionalField, UnsignedChannelUpdate,
+       DecodeError, ErrorAction, LightningError, UnsignedChannelUpdate,
 };
 use lightning::routing::gossip::NetworkGraph;
 use lightning::util::logger::Logger;
@@ -119,12 +119,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
                let default_htlc_minimum_msat: u64 = Readable::read(&mut read_cursor)?;
                let default_fee_base_msat: u32 = Readable::read(&mut read_cursor)?;
                let default_fee_proportional_millionths: u32 = Readable::read(&mut read_cursor)?;
-               let tentative_default_htlc_maximum_msat: u64 = Readable::read(&mut read_cursor)?;
-               let default_htlc_maximum_msat = if tentative_default_htlc_maximum_msat == u64::max_value() {
-                       OptionalField::Absent
-               } else {
-                       OptionalField::Present(tentative_default_htlc_maximum_msat)
-               };
+               let default_htlc_maximum_msat: u64 = Readable::read(&mut read_cursor)?;
 
                for _ in 0..update_count {
                        let scid_delta: BigSize = Readable::read(read_cursor)?;
@@ -147,7 +142,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
                                        flags: standard_channel_flags,
                                        cltv_expiry_delta: default_cltv_expiry_delta,
                                        htlc_minimum_msat: default_htlc_minimum_msat,
-                                       htlc_maximum_msat: default_htlc_maximum_msat.clone(),
+                                       htlc_maximum_msat: default_htlc_maximum_msat,
                                        fee_base_msat: default_fee_base_msat,
                                        fee_proportional_millionths: default_fee_proportional_millionths,
                                        excess_data: vec![],
@@ -170,13 +165,6 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
                                                action: ErrorAction::IgnoreError,
                                        })?;
 
-                               let htlc_maximum_msat =
-                                       if let Some(htlc_maximum_msat) = directional_info.htlc_maximum_msat {
-                                               OptionalField::Present(htlc_maximum_msat)
-                                       } else {
-                                               OptionalField::Absent
-                                       };
-
                                UnsignedChannelUpdate {
                                        chain_hash,
                                        short_channel_id,
@@ -184,7 +172,7 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
                                        flags: standard_channel_flags,
                                        cltv_expiry_delta: directional_info.cltv_expiry_delta,
                                        htlc_minimum_msat: directional_info.htlc_minimum_msat,
-                                       htlc_maximum_msat,
+                                       htlc_maximum_msat: directional_info.htlc_maximum_msat,
                                        fee_base_msat: directional_info.fees.base_msat,
                                        fee_proportional_millionths: directional_info.fees.proportional_millionths,
                                        excess_data: vec![],
@@ -212,13 +200,8 @@ impl<NG: Deref<Target=NetworkGraph<L>>, L: Deref> RapidGossipSync<NG, L> where L
                        }
 
                        if channel_flags & 0b_0000_0100 > 0 {
-                               let tentative_htlc_maximum_msat: u64 = Readable::read(read_cursor)?;
-                               synthetic_update.htlc_maximum_msat = if tentative_htlc_maximum_msat == u64::max_value()
-                               {
-                                       OptionalField::Absent
-                               } else {
-                                       OptionalField::Present(tentative_htlc_maximum_msat)
-                               };
+                               let htlc_maximum_msat: u64 = Readable::read(read_cursor)?;
+                               synthetic_update.htlc_maximum_msat = htlc_maximum_msat;
                        }
 
                        network_graph.update_channel_unsigned(&synthetic_update)?;
index be86261f0c0300bafd1deb5d495838716acd1391..cbf609485ce1984800fa991e22d952e33ab5db56 100644 (file)
@@ -56,7 +56,7 @@ pub trait FeeEstimator {
 pub const MIN_RELAY_FEE_SAT_PER_1000_WEIGHT: u64 = 4000;
 /// Minimum feerate that takes a sane approach to bitcoind weight-to-vbytes rounding.
 /// See the following Core Lightning commit for an explanation:
-/// https://github.com/ElementsProject/lightning/commit/2e687b9b352c9092b5e8bd4a688916ac50b44af0
+/// <https://github.com/ElementsProject/lightning/commit/2e687b9b352c9092b5e8bd4a688916ac50b44af0>
 pub const FEERATE_FLOOR_SATS_PER_KW: u32 = 253;
 
 /// Wraps a `Deref` to a `FeeEstimator` so that any fee estimations provided by it
index f91af7cc66cd3c9a3ea70b07c9cb5aef82bcca86..8a6f02452ec1abb6c61812ac4bc1974e54a07df2 100644 (file)
@@ -965,6 +965,13 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 }
 
 impl<Signer: Sign> ChannelMonitor<Signer> {
+       /// For lockorder enforcement purposes, we need to have a single site which constructs the
+       /// `inner` mutex, otherwise cases where we lock two monitors at the same time (eg in our
+       /// PartialEq implementation) we may decide a lockorder violation has occurred.
+       fn from_impl(imp: ChannelMonitorImpl<Signer>) -> Self {
+               ChannelMonitor { inner: Mutex::new(imp) }
+       }
+
        pub(crate) fn new(secp_ctx: Secp256k1<secp256k1::All>, keys: Signer, shutdown_script: Option<Script>,
                          on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
                          channel_parameters: &ChannelTransactionParameters,
@@ -1012,60 +1019,58 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                let mut outputs_to_watch = HashMap::new();
                outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
 
-               ChannelMonitor {
-                       inner: Mutex::new(ChannelMonitorImpl {
-                               latest_update_id: 0,
-                               commitment_transaction_number_obscure_factor,
+               Self::from_impl(ChannelMonitorImpl {
+                       latest_update_id: 0,
+                       commitment_transaction_number_obscure_factor,
 
-                               destination_script: destination_script.clone(),
-                               broadcasted_holder_revokable_script: None,
-                               counterparty_payment_script,
-                               shutdown_script,
+                       destination_script: destination_script.clone(),
+                       broadcasted_holder_revokable_script: None,
+                       counterparty_payment_script,
+                       shutdown_script,
 
-                               channel_keys_id,
-                               holder_revocation_basepoint,
-                               funding_info,
-                               current_counterparty_commitment_txid: None,
-                               prev_counterparty_commitment_txid: None,
+                       channel_keys_id,
+                       holder_revocation_basepoint,
+                       funding_info,
+                       current_counterparty_commitment_txid: None,
+                       prev_counterparty_commitment_txid: None,
 
-                               counterparty_commitment_params,
-                               funding_redeemscript,
-                               channel_value_satoshis,
-                               their_cur_per_commitment_points: None,
+                       counterparty_commitment_params,
+                       funding_redeemscript,
+                       channel_value_satoshis,
+                       their_cur_per_commitment_points: None,
 
-                               on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
+                       on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
-                               commitment_secrets: CounterpartyCommitmentSecrets::new(),
-                               counterparty_claimable_outpoints: HashMap::new(),
-                               counterparty_commitment_txn_on_chain: HashMap::new(),
-                               counterparty_hash_commitment_number: HashMap::new(),
+                       commitment_secrets: CounterpartyCommitmentSecrets::new(),
+                       counterparty_claimable_outpoints: HashMap::new(),
+                       counterparty_commitment_txn_on_chain: HashMap::new(),
+                       counterparty_hash_commitment_number: HashMap::new(),
 
-                               prev_holder_signed_commitment_tx: None,
-                               current_holder_commitment_tx: holder_commitment_tx,
-                               current_counterparty_commitment_number: 1 << 48,
-                               current_holder_commitment_number,
+                       prev_holder_signed_commitment_tx: None,
+                       current_holder_commitment_tx: holder_commitment_tx,
+                       current_counterparty_commitment_number: 1 << 48,
+                       current_holder_commitment_number,
 
-                               payment_preimages: HashMap::new(),
-                               pending_monitor_events: Vec::new(),
-                               pending_events: Vec::new(),
+                       payment_preimages: HashMap::new(),
+                       pending_monitor_events: Vec::new(),
+                       pending_events: Vec::new(),
 
-                               onchain_events_awaiting_threshold_conf: Vec::new(),
-                               outputs_to_watch,
+                       onchain_events_awaiting_threshold_conf: Vec::new(),
+                       outputs_to_watch,
 
-                               onchain_tx_handler,
+                       onchain_tx_handler,
 
-                               lockdown_from_offchain: false,
-                               holder_tx_signed: false,
-                               funding_spend_seen: false,
-                               funding_spend_confirmed: None,
-                               htlcs_resolved_on_chain: Vec::new(),
+                       lockdown_from_offchain: false,
+                       holder_tx_signed: false,
+                       funding_spend_seen: false,
+                       funding_spend_confirmed: None,
+                       htlcs_resolved_on_chain: Vec::new(),
 
-                               best_block,
-                               counterparty_node_id: Some(counterparty_node_id),
+                       best_block,
+                       counterparty_node_id: Some(counterparty_node_id),
 
-                               secp_ctx,
-                       }),
-               }
+                       secp_ctx,
+               })
        }
 
        #[cfg(test)]
@@ -3361,60 +3366,58 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
-               Ok((best_block.block_hash(), ChannelMonitor {
-                       inner: Mutex::new(ChannelMonitorImpl {
-                               latest_update_id,
-                               commitment_transaction_number_obscure_factor,
+               Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl {
+                       latest_update_id,
+                       commitment_transaction_number_obscure_factor,
 
-                               destination_script,
-                               broadcasted_holder_revokable_script,
-                               counterparty_payment_script,
-                               shutdown_script,
+                       destination_script,
+                       broadcasted_holder_revokable_script,
+                       counterparty_payment_script,
+                       shutdown_script,
 
-                               channel_keys_id,
-                               holder_revocation_basepoint,
-                               funding_info,
-                               current_counterparty_commitment_txid,
-                               prev_counterparty_commitment_txid,
+                       channel_keys_id,
+                       holder_revocation_basepoint,
+                       funding_info,
+                       current_counterparty_commitment_txid,
+                       prev_counterparty_commitment_txid,
 
-                               counterparty_commitment_params,
-                               funding_redeemscript,
-                               channel_value_satoshis,
-                               their_cur_per_commitment_points,
+                       counterparty_commitment_params,
+                       funding_redeemscript,
+                       channel_value_satoshis,
+                       their_cur_per_commitment_points,
 
-                               on_holder_tx_csv,
+                       on_holder_tx_csv,
 
-                               commitment_secrets,
-                               counterparty_claimable_outpoints,
-                               counterparty_commitment_txn_on_chain,
-                               counterparty_hash_commitment_number,
+                       commitment_secrets,
+                       counterparty_claimable_outpoints,
+                       counterparty_commitment_txn_on_chain,
+                       counterparty_hash_commitment_number,
 
-                               prev_holder_signed_commitment_tx,
-                               current_holder_commitment_tx,
-                               current_counterparty_commitment_number,
-                               current_holder_commitment_number,
+                       prev_holder_signed_commitment_tx,
+                       current_holder_commitment_tx,
+                       current_counterparty_commitment_number,
+                       current_holder_commitment_number,
 
-                               payment_preimages,
-                               pending_monitor_events: pending_monitor_events.unwrap(),
-                               pending_events,
+                       payment_preimages,
+                       pending_monitor_events: pending_monitor_events.unwrap(),
+                       pending_events,
 
-                               onchain_events_awaiting_threshold_conf,
-                               outputs_to_watch,
+                       onchain_events_awaiting_threshold_conf,
+                       outputs_to_watch,
 
-                               onchain_tx_handler,
+                       onchain_tx_handler,
 
-                               lockdown_from_offchain,
-                               holder_tx_signed,
-                               funding_spend_seen: funding_spend_seen.unwrap(),
-                               funding_spend_confirmed,
-                               htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
+                       lockdown_from_offchain,
+                       holder_tx_signed,
+                       funding_spend_seen: funding_spend_seen.unwrap(),
+                       funding_spend_confirmed,
+                       htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
 
-                               best_block,
-                               counterparty_node_id,
+                       best_block,
+                       counterparty_node_id,
 
-                               secp_ctx,
-                       }),
-               }))
+                       secp_ctx,
+               })))
        }
 }
 
index 6b36682f43233f876dcacaf2e1ae34da0fef5e4e..daec309e9b5e35f81684ef9cdc5ed1df2be9e55c 100644 (file)
@@ -2,11 +2,9 @@ pub use ::alloc::sync::Arc;
 use core::ops::{Deref, DerefMut};
 use core::time::Duration;
 
-use std::collections::HashSet;
 use std::cell::RefCell;
 
 use std::sync::atomic::{AtomicUsize, Ordering};
-
 use std::sync::Mutex as StdMutex;
 use std::sync::MutexGuard as StdMutexGuard;
 use std::sync::RwLock as StdRwLock;
@@ -14,8 +12,15 @@ use std::sync::RwLockReadGuard as StdRwLockReadGuard;
 use std::sync::RwLockWriteGuard as StdRwLockWriteGuard;
 use std::sync::Condvar as StdCondvar;
 
+use prelude::HashMap;
+
 #[cfg(feature = "backtrace")]
-use backtrace::Backtrace;
+use {prelude::hash_map, backtrace::Backtrace, std::sync::Once};
+
+#[cfg(not(feature = "backtrace"))]
+struct Backtrace{}
+#[cfg(not(feature = "backtrace"))]
+impl Backtrace { fn new() -> Backtrace { Backtrace {} } }
 
 pub type LockResult<Guard> = Result<Guard, ()>;
 
@@ -44,34 +49,76 @@ impl Condvar {
 
 thread_local! {
        /// We track the set of locks currently held by a reference to their `LockMetadata`
-       static LOCKS_HELD: RefCell<HashSet<Arc<LockMetadata>>> = RefCell::new(HashSet::new());
+       static LOCKS_HELD: RefCell<HashMap<u64, Arc<LockMetadata>>> = RefCell::new(HashMap::new());
 }
 static LOCK_IDX: AtomicUsize = AtomicUsize::new(0);
 
+#[cfg(feature = "backtrace")]
+static mut LOCKS: Option<StdMutex<HashMap<String, Arc<LockMetadata>>>> = None;
+#[cfg(feature = "backtrace")]
+static LOCKS_INIT: Once = Once::new();
+
 /// Metadata about a single lock, by id, the set of things locked-before it, and the backtrace of
 /// when the Mutex itself was constructed.
 struct LockMetadata {
        lock_idx: u64,
-       locked_before: StdMutex<HashSet<Arc<LockMetadata>>>,
-       #[cfg(feature = "backtrace")]
-       lock_construction_bt: Backtrace,
+       locked_before: StdMutex<HashMap<u64, LockDep>>,
+       _lock_construction_bt: Backtrace,
 }
-impl PartialEq for LockMetadata {
-       fn eq(&self, o: &LockMetadata) -> bool { self.lock_idx == o.lock_idx }
+
+struct LockDep {
+       lock: Arc<LockMetadata>,
+       lockdep_trace: Backtrace,
 }
-impl Eq for LockMetadata {}
-impl std::hash::Hash for LockMetadata {
-       fn hash<H: std::hash::Hasher>(&self, hasher: &mut H) { hasher.write_u64(self.lock_idx); }
+
+#[cfg(feature = "backtrace")]
+fn get_construction_location(backtrace: &Backtrace) -> String {
+       // Find the first frame that is after `debug_sync` (or that is in our tests) and use
+       // that as the mutex construction site. Note that the first few frames may be in
+       // the `backtrace` crate, so we have to ignore those.
+       let sync_mutex_constr_regex = regex::Regex::new(r"lightning.*debug_sync.*new").unwrap();
+       let mut found_debug_sync = false;
+       for frame in backtrace.frames() {
+               for symbol in frame.symbols() {
+                       let symbol_name = symbol.name().unwrap().as_str().unwrap();
+                       if !sync_mutex_constr_regex.is_match(symbol_name) {
+                               if found_debug_sync {
+                                       if let Some(col) = symbol.colno() {
+                                               return format!("{}:{}:{}", symbol.filename().unwrap().display(), symbol.lineno().unwrap(), col);
+                                       } else {
+                                               // Windows debug symbols don't support column numbers, so fall back to
+                                               // line numbers only if no `colno` is available
+                                               return format!("{}:{}", symbol.filename().unwrap().display(), symbol.lineno().unwrap());
+                                       }
+                               }
+                       } else { found_debug_sync = true; }
+               }
+       }
+       panic!("Couldn't find mutex construction callsite");
 }
 
 impl LockMetadata {
-       fn new() -> LockMetadata {
-               LockMetadata {
-                       locked_before: StdMutex::new(HashSet::new()),
-                       lock_idx: LOCK_IDX.fetch_add(1, Ordering::Relaxed) as u64,
-                       #[cfg(feature = "backtrace")]
-                       lock_construction_bt: Backtrace::new(),
+       fn new() -> Arc<LockMetadata> {
+               let backtrace = Backtrace::new();
+               let lock_idx = LOCK_IDX.fetch_add(1, Ordering::Relaxed) as u64;
+
+               let res = Arc::new(LockMetadata {
+                       locked_before: StdMutex::new(HashMap::new()),
+                       lock_idx,
+                       _lock_construction_bt: backtrace,
+               });
+
+               #[cfg(feature = "backtrace")]
+               {
+                       let lock_constr_location = get_construction_location(&res._lock_construction_bt);
+                       LOCKS_INIT.call_once(|| { unsafe { LOCKS = Some(StdMutex::new(HashMap::new())); } });
+                       let mut locks = unsafe { LOCKS.as_ref() }.unwrap().lock().unwrap();
+                       match locks.entry(lock_constr_location) {
+                               hash_map::Entry::Occupied(e) => return Arc::clone(e.get()),
+                               hash_map::Entry::Vacant(e) => { e.insert(Arc::clone(&res)); },
+                       }
                }
+               res
        }
 
        // Returns whether we were a recursive lock (only relevant for read)
@@ -81,28 +128,37 @@ impl LockMetadata {
                        // For each lock which is currently locked, check that no lock's locked-before
                        // set includes the lock we're about to lock, which would imply a lockorder
                        // inversion.
-                       for locked in held.borrow().iter() {
-                               if read && *locked == *this {
+                       for (locked_idx, _locked) in held.borrow().iter() {
+                               if read && *locked_idx == this.lock_idx {
                                        // Recursive read locks are explicitly allowed
                                        return;
                                }
                        }
-                       for locked in held.borrow().iter() {
-                               if !read && *locked == *this {
-                                       panic!("Tried to lock a lock while it was held!");
+                       for (locked_idx, locked) in held.borrow().iter() {
+                               if !read && *locked_idx == this.lock_idx {
+                                       // With `feature = "backtrace"` set, we may be looking at different instances
+                                       // of the same lock.
+                                       debug_assert!(cfg!(feature = "backtrace"), "Tried to acquire a lock while it was held!");
                                }
-                               for locked_dep in locked.locked_before.lock().unwrap().iter() {
-                                       if *locked_dep == *this {
+                               for (locked_dep_idx, locked_dep) in locked.locked_before.lock().unwrap().iter() {
+                                       if *locked_dep_idx == this.lock_idx && *locked_dep_idx != locked.lock_idx {
                                                #[cfg(feature = "backtrace")]
-                                               panic!("Tried to violate existing lockorder.\nMutex that should be locked after the current lock was created at the following backtrace.\nNote that to get a backtrace for the lockorder violation, you should set RUST_BACKTRACE=1\n{:?}", locked.lock_construction_bt);
+                                               panic!("Tried to violate existing lockorder.\nMutex that should be locked after the current lock was created at the following backtrace.\nNote that to get a backtrace for the lockorder violation, you should set RUST_BACKTRACE=1\nLock being taken constructed at: {} ({}):\n{:?}\nLock constructed at: {} ({})\n{:?}\n\nLock dep created at:\n{:?}\n\n",
+                                                       get_construction_location(&this._lock_construction_bt), this.lock_idx, this._lock_construction_bt,
+                                                       get_construction_location(&locked._lock_construction_bt), locked.lock_idx, locked._lock_construction_bt,
+                                                       locked_dep.lockdep_trace);
                                                #[cfg(not(feature = "backtrace"))]
                                                panic!("Tried to violate existing lockorder. Build with the backtrace feature for more info.");
                                        }
                                }
                                // Insert any already-held locks in our locked-before set.
-                               this.locked_before.lock().unwrap().insert(Arc::clone(locked));
+                               let mut locked_before = this.locked_before.lock().unwrap();
+                               if !locked_before.contains_key(&locked.lock_idx) {
+                                       let lockdep = LockDep { lock: Arc::clone(locked), lockdep_trace: Backtrace::new() };
+                                       locked_before.insert(lockdep.lock.lock_idx, lockdep);
+                               }
                        }
-                       held.borrow_mut().insert(Arc::clone(this));
+                       held.borrow_mut().insert(this.lock_idx, Arc::clone(this));
                        inserted = true;
                });
                inserted
@@ -116,10 +172,14 @@ impl LockMetadata {
                        // Since a try-lock will simply fail if the lock is held already, we do not
                        // consider try-locks to ever generate lockorder inversions. However, if a try-lock
                        // succeeds, we do consider it to have created lockorder dependencies.
-                       for locked in held.borrow().iter() {
-                               this.locked_before.lock().unwrap().insert(Arc::clone(locked));
+                       let mut locked_before = this.locked_before.lock().unwrap();
+                       for (locked_idx, locked) in held.borrow().iter() {
+                               if !locked_before.contains_key(locked_idx) {
+                                       let lockdep = LockDep { lock: Arc::clone(locked), lockdep_trace: Backtrace::new() };
+                                       locked_before.insert(*locked_idx, lockdep);
+                               }
                        }
-                       held.borrow_mut().insert(Arc::clone(this));
+                       held.borrow_mut().insert(this.lock_idx, Arc::clone(this));
                });
        }
 }
@@ -149,7 +209,7 @@ impl<'a, T: Sized> MutexGuard<'a, T> {
 impl<T: Sized> Drop for MutexGuard<'_, T> {
        fn drop(&mut self) {
                LOCKS_HELD.with(|held| {
-                       held.borrow_mut().remove(&self.mutex.deps);
+                       held.borrow_mut().remove(&self.mutex.deps.lock_idx);
                });
        }
 }
@@ -170,7 +230,7 @@ impl<T: Sized> DerefMut for MutexGuard<'_, T> {
 
 impl<T> Mutex<T> {
        pub fn new(inner: T) -> Mutex<T> {
-               Mutex { inner: StdMutex::new(inner), deps: Arc::new(LockMetadata::new()) }
+               Mutex { inner: StdMutex::new(inner), deps: LockMetadata::new() }
        }
 
        pub fn lock<'a>(&'a self) -> LockResult<MutexGuard<'a, T>> {
@@ -220,7 +280,7 @@ impl<T: Sized> Drop for RwLockReadGuard<'_, T> {
                        return;
                }
                LOCKS_HELD.with(|held| {
-                       held.borrow_mut().remove(&self.lock.deps);
+                       held.borrow_mut().remove(&self.lock.deps.lock_idx);
                });
        }
 }
@@ -236,7 +296,7 @@ impl<T: Sized> Deref for RwLockWriteGuard<'_, T> {
 impl<T: Sized> Drop for RwLockWriteGuard<'_, T> {
        fn drop(&mut self) {
                LOCKS_HELD.with(|held| {
-                       held.borrow_mut().remove(&self.lock.deps);
+                       held.borrow_mut().remove(&self.lock.deps.lock_idx);
                });
        }
 }
@@ -249,7 +309,7 @@ impl<T: Sized> DerefMut for RwLockWriteGuard<'_, T> {
 
 impl<T> RwLock<T> {
        pub fn new(inner: T) -> RwLock<T> {
-               RwLock { inner: StdRwLock::new(inner), deps: Arc::new(LockMetadata::new()) }
+               RwLock { inner: StdRwLock::new(inner), deps: LockMetadata::new() }
        }
 
        pub fn read<'a>(&'a self) -> LockResult<RwLockReadGuard<'a, T>> {
@@ -271,96 +331,101 @@ impl<T> RwLock<T> {
        }
 }
 
-#[test]
-#[should_panic]
-fn recursive_lock_fail() {
-       let mutex = Mutex::new(());
-       let _a = mutex.lock().unwrap();
-       let _b = mutex.lock().unwrap();
-}
-
-#[test]
-fn recursive_read() {
-       let lock = RwLock::new(());
-       let _a = lock.read().unwrap();
-       let _b = lock.read().unwrap();
-}
+pub type FairRwLock<T> = RwLock<T>;
 
-#[test]
-#[should_panic]
-fn lockorder_fail() {
-       let a = Mutex::new(());
-       let b = Mutex::new(());
-       {
-               let _a = a.lock().unwrap();
-               let _b = b.lock().unwrap();
-       }
-       {
-               let _b = b.lock().unwrap();
-               let _a = a.lock().unwrap();
+mod tests {
+       use super::{RwLock, Mutex};
+
+       #[test]
+       #[should_panic]
+       #[cfg(not(feature = "backtrace"))]
+       fn recursive_lock_fail() {
+               let mutex = Mutex::new(());
+               let _a = mutex.lock().unwrap();
+               let _b = mutex.lock().unwrap();
+       }
+
+       #[test]
+       fn recursive_read() {
+               let lock = RwLock::new(());
+               let _a = lock.read().unwrap();
+               let _b = lock.read().unwrap();
+       }
+
+       #[test]
+       #[should_panic]
+       fn lockorder_fail() {
+               let a = Mutex::new(());
+               let b = Mutex::new(());
+               {
+                       let _a = a.lock().unwrap();
+                       let _b = b.lock().unwrap();
+               }
+               {
+                       let _b = b.lock().unwrap();
+                       let _a = a.lock().unwrap();
+               }
        }
-}
 
-#[test]
-#[should_panic]
-fn write_lockorder_fail() {
-       let a = RwLock::new(());
-       let b = RwLock::new(());
-       {
-               let _a = a.write().unwrap();
-               let _b = b.write().unwrap();
-       }
-       {
-               let _b = b.write().unwrap();
-               let _a = a.write().unwrap();
+       #[test]
+       #[should_panic]
+       fn write_lockorder_fail() {
+               let a = RwLock::new(());
+               let b = RwLock::new(());
+               {
+                       let _a = a.write().unwrap();
+                       let _b = b.write().unwrap();
+               }
+               {
+                       let _b = b.write().unwrap();
+                       let _a = a.write().unwrap();
+               }
        }
-}
 
-#[test]
-#[should_panic]
-fn read_lockorder_fail() {
-       let a = RwLock::new(());
-       let b = RwLock::new(());
-       {
-               let _a = a.read().unwrap();
-               let _b = b.read().unwrap();
-       }
-       {
-               let _b = b.read().unwrap();
-               let _a = a.read().unwrap();
+       #[test]
+       #[should_panic]
+       fn read_lockorder_fail() {
+               let a = RwLock::new(());
+               let b = RwLock::new(());
+               {
+                       let _a = a.read().unwrap();
+                       let _b = b.read().unwrap();
+               }
+               {
+                       let _b = b.read().unwrap();
+                       let _a = a.read().unwrap();
+               }
        }
-}
 
-#[test]
-fn read_recurisve_no_lockorder() {
-       // Like the above, but note that no lockorder is implied when we recursively read-lock a
-       // RwLock, causing this to pass just fine.
-       let a = RwLock::new(());
-       let b = RwLock::new(());
-       let _outer = a.read().unwrap();
-       {
-               let _a = a.read().unwrap();
-               let _b = b.read().unwrap();
-       }
-       {
-               let _b = b.read().unwrap();
-               let _a = a.read().unwrap();
+       #[test]
+       fn read_recursive_no_lockorder() {
+               // Like the above, but note that no lockorder is implied when we recursively read-lock a
+               // RwLock, causing this to pass just fine.
+               let a = RwLock::new(());
+               let b = RwLock::new(());
+               let _outer = a.read().unwrap();
+               {
+                       let _a = a.read().unwrap();
+                       let _b = b.read().unwrap();
+               }
+               {
+                       let _b = b.read().unwrap();
+                       let _a = a.read().unwrap();
+               }
        }
-}
 
-#[test]
-#[should_panic]
-fn read_write_lockorder_fail() {
-       let a = RwLock::new(());
-       let b = RwLock::new(());
-       {
-               let _a = a.write().unwrap();
-               let _b = b.read().unwrap();
-       }
-       {
-               let _b = b.read().unwrap();
-               let _a = a.write().unwrap();
+       #[test]
+       #[should_panic]
+       fn read_write_lockorder_fail() {
+               let a = RwLock::new(());
+               let b = RwLock::new(());
+               {
+                       let _a = a.write().unwrap();
+                       let _b = b.read().unwrap();
+               }
+               {
+                       let _b = b.read().unwrap();
+                       let _a = a.write().unwrap();
+               }
        }
 }
-
-pub type FairRwLock<T> = RwLock<T>;
index b698a919b01c4868f7ee0b7a65da9b718eb6a85a..972707a0bc3732f8729d29b96bf91165b2223231 100644 (file)
@@ -6572,7 +6572,7 @@ mod tests {
        use ln::channel::{Channel, InboundHTLCOutput, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator};
        use ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS};
        use ln::features::{InitFeatures, ChannelTypeFeatures};
-       use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate};
+       use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate, MAX_VALUE_MSAT};
        use ln::script::ShutdownScript;
        use ln::chan_utils;
        use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight};
@@ -6988,7 +6988,7 @@ mod tests {
                                flags: 0,
                                cltv_expiry_delta: 100,
                                htlc_minimum_msat: 5,
-                               htlc_maximum_msat: OptionalField::Absent,
+                               htlc_maximum_msat: MAX_VALUE_MSAT,
                                fee_base_msat: 110,
                                fee_proportional_millionths: 11,
                                excess_data: Vec::new(),
index 979cea395278cfa33a1981d205ae22f72beda7ac..56d705ef71e2ba64ec95a53e6d99dd9dd9eb775d 100644 (file)
@@ -48,7 +48,7 @@ use routing::router::{PaymentParameters, Route, RouteHop, RoutePath, RouteParame
 use ln::msgs;
 use ln::msgs::NetAddress;
 use ln::onion_utils;
-use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT, OptionalField};
+use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
 use ln::wire::Encode;
 use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner, Recipient};
 use util::config::{UserConfig, ChannelConfig};
@@ -2397,7 +2397,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                        flags: (!were_node_one) as u8 | ((!chan.is_live() as u8) << 1),
                        cltv_expiry_delta: chan.get_cltv_expiry_delta(),
                        htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(),
-                       htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()),
+                       htlc_maximum_msat: chan.get_announced_htlc_max_msat(),
                        fee_base_msat: chan.get_outbound_forwarding_fee_base_msat(),
                        fee_proportional_millionths: chan.get_fee_proportional_millionths(),
                        excess_data: Vec::new(),
@@ -3894,7 +3894,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        return;
                                }
                                mem::drop(channel_state_lock);
-                               let retry = if let Some(payment_params_data) = payment_params {
+                               let mut retry = if let Some(payment_params_data) = payment_params {
                                        let path_last_hop = path.last().expect("Outbound payments must have had a valid path");
                                        Some(RouteParameters {
                                                payment_params: payment_params_data.clone(),
@@ -3930,6 +3930,9 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        // TODO: If we decided to blame ourselves (or one of our channels) in
                                                        // process_onion_failure we should close that channel as it implies our
                                                        // next-hop is needlessly blaming us!
+                                                       if let Some(scid) = short_channel_id {
+                                                               retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
+                                                       }
                                                        events::Event::PaymentPathFailed {
                                                                payment_id: Some(payment_id),
                                                                payment_hash: payment_hash.clone(),
@@ -3959,6 +3962,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                // ChannelDetails.
                                                // TODO: For non-temporary failures, we really should be closing the
                                                // channel here as we apparently can't relay through them anyway.
+                                               let scid = path.first().unwrap().short_channel_id;
+                                               retry.as_mut().map(|r| r.payment_params.previously_failed_channels.push(scid));
                                                events::Event::PaymentPathFailed {
                                                        payment_id: Some(payment_id),
                                                        payment_hash: payment_hash.clone(),
@@ -3966,7 +3971,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                        network_update: None,
                                                        all_paths_failed,
                                                        path: path.clone(),
-                                                       short_channel_id: Some(path.first().unwrap().short_channel_id),
+                                                       short_channel_id: Some(scid),
                                                        retry,
 #[cfg(test)]
                                                        error_code: Some(*failure_code),
index fe78395e2f97e685698968b0053d44dbc8a772ce..cff632e4bc86b6a9e2a9f36b9b9787c0d3d6f8e7 100644 (file)
@@ -352,6 +352,11 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                                }
                        }
 
+                       let broadcaster = test_utils::TestBroadcaster {
+                               txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
+                               blocks: Arc::new(Mutex::new(self.tx_broadcaster.blocks.lock().unwrap().clone())),
+                       };
+
                        // Before using all the new monitors to check the watch outpoints, use the full set of
                        // them to ensure we can write and reload our ChannelManager.
                        {
@@ -367,20 +372,13 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
                                        keys_manager: self.keys_manager,
                                        fee_estimator: &test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) },
                                        chain_monitor: self.chain_monitor,
-                                       tx_broadcaster: &test_utils::TestBroadcaster {
-                                               txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
-                                               blocks: Arc::new(Mutex::new(self.tx_broadcaster.blocks.lock().unwrap().clone())),
-                                       },
+                                       tx_broadcaster: &broadcaster,
                                        logger: &self.logger,
                                        channel_monitors,
                                }).unwrap();
                        }
 
                        let persister = test_utils::TestPersister::new();
-                       let broadcaster = test_utils::TestBroadcaster {
-                               txn_broadcasted: Mutex::new(self.tx_broadcaster.txn_broadcasted.lock().unwrap().clone()),
-                               blocks: Arc::new(Mutex::new(self.tx_broadcaster.blocks.lock().unwrap().clone())),
-                       };
                        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(..) {
@@ -1492,7 +1490,7 @@ pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
        let mut events = node.node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        let expected_payment_id = match events.pop().unwrap() {
-               Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update,
+               Event::PaymentPathFailed { payment_hash, rejected_by_dest, path, retry, payment_id, network_update, short_channel_id,
                        #[cfg(test)]
                        error_code,
                        #[cfg(test)]
@@ -1502,6 +1500,9 @@ pub fn expect_payment_failed_conditions<'a, 'b, 'c, 'd, 'e>(
                        assert!(retry.is_some(), "expected retry.is_some()");
                        assert_eq!(retry.as_ref().unwrap().final_value_msat, path.last().unwrap().fee_msat, "Retry amount should match last hop in path");
                        assert_eq!(retry.as_ref().unwrap().payment_params.payee_pubkey, path.last().unwrap().pubkey, "Retry payee node_id should match last hop in path");
+                       if let Some(scid) = short_channel_id {
+                               assert!(retry.as_ref().unwrap().payment_params.previously_failed_channels.contains(&scid));
+                       }
 
                        #[cfg(test)]
                        {
index e58b541723c203329a01ea9085314a91e1b8625a..7572abff8313e90cdd149483a3ec5e919c864398 100644 (file)
@@ -28,7 +28,7 @@ use routing::gossip::NetworkGraph;
 use routing::router::{PaymentParameters, Route, RouteHop, RouteParameters, find_route, get_route};
 use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ErrorAction};
+use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction};
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::{byte_utils, test_utils};
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider, PaymentPurpose, ClosureReason};
@@ -1059,26 +1059,6 @@ fn fake_network_test() {
        fail_payment(&nodes[1], &vec!(&nodes[3], &nodes[2], &nodes[1])[..], payment_hash_2);
        claim_payment(&nodes[1], &vec!(&nodes[2], &nodes[3], &nodes[1])[..], payment_preimage_1);
 
-       // Add a duplicate new channel from 2 to 4
-       let chan_5 = create_announced_chan_between_nodes(&nodes, 1, 3, InitFeatures::known(), InitFeatures::known());
-
-       // Send some payments across both channels
-       let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
-       let payment_preimage_4 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
-       let payment_preimage_5 = route_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000).0;
-
-
-       route_over_limit(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], 3000000);
-       let events = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(events.len(), 0);
-       nodes[0].logger.assert_log_regex("lightning::ln::channelmanager".to_string(), regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap(), 1);
-
-       //TODO: Test that routes work again here as we've been notified that the channel is full
-
-       claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_3);
-       claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_4);
-       claim_payment(&nodes[0], &vec!(&nodes[1], &nodes[3])[..], payment_preimage_5);
-
        // Close down the channels...
        close_channel(&nodes[0], &nodes[1], &chan_1.2, chan_1.3, true);
        check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure);
@@ -1092,9 +1072,6 @@ fn fake_network_test() {
        close_channel(&nodes[1], &nodes[3], &chan_4.2, chan_4.3, false);
        check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
        check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure);
-       close_channel(&nodes[1], &nodes[3], &chan_5.2, chan_5.3, false);
-       check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure);
-       check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure);
 }
 
 #[test]
@@ -1824,9 +1801,12 @@ fn test_channel_reserve_holding_cell_htlcs() {
 
        // attempt to send amt_msat > their_max_htlc_value_in_flight_msat
        {
-               let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], recv_value_0);
+               let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+                       .with_features(InvoiceFeatures::known()).with_max_channel_saturation_power_of_half(0);
+               let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[2], payment_params, recv_value_0, TEST_FINAL_CLTV);
                route.paths[0].last_mut().unwrap().fee_msat += 1;
                assert!(route.paths[0].iter().rev().skip(1).all(|h| h.fee_msat == feemsat));
+
                unwrap_send_err!(nodes[0].node.send_payment(&route, our_payment_hash, &Some(our_payment_secret)), true, APIError::ChannelUnavailable { ref err },
                        assert!(regex::Regex::new(r"Cannot send value that would put us over the max HTLC value in flight our peer will accept \(\d+\)").unwrap().is_match(err)));
                assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
@@ -1845,7 +1825,12 @@ fn test_channel_reserve_holding_cell_htlcs() {
                if stat01.value_to_self_msat < stat01.channel_reserve_msat + commit_tx_fee_all_htlcs + ensure_htlc_amounts_above_dust_buffer + amt_msat {
                        break;
                }
-               send_payment(&nodes[0], &vec![&nodes[1], &nodes[2]][..], recv_value_0);
+
+               let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id())
+                       .with_features(InvoiceFeatures::known()).with_max_channel_saturation_power_of_half(0);
+               let route = get_route!(nodes[0], payment_params, recv_value_0, TEST_FINAL_CLTV).unwrap();
+               let (payment_preimage, ..) = send_along_route(&nodes[0], route, &[&nodes[1], &nodes[2]], recv_value_0);
+               claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage);
 
                let (stat01_, stat11_, stat12_, stat22_) = (
                        get_channel_value_stat!(nodes[0], chan_1.2),
@@ -3410,7 +3395,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
        check_added_monitors!(nodes[0], 1);
        check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
 
-       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(node_txn.len(), 3);
        assert_eq!(node_txn[0], node_txn[1]);
 
@@ -4828,7 +4813,7 @@ fn test_claim_on_remote_sizeable_push_msat() {
        check_added_monitors!(nodes[0], 1);
        check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed);
 
-       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(node_txn.len(), 1);
        check_spends!(node_txn[0], chan.3);
        assert_eq!(node_txn[0].output.len(), 2); // We can't force trimming of to_remote output as channel_reserve_satoshis block us to do so at channel opening
@@ -5034,7 +5019,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() {
        check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed);
        connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires
 
-       let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(revoked_htlc_txn.len(), 2);
        check_spends!(revoked_htlc_txn[0], chan_1.3);
        assert_eq!(revoked_htlc_txn[1].input.len(), 1);
@@ -7839,7 +7824,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() {
        check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
        connect_blocks(&nodes[1], 49); // Confirm blocks until the HTLC expires (note CLTV was explicitly 50 above)
 
-       let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+       let revoked_htlc_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(revoked_htlc_txn.len(), 3);
        check_spends!(revoked_htlc_txn[1], chan.3);
 
@@ -8100,22 +8085,26 @@ fn test_counterparty_raa_skip_no_crash() {
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
        let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 
-       let mut guard = nodes[0].node.channel_state.lock().unwrap();
-       let keys = guard.by_id.get_mut(&channel_id).unwrap().get_signer();
+       let per_commitment_secret;
+       let next_per_commitment_point;
+       {
+               let mut guard = nodes[0].node.channel_state.lock().unwrap();
+               let keys = guard.by_id.get_mut(&channel_id).unwrap().get_signer();
 
-       const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+               const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
 
-       // Make signer believe we got a counterparty signature, so that it allows the revocation
-       keys.get_enforcement_state().last_holder_commitment -= 1;
-       let per_commitment_secret = keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER);
+               // Make signer believe we got a counterparty signature, so that it allows the revocation
+               keys.get_enforcement_state().last_holder_commitment -= 1;
+               per_commitment_secret = keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER);
 
-       // Must revoke without gaps
-       keys.get_enforcement_state().last_holder_commitment -= 1;
-       keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
+               // Must revoke without gaps
+               keys.get_enforcement_state().last_holder_commitment -= 1;
+               keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
 
-       keys.get_enforcement_state().last_holder_commitment -= 1;
-       let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
-               &SecretKey::from_slice(&keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
+               keys.get_enforcement_state().last_holder_commitment -= 1;
+               next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
+                       &SecretKey::from_slice(&keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
+       }
 
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
                &msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });
@@ -8323,19 +8312,19 @@ fn test_channel_update_has_correct_htlc_maximum_msat() {
 
        // Assert that `node[0]`'s `ChannelUpdate` is capped at 50 percent of the `channel_value`, as
        // that's the value of `node[1]`'s `holder_max_htlc_value_in_flight_msat`.
-       assert_eq!(node_0_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_50_percent_msat));
+       assert_eq!(node_0_chan_update.contents.htlc_maximum_msat, channel_value_50_percent_msat);
        // Assert that `node[1]`'s `ChannelUpdate` is capped at 30 percent of the `channel_value`, as
        // that's the value of `node[0]`'s `holder_max_htlc_value_in_flight_msat`.
-       assert_eq!(node_1_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_30_percent_msat));
+       assert_eq!(node_1_chan_update.contents.htlc_maximum_msat, channel_value_30_percent_msat);
 
        // Assert that `node[2]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
        // the value of `node[3]`'s `holder_max_htlc_value_in_flight_msat` (100%), exceeds 90% of the
        // `channel_value`.
-       assert_eq!(node_2_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
+       assert_eq!(node_2_chan_update.contents.htlc_maximum_msat, channel_value_90_percent_msat);
        // Assert that `node[3]`'s `ChannelUpdate` is capped at 90 percent of the `channel_value`, as
        // the value of `node[2]`'s `holder_max_htlc_value_in_flight_msat` (95%), exceeds 90% of the
        // `channel_value`.
-       assert_eq!(node_3_chan_update.contents.htlc_maximum_msat, OptionalField::Present(channel_value_90_percent_msat));
+       assert_eq!(node_3_chan_update.contents.htlc_maximum_msat, channel_value_90_percent_msat);
 }
 
 #[test]
@@ -8457,12 +8446,12 @@ fn test_reject_funding_before_inbound_channel_accepted() {
        // `MessageSendEvent::SendAcceptChannel` event. The message is passed to `nodes[0]`
        // `handle_accept_channel`, which is required in order for `create_funding_transaction` to
        // succeed when `nodes[0]` is passed to it.
-       {
+       let accept_chan_msg = {
                let mut lock;
                let channel = get_channel_ref!(&nodes[1], lock, temp_channel_id);
-               let accept_chan_msg = channel.get_accept_channel_message();
-               nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_chan_msg);
-       }
+               channel.get_accept_channel_message()
+       };
+       nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), InitFeatures::known(), &accept_chan_msg);
 
        let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42);
 
index 3e44cfcf024034a7de39a614e50cc5a974cfc632..c2925d7354de4142c109ee79bf7b8c33fb0819ba 100644 (file)
@@ -647,8 +647,8 @@ pub struct UnsignedChannelUpdate {
        pub cltv_expiry_delta: u16,
        /// The minimum HTLC size incoming to sender, in milli-satoshi
        pub htlc_minimum_msat: u64,
-       /// Optionally, the maximum HTLC value incoming to sender, in milli-satoshi
-       pub htlc_maximum_msat: OptionalField<u64>,
+       /// The maximum HTLC value incoming to sender, in milli-satoshi. Used to be optional.
+       pub htlc_maximum_msat: u64,
        /// The base HTLC fee charged by sender, in milli-satoshi
        pub fee_base_msat: u32,
        /// The amount to fee multiplier, in micro-satoshi
@@ -1514,14 +1514,12 @@ impl_writeable!(ChannelAnnouncement, {
 
 impl Writeable for UnsignedChannelUpdate {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-               let mut message_flags: u8 = 0;
-               if let OptionalField::Present(_) = self.htlc_maximum_msat {
-                       message_flags = 1;
-               }
+               // `message_flags` used to indicate presence of `htlc_maximum_msat`, but was deprecated in the spec.
+               const MESSAGE_FLAGS: u8 = 1;
                self.chain_hash.write(w)?;
                self.short_channel_id.write(w)?;
                self.timestamp.write(w)?;
-               let all_flags = self.flags as u16 | ((message_flags as u16) << 8);
+               let all_flags = self.flags as u16 | ((MESSAGE_FLAGS as u16) << 8);
                all_flags.write(w)?;
                self.cltv_expiry_delta.write(w)?;
                self.htlc_minimum_msat.write(w)?;
@@ -1535,22 +1533,20 @@ impl Writeable for UnsignedChannelUpdate {
 
 impl Readable for UnsignedChannelUpdate {
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let has_htlc_maximum_msat;
                Ok(Self {
                        chain_hash: Readable::read(r)?,
                        short_channel_id: Readable::read(r)?,
                        timestamp: Readable::read(r)?,
                        flags: {
                                let flags: u16 = Readable::read(r)?;
-                               let message_flags = flags >> 8;
-                               has_htlc_maximum_msat = (message_flags as i32 & 1) == 1;
+                               // Note: we ignore the `message_flags` for now, since it was deprecated by the spec.
                                flags as u8
                        },
                        cltv_expiry_delta: Readable::read(r)?,
                        htlc_minimum_msat: Readable::read(r)?,
                        fee_base_msat: Readable::read(r)?,
                        fee_proportional_millionths: Readable::read(r)?,
-                       htlc_maximum_msat: if has_htlc_maximum_msat { Readable::read(r)? } else { OptionalField::Absent },
+                       htlc_maximum_msat: Readable::read(r)?,
                        excess_data: read_to_end(r)?,
                })
        }
@@ -2103,7 +2099,7 @@ mod tests {
                do_encoding_node_announcement(false, false, true, false, true, false, false, false);
        }
 
-       fn do_encoding_channel_update(direction: bool, disable: bool, htlc_maximum_msat: bool, excess_data: bool) {
+       fn do_encoding_channel_update(direction: bool, disable: bool, excess_data: bool) {
                let secp_ctx = Secp256k1::new();
                let (privkey_1, _) = get_keys_from!("0101010101010101010101010101010101010101010101010101010101010101", secp_ctx);
                let sig_1 = get_sig_on!(privkey_1, secp_ctx, String::from("01010101010101010101010101010101"));
@@ -2114,7 +2110,7 @@ mod tests {
                        flags: if direction { 1 } else { 0 } | if disable { 1 << 1 } else { 0 },
                        cltv_expiry_delta: 144,
                        htlc_minimum_msat: 1000000,
-                       htlc_maximum_msat: if htlc_maximum_msat { OptionalField::Present(131355275467161) } else { OptionalField::Absent },
+                       htlc_maximum_msat: 131355275467161,
                        fee_base_msat: 10000,
                        fee_proportional_millionths: 20,
                        excess_data: if excess_data { vec![0, 0, 0, 0, 59, 154, 202, 0] } else { Vec::new() }
@@ -2127,11 +2123,7 @@ mod tests {
                let mut target_value = hex::decode("d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a").unwrap();
                target_value.append(&mut hex::decode("000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f").unwrap());
                target_value.append(&mut hex::decode("00083a840000034d013413a7").unwrap());
-               if htlc_maximum_msat {
-                       target_value.append(&mut hex::decode("01").unwrap());
-               } else {
-                       target_value.append(&mut hex::decode("00").unwrap());
-               }
+               target_value.append(&mut hex::decode("01").unwrap());
                target_value.append(&mut hex::decode("00").unwrap());
                if direction {
                        let flag = target_value.last_mut().unwrap();
@@ -2142,9 +2134,7 @@ mod tests {
                        *flag = *flag | 1 << 1;
                }
                target_value.append(&mut hex::decode("009000000000000f42400000271000000014").unwrap());
-               if htlc_maximum_msat {
-                       target_value.append(&mut hex::decode("0000777788889999").unwrap());
-               }
+               target_value.append(&mut hex::decode("0000777788889999").unwrap());
                if excess_data {
                        target_value.append(&mut hex::decode("000000003b9aca00").unwrap());
                }
@@ -2153,16 +2143,14 @@ mod tests {
 
        #[test]
        fn encoding_channel_update() {
-               do_encoding_channel_update(false, false, false, false);
-               do_encoding_channel_update(false, false, false, true);
-               do_encoding_channel_update(true, false, false, false);
-               do_encoding_channel_update(true, false, false, true);
-               do_encoding_channel_update(false, true, false, false);
-               do_encoding_channel_update(false, true, false, true);
-               do_encoding_channel_update(false, false, true, false);
-               do_encoding_channel_update(false, false, true, true);
-               do_encoding_channel_update(true, true, true, false);
-               do_encoding_channel_update(true, true, true, true);
+               do_encoding_channel_update(false, false, false);
+               do_encoding_channel_update(false, false, true);
+               do_encoding_channel_update(true, false, false);
+               do_encoding_channel_update(true, false, true);
+               do_encoding_channel_update(false, true, false);
+               do_encoding_channel_update(false, true, true);
+               do_encoding_channel_update(true, true, false);
+               do_encoding_channel_update(true, true, true);
        }
 
        fn do_encoding_open_channel(random_bit: bool, shutdown: bool, incl_chan_type: bool) {
index c66f45a44923b596e1e31ac0a08d2ee531826128..ed1516efcb01f2c7955bd1285e476158225d58f8 100644 (file)
@@ -21,7 +21,7 @@ use routing::gossip::{NetworkUpdate, RoutingFees, NodeId};
 use routing::router::{get_route, PaymentParameters, Route, RouteHint, RouteHintHop};
 use ln::features::{InitFeatures, InvoiceFeatures, NodeFeatures};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, ChannelUpdate, OptionalField};
+use ln::msgs::{ChannelMessageHandler, ChannelUpdate};
 use ln::wire::Encode;
 use util::events::{Event, MessageSendEvent, MessageSendEventsProvider};
 use util::ser::{ReadableArgs, Writeable, Writer};
@@ -227,7 +227,7 @@ impl msgs::ChannelUpdate {
                                flags: 0,
                                cltv_expiry_delta: 0,
                                htlc_minimum_msat: 0,
-                               htlc_maximum_msat: OptionalField::Absent,
+                               htlc_maximum_msat: msgs::MAX_VALUE_MSAT,
                                fee_base_msat: 0,
                                fee_proportional_millionths: 0,
                                excess_data: vec![],
index 99c8682738da55467b5866f0c8c751047ff2c001..d3feb96df111766d9625953a8765c1b0a2ff9e8b 100644 (file)
@@ -942,7 +942,7 @@ fn failed_probe_yields_event() {
 
        let payment_params = PaymentParameters::from_node_id(nodes[2].node.get_our_node_id());
 
-       let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_999_000, 42);
+       let (route, _, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[2], &payment_params, 9_998_000, 42);
 
        let (payment_hash, payment_id) = nodes[0].node.send_probe(route.paths[0].clone()).unwrap();
 
index 2a026821f8adf91a9f0d2ba9bfdacbc861e1db48..f8339f0eae9e6942b95e6c038504eda669aea423 100644 (file)
@@ -1926,11 +1926,18 @@ mod tests {
                peer_a.new_inbound_connection(fd_a.clone(), None).unwrap();
                assert_eq!(peer_a.read_event(&mut fd_a, &initial_data).unwrap(), false);
                peer_a.process_events();
-               assert_eq!(peer_b.read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+
+               let a_data = fd_a.outbound_data.lock().unwrap().split_off(0);
+               assert_eq!(peer_b.read_event(&mut fd_b, &a_data).unwrap(), false);
+
                peer_b.process_events();
-               assert_eq!(peer_a.read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+               let b_data = fd_b.outbound_data.lock().unwrap().split_off(0);
+               assert_eq!(peer_a.read_event(&mut fd_a, &b_data).unwrap(), false);
+
                peer_a.process_events();
-               assert_eq!(peer_b.read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+               let a_data = fd_a.outbound_data.lock().unwrap().split_off(0);
+               assert_eq!(peer_b.read_event(&mut fd_b, &a_data).unwrap(), false);
+
                (fd_a.clone(), fd_b.clone())
        }
 
@@ -2084,14 +2091,16 @@ mod tests {
 
                assert_eq!(peers[0].read_event(&mut fd_a, &initial_data).unwrap(), false);
                peers[0].process_events();
-               assert_eq!(peers[1].read_event(&mut fd_b, &fd_a.outbound_data.lock().unwrap().split_off(0)).unwrap(), false);
+               let a_data = fd_a.outbound_data.lock().unwrap().split_off(0);
+               assert_eq!(peers[1].read_event(&mut fd_b, &a_data).unwrap(), false);
                peers[1].process_events();
 
                // ...but if we get a second timer tick, we should disconnect the peer
                peers[0].timer_tick_occurred();
                assert_eq!(peers[0].peers.read().unwrap().len(), 0);
 
-               assert!(peers[0].read_event(&mut fd_a, &fd_b.outbound_data.lock().unwrap().split_off(0)).is_err());
+               let b_data = fd_b.outbound_data.lock().unwrap().split_off(0);
+               assert!(peers[0].read_event(&mut fd_a, &b_data).is_err());
        }
 
        #[test]
index 508ba414037107b260938b6c82bdbf2ad107dd4a..d5dd51cac4e86b83b77a38d22874a8915e9c398d 100644 (file)
@@ -19,7 +19,7 @@ use routing::gossip::RoutingFees;
 use routing::router::{PaymentParameters, RouteHint, RouteHintHop};
 use ln::features::{InitFeatures, InvoiceFeatures, ChannelTypeFeatures};
 use ln::msgs;
-use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, OptionalField, ChannelUpdate, ErrorAction};
+use ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ChannelUpdate, ErrorAction};
 use ln::wire::Encode;
 use util::enforcing_trait_impls::EnforcingSigner;
 use util::events::{ClosureReason, Event, MessageSendEvent, MessageSendEventsProvider};
@@ -523,7 +523,7 @@ fn test_scid_alias_returned() {
                flags: 1,
                cltv_expiry_delta: accept_forward_cfg.channel_config.cltv_expiry_delta,
                htlc_minimum_msat: 1_000,
-               htlc_maximum_msat: OptionalField::Present(1_000_000), // Defaults to 10% of the channel value
+               htlc_maximum_msat: 1_000_000, // Defaults to 10% of the channel value
                fee_base_msat: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_base_msat,
                fee_proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths,
                excess_data: Vec::new(),
index 29349392f761438b311cca2eb98f49ba14bd338f..561fa8104694985f321c692a4cf949c7ef6ddddb 100644 (file)
@@ -82,7 +82,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) {
                check_added_monitors!(nodes[2], 1);
                check_closed_broadcast!(nodes[2], true); // We should get a BroadcastChannelUpdate (and *only* a BroadcstChannelUpdate)
                check_closed_event!(nodes[2], 1, ClosureReason::CommitmentTxConfirmed);
-               let node_2_commitment_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap();
+               let node_2_commitment_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
                assert_eq!(node_2_commitment_txn.len(), 3); // ChannelMonitor: 1 offered HTLC-Claim, ChannelManger: 1 local commitment tx, 1 Received HTLC-Claim
                assert_eq!(node_2_commitment_txn[1].output.len(), 2); // to-remote and Received HTLC (to-self is dust)
                check_spends!(node_2_commitment_txn[1], chan_2.3);
index 716ca2b305f3a48da3b5c698c571b11fed2fcdd0..37bf1ea5a6f9757d1da5a29f636aa0a853f2de8e 100644 (file)
@@ -25,15 +25,16 @@ use chain;
 use chain::Access;
 use ln::features::{ChannelFeatures, NodeFeatures};
 use ln::msgs::{DecodeError, ErrorAction, Init, LightningError, RoutingMessageHandler, NetAddress, MAX_VALUE_MSAT};
-use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, OptionalField, GossipTimestampFilter};
+use ln::msgs::{ChannelAnnouncement, ChannelUpdate, NodeAnnouncement, GossipTimestampFilter};
 use ln::msgs::{QueryChannelRange, ReplyChannelRange, QueryShortChannelIds, ReplyShortChannelIdsEnd};
 use ln::msgs;
-use util::ser::{Readable, ReadableArgs, Writeable, Writer};
+use util::ser::{Readable, ReadableArgs, Writeable, Writer, MaybeReadable};
 use util::logger::{Logger, Level};
 use util::events::{Event, EventHandler, MessageSendEvent, MessageSendEventsProvider};
 use util::scid_utils::{block_from_scid, scid_from_parts, MAX_SCID_BLOCK};
 
 use io;
+use io_extras::{copy, sink};
 use prelude::*;
 use alloc::collections::{BTreeMap, btree_map::Entry as BtreeEntry};
 use core::{cmp, fmt};
@@ -611,7 +612,7 @@ pub struct ChannelUpdateInfo {
        /// The minimum value, which must be relayed to the next hop via the channel
        pub htlc_minimum_msat: u64,
        /// The maximum value which may be relayed to the next hop via the channel.
-       pub htlc_maximum_msat: Option<u64>,
+       pub htlc_maximum_msat: u64,
        /// Fees charged when the channel is used for routing
        pub fees: RoutingFees,
        /// Most recent update for the channel received from the network
@@ -628,15 +629,58 @@ impl fmt::Display for ChannelUpdateInfo {
        }
 }
 
-impl_writeable_tlv_based!(ChannelUpdateInfo, {
-       (0, last_update, required),
-       (2, enabled, required),
-       (4, cltv_expiry_delta, required),
-       (6, htlc_minimum_msat, required),
-       (8, htlc_maximum_msat, required),
-       (10, fees, required),
-       (12, last_update_message, required),
-});
+impl Writeable for ChannelUpdateInfo {
+       fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               write_tlv_fields!(writer, {
+                       (0, self.last_update, required),
+                       (2, self.enabled, required),
+                       (4, self.cltv_expiry_delta, required),
+                       (6, self.htlc_minimum_msat, required),
+                       // Writing htlc_maximum_msat as an Option<u64> is required to maintain backwards
+                       // compatibility with LDK versions prior to v0.0.110.
+                       (8, Some(self.htlc_maximum_msat), required),
+                       (10, self.fees, required),
+                       (12, self.last_update_message, required),
+               });
+               Ok(())
+       }
+}
+
+impl Readable for ChannelUpdateInfo {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               init_tlv_field_var!(last_update, required);
+               init_tlv_field_var!(enabled, required);
+               init_tlv_field_var!(cltv_expiry_delta, required);
+               init_tlv_field_var!(htlc_minimum_msat, required);
+               init_tlv_field_var!(htlc_maximum_msat, option);
+               init_tlv_field_var!(fees, required);
+               init_tlv_field_var!(last_update_message, required);
+
+               read_tlv_fields!(reader, {
+                       (0, last_update, required),
+                       (2, enabled, required),
+                       (4, cltv_expiry_delta, required),
+                       (6, htlc_minimum_msat, required),
+                       (8, htlc_maximum_msat, required),
+                       (10, fees, required),
+                       (12, last_update_message, required)
+               });
+
+               if let Some(htlc_maximum_msat) = htlc_maximum_msat {
+                       Ok(ChannelUpdateInfo {
+                               last_update: init_tlv_based_struct_field!(last_update, required),
+                               enabled: init_tlv_based_struct_field!(enabled, required),
+                               cltv_expiry_delta: init_tlv_based_struct_field!(cltv_expiry_delta, required),
+                               htlc_minimum_msat: init_tlv_based_struct_field!(htlc_minimum_msat, required),
+                               htlc_maximum_msat,
+                               fees: init_tlv_based_struct_field!(fees, required),
+                               last_update_message: init_tlv_based_struct_field!(last_update_message, required),
+                       })
+               } else {
+                       Err(DecodeError::InvalidValue)
+               }
+       }
+}
 
 #[derive(Clone, Debug, PartialEq)]
 /// Details about a channel (both directions).
@@ -715,16 +759,73 @@ impl fmt::Display for ChannelInfo {
        }
 }
 
-impl_writeable_tlv_based!(ChannelInfo, {
-       (0, features, required),
-       (1, announcement_received_time, (default_value, 0)),
-       (2, node_one, required),
-       (4, one_to_two, required),
-       (6, node_two, required),
-       (8, two_to_one, required),
-       (10, capacity_sats, required),
-       (12, announcement_message, required),
-});
+impl Writeable for ChannelInfo {
+       fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               write_tlv_fields!(writer, {
+                       (0, self.features, required),
+                       (1, self.announcement_received_time, (default_value, 0)),
+                       (2, self.node_one, required),
+                       (4, self.one_to_two, required),
+                       (6, self.node_two, required),
+                       (8, self.two_to_one, required),
+                       (10, self.capacity_sats, required),
+                       (12, self.announcement_message, required),
+               });
+               Ok(())
+       }
+}
+
+// A wrapper allowing for the optional deseralization of ChannelUpdateInfo. Utilizing this is
+// necessary to maintain backwards compatibility with previous serializations of `ChannelUpdateInfo`
+// that may have no `htlc_maximum_msat` field set. In case the field is absent, we simply ignore
+// the error and continue reading the `ChannelInfo`. Hopefully, we'll then eventually receive newer
+// channel updates via the gossip network.
+struct ChannelUpdateInfoDeserWrapper(Option<ChannelUpdateInfo>);
+
+impl MaybeReadable for ChannelUpdateInfoDeserWrapper {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+               match ::util::ser::Readable::read(reader) {
+                       Ok(channel_update_option) => Ok(Some(Self(channel_update_option))),
+                       Err(DecodeError::ShortRead) => Ok(None),
+                       Err(DecodeError::InvalidValue) => Ok(None),
+                       Err(err) => Err(err),
+               }
+       }
+}
+
+impl Readable for ChannelInfo {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               init_tlv_field_var!(features, required);
+               init_tlv_field_var!(announcement_received_time, (default_value, 0));
+               init_tlv_field_var!(node_one, required);
+               let mut one_to_two_wrap: Option<ChannelUpdateInfoDeserWrapper> = None;
+               init_tlv_field_var!(node_two, required);
+               let mut two_to_one_wrap: Option<ChannelUpdateInfoDeserWrapper> = None;
+               init_tlv_field_var!(capacity_sats, required);
+               init_tlv_field_var!(announcement_message, required);
+               read_tlv_fields!(reader, {
+                       (0, features, required),
+                       (1, announcement_received_time, (default_value, 0)),
+                       (2, node_one, required),
+                       (4, one_to_two_wrap, ignorable),
+                       (6, node_two, required),
+                       (8, two_to_one_wrap, ignorable),
+                       (10, capacity_sats, required),
+                       (12, announcement_message, required),
+               });
+
+               Ok(ChannelInfo {
+                       features: init_tlv_based_struct_field!(features, required),
+                       node_one: init_tlv_based_struct_field!(node_one, required),
+                       one_to_two: one_to_two_wrap.map(|w| w.0).unwrap_or(None),
+                       node_two: init_tlv_based_struct_field!(node_two, required),
+                       two_to_one: two_to_one_wrap.map(|w| w.0).unwrap_or(None),
+                       capacity_sats: init_tlv_based_struct_field!(capacity_sats, required),
+                       announcement_message: init_tlv_based_struct_field!(announcement_message, required),
+                       announcement_received_time: init_tlv_based_struct_field!(announcement_received_time, (default_value, 0)),
+               })
+       }
+}
 
 /// A wrapper around [`ChannelInfo`] representing information about the channel as directed from a
 /// source node to a target node.
@@ -739,7 +840,7 @@ pub struct DirectedChannelInfo<'a> {
 impl<'a> DirectedChannelInfo<'a> {
        #[inline]
        fn new(channel: &'a ChannelInfo, direction: Option<&'a ChannelUpdateInfo>) -> Self {
-               let htlc_maximum_msat = direction.and_then(|direction| direction.htlc_maximum_msat);
+               let htlc_maximum_msat = direction.map(|direction| direction.htlc_maximum_msat);
                let capacity_msat = channel.capacity_sats.map(|capacity_sats| capacity_sats * 1000);
 
                let (htlc_maximum_msat, effective_capacity) = match (htlc_maximum_msat, capacity_msat) {
@@ -816,10 +917,6 @@ impl<'a> DirectedChannelInfoWithUpdate<'a> {
        /// Returns the [`EffectiveCapacity`] of the channel in the direction.
        #[inline]
        pub(super) fn effective_capacity(&self) -> EffectiveCapacity { self.inner.effective_capacity() }
-
-       /// Returns the maximum HTLC amount allowed over the channel in the direction.
-       #[inline]
-       pub(super) fn htlc_maximum_msat(&self) -> u64 { self.inner.htlc_maximum_msat() }
 }
 
 impl<'a> fmt::Debug for DirectedChannelInfoWithUpdate<'a> {
@@ -993,11 +1090,54 @@ impl fmt::Display for NodeInfo {
        }
 }
 
-impl_writeable_tlv_based!(NodeInfo, {
-       (0, lowest_inbound_channel_fees, option),
-       (2, announcement_info, option),
-       (4, channels, vec_type),
-});
+impl Writeable for NodeInfo {
+       fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               write_tlv_fields!(writer, {
+                       (0, self.lowest_inbound_channel_fees, option),
+                       (2, self.announcement_info, option),
+                       (4, self.channels, vec_type),
+               });
+               Ok(())
+       }
+}
+
+// A wrapper allowing for the optional deseralization of `NodeAnnouncementInfo`. Utilizing this is
+// necessary to maintain compatibility with previous serializations of `NetAddress` that have an
+// invalid hostname set. We ignore and eat all errors until we are either able to read a
+// `NodeAnnouncementInfo` or hit a `ShortRead`, i.e., read the TLV field to the end.
+struct NodeAnnouncementInfoDeserWrapper(NodeAnnouncementInfo);
+
+impl MaybeReadable for NodeAnnouncementInfoDeserWrapper {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+               match ::util::ser::Readable::read(reader) {
+                       Ok(node_announcement_info) => return Ok(Some(Self(node_announcement_info))),
+                       Err(_) => {
+                               copy(reader, &mut sink()).unwrap();
+                               return Ok(None)
+                       },
+               };
+       }
+}
+
+impl Readable for NodeInfo {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               init_tlv_field_var!(lowest_inbound_channel_fees, option);
+               let mut announcement_info_wrap: Option<NodeAnnouncementInfoDeserWrapper> = None;
+               init_tlv_field_var!(channels, vec_type);
+
+               read_tlv_fields!(reader, {
+                       (0, lowest_inbound_channel_fees, option),
+                       (2, announcement_info_wrap, ignorable),
+                       (4, channels, vec_type),
+               });
+
+               Ok(NodeInfo {
+                       lowest_inbound_channel_fees: init_tlv_based_struct_field!(lowest_inbound_channel_fees, option),
+                       announcement_info: announcement_info_wrap.map(|w| w.0),
+                       channels: init_tlv_based_struct_field!(channels, vec_type),
+               })
+       }
+}
 
 const SERIALIZATION_VERSION: u8 = 1;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
@@ -1499,17 +1639,19 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
                match channels.get_mut(&msg.short_channel_id) {
                        None => return Err(LightningError{err: "Couldn't find channel for update".to_owned(), action: ErrorAction::IgnoreError}),
                        Some(channel) => {
-                               if let OptionalField::Present(htlc_maximum_msat) = msg.htlc_maximum_msat {
-                                       if htlc_maximum_msat > MAX_VALUE_MSAT {
-                                               return Err(LightningError{err: "htlc_maximum_msat is larger than maximum possible msats".to_owned(), action: ErrorAction::IgnoreError});
-                                       }
+                               if msg.htlc_maximum_msat > MAX_VALUE_MSAT {
+                                       return Err(LightningError{err:
+                                               "htlc_maximum_msat is larger than maximum possible msats".to_owned(),
+                                               action: ErrorAction::IgnoreError});
+                               }
 
-                                       if let Some(capacity_sats) = channel.capacity_sats {
-                                               // It's possible channel capacity is available now, although it wasn't available at announcement (so the field is None).
-                                               // Don't query UTXO set here to reduce DoS risks.
-                                               if capacity_sats > MAX_VALUE_MSAT / 1000 || htlc_maximum_msat > capacity_sats * 1000 {
-                                                       return Err(LightningError{err: "htlc_maximum_msat is larger than channel capacity or capacity is bogus".to_owned(), action: ErrorAction::IgnoreError});
-                                               }
+                               if let Some(capacity_sats) = channel.capacity_sats {
+                                       // It's possible channel capacity is available now, although it wasn't available at announcement (so the field is None).
+                                       // Don't query UTXO set here to reduce DoS risks.
+                                       if capacity_sats > MAX_VALUE_MSAT / 1000 || msg.htlc_maximum_msat > capacity_sats * 1000 {
+                                               return Err(LightningError{err:
+                                                       "htlc_maximum_msat is larger than channel capacity or capacity is bogus".to_owned(),
+                                                       action: ErrorAction::IgnoreError});
                                        }
                                }
                                macro_rules! check_update_latest {
@@ -1543,7 +1685,7 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
                                                        last_update: msg.timestamp,
                                                        cltv_expiry_delta: msg.cltv_expiry_delta,
                                                        htlc_minimum_msat: msg.htlc_minimum_msat,
-                                                       htlc_maximum_msat: if let OptionalField::Present(max_value) = msg.htlc_maximum_msat { Some(max_value) } else { None },
+                                                       htlc_maximum_msat: msg.htlc_maximum_msat,
                                                        fees: RoutingFees {
                                                                base_msat: msg.fee_base_msat,
                                                                proportional_millionths: msg.fee_proportional_millionths,
@@ -1649,6 +1791,11 @@ impl ReadOnlyNetworkGraph<'_> {
                &*self.channels
        }
 
+       /// Returns information on a channel with the given id.
+       pub fn channel(&self, short_channel_id: u64) -> Option<&ChannelInfo> {
+               self.channels.get(&short_channel_id)
+       }
+
        /// Returns all known nodes' public keys along with announced node info.
        ///
        /// (C-not exported) because we have no mapping for `BTreeMap`s
@@ -1656,6 +1803,11 @@ impl ReadOnlyNetworkGraph<'_> {
                &*self.nodes
        }
 
+       /// Returns information on a node with the given id.
+       pub fn node(&self, node_id: &NodeId) -> Option<&NodeInfo> {
+               self.nodes.get(node_id)
+       }
+
        /// Get network addresses by node id.
        /// Returns None if the requested node is completely unknown,
        /// or if node announcement for the node was never received.
@@ -1674,8 +1826,8 @@ mod tests {
        use chain;
        use ln::PaymentHash;
        use ln::features::{ChannelFeatures, InitFeatures, NodeFeatures};
-       use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY};
-       use ln::msgs::{Init, OptionalField, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement,
+       use routing::gossip::{P2PGossipSync, NetworkGraph, NetworkUpdate, NodeAlias, MAX_EXCESS_BYTES_FOR_RELAY, NodeId, RoutingFees, ChannelUpdateInfo, ChannelInfo, NodeAnnouncementInfo, NodeInfo};
+       use ln::msgs::{Init, RoutingMessageHandler, UnsignedNodeAnnouncement, NodeAnnouncement,
                UnsignedChannelAnnouncement, ChannelAnnouncement, UnsignedChannelUpdate, ChannelUpdate,
                ReplyChannelRange, QueryChannelRange, QueryShortChannelIds, MAX_VALUE_MSAT};
        use util::test_utils;
@@ -1799,7 +1951,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 144,
                        htlc_minimum_msat: 1_000_000,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: 1_000_000,
                        fee_base_msat: 10_000,
                        fee_proportional_millionths: 20,
                        excess_data: Vec::new()
@@ -2020,7 +2172,7 @@ mod tests {
                let valid_channel_update = get_signed_channel_update(|_| {}, node_1_privkey, &secp_ctx);
                match gossip_sync.handle_channel_update(&valid_channel_update) {
                        Ok(res) => assert!(res),
-                       _ => panic!()
+                       _ => panic!(),
                };
 
                {
@@ -2053,7 +2205,7 @@ mod tests {
                };
 
                let valid_channel_update = get_signed_channel_update(|unsigned_channel_update| {
-                       unsigned_channel_update.htlc_maximum_msat = OptionalField::Present(MAX_VALUE_MSAT + 1);
+                       unsigned_channel_update.htlc_maximum_msat = MAX_VALUE_MSAT + 1;
                        unsigned_channel_update.timestamp += 110;
                }, node_1_privkey, &secp_ctx);
                match gossip_sync.handle_channel_update(&valid_channel_update) {
@@ -2062,7 +2214,7 @@ mod tests {
                };
 
                let valid_channel_update = get_signed_channel_update(|unsigned_channel_update| {
-                       unsigned_channel_update.htlc_maximum_msat = OptionalField::Present(amount_sats * 1000 + 1);
+                       unsigned_channel_update.htlc_maximum_msat = amount_sats * 1000 + 1;
                        unsigned_channel_update.timestamp += 110;
                }, node_1_privkey, &secp_ctx);
                match gossip_sync.handle_channel_update(&valid_channel_update) {
@@ -2800,6 +2952,142 @@ mod tests {
                assert_eq!(format_bytes_alias(b"\xFFI <heart>\0LDK!"), "\u{FFFD}I <heart>");
                assert_eq!(format_bytes_alias(b"\xFFI <heart>\tLDK!"), "\u{FFFD}I <heart>\u{FFFD}LDK!");
        }
+
+       #[test]
+       fn channel_info_is_readable() {
+               let chanmon_cfgs = ::ln::functional_test_utils::create_chanmon_cfgs(2);
+               let node_cfgs = ::ln::functional_test_utils::create_node_cfgs(2, &chanmon_cfgs);
+               let node_chanmgrs = ::ln::functional_test_utils::create_node_chanmgrs(2, &node_cfgs, &[None, None, None, None]);
+               let nodes = ::ln::functional_test_utils::create_network(2, &node_cfgs, &node_chanmgrs);
+
+               // 1. Test encoding/decoding of ChannelUpdateInfo
+               let chan_update_info = ChannelUpdateInfo {
+                       last_update: 23,
+                       enabled: true,
+                       cltv_expiry_delta: 42,
+                       htlc_minimum_msat: 1234,
+                       htlc_maximum_msat: 5678,
+                       fees: RoutingFees { base_msat: 9, proportional_millionths: 10 },
+                       last_update_message: None,
+               };
+
+               let mut encoded_chan_update_info: Vec<u8> = Vec::new();
+               assert!(chan_update_info.write(&mut encoded_chan_update_info).is_ok());
+
+               // First make sure we can read ChannelUpdateInfos we just wrote
+               let read_chan_update_info: ChannelUpdateInfo = ::util::ser::Readable::read(&mut encoded_chan_update_info.as_slice()).unwrap();
+               assert_eq!(chan_update_info, read_chan_update_info);
+
+               // Check the serialization hasn't changed.
+               let legacy_chan_update_info_with_some: Vec<u8> = hex::decode("340004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c0100").unwrap();
+               assert_eq!(encoded_chan_update_info, legacy_chan_update_info_with_some);
+
+               // Check we fail if htlc_maximum_msat is not present in either the ChannelUpdateInfo itself
+               // or the ChannelUpdate enclosed with `last_update_message`.
+               let legacy_chan_update_info_with_some_and_fail_update: Vec<u8> = hex::decode("b40004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c8181d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f00083a840000034d013413a70000009000000000000f42400000271000000014").unwrap();
+               let read_chan_update_info_res: Result<ChannelUpdateInfo, ::ln::msgs::DecodeError> = ::util::ser::Readable::read(&mut legacy_chan_update_info_with_some_and_fail_update.as_slice());
+               assert!(read_chan_update_info_res.is_err());
+
+               let legacy_chan_update_info_with_none: Vec<u8> = hex::decode("2c0004000000170201010402002a060800000000000004d20801000a0d0c00040000000902040000000a0c0100").unwrap();
+               let read_chan_update_info_res: Result<ChannelUpdateInfo, ::ln::msgs::DecodeError> = ::util::ser::Readable::read(&mut legacy_chan_update_info_with_none.as_slice());
+               assert!(read_chan_update_info_res.is_err());
+                       
+               // 2. Test encoding/decoding of ChannelInfo
+               // Check we can encode/decode ChannelInfo without ChannelUpdateInfo fields present.
+               let chan_info_none_updates = ChannelInfo {
+                       features: ChannelFeatures::known(),
+                       node_one: NodeId::from_pubkey(&nodes[0].node.get_our_node_id()),
+                       one_to_two: None,
+                       node_two: NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
+                       two_to_one: None,
+                       capacity_sats: None,
+                       announcement_message: None,
+                       announcement_received_time: 87654,
+               };
+
+               let mut encoded_chan_info: Vec<u8> = Vec::new();
+               assert!(chan_info_none_updates.write(&mut encoded_chan_info).is_ok());
+
+               let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut encoded_chan_info.as_slice()).unwrap();
+               assert_eq!(chan_info_none_updates, read_chan_info);
+
+               // Check we can encode/decode ChannelInfo with ChannelUpdateInfo fields present.
+               let chan_info_some_updates = ChannelInfo {
+                       features: ChannelFeatures::known(),
+                       node_one: NodeId::from_pubkey(&nodes[0].node.get_our_node_id()),
+                       one_to_two: Some(chan_update_info.clone()),
+                       node_two: NodeId::from_pubkey(&nodes[1].node.get_our_node_id()),
+                       two_to_one: Some(chan_update_info.clone()),
+                       capacity_sats: None,
+                       announcement_message: None,
+                       announcement_received_time: 87654,
+               };
+
+               let mut encoded_chan_info: Vec<u8> = Vec::new();
+               assert!(chan_info_some_updates.write(&mut encoded_chan_info).is_ok());
+
+               let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut encoded_chan_info.as_slice()).unwrap();
+               assert_eq!(chan_info_some_updates, read_chan_info);
+
+               // Check the serialization hasn't changed.
+               let legacy_chan_info_with_some: Vec<u8> = hex::decode("ca00020000010800000000000156660221027f921585f2ac0c7c70e36110adecfd8fd14b8a99bfb3d000a283fcac358fce88043636340004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c010006210355f8d2238a322d16b602bd0ceaad5b01019fb055971eaadcc9b29226a4da6c23083636340004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c01000a01000c0100").unwrap();
+               assert_eq!(encoded_chan_info, legacy_chan_info_with_some);
+
+               // Check we can decode legacy ChannelInfo, even if the `two_to_one` / `one_to_two` /
+               // `last_update_message` fields fail to decode due to missing htlc_maximum_msat.
+               let legacy_chan_info_with_some_and_fail_update = hex::decode("fd01ca00020000010800000000000156660221027f921585f2ac0c7c70e36110adecfd8fd14b8a99bfb3d000a283fcac358fce8804b6b6b40004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c8181d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f00083a840000034d013413a70000009000000000000f4240000027100000001406210355f8d2238a322d16b602bd0ceaad5b01019fb055971eaadcc9b29226a4da6c2308b6b6b40004000000170201010402002a060800000000000004d2080909000000000000162e0a0d0c00040000000902040000000a0c8181d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f00083a840000034d013413a70000009000000000000f424000002710000000140a01000c0100").unwrap();
+               let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut legacy_chan_info_with_some_and_fail_update.as_slice()).unwrap();
+               assert_eq!(read_chan_info.announcement_received_time, 87654);
+               assert_eq!(read_chan_info.one_to_two, None);
+               assert_eq!(read_chan_info.two_to_one, None);
+
+               let legacy_chan_info_with_none: Vec<u8> = hex::decode("ba00020000010800000000000156660221027f921585f2ac0c7c70e36110adecfd8fd14b8a99bfb3d000a283fcac358fce88042e2e2c0004000000170201010402002a060800000000000004d20801000a0d0c00040000000902040000000a0c010006210355f8d2238a322d16b602bd0ceaad5b01019fb055971eaadcc9b29226a4da6c23082e2e2c0004000000170201010402002a060800000000000004d20801000a0d0c00040000000902040000000a0c01000a01000c0100").unwrap();
+               let read_chan_info: ChannelInfo = ::util::ser::Readable::read(&mut legacy_chan_info_with_none.as_slice()).unwrap();
+               assert_eq!(read_chan_info.announcement_received_time, 87654);
+               assert_eq!(read_chan_info.one_to_two, None);
+               assert_eq!(read_chan_info.two_to_one, None);
+       }
+
+       #[test]
+       fn node_info_is_readable() {
+               use std::convert::TryFrom;
+
+               // 1. Check we can read a valid NodeAnnouncementInfo and fail on an invalid one
+               let valid_netaddr = ::ln::msgs::NetAddress::Hostname { hostname: ::util::ser::Hostname::try_from("A".to_string()).unwrap(), port: 1234 };
+               let valid_node_ann_info = NodeAnnouncementInfo {
+                       features: NodeFeatures::known(),
+                       last_update: 0,
+                       rgb: [0u8; 3],
+                       alias: NodeAlias([0u8; 32]),
+                       addresses: vec![valid_netaddr],
+                       announcement_message: None,
+               };
+
+               let mut encoded_valid_node_ann_info = Vec::new();
+               assert!(valid_node_ann_info.write(&mut encoded_valid_node_ann_info).is_ok());
+               let read_valid_node_ann_info: NodeAnnouncementInfo = ::util::ser::Readable::read(&mut encoded_valid_node_ann_info.as_slice()).unwrap();
+               assert_eq!(read_valid_node_ann_info, valid_node_ann_info);
+
+               let encoded_invalid_node_ann_info = hex::decode("3f0009000788a000080a51a20204000000000403000000062000000000000000000000000000000000000000000000000000000000000000000a0505014004d2").unwrap();
+               let read_invalid_node_ann_info_res: Result<NodeAnnouncementInfo, ::ln::msgs::DecodeError> = ::util::ser::Readable::read(&mut encoded_invalid_node_ann_info.as_slice());
+               assert!(read_invalid_node_ann_info_res.is_err());
+
+               // 2. Check we can read a NodeInfo anyways, but set the NodeAnnouncementInfo to None if invalid
+               let valid_node_info = NodeInfo {
+                       channels: Vec::new(),
+                       lowest_inbound_channel_fees: None,
+                       announcement_info: Some(valid_node_ann_info),
+               };
+
+               let mut encoded_valid_node_info = Vec::new();
+               assert!(valid_node_info.write(&mut encoded_valid_node_info).is_ok());
+               let read_valid_node_info: NodeInfo = ::util::ser::Readable::read(&mut encoded_valid_node_info.as_slice()).unwrap();
+               assert_eq!(read_valid_node_info, valid_node_info);
+
+               let encoded_invalid_node_info_hex = hex::decode("4402403f0009000788a000080a51a20204000000000403000000062000000000000000000000000000000000000000000000000000000000000000000a0505014004d20400").unwrap();
+               let read_invalid_node_info: NodeInfo = ::util::ser::Readable::read(&mut encoded_invalid_node_info_hex.as_slice()).unwrap();
+               assert_eq!(read_invalid_node_info.announcement_info, None);
+       }
 }
 
 #[cfg(all(test, feature = "_bench_unstable"))]
index 289221601c5c82e92cd2db457dbfff69a5eef460..4c00dc5fcf9327126d0f9bd580823fd1447ea4f7 100644 (file)
@@ -225,6 +225,26 @@ pub struct PaymentParameters {
        /// The maximum number of paths that may be used by (MPP) payments.
        /// Defaults to [`DEFAULT_MAX_PATH_COUNT`].
        pub max_path_count: u8,
+
+       /// Selects the maximum share of a channel's total capacity which will be sent over a channel,
+       /// as a power of 1/2. A higher value prefers to send the payment using more MPP parts whereas
+       /// a lower value prefers to send larger MPP parts, potentially saturating channels and
+       /// increasing failure probability for those paths.
+       ///
+       /// Note that this restriction will be relaxed during pathfinding after paths which meet this
+       /// restriction have been found. While paths which meet this criteria will be searched for, it
+       /// is ultimately up to the scorer to select them over other paths.
+       ///
+       /// A value of 0 will allow payments up to and including a channel's total announced usable
+       /// capacity, a value of one will only use up to half its capacity, two 1/4, etc.
+       ///
+       /// Default value: 2
+       pub max_channel_saturation_power_of_half: u8,
+
+       /// A list of SCIDs which this payment was previously attempted over and which caused the
+       /// payment to fail. Future attempts for the same payment shouldn't be relayed through any of
+       /// these SCIDs.
+       pub previously_failed_channels: Vec<u64>,
 }
 
 impl_writeable_tlv_based!(PaymentParameters, {
@@ -233,7 +253,9 @@ impl_writeable_tlv_based!(PaymentParameters, {
        (2, features, option),
        (3, max_path_count, (default_value, DEFAULT_MAX_PATH_COUNT)),
        (4, route_hints, vec_type),
+       (5, max_channel_saturation_power_of_half, (default_value, 2)),
        (6, expiry_time, option),
+       (7, previously_failed_channels, vec_type),
 });
 
 impl PaymentParameters {
@@ -246,6 +268,8 @@ impl PaymentParameters {
                        expiry_time: None,
                        max_total_cltv_expiry_delta: DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA,
                        max_path_count: DEFAULT_MAX_PATH_COUNT,
+                       max_channel_saturation_power_of_half: 2,
+                       previously_failed_channels: Vec::new(),
                }
        }
 
@@ -288,6 +312,13 @@ impl PaymentParameters {
        pub fn with_max_path_count(self, max_path_count: u8) -> Self {
                Self { max_path_count, ..self }
        }
+
+       /// Includes a limit for the maximum number of payment paths that may be used.
+       ///
+       /// (C-not exported) since bindings don't support move semantics
+       pub fn with_max_channel_saturation_power_of_half(self, max_channel_saturation_power_of_half: u8) -> Self {
+               Self { max_channel_saturation_power_of_half, ..self }
+       }
 }
 
 /// A list of hops along a payment path terminating with a channel to the recipient.
@@ -433,16 +464,6 @@ impl<'a> CandidateRouteHop<'a> {
                }
        }
 
-       fn htlc_maximum_msat(&self) -> u64 {
-               match self {
-                       CandidateRouteHop::FirstHop { details } => details.next_outbound_htlc_limit_msat,
-                       CandidateRouteHop::PublicHop { info, .. } => info.htlc_maximum_msat(),
-                       CandidateRouteHop::PrivateHop { hint } => {
-                               hint.htlc_maximum_msat.unwrap_or(u64::max_value())
-                       },
-               }
-       }
-
        fn fees(&self) -> RoutingFees {
                match self {
                        CandidateRouteHop::FirstHop { .. } => RoutingFees {
@@ -464,6 +485,33 @@ impl<'a> CandidateRouteHop<'a> {
        }
 }
 
+#[inline]
+fn max_htlc_from_capacity(capacity: EffectiveCapacity, max_channel_saturation_power_of_half: u8) -> u64 {
+       let saturation_shift: u32 = max_channel_saturation_power_of_half as u32;
+       match capacity {
+               EffectiveCapacity::ExactLiquidity { liquidity_msat } => liquidity_msat,
+               EffectiveCapacity::Infinite => u64::max_value(),
+               EffectiveCapacity::Unknown => EffectiveCapacity::Unknown.as_msat(),
+               EffectiveCapacity::MaximumHTLC { amount_msat } =>
+                       amount_msat.checked_shr(saturation_shift).unwrap_or(0),
+               EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: None } =>
+                       capacity_msat.checked_shr(saturation_shift).unwrap_or(0),
+               EffectiveCapacity::Total { capacity_msat, htlc_maximum_msat: Some(htlc_max) } =>
+                       cmp::min(capacity_msat.checked_shr(saturation_shift).unwrap_or(0), htlc_max),
+       }
+}
+
+fn iter_equal<I1: Iterator, I2: Iterator>(mut iter_a: I1, mut iter_b: I2)
+-> bool where I1::Item: PartialEq<I2::Item> {
+       loop {
+               let a = iter_a.next();
+               let b = iter_b.next();
+               if a.is_none() && b.is_none() { return true; }
+               if a.is_none() || b.is_none() { return false; }
+               if a.unwrap().ne(&b.unwrap()) { return false; }
+       }
+}
+
 /// It's useful to keep track of the hops associated with the fees required to use them,
 /// so that we can choose cheaper paths (as per Dijkstra's algorithm).
 /// Fee values should be updated only in the context of the whole path, see update_value_and_recompute_fees.
@@ -571,10 +619,9 @@ impl<'a> PaymentPath<'a> {
        // to the fees being paid not lining up with the actual limits.
        //
        // Note that this function is not aware of the available_liquidity limit, and thus does not
-       // support increasing the value being transferred.
+       // support increasing the value being transferred beyond what was selected during the initial
+       // routing passes.
        fn update_value_and_recompute_fees(&mut self, value_msat: u64) {
-               assert!(value_msat <= self.hops.last().unwrap().0.fee_msat);
-
                let mut total_fee_paid_msat = 0 as u64;
                for i in (0..self.hops.len()).rev() {
                        let last_hop = i == self.hops.len() - 1;
@@ -707,7 +754,7 @@ where L::Target: Logger, GL::Target: Logger {
 pub(crate) fn get_route<L: Deref, S: Score>(
        our_node_pubkey: &PublicKey, payment_params: &PaymentParameters, network_graph: &ReadOnlyNetworkGraph,
        first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
-       logger: L, scorer: &S, random_seed_bytes: &[u8; 32]
+       logger: L, scorer: &S, _random_seed_bytes: &[u8; 32]
 ) -> Result<Route, LightningError>
 where L::Target: Logger {
        let payee_node_id = NodeId::from_pubkey(&payment_params.payee_pubkey);
@@ -749,11 +796,11 @@ where L::Target: Logger {
        // 4. See if we managed to collect paths which aggregately are able to transfer target value
        //    (not recommended value).
        // 5. If yes, proceed. If not, fail routing.
-       // 6. Randomly combine paths into routes having enough to fulfill the payment. (TODO: knapsack)
-       // 7. Of all the found paths, select only those with the lowest total fee.
-       // 8. The last path in every selected route is likely to be more than we need.
-       //    Reduce its value-to-transfer and recompute fees.
-       // 9. Choose the best route by the lowest total fee.
+       // 6. Select the paths which have the lowest cost (fee plus scorer penalty) per amount
+       //    transferred up to the transfer target value.
+       // 7. Reduce the value of the last path until we are sending only the target value.
+       // 8. If our maximum channel saturation limit caused us to pick two identical paths, combine
+       //    them so that we're not sending two HTLCs along the same path.
 
        // As for the actual search algorithm,
        // we do a payee-to-payer pseudo-Dijkstra's sorting by each node's distance from the payee
@@ -882,6 +929,11 @@ where L::Target: Logger {
                final_value_msat
        };
 
+       // When we start collecting routes we enforce the max_channel_saturation_power_of_half
+       // requirement strictly. After we've collected enough (or if we fail to find new routes) we
+       // drop the requirement by setting this to 0.
+       let mut channel_saturation_pow_half = payment_params.max_channel_saturation_power_of_half;
+
        // Keep track of how much liquidity has been used in selected channels. Used to determine
        // if the channel can be used by additional MPP paths or to inform path finding decisions. It is
        // aware of direction *only* to ensure that the correct htlc_maximum_msat value is used. Hence,
@@ -934,7 +986,8 @@ where L::Target: Logger {
                        // - for first and last hops early in get_route
                        if $src_node_id != $dest_node_id {
                                let short_channel_id = $candidate.short_channel_id();
-                               let htlc_maximum_msat = $candidate.htlc_maximum_msat();
+                               let effective_capacity = $candidate.effective_capacity();
+                               let htlc_maximum_msat = max_htlc_from_capacity(effective_capacity, channel_saturation_pow_half);
 
                                // It is tricky to subtract $next_hops_fee_msat from available liquidity here.
                                // It may be misleading because we might later choose to reduce the value transferred
@@ -956,7 +1009,7 @@ where L::Target: Logger {
                                        let contributes_sufficient_value = available_value_contribution_msat >= minimal_value_contribution_msat;
                                        // Do not consider candidate hops that would exceed the maximum path length.
                                        let path_length_to_node = $next_hops_path_length + 1;
-                                       let doesnt_exceed_max_path_length = path_length_to_node <= MAX_PATH_LENGTH_ESTIMATE;
+                                       let exceeds_max_path_length = path_length_to_node > MAX_PATH_LENGTH_ESTIMATE;
 
                                        // Do not consider candidates that exceed the maximum total cltv expiry limit.
                                        // In order to already account for some of the privacy enhancing random CLTV
@@ -967,7 +1020,7 @@ where L::Target: Logger {
                                                .unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta);
                                        let hop_total_cltv_delta = ($next_hops_cltv_delta as u32)
                                                .saturating_add($candidate.cltv_expiry_delta());
-                                       let doesnt_exceed_cltv_delta_limit = hop_total_cltv_delta <= max_total_cltv_expiry_delta;
+                                       let exceeds_cltv_delta_limit = hop_total_cltv_delta > max_total_cltv_expiry_delta;
 
                                        let value_contribution_msat = cmp::min(available_value_contribution_msat, $next_hops_value_contribution);
                                        // Includes paying fees for the use of the following channels.
@@ -987,15 +1040,19 @@ where L::Target: Logger {
                                                 (amount_to_transfer_over_msat < $next_hops_path_htlc_minimum_msat &&
                                                  recommended_value_msat > $next_hops_path_htlc_minimum_msat));
 
+                                       let payment_failed_on_this_channel =
+                                               payment_params.previously_failed_channels.contains(&short_channel_id);
+
                                        // If HTLC minimum is larger than the amount we're going to transfer, we shouldn't
                                        // bother considering this channel. If retrying with recommended_value_msat may
                                        // allow us to hit the HTLC minimum limit, set htlc_minimum_limit so that we go
                                        // around again with a higher amount.
-                                       if contributes_sufficient_value && doesnt_exceed_max_path_length &&
-                                               doesnt_exceed_cltv_delta_limit && may_overpay_to_meet_path_minimum_msat {
+                                       if !contributes_sufficient_value || exceeds_max_path_length ||
+                                               exceeds_cltv_delta_limit || payment_failed_on_this_channel {
+                                               // Path isn't useful, ignore it and move on.
+                                       } else if may_overpay_to_meet_path_minimum_msat {
                                                hit_minimum_limit = true;
-                                       } else if contributes_sufficient_value && doesnt_exceed_max_path_length &&
-                                               doesnt_exceed_cltv_delta_limit && over_path_minimum_msat {
+                                       } else if over_path_minimum_msat {
                                                // Note that low contribution here (limited by available_liquidity_msat)
                                                // might violate htlc_minimum_msat on the hops which are next along the
                                                // payment path (upstream to the payee). To avoid that, we recompute
@@ -1084,7 +1141,7 @@ where L::Target: Logger {
                                                        let channel_usage = ChannelUsage {
                                                                amount_msat: amount_to_transfer_over_msat,
                                                                inflight_htlc_msat: used_liquidity_msat,
-                                                               effective_capacity: $candidate.effective_capacity(),
+                                                               effective_capacity,
                                                        };
                                                        let channel_penalty_msat = scorer.channel_penalty_msat(
                                                                short_channel_id, &$src_node_id, &$dest_node_id, channel_usage
@@ -1419,7 +1476,7 @@ where L::Target: Logger {
                // Both these cases (and other cases except reaching recommended_value_msat) mean that
                // paths_collection will be stopped because found_new_path==false.
                // This is not necessarily a routing failure.
-               'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, total_cltv_delta, value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, path_length_to_node, .. }) = targets.pop() {
+               'path_construction: while let Some(RouteGraphNode { node_id, lowest_fee_to_node, total_cltv_delta, mut value_contribution_msat, path_htlc_minimum_msat, path_penalty_msat, path_length_to_node, .. }) = targets.pop() {
 
                        // Since we're going payee-to-payer, hitting our node as a target means we should stop
                        // traversing the graph and arrange the path out of what we found.
@@ -1485,7 +1542,9 @@ where L::Target: Logger {
                                // on some channels we already passed (assuming dest->source direction). Here, we
                                // recompute the fees again, so that if that's the case, we match the currently
                                // underpaid htlc_minimum_msat with fees.
-                               payment_path.update_value_and_recompute_fees(cmp::min(value_contribution_msat, final_value_msat));
+                               debug_assert_eq!(payment_path.get_value_msat(), value_contribution_msat);
+                               value_contribution_msat = cmp::min(value_contribution_msat, final_value_msat);
+                               payment_path.update_value_and_recompute_fees(value_contribution_msat);
 
                                // Since a path allows to transfer as much value as
                                // the smallest channel it has ("bottleneck"), we should recompute
@@ -1505,12 +1564,14 @@ where L::Target: Logger {
                                                .entry((hop.candidate.short_channel_id(), *prev_hop < hop.node_id))
                                                .and_modify(|used_liquidity_msat| *used_liquidity_msat += spent_on_hop_msat)
                                                .or_insert(spent_on_hop_msat);
-                                       if *used_liquidity_msat == hop.candidate.htlc_maximum_msat() {
+                                       let hop_capacity = hop.candidate.effective_capacity();
+                                       let hop_max_msat = max_htlc_from_capacity(hop_capacity, channel_saturation_pow_half);
+                                       if *used_liquidity_msat == hop_max_msat {
                                                // If this path used all of this channel's available liquidity, we know
                                                // this path will not be selected again in the next loop iteration.
                                                prevented_redundant_path_selection = true;
                                        }
-                                       debug_assert!(*used_liquidity_msat <= hop.candidate.htlc_maximum_msat());
+                                       debug_assert!(*used_liquidity_msat <= hop_max_msat);
                                }
                                if !prevented_redundant_path_selection {
                                        // If we weren't capped by hitting a liquidity limit on a channel in the path,
@@ -1551,6 +1612,10 @@ where L::Target: Logger {
                }
 
                if !allow_mpp {
+                       if !found_new_path && channel_saturation_pow_half != 0 {
+                               channel_saturation_pow_half = 0;
+                               continue 'paths_collection;
+                       }
                        // If we don't support MPP, no use trying to gather more value ever.
                        break 'paths_collection;
                }
@@ -1560,7 +1625,9 @@ where L::Target: Logger {
                // iteration.
                // In the latter case, making another path finding attempt won't help,
                // because we deterministically terminated the search due to low liquidity.
-               if already_collected_value_msat >= recommended_value_msat || !found_new_path {
+               if !found_new_path && channel_saturation_pow_half != 0 {
+                       channel_saturation_pow_half = 0;
+               } else if already_collected_value_msat >= recommended_value_msat || !found_new_path {
                        log_trace!(logger, "Have now collected {} msat (seeking {} msat) in paths. Last path loop {} a new path.",
                                already_collected_value_msat, recommended_value_msat, if found_new_path { "found" } else { "did not find" });
                        break 'paths_collection;
@@ -1588,96 +1655,79 @@ where L::Target: Logger {
                return Err(LightningError{err: "Failed to find a sufficient route to the given destination".to_owned(), action: ErrorAction::IgnoreError});
        }
 
-       // Sort by total fees and take the best paths.
-       payment_paths.sort_unstable_by_key(|path| path.get_total_fee_paid_msat());
-       if payment_paths.len() > 50 {
-               payment_paths.truncate(50);
-       }
-
-       // Draw multiple sufficient routes by randomly combining the selected paths.
-       let mut drawn_routes = Vec::new();
-       let mut prng = ChaCha20::new(random_seed_bytes, &[0u8; 12]);
-       let mut random_index_bytes = [0u8; ::core::mem::size_of::<usize>()];
-
-       let num_permutations = payment_paths.len();
-       for _ in 0..num_permutations {
-               let mut cur_route = Vec::<PaymentPath>::new();
-               let mut aggregate_route_value_msat = 0;
-
-               // Step (6).
-               // Do a Fisher-Yates shuffle to create a random permutation of the payment paths
-               for cur_index in (1..payment_paths.len()).rev() {
-                       prng.process_in_place(&mut random_index_bytes);
-                       let random_index = usize::from_be_bytes(random_index_bytes).wrapping_rem(cur_index+1);
-                       payment_paths.swap(cur_index, random_index);
+       // Step (6).
+       let mut selected_route = payment_paths;
+
+       debug_assert_eq!(selected_route.iter().map(|p| p.get_value_msat()).sum::<u64>(), already_collected_value_msat);
+       let mut overpaid_value_msat = already_collected_value_msat - final_value_msat;
+
+       // First, sort by the cost-per-value of the path, dropping the paths that cost the most for
+       // the value they contribute towards the payment amount.
+       // We sort in descending order as we will remove from the front in `retain`, next.
+       selected_route.sort_unstable_by(|a, b|
+               (((b.get_cost_msat() as u128) << 64) / (b.get_value_msat() as u128))
+                       .cmp(&(((a.get_cost_msat() as u128) << 64) / (a.get_value_msat() as u128)))
+       );
+
+       // We should make sure that at least 1 path left.
+       let mut paths_left = selected_route.len();
+       selected_route.retain(|path| {
+               if paths_left == 1 {
+                       return true
                }
+               let path_value_msat = path.get_value_msat();
+               if path_value_msat <= overpaid_value_msat {
+                       overpaid_value_msat -= path_value_msat;
+                       paths_left -= 1;
+                       return false;
+               }
+               true
+       });
+       debug_assert!(selected_route.len() > 0);
 
+       if overpaid_value_msat != 0 {
                // Step (7).
-               for payment_path in &payment_paths {
-                       cur_route.push(payment_path.clone());
-                       aggregate_route_value_msat += payment_path.get_value_msat();
-                       if aggregate_route_value_msat > final_value_msat {
-                               // Last path likely overpaid. Substract it from the most expensive
-                               // (in terms of proportional fee) path in this route and recompute fees.
-                               // This might be not the most economically efficient way, but fewer paths
-                               // also makes routing more reliable.
-                               let mut overpaid_value_msat = aggregate_route_value_msat - final_value_msat;
-
-                               // First, we drop some expensive low-value paths entirely if possible, since fewer
-                               // paths is better: the payment is less likely to fail. In order to do so, we sort
-                               // by value and fall back to total fees paid, i.e., in case of equal values we
-                               // prefer lower cost paths.
-                               cur_route.sort_unstable_by(|a, b| {
-                                       a.get_value_msat().cmp(&b.get_value_msat())
-                                               // Reverse ordering for cost, so we drop higher-cost paths first
-                                               .then_with(|| b.get_cost_msat().cmp(&a.get_cost_msat()))
-                               });
-
-                               // We should make sure that at least 1 path left.
-                               let mut paths_left = cur_route.len();
-                               cur_route.retain(|path| {
-                                       if paths_left == 1 {
-                                               return true
-                                       }
-                                       let mut keep = true;
-                                       let path_value_msat = path.get_value_msat();
-                                       if path_value_msat <= overpaid_value_msat {
-                                               keep = false;
-                                               overpaid_value_msat -= path_value_msat;
-                                               paths_left -= 1;
-                                       }
-                                       keep
-                               });
+               // Now, subtract the remaining overpaid value from the most-expensive path.
+               // TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
+               // so that the sender pays less fees overall. And also htlc_minimum_msat.
+               selected_route.sort_unstable_by(|a, b| {
+                       let a_f = a.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>();
+                       let b_f = b.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>();
+                       a_f.cmp(&b_f).then_with(|| b.get_cost_msat().cmp(&a.get_cost_msat()))
+               });
+               let expensive_payment_path = selected_route.first_mut().unwrap();
 
-                               if overpaid_value_msat == 0 {
-                                       break;
-                               }
+               // We already dropped all the paths with value below `overpaid_value_msat` above, thus this
+               // can't go negative.
+               let expensive_path_new_value_msat = expensive_payment_path.get_value_msat() - overpaid_value_msat;
+               expensive_payment_path.update_value_and_recompute_fees(expensive_path_new_value_msat);
+       }
 
-                               assert!(cur_route.len() > 0);
-
-                               // Step (8).
-                               // Now, subtract the overpaid value from the most-expensive path.
-                               // TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
-                               // so that the sender pays less fees overall. And also htlc_minimum_msat.
-                               cur_route.sort_unstable_by_key(|path| { path.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>() });
-                               let expensive_payment_path = cur_route.first_mut().unwrap();
-
-                               // We already dropped all the small value paths above, meaning all the
-                               // remaining paths are larger than remaining overpaid_value_msat.
-                               // Thus, this can't be negative.
-                               let expensive_path_new_value_msat = expensive_payment_path.get_value_msat() - overpaid_value_msat;
-                               expensive_payment_path.update_value_and_recompute_fees(expensive_path_new_value_msat);
-                               break;
-                       }
+       // Step (8).
+       // Sort by the path itself and combine redundant paths.
+       // Note that we sort by SCIDs alone as its simpler but when combining we have to ensure we
+       // compare both SCIDs and NodeIds as individual nodes may use random aliases causing collisions
+       // across nodes.
+       selected_route.sort_unstable_by_key(|path| {
+               let mut key = [0u64; MAX_PATH_LENGTH_ESTIMATE as usize];
+               debug_assert!(path.hops.len() <= key.len());
+               for (scid, key) in path.hops.iter().map(|h| h.0.candidate.short_channel_id()).zip(key.iter_mut()) {
+                       *key = scid;
+               }
+               key
+       });
+       for idx in 0..(selected_route.len() - 1) {
+               if idx + 1 >= selected_route.len() { break; }
+               if iter_equal(selected_route[idx    ].hops.iter().map(|h| (h.0.candidate.short_channel_id(), h.0.node_id)),
+                             selected_route[idx + 1].hops.iter().map(|h| (h.0.candidate.short_channel_id(), h.0.node_id))) {
+                       let new_value = selected_route[idx].get_value_msat() + selected_route[idx + 1].get_value_msat();
+                       selected_route[idx].update_value_and_recompute_fees(new_value);
+                       selected_route.remove(idx + 1);
                }
-               drawn_routes.push(cur_route);
        }
 
-       // Step (9).
-       // Select the best route by lowest total cost.
-       drawn_routes.sort_unstable_by_key(|paths| paths.iter().map(|path| path.get_cost_msat()).sum::<u64>());
        let mut selected_paths = Vec::<Vec<Result<RouteHop, LightningError>>>::new();
-       for payment_path in drawn_routes.first().unwrap() {
+       for payment_path in selected_route {
                let mut path = payment_path.hops.iter().map(|(payment_hop, node_features)| {
                        Ok(RouteHop {
                                pubkey: PublicKey::from_slice(payment_hop.node_id.as_slice()).map_err(|_| LightningError{err: format!("Public key {:?} is invalid", &payment_hop.node_id), action: ErrorAction::IgnoreAndLog(Level::Trace)})?,
@@ -1890,8 +1940,8 @@ mod tests {
        use chain::transaction::OutPoint;
        use chain::keysinterface::KeysInterface;
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
-       use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
-               NodeAnnouncement, UnsignedNodeAnnouncement, ChannelUpdate, UnsignedChannelUpdate};
+       use ln::msgs::{ErrorAction, LightningError, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
+               NodeAnnouncement, UnsignedNodeAnnouncement, ChannelUpdate, UnsignedChannelUpdate, MAX_VALUE_MSAT};
        use ln::channelmanager;
        use util::test_utils;
        use util::chacha20::ChaCha20;
@@ -1915,6 +1965,8 @@ mod tests {
        use prelude::*;
        use sync::{self, Arc};
 
+       use core::convert::TryInto;
+
        fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
                        features: InitFeatures, outbound_capacity_msat: u64) -> channelmanager::ChannelDetails {
                channelmanager::ChannelDetails {
@@ -2081,7 +2133,7 @@ mod tests {
                                flags: 0,
                                cltv_expiry_delta: 0,
                                htlc_minimum_msat: 0,
-                               htlc_maximum_msat: OptionalField::Absent,
+                               htlc_maximum_msat: MAX_VALUE_MSAT,
                                fee_base_msat: 0,
                                fee_proportional_millionths: 0,
                                excess_data: Vec::new()
@@ -2093,7 +2145,7 @@ mod tests {
                                flags: 1,
                                cltv_expiry_delta: 0,
                                htlc_minimum_msat: 0,
-                               htlc_maximum_msat: OptionalField::Absent,
+                               htlc_maximum_msat: MAX_VALUE_MSAT,
                                fee_base_msat: 0,
                                fee_proportional_millionths: 0,
                                excess_data: Vec::new()
@@ -2187,7 +2239,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2203,7 +2255,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (5 << 4) | 3,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: u32::max_value(),
                        fee_proportional_millionths: u32::max_value(),
                        excess_data: Vec::new()
@@ -2215,7 +2267,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2231,7 +2283,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (5 << 4) | 3,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: u32::max_value(),
                        fee_proportional_millionths: u32::max_value(),
                        excess_data: Vec::new()
@@ -2243,7 +2295,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2259,7 +2311,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (3 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2271,7 +2323,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (3 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 100,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2285,7 +2337,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (4 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 1000000,
                        excess_data: Vec::new()
@@ -2297,7 +2349,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (4 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2311,7 +2363,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (13 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 2000000,
                        excess_data: Vec::new()
@@ -2323,7 +2375,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (13 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2339,7 +2391,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (6 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2351,7 +2403,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (6 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new(),
@@ -2365,7 +2417,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (11 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2377,7 +2429,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (11 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2395,7 +2447,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (7 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 1000000,
                        excess_data: Vec::new()
@@ -2407,7 +2459,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (7 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2492,7 +2544,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2504,7 +2556,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2516,7 +2568,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2528,7 +2580,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2540,7 +2592,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2555,7 +2607,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 200_000_000,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2570,7 +2622,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(199_999_999),
+                       htlc_maximum_msat: 199_999_999,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2589,7 +2641,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2619,7 +2671,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 35_000,
-                       htlc_maximum_msat: OptionalField::Present(40_000),
+                       htlc_maximum_msat: 40_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2631,7 +2683,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 35_000,
-                       htlc_maximum_msat: OptionalField::Present(40_000),
+                       htlc_maximum_msat: 40_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2645,7 +2697,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2657,7 +2709,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2671,7 +2723,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2692,7 +2744,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 65_000,
-                       htlc_maximum_msat: OptionalField::Present(80_000),
+                       htlc_maximum_msat: 80_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2704,7 +2756,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2716,7 +2768,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 100_000,
                        excess_data: Vec::new()
@@ -2755,7 +2807,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -2767,7 +2819,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3173,7 +3225,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3185,7 +3237,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3245,7 +3297,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3257,7 +3309,7 @@ mod tests {
                        flags: 2, // to disable
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3590,7 +3642,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3602,7 +3654,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3617,7 +3669,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(1_000_000_000),
+                       htlc_maximum_msat: 1_000_000_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3632,7 +3684,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: 250_000_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3665,7 +3717,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(1_000_000_000),
+                       htlc_maximum_msat: 1_000_000_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3700,7 +3752,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(1_000_000_000),
+                       htlc_maximum_msat: 1_000_000_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3715,7 +3767,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(15_000),
+                       htlc_maximum_msat: 15_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3750,7 +3802,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3773,7 +3825,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (3 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: 15_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3785,7 +3837,7 @@ mod tests {
                        flags: 1,
                        cltv_expiry_delta: (3 << 4) | 2,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: 15_000,
                        fee_base_msat: 100,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3817,7 +3869,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(10_000),
+                       htlc_maximum_msat: 10_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3865,7 +3917,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3877,7 +3929,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3892,7 +3944,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3904,7 +3956,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3917,7 +3969,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(50_000),
+                       htlc_maximum_msat: 50_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3929,7 +3981,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3986,7 +4038,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 1_000_000,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -3998,7 +4050,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(50_000),
+                       htlc_maximum_msat: 50_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4043,7 +4095,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4055,7 +4107,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(50_000),
+                       htlc_maximum_msat: 50_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4070,7 +4122,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(60_000),
+                       htlc_maximum_msat: 60_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4082,7 +4134,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(60_000),
+                       htlc_maximum_msat: 60_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4097,7 +4149,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4109,7 +4161,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(180_000),
+                       htlc_maximum_msat: 180_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4200,7 +4252,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4212,7 +4264,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4226,7 +4278,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4238,7 +4290,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4253,7 +4305,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4269,7 +4321,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4281,7 +4333,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4294,7 +4346,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4306,7 +4358,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4368,7 +4420,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4380,7 +4432,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4394,7 +4446,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4406,7 +4458,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4421,7 +4473,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4437,7 +4489,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4449,7 +4501,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(200_000),
+                       htlc_maximum_msat: 200_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4462,7 +4514,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 1_000,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4474,7 +4526,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4535,7 +4587,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4548,7 +4600,7 @@ mod tests {
                        flags: 2,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4562,7 +4614,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4574,7 +4626,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4588,7 +4640,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4611,7 +4663,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(250_000),
+                       htlc_maximum_msat: 250_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4623,7 +4675,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4636,7 +4688,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 150_000,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4648,7 +4700,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4705,7 +4757,7 @@ mod tests {
                                cltv_expiry_delta: 42,
                                htlc_minimum_msat: None,
                                htlc_maximum_msat: None,
-                       }])]);
+                       }])]).with_max_channel_saturation_power_of_half(0);
 
                // Keep only two paths from us to nodes[2], both with a 99sat HTLC maximum, with one with
                // no fee and one with a 1msat fee. Previously, trying to route 100 sats to nodes[2] here
@@ -4719,7 +4771,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (5 << 4) | 5,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(99_000),
+                       htlc_maximum_msat: 99_000,
                        fee_base_msat: u32::max_value(),
                        fee_proportional_millionths: u32::max_value(),
                        excess_data: Vec::new()
@@ -4731,7 +4783,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (5 << 4) | 3,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(99_000),
+                       htlc_maximum_msat: 99_000,
                        fee_base_msat: u32::max_value(),
                        fee_proportional_millionths: u32::max_value(),
                        excess_data: Vec::new()
@@ -4743,7 +4795,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (4 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 1,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4755,7 +4807,7 @@ mod tests {
                        flags: 0|2, // Channel disabled
                        cltv_expiry_delta: (13 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 2000000,
                        excess_data: Vec::new()
@@ -4763,17 +4815,18 @@ mod tests {
 
                // Get a route for 100 sats and check that we found the MPP route no problem and didn't
                // overpay at all.
-               let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+               let mut route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
                assert_eq!(route.paths.len(), 2);
-               // Paths are somewhat randomly ordered, but:
-               // * the first is channel 2 (1 msat fee) -> channel 4 -> channel 42
-               // * the second is channel 1 (0 fee, but 99 sat maximum) -> channel 3 -> channel 42
-               assert_eq!(route.paths[0][0].short_channel_id, 2);
-               assert_eq!(route.paths[0][0].fee_msat, 1);
-               assert_eq!(route.paths[0][2].fee_msat, 1_000);
-               assert_eq!(route.paths[1][0].short_channel_id, 1);
-               assert_eq!(route.paths[1][0].fee_msat, 0);
-               assert_eq!(route.paths[1][2].fee_msat, 99_000);
+               route.paths.sort_by_key(|path| path[0].short_channel_id);
+               // Paths are manually ordered ordered by SCID, so:
+               // * the first is channel 1 (0 fee, but 99 sat maximum) -> channel 3 -> channel 42
+               // * the second is channel 2 (1 msat fee) -> channel 4 -> channel 42
+               assert_eq!(route.paths[0][0].short_channel_id, 1);
+               assert_eq!(route.paths[0][0].fee_msat, 0);
+               assert_eq!(route.paths[0][2].fee_msat, 99_000);
+               assert_eq!(route.paths[1][0].short_channel_id, 2);
+               assert_eq!(route.paths[1][0].fee_msat, 1);
+               assert_eq!(route.paths[1][2].fee_msat, 1_000);
                assert_eq!(route.get_total_fees(), 1);
                assert_eq!(route.get_total_amount(), 100_000);
        }
@@ -4787,7 +4840,8 @@ mod tests {
                let scorer = test_utils::TestScorer::with_penalty(0);
                let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
                let random_seed_bytes = keys_manager.get_secure_random_bytes();
-               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known())
+                       .with_max_channel_saturation_power_of_half(0);
 
                // We need a route consisting of 3 paths:
                // From our node to node2 via node0, node7, node1 (three paths one hop each).
@@ -4806,7 +4860,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(100_000),
+                       htlc_maximum_msat: 100_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4818,7 +4872,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(50_000),
+                       htlc_maximum_msat: 50_000,
                        fee_base_msat: 100,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4832,7 +4886,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(60_000),
+                       htlc_maximum_msat: 60_000,
                        fee_base_msat: 100,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4844,7 +4898,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(60_000),
+                       htlc_maximum_msat: 60_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4858,7 +4912,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(20_000),
+                       htlc_maximum_msat: 20_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4870,7 +4924,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(20_000),
+                       htlc_maximum_msat: 20_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4957,7 +5011,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (6 << 4) | 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4972,7 +5026,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (5 << 4) | 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 100,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -4987,7 +5041,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (4 << 4) | 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5002,7 +5056,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (3 << 4) | 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5017,7 +5071,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (2 << 4) | 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5031,7 +5085,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (1 << 4) | 0,
                        htlc_minimum_msat: 100,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5089,7 +5143,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(85_000),
+                       htlc_maximum_msat: 85_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5102,7 +5156,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (4 << 4) | 1,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(270_000),
+                       htlc_maximum_msat: 270_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 1000000,
                        excess_data: Vec::new()
@@ -5154,7 +5208,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(80_000),
+                       htlc_maximum_msat: 80_000,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5166,7 +5220,7 @@ mod tests {
                        flags: 0,
                        cltv_expiry_delta: (4 << 4) | 1,
                        htlc_minimum_msat: 90_000,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new()
@@ -5235,12 +5289,13 @@ mod tests {
                        assert_eq!(route.paths[0].len(), 1);
                        assert_eq!(route.paths[1].len(), 1);
 
+                       assert!((route.paths[0][0].short_channel_id == 3 && route.paths[1][0].short_channel_id == 2) ||
+                               (route.paths[0][0].short_channel_id == 2 && route.paths[1][0].short_channel_id == 3));
+
                        assert_eq!(route.paths[0][0].pubkey, nodes[0]);
-                       assert_eq!(route.paths[0][0].short_channel_id, 3);
                        assert_eq!(route.paths[0][0].fee_msat, 50_000);
 
                        assert_eq!(route.paths[1][0].pubkey, nodes[0]);
-                       assert_eq!(route.paths[1][0].short_channel_id, 2);
                        assert_eq!(route.paths[1][0].fee_msat, 50_000);
                }
 
@@ -5492,6 +5547,35 @@ mod tests {
                }
        }
 
+       #[test]
+       fn avoids_recently_failed_paths() {
+               // Ensure that the router always avoids all of the `previously_failed_channels` channels by
+               // randomly inserting channels into it until we can't find a route anymore.
+               let (secp_ctx, network, _, _, logger) = build_graph();
+               let (_, our_id, _, nodes) = get_nodes(&secp_ctx);
+               let network_graph = network.read_only();
+
+               let scorer = test_utils::TestScorer::with_penalty(0);
+               let mut payment_params = PaymentParameters::from_node_id(nodes[6]).with_route_hints(last_hops(&nodes))
+                       .with_max_path_count(1);
+               let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
+               let random_seed_bytes = keys_manager.get_secure_random_bytes();
+
+               // We should be able to find a route initially, and then after we fail a few random
+               // channels eventually we won't be able to any longer.
+               assert!(get_route(&our_id, &payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes).is_ok());
+               loop {
+                       if let Ok(route) = get_route(&our_id, &payment_params, &network_graph, None, 100, 0, Arc::clone(&logger), &scorer, &random_seed_bytes) {
+                               for chan in route.paths[0].iter() {
+                                       assert!(!payment_params.previously_failed_channels.contains(&chan.short_channel_id));
+                               }
+                               let victim = (u64::from_ne_bytes(random_seed_bytes[0..8].try_into().unwrap()) as usize)
+                                       % route.paths[0].len();
+                               payment_params.previously_failed_channels.push(route.paths[0][victim].short_channel_id);
+                       } else { break; }
+               }
+       }
+
        #[test]
        fn limits_path_length() {
                let (secp_ctx, network, _, _, logger) = build_line_graph();
@@ -5641,6 +5725,50 @@ mod tests {
                }
        }
 
+       #[test]
+       fn avoids_saturating_channels() {
+               let (secp_ctx, network_graph, gossip_sync, _, logger) = build_graph();
+               let (_, our_id, privkeys, nodes) = get_nodes(&secp_ctx);
+
+               let scorer = ProbabilisticScorer::new(Default::default(), &*network_graph, Arc::clone(&logger));
+
+               // Set the fee on channel 13 to 100% to match channel 4 giving us two equivalent paths (us
+               // -> node 7 -> node2 and us -> node 1 -> node 2) which we should balance over.
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 4,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: (4 << 4) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: 250_000_000,
+                       fee_base_msat: 0,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+               update_channel(&gossip_sync, &secp_ctx, &privkeys[7], UnsignedChannelUpdate {
+                       chain_hash: genesis_block(Network::Testnet).header.block_hash(),
+                       short_channel_id: 13,
+                       timestamp: 2,
+                       flags: 0,
+                       cltv_expiry_delta: (13 << 4) | 1,
+                       htlc_minimum_msat: 0,
+                       htlc_maximum_msat: 250_000_000,
+                       fee_base_msat: 0,
+                       fee_proportional_millionths: 0,
+                       excess_data: Vec::new()
+               });
+
+               let payment_params = PaymentParameters::from_node_id(nodes[2]).with_features(InvoiceFeatures::known());
+               let keys_manager = test_utils::TestKeysInterface::new(&[0u8; 32], Network::Testnet);
+               let random_seed_bytes = keys_manager.get_secure_random_bytes();
+               // 100,000 sats is less than the available liquidity on each channel, set above.
+               let route = get_route(&our_id, &payment_params, &network_graph.read_only(), None, 100_000_000, 42, Arc::clone(&logger), &scorer, &random_seed_bytes).unwrap();
+               assert_eq!(route.paths.len(), 2);
+               assert!((route.paths[0][1].short_channel_id == 4 && route.paths[1][1].short_channel_id == 13) ||
+                       (route.paths[1][1].short_channel_id == 4 && route.paths[0][1].short_channel_id == 13));
+       }
+
        #[cfg(not(feature = "no-std"))]
        pub(super) fn random_init_seed() -> u64 {
                // Because the default HashMap in std pulls OS randomness, we can use it as a (bad) RNG.
index 4922fad78476f56d62e6c5ebe859ce36323c93bf..458a24d9f1bbd7ff83434837d6cd8c83d05a19af 100644 (file)
@@ -394,6 +394,25 @@ pub struct ProbabilisticScoringParameters {
        ///
        /// 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.
+       ///
+       /// Note that in this case all other penalties, including the
+       /// [`liquidity_penalty_multiplier_msat`] and [`amount_penalty_multiplier_msat`]-based
+       /// penalties, as well as the [`base_penalty_msat`] and the [`anti_probing_penalty_msat`], if
+       /// applicable, are still included in the overall penalty.
+       ///
+       /// If you wish to avoid creating paths with such channels entirely, setting this to a value of
+       /// `u64::max_value()` will guarantee that.
+       ///
+       /// Default value: 1_0000_0000_000 msat (1 Bitcoin)
+       ///
+       /// [`liquidity_penalty_multiplier_msat`]: Self::liquidity_penalty_multiplier_msat
+       /// [`amount_penalty_multiplier_msat`]: Self::amount_penalty_multiplier_msat
+       /// [`base_penalty_msat`]: Self::base_penalty_msat
+       /// [`anti_probing_penalty_msat`]: Self::anti_probing_penalty_msat
+       pub considered_impossible_penalty_msat: u64,
 }
 
 /// Accounting for channel liquidity balance uncertainty.
@@ -522,6 +541,7 @@ impl ProbabilisticScoringParameters {
                        amount_penalty_multiplier_msat: 0,
                        manual_node_penalties: HashMap::new(),
                        anti_probing_penalty_msat: 0,
+                       considered_impossible_penalty_msat: 0,
                }
        }
 
@@ -543,6 +563,7 @@ impl Default for ProbabilisticScoringParameters {
                        amount_penalty_multiplier_msat: 256,
                        manual_node_penalties: HashMap::new(),
                        anti_probing_penalty_msat: 250,
+                       considered_impossible_penalty_msat: 1_0000_0000_000,
                }
        }
 }
@@ -620,17 +641,12 @@ impl<L: Deref<Target = u64>, T: Time, U: Deref<Target = T>> DirectedChannelLiqui
                if amount_msat <= min_liquidity_msat {
                        0
                } else if amount_msat >= max_liquidity_msat {
-                       if amount_msat > max_liquidity_msat {
-                               u64::max_value()
-                       } else if max_liquidity_msat != self.capacity_msat {
-                               // Avoid using the failed channel on retry.
-                               u64::max_value()
-                       } else {
-                               // Equivalent to hitting the else clause below with the amount equal to the
-                               // effective capacity and without any certainty on the liquidity upper bound.
-                               let negative_log10_times_2048 = NEGATIVE_LOG10_UPPER_BOUND * 2048;
-                               self.combined_penalty_msat(amount_msat, negative_log10_times_2048, params)
-                       }
+                       // Equivalent to hitting the else clause below with the amount equal to the effective
+                       // capacity and without any certainty on the liquidity upper bound, plus the
+                       // impossibility penalty.
+                       let negative_log10_times_2048 = NEGATIVE_LOG10_UPPER_BOUND * 2048;
+                       self.combined_penalty_msat(amount_msat, negative_log10_times_2048, params)
+                               .saturating_add(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);
@@ -1242,7 +1258,7 @@ mod tests {
        use util::time::tests::SinceEpoch;
 
        use ln::features::{ChannelFeatures, NodeFeatures};
-       use ln::msgs::{ChannelAnnouncement, ChannelUpdate, OptionalField, UnsignedChannelAnnouncement, UnsignedChannelUpdate};
+       use ln::msgs::{ChannelAnnouncement, ChannelUpdate, UnsignedChannelAnnouncement, UnsignedChannelUpdate};
        use routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId};
        use routing::router::RouteHop;
        use routing::scoring::{ChannelUsage, Score};
@@ -1369,7 +1385,7 @@ mod tests {
                        flags,
                        cltv_expiry_delta: 18,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Present(1_000),
+                       htlc_maximum_msat: 1_000,
                        fee_base_msat: 1,
                        fee_proportional_millionths: 0,
                        excess_data: Vec::new(),
@@ -1624,7 +1640,7 @@ mod tests {
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 0);
                let usage = ChannelUsage { amount_msat: 102_400, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 47);
-               let usage = ChannelUsage { amount_msat: 1_024_000, ..usage };
+               let usage = ChannelUsage { amount_msat: 1_023_999, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
 
                let usage = ChannelUsage {
@@ -1654,6 +1670,7 @@ mod tests {
                let network_graph = network_graph(&logger);
                let params = ProbabilisticScoringParameters {
                        liquidity_penalty_multiplier_msat: 1_000,
+                       considered_impossible_penalty_msat: u64::max_value(),
                        ..ProbabilisticScoringParameters::zero_penalty()
                };
                let scorer = ProbabilisticScorer::new(params, &network_graph, &logger)
@@ -1745,6 +1762,7 @@ mod tests {
                let network_graph = network_graph(&logger);
                let params = ProbabilisticScoringParameters {
                        liquidity_penalty_multiplier_msat: 1_000,
+                       considered_impossible_penalty_msat: u64::max_value(),
                        ..ProbabilisticScoringParameters::zero_penalty()
                };
                let mut scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
@@ -1811,6 +1829,7 @@ mod tests {
                let params = ProbabilisticScoringParameters {
                        liquidity_penalty_multiplier_msat: 1_000,
                        liquidity_offset_half_life: Duration::from_secs(10),
+                       considered_impossible_penalty_msat: u64::max_value(),
                        ..ProbabilisticScoringParameters::zero_penalty()
                };
                let mut scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
@@ -1820,10 +1839,10 @@ mod tests {
                let usage = ChannelUsage {
                        amount_msat: 0,
                        inflight_htlc_msat: 0,
-                       effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: Some(1_000) },
+                       effective_capacity: EffectiveCapacity::Total { capacity_msat: 1_024, htlc_maximum_msat: Some(1_024) },
                };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 0);
-               let usage = ChannelUsage { amount_msat: 1_024, ..usage };
+               let usage = ChannelUsage { amount_msat: 1_023, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
 
                scorer.payment_path_failed(&payment_path_for_amount(768).iter().collect::<Vec<_>>(), 42);
@@ -1867,20 +1886,20 @@ mod tests {
                let usage = ChannelUsage { amount_msat: 1_023, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
                let usage = ChannelUsage { amount_msat: 1_024, ..usage };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), u64::max_value());
 
                // Fully decay liquidity upper bound.
                SinceEpoch::advance(Duration::from_secs(10));
                let usage = ChannelUsage { amount_msat: 0, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 0);
                let usage = ChannelUsage { amount_msat: 1_024, ..usage };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), u64::max_value());
 
                SinceEpoch::advance(Duration::from_secs(10));
                let usage = ChannelUsage { amount_msat: 0, ..usage };
                assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 0);
                let usage = ChannelUsage { amount_msat: 1_024, ..usage };
-               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), 2_000);
+               assert_eq!(scorer.channel_penalty_msat(42, &source, &target, usage), u64::max_value());
        }
 
        #[test]
@@ -1965,6 +1984,7 @@ mod tests {
                let params = ProbabilisticScoringParameters {
                        liquidity_penalty_multiplier_msat: 1_000,
                        liquidity_offset_half_life: Duration::from_secs(10),
+                       considered_impossible_penalty_msat: u64::max_value(),
                        ..ProbabilisticScoringParameters::zero_penalty()
                };
                let mut scorer = ProbabilisticScorer::new(params.clone(), &network_graph, &logger);
@@ -2001,6 +2021,7 @@ mod tests {
                let params = ProbabilisticScoringParameters {
                        liquidity_penalty_multiplier_msat: 1_000,
                        liquidity_offset_half_life: Duration::from_secs(10),
+                       considered_impossible_penalty_msat: u64::max_value(),
                        ..ProbabilisticScoringParameters::zero_penalty()
                };
                let mut scorer = ProbabilisticScorer::new(params.clone(), &network_graph, &logger);
@@ -2171,7 +2192,10 @@ mod tests {
        fn accounts_for_inflight_htlc_usage() {
                let logger = TestLogger::new();
                let network_graph = network_graph(&logger);
-               let params = ProbabilisticScoringParameters::default();
+               let params = ProbabilisticScoringParameters {
+                       considered_impossible_penalty_msat: u64::max_value(),
+                       ..ProbabilisticScoringParameters::zero_penalty()
+               };
                let scorer = ProbabilisticScorer::new(params, &network_graph, &logger);
                let source = source_node_id();
                let target = target_node_id();
index 4f3d800be5dd8479f79a7999318fc627017e7078..c2d14073d990dd9c4a407c93c1a7fef87724f92d 100644 (file)
@@ -19,7 +19,6 @@ use chain::transaction::OutPoint;
 use chain::keysinterface;
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::{msgs, wire};
-use ln::msgs::OptionalField;
 use ln::script::ShutdownScript;
 use routing::scoring::FixedPenaltyScorer;
 use util::enforcing_trait_impls::{EnforcingSigner, EnforcementState};
@@ -407,7 +406,7 @@ fn get_dummy_channel_update(short_chan_id: u64) -> msgs::ChannelUpdate {
                        flags: 0,
                        cltv_expiry_delta: 0,
                        htlc_minimum_msat: 0,
-                       htlc_maximum_msat: OptionalField::Absent,
+                       htlc_maximum_msat: msgs::MAX_VALUE_MSAT,
                        fee_base_msat: 0,
                        fee_proportional_millionths: 0,
                        excess_data: vec![],