From f91718722af63bd6dfb960340d539c093c0288aa Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 15 Jun 2020 17:28:01 -0400 Subject: [PATCH 1/1] Switch all feerate u64's to u32's. The protocol only allows a u32, so if we received or sent something larger it would be an issue (though it's unlikely). --- fuzz/src/chanmon_consistency.rs | 2 +- fuzz/src/full_stack.rs | 4 +- lightning/src/chain/chaininterface.rs | 2 +- lightning/src/chain/keysinterface.rs | 4 +- lightning/src/ln/chan_utils.rs | 10 +-- lightning/src/ln/channel.rs | 68 ++++++++++----------- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/channelmonitor.rs | 6 +- lightning/src/ln/functional_tests.rs | 6 +- lightning/src/ln/onchaintx.rs | 46 +++++++------- lightning/src/util/enforcing_trait_impls.rs | 2 +- lightning/src/util/errors.rs | 2 +- lightning/src/util/test_utils.rs | 4 +- 13 files changed, 79 insertions(+), 79 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index e0a4d654..8be994bd 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -52,7 +52,7 @@ use std::io::Cursor; struct FuzzEstimator {} impl FeeEstimator for FuzzEstimator { - fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 { + fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 { 253 } } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 8cef7fb5..726700ad 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -94,10 +94,10 @@ struct FuzzEstimator { input: Arc, } impl FeeEstimator for FuzzEstimator { - fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 { + fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 { //TODO: We should actually be testing at least much more than 64k... match self.input.get_slice(2) { - Some(slice) => cmp::max(slice_to_be16(slice) as u64, 253), + Some(slice) => cmp::max(slice_to_be16(slice) as u32, 253), None => 253 } } diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index c187d2f8..89c57dc4 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/lightning/src/chain/chaininterface.rs @@ -119,7 +119,7 @@ pub trait FeeEstimator: Sync + Send { /// This translates to: /// * satoshis-per-byte * 250 /// * ceil(satoshis-per-kbyte / 4) - fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u64; + fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32; } /// Minimum relay fee as required by bitcoin network mempool policy. diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index fb753819..31ed2d15 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -211,7 +211,7 @@ pub trait ChannelKeys : Send+Clone { // TODO: Document the things someone using this interface should enforce before signing. // TODO: Add more input vars to enable better checking (preferably removing commitment_tx and // making the callee generate it via some util function we expose)! - fn sign_remote_commitment(&self, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()>; /// Create a signature for a local commitment transaction. This will only ever be called with /// the same local_commitment_tx (or a copy thereof), though there are currently no guarantees @@ -408,7 +408,7 @@ impl ChannelKeys for InMemoryChannelKeys { fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys } fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params } - fn sign_remote_commitment(&self, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { if commitment_tx.input.len() != 1 { return Err(()); } let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key); diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index a97a5a60..20ebc352 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -462,7 +462,7 @@ pub fn make_funding_redeemscript(a: &PublicKey, b: &PublicKey) -> Script { } /// panics if htlc.transaction_output_index.is_none()! -pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u64, to_self_delay: u16, htlc: &HTLCOutputInCommitment, a_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction { +pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u32, to_self_delay: u16, htlc: &HTLCOutputInCommitment, a_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction { let mut txins: Vec = Vec::new(); txins.push(TxIn { previous_output: OutPoint { @@ -475,9 +475,9 @@ pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u64, to_self_del }); let total_fee = if htlc.offered { - feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000 + feerate_per_kw as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000 } else { - feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000 + feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000 }; let mut txouts: Vec = Vec::new(); @@ -513,7 +513,7 @@ pub struct LocalCommitmentTransaction { pub local_keys: TxCreationKeys, /// The feerate paid per 1000-weight-unit in this commitment transaction. This value is /// controlled by the channel initiator. - pub feerate_per_kw: u64, + pub feerate_per_kw: u32, /// The HTLCs and remote htlc signatures which were included in this commitment transaction. /// /// Note that this includes all HTLCs, including ones which were considered dust and not @@ -561,7 +561,7 @@ impl LocalCommitmentTransaction { /// Generate a new LocalCommitmentTransaction based on a raw commitment transaction, /// remote signature and both parties keys - pub(crate) fn new_missing_local_sig(unsigned_tx: Transaction, their_sig: Signature, our_funding_key: &PublicKey, their_funding_key: &PublicKey, local_keys: TxCreationKeys, feerate_per_kw: u64, htlc_data: Vec<(HTLCOutputInCommitment, Option)>) -> LocalCommitmentTransaction { + pub(crate) fn new_missing_local_sig(unsigned_tx: Transaction, their_sig: Signature, our_funding_key: &PublicKey, their_funding_key: &PublicKey, local_keys: TxCreationKeys, feerate_per_kw: u32, htlc_data: Vec<(HTLCOutputInCommitment, Option)>) -> LocalCommitmentTransaction { if unsigned_tx.input.len() != 1 { panic!("Tried to store a commitment transaction that had input count != 1!"); } if unsigned_tx.input[0].witness.len() != 0 { panic!("Tried to store a signed commitment transaction?"); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 55b3d623..e9651532 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -310,16 +310,16 @@ pub(super) struct Channel { // revoke_and_ack is received and new commitment_signed is generated to be // sent to the funder. Otherwise, the pending value is removed when receiving // commitment_signed. - pending_update_fee: Option, + pending_update_fee: Option, // update_fee() during ChannelState::AwaitingRemoteRevoke is hold in // holdina_cell_update_fee then moved to pending_udpate_fee when revoke_and_ack // is received. holding_cell_update_fee is updated when there are additional // update_fee() during ChannelState::AwaitingRemoteRevoke. - holding_cell_update_fee: Option, + holding_cell_update_fee: Option, next_local_htlc_id: u64, next_remote_htlc_id: u64, update_time_counter: u32, - feerate_per_kw: u64, + feerate_per_kw: u32, #[cfg(debug_assertions)] /// Max to_local and to_remote outputs in a locally-generated commitment transaction @@ -328,7 +328,7 @@ pub(super) struct Channel { /// Max to_local and to_remote outputs in a remote-generated commitment transaction max_commitment_tx_output_remote: ::std::sync::Mutex<(u64, u64)>, - last_sent_closing_fee: Option<(u64, u64, Signature)>, // (feerate, fee, our_sig) + last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, our_sig) funding_txo: Option, @@ -448,8 +448,8 @@ impl Channel { cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO } - fn derive_our_dust_limit_satoshis(at_open_background_feerate: u64) -> u64 { - cmp::max(at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO + fn derive_our_dust_limit_satoshis(at_open_background_feerate: u32) -> u64 { + cmp::max(at_open_background_feerate as u64 * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000, 546) //TODO } // Constructors: @@ -558,10 +558,10 @@ impl Channel { fn check_remote_fee(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError> where F::Target: FeeEstimator { - if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) { + if feerate_per_kw < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) { return Err(ChannelError::Close("Peer's feerate much too low")); } - if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 { + if feerate_per_kw as u64 > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 2 { return Err(ChannelError::Close("Peer's feerate much too high")); } Ok(()) @@ -670,12 +670,12 @@ impl Channel { // check if the funder's amount for the initial commitment tx is sufficient // for full fee payment let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat; - if funders_amount_msat < background_feerate * COMMITMENT_TX_BASE_WEIGHT { + if funders_amount_msat < background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT { return Err(ChannelError::Close("Insufficient funding amount for initial commitment")); } let to_local_msat = msg.push_msat; - let to_remote_msat = funders_amount_msat - background_feerate * COMMITMENT_TX_BASE_WEIGHT; + let to_remote_msat = funders_amount_msat - background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT; if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= remote_channel_reserve_satoshis * 1000 { return Err(ChannelError::Close("Insufficient funding amount for initial commitment")); } @@ -750,7 +750,7 @@ impl Channel { last_block_connected: Default::default(), funding_tx_confirmations: 0, - feerate_per_kw: msg.feerate_per_kw as u64, + feerate_per_kw: msg.feerate_per_kw, channel_value_satoshis: msg.funding_satoshis, their_dust_limit_satoshis: msg.dust_limit_satoshis, our_dust_limit_satoshis: our_dust_limit_satoshis, @@ -828,7 +828,7 @@ impl Channel { /// Note that below-dust HTLCs are included in the third return value, but not the second, and /// sources are provided only for outbound HTLCs in the third return value. #[inline] - fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64, logger: &L) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger { + fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u32, logger: &L) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger { let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number); let txins = { @@ -868,7 +868,7 @@ impl Channel { ($htlc: expr, $outbound: expr, $source: expr, $state_name: expr) => { if $outbound == local { // "offered HTLC output" let htlc_in_tx = get_htlc_in_commitment!($htlc, true); - if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) { + if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat); txouts.push((TxOut { script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(), @@ -880,7 +880,7 @@ impl Channel { } } else { let htlc_in_tx = get_htlc_in_commitment!($htlc, false); - if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) { + if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) { log_trace!(logger, " ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat); txouts.push((TxOut { // "received HTLC output" script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(), @@ -973,7 +973,7 @@ impl Channel { max_commitment_tx_output.1 = cmp::max(max_commitment_tx_output.1, value_to_remote_msat as u64); } - let total_fee: u64 = feerate_per_kw * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let (value_to_self, value_to_remote) = if self.channel_outbound { (value_to_self_msat / 1000 - total_fee as i64, value_to_remote_msat / 1000) } else { @@ -1150,7 +1150,7 @@ impl Channel { /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output /// @local is used only to convert relevant internal structures which refer to remote vs local /// to decide value of outputs and direction of HTLCs. - fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u64) -> Transaction { + fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction { chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.their_to_self_delay } else { self.our_to_self_delay }, htlc, &keys.a_delayed_payment_key, &keys.revocation_key) } @@ -1693,7 +1693,7 @@ impl Channel { fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 { // Note that we need to divide before multiplying to round properly, // since the lowest denomination of bitcoin on-chain is the satoshi. - (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw / 1000 * 1000 + (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw as u64 / 1000 * 1000 } // Get the commitment tx fee for the local (i.e our) next commitment transaction @@ -1986,7 +1986,7 @@ impl Channel { //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction if update_fee { let num_htlcs = local_commitment_tx.1; - let total_fee: u64 = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let remote_reserve_we_require = Channel::::get_remote_channel_reserve_satoshis(self.channel_value_satoshis); if self.channel_value_satoshis - self.value_to_self_msat / 1000 < total_fee + remote_reserve_we_require { @@ -2466,7 +2466,7 @@ impl Channel { /// Adds a pending update to this channel. See the doc for send_htlc for /// further details on the optionness of the return value. /// You MUST call send_commitment prior to any other calls on this Channel - fn send_update_fee(&mut self, feerate_per_kw: u64) -> Option { + fn send_update_fee(&mut self, feerate_per_kw: u32) -> Option { if !self.channel_outbound { panic!("Cannot send fee from inbound channel"); } @@ -2487,11 +2487,11 @@ impl Channel { Some(msgs::UpdateFee { channel_id: self.channel_id, - feerate_per_kw: feerate_per_kw as u32, + feerate_per_kw: feerate_per_kw, }) } - pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u64, logger: &L) -> Result, ChannelError> where L::Target: Logger { + pub fn send_update_fee_and_commit(&mut self, feerate_per_kw: u32, logger: &L) -> Result, ChannelError> where L::Target: Logger { match self.send_update_fee(feerate_per_kw) { Some(update_fee) => { let (commitment_signed, monitor_update) = self.send_commitment_no_status_check(logger)?; @@ -2652,7 +2652,7 @@ impl Channel { return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish")); } Channel::::check_remote_fee(fee_estimator, msg.feerate_per_kw)?; - self.pending_update_fee = Some(msg.feerate_per_kw as u64); + self.pending_update_fee = Some(msg.feerate_per_kw); self.update_time_counter += 1; Ok(()) } @@ -2870,7 +2870,7 @@ impl Channel { proposed_feerate = self.feerate_per_kw; } let tx_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap()); - let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000; + let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000; let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false); let our_sig = self.local_keys @@ -3032,7 +3032,7 @@ impl Channel { macro_rules! propose_new_feerate { ($new_feerate: expr) => { let closing_tx_max_weight = Self::get_closing_transaction_weight(&self.get_closing_scriptpubkey(), self.their_shutdown_scriptpubkey.as_ref().unwrap()); - let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false); + let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * closing_tx_max_weight / 1000, false); let our_sig = self.local_keys .sign_closing_transaction(&closing_tx, &self.secp_ctx) .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction"))?; @@ -3045,10 +3045,10 @@ impl Channel { } } - let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight() as u64; + let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight() as u64; if self.channel_outbound { let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); - if proposed_sat_per_kw > our_max_feerate { + if (proposed_sat_per_kw as u32) > our_max_feerate { if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_max_feerate <= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate")); @@ -3058,7 +3058,7 @@ impl Channel { } } else { let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - if proposed_sat_per_kw < our_min_feerate { + if (proposed_sat_per_kw as u32) < our_min_feerate { if let Some((last_feerate, _, _)) = self.last_sent_closing_fee { if our_min_feerate >= last_feerate { return Err(ChannelError::Close("Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate")); @@ -3140,7 +3140,7 @@ impl Channel { } #[cfg(test)] - pub fn get_feerate(&self) -> u64 { + pub fn get_feerate(&self) -> u32 { self.feerate_per_kw } @@ -3212,15 +3212,15 @@ impl Channel { // output value back into a transaction with the regular channel output: // the fee cost of the HTLC-Success/HTLC-Timeout transaction: - let mut res = self.feerate_per_kw * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; + let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000; if self.channel_outbound { // + the marginal fee increase cost to us in the commitment transaction: - res += self.feerate_per_kw * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000; + res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000; } // + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output: - res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; + res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000; res as u32 } @@ -4481,10 +4481,10 @@ mod tests { use rand::{thread_rng,Rng}; struct TestFeeEstimator { - fee_est: u64 + fee_est: u32 } impl FeeEstimator for TestFeeEstimator { - fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 { + fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 { self.fee_est } } @@ -4547,7 +4547,7 @@ mod tests { // same as the old fee. fee_est.fee_est = 500; let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.bitcoin_hash()); - assert_eq!(open_channel_msg.feerate_per_kw as u64, original_fee); + assert_eq!(open_channel_msg.feerate_per_kw, original_fee); } #[test] diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index bbbfda69..c0bea87b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2843,7 +2843,7 @@ impl /// PeerManager::process_events afterwards. /// Note: This API is likely to change! #[doc(hidden)] - pub fn update_fee(&self, channel_id: [u8;32], feerate_per_kw: u64) -> Result<(), APIError> { + pub fn update_fee(&self, channel_id: [u8;32], feerate_per_kw: u32) -> Result<(), APIError> { let _ = self.total_consistency_lock.read().unwrap(); let their_node_id; let err: Result<(), _> = loop { diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 4f20b1d4..0b8f8c07 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -366,7 +366,7 @@ struct LocalSignedTx { b_htlc_key: PublicKey, delayed_payment_key: PublicKey, per_commitment_point: PublicKey, - feerate_per_kw: u64, + feerate_per_kw: u32, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, } @@ -1028,7 +1028,7 @@ impl ChannelMonitor { writer.write_all(&$local_tx.delayed_payment_key.serialize())?; writer.write_all(&$local_tx.per_commitment_point.serialize())?; - writer.write_all(&byte_utils::be64_to_array($local_tx.feerate_per_kw))?; + writer.write_all(&byte_utils::be32_to_array($local_tx.feerate_per_kw))?; writer.write_all(&byte_utils::be64_to_array($local_tx.htlc_outputs.len() as u64))?; for &(ref htlc_output, ref sig, ref htlc_source) in $local_tx.htlc_outputs.iter() { serialize_htlc_in_commitment!(htlc_output); @@ -2367,7 +2367,7 @@ impl Readable for (BlockHash, ChannelMonitor let b_htlc_key = Readable::read(reader)?; let delayed_payment_key = Readable::read(reader)?; let per_commitment_point = Readable::read(reader)?; - let feerate_per_kw: u64 = Readable::read(reader)?; + let feerate_per_kw: u32 = Readable::read(reader)?; let htlcs_len: u64 = Readable::read(reader)?; let mut htlcs = Vec::with_capacity(cmp::min(htlcs_len as usize, MAX_ALLOC_SIZE / 128)); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 61117201..7047de4d 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -585,7 +585,7 @@ fn test_update_fee_that_funder_cannot_afford() { //We made sure neither party's funds are below the dust limit so -2 non-HTLC txns from number of outputs let num_htlcs = commitment_tx.output.len() - 2; - let total_fee: u64 = feerate * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + let total_fee: u64 = feerate as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let mut actual_fee = commitment_tx.output.iter().fold(0, |acc, output| acc + output.value); actual_fee = channel_value - actual_fee; assert_eq!(total_fee, actual_fee); @@ -1722,8 +1722,8 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() { check_added_monitors!(nodes[1], 1); } -fn commit_tx_fee_msat(feerate: u64, num_htlcs: u64) -> u64 { - (COMMITMENT_TX_BASE_WEIGHT + num_htlcs * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate / 1000 * 1000 +fn commit_tx_fee_msat(feerate: u32, num_htlcs: u64) -> u64 { + (COMMITMENT_TX_BASE_WEIGHT + num_htlcs * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate as u64 / 1000 * 1000 } #[test] diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs index c99dc87d..9bfb4d00 100644 --- a/lightning/src/ln/onchaintx.rs +++ b/lightning/src/ln/onchaintx.rs @@ -54,7 +54,7 @@ pub struct ClaimTxBumpMaterial { // much time for confirmation and we need to bump it. height_timer: Option, // Tracked in case of reorg to wipe out now-superflous bump material - feerate_previous: u64, + feerate_previous: u32, // Soonest timelocks among set of outpoints claimed, used to compute // a priority of not feerate soonest_timelock: u32, @@ -65,7 +65,7 @@ pub struct ClaimTxBumpMaterial { impl Writeable for ClaimTxBumpMaterial { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { self.height_timer.write(writer)?; - writer.write_all(&byte_utils::be64_to_array(self.feerate_previous))?; + writer.write_all(&byte_utils::be32_to_array(self.feerate_previous))?; writer.write_all(&byte_utils::be32_to_array(self.soonest_timelock))?; writer.write_all(&byte_utils::be64_to_array(self.per_input_material.len() as u64))?; for (outp, tx_material) in self.per_input_material.iter() { @@ -151,14 +151,14 @@ impl Readable for InputDescriptors { macro_rules! subtract_high_prio_fee { ($logger: ident, $fee_estimator: expr, $value: expr, $predicted_weight: expr, $used_feerate: expr) => { { - $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority); - let mut fee = $used_feerate * ($predicted_weight as u64) / 1000; + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority).into(); + let mut fee = $used_feerate as u64 * $predicted_weight / 1000; if $value <= fee { - $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal); - fee = $used_feerate * ($predicted_weight as u64) / 1000; - if $value <= fee { - $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background); - fee = $used_feerate * ($predicted_weight as u64) / 1000; + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal).into(); + fee = $used_feerate as u64 * $predicted_weight / 1000; + if $value <= fee.into() { + $used_feerate = $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background).into(); + fee = $used_feerate as u64 * $predicted_weight / 1000; if $value <= fee { log_error!($logger, "Failed to generate an on-chain punishment tx as even low priority fee ({} sat) was more than the entire claim balance ({} sat)", fee, $value); @@ -462,7 +462,7 @@ impl OnchainTxHandler { /// Lightning security model (i.e being able to redeem/timeout HTLC or penalize coutnerparty onchain) lays on the assumption of claim transactions getting confirmed before timelock expiration /// (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. - fn generate_claim_tx(&mut self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F, logger: L) -> Option<(Option, u64, Transaction)> + fn generate_claim_tx(&mut self, height: u32, cached_claim_datas: &ClaimTxBumpMaterial, fee_estimator: F, logger: L) -> Option<(Option, u32, Transaction)> where F::Target: FeeEstimator, L::Target: Logger, { @@ -490,20 +490,20 @@ impl OnchainTxHandler { macro_rules! RBF_bump { ($amount: expr, $old_feerate: expr, $fee_estimator: expr, $predicted_weight: expr) => { { - let mut used_feerate; + let mut used_feerate: u32; // If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee... let new_fee = if $old_feerate < $fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) { let mut value = $amount; if subtract_high_prio_fee!(logger, $fee_estimator, value, $predicted_weight, used_feerate) { // Overflow check is done in subtract_high_prio_fee - $amount - value + ($amount - value) } else { log_trace!(logger, "Can't new-estimation bump new claiming tx, amount {} is too small", $amount); return None; } // ...else just increase the previous feerate by 25% (because that's a nice number) } else { - let fee = $old_feerate * $predicted_weight / 750; + let fee = $old_feerate as u64 * ($predicted_weight as u64) / 750; if $amount <= fee { log_trace!(logger, "Can't 25% bump new claiming tx, amount {} is too small", $amount); return None; @@ -511,8 +511,8 @@ impl OnchainTxHandler { fee }; - let previous_fee = $old_feerate * $predicted_weight / 1000; - let min_relay_fee = MIN_RELAY_FEE_SAT_PER_1000_WEIGHT * $predicted_weight / 1000; + let previous_fee = $old_feerate as u64 * ($predicted_weight as u64) / 1000; + let min_relay_fee = MIN_RELAY_FEE_SAT_PER_1000_WEIGHT * ($predicted_weight as u64) / 1000; // BIP 125 Opt-in Full Replace-by-Fee Signaling // * 3. The replacement transaction pays an absolute fee of at least the sum paid by the original transactions. // * 4. The replacement transaction must also pay for its own bandwidth at or above the rate set by the node's minimum relay fee setting. @@ -521,7 +521,7 @@ impl OnchainTxHandler { } else { new_fee }; - Some((new_fee, new_fee * 1000 / $predicted_weight)) + Some((new_fee, new_fee * 1000 / ($predicted_weight as u64))) } } } @@ -551,16 +551,16 @@ impl OnchainTxHandler { } } if dynamic_fee { - let predicted_weight = bumped_tx.get_weight() + inputs_witnesses_weight; + let predicted_weight = (bumped_tx.get_weight() + inputs_witnesses_weight) as u64; let mut new_feerate; // If old feerate is 0, first iteration of this claim, use normal fee calculation if cached_claim_datas.feerate_previous != 0 { - if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) { + if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight) { // If new computed fee is superior at the whole claimable amount burn all in fees - if new_fee > amt { + if new_fee as u64 > amt { bumped_tx.output[0].value = 0; } else { - bumped_tx.output[0].value = amt - new_fee; + bumped_tx.output[0].value = amt - new_fee as u64; } new_feerate = feerate; } else { return None; } @@ -620,8 +620,8 @@ impl OnchainTxHandler { } } log_trace!(logger, "...with timer {}", new_timer.unwrap()); - assert!(predicted_weight >= bumped_tx.get_weight()); - return Some((new_timer, new_feerate, bumped_tx)) + assert!(predicted_weight >= bumped_tx.get_weight() as u64); + return Some((new_timer, new_feerate as u32, bumped_tx)) } else { for (_, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() { match per_outp_material { @@ -631,7 +631,7 @@ impl OnchainTxHandler { let feerate = (amount - htlc_tx.output[0].value) * 1000 / htlc_tx.get_weight() as u64; // Timer set to $NEVER given we can't bump tx without anchor outputs log_trace!(logger, "Going to broadcast Local HTLC-{} claiming HTLC output {} from {}...", if preimage.is_some() { "Success" } else { "Timeout" }, outp.vout, outp.txid); - return Some((None, feerate, htlc_tx)); + return Some((None, feerate as u32, htlc_tx)); } return None; }, diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 6856805a..5157c1a6 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -52,7 +52,7 @@ impl ChannelKeys for EnforcingChannelKeys { fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { self.inner.pubkeys() } fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() } - fn sign_remote_commitment(&self, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { + fn sign_remote_commitment(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1) -> Result<(Signature, Vec), ()> { if commitment_tx.input.len() != 1 { panic!("lightning commitment transactions have a single input"); } self.check_keys(secp_ctx, keys); let obscured_commitment_transaction_number = (commitment_tx.lock_time & 0xffffff) as u64 | ((commitment_tx.input[0].sequence as u64 & 0xffffff) << 3*8); diff --git a/lightning/src/util/errors.rs b/lightning/src/util/errors.rs index 35bf00bd..1b29916e 100644 --- a/lightning/src/util/errors.rs +++ b/lightning/src/util/errors.rs @@ -18,7 +18,7 @@ pub enum APIError { /// A human-readable error message err: String, /// The feerate which was too high. - feerate: u64 + feerate: u32 }, /// A malformed Route was provided (eg overflowed value, node id mismatch, overly-looped route, /// too-many-hops, etc). diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index 91af4202..a79b8617 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -40,10 +40,10 @@ impl Writer for TestVecWriter { } pub struct TestFeeEstimator { - pub sat_per_kw: u64, + pub sat_per_kw: u32, } impl chaininterface::FeeEstimator for TestFeeEstimator { - fn get_est_sat_per_1000_weight(&self, _confirmation_target: ConfirmationTarget) -> u64 { + fn get_est_sat_per_1000_weight(&self, _confirmation_target: ConfirmationTarget) -> u32 { self.sat_per_kw } } -- 2.30.2