Fix a replay compatibility problem

rs/bitcoin/ckbtc/minter/ckbtc_minter.did

@@ -191,6 +191,9 @@ type InitArgs = record {
     /// The fee paid per check by the KYT canister.
     kyt_fee : opt nat64;
 
+    /// The canister id of the KYT canister.
+    kyt_principal: opt principal;
+
     /// The canister id of the new KYT canister.
     new_kyt_principal: opt principal;
 };
@@ -214,6 +217,9 @@ type UpgradeArgs = record {
     /// The fee per check by the KYT canister.
     kyt_fee : opt nat64;
 
+    /// The principal of the KYT canister.
+    kyt_principal : opt principal;
+
     /// The principal of the new KYT canister.
     new_kyt_principal : opt principal;
 };

rs/bitcoin/ckbtc/minter/src/guard.rs

@@ -145,6 +145,7 @@ mod tests {
             max_time_in_queue_nanos: 0,
             min_confirmations: None,
             mode: crate::state::Mode::GeneralAvailability,
+            kyt_principal: None,
             new_kyt_principal: Some(CanisterId::from(0)),
             kyt_fee: None,
         }

rs/bitcoin/ckbtc/minter/src/lifecycle/init.rs

@@ -80,6 +80,11 @@ pub struct InitArgs {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub kyt_fee: Option<u64>,
 
+    /// The principal of the KYT canister.
+    /// NOTE: this field is optional for backward compatibility.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub kyt_principal: Option<CanisterId>,
+
     /// The principal of the new KYT canister.
     /// NOTE: this field is optional for backward compatibility.
     #[serde(skip_serializing_if = "Option::is_none")]

rs/bitcoin/ckbtc/minter/src/lifecycle/upgrade.rs

@@ -31,6 +31,9 @@ pub struct UpgradeArgs {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub kyt_fee: Option<u64>,
 
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub kyt_principal: Option<CanisterId>,
+
     #[serde(skip_serializing_if = "Option::is_none")]
     pub new_kyt_principal: Option<CanisterId>,
 }

rs/bitcoin/ckbtc/minter/src/state.rs

@@ -432,6 +432,7 @@ impl CkBtcMinterState {
             mode,
             kyt_fee,
             new_kyt_principal,
+            ..
         }: InitArgs,
     ) {
         self.btc_network = btc_network.into();
@@ -459,6 +460,7 @@ impl CkBtcMinterState {
             mode,
             new_kyt_principal,
             kyt_fee,
+            ..
         }: UpgradeArgs,
     ) {
         if let Some(retrieve_btc_min_amount) = retrieve_btc_min_amount {

rs/bitcoin/ckbtc/minter/src/state/eventlog.rs

@@ -203,12 +203,25 @@ pub fn replay<I: CheckInvariants>(
         None => return Err(ReplayLogError::EmptyLog),
     };
 
+    // Because `kyt_principal` was previously used as a default
+    // substitute for `kyt_provider` during kyt_fee accounting,
+    // we need to keep track of this value so that `distribute_kyt_fee`
+    // knows when to skip giving fees to `kyt_principal`.
+    let mut previous_kyt_principal = None;
     for event in events {
         match event {
             Event::Init(args) => {
+                if args.kyt_principal.is_some() {
+                    previous_kyt_principal = args.kyt_principal.map(Principal::from);
+                }
                 state.reinit(args);
             }
-            Event::Upgrade(args) => state.upgrade(args),
+            Event::Upgrade(args) => {
+                if args.kyt_principal.is_some() {
+                    previous_kyt_principal = args.kyt_principal.map(Principal::from);
+                }
+                state.upgrade(args);
+            }
             Event::ReceivedUtxos {
                 to_account, utxos, ..
             } => state.add_utxos::<I>(to_account, utxos),
@@ -322,8 +335,12 @@ pub fn replay<I: CheckInvariants>(
                 amount,
                 ..
             } => {
-                if let Err(Overdraft(overdraft)) = state.distribute_kyt_fee(kyt_provider, amount) {
-                    return Err(ReplayLogError::InconsistentLog(format!("Attempted to distribute {amount} to {kyt_provider}, causing an overdraft of {overdraft}")));
+                if Some(kyt_provider) != previous_kyt_principal {
+                    if let Err(Overdraft(overdraft)) =
+                        state.distribute_kyt_fee(kyt_provider, amount)
+                    {
+                        return Err(ReplayLogError::InconsistentLog(format!("Attempted to distribute {amount} to {kyt_provider}, causing an overdraft of {overdraft}")));
+                    }
                 }
             }
             Event::RetrieveBtcKytFailed { kyt_provider, .. } => {

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

@@ -30,6 +30,21 @@ use serde_bytes::ByteBuf;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
 use std::str::FromStr;
 
+fn default_init_args() -> InitArgs {
+    InitArgs {
+        btc_network: Network::Regtest.into(),
+        ecdsa_key_name: "".to_string(),
+        retrieve_btc_min_amount: 0,
+        ledger_id: CanisterId::from_u64(42),
+        max_time_in_queue_nanos: 0,
+        min_confirmations: None,
+        mode: Mode::GeneralAvailability,
+        kyt_fee: None,
+        kyt_principal: None,
+        new_kyt_principal: None,
+    }
+}
+
 fn dummy_utxo_from_value(v: u64) -> Utxo {
     let mut bytes = [0u8; 32];
     bytes[0..8].copy_from_slice(&v.to_be_bytes());
@@ -820,15 +835,8 @@ proptest! {
         accounts in pvec(arb_account(), 5),
     ) {
         let mut state = CkBtcMinterState::from(InitArgs {
-            btc_network: Network::Regtest.into(),
-            ecdsa_key_name: "".to_string(),
-            retrieve_btc_min_amount: 0,
-            ledger_id: CanisterId::from_u64(42),
-            max_time_in_queue_nanos: 0,
-            min_confirmations: None,
-            mode: Mode::GeneralAvailability,
-            kyt_fee: None,
-            new_kyt_principal: None
+            retrieve_btc_min_amount: 1000,
+            ..default_init_args()
         });
         for (utxo, acc_idx) in utxos_acc_idx {
             state.add_utxos::<CheckInvariantsImpl>(accounts[acc_idx], vec![utxo]);
@@ -844,17 +852,9 @@ proptest! {
         limit in 1..25usize,
     ) {
         let mut state = CkBtcMinterState::from(InitArgs {
-            btc_network: Network::Regtest.into(),
-            ecdsa_key_name: "".to_string(),
-            retrieve_btc_min_amount: 5_000u64,
-            ledger_id: CanisterId::from_u64(42),
-            max_time_in_queue_nanos: 0,
-            min_confirmations: None,
-            mode: Mode::GeneralAvailability,
-            kyt_fee: None,
-            new_kyt_principal: None
+            retrieve_btc_min_amount: 5000,
+            ..default_init_args()
         });
-
         let mut available_amount = 0;
         for (utxo, acc_idx) in utxos_acc_idx {
             available_amount += utxo.value;
@@ -887,17 +887,9 @@ proptest! {
         resubmission_chain_length in 1..=5,
     ) {
         let mut state = CkBtcMinterState::from(InitArgs {
-            btc_network: Network::Regtest.into(),
-            ecdsa_key_name: "".to_string(),
             retrieve_btc_min_amount: 100_000,
-            ledger_id: CanisterId::from_u64(42),
-            max_time_in_queue_nanos: 0,
-            min_confirmations: None,
-            mode: Mode::GeneralAvailability,
-            kyt_fee: None,
-            new_kyt_principal: None
+            ..default_init_args()
         });
-
         for (utxo, acc_idx) in utxos_acc_idx {
             state.add_utxos::<CheckInvariantsImpl>(accounts[acc_idx], vec![utxo]);
         }
@@ -1113,15 +1105,8 @@ proptest! {
 #[test]
 fn can_form_a_batch_conditions() {
     let mut state = CkBtcMinterState::from(InitArgs {
-        btc_network: Network::Regtest.into(),
-        ecdsa_key_name: "".to_string(),
-        retrieve_btc_min_amount: 0,
-        ledger_id: CanisterId::from_u64(42),
         max_time_in_queue_nanos: 1000,
-        min_confirmations: None,
-        mode: Mode::GeneralAvailability,
-        kyt_fee: None,
-        new_kyt_principal: None,
+        ..default_init_args()
     });
     // no request, can't form a batch, fail.
     assert!(!state.can_form_a_batch(1, 0));
@@ -1170,15 +1155,8 @@ fn test_build_account_to_utxos_table_pagination() {
     use crate::dashboard;
 
     let mut state = CkBtcMinterState::from(InitArgs {
-        btc_network: Network::Regtest.into(),
-        ecdsa_key_name: "".to_string(),
         retrieve_btc_min_amount: 5_000u64,
-        ledger_id: CanisterId::from_u64(42),
-        max_time_in_queue_nanos: 0,
-        min_confirmations: None,
-        mode: Mode::GeneralAvailability,
-        kyt_fee: None,
-        new_kyt_principal: None,
+        ..default_init_args()
     });
     let account1 = Account::from(
         Principal::from_str("gjfkw-yiolw-ncij7-yzhg2-gq6ec-xi6jy-feyni-g26f4-x7afk-thx6z-6ae")

rs/bitcoin/ckbtc/minter/tests/tests.rs

@@ -44,6 +44,21 @@ const MIN_CONFIRMATIONS: u32 = 12;
 const MAX_TIME_IN_QUEUE: Duration = Duration::from_secs(10);
 const WITHDRAWAL_ADDRESS: &str = "bc1q34aq5drpuwy3wgl9lhup9892qp6svr8ldzyy7c";
 
+fn default_init_args() -> CkbtcMinterInitArgs {
+    CkbtcMinterInitArgs {
+        btc_network: Network::Regtest.into(),
+        ecdsa_key_name: "master_ecdsa_public_key".into(),
+        retrieve_btc_min_amount: 2000,
+        ledger_id: CanisterId::from(0),
+        max_time_in_queue_nanos: MAX_TIME_IN_QUEUE.as_nanos() as u64,
+        min_confirmations: Some(MIN_CONFIRMATIONS),
+        mode: Mode::GeneralAvailability,
+        kyt_fee: None,
+        kyt_principal: None,
+        new_kyt_principal: Some(CanisterId::from(0)),
+    }
+}
+
 fn ledger_wasm() -> Vec<u8> {
     let path = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap())
         .parent()
@@ -102,17 +117,8 @@ fn install_ledger(env: &StateMachine) -> CanisterId {
 
 fn install_minter(env: &StateMachine, ledger_id: CanisterId) -> CanisterId {
     let args = CkbtcMinterInitArgs {
-        btc_network: Network::Regtest.into(),
-        // The name of the [EcdsaKeyId]. Use "dfx_test_key" for local replica and "test_key_1" for
-        // a testing key for testnet and mainnet
-        ecdsa_key_name: "dfx_test_key".parse().unwrap(),
-        retrieve_btc_min_amount: 2000,
         ledger_id,
-        max_time_in_queue_nanos: 0,
-        min_confirmations: Some(1),
-        mode: Mode::GeneralAvailability,
-        kyt_fee: None,
-        new_kyt_principal: Some(CanisterId::from(0)),
+        ..default_init_args()
     };
     let minter_arg = MinterArg::Init(args);
     env.install_canister(minter_wasm(), Encode!(&minter_arg).unwrap(), None)
@@ -175,30 +181,16 @@ fn test_wrong_upgrade_parameter() {
     // wrong init args
 
     let args = MinterArg::Init(CkbtcMinterInitArgs {
-        btc_network: Network::Regtest.into(),
         ecdsa_key_name: "".into(),
-        retrieve_btc_min_amount: 100_000,
-        ledger_id: CanisterId::from_u64(0),
-        max_time_in_queue_nanos: MAX_TIME_IN_QUEUE.as_nanos() as u64,
-        min_confirmations: Some(6_u32),
-        mode: Mode::GeneralAvailability,
-        kyt_fee: Some(1001),
-        new_kyt_principal: None,
+        ..default_init_args()
     });
     let args = Encode!(&args).unwrap();
     if env.install_canister(minter_wasm(), args, None).is_ok() {
         panic!("init expected to fail")
     }
     let args = MinterArg::Init(CkbtcMinterInitArgs {
-        btc_network: Network::Regtest.into(),
-        ecdsa_key_name: "some_key".into(),
-        retrieve_btc_min_amount: 100_000,
-        ledger_id: CanisterId::from_u64(0),
-        max_time_in_queue_nanos: MAX_TIME_IN_QUEUE.as_nanos() as u64,
-        min_confirmations: Some(6_u32),
-        mode: Mode::GeneralAvailability,
-        kyt_fee: Some(1001),
         new_kyt_principal: None,
+        ..default_init_args()
     });
     let args = Encode!(&args).unwrap();
     if env.install_canister(minter_wasm(), args, None).is_ok() {
@@ -213,11 +205,9 @@ fn test_wrong_upgrade_parameter() {
 
     let upgrade_args = UpgradeArgs {
         retrieve_btc_min_amount: Some(100),
-        min_confirmations: None,
         max_time_in_queue_nanos: Some(100),
         mode: Some(Mode::ReadOnly),
-        new_kyt_principal: None,
-        kyt_fee: None,
+        ..Default::default()
     };
     let minter_arg = MinterArg::Upgrade(Some(upgrade_args));
     if env
@@ -240,12 +230,8 @@ fn test_upgrade_read_only() {
 
     // upgrade
     let upgrade_args = UpgradeArgs {
-        retrieve_btc_min_amount: Some(2000),
-        min_confirmations: None,
-        max_time_in_queue_nanos: Some(100),
         mode: Some(Mode::ReadOnly),
-        new_kyt_principal: Some(CanisterId::from(0)),
-        kyt_fee: None,
+        ..Default::default()
     };
     let minter_arg = MinterArg::Upgrade(Some(upgrade_args));
     env.upgrade_canister(minter_id, minter_wasm(), Encode!(&minter_arg).unwrap())
@@ -310,12 +296,8 @@ fn test_upgrade_restricted() {
 
     // upgrade
     let upgrade_args = UpgradeArgs {
-        retrieve_btc_min_amount: Some(2000),
-        min_confirmations: None,
-        max_time_in_queue_nanos: Some(100),
         mode: Some(Mode::RestrictedTo(vec![authorized_principal])),
-        kyt_fee: None,
-        new_kyt_principal: Some(CanisterId::from(0)),
+        ..Default::default()
     };
     let minter_arg = MinterArg::Upgrade(Some(upgrade_args));
     env.upgrade_canister(minter_id, minter_wasm(), Encode!(&minter_arg).unwrap())
@@ -364,14 +346,7 @@ fn test_upgrade_restricted() {
     );
 
     // Test restricted BTC deposits.
-    let upgrade_args = UpgradeArgs {
-        retrieve_btc_min_amount: Some(100),
-        min_confirmations: None,
-        max_time_in_queue_nanos: Some(100),
-        mode: Some(Mode::DepositsRestrictedTo(vec![authorized_principal])),
-        new_kyt_principal: Some(CanisterId::from(0)),
-        kyt_fee: None,
-    };
+    let upgrade_args = UpgradeArgs::default();
     env.upgrade_canister(minter_id, minter_wasm(), Encode!(&upgrade_args).unwrap())
         .expect("Failed to upgrade the minter canister");
 
@@ -450,12 +425,8 @@ fn test_no_new_utxos() {
 fn update_balance_should_return_correct_confirmations() {
     let ckbtc = CkBtcSetup::new();
     let upgrade_args = UpgradeArgs {
-        retrieve_btc_min_amount: None,
         min_confirmations: Some(3),
-        max_time_in_queue_nanos: None,
-        mode: None,
-        new_kyt_principal: None,
-        kyt_fee: None,
+        ..Default::default()
     };
     let minter_arg = MinterArg::Upgrade(Some(upgrade_args));
     ckbtc
@@ -565,15 +536,10 @@ fn test_minter() {
 
     let env = new_state_machine();
     let args = MinterArg::Init(CkbtcMinterInitArgs {
-        btc_network: Network::Regtest.into(),
-        ecdsa_key_name: "master_ecdsa_public_key".into(),
         retrieve_btc_min_amount: 100_000,
-        ledger_id: CanisterId::from_u64(0),
-        max_time_in_queue_nanos: MAX_TIME_IN_QUEUE.as_nanos() as u64,
         min_confirmations: Some(6_u32),
-        mode: Mode::GeneralAvailability,
         kyt_fee: Some(1001),
-        new_kyt_principal: Some(CanisterId::from(0)),
+        ..default_init_args()
     });
     let args = Encode!(&args).unwrap();
     let minter_id = env.install_canister(minter_wasm(), args, None).unwrap();
@@ -669,14 +635,12 @@ impl CkBtcSetup {
             minter_wasm(),
             Encode!(&MinterArg::Init(CkbtcMinterInitArgs {
                 btc_network: btc_network.into(),
-                ecdsa_key_name: "master_ecdsa_public_key".to_string(),
                 retrieve_btc_min_amount,
                 ledger_id,
                 max_time_in_queue_nanos: 100,
-                min_confirmations: Some(MIN_CONFIRMATIONS),
-                mode: Mode::GeneralAvailability,
                 kyt_fee: Some(KYT_FEE),
                 new_kyt_principal: new_kyt_id.into(),
+                ..default_init_args()
             }))
             .unwrap(),
         )