From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Sat, 24 Apr 2021 00:03:42 +0000 (+0000) Subject: Merge pull request #890 from TheBlueMatt/2021-04-fix-chan-shutdown-crash X-Git-Tag: v0.0.14~17 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=36570f4593bbbb8d5f894e5ed9d5a7f9a4720004;hp=eb42caf8a0608ab004b8d3ae179015702ea8a64d;p=rust-lightning Merge pull request #890 from TheBlueMatt/2021-04-fix-chan-shutdown-crash Fix (and test) panic when our counterparty uses a bogus funding tx --- diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a379d1cd..d71a47da 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -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: | diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 9d0a90ee..96da87b6 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -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(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); } } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 91ebb990..73a9a67a 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -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 { diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index fb720c99..e80e080f 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -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(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 { diff --git a/lightning-invoice/Cargo.toml b/lightning-invoice/Cargo.toml index 47bf9d74..758578ca 100644 --- a/lightning-invoice/Cargo.toml +++ b/lightning-invoice/Cargo.toml @@ -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" diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs index d6cb9207..fe77a93a 100644 --- a/lightning-invoice/src/de.rs +++ b/lightning-invoice/src/de.rs @@ -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(digits: &[U], base: T) -> Option +pub(crate) fn parse_int_be(digits: &[U], base: T) -> Option where T: CheckedAdd + CheckedMul + From + Default, U: Into + 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 { + fn from_base32(field_data: &[u5]) -> Result { let bytes = Vec::::from_base32(field_data)?; if bytes.len() % 51 != 0 { return Err(ParseError::UnexpectedEndOfTaggedFields); } - let mut route_hops = Vec::::new(); + let mut route_hops = Vec::::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::::new(); - expected.push(RouteHop { - pubkey: PublicKey::from_slice( + let mut expected = Vec::::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) ); } diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs index e9ca442f..2ce02224 100644 --- a/lightning-invoice/src/lib.rs +++ b/lightning-invoice/src/lib.rs @@ -17,12 +17,16 @@ 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); - -/// 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); /// Tag constants as specified in BOLT11 #[allow(missing_docs)] @@ -499,8 +484,8 @@ impl InvoiceBuilder { } /// Adds a private route. - pub fn route(mut self, route: Vec) -> Self { - match Route::new(route) { + pub fn route(mut self, route: Vec) -> 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::>() } - 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::>() + }).collect::>() } pub fn amount_pico_btc(&self) -> Option { @@ -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) -> Result { + pub fn new(hops: Vec) -> Result { 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 { + pub fn into_inner(self) -> Vec { self.0 } } -impl Into> for Route { - fn into(self) -> Vec { +impl Into> for RouteHint { + fn into(self) -> Vec { self.into_inner() } } -impl Deref for Route { - type Target = Vec; +impl Deref for RouteHint { + type Target = Vec; - fn deref(&self) -> &Vec { + fn deref(&self) -> &Vec { &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())) diff --git a/lightning-invoice/src/ser.rs b/lightning-invoice/src/ser.rs index 2b4332f8..83888e82 100644 --- a/lightning-invoice/src/ser.rs +++ b/lightning-invoice/src/ser.rs @@ -365,22 +365,26 @@ impl Base32Len for Fallback { } } -impl ToBase32 for Route { +impl ToBase32 for RouteHint { fn write_base32(&self, writer: &mut W) -> Result<(), ::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) } diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index ee6df63a..95a786f6 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -74,7 +74,7 @@ where C::Target: chain::Filter, P::Target: channelmonitor::Persist, { /// 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(&self, header: &BlockHeader, txdata: &TransactionData, process: FN) where FN: Fn(&ChannelMonitor, &TransactionData) -> Vec @@ -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 { - 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 +impl chain::Listen for ChainMonitor where ChannelSigner: Sign, @@ -242,12 +152,67 @@ where P::Target: channelmonitor::Persist, { 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 +chain::Confirm for ChainMonitor +where + ChannelSigner: Sign, + C::Target: chain::Filter, + T::Target: BroadcasterInterface, + F::Target: FeeEstimator, + L::Target: Logger, + P::Target: channelmonitor::Persist, +{ + 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 { + 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 } } diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 573d19df..f0b23c08 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -1311,11 +1311,9 @@ impl ChannelMonitor { /// 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( &self, header: &BlockHeader, @@ -1337,11 +1335,9 @@ impl ChannelMonitor { /// 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( &self, txid: &Txid, @@ -1361,18 +1357,10 @@ impl ChannelMonitor { /// [`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( + pub fn best_block_updated( &self, header: &BlockHeader, height: u32, @@ -1385,7 +1373,7 @@ impl ChannelMonitor { 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 ChannelMonitorImpl { self.transactions_confirmed(header, txdata, height, broadcaster, fee_estimator, logger) } - fn update_best_block( + fn best_block_updated( &mut self, header: &BlockHeader, height: u32, @@ -2727,6 +2715,29 @@ where } } +impl chain::Confirm for (ChannelMonitor, 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 { + self.0.get_relevant_txids() + } +} + const MAX_ALLOC_SIZE: usize = 64*1024; impl<'a, Signer: Sign, K: KeysInterface> ReadableArgs<&'a K> diff --git a/lightning/src/chain/mod.rs b/lightning/src/chain/mod.rs index 18c7fd55..7d25a066 100644 --- a/lightning/src/chain/mod.rs +++ b/lightning/src/chain/mod.rs @@ -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; } -/// 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; +} + /// The `Watch` trait defines behavior for watching on-chain activity pertaining to channels as /// blocks are connected and disconnected. /// diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index a7cc5377..ee9936b3 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -12,11 +12,12 @@ //! 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)); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1c34e339..8a1489d4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3599,7 +3599,7 @@ impl Channel { } } // 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 Channel { /// /// 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, Vec<(HTLCSource, PaymentHash)>), msgs::ErrorMessage> { + pub fn best_block_updated(&mut self, height: u32, highest_header_time: u32) -> Result<(Option, 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 Channel { /// 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?"); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0360f8a6..80edbacf 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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 chain::Confirm for ChannelManager +where + M::Target: chain::Watch, + T::Target: BroadcasterInterface, + K::Target: KeysInterface, + 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 { + 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 ChannelManager - where M::Target: chain::Watch, - T::Target: BroadcasterInterface, - K::Target: KeysInterface, - F::Target: FeeEstimator, - L::Target: Logger, +where + M::Target: chain::Watch, + T::Target: BroadcasterInterface, + K::Target: KeysInterface, + 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 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 { - 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 diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index dee3c6ea..dce4e628 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -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); }, } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index f3b8ca15..7a39db2f 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -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 diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 9488a34d..d4eb9eae 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -1020,9 +1020,9 @@ impl PeerManager { - // 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 { /// 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(our_node_id: &PublicKey, network: &NetworkGraph, payee: &PublicKey, payee_features: Option, first_hops: Option<&[&ChannelDetails]>, - last_hops: &[&RouteHint], final_value_msat: u64, final_cltv: u32, logger: L) -> Result where L::Target: Logger { + last_hops: &[&RouteHintHop], final_value_msat: u64, final_cltv: u32, logger: L) -> Result 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(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) -> Vec { + fn last_hops(nodes: &Vec) -> Vec { 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 {