Merge pull request #890 from TheBlueMatt/2021-04-fix-chan-shutdown-crash
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 24 Apr 2021 00:03:42 +0000 (00:03 +0000)
committerGitHub <noreply@github.com>
Sat, 24 Apr 2021 00:03:42 +0000 (00:03 +0000)
Fix (and test) panic when our counterparty uses a bogus funding tx

19 files changed:
.github/workflows/build.yml
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
fuzz/src/router.rs
lightning-invoice/Cargo.toml
lightning-invoice/src/de.rs
lightning-invoice/src/lib.rs
lightning-invoice/src/ser.rs
lightning/src/chain/chainmonitor.rs
lightning/src/chain/channelmonitor.rs
lightning/src/chain/mod.rs
lightning/src/ln/chanmon_update_fail_tests.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/peer_handler.rs
lightning/src/ln/reorg_tests.rs
lightning/src/routing/router.rs

index a379d1cd64ba73c4c07c57336f698ce245214724..d71a47da4aaa163af589afb56c205285c6934569 100644 (file)
@@ -47,7 +47,7 @@ jobs:
         run: RUSTFLAGS="-C link-dead-code" cargo build --verbose --color always
       - name: Build on Rust ${{ matrix.toolchain }}
         if: "! matrix.build-net-tokio"
-        run: cargo build --verbose  --color always -p lightning
+        run: cargo build --verbose  --color always -p lightning && cargo build --verbose  --color always -p lightning-invoice
       - name: Build Block Sync Clients on Rust ${{ matrix.toolchain }} with features
         if: "matrix.build-net-tokio && !matrix.coverage"
         run: |
@@ -74,7 +74,7 @@ jobs:
         run: RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always
       - name: Test on Rust ${{ matrix.toolchain }}
         if: "! matrix.build-net-tokio"
-        run: cargo test --verbose --color always  -p lightning
+        run: cargo test --verbose --color always  -p lightning && cargo test --verbose --color always  -p lightning-invoice
       - name: Test Block Sync Clients on Rust ${{ matrix.toolchain }} with features
         if: "matrix.build-net-tokio && !matrix.coverage"
         run: |
index 9d0a90ee1cdc0736ea6e3384348136eaef44c9a8..96da87b624e4e54d8950fde5ecb55b376be2d69d 100644 (file)
@@ -30,6 +30,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hash_types::{BlockHash, WPubkeyHash};
 
 use lightning::chain;
+use lightning::chain::Confirm;
 use lightning::chain::chainmonitor;
 use lightning::chain::channelmonitor;
 use lightning::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, MonitorEvent};
@@ -428,11 +429,11 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                        let chain_hash = genesis_block(Network::Bitcoin).block_hash();
                        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: chain_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                        let txdata: Vec<_> = channel_txn.iter().enumerate().map(|(i, tx)| (i + 1, tx)).collect();
-                       $node.transactions_confirmed(&header, 1, &txdata);
+                       $node.transactions_confirmed(&header, &txdata, 1);
                        for _ in 2..100 {
                                header = BlockHeader { version: 0x20000000, prev_blockhash: header.block_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
                        }
-                       $node.update_best_block(&header, 99);
+                       $node.best_block_updated(&header, 99);
                } }
        }
 
index 91ebb99018cca6cc5171c964394a32d5604a0bd1..73a9a67a74ef8ed5b774abd20905635e531ea73e 100644 (file)
@@ -27,7 +27,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
 
 use lightning::chain;
-use lightning::chain::Listen;
+use lightning::chain::{Confirm, Listen};
 use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
 use lightning::chain::chainmonitor;
 use lightning::chain::transaction::OutPoint;
@@ -207,9 +207,10 @@ impl<'a> MoneyLossDetector<'a> {
                self.blocks_connected += 1;
                let header = BlockHeader { version: 0x20000000, prev_blockhash: self.header_hashes[self.height].0, merkle_root: Default::default(), time: self.blocks_connected, bits: 42, nonce: 42 };
                self.height += 1;
-               self.manager.transactions_confirmed(&header, self.height as u32, &txdata);
-               self.manager.update_best_block(&header, self.height as u32);
-               (*self.monitor).block_connected(&header, &txdata, self.height as u32);
+               self.manager.transactions_confirmed(&header, &txdata, self.height as u32);
+               self.manager.best_block_updated(&header, self.height as u32);
+               (*self.monitor).transactions_confirmed(&header, &txdata, self.height as u32);
+               (*self.monitor).best_block_updated(&header, self.height as u32);
                if self.header_hashes.len() > self.height {
                        self.header_hashes[self.height] = (header.block_hash(), self.blocks_connected);
                } else {
index fb720c9916c2d7b38ee653df9fea675f3f8ea23f..e80e080f31786bc76b40b10bc9b6fb96d27c8e2a 100644 (file)
@@ -15,7 +15,7 @@ use lightning::chain;
 use lightning::ln::channelmanager::ChannelDetails;
 use lightning::ln::features::InitFeatures;
 use lightning::ln::msgs;
-use lightning::routing::router::{get_route, RouteHint};
+use lightning::routing::router::{get_route, RouteHintHop};
 use lightning::util::logger::Logger;
 use lightning::util::ser::Readable;
 use lightning::routing::network_graph::{NetworkGraph, RoutingFees};
@@ -224,7 +224,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
                                        for _ in 0..count {
                                                scid += 1;
                                                let rnid = node_pks.iter().skip(slice_to_be16(get_slice!(2))as usize % node_pks.len()).next().unwrap();
-                                               last_hops_vec.push(RouteHint {
+                                               last_hops_vec.push(RouteHintHop {
                                                        src_node_id: *rnid,
                                                        short_channel_id: scid,
                                                        fees: RoutingFees {
index 47bf9d744dcb91f554a69400521e439759e03f66..758578ca465b5f6cf57e29440e5439002c0ced64 100644 (file)
@@ -10,6 +10,7 @@ readme = "README.md"
 
 [dependencies]
 bech32 = "0.7"
+lightning = { version = "0.0.13", path = "../lightning" }
 secp256k1 = { version = "0.20", features = ["recovery"] }
 num-traits = "0.2.8"
 bitcoin_hashes = "0.9.4"
index d6cb92072bc69fc56815c4ebd713f0ba1d29b562..fe77a93a99b03f9707563f56ea556c56d5ac7c76 100644 (file)
@@ -10,6 +10,8 @@ use bech32::{u5, FromBase32};
 
 use bitcoin_hashes::Hash;
 use bitcoin_hashes::sha256;
+use lightning::routing::network_graph::RoutingFees;
+use lightning::routing::router::RouteHintHop;
 
 use num_traits::{CheckedAdd, CheckedMul};
 
@@ -353,7 +355,7 @@ impl FromBase32 for Signature {
        }
 }
 
-fn parse_int_be<T, U>(digits: &[U], base: T) -> Option<T>
+pub(crate) fn parse_int_be<T, U>(digits: &[U], base: T) -> Option<T>
        where T: CheckedAdd + CheckedMul + From<u8> + Default,
              U: Into<u8> + Copy
 {
@@ -428,7 +430,7 @@ impl FromBase32 for TaggedField {
                        constants::TAG_FALLBACK =>
                                Ok(TaggedField::Fallback(Fallback::from_base32(field_data)?)),
                        constants::TAG_ROUTE =>
-                               Ok(TaggedField::Route(Route::from_base32(field_data)?)),
+                               Ok(TaggedField::Route(RouteHint::from_base32(field_data)?)),
                        constants::TAG_PAYMENT_SECRET =>
                                Ok(TaggedField::PaymentSecret(PaymentSecret::from_base32(field_data)?)),
                        _ => {
@@ -565,17 +567,17 @@ impl FromBase32 for Fallback {
        }
 }
 
-impl FromBase32 for Route {
+impl FromBase32 for RouteHint {
        type Err = ParseError;
 
-       fn from_base32(field_data: &[u5]) -> Result<Route, ParseError> {
+       fn from_base32(field_data: &[u5]) -> Result<RouteHint, ParseError> {
                let bytes = Vec::<u8>::from_base32(field_data)?;
 
                if bytes.len() % 51 != 0 {
                        return Err(ParseError::UnexpectedEndOfTaggedFields);
                }
 
-               let mut route_hops = Vec::<RouteHop>::new();
+               let mut route_hops = Vec::<RouteHintHop>::new();
 
                let mut bytes = bytes.as_slice();
                while !bytes.is_empty() {
@@ -585,18 +587,22 @@ impl FromBase32 for Route {
                        let mut channel_id: [u8; 8] = Default::default();
                        channel_id.copy_from_slice(&hop_bytes[33..41]);
 
-                       let hop = RouteHop {
-                               pubkey: PublicKey::from_slice(&hop_bytes[0..33])?,
-                               short_channel_id: channel_id,
-                               fee_base_msat: parse_int_be(&hop_bytes[41..45], 256).expect("slice too big?"),
-                               fee_proportional_millionths: parse_int_be(&hop_bytes[45..49], 256).expect("slice too big?"),
-                               cltv_expiry_delta: parse_int_be(&hop_bytes[49..51], 256).expect("slice too big?")
+                       let hop = RouteHintHop {
+                               src_node_id: PublicKey::from_slice(&hop_bytes[0..33])?,
+                               short_channel_id: parse_int_be(&channel_id, 256).expect("short chan ID slice too big?"),
+                               fees: RoutingFees {
+                                       base_msat: parse_int_be(&hop_bytes[41..45], 256).expect("slice too big?"),
+                                       proportional_millionths: parse_int_be(&hop_bytes[45..49], 256).expect("slice too big?"),
+                               },
+                               cltv_expiry_delta: parse_int_be(&hop_bytes[49..51], 256).expect("slice too big?"),
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
                        };
 
                        route_hops.push(hop);
                }
 
-               Ok(Route(route_hops))
+               Ok(RouteHint(route_hops))
        }
 }
 
@@ -931,47 +937,57 @@ mod test {
 
        #[test]
        fn test_parse_route() {
-               use RouteHop;
-               use ::Route;
+               use lightning::routing::network_graph::RoutingFees;
+               use lightning::routing::router::RouteHintHop;
+               use ::RouteHint;
                use bech32::FromBase32;
+               use de::parse_int_be;
 
                let input = from_bech32(
                        "q20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqa\
                        fqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzq".as_bytes()
                );
 
-               let mut expected = Vec::<RouteHop>::new();
-               expected.push(RouteHop {
-                       pubkey: PublicKey::from_slice(
+               let mut expected = Vec::<RouteHintHop>::new();
+               expected.push(RouteHintHop {
+                       src_node_id: PublicKey::from_slice(
                                &[
                                        0x02u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
                                        0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
                                        0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
                                ][..]
                        ).unwrap(),
-                       short_channel_id: [0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08],
-                       fee_base_msat: 1,
-                       fee_proportional_millionths: 20,
-                       cltv_expiry_delta: 3
+                       short_channel_id: parse_int_be(&[0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08], 256).expect("short chan ID slice too big?"),
+                       fees: RoutingFees {
+                               base_msat: 1,
+                               proportional_millionths: 20,
+                       },
+                       cltv_expiry_delta: 3,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None
                });
-               expected.push(RouteHop {
-                       pubkey: PublicKey::from_slice(
+               expected.push(RouteHintHop {
+                       src_node_id: PublicKey::from_slice(
                                &[
                                        0x03u8, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4, 0x3c,
                                        0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a, 0x95, 0xc3,
                                        0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
                                ][..]
                        ).unwrap(),
-                       short_channel_id: [0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a],
-                       fee_base_msat: 2,
-                       fee_proportional_millionths: 30,
-                       cltv_expiry_delta: 4
+                       short_channel_id: parse_int_be(&[0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a], 256).expect("short chan ID slice too big?"),
+                       fees: RoutingFees {
+                               base_msat: 2,
+                               proportional_millionths: 30,
+                       },
+                       cltv_expiry_delta: 4,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None
                });
 
-               assert_eq!(Route::from_base32(&input), Ok(Route(expected)));
+               assert_eq!(RouteHint::from_base32(&input), Ok(RouteHint(expected)));
 
                assert_eq!(
-                       Route::from_base32(&[u5::try_from_u8(0).unwrap(); 40][..]),
+                       RouteHint::from_base32(&[u5::try_from_u8(0).unwrap(); 40][..]),
                        Err(ParseError::UnexpectedEndOfTaggedFields)
                );
        }
index e9ca442f2b2006ea5ff343a0d4785ce7dc945899..2ce022248d0ad2479247bb4b9e2d29ff7d0451ca 100644 (file)
 
 extern crate bech32;
 extern crate bitcoin_hashes;
+extern crate lightning;
 extern crate num_traits;
 extern crate secp256k1;
 
 use bech32::u5;
 use bitcoin_hashes::Hash;
 use bitcoin_hashes::sha256;
+#[cfg(any(doc, test))]
+use lightning::routing::network_graph::RoutingFees;
+use lightning::routing::router::RouteHintHop;
 
 use secp256k1::key::PublicKey;
 use secp256k1::{Message, Secp256k1};
@@ -323,7 +327,7 @@ pub enum TaggedField {
        ExpiryTime(ExpiryTime),
        MinFinalCltvExpiry(MinFinalCltvExpiry),
        Fallback(Fallback),
-       Route(Route),
+       Route(RouteHint),
        PaymentSecret(PaymentSecret),
 }
 
@@ -383,26 +387,7 @@ pub struct Signature(pub RecoverableSignature);
 /// The encoded route has to be <1024 5bit characters long (<=639 bytes or <=12 hops)
 ///
 #[derive(Eq, PartialEq, Debug, Clone)]
-pub struct Route(Vec<RouteHop>);
-
-/// Node on a private route
-#[derive(Eq, PartialEq, Debug, Clone)]
-pub struct RouteHop {
-       /// Node's public key
-       pub pubkey: PublicKey,
-
-       /// Which channel of this node we would be using
-       pub short_channel_id: [u8; 8],
-
-       /// Fee charged by this node per transaction
-       pub fee_base_msat: u32,
-
-       /// Fee charged by this node proportional to the amount routed
-       pub fee_proportional_millionths: u32,
-
-       /// Delta substracted by this node from incoming cltv_expiry value
-       pub cltv_expiry_delta: u16,
-}
+pub struct RouteHint(Vec<RouteHintHop>);
 
 /// Tag constants as specified in BOLT11
 #[allow(missing_docs)]
@@ -499,8 +484,8 @@ impl<D: tb::Bool, H: tb::Bool, T: tb::Bool> InvoiceBuilder<D, H, T> {
        }
 
        /// Adds a private route.
-       pub fn route(mut self, route: Vec<RouteHop>) -> Self {
-               match Route::new(route) {
+       pub fn route(mut self, route: Vec<RouteHintHop>) -> Self {
+               match RouteHint::new(route) {
                        Ok(r) => self.tagged_fields.push(TaggedField::Route(r)),
                        Err(e) => self.error = Some(e),
                }
@@ -832,11 +817,11 @@ impl RawInvoice {
                }).collect::<Vec<&Fallback>>()
        }
 
-       pub fn routes(&self) -> Vec<&Route> {
+       pub fn routes(&self) -> Vec<&RouteHint> {
                self.known_tagged_fields().filter_map(|tf| match tf {
                        &TaggedField::Route(ref r) => Some(r),
                        _ => None,
-               }).collect::<Vec<&Route>>()
+               }).collect::<Vec<&RouteHint>>()
        }
 
        pub fn amount_pico_btc(&self) -> Option<u64> {
@@ -1035,7 +1020,7 @@ impl Invoice {
        }
 
        /// Returns a list of all routes included in the invoice
-       pub fn routes(&self) -> Vec<&Route> {
+       pub fn routes(&self) -> Vec<&RouteHint> {
                self.signed_invoice.routes()
        }
 
@@ -1157,32 +1142,32 @@ impl ExpiryTime {
        }
 }
 
-impl Route {
+impl RouteHint {
        /// Create a new (partial) route from a list of hops
-       pub fn new(hops: Vec<RouteHop>) -> Result<Route, CreationError> {
+       pub fn new(hops: Vec<RouteHintHop>) -> Result<RouteHint, CreationError> {
                if hops.len() <= 12 {
-                       Ok(Route(hops))
+                       Ok(RouteHint(hops))
                } else {
                        Err(CreationError::RouteTooLong)
                }
        }
 
        /// Returrn the underlying vector of hops
-       pub fn into_inner(self) -> Vec<RouteHop> {
+       pub fn into_inner(self) -> Vec<RouteHintHop> {
                self.0
        }
 }
 
-impl Into<Vec<RouteHop>> for Route {
-       fn into(self) -> Vec<RouteHop> {
+impl Into<Vec<RouteHintHop>> for RouteHint {
+       fn into(self) -> Vec<RouteHintHop> {
                self.into_inner()
        }
 }
 
-impl Deref for Route {
-       type Target = Vec<RouteHop>;
+impl Deref for RouteHint {
+       type Target = Vec<RouteHintHop>;
 
-       fn deref(&self) -> &Vec<RouteHop> {
+       fn deref(&self) -> &Vec<RouteHintHop> {
                &self.0
        }
 }
@@ -1458,18 +1443,22 @@ mod test {
                        .build_raw();
                assert_eq!(long_desc_res, Err(CreationError::DescriptionTooLong));
 
-               let route_hop = RouteHop {
-                       pubkey: PublicKey::from_slice(
+               let route_hop = RouteHintHop {
+                       src_node_id: PublicKey::from_slice(
                                        &[
                                                0x03, 0x9e, 0x03, 0xa9, 0x01, 0xb8, 0x55, 0x34, 0xff, 0x1e, 0x92, 0xc4,
                                                0x3c, 0x74, 0x43, 0x1f, 0x7c, 0xe7, 0x20, 0x46, 0x06, 0x0f, 0xcf, 0x7a,
                                                0x95, 0xc3, 0x7e, 0x14, 0x8f, 0x78, 0xc7, 0x72, 0x55
                                        ][..]
                                ).unwrap(),
-                       short_channel_id: [0; 8],
-                       fee_base_msat: 0,
-                       fee_proportional_millionths: 0,
+                       short_channel_id: 0,
+                       fees: RoutingFees {
+                               base_msat: 0,
+                               proportional_millionths: 0,
+                       },
                        cltv_expiry_delta: 0,
+                       htlc_minimum_msat: None,
+                       htlc_maximum_msat: None,
                };
                let too_long_route = vec![route_hop; 13];
                let long_route_res = builder.clone()
@@ -1505,36 +1494,52 @@ mod test {
                let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
 
                let route_1 = vec![
-                       RouteHop {
-                               pubkey: public_key.clone(),
-                               short_channel_id: [123; 8],
-                               fee_base_msat: 2,
-                               fee_proportional_millionths: 1,
+                       RouteHintHop {
+                               src_node_id: public_key.clone(),
+                               short_channel_id: de::parse_int_be(&[123; 8], 256).expect("short chan ID slice too big?"),
+                               fees: RoutingFees {
+                                       base_msat: 2,
+                                       proportional_millionths: 1,
+                               },
                                cltv_expiry_delta: 145,
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
                        },
-                       RouteHop {
-                               pubkey: public_key.clone(),
-                               short_channel_id: [42; 8],
-                               fee_base_msat: 3,
-                               fee_proportional_millionths: 2,
+                       RouteHintHop {
+                               src_node_id: public_key.clone(),
+                               short_channel_id: de::parse_int_be(&[42; 8], 256).expect("short chan ID slice too big?"),
+                               fees: RoutingFees {
+                                       base_msat: 3,
+                                       proportional_millionths: 2,
+                               },
                                cltv_expiry_delta: 146,
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
                        }
                ];
 
                let route_2 = vec![
-                       RouteHop {
-                               pubkey: public_key.clone(),
-                               short_channel_id: [0; 8],
-                               fee_base_msat: 4,
-                               fee_proportional_millionths: 3,
+                       RouteHintHop {
+                               src_node_id: public_key.clone(),
+                               short_channel_id: 0,
+                               fees: RoutingFees {
+                                       base_msat: 4,
+                                       proportional_millionths: 3,
+                               },
                                cltv_expiry_delta: 147,
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
                        },
-                       RouteHop {
-                               pubkey: public_key.clone(),
-                               short_channel_id: [1; 8],
-                               fee_base_msat: 5,
-                               fee_proportional_millionths: 4,
+                       RouteHintHop {
+                               src_node_id: public_key.clone(),
+                               short_channel_id: de::parse_int_be(&[1; 8], 256).expect("short chan ID slice too big?"),
+                               fees: RoutingFees {
+                                       base_msat: 5,
+                                       proportional_millionths: 4,
+                               },
                                cltv_expiry_delta: 148,
+                               htlc_minimum_msat: None,
+                               htlc_maximum_msat: None,
                        }
                ];
 
@@ -1568,7 +1573,7 @@ mod test {
                assert_eq!(invoice.expiry_time(), Duration::from_secs(54321));
                assert_eq!(invoice.min_final_cltv_expiry(), Some(&144));
                assert_eq!(invoice.fallbacks(), vec![&Fallback::PubKeyHash([0;20])]);
-               assert_eq!(invoice.routes(), vec![&Route(route_1), &Route(route_2)]);
+               assert_eq!(invoice.routes(), vec![&RouteHint(route_1), &RouteHint(route_2)]);
                assert_eq!(
                        invoice.description(),
                        InvoiceDescription::Hash(&Sha256(sha256::Hash::from_slice(&[3;32][..]).unwrap()))
index 2b4332f8633bea9c16fd943de8844d99b5b69e68..83888e8269da1af358cd8cbf851d6e8b2c0e06da 100644 (file)
@@ -365,22 +365,26 @@ impl Base32Len for Fallback {
        }
 }
 
-impl ToBase32 for Route {
+impl ToBase32 for RouteHint {
        fn write_base32<W: WriteBase32>(&self, writer: &mut W) -> Result<(), <W as WriteBase32>::Err> {
                let mut converter = BytesToBase32::new(writer);
 
                for hop in self.iter() {
-                       converter.append(&hop.pubkey.serialize()[..])?;
-                       converter.append(&hop.short_channel_id[..])?;
+                       converter.append(&hop.src_node_id.serialize()[..])?;
+                       let short_channel_id = try_stretch(
+                               encode_int_be_base256(hop.short_channel_id),
+                               8
+                       ).expect("sizeof(u64) == 8");
+                       converter.append(&short_channel_id)?;
 
                        let fee_base_msat = try_stretch(
-                               encode_int_be_base256(hop.fee_base_msat),
+                               encode_int_be_base256(hop.fees.base_msat),
                                4
                        ).expect("sizeof(u32) == 4");
                        converter.append(&fee_base_msat)?;
 
                        let fee_proportional_millionths = try_stretch(
-                               encode_int_be_base256(hop.fee_proportional_millionths),
+                               encode_int_be_base256(hop.fees.proportional_millionths),
                                4
                        ).expect("sizeof(u32) == 4");
                        converter.append(&fee_proportional_millionths)?;
@@ -397,7 +401,7 @@ impl ToBase32 for Route {
        }
 }
 
-impl Base32Len for Route {
+impl Base32Len for RouteHint {
        fn base32_len(&self) -> usize {
                bytes_size_to_base32_size(self.0.len() * 51)
        }
index ee6df63a1e07ee272fda9f2217526e15c3d7beca..95a786f696dbc29174324f540590166c36d09553 100644 (file)
@@ -74,7 +74,7 @@ where C::Target: chain::Filter,
            P::Target: channelmonitor::Persist<ChannelSigner>,
 {
        /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
-       /// of a channel and reacting accordingly based on transactions in the connected block. See
+       /// of a channel and reacting accordingly based on transactions in the given chain data. See
        /// [`ChannelMonitor::block_connected`] for details. Any HTLCs that were resolved on chain will
        /// be returned by [`chain::Watch::release_pending_monitor_events`].
        ///
@@ -82,56 +82,6 @@ where C::Target: chain::Filter,
        /// calls must not exclude any transactions matching the new outputs nor any in-block
        /// descendants of such transactions. It is not necessary to re-fetch the block to obtain
        /// updated `txdata`.
-       pub fn block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
-               self.process_chain_data(header, txdata, |monitor, txdata| {
-                       monitor.block_connected(
-                               header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
-               });
-       }
-
-       /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
-       /// of a channel and reacting accordingly to newly confirmed transactions. For details, see
-       /// [`ChannelMonitor::transactions_confirmed`].
-       ///
-       /// Used instead of [`block_connected`] by clients that are notified of transactions rather than
-       /// blocks. May be called before or after [`update_best_block`] for transactions in the
-       /// corresponding block. See [`update_best_block`] for further calling expectations.
-       ///
-       /// [`block_connected`]: Self::block_connected
-       /// [`update_best_block`]: Self::update_best_block
-       pub fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
-               self.process_chain_data(header, txdata, |monitor, txdata| {
-                       monitor.transactions_confirmed(
-                               header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
-               });
-       }
-
-       /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
-       /// of a channel and reacting accordingly based on the new chain tip. For details, see
-       /// [`ChannelMonitor::update_best_block`].
-       ///
-       /// Used instead of [`block_connected`] by clients that are notified of transactions rather than
-       /// blocks. May be called before or after [`transactions_confirmed`] for the corresponding
-       /// block.
-       ///
-       /// Must be called after new blocks become available for the most recent block. Intermediary
-       /// blocks, however, may be safely skipped. In the event of a chain re-organization, this only
-       /// needs to be called for the most recent block assuming `transaction_unconfirmed` is called
-       /// for any affected transactions.
-       ///
-       /// [`block_connected`]: Self::block_connected
-       /// [`transactions_confirmed`]: Self::transactions_confirmed
-       /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
-       pub fn update_best_block(&self, header: &BlockHeader, height: u32) {
-               self.process_chain_data(header, &[], |monitor, txdata| {
-                       // While in practice there shouldn't be any recursive calls when given empty txdata,
-                       // it's still possible if a chain::Filter implementation returns a transaction.
-                       debug_assert!(txdata.is_empty());
-                       monitor.update_best_block(
-                               header, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
-               });
-       }
-
        fn process_chain_data<FN>(&self, header: &BlockHeader, txdata: &TransactionData, process: FN)
        where
                FN: Fn(&ChannelMonitor<ChannelSigner>, &TransactionData) -> Vec<TransactionOutputs>
@@ -172,46 +122,6 @@ where C::Target: chain::Filter,
                }
        }
 
-       /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
-       /// of a channel based on the disconnected block. See [`ChannelMonitor::block_disconnected`] for
-       /// details.
-       pub fn block_disconnected(&self, header: &BlockHeader, disconnected_height: u32) {
-               let monitors = self.monitors.read().unwrap();
-               for monitor in monitors.values() {
-                       monitor.block_disconnected(header, disconnected_height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
-               }
-       }
-
-       /// Dispatches to per-channel monitors, which are responsible for updating their on-chain view
-       /// of a channel based on transactions unconfirmed as a result of a chain reorganization. See
-       /// [`ChannelMonitor::transaction_unconfirmed`] for details.
-       ///
-       /// Used instead of [`block_disconnected`] by clients that are notified of transactions rather
-       /// than blocks. May be called before or after [`update_best_block`] for transactions in the
-       /// corresponding block. See [`update_best_block`] for further calling expectations. 
-       ///
-       /// [`block_disconnected`]: Self::block_disconnected
-       /// [`update_best_block`]: Self::update_best_block
-       pub fn transaction_unconfirmed(&self, txid: &Txid) {
-               let monitors = self.monitors.read().unwrap();
-               for monitor in monitors.values() {
-                       monitor.transaction_unconfirmed(txid, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
-               }
-       }
-
-       /// Returns the set of txids that should be monitored for re-organization out of the chain.
-       pub fn get_relevant_txids(&self) -> Vec<Txid> {
-               let mut txids = Vec::new();
-               let monitors = self.monitors.read().unwrap();
-               for monitor in monitors.values() {
-                       txids.append(&mut monitor.get_relevant_txids());
-               }
-
-               txids.sort_unstable();
-               txids.dedup();
-               txids
-       }
-
        /// Creates a new `ChainMonitor` used to watch on-chain activity pertaining to channels.
        ///
        /// When an optional chain source implementing [`chain::Filter`] is provided, the chain monitor
@@ -231,7 +141,7 @@ where C::Target: chain::Filter,
        }
 }
 
-impl<ChannelSigner: Sign, C: Deref + Send + Sync, T: Deref + Send + Sync, F: Deref + Send + Sync, L: Deref + Send + Sync, P: Deref + Send + Sync>
+impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
 chain::Listen for ChainMonitor<ChannelSigner, C, T, F, L, P>
 where
        ChannelSigner: Sign,
@@ -242,12 +152,67 @@ where
        P::Target: channelmonitor::Persist<ChannelSigner>,
 {
        fn block_connected(&self, block: &Block, height: u32) {
+               let header = &block.header;
                let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-               ChainMonitor::block_connected(self, &block.header, &txdata, height);
+               self.process_chain_data(header, &txdata, |monitor, txdata| {
+                       monitor.block_connected(
+                               header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
+               });
        }
 
        fn block_disconnected(&self, header: &BlockHeader, height: u32) {
-               ChainMonitor::block_disconnected(self, header, height);
+               let monitors = self.monitors.read().unwrap();
+               for monitor in monitors.values() {
+                       monitor.block_disconnected(
+                               header, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+               }
+       }
+}
+
+impl<ChannelSigner: Sign, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
+chain::Confirm for ChainMonitor<ChannelSigner, C, T, F, L, P>
+where
+       ChannelSigner: Sign,
+       C::Target: chain::Filter,
+       T::Target: BroadcasterInterface,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
+       P::Target: channelmonitor::Persist<ChannelSigner>,
+{
+       fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               self.process_chain_data(header, txdata, |monitor, txdata| {
+                       monitor.transactions_confirmed(
+                               header, txdata, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
+               });
+       }
+
+       fn transaction_unconfirmed(&self, txid: &Txid) {
+               let monitors = self.monitors.read().unwrap();
+               for monitor in monitors.values() {
+                       monitor.transaction_unconfirmed(txid, &*self.broadcaster, &*self.fee_estimator, &*self.logger);
+               }
+       }
+
+       fn best_block_updated(&self, header: &BlockHeader, height: u32) {
+               self.process_chain_data(header, &[], |monitor, txdata| {
+                       // While in practice there shouldn't be any recursive calls when given empty txdata,
+                       // it's still possible if a chain::Filter implementation returns a transaction.
+                       debug_assert!(txdata.is_empty());
+                       monitor.best_block_updated(
+                               header, height, &*self.broadcaster, &*self.fee_estimator, &*self.logger)
+               });
+       }
+
+       fn get_relevant_txids(&self) -> Vec<Txid> {
+               let mut txids = Vec::new();
+               let monitors = self.monitors.read().unwrap();
+               for monitor in monitors.values() {
+                       txids.append(&mut monitor.get_relevant_txids());
+               }
+
+               txids.sort_unstable();
+               txids.dedup();
+               txids
        }
 }
 
index 573d19df50f94a9062af19b82988b18185cb1241..f0b23c08619632c4ab14d45f5322d51cc017c38d 100644 (file)
@@ -1311,11 +1311,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        /// outputs to watch. See [`block_connected`] for details.
        ///
        /// Used instead of [`block_connected`] by clients that are notified of transactions rather than
-       /// blocks. May be called before or after [`update_best_block`] for transactions in the
-       /// corresponding block. See [`update_best_block`] for further calling expectations. 
+       /// blocks. See [`chain::Confirm`] for calling expectations.
        ///
        /// [`block_connected`]: Self::block_connected
-       /// [`update_best_block`]: Self::update_best_block
        pub fn transactions_confirmed<B: Deref, F: Deref, L: Deref>(
                &self,
                header: &BlockHeader,
@@ -1337,11 +1335,9 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        /// Processes a transaction that was reorganized out of the chain.
        ///
        /// Used instead of [`block_disconnected`] by clients that are notified of transactions rather
-       /// than blocks. May be called before or after [`update_best_block`] for transactions in the
-       /// corresponding block. See [`update_best_block`] for further calling expectations.
+       /// than blocks. See [`chain::Confirm`] for calling expectations.
        ///
        /// [`block_disconnected`]: Self::block_disconnected
-       /// [`update_best_block`]: Self::update_best_block
        pub fn transaction_unconfirmed<B: Deref, F: Deref, L: Deref>(
                &self,
                txid: &Txid,
@@ -1361,18 +1357,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        /// [`block_connected`] for details.
        ///
        /// Used instead of [`block_connected`] by clients that are notified of transactions rather than
-       /// blocks. May be called before or after [`transactions_confirmed`] for the corresponding
-       /// block.
-       ///
-       /// Must be called after new blocks become available for the most recent block. Intermediary
-       /// blocks, however, may be safely skipped. In the event of a chain re-organization, this only
-       /// needs to be called for the most recent block assuming `transaction_unconfirmed` is called
-       /// for any affected transactions.
+       /// blocks. See [`chain::Confirm`] for calling expectations.
        ///
        /// [`block_connected`]: Self::block_connected
-       /// [`transactions_confirmed`]: Self::transactions_confirmed
-       /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
-       pub fn update_best_block<B: Deref, F: Deref, L: Deref>(
+       pub fn best_block_updated<B: Deref, F: Deref, L: Deref>(
                &self,
                header: &BlockHeader,
                height: u32,
@@ -1385,7 +1373,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
-               self.inner.lock().unwrap().update_best_block(
+               self.inner.lock().unwrap().best_block_updated(
                        header, height, broadcaster, fee_estimator, logger)
        }
 
@@ -2109,7 +2097,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                self.transactions_confirmed(header, txdata, height, broadcaster, fee_estimator, logger)
        }
 
-       fn update_best_block<B: Deref, F: Deref, L: Deref>(
+       fn best_block_updated<B: Deref, F: Deref, L: Deref>(
                &mut self,
                header: &BlockHeader,
                height: u32,
@@ -2727,6 +2715,29 @@ where
        }
 }
 
+impl<Signer: Sign, T: Deref, F: Deref, L: Deref> chain::Confirm for (ChannelMonitor<Signer>, T, F, L)
+where
+       T::Target: BroadcasterInterface,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
+{
+       fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               self.0.transactions_confirmed(header, txdata, height, &*self.1, &*self.2, &*self.3);
+       }
+
+       fn transaction_unconfirmed(&self, txid: &Txid) {
+               self.0.transaction_unconfirmed(txid, &*self.1, &*self.2, &*self.3);
+       }
+
+       fn best_block_updated(&self, header: &BlockHeader, height: u32) {
+               self.0.best_block_updated(header, height, &*self.1, &*self.2, &*self.3);
+       }
+
+       fn get_relevant_txids(&self) -> Vec<Txid> {
+               self.0.get_relevant_txids()
+       }
+}
+
 const MAX_ALLOC_SIZE: usize = 64*1024;
 
 impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
index 18c7fd55d7b6643bbbc140a5208df42e39e42ea3..7d25a066076380896b54040edd3c361faf6494d2 100644 (file)
@@ -16,7 +16,7 @@ use bitcoin::hash_types::{BlockHash, Txid};
 
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateErr, MonitorEvent};
 use chain::keysinterface::Sign;
-use chain::transaction::OutPoint;
+use chain::transaction::{OutPoint, TransactionData};
 
 pub mod chaininterface;
 pub mod chainmonitor;
@@ -45,10 +45,13 @@ pub trait Access: Send + Sync {
        fn get_utxo(&self, genesis_hash: &BlockHash, short_channel_id: u64) -> Result<TxOut, AccessError>;
 }
 
-/// The `Listen` trait is used to be notified of when blocks have been connected or disconnected
-/// from the chain.
+/// The `Listen` trait is used to notify when blocks have been connected or disconnected from the
+/// chain.
 ///
-/// Useful when needing to replay chain data upon startup or as new chain events occur.
+/// Useful when needing to replay chain data upon startup or as new chain events occur. Clients
+/// sourcing chain data using a block-oriented API should prefer this interface over [`Confirm`].
+/// Such clients fetch the entire header chain whereas clients using [`Confirm`] only fetch headers
+/// when needed.
 pub trait Listen {
        /// Notifies the listener that a block was added at the given height.
        fn block_connected(&self, block: &Block, height: u32);
@@ -57,6 +60,86 @@ pub trait Listen {
        fn block_disconnected(&self, header: &BlockHeader, height: u32);
 }
 
+/// The `Confirm` trait is used to notify when transactions have been confirmed on chain or
+/// unconfirmed during a chain reorganization.
+///
+/// Clients sourcing chain data using a transaction-oriented API should prefer this interface over
+/// [`Listen`]. For instance, an Electrum client may implement [`Filter`] by subscribing to activity
+/// related to registered transactions and outputs. Upon notification, it would pass along the
+/// matching transactions using this interface.
+///
+/// # Use
+///
+/// The intended use is as follows:
+/// - Call [`transactions_confirmed`] to process any on-chain activity of interest.
+/// - Call [`transaction_unconfirmed`] to process any transaction returned by [`get_relevant_txids`]
+///   that has been reorganized out of the chain.
+/// - Call [`best_block_updated`] whenever a new chain tip becomes available.
+///
+/// # Order
+///
+/// Clients must call these methods in chain order. Specifically:
+/// - Transactions confirmed in a block must be given before transactions confirmed in a later
+///   block.
+/// - Dependent transactions within the same block must be given in topological order, possibly in
+///   separate calls.
+/// - Unconfirmed transactions must be given after the original confirmations and before any
+///   reconfirmation.
+///
+/// See individual method documentation for further details.
+///
+/// [`transactions_confirmed`]: Self::transactions_confirmed
+/// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
+/// [`best_block_updated`]: Self::best_block_updated
+/// [`get_relevant_txids`]: Self::get_relevant_txids
+pub trait Confirm {
+       /// Processes transactions confirmed in a block with a given header and height.
+       ///
+       /// Should be called for any transactions registered by [`Filter::register_tx`] or any
+       /// transactions spending an output registered by [`Filter::register_output`]. Such transactions
+       /// appearing in the same block do not need to be included in the same call; instead, multiple
+       /// calls with additional transactions may be made so long as they are made in [chain order].
+       ///
+       /// May be called before or after [`best_block_updated`] for the corresponding block. However,
+       /// in the event of a chain reorganization, it must not be called with a `header` that is no
+       /// longer in the chain as of the last call to [`best_block_updated`].
+       ///
+       /// [chain order]: Self#order
+       /// [`best_block_updated`]: Self::best_block_updated
+       fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32);
+
+       /// Processes a transaction that is no longer confirmed as result of a chain reorganization.
+       ///
+       /// Should be called for any transaction returned by [`get_relevant_txids`] if it has been
+       /// reorganized out of the best chain. Once called, the given transaction should not be returned
+       /// by [`get_relevant_txids`] unless it has been reconfirmed via [`transactions_confirmed`].
+       ///
+       /// [`get_relevant_txids`]: Self::get_relevant_txids
+       /// [`transactions_confirmed`]: Self::transactions_confirmed
+       fn transaction_unconfirmed(&self, txid: &Txid);
+
+       /// Processes an update to the best header connected at the given height.
+       ///
+       /// Should be called when a new header is available but may be skipped for intermediary blocks
+       /// if they become available at the same time.
+       fn best_block_updated(&self, header: &BlockHeader, height: u32);
+
+       /// Returns transactions that should be monitored for reorganization out of the chain.
+       ///
+       /// Should include any transactions passed to [`transactions_confirmed`] that have insufficient
+       /// confirmations to be safe from a chain reorganization. Should not include any transactions
+       /// passed to [`transaction_unconfirmed`] unless later reconfirmed.
+       ///
+       /// May be called to determine the subset of transactions that must still be monitored for
+       /// reorganization. Will be idempotent between calls but may change as a result of calls to the
+       /// other interface methods. Thus, this is useful to determine which transactions may need to be
+       /// given to [`transaction_unconfirmed`].
+       ///
+       /// [`transactions_confirmed`]: Self::transactions_confirmed
+       /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
+       fn get_relevant_txids(&self) -> Vec<Txid>;
+}
+
 /// The `Watch` trait defines behavior for watching on-chain activity pertaining to channels as
 /// blocks are connected and disconnected.
 ///
index a7cc5377a2532a14644619e623a64ed6a0278ad1..ee9936b3329222a9974a93014849ad38be82bd3b 100644 (file)
 //! There are a bunch of these as their handling is relatively error-prone so they are split out
 //! here. See also the chanmon_fail_consistency fuzz test.
 
-use bitcoin::blockdata::block::BlockHeader;
+use bitcoin::blockdata::block::{Block, BlockHeader};
 use bitcoin::hash_types::BlockHash;
 use bitcoin::network::constants::Network;
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr};
 use chain::transaction::OutPoint;
+use chain::Listen;
 use chain::Watch;
 use ln::channelmanager::{RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure};
 use ln::features::InitFeatures;
@@ -114,7 +115,7 @@ fn test_monitor_and_persister_update_fail() {
                chain_mon
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       chain_mon.chain_monitor.block_connected(&header, &[], 200);
+       chain_mon.chain_monitor.block_connected(&Block { header, txdata: vec![] }, 200);
 
        // Set the persister's return value to be a TemporaryFailure.
        persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
index 1c34e3391493fe33a9bddff6c2c680a66f9eac34..8a1489d4ef775c7be7d515bcc3153895437eb726 100644 (file)
@@ -3599,7 +3599,7 @@ impl<Signer: Sign> Channel<Signer> {
                                                }
                                        }
                                        // If we allow 1-conf funding, we may need to check for funding_locked here and
-                                       // send it immediately instead of waiting for an update_best_block call (which
+                                       // send it immediately instead of waiting for a best_block_updated call (which
                                        // may have already happened for this block).
                                        if let Some(funding_locked) = self.check_get_funding_locked(height) {
                                                return Ok(Some(funding_locked));
@@ -3630,7 +3630,7 @@ impl<Signer: Sign> Channel<Signer> {
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn update_best_block(&mut self, height: u32, highest_header_time: u32) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
+       pub fn best_block_updated(&mut self, height: u32, highest_header_time: u32) -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> {
                let mut timed_out_htlcs = Vec::new();
                let unforwarded_htlc_cltv_limit = height + HTLC_FAIL_BACK_BUFFER;
                self.holding_cell_htlc_updates.retain(|htlc_update| {
@@ -3682,14 +3682,14 @@ impl<Signer: Sign> Channel<Signer> {
        /// before the channel has reached funding_locked and we can just wait for more blocks.
        pub fn funding_transaction_unconfirmed(&mut self) -> Result<(), msgs::ErrorMessage> {
                if self.funding_tx_confirmation_height != 0 {
-                       // We handle the funding disconnection by calling update_best_block with a height one
+                       // We handle the funding disconnection by calling best_block_updated with a height one
                        // below where our funding was connected, implying a reorg back to conf_height - 1.
                        let reorg_height = self.funding_tx_confirmation_height - 1;
                        // We use the time field to bump the current time we set on channel updates if its
                        // larger. If we don't know that time has moved forward, we can just set it to the last
                        // time we saw and it will be ignored.
                        let best_time = self.update_time_counter;
-                       match self.update_best_block(reorg_height, best_time) {
+                       match self.best_block_updated(reorg_height, best_time) {
                                Ok((funding_locked, timed_out_htlcs)) => {
                                        assert!(funding_locked.is_none(), "We can't generate a funding with 0 confirmations?");
                                        assert!(timed_out_htlcs.is_empty(), "We can't have accepted HTLCs with a timeout before our funding confirmation?");
index 0360f8a69a8649dcb72e03fcf295ed2f2e9daa59..80edbacf573a7a545107a7908698fb0bf371cb13 100644 (file)
@@ -36,6 +36,7 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1;
 
 use chain;
+use chain::Confirm;
 use chain::Watch;
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, ChannelMonitorUpdateErr, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
@@ -3378,8 +3379,8 @@ where
                }
 
                let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-               self.transactions_confirmed(&block.header, height, &txdata);
-               self.update_best_block(&block.header, height);
+               self.transactions_confirmed(&block.header, &txdata, height);
+               self.best_block_updated(&block.header, height);
        }
 
        fn block_disconnected(&self, header: &BlockHeader, height: u32) {
@@ -3394,16 +3395,88 @@ where
                        *best_block = BestBlock::new(header.prev_blockhash, new_height)
                }
 
-               self.do_chain_event(Some(new_height), |channel| channel.update_best_block(new_height, header.time));
+               self.do_chain_event(Some(new_height), |channel| channel.best_block_updated(new_height, header.time));
+       }
+}
+
+impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> chain::Confirm for ChannelManager<Signer, M, T, K, F, L>
+where
+       M::Target: chain::Watch<Signer>,
+       T::Target: BroadcasterInterface,
+       K::Target: KeysInterface<Signer = Signer>,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
+{
+       fn transactions_confirmed(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
+               // during initialization prior to the chain_monitor being fully configured in some cases.
+               // See the docs for `ChannelManagerReadArgs` for more.
+
+               let block_hash = header.block_hash();
+               log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
+
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+               self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
+       }
+
+       fn best_block_updated(&self, header: &BlockHeader, height: u32) {
+               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
+               // during initialization prior to the chain_monitor being fully configured in some cases.
+               // See the docs for `ChannelManagerReadArgs` for more.
+
+               let block_hash = header.block_hash();
+               log_trace!(self.logger, "New best block: {} at height {}", block_hash, height);
+
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+
+               *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
+
+               self.do_chain_event(Some(height), |channel| channel.best_block_updated(height, header.time));
+
+               loop {
+                       // Update last_node_announcement_serial to be the max of its current value and the
+                       // block timestamp. This should keep us close to the current time without relying on
+                       // having an explicit local time source.
+                       // Just in case we end up in a race, we loop until we either successfully update
+                       // last_node_announcement_serial or decide we don't need to.
+                       let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
+                       if old_serial >= header.time as usize { break; }
+                       if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
+                               break;
+                       }
+               }
+       }
+
+       fn get_relevant_txids(&self) -> Vec<Txid> {
+               let channel_state = self.channel_state.lock().unwrap();
+               let mut res = Vec::with_capacity(channel_state.short_to_id.len());
+               for chan in channel_state.by_id.values() {
+                       if let Some(funding_txo) = chan.get_funding_txo() {
+                               res.push(funding_txo.txid);
+                       }
+               }
+               res
+       }
+
+       fn transaction_unconfirmed(&self, txid: &Txid) {
+               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
+               self.do_chain_event(None, |channel| {
+                       if let Some(funding_txo) = channel.get_funding_txo() {
+                               if funding_txo.txid == *txid {
+                                       channel.funding_transaction_unconfirmed().map(|_| (None, Vec::new()))
+                               } else { Ok((None, Vec::new())) }
+                       } else { Ok((None, Vec::new())) }
+               });
        }
 }
 
 impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<Signer, M, T, K, F, L>
-       where M::Target: chain::Watch<Signer>,
-        T::Target: BroadcasterInterface,
-        K::Target: KeysInterface<Signer = Signer>,
-        F::Target: FeeEstimator,
-        L::Target: Logger,
+where
+       M::Target: chain::Watch<Signer>,
+       T::Target: BroadcasterInterface,
+       K::Target: KeysInterface<Signer = Signer>,
+       F::Target: FeeEstimator,
+       L::Target: Logger,
 {
        /// Calls a function which handles an on-chain event (blocks dis/connected, transactions
        /// un/confirmed, etc) on each channel, handling any resulting errors or messages generated by
@@ -3497,131 +3570,6 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                }
        }
 
-       /// Updates channel state to take note of transactions which were confirmed in the given block
-       /// at the given height.
-       ///
-       /// Note that you must still call (or have called) [`update_best_block`] with the block
-       /// information which is included here.
-       ///
-       /// This method may be called before or after [`update_best_block`] for a given block's
-       /// transaction data and may be called multiple times with additional transaction data for a
-       /// given block.
-       ///
-       /// This method may be called for a previous block after an [`update_best_block`] call has
-       /// been made for a later block, however it must *not* be called with transaction data from a
-       /// block which is no longer in the best chain (ie where [`update_best_block`] has already
-       /// been informed about a blockchain reorganization which no longer includes the block which
-       /// corresponds to `header`).
-       ///
-       /// [`update_best_block`]: `Self::update_best_block`
-       pub fn transactions_confirmed(&self, header: &BlockHeader, height: u32, txdata: &TransactionData) {
-               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
-               // during initialization prior to the chain_monitor being fully configured in some cases.
-               // See the docs for `ChannelManagerReadArgs` for more.
-
-               let block_hash = header.block_hash();
-               log_trace!(self.logger, "{} transactions included in block {} at height {} provided", txdata.len(), block_hash, height);
-
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-               self.do_chain_event(Some(height), |channel| channel.transactions_confirmed(&block_hash, height, txdata, &self.logger).map(|a| (a, Vec::new())));
-       }
-
-       /// Updates channel state with the current best blockchain tip. You should attempt to call this
-       /// quickly after a new block becomes available, however if multiple new blocks become
-       /// available at the same time, only a single `update_best_block()` call needs to be made.
-       ///
-       /// This method should also be called immediately after any block disconnections, once at the
-       /// reorganization fork point, and once with the new chain tip. Calling this method at the
-       /// blockchain reorganization fork point ensures we learn when a funding transaction which was
-       /// previously confirmed is reorganized out of the blockchain, ensuring we do not continue to
-       /// accept payments which cannot be enforced on-chain.
-       ///
-       /// In both the block-connection and block-disconnection case, this method may be called either
-       /// once per block connected or disconnected, or simply at the fork point and new tip(s),
-       /// skipping any intermediary blocks.
-       pub fn update_best_block(&self, header: &BlockHeader, height: u32) {
-               // Note that we MUST NOT end up calling methods on self.chain_monitor here - we're called
-               // during initialization prior to the chain_monitor being fully configured in some cases.
-               // See the docs for `ChannelManagerReadArgs` for more.
-
-               let block_hash = header.block_hash();
-               log_trace!(self.logger, "New best block: {} at height {}", block_hash, height);
-
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-
-               *self.best_block.write().unwrap() = BestBlock::new(block_hash, height);
-
-               self.do_chain_event(Some(height), |channel| channel.update_best_block(height, header.time));
-
-               loop {
-                       // Update last_node_announcement_serial to be the max of its current value and the
-                       // block timestamp. This should keep us close to the current time without relying on
-                       // having an explicit local time source.
-                       // Just in case we end up in a race, we loop until we either successfully update
-                       // last_node_announcement_serial or decide we don't need to.
-                       let old_serial = self.last_node_announcement_serial.load(Ordering::Acquire);
-                       if old_serial >= header.time as usize { break; }
-                       if self.last_node_announcement_serial.compare_exchange(old_serial, header.time as usize, Ordering::AcqRel, Ordering::Relaxed).is_ok() {
-                               break;
-                       }
-               }
-       }
-
-       /// Gets the set of txids which should be monitored for their confirmation state.
-       ///
-       /// If you're providing information about reorganizations via [`transaction_unconfirmed`], this
-       /// is the set of transactions which you may need to call [`transaction_unconfirmed`] for.
-       ///
-       /// This may be useful to poll to determine the set of transactions which must be registered
-       /// with an Electrum server or for which an Electrum server needs to be polled to determine
-       /// transaction confirmation state.
-       ///
-       /// This may update after any [`transactions_confirmed`] or [`block_connected`] call.
-       ///
-       /// Note that this is NOT the set of transactions which must be included in calls to
-       /// [`transactions_confirmed`] if they are confirmed, but a small subset of it.
-       ///
-       /// [`transactions_confirmed`]: Self::transactions_confirmed
-       /// [`transaction_unconfirmed`]: Self::transaction_unconfirmed
-       /// [`block_connected`]: chain::Listen::block_connected
-       pub fn get_relevant_txids(&self) -> Vec<Txid> {
-               let channel_state = self.channel_state.lock().unwrap();
-               let mut res = Vec::with_capacity(channel_state.short_to_id.len());
-               for chan in channel_state.by_id.values() {
-                       if let Some(funding_txo) = chan.get_funding_txo() {
-                               res.push(funding_txo.txid);
-                       }
-               }
-               res
-       }
-
-       /// Marks a transaction as having been reorganized out of the blockchain.
-       ///
-       /// If a transaction is included in [`get_relevant_txids`], and is no longer in the main branch
-       /// of the blockchain, this function should be called to indicate that the transaction should
-       /// be considered reorganized out.
-       ///
-       /// Once this is called, the given transaction will no longer appear on [`get_relevant_txids`],
-       /// though this may be called repeatedly for a given transaction without issue.
-       ///
-       /// Note that if the transaction is confirmed on the main chain in a different block (indicated
-       /// via a call to [`transactions_confirmed`]), it may re-appear in [`get_relevant_txids`], thus
-       /// be very wary of race-conditions wherein the final state of a transaction indicated via
-       /// these APIs is not the same as its state on the blockchain.
-       ///
-       /// [`transactions_confirmed`]: Self::transactions_confirmed
-       /// [`get_relevant_txids`]: Self::get_relevant_txids
-       pub fn transaction_unconfirmed(&self, txid: &Txid) {
-               let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
-               self.do_chain_event(None, |channel| {
-                       if let Some(funding_txo) = channel.get_funding_txo() {
-                               if funding_txo.txid == *txid {
-                                       channel.funding_transaction_unconfirmed().map(|_| (None, Vec::new()))
-                               } else { Ok((None, Vec::new())) }
-                       } else { Ok((None, Vec::new())) }
-               });
-       }
-
        /// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool
        /// indicating whether persistence is necessary. Only one listener on
        /// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken
index dee3c6ea3957b4ccf5b84c907df6bb2888bee40e..dce4e6289b9536352270bcbbe9eb33268192571a 100644 (file)
@@ -10,7 +10,7 @@
 //! A bunch of useful utilities for building networks of nodes and exchanging messages between
 //! nodes for functional tests.
 
-use chain::{Listen, Watch};
+use chain::{Confirm, Listen, Watch};
 use chain::channelmonitor::ChannelMonitor;
 use chain::transaction::OutPoint;
 use ln::channelmanager::{BestBlock, ChainParameters, ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentPreimage, PaymentHash, PaymentSecret, PaymentSendFailure};
@@ -79,17 +79,17 @@ pub fn confirm_transaction_at<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, tx: &T
 /// The possible ways we may notify a ChannelManager of a new block
 #[derive(Clone, Copy, PartialEq)]
 pub enum ConnectStyle {
-       /// Calls update_best_block first, detecting transactions in the block only after receiving the
+       /// Calls best_block_updated first, detecting transactions in the block only after receiving the
        /// header and height information.
        BestBlockFirst,
        /// The same as BestBlockFirst, however when we have multiple blocks to connect, we only
-       /// make a single update_best_block call.
+       /// make a single best_block_updated call.
        BestBlockFirstSkippingBlocks,
        /// Calls transactions_confirmed first, detecting transactions in the block before updating the
        /// header and height information.
        TransactionsFirst,
        /// The same as TransactionsFirst, however when we have multiple blocks to connect, we only
-       /// make a single update_best_block call.
+       /// make a single best_block_updated call.
        TransactionsFirstSkippingBlocks,
        /// Provides the full block via the chain::Listen interface. In the current code this is
        /// equivalent to TransactionsFirst with some additional assertions.
@@ -128,20 +128,20 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: &Block, s
                let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
                match *node.connect_style.borrow() {
                        ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstSkippingBlocks => {
-                               node.chain_monitor.chain_monitor.update_best_block(&block.header, height);
+                               node.chain_monitor.chain_monitor.best_block_updated(&block.header, height);
                                node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height);
-                               node.node.update_best_block(&block.header, height);
-                               node.node.transactions_confirmed(&block.header, height, &txdata);
+                               node.node.best_block_updated(&block.header, height);
+                               node.node.transactions_confirmed(&block.header, &txdata, height);
                        },
                        ConnectStyle::TransactionsFirst|ConnectStyle::TransactionsFirstSkippingBlocks => {
                                node.chain_monitor.chain_monitor.transactions_confirmed(&block.header, &txdata, height);
-                               node.chain_monitor.chain_monitor.update_best_block(&block.header, height);
-                               node.node.transactions_confirmed(&block.header, height, &txdata);
-                               node.node.update_best_block(&block.header, height);
+                               node.chain_monitor.chain_monitor.best_block_updated(&block.header, height);
+                               node.node.transactions_confirmed(&block.header, &txdata, height);
+                               node.node.best_block_updated(&block.header, height);
                        },
                        ConnectStyle::FullBlockViaListen => {
-                               node.chain_monitor.chain_monitor.block_connected(&block.header, &txdata, height);
-                               Listen::block_connected(node.node, &block, height);
+                               node.chain_monitor.chain_monitor.block_connected(&block, height);
+                               node.node.block_connected(&block, height);
                        }
                }
        }
@@ -162,13 +162,13 @@ pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32)
                        },
                        ConnectStyle::BestBlockFirstSkippingBlocks|ConnectStyle::TransactionsFirstSkippingBlocks => {
                                if i == count - 1 {
-                                       node.chain_monitor.chain_monitor.update_best_block(&prev_header.0, prev_header.1);
-                                       node.node.update_best_block(&prev_header.0, prev_header.1);
+                                       node.chain_monitor.chain_monitor.best_block_updated(&prev_header.0, prev_header.1);
+                                       node.node.best_block_updated(&prev_header.0, prev_header.1);
                                }
                        },
                        _ => {
-                               node.chain_monitor.chain_monitor.update_best_block(&prev_header.0, prev_header.1);
-                               node.node.update_best_block(&prev_header.0, prev_header.1);
+                               node.chain_monitor.chain_monitor.best_block_updated(&prev_header.0, prev_header.1);
+                               node.node.best_block_updated(&prev_header.0, prev_header.1);
                        },
                }
        }
index f3b8ca15594dcccfa6ec0965a59e94e8c1957f14..7a39db2f312b1bf97b7a340906d55018b6d13e53 100644 (file)
@@ -12,6 +12,7 @@
 //! claim outputs on-chain.
 
 use chain;
+use chain::Listen;
 use chain::Watch;
 use chain::channelmonitor;
 use chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY};
@@ -8225,7 +8226,7 @@ fn test_update_err_monitor_lockdown() {
                watchtower
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       watchtower.chain_monitor.block_connected(&header, &[], 200);
+       watchtower.chain_monitor.block_connected(&Block { header, txdata: vec![] }, 200);
 
        // Try to update ChannelMonitor
        assert!(nodes[1].node.claim_funds(preimage, &None, 9_000_000));
@@ -8284,7 +8285,7 @@ fn test_concurrent_monitor_claim() {
                watchtower
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       watchtower_alice.chain_monitor.block_connected(&header, &vec![], CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
+       watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![] }, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
        // Watchtower Alice should have broadcast a commitment/HTLC-timeout
        {
@@ -8310,7 +8311,7 @@ fn test_concurrent_monitor_claim() {
                watchtower
        };
        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-       watchtower_bob.chain_monitor.block_connected(&header, &vec![], CHAN_CONFIRM_DEPTH + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
+       watchtower_bob.chain_monitor.block_connected(&Block { header, txdata: vec![] }, CHAN_CONFIRM_DEPTH + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
        // Route another payment to generate another update with still previous HTLC pending
        let (_, payment_hash) = get_payment_preimage_hash!(nodes[0]);
@@ -8336,7 +8337,8 @@ fn test_concurrent_monitor_claim() {
        check_added_monitors!(nodes[0], 1);
 
        //// Provide one more block to watchtower Bob, expect broadcast of commitment and HTLC-Timeout
-       watchtower_bob.chain_monitor.block_connected(&header, &vec![], CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
+       let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       watchtower_bob.chain_monitor.block_connected(&Block { header, txdata: vec![] }, CHAN_CONFIRM_DEPTH + 1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
 
        // Watchtower Bob should have broadcast a commitment/HTLC-timeout
        let bob_state_y;
@@ -8348,7 +8350,8 @@ fn test_concurrent_monitor_claim() {
        };
 
        // We confirm Bob's state Y on Alice, she should broadcast a HTLC-timeout
-       watchtower_alice.chain_monitor.block_connected(&header, &vec![(0, &bob_state_y)], CHAN_CONFIRM_DEPTH + 2 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
+       let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+       watchtower_alice.chain_monitor.block_connected(&Block { header, txdata: vec![bob_state_y.clone()] }, CHAN_CONFIRM_DEPTH + 2 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS);
        {
                let htlc_txn = chanmon_cfgs[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                // We broadcast twice the transaction, once due to the HTLC-timeout, once due
index 9488a34db0d7e04d322b787bc47501be5cba0c80..d4eb9eae5e7910f7a0b4de52ab6591794210f8ea 100644 (file)
@@ -1020,9 +1020,9 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, L: Deref> PeerManager<D
                        // buffer by doing things like announcing channels on another node. We should be willing to
                        // drop optional-ish messages when send buffers get full!
 
+                       let mut peers_lock = self.peers.lock().unwrap();
                        let mut events_generated = self.message_handler.chan_handler.get_and_clear_pending_msg_events();
                        events_generated.append(&mut self.message_handler.route_handler.get_and_clear_pending_msg_events());
-                       let mut peers_lock = self.peers.lock().unwrap();
                        let peers = &mut *peers_lock;
                        for event in events_generated.drain(..) {
                                macro_rules! get_peer_for_forwarding {
index d5a3d042f297846d3d4af516a878b8985f48427d..05001d69e0c72a5c1765b5fec23bffe6344cf132 100644 (file)
@@ -10,7 +10,7 @@
 //! Further functional tests which test blockchain reorganizations.
 
 use chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor};
-use chain::Watch;
+use chain::{Confirm, Watch};
 use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs};
 use ln::features::InitFeatures;
 use ln::msgs::{ChannelMessageHandler, ErrorAction, HTLCFailChannelUpdate};
index 0ad307cdf2f17bf1a347b535ebf5009e9a000c14..08fe95d2305293e7440a04bc407a4110b87c3901 100644 (file)
@@ -117,8 +117,8 @@ impl Readable for Route {
 }
 
 /// A channel descriptor which provides a last-hop route to get_route
-#[derive(Clone)]
-pub struct RouteHint {
+#[derive(Eq, PartialEq, Debug, Clone)]
+pub struct RouteHintHop {
        /// The node_id of the non-target end of the route
        pub src_node_id: PublicKey,
        /// The short_channel_id of this channel
@@ -176,7 +176,7 @@ struct DummyDirectionalChannelInfo {
 /// These fee values are useful to choose hops as we traverse the graph "payee-to-payer".
 #[derive(Clone)]
 struct PathBuildingHop<'a> {
-       // The RouteHint fields which will eventually be used if this hop is used in a final Route.
+       // The RouteHintHop fields which will eventually be used if this hop is used in a final Route.
        // Note that node_features is calculated separately after our initial graph walk.
        pubkey: PublicKey,
        short_channel_id: u64,
@@ -353,7 +353,7 @@ fn compute_fees(amount_msat: u64, channel_fees: RoutingFees) -> Option<u64> {
 /// equal), however the enabled/disabled bit on such channels as well as the
 /// htlc_minimum_msat/htlc_maximum_msat *are* checked as they may change based on the receiving node.
 pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, payee: &PublicKey, payee_features: Option<InvoiceFeatures>, first_hops: Option<&[&ChannelDetails]>,
-       last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
+       last_hops: &[&RouteHintHop], final_value_msat: u64, final_cltv: u32, logger: L) -> Result<Route, LightningError> where L::Target: Logger {
        // TODO: Obviously *only* using total fee cost sucks. We should consider weighting by
        // uptime/success in using a node in the past.
        if *payee == *our_node_id {
@@ -1163,7 +1163,7 @@ pub fn get_route<L: Deref>(our_node_id: &PublicKey, network: &NetworkGraph, paye
 
 #[cfg(test)]
 mod tests {
-       use routing::router::{get_route, RouteHint, RoutingFees};
+       use routing::router::{get_route, RouteHintHop, RoutingFees};
        use routing::network_graph::{NetworkGraph, NetGraphMsgHandler};
        use ln::features::{ChannelFeatures, InitFeatures, InvoiceFeatures, NodeFeatures};
        use ln::msgs::{ErrorAction, LightningError, OptionalField, UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
@@ -2084,19 +2084,19 @@ mod tests {
                assert_eq!(route.paths[0][1].channel_features.le_flags(), &id_to_feature_flags(13));
        }
 
-       fn last_hops(nodes: &Vec<PublicKey>) -> Vec<RouteHint> {
+       fn last_hops(nodes: &Vec<PublicKey>) -> Vec<RouteHintHop> {
                let zero_fees = RoutingFees {
                        base_msat: 0,
                        proportional_millionths: 0,
                };
-               vec!(RouteHint {
+               vec!(RouteHintHop {
                        src_node_id: nodes[3].clone(),
                        short_channel_id: 8,
                        fees: zero_fees,
                        cltv_expiry_delta: (8 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               }, RouteHint {
+               }, RouteHintHop {
                        src_node_id: nodes[4].clone(),
                        short_channel_id: 9,
                        fees: RoutingFees {
@@ -2106,7 +2106,7 @@ mod tests {
                        cltv_expiry_delta: (9 << 8) | 1,
                        htlc_minimum_msat: None,
                        htlc_maximum_msat: None,
-               }, RouteHint {
+               }, RouteHintHop {
                        src_node_id: nodes[5].clone(),
                        short_channel_id: 10,
                        fees: zero_fees,
@@ -2124,7 +2124,7 @@ mod tests {
                // Simple test across 2, 3, 5, and 4 via a last_hop channel
 
                // First check that lst hop can't have its source as the payee.
-               let invalid_last_hop = RouteHint {
+               let invalid_last_hop = RouteHintHop {
                        src_node_id: nodes[6],
                        short_channel_id: 8,
                        fees: RoutingFees {
@@ -2309,7 +2309,7 @@ mod tests {
                let target_node_id = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&hex::decode(format!("{:02}", 43).repeat(32)).unwrap()[..]).unwrap());
 
                // If we specify a channel to a middle hop, that overrides our local channel view and that gets used
-               let last_hops = vec![RouteHint {
+               let last_hops = vec![RouteHintHop {
                        src_node_id: middle_node_id,
                        short_channel_id: 8,
                        fees: RoutingFees {