From 7bc6d0e606e1b0ac4738ee72be798d93cf93f01f Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Wed, 29 Jun 2022 15:13:40 +0200 Subject: [PATCH] Make all internal signatures accept LowerBoundedFeeEstimator --- lightning/src/chain/chaininterface.rs | 11 ++-- lightning/src/chain/channelmonitor.rs | 41 ++++++++------ lightning/src/chain/onchaintx.rs | 12 ++--- lightning/src/chain/package.rs | 8 +-- lightning/src/ln/channel.rs | 78 ++++++++++++++------------- lightning/src/ln/channelmanager.rs | 12 +++-- lightning/src/ln/functional_tests.rs | 9 ++-- 7 files changed, 97 insertions(+), 74 deletions(-) diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index 02961f2dc..2ec7f318f 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -46,10 +46,9 @@ pub trait FeeEstimator { /// LDK will wrap this method and ensure that the value returned is no smaller than 253 /// (ie 1 satoshi-per-byte rounded up to ensure later round-downs don't put us below 1 satoshi-per-byte). /// - /// The following unit conversions can be used to convert to sats/KW. Note that it is not - /// necessary to use max() as the minimum of 253 will be enforced by LDK: - /// * max(satoshis-per-byte * 250, 253) - /// * max(satoshis-per-kbyte / 4, 253) + /// The following unit conversions can be used to convert to sats/KW: + /// * satoshis-per-byte * 250 + /// * satoshis-per-kbyte / 4 fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32; } @@ -77,8 +76,10 @@ impl LowerBoundedFeeEstimator where F::Target: FeeEstimator { pub fn new(fee_estimator: F) -> Self { LowerBoundedFeeEstimator(fee_estimator) } +} - pub fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32 { +impl FeeEstimator for LowerBoundedFeeEstimator where F::Target: FeeEstimator { + fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32 { cmp::max( self.0.get_est_sat_per_1000_weight(confirmation_target), FEERATE_FLOOR_SATS_PER_KW, diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 6492ba176..80cd9cb9d 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -40,7 +40,7 @@ use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLC use ln::channelmanager::HTLCSource; use chain; use chain::{BestBlock, WatchedOutput}; -use chain::chaininterface::{BroadcasterInterface, FeeEstimator}; +use chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator}; use chain::transaction::{OutPoint, TransactionData}; use chain::keysinterface::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, Sign, KeysInterface}; use chain::onchaintx::OnchainTxHandler; @@ -1104,7 +1104,7 @@ impl ChannelMonitor { payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, - fee_estimator: &F, + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) where B::Target: BroadcasterInterface, @@ -1300,8 +1300,9 @@ impl ChannelMonitor { F::Target: FeeEstimator, L::Target: Logger, { + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); self.inner.lock().unwrap().transactions_confirmed( - header, txdata, height, broadcaster, fee_estimator, logger) + header, txdata, height, broadcaster, &bounded_fee_estimator, logger) } /// Processes a transaction that was reorganized out of the chain. @@ -1321,8 +1322,9 @@ impl ChannelMonitor { F::Target: FeeEstimator, L::Target: Logger, { + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); self.inner.lock().unwrap().transaction_unconfirmed( - txid, broadcaster, fee_estimator, logger); + txid, broadcaster, &bounded_fee_estimator, logger); } /// Updates the monitor with the current best chain tip, returning new outputs to watch. See @@ -1345,8 +1347,9 @@ impl ChannelMonitor { F::Target: FeeEstimator, L::Target: Logger, { + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); self.inner.lock().unwrap().best_block_updated( - header, height, broadcaster, fee_estimator, logger) + header, height, broadcaster, &bounded_fee_estimator, logger) } /// Returns the set of txids that should be monitored for re-organization out of the chain. @@ -1882,7 +1885,9 @@ impl ChannelMonitorImpl { /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all /// commitment_tx_infos which contain the payment hash have been revoked. - fn provide_payment_preimage(&mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, fee_estimator: &F, logger: &L) + fn provide_payment_preimage( + &mut self, payment_hash: &PaymentHash, payment_preimage: &PaymentPreimage, broadcaster: &B, + fee_estimator: &LowerBoundedFeeEstimator, logger: &L) where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -1980,7 +1985,8 @@ impl ChannelMonitorImpl { }, ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => { log_trace!(logger, "Updating ChannelMonitor with payment preimage"); - self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, fee_estimator, logger) + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); + self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).into_inner()), &payment_preimage, broadcaster, &bounded_fee_estimator, logger) }, ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => { log_trace!(logger, "Updating ChannelMonitor with commitment secret"); @@ -2406,7 +2412,8 @@ impl ChannelMonitorImpl { let block_hash = header.block_hash(); self.best_block = BestBlock::new(block_hash, height); - self.transactions_confirmed(header, txdata, height, broadcaster, fee_estimator, logger) + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); + self.transactions_confirmed(header, txdata, height, broadcaster, &bounded_fee_estimator, logger) } fn best_block_updated( @@ -2414,7 +2421,7 @@ impl ChannelMonitorImpl { header: &BlockHeader, height: u32, broadcaster: B, - fee_estimator: F, + fee_estimator: &LowerBoundedFeeEstimator, logger: L, ) -> Vec where @@ -2441,7 +2448,7 @@ impl ChannelMonitorImpl { txdata: &TransactionData, height: u32, broadcaster: B, - fee_estimator: F, + fee_estimator: &LowerBoundedFeeEstimator, logger: L, ) -> Vec where @@ -2538,7 +2545,7 @@ impl ChannelMonitorImpl { mut watch_outputs: Vec, mut claimable_outpoints: Vec, broadcaster: &B, - fee_estimator: &F, + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Vec where @@ -2676,7 +2683,8 @@ impl ChannelMonitorImpl { //- maturing spendable output has transaction paying us has been disconnected self.onchain_events_awaiting_threshold_conf.retain(|ref entry| entry.height < height); - self.onchain_tx_handler.block_disconnected(height, broadcaster, fee_estimator, logger); + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator); + self.onchain_tx_handler.block_disconnected(height, broadcaster, &bounded_fee_estimator, logger); self.best_block = BestBlock::new(header.prev_blockhash, height - 1); } @@ -2685,7 +2693,7 @@ impl ChannelMonitorImpl { &mut self, txid: &Txid, broadcaster: B, - fee_estimator: F, + fee_estimator: &LowerBoundedFeeEstimator, logger: L, ) where B::Target: BroadcasterInterface, @@ -3428,6 +3436,8 @@ mod tests { use hex; + use crate::chain::chaininterface::LowerBoundedFeeEstimator; + use super::ChannelMonitorUpdateStep; use ::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err}; use chain::{BestBlock, Confirm}; @@ -3549,7 +3559,7 @@ mod tests { let secp_ctx = Secp256k1::new(); let logger = Arc::new(TestLogger::new()); let broadcaster = Arc::new(TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))}); - let fee_estimator = Arc::new(TestFeeEstimator { sat_per_kw: Mutex::new(253) }); + let fee_estimator = TestFeeEstimator { sat_per_kw: Mutex::new(253) }; let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let dummy_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; @@ -3648,7 +3658,8 @@ mod tests { monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger); monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger); for &(ref preimage, ref hash) in preimages.iter() { - monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger); + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); + monitor.provide_payment_preimage(hash, preimage, &broadcaster, &bounded_fee_estimator, &logger); } // Now provide a secret, pruning preimages 10-15 diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 1ca3effab..ac01cacaa 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -24,7 +24,7 @@ use bitcoin::secp256k1; use ln::msgs::DecodeError; use ln::PaymentPreimage; use ln::chan_utils::{ChannelTransactionParameters, HolderCommitmentTransaction}; -use chain::chaininterface::{FeeEstimator, BroadcasterInterface}; +use chain::chaininterface::{FeeEstimator, BroadcasterInterface, LowerBoundedFeeEstimator}; use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER}; use chain::keysinterface::{Sign, KeysInterface}; use chain::package::PackageTemplate; @@ -377,7 +377,7 @@ impl OnchainTxHandler { /// (CSV or CLTV following cases). In case of high-fee spikes, claim tx may stuck in the mempool, so you need to bump its feerate quickly using Replace-By-Fee or Child-Pay-For-Parent. /// Panics if there are signing errors, because signing operations in reaction to on-chain events /// are not expected to fail, and if they do, we may lose funds. - fn generate_claim_tx(&mut self, cur_height: u32, cached_request: &PackageTemplate, fee_estimator: &F, logger: &L) -> Option<(Option, u64, Transaction)> + fn generate_claim_tx(&mut self, cur_height: u32, cached_request: &PackageTemplate, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Option<(Option, u64, Transaction)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -415,7 +415,7 @@ impl OnchainTxHandler { /// `conf_height` represents the height at which the transactions in `txn_matched` were /// confirmed. This does not need to equal the current blockchain tip height, which should be /// provided via `cur_height`, however it must never be higher than `cur_height`. - pub(crate) fn update_claims_view(&mut self, txn_matched: &[&Transaction], requests: Vec, conf_height: u32, cur_height: u32, broadcaster: &B, fee_estimator: &F, logger: &L) + pub(crate) fn update_claims_view(&mut self, txn_matched: &[&Transaction], requests: Vec, conf_height: u32, cur_height: u32, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -617,7 +617,7 @@ impl OnchainTxHandler { &mut self, txid: &Txid, broadcaster: B, - fee_estimator: F, + fee_estimator: &LowerBoundedFeeEstimator, logger: L, ) where B::Target: BroadcasterInterface, @@ -637,7 +637,7 @@ impl OnchainTxHandler { } } - pub(crate) fn block_disconnected(&mut self, height: u32, broadcaster: B, fee_estimator: F, logger: L) + pub(crate) fn block_disconnected(&mut self, height: u32, broadcaster: B, fee_estimator: &LowerBoundedFeeEstimator, logger: L) where B::Target: BroadcasterInterface, F::Target: FeeEstimator, L::Target: Logger, @@ -667,7 +667,7 @@ impl OnchainTxHandler { } } for (_, request) in bump_candidates.iter_mut() { - if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &request, &&*fee_estimator, &&*logger) { + if let Some((new_timer, new_feerate, bump_tx)) = self.generate_claim_tx(height, &request, fee_estimator, &&*logger) { request.set_timer(new_timer); request.set_feerate(new_feerate); log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx)); diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index b0961293c..b0cfa9bf0 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -38,6 +38,8 @@ use core::mem; use core::ops::Deref; use bitcoin::Witness; +use super::chaininterface::LowerBoundedFeeEstimator; + const MAX_ALLOC_SIZE: usize = 64*1024; @@ -665,7 +667,7 @@ impl PackageTemplate { /// Returns value in satoshis to be included as package outgoing output amount and feerate /// which was used to generate the value. Will not return less than `dust_limit_sats` for the /// value. - pub(crate) fn compute_package_output(&self, predicted_weight: usize, dust_limit_sats: u64, fee_estimator: &F, logger: &L) -> Option<(u64, u64)> + pub(crate) fn compute_package_output(&self, predicted_weight: usize, dust_limit_sats: u64, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Option<(u64, u64)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -772,7 +774,7 @@ impl Readable for PackageTemplate { /// If the proposed fee is less than the available spent output's values, we return the proposed /// fee and the corresponding updated feerate. If the proposed fee is equal or more than the /// available spent output's values, we return nothing -fn compute_fee_from_spent_amounts(input_amounts: u64, predicted_weight: usize, fee_estimator: &F, logger: &L) -> Option<(u64, u64)> +fn compute_fee_from_spent_amounts(input_amounts: u64, predicted_weight: usize, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Option<(u64, u64)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -808,7 +810,7 @@ fn compute_fee_from_spent_amounts(input_amounts: u64, predic /// attempt, use them. Otherwise, blindly bump the feerate by 25% of the previous feerate. We also /// verify that those bumping heuristics respect BIP125 rules 3) and 4) and if required adjust /// the new fee to meet the RBF policy requirement. -fn feerate_bump(predicted_weight: usize, input_amounts: u64, previous_feerate: u64, fee_estimator: &F, logger: &L) -> Option<(u64, u64)> +fn feerate_bump(predicted_weight: usize, input_amounts: u64, previous_feerate: u64, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Option<(u64, u64)> where F::Target: FeeEstimator, L::Target: Logger, { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index ba72fc3c9..f612cb979 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -31,7 +31,7 @@ use ln::channelmanager::{CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSour use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction}; use ln::chan_utils; use chain::BestBlock; -use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; +use chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator}; use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; use chain::transaction::{OutPoint, TransactionData}; use chain::keysinterface::{Sign, KeysInterface}; @@ -886,7 +886,7 @@ impl Channel { // Constructors: pub fn new_outbound( - fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, + fee_estimator: &LowerBoundedFeeEstimator, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32, outbound_scid_alias: u64 ) -> Result, APIError> @@ -1056,7 +1056,7 @@ impl Channel { }) } - fn check_remote_fee(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError> + fn check_remote_fee(fee_estimator: &LowerBoundedFeeEstimator, feerate_per_kw: u32) -> Result<(), ChannelError> where F::Target: FeeEstimator { // We only bound the fee updates on the upper side to prevent completely absurd feerates, @@ -1082,7 +1082,7 @@ impl Channel { /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! pub fn new_from_req( - fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, + fee_estimator: &LowerBoundedFeeEstimator, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L, outbound_scid_alias: u64 ) -> Result, ChannelError> @@ -3687,7 +3687,7 @@ impl Channel { } } - pub fn update_fee(&mut self, fee_estimator: &F, msg: &msgs::UpdateFee) -> Result<(), ChannelError> + pub fn update_fee(&mut self, fee_estimator: &LowerBoundedFeeEstimator, msg: &msgs::UpdateFee) -> Result<(), ChannelError> where F::Target: FeeEstimator { if self.is_outbound() { @@ -4013,7 +4013,8 @@ impl Channel { /// Calculates and returns our minimum and maximum closing transaction fee amounts, in whole /// satoshis. The amounts remain consistent unless a peer disconnects/reconnects or we restart, /// at which point they will be recalculated. - fn calculate_closing_fee_limits(&mut self, fee_estimator: &F) -> (u64, u64) + fn calculate_closing_fee_limits(&mut self, fee_estimator: &LowerBoundedFeeEstimator) + -> (u64, u64) where F::Target: FeeEstimator { if let Some((min, max)) = self.closing_fee_limits { return (min, max); } @@ -4087,7 +4088,8 @@ impl Channel { Ok(()) } - pub fn maybe_propose_closing_signed(&mut self, fee_estimator: &F, logger: &L) + pub fn maybe_propose_closing_signed( + &mut self, fee_estimator: &LowerBoundedFeeEstimator, logger: &L) -> Result<(Option, Option), ChannelError> where F::Target: FeeEstimator, L::Target: Logger { @@ -4241,7 +4243,9 @@ impl Channel { tx } - pub fn closing_signed(&mut self, fee_estimator: &F, msg: &msgs::ClosingSigned) -> Result<(Option, Option), ChannelError> + pub fn closing_signed( + &mut self, fee_estimator: &LowerBoundedFeeEstimator, msg: &msgs::ClosingSigned) + -> Result<(Option, Option), ChannelError> where F::Target: FeeEstimator { if self.channel_state & BOTH_SIDES_SHUTDOWN_MASK != BOTH_SIDES_SHUTDOWN_MASK { @@ -6573,7 +6577,7 @@ mod tests { use ln::chan_utils; use ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight}; use chain::BestBlock; - use chain::chaininterface::{FeeEstimator,ConfirmationTarget}; + use chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget}; use chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, KeysInterface}; use chain::transaction::OutPoint; use util::config::UserConfig; @@ -6612,7 +6616,9 @@ mod tests { fn test_no_fee_check_overflow() { // Previously, calling `check_remote_fee` with a fee of 0xffffffff would overflow in // arithmetic, causing a panic with debug assertions enabled. - assert!(Channel::::check_remote_fee(&&TestFeeEstimator { fee_est: 42 }, u32::max_value()).is_err()); + let fee_est = TestFeeEstimator { fee_est: 42 }; + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_est); + assert!(Channel::::check_remote_fee(&bounded_fee_estimator, u32::max_value()).is_err()); } struct Keys { @@ -6662,11 +6668,10 @@ mod tests { returns: non_v0_segwit_shutdown_script.clone(), }); - let fee_estimator = TestFeeEstimator { fee_est: 253 }; let secp_ctx = Secp256k1::new(); let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - match Channel::::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0, 42) { + match Channel::::new_outbound(&LowerBoundedFeeEstimator::new(&TestFeeEstimator { fee_est: 253 }), &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0, 42) { Err(APIError::IncompatibleShutdownScript { script }) => { assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner()); }, @@ -6681,6 +6686,7 @@ mod tests { fn test_open_channel_msg_fee() { let original_fee = 253; let mut fee_est = TestFeeEstimator{fee_est: original_fee }; + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_est); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; @@ -6688,7 +6694,7 @@ mod tests { let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let node_a_chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); + let node_a_chan = Channel::::new_outbound(&bounded_fee_estimator, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); // Now change the fee so we can check that the fee in the open_channel message is the // same as the old fee. @@ -6701,7 +6707,7 @@ mod tests { fn test_holder_vs_counterparty_dust_limit() { // Test that when calculating the local and remote commitment transaction fees, the correct // dust limits are used. - let feeest = TestFeeEstimator{fee_est: 15000}; + let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; @@ -6714,13 +6720,13 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); // Create Node B's channel by receiving Node A's open_channel message // Make sure A's dust limit is as we expect. let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap(); + let mut node_b_chan = Channel::::new_from_req(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0); @@ -6776,7 +6782,7 @@ mod tests { // calculate the real dust limits for HTLCs (i.e. the dust limit given by the counterparty // *plus* the fees paid for the HTLC) they don't swap `HTLC_SUCCESS_TX_WEIGHT` for // `HTLC_TIMEOUT_TX_WEIGHT`, and vice versa. - let fee_est = TestFeeEstimator{fee_est: 253 }; + let fee_est = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 253 }); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; @@ -6784,7 +6790,7 @@ mod tests { let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut chan = Channel::::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); + let mut chan = Channel::::new_outbound(&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); let commitment_tx_fee_0_htlcs = Channel::::commit_tx_fee_msat(chan.feerate_per_kw, 0, chan.opt_anchors()); let commitment_tx_fee_1_htlc = Channel::::commit_tx_fee_msat(chan.feerate_per_kw, 1, chan.opt_anchors()); @@ -6819,7 +6825,7 @@ mod tests { #[test] fn channel_reestablish_no_updates() { - let feeest = TestFeeEstimator{fee_est: 15000}; + let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let logger = test_utils::TestLogger::new(); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; @@ -6833,12 +6839,12 @@ mod tests { // Create Node A's channel pointing to Node B's pubkey let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); // Create Node B's channel by receiving Node A's open_channel message let open_channel_msg = node_a_chan.get_open_channel(chain_hash); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = Channel::::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap(); + let mut node_b_chan = Channel::::new_from_req(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap(); // Node B --> Node A: accept channel let accept_channel_msg = node_b_chan.accept_inbound_channel(0); @@ -6885,7 +6891,7 @@ mod tests { #[test] fn test_configured_holder_max_htlc_value_in_flight() { - let feeest = TestFeeEstimator{fee_est: 15000}; + let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let logger = test_utils::TestLogger::new(); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; @@ -6906,12 +6912,12 @@ mod tests { // Test that `new_outbound` creates a channel with the correct value for // `holder_max_htlc_value_in_flight_msat`, when configured with a valid percentage value, // which is set to the lower bound + 1 (2%) of the `channel_value`. - let chan_1 = Channel::::new_outbound(&&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_2_percent, 0, 42).unwrap(); + let chan_1 = Channel::::new_outbound(&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_2_percent, 0, 42).unwrap(); let chan_1_value_msat = chan_1.channel_value_satoshis * 1000; assert_eq!(chan_1.holder_max_htlc_value_in_flight_msat, (chan_1_value_msat as f64 * 0.02) as u64); // Test with the upper bound - 1 of valid values (99%). - let chan_2 = Channel::::new_outbound(&&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_99_percent, 0, 42).unwrap(); + let chan_2 = Channel::::new_outbound(&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_99_percent, 0, 42).unwrap(); let chan_2_value_msat = chan_2.channel_value_satoshis * 1000; assert_eq!(chan_2.holder_max_htlc_value_in_flight_msat, (chan_2_value_msat as f64 * 0.99) as u64); @@ -6920,45 +6926,45 @@ mod tests { // Test that `new_from_req` creates a channel with the correct value for // `holder_max_htlc_value_in_flight_msat`, when configured with a valid percentage value, // which is set to the lower bound - 1 (2%) of the `channel_value`. - let chan_3 = Channel::::new_from_req(&&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42).unwrap(); + let chan_3 = Channel::::new_from_req(&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42).unwrap(); let chan_3_value_msat = chan_3.channel_value_satoshis * 1000; assert_eq!(chan_3.holder_max_htlc_value_in_flight_msat, (chan_3_value_msat as f64 * 0.02) as u64); // Test with the upper bound - 1 of valid values (99%). - let chan_4 = Channel::::new_from_req(&&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42).unwrap(); + let chan_4 = Channel::::new_from_req(&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42).unwrap(); let chan_4_value_msat = chan_4.channel_value_satoshis * 1000; assert_eq!(chan_4.holder_max_htlc_value_in_flight_msat, (chan_4_value_msat as f64 * 0.99) as u64); // Test that `new_outbound` uses the lower bound of the configurable percentage values (1%) // if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a value less than 1. - let chan_5 = Channel::::new_outbound(&&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_0_percent, 0, 42).unwrap(); + let chan_5 = Channel::::new_outbound(&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_0_percent, 0, 42).unwrap(); let chan_5_value_msat = chan_5.channel_value_satoshis * 1000; assert_eq!(chan_5.holder_max_htlc_value_in_flight_msat, (chan_5_value_msat as f64 * 0.01) as u64); // Test that `new_outbound` uses the upper bound of the configurable percentage values // (100%) if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a larger value // than 100. - let chan_6 = Channel::::new_outbound(&&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_101_percent, 0, 42).unwrap(); + let chan_6 = Channel::::new_outbound(&feeest, &&keys_provider, outbound_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config_101_percent, 0, 42).unwrap(); let chan_6_value_msat = chan_6.channel_value_satoshis * 1000; assert_eq!(chan_6.holder_max_htlc_value_in_flight_msat, chan_6_value_msat); // Test that `new_from_req` uses the lower bound of the configurable percentage values (1%) // if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a value less than 1. - let chan_7 = Channel::::new_from_req(&&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42).unwrap(); + let chan_7 = Channel::::new_from_req(&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42).unwrap(); let chan_7_value_msat = chan_7.channel_value_satoshis * 1000; assert_eq!(chan_7.holder_max_htlc_value_in_flight_msat, (chan_7_value_msat as f64 * 0.01) as u64); // Test that `new_from_req` uses the upper bound of the configurable percentage values // (100%) if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a larger value // than 100. - let chan_8 = Channel::::new_from_req(&&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42).unwrap(); + let chan_8 = Channel::::new_from_req(&feeest, &&keys_provider, inbound_node_id, &InitFeatures::known(), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42).unwrap(); let chan_8_value_msat = chan_8.channel_value_satoshis * 1000; assert_eq!(chan_8.holder_max_htlc_value_in_flight_msat, chan_8_value_msat); } #[test] fn channel_update() { - let feeest = TestFeeEstimator{fee_est: 15000}; + let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; @@ -6968,7 +6974,7 @@ mod tests { // Create a channel. let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let mut node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); + let mut node_a_chan = Channel::::new_outbound(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); assert!(node_a_chan.counterparty_forwarding_info.is_none()); assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default assert!(node_a_chan.counterparty_forwarding_info().is_none()); @@ -7047,7 +7053,7 @@ mod tests { let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let mut config = UserConfig::default(); config.channel_handshake_config.announced_channel = false; - let mut chan = Channel::::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0, 42).unwrap(); // Nothing uses their network key in this test + let mut chan = Channel::::new_outbound(&LowerBoundedFeeEstimator::new(&feeest), &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0, 42).unwrap(); // Nothing uses their network key in this test chan.holder_dust_limit_satoshis = 546; chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel @@ -7885,7 +7891,7 @@ mod tests { #[test] fn test_zero_conf_channel_type_support() { - let feeest = TestFeeEstimator{fee_est: 15000}; + let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; @@ -7894,7 +7900,7 @@ mod tests { let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); - let node_a_chan = Channel::::new_outbound(&&feeest, &&keys_provider, + let node_a_chan = Channel::::new_outbound(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0, 42).unwrap(); let mut channel_type_features = ChannelTypeFeatures::only_static_remote_key(); @@ -7903,7 +7909,7 @@ mod tests { let mut open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); open_channel_msg.channel_type = Some(channel_type_features); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let res = Channel::::new_from_req(&&feeest, &&keys_provider, + let res = Channel::::new_from_req(&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger, 42); assert!(res.is_ok()); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 3e364829e..bae408d29 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -36,7 +36,7 @@ use bitcoin::secp256k1; use chain; use chain::{Confirm, ChannelMonitorUpdateErr, Watch, BestBlock}; -use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator}; +use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator}; use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID}; use chain::transaction::{OutPoint, TransactionData}; // Since this struct is returned in `list_channels` methods, expose it here in case users want to @@ -688,7 +688,7 @@ pub struct ChannelManager, chain_monitor: M, tx_broadcaster: T, @@ -1592,7 +1592,7 @@ impl ChannelMana ChannelManager { default_configuration: config.clone(), genesis_hash: genesis_block(params.network).header.block_hash(), - fee_estimator: fee_est, + fee_estimator: LowerBoundedFeeEstimator::new(fee_est), chain_monitor, tx_broadcaster, @@ -7194,6 +7194,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> } } + let bounded_fee_estimator = LowerBoundedFeeEstimator::new(args.fee_estimator); + for (_, monitor) in args.channel_monitors.iter() { for (payment_hash, payment_preimage) in monitor.get_stored_preimages() { if let Some((payment_purpose, claimable_htlcs)) = claimable_htlcs.remove(&payment_hash) { @@ -7222,7 +7224,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> channel.claim_htlc_while_disconnected_dropping_mon_update(claimable_htlc.prev_hop.htlc_id, payment_preimage, &args.logger); } if let Some(previous_hop_monitor) = args.channel_monitors.get(&claimable_htlc.prev_hop.outpoint) { - previous_hop_monitor.provide_payment_preimage(&payment_hash, &payment_preimage, &args.tx_broadcaster, &args.fee_estimator, &args.logger); + previous_hop_monitor.provide_payment_preimage(&payment_hash, &payment_preimage, &args.tx_broadcaster, &bounded_fee_estimator, &args.logger); } } pending_events_read.push(events::Event::PaymentClaimed { @@ -7236,7 +7238,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> let channel_manager = ChannelManager { genesis_hash, - fee_estimator: args.fee_estimator, + fee_estimator: bounded_fee_estimator, chain_monitor: args.chain_monitor, tx_broadcaster: args.tx_broadcaster, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9ecbee739..e58b54172 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -13,6 +13,7 @@ use chain; use chain::{Confirm, Listen, Watch}; +use chain::chaininterface::LowerBoundedFeeEstimator; use chain::channelmonitor; use chain::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use chain::transaction::OutPoint; @@ -3489,7 +3490,7 @@ fn test_force_close_fail_back() { // Now check that if we add the preimage to ChannelMonitor it broadcasts our HTLC-Success.. { get_monitor!(nodes[2], payment_event.commitment_msg.channel_id) - .provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &node_cfgs[2].fee_estimator, &node_cfgs[2].logger); + .provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &LowerBoundedFeeEstimator::new(node_cfgs[2].fee_estimator), &node_cfgs[2].logger); } mine_transaction(&nodes[2], &tx); let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap(); @@ -7342,7 +7343,7 @@ fn test_user_configurable_csv_delay() { let nodes = create_network(2, &node_cfgs, &node_chanmgrs); // We test config.our_to_self > BREAKDOWN_TIMEOUT is enforced in Channel::new_outbound() - if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, + if let Err(error) = Channel::new_outbound(&LowerBoundedFeeEstimator::new(&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }), &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config, 0, 42) { @@ -7356,7 +7357,7 @@ fn test_user_configurable_csv_delay() { nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap(); let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id()); open_channel.to_self_delay = 200; - if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, + if let Err(error) = Channel::new_from_req(&LowerBoundedFeeEstimator::new(&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }), &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &low_our_to_self_config, 0, &nodes[0].logger, 42) { @@ -7388,7 +7389,7 @@ fn test_user_configurable_csv_delay() { nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap(); let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id()); open_channel.to_self_delay = 200; - if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, + if let Err(error) = Channel::new_from_req(&LowerBoundedFeeEstimator::new(&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }), &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &high_their_to_self_config, 0, &nodes[0].logger, 42) { -- 2.39.5