fix(ckbtc/ckdoge): ensure that the fee rate of replacement transactions increases

rs/bitcoin/ckbtc/minter/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test")
+load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_doc_test", "rust_library", "rust_test")
 load("//bazel:canbench.bzl", "rust_canbench")
 load("//bazel:canisters.bzl", "rust_canister")
 load("//bazel:defs.bzl", "rust_ic_test")
@@ -80,6 +80,11 @@ alias(
     actual = ":ckbtc_minter_lib",
 )
 
+rust_doc_test(
+    name = "ckbtc_minter_doc_tests",
+    crate = ":minter",
+)
+
 [
     rust_canister(
         name = name,

rs/bitcoin/ckbtc/minter/src/fees/mod.rs

@@ -1,7 +1,7 @@
 use crate::state::CkBtcMinterState;
-use crate::tx::UnsignedTransaction;
+use crate::tx::{FeeRate, UnsignedTransaction};
 use crate::{Network, fake_sign};
-use ic_btc_interface::{MillisatoshiPerByte, Satoshi};
+use ic_btc_interface::Satoshi;
 use std::cmp::max;
 #[cfg(test)]
 mod tests;
@@ -10,31 +10,24 @@ pub trait FeeEstimator {
     const DUST_LIMIT: u64;
     /// The minimum fee increment for transaction resubmission.
     /// See https://en.bitcoin.it/wiki/Miner_fees#Relaying for more detail.
-    const MIN_RELAY_FEE_RATE_INCREASE: MillisatoshiPerByte;
+    const MIN_RELAY_FEE_RATE_INCREASE: FeeRate;
 
     /// Estimate the median fees based on the given fee percentiles (slice of fee rates in milli base unit per vbyte/byte).
-    fn estimate_median_fee(
-        &self,
-        fee_percentiles: &[MillisatoshiPerByte],
-    ) -> Option<MillisatoshiPerByte> {
+    fn estimate_median_fee(&self, fee_percentiles: &[FeeRate]) -> Option<FeeRate> {
         self.estimate_nth_fee(fee_percentiles, 50)
     }
 
     /// Estimate the n-th percentile fees (n < 100) based on the given fee percentiles (slice of fee rates in milli base unit per vbyte/byte).
-    fn estimate_nth_fee(
-        &self,
-        fee_percentiles: &[MillisatoshiPerByte],
-        nth: usize,
-    ) -> Option<MillisatoshiPerByte>;
+    fn estimate_nth_fee(&self, fee_percentiles: &[FeeRate], nth: usize) -> Option<FeeRate>;
 
     /// Evaluate the fee necessary to cover the minter's cycles consumption.
     fn evaluate_minter_fee(&self, num_inputs: u64, num_outputs: u64) -> Satoshi;
 
     /// Evaluate transaction fee with the given fee rate (in milli base unit per vbyte/byte)
-    fn evaluate_transaction_fee(&self, tx: &UnsignedTransaction, fee_rate: u64) -> u64;
+    fn evaluate_transaction_fee(&self, tx: &UnsignedTransaction, fee_rate: FeeRate) -> u64;
 
     /// Compute a new minimum withdrawal amount based on the current fee rate
-    fn fee_based_minimum_withdrawal_amount(&self, median_fee: MillisatoshiPerByte) -> Satoshi;
+    fn fee_based_minimum_withdrawal_amount(&self, median_fee: FeeRate) -> Satoshi;
 
     /// Reimbursement fee in base unit for when a batch of *pending* withdrawal requests could not be processed,
     /// e.g., because it would require too many inputs.
@@ -83,12 +76,13 @@ impl BitcoinFeeEstimator {
     /// An estimated fee per vbyte of 142 millisatoshis per vbyte was selected around 2025.06.21 01:09:50 UTC
     /// for Bitcoin Mainnet, whereas the median fee around that time should have been 2_000.
     /// Until we know the root cause, we ensure that the estimated fee has a meaningful minimum value.
-    const fn minimum_fee_per_vbyte(&self) -> MillisatoshiPerByte {
-        match &self.network {
+    const fn minimum_fee_per_vbyte(&self) -> FeeRate {
+        let rate = match &self.network {
             Network::Mainnet => 1_500,
             Network::Testnet => 1_000,
             Network::Regtest => 0,
-        }
+        };
+        FeeRate::from_millis_per_byte(rate)
     }
 }
 
@@ -99,15 +93,11 @@ impl FeeEstimator for BitcoinFeeEstimator {
     // so we simply use 546 satoshi as the minimum amount per output.
     const DUST_LIMIT: u64 = 546;
 
-    const MIN_RELAY_FEE_RATE_INCREASE: MillisatoshiPerByte = 1_000;
+    const MIN_RELAY_FEE_RATE_INCREASE: FeeRate = FeeRate::from_millis_per_byte(1_000);
 
-    fn estimate_nth_fee(
-        &self,
-        fee_percentiles: &[MillisatoshiPerByte],
-        nth: usize,
-    ) -> Option<MillisatoshiPerByte> {
+    fn estimate_nth_fee(&self, fee_percentiles: &[FeeRate], nth: usize) -> Option<FeeRate> {
         /// The default fee we use on regtest networks.
-        const DEFAULT_REGTEST_FEE: MillisatoshiPerByte = 5_000;
+        const DEFAULT_REGTEST_FEE: FeeRate = FeeRate::from_millis_per_byte(5_000);
 
         let median_fee = match &self.network {
             Network::Mainnet | Network::Testnet => {
@@ -136,16 +126,15 @@ impl FeeEstimator for BitcoinFeeEstimator {
 
     /// Returns the minimum withdrawal amount based on the current median fee rate (in millisatoshi per byte).
     /// The returned amount is in satoshi.
-    fn fee_based_minimum_withdrawal_amount(&self, median_fee: MillisatoshiPerByte) -> Satoshi {
+    fn fee_based_minimum_withdrawal_amount(&self, median_fee_rate: FeeRate) -> Satoshi {
         match self.network {
             Network::Mainnet | Network::Testnet => {
                 const PER_REQUEST_RBF_BOUND: u64 = 22_100;
                 const PER_REQUEST_VSIZE_BOUND: u64 = 221;
                 const PER_REQUEST_MINTER_FEE_BOUND: u64 = 305;
 
-                let median_fee_rate = median_fee / 1_000;
                 ((PER_REQUEST_RBF_BOUND
-                    + PER_REQUEST_VSIZE_BOUND * median_fee_rate
+                    + median_fee_rate.fee_ceil(PER_REQUEST_VSIZE_BOUND)
                     + PER_REQUEST_MINTER_FEE_BOUND
                     + self.check_fee)
                     / 50_000) //TODO DEFI-2187: adjust increment of minimum withdrawal amount to be a multiple of retrieve_btc_min_amount/2
@@ -156,13 +145,9 @@ impl FeeEstimator for BitcoinFeeEstimator {
         }
     }
 
-    fn evaluate_transaction_fee(
-        &self,
-        unsigned_tx: &UnsignedTransaction,
-        fee_per_vbyte: u64,
-    ) -> u64 {
-        let tx_vsize = fake_sign(unsigned_tx).vsize();
-        (tx_vsize as u64 * fee_per_vbyte) / 1000
+    fn evaluate_transaction_fee(&self, tx: &UnsignedTransaction, fee_rate: FeeRate) -> u64 {
+        let tx_vsize = fake_sign(tx).vsize();
+        fee_rate.fee_ceil(tx_vsize as u64)
     }
 
     fn reimbursement_fee_for_pending_withdrawal_requests(&self, num_requests: u64) -> u64 {

rs/bitcoin/ckbtc/minter/src/fees/tests.rs

@@ -6,7 +6,9 @@ fn test_estimate_nth_fee() {
     let estimator = bitcoin_fee_estimator();
     let min_fee = estimator.minimum_fee_per_vbyte();
     assert_eq!(estimator.estimate_nth_fee(&[], 10), None);
-    let percentiles = (1..=100).map(|i| i * 150).collect::<Vec<_>>();
+    let percentiles = (1..=100)
+        .map(|i| FeeRate::from_millis_per_byte(i * 150))
+        .collect::<Vec<_>>();
     for i in 0..100 {
         assert_eq!(
             estimator.estimate_nth_fee(&percentiles, i),

rs/bitcoin/ckbtc/minter/src/lib.rs

@@ -21,12 +21,12 @@ use crate::fees::{BitcoinFeeEstimator, FeeEstimator};
 use crate::state::eventlog::{CkBtcEventLogger, EventLogger};
 use crate::state::utxos::UtxoSet;
 use crate::state::{CkBtcMinterState, mutate_state, read_state};
-use crate::tx::{BitcoinTransactionSigner, SignedRawTransaction, UnsignedTransaction};
+use crate::tx::{BitcoinTransactionSigner, FeeRate, SignedRawTransaction, UnsignedTransaction};
 use crate::updates::get_btc_address;
 use crate::updates::retrieve_btc::BtcAddressCheckStatus;
 pub use ic_btc_checker::CheckTransactionResponse;
 use ic_btc_checker::{CheckAddressArgs, CheckAddressResponse};
-pub use ic_btc_interface::{MillisatoshiPerByte, OutPoint, Page, Satoshi, Txid, Utxo};
+pub use ic_btc_interface::{OutPoint, Page, Satoshi, Txid, Utxo};
 
 pub mod address;
 pub mod dashboard;
@@ -223,9 +223,7 @@ async fn fetch_main_utxos<R: CanisterRuntime>(
 /// Returns an estimate for transaction fees in millisatoshi per vbyte. Returns
 /// None if the Bitcoin canister is unavailable or does not have enough data for
 /// an estimate yet.
-pub async fn estimate_fee_per_vbyte<R: CanisterRuntime>(
-    runtime: &R,
-) -> Option<MillisatoshiPerByte> {
+pub async fn estimate_fee_per_vbyte<R: CanisterRuntime>(runtime: &R) -> Option<FeeRate> {
     let btc_network = state::read_state(|s| s.btc_network);
     match runtime
         .get_current_fee_percentiles(&bitcoin_canister::GetCurrentFeePercentilesRequest {
@@ -241,7 +239,7 @@ pub async fn estimate_fee_per_vbyte<R: CanisterRuntime>(
                         fee_estimator.fee_based_minimum_withdrawal_amount(median_fee);
                     log!(
                         Priority::Debug,
-                        "[estimate_fee_per_vbyte]: update median fee per vbyte to {median_fee} and fee-based minimum retrieve amount to {fee_based_retrieve_btc_min_amount} with {fees:?}"
+                        "[estimate_fee_per_vbyte]: update median fee per vbyte to {median_fee:?} and fee-based minimum retrieve amount to {fee_based_retrieve_btc_min_amount} with {fees:?}"
                     );
                     mutate_state(|s| {
                         s.last_fee_per_vbyte = fees;
@@ -267,9 +265,7 @@ pub async fn estimate_fee_per_vbyte<R: CanisterRuntime>(
 /// Returns an estimate for transaction fees in the 25th percentile in millisatoshi per vbyte. Returns
 /// None if the Bitcoin canister is unavailable or does not have enough data for
 /// an estimate yet.
-pub async fn estimate_25th_fee_per_vbyte<R: CanisterRuntime>(
-    runtime: &R,
-) -> Option<MillisatoshiPerByte> {
+pub async fn estimate_25th_fee_per_vbyte<R: CanisterRuntime>(runtime: &R) -> Option<FeeRate> {
     let btc_network = state::read_state(|s| s.btc_network);
     match runtime
         .get_current_fee_percentiles(&bitcoin_canister::GetCurrentFeePercentilesRequest {
@@ -490,13 +486,12 @@ async fn submit_pending_requests<R: CanisterRuntime>(runtime: &R) {
         }
     });
     if let Some((req, total_fee)) = maybe_sign_request {
-        let _ = sign_and_submit_request(req, fee_millisatoshi_per_vbyte, total_fee, runtime).await;
+        let _ = sign_and_submit_request(req, total_fee, runtime).await;
     }
 }
 
 async fn sign_and_submit_request<R: CanisterRuntime>(
     req: SignTxRequest,
-    fee_millisatoshi_per_vbyte: u64,
     total_fee: WithdrawalFee,
     runtime: &R,
 ) -> Result<Txid, CallError> {
@@ -534,6 +529,7 @@ async fn sign_and_submit_request<R: CanisterRuntime>(
             );
         })?;
     let txid = signed_tx.txid();
+    let fee_rate = signed_tx.fee_rate();
 
     state::mutate_state(|s| {
         for block_index in requests_guard.0.iter_block_index() {
@@ -583,7 +579,7 @@ async fn sign_and_submit_request<R: CanisterRuntime>(
                 used_utxos,
                 change_output: Some(req.change_output),
                 submitted_at: runtime.time(),
-                fee_per_vbyte: Some(fee_millisatoshi_per_vbyte),
+                effective_fee_per_vbyte: Some(fee_rate),
                 withdrawal_fee: Some(total_fee),
                 signed_tx,
             },
@@ -807,7 +803,7 @@ pub async fn resubmit_transactions<
     Fee: FeeEstimator,
 >(
     key_name: &str,
-    fee_per_vbyte: u64,
+    fee_rate: FeeRate,
     main_address: BitcoinAddress,
     ecdsa_public_key: ECDSAPublicKey,
     btc_network: Network,
@@ -860,13 +856,19 @@ pub async fn resubmit_transactions<
             }
             continue;
         }
-        let tx_fee_per_vbyte = match submitted_tx.fee_per_vbyte {
-            Some(prev_fee) => {
-                // Ensure that the fee is at least min relay fee higher than the previous
-                // transaction fee to comply with BIP-125 (https://en.bitcoin.it/wiki/BIP_0125).
-                fee_per_vbyte.max(prev_fee + Fee::MIN_RELAY_FEE_RATE_INCREASE)
+        let tx_fee_per_vbyte = match submitted_tx.effective_fee_per_vbyte {
+            Some(prev_fee_rate) => {
+                // There are 2 requirements on the fee of a replacement transaction:
+                // 1) The fee rate strictly increases. Although not required from [BIP-125](https://en.bitcoin.it/wiki/BIP_0125),
+                // it is actually required by the [implementation](https://github.com/bitcoin/bitcoin/blob/d2ecd6815d89c9b089b55bc96fdf93b023be8dda/src/policy/rbf.cpp#L149).
+                // 2) The total fee of the replacement transaction must be at least as high as the previous transaction fee plus the minimum relay fee.
+                //
+                // To satisfy both conditions, we choose the new fee rate to be the previous one plus the minimum relay fee rate increase.
+                // This will satisfy 2) because the computed total fee of a transaction is not dependent on the variable signature sizes
+                // (see `FeeEstimator::evaluate_transaction_fee` and `fake_sign`)
+                fee_rate.max(prev_fee_rate + Fee::MIN_RELAY_FEE_RATE_INCREASE)
             }
-            None => fee_per_vbyte,
+            None => fee_rate,
         };
 
         let outputs = match &submitted_tx.requests {
@@ -928,7 +930,7 @@ pub async fn resubmit_transactions<
                     outputs,
                     &main_address,
                     max_num_inputs_in_transaction,
-                    fee_per_vbyte, // Use normal fee
+                    fee_rate, // Use normal fee
                     fee_estimator,
                 )
             }
@@ -971,6 +973,7 @@ pub async fn resubmit_transactions<
             }
         };
         let new_txid = signed_tx.txid();
+        let fee_rate = signed_tx.fee_rate();
 
         let signed_tx_hex = hex::encode(&signed_tx);
         match runtime
@@ -1002,7 +1005,7 @@ pub async fn resubmit_transactions<
                     txid: new_txid,
                     submitted_at: runtime.time(),
                     change_output: Some(change_output),
-                    fee_per_vbyte: Some(tx_fee_per_vbyte),
+                    effective_fee_per_vbyte: Some(fee_rate),
                     withdrawal_fee: Some(total_fee),
                     // Do not fill signed_tx because this is not a consolidation transaction
                     signed_tx: None,
@@ -1183,7 +1186,7 @@ pub fn build_unsigned_transaction<F: FeeEstimator>(
     outputs: Vec<(BitcoinAddress, Satoshi)>,
     main_address: &BitcoinAddress,
     max_num_inputs_in_transaction: usize,
-    fee_per_vbyte: u64,
+    fee_rate: FeeRate,
     fee_estimator: &F,
 ) -> Result<
     (
@@ -1205,7 +1208,7 @@ pub fn build_unsigned_transaction<F: FeeEstimator>(
         outputs,
         main_address,
         max_num_inputs_in_transaction,
-        fee_per_vbyte,
+        fee_rate,
         fee_estimator,
     ) {
         Ok((tx, change, total_fee)) => Ok((tx, change, total_fee, inputs)),
@@ -1224,7 +1227,7 @@ pub fn build_unsigned_transaction_from_inputs<F: FeeEstimator>(
     outputs: Vec<(BitcoinAddress, Satoshi)>,
     main_address: &BitcoinAddress,
     max_num_inputs_in_transaction: usize,
-    fee_per_vbyte: u64,
+    fee_rate: FeeRate,
     fee_estimator: &F,
 ) -> Result<(tx::UnsignedTransaction, state::ChangeOutput, WithdrawalFee), BuildTxError> {
     #[cfg(feature = "canbench-rs")]
@@ -1297,7 +1300,7 @@ pub fn build_unsigned_transaction_from_inputs<F: FeeEstimator>(
         lock_time: 0,
     };
 
-    let fee = fee_estimator.evaluate_transaction_fee(&unsigned_tx, fee_per_vbyte);
+    let fee = fee_estimator.evaluate_transaction_fee(&unsigned_tx, fee_rate);
 
     if fee + minter_fee > amount {
         return Err(BuildTxError::AmountTooLow);
@@ -1380,7 +1383,7 @@ pub fn timer<R: CanisterRuntime + 'static>(runtime: R) {
 pub fn estimate_retrieve_btc_fee<F: FeeEstimator>(
     available_utxos: &mut UtxoSet,
     withdrawal_amount: u64,
-    median_fee_millisatoshi_per_vbyte: u64,
+    median_fee_millisatoshi_per_vbyte: FeeRate,
     max_num_inputs_in_transaction: usize,
     fee_estimator: &F,
 ) -> Result<WithdrawalFee, BuildTxError> {
@@ -1538,7 +1541,7 @@ pub async fn consolidate_utxos<R: CanisterRuntime>(
         utxos,
     });
 
-    sign_and_submit_request(request, fee_millisatoshi_per_vbyte, total_fee, runtime)
+    sign_and_submit_request(request, total_fee, runtime)
         .await
         .map_err(ConsolidateUtxosError::SubmitRequest)
 }
@@ -1618,7 +1621,7 @@ pub trait CanisterRuntime {
     async fn get_current_fee_percentiles(
         &self,
         request: &GetCurrentFeePercentilesRequest,
-    ) -> Result<Vec<u64>, CallError>;
+    ) -> Result<Vec<FeeRate>, CallError>;
 
     /// Fetches all unspent transaction outputs (UTXOs) associated with the provided address in the specified network.
     async fn get_utxos(&self, request: &GetUtxosRequest) -> Result<GetUtxosResponse, CallError>;
@@ -1690,7 +1693,7 @@ impl CanisterRuntime for IcCanisterRuntime {
     async fn get_current_fee_percentiles(
         &self,
         request: &GetCurrentFeePercentilesRequest,
-    ) -> Result<Vec<u64>, CallError> {
+    ) -> Result<Vec<FeeRate>, CallError> {
         management::bitcoin_get_current_fee_percentiles(request).await
     }
 

rs/bitcoin/ckbtc/minter/src/management.rs

@@ -1,9 +1,10 @@
 //! This module contains async functions for interacting with the management canister.
 use crate::metrics::{observe_get_utxos_latency, observe_sign_with_ecdsa_latency};
+use crate::tx::FeeRate;
 use crate::{CanisterRuntime, ECDSAPublicKey, GetUtxosRequest, GetUtxosResponse, Network, tx};
 use candid::Principal;
 use ic_btc_checker::{CheckTransactionArgs, CheckTransactionResponse};
-use ic_btc_interface::{Address, MillisatoshiPerByte, Utxo};
+use ic_btc_interface::{Address, Utxo};
 use ic_cdk::bitcoin_canister;
 use ic_cdk::bitcoin_canister::GetCurrentFeePercentilesRequest;
 use ic_cdk::management_canister::SignCallError;
@@ -193,9 +194,14 @@ pub async fn bitcoin_get_utxos(request: &GetUtxosRequest) -> Result<GetUtxosResp
 /// Returns the current fee percentiles on the Bitcoin network.
 pub async fn bitcoin_get_current_fee_percentiles(
     request: &GetCurrentFeePercentilesRequest,
-) -> Result<Vec<MillisatoshiPerByte>, CallError> {
+) -> Result<Vec<FeeRate>, CallError> {
     bitcoin_canister::bitcoin_get_current_fee_percentiles(request)
         .await
+        .map(|fees| {
+            fees.into_iter()
+                .map(FeeRate::from_millis_per_byte)
+                .collect()
+        })
         .map_err(|err| CallError::from_cdk_call_error("bitcoin_get_current_fee_percentiles", err))
 }
 

rs/bitcoin/ckbtc/minter/src/metrics.rs

@@ -355,7 +355,7 @@ pub fn encode_metrics(
 
     metrics.encode_gauge(
         "ckbtc_minter_median_fee_per_vbyte",
-        state::read_state(|s| s.last_fee_per_vbyte[50]) as f64,
+        state::read_state(|s| s.last_fee_per_vbyte[50].millis()) as f64,
         "Median Bitcoin transaction fee per vbyte in Satoshi.",
     )?;