Convert fee API to per_kw instead of per_vb

This (a) cuts down a bit on some conversions, reducing chances for
bugsand (b) provides greater accuracy for clients.

diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs
index fd23d9c2f876010d7287bfc1f08429c697f496d0..27891e11aa9aad2e96783659158fbc4d81aa5eac 100644 (file)
--- a/fuzz/fuzz_targets/channel_target.rs
+++ b/fuzz/fuzz_targets/channel_target.rs
@@ -80,10 +80,10 @@ struct FuzzEstimator<'a> {
        input: &'a InputData<'a>,
 }
 impl<'a> FeeEstimator for FuzzEstimator<'a> {
-       fn get_est_sat_per_vbyte(&self, _: ConfirmationTarget) -> u64 {
+       fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 {
                //TODO: We should actually be testing at least much more than 64k...
                match self.input.get_slice(2) {
-                       Some(slice) => slice_to_be16(slice) as u64,
+                       Some(slice) => slice_to_be16(slice) as u64 * 250,
                        None => 0
                }
        }

diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs
index 25d539922e0ea2cc16d3c8f2f0317d061781e00d..59f9d2bb47cc32b3e4b9f9b1cedb8a5d7c02bb19 100644 (file)
--- a/fuzz/fuzz_targets/full_stack_target.rs
+++ b/fuzz/fuzz_targets/full_stack_target.rs
@@ -84,10 +84,10 @@ struct FuzzEstimator {
        input: Arc<InputData>,
 }
 impl FeeEstimator for FuzzEstimator {
-       fn get_est_sat_per_vbyte(&self, _: ConfirmationTarget) -> u64 {
+       fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 {
                //TODO: We should actually be testing at least much more than 64k...
                match self.input.get_slice(2) {
-                       Some(slice) => slice_to_be16(slice) as u64,
+                       Some(slice) => slice_to_be16(slice) as u64 * 250,
                        None => 0
                }
        }

diff --git a/src/chain/chaininterface.rs b/src/chain/chaininterface.rs
index 518b20b128c70a6143cdb15587b694fe2bd23585..7e9e9aeff05b902923703846337cc5c9b9666edd 100644 (file)
--- a/src/chain/chaininterface.rs
+++ b/src/chain/chaininterface.rs
@@ -57,7 +57,12 @@ pub enum ConfirmationTarget {
 /// called from inside the library in response to ChainListener events, P2P events, or timer
 /// events).
 pub trait FeeEstimator: Sync + Send {
-       fn get_est_sat_per_vbyte(&self, confirmation_target: ConfirmationTarget) -> u64;
+       /// Gets estimated satoshis of fee required per 1000 Weight-Units. This translates to:
+       ///  * satoshis-per-byte * 250
+       ///  * ceil(satoshis-per-kbyte / 4)
+       /// Must be 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).
+       fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u64;
 }
 
 /// Utility to capture some common parts of ChainWatchInterface implementors.

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index 76d6f43e217b23f99bcd9aa0f4861a51e9e5d309..eb5ae008dcda8c411524c97b92bbfb53183692cd 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -350,7 +350,7 @@ impl Channel {
        }
 
        fn derive_our_dust_limit_satoshis(at_open_background_feerate: u64) -> u64 {
-               at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT //TODO
+               at_open_background_feerate * B_OUTPUT_PLUS_SPENDING_INPUT_WEIGHT / 1000 //TODO
        }
 
        fn derive_our_htlc_minimum_msat(_at_open_channel_feerate_per_kw: u64) -> u64 {
@@ -365,8 +365,8 @@ impl Channel {
                        panic!("funding value > 2^24");
                }
 
-               let feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Normal);
-               let background_feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background);
+               let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
+               let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
 
                let secp_ctx = Secp256k1::new();
                let our_channel_monitor_claim_key_hash = Hash160::from_data(&PublicKey::from_secret_key(&secp_ctx, &chan_keys.channel_monitor_claim_key).unwrap().serialize());
@@ -405,13 +405,13 @@ impl Channel {
                        last_block_connected: Default::default(),
                        funding_tx_confirmations: 0,
 
-                       feerate_per_kw: feerate * 250,
+                       feerate_per_kw: feerate,
                        their_dust_limit_satoshis: 0,
                        our_dust_limit_satoshis: Channel::derive_our_dust_limit_satoshis(background_feerate),
                        their_max_htlc_value_in_flight_msat: 0,
                        their_channel_reserve_satoshis: 0,
                        their_htlc_minimum_msat: 0,
-                       our_htlc_minimum_msat: Channel::derive_our_htlc_minimum_msat(feerate * 250),
+                       our_htlc_minimum_msat: Channel::derive_our_htlc_minimum_msat(feerate),
                        their_to_self_delay: 0,
                        their_max_accepted_htlcs: 0,
 
@@ -432,10 +432,10 @@ impl Channel {
        }
 
        fn check_remote_fee(fee_estimator: &FeeEstimator, feerate_per_kw: u32) -> Result<(), HandleError> {
-               if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background) * 250 {
+               if (feerate_per_kw as u64) < fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) {
                        return Err(HandleError{err: "Peer's feerate much too low", action: Some(msgs::ErrorAction::DisconnectPeer{ msg: None })});
                }
-               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::HighPriority) * 375 { // 375 = 250 * 1.5x
+               if (feerate_per_kw as u64) > fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) * 2 {
                        return Err(HandleError{err: "Peer's feerate much too high", action: Some(msgs::ErrorAction::DisconnectPeer{ msg: None })});
                }
                Ok(())
@@ -477,7 +477,7 @@ impl Channel {
 
                let their_announce = if (msg.channel_flags & 1) == 1 { true } else { false };
 
-               let background_feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background);
+               let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
 
                let secp_ctx = Secp256k1::new();
                let our_channel_monitor_claim_key_hash = Hash160::from_data(&PublicKey::from_secret_key(&secp_ctx, &chan_keys.channel_monitor_claim_key).unwrap().serialize());
@@ -1650,12 +1650,12 @@ impl Channel {
                let our_closing_script = self.get_closing_scriptpubkey();
 
                let (proposed_feerate, proposed_fee, our_sig) = if self.channel_outbound && self.pending_htlcs.is_empty() {
-                       let mut proposed_feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background);
-                       if self.feerate_per_kw > proposed_feerate * 250 {
-                               proposed_feerate = self.feerate_per_kw / 250;
+                       let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+                       if self.feerate_per_kw > proposed_feerate {
+                               proposed_feerate = self.feerate_per_kw;
                        }
                        let tx_weight = Self::get_closing_transaction_weight(&our_closing_script, &msg.scriptpubkey);
-                       let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 4;
+                       let proposed_total_fee_satoshis = proposed_feerate * tx_weight / 1000;
 
                        let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                        let funding_redeemscript = self.get_funding_redeemscript();
@@ -1754,7 +1754,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 / 4, false);
+                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate * closing_tx_max_weight / 1000, false);
                                sighash = Message::from_slice(&bip143::SighashComponents::new(&closing_tx).sighash_all(&closing_tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..]).unwrap();
                                let our_sig = self.secp_ctx.sign(&sighash, &self.local_keys.funding_key).unwrap();
                                self.last_sent_closing_fee = Some(($new_feerate, used_total_fee));
@@ -1766,10 +1766,10 @@ impl Channel {
                        }
                }
 
-               let proposed_sat_per_vbyte = msg.fee_satoshis * 4 / closing_tx.get_weight();
+               let proposed_sat_per_kw = msg.fee_satoshis * 1000 / closing_tx.get_weight();
                if self.channel_outbound {
-                       let our_max_feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Normal);
-                       if proposed_sat_per_vbyte > our_max_feerate {
+                       let our_max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
+                       if proposed_sat_per_kw > our_max_feerate {
                                if let Some((last_feerate, _)) = self.last_sent_closing_fee {
                                        if our_max_feerate <= last_feerate {
                                                return Err(HandleError{err: "Unable to come to consensus about closing feerate, remote wanted something higher than our Normal feerate", action: None});
@@ -1778,8 +1778,8 @@ impl Channel {
                                propose_new_feerate!(our_max_feerate);
                        }
                } else {
-                       let our_min_feerate = fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background);
-                       if proposed_sat_per_vbyte < our_min_feerate {
+                       let our_min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+                       if proposed_sat_per_kw < our_min_feerate {
                                if let Some((last_feerate, _)) = self.last_sent_closing_fee {
                                        if our_min_feerate >= last_feerate {
                                                return Err(HandleError{err: "Unable to come to consensus about closing feerate, remote wanted something lower than our Background feerate", action: None});
@@ -1857,15 +1857,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);
+               let mut res = self.feerate_per_kw * 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;
+                       res += self.feerate_per_kw * 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_vbyte(ConfirmationTarget::Normal) * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT * 250;
+               res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000;
 
                res as u32
        }
@@ -1988,7 +1988,7 @@ impl Channel {
                        max_htlc_value_in_flight_msat: Channel::get_our_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
                        channel_reserve_satoshis: Channel::get_our_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.our_htlc_minimum_msat,
-                       feerate_per_kw: fee_estimator.get_est_sat_per_vbyte(ConfirmationTarget::Background) as u32 * 250,
+                       feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
                        to_self_delay: BREAKDOWN_TIMEOUT,
                        max_accepted_htlcs: OUR_MAX_HTLCS,
                        funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key).unwrap(),
@@ -2350,7 +2350,7 @@ mod tests {
                fee_est: u64
        }
        impl FeeEstimator for TestFeeEstimator {
-               fn get_est_sat_per_vbyte(&self, _: ConfirmationTarget) -> u64 {
+               fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u64 {
                        self.fee_est
                }
        }
@@ -2364,7 +2364,7 @@ mod tests {
        #[test]
        fn outbound_commitment_test() {
                // Test vectors from BOLT 3 Appendix C:
-               let feeest = TestFeeEstimator{fee_est: 15000/250};
+               let feeest = TestFeeEstimator{fee_est: 15000};
                let secp_ctx = Secp256k1::new();
 
                let chan_keys = ChannelKeys {

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 06aa6d05271ec0f9ba089a8c5a52daa1845fee73..174f63cc76cbff7c95f212d79fc83f7c92c59c1e 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -2595,7 +2595,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
 
                for _ in 0..node_count {
-                       let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_vbyte: 1 });
+                       let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 });
                        let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new());
                        let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())});
                        let chan_monitor = Arc::new(test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone()));

diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs
index 5eec3f027897b609e453ff5ec91f130b8e2edeb6..9646754f2d0097f51358cf5be9d86c8519acfcd4 100644 (file)
--- a/src/util/test_utils.rs
+++ b/src/util/test_utils.rs
@@ -14,11 +14,11 @@ use std::sync::{Arc,Mutex};
 use std::{mem};
 
 pub struct TestFeeEstimator {
-       pub sat_per_vbyte: u64,
+       pub sat_per_kw: u64,
 }
 impl chaininterface::FeeEstimator for TestFeeEstimator {
-       fn get_est_sat_per_vbyte(&self, _confirmation_target: ConfirmationTarget) -> u64 {
-               self.sat_per_vbyte
+       fn get_est_sat_per_1000_weight(&self, _confirmation_target: ConfirmationTarget) -> u64 {
+               self.sat_per_kw
        }
 }