Skip to content

Commit

Permalink
chore(ckbtc): remove distribute_kyt_fee and reimburse_failed_kyt (#3325)
Browse files Browse the repository at this point in the history
XC-237

Since the ckBTC minter switched to using the checker canister, it no
longer needs to distribute kyt fees, or reimburse failed kyt. These
calls can be removed from the code, but we do still need to keep them
for event and state handling during replay.
  • Loading branch information
ninegua authored Jan 11, 2025
1 parent 8acef9e commit cfd1859
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 260 deletions.
23 changes: 0 additions & 23 deletions rs/bitcoin/ckbtc/minter/src/guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,29 +89,6 @@ impl Drop for TimerLogicGuard {
}
}

#[must_use]
pub struct DistributeKytFeeGuard(());

impl DistributeKytFeeGuard {
pub fn new() -> Option<Self> {
mutate_state(|s| {
if s.is_distributing_fee {
return None;
}
s.is_distributing_fee = true;
Some(DistributeKytFeeGuard(()))
})
}
}

impl Drop for DistributeKytFeeGuard {
fn drop(&mut self) {
mutate_state(|s| {
s.is_distributing_fee = false;
});
}
}

pub fn balance_update_guard(p: Principal) -> Result<Guard<PendingBalanceUpdates>, GuardError> {
Guard::new(p)
}
Expand Down
129 changes: 1 addition & 128 deletions rs/bitcoin/ckbtc/minter/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use crate::address::BitcoinAddress;
use crate::logs::{P0, P1};
use crate::management::CallError;
use crate::memo::Status;
use crate::queries::WithdrawalFee;
use crate::state::ReimbursementReason;
use crate::updates::update_balance::UpdateBalanceError;
use async_trait::async_trait;
use candid::{CandidType, Deserialize, Principal};
Expand All @@ -14,8 +12,7 @@ use ic_btc_interface::{
use ic_canister_log::log;
use ic_management_canister_types::DerivationPath;
use icrc_ledger_types::icrc1::account::Account;
use icrc_ledger_types::icrc1::transfer::{Memo, TransferError};
use num_traits::ToPrimitive;
use icrc_ledger_types::icrc1::transfer::Memo;
use scopeguard::{guard, ScopeGuard};
use serde::Serialize;
use serde_bytes::ByteBuf;
Expand Down Expand Up @@ -464,40 +461,6 @@ fn finalized_txids(candidates: &[state::SubmittedBtcTransaction], new_utxos: &[U
.collect()
}

async fn reimburse_failed_kyt() {
let try_to_reimburse = state::read_state(|s| s.pending_reimbursements.clone());
for (burn_block_index, entry) in try_to_reimburse {
let (memo_status, kyt_fee) = match entry.reason {
ReimbursementReason::TaintedDestination { kyt_fee, .. } => (Status::Rejected, kyt_fee),
ReimbursementReason::CallFailed => (Status::CallFailed, 0),
};
let reimburse_memo = crate::memo::MintMemo::KytFail {
kyt_fee: Some(kyt_fee),
status: Some(memo_status),
associated_burn_index: Some(burn_block_index),
};
if let Ok(block_index) = crate::updates::update_balance::mint(
entry
.amount
.checked_sub(kyt_fee)
.expect("reimburse underflow"),
entry.account,
crate::memo::encode(&reimburse_memo).into(),
)
.await
{
state::mutate_state(|s| {
state::audit::reimbursed_failed_deposit(
s,
burn_block_index,
block_index,
&IC_CANISTER_RUNTIME,
)
});
}
}
}

async fn finalize_requests() {
if state::read_state(|s| s.submitted_transactions.is_empty()) {
return;
Expand Down Expand Up @@ -1119,96 +1082,6 @@ fn distribute(amount: u64, n: u64) -> Vec<u64> {
shares
}

pub async fn distribute_kyt_fees() {
use icrc_ledger_client_cdk::CdkRuntime;
use icrc_ledger_client_cdk::ICRC1Client;
use icrc_ledger_types::icrc1::transfer::TransferArg;

enum MintError {
TransferError(TransferError),
CallError(i32, String),
}

impl std::fmt::Debug for MintError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
MintError::TransferError(e) => write!(f, "TransferError({:?})", e),
MintError::CallError(code, msg) => write!(f, "CallError({}, {:?})", code, msg),
}
}
}

async fn mint(amount: u64, to: candid::Principal, memo: Memo) -> Result<u64, MintError> {
debug_assert!(memo.0.len() <= CKBTC_LEDGER_MEMO_SIZE as usize);

let client = ICRC1Client {
runtime: CdkRuntime,
ledger_canister_id: state::read_state(|s| s.ledger_id.get().into()),
};
client
.transfer(TransferArg {
from_subaccount: None,
to: Account {
owner: to,
subaccount: None,
},
fee: None,
created_at_time: None,
memo: Some(memo),
amount: candid::Nat::from(amount),
})
.await
.map_err(|(code, msg)| MintError::CallError(code, msg))?
.map_err(MintError::TransferError)
.map(|n| n.0.to_u64().expect("nat does not fit into u64"))
}

let fees_to_distribute = state::read_state(|s| s.owed_kyt_amount.clone());
for (provider, amount) in fees_to_distribute {
let memo = crate::memo::MintMemo::Kyt;
match mint(amount, provider, crate::memo::encode(&memo).into()).await {
Ok(block_index) => {
state::mutate_state(|s| {
if let Err(state::Overdraft(overdraft)) = state::audit::distributed_kyt_fee(
s,
provider,
amount,
block_index,
&IC_CANISTER_RUNTIME,
) {
// This should never happen because:
// 1. The fee distribution task is guarded (at most one copy is active).
// 2. Fee distribution is the only way to decrease the balance.
log!(
P0,
"BUG[distribute_kyt_fees]: distributed {} to {} but the balance is only {}",
tx::DisplayAmount(amount),
provider,
tx::DisplayAmount(amount - overdraft),
);
} else {
log!(
P0,
"[distribute_kyt_fees]: minted {} to {}",
tx::DisplayAmount(amount),
provider,
);
}
});
}
Err(error) => {
log!(
P0,
"[distribute_kyt_fees]: failed to mint {} to {} with error: {:?}",
tx::DisplayAmount(amount),
provider,
error
);
}
}
}
}

pub fn timer<R: CanisterRuntime + 'static>(runtime: R) {
use tasks::{pop_if_ready, run_task};

Expand Down
11 changes: 0 additions & 11 deletions rs/bitcoin/ckbtc/minter/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ fn init(args: MinterArg) {
fn setup_tasks() {
schedule_now(TaskType::ProcessLogic, &IC_CANISTER_RUNTIME);
schedule_now(TaskType::RefreshFeePercentiles, &IC_CANISTER_RUNTIME);
schedule_now(TaskType::DistributeKytFee, &IC_CANISTER_RUNTIME);
}

#[cfg(feature = "self_check")]
Expand Down Expand Up @@ -84,16 +83,6 @@ fn check_invariants() -> Result<(), String> {
})
}

#[cfg(feature = "self_check")]
#[update]
async fn distribute_kyt_fee() {
let _guard = match ic_ckbtc_minter::guard::DistributeKytFeeGuard::new() {
Some(guard) => guard,
None => return,
};
ic_ckbtc_minter::distribute_kyt_fees().await;
}

#[cfg(feature = "self_check")]
#[update]
async fn refresh_fee_percentiles() {
Expand Down
3 changes: 3 additions & 0 deletions rs/bitcoin/ckbtc/minter/src/memo.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![allow(deprecated)]
use minicbor::Encoder;
use minicbor::{Decode, Encode};

Expand Down Expand Up @@ -37,9 +38,11 @@ pub enum MintMemo<'a> {
kyt_fee: Option<u64>,
},
#[n(1)]
#[deprecated]
/// The minter minted accumulated check fees to the KYT provider.
Kyt,
#[n(2)]
#[deprecated]
/// The minter failed to check retrieve btc destination address
/// or the destination address is tainted.
KytFail {
Expand Down
58 changes: 1 addition & 57 deletions rs/bitcoin/ckbtc/minter/src/state/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ use super::{
RetrieveBtcRequest, SubmittedBtcTransaction, SuspendedReason,
};
use crate::state::invariants::CheckInvariantsImpl;
use crate::state::{ReimburseDepositTask, ReimbursedDeposit};
use crate::storage::record_event;
use crate::{CanisterRuntime, ReimbursementReason, Timestamp};
use crate::{CanisterRuntime, Timestamp};
use candid::Principal;
use ic_btc_interface::{Txid, Utxo};
use icrc_ledger_types::icrc1::account::Account;
Expand Down Expand Up @@ -213,58 +212,3 @@ pub fn distributed_kyt_fee<R: CanisterRuntime>(
);
state.distribute_kyt_fee(kyt_provider, amount)
}

pub fn schedule_deposit_reimbursement<R: CanisterRuntime>(
state: &mut CkBtcMinterState,
account: Account,
amount: u64,
reason: ReimbursementReason,
burn_block_index: u64,
runtime: &R,
) {
record_event(
EventType::ScheduleDepositReimbursement {
account,
amount,
reason,
burn_block_index,
},
runtime,
);
state.schedule_deposit_reimbursement(
burn_block_index,
ReimburseDepositTask {
account,
amount,
reason,
},
);
}

pub fn reimbursed_failed_deposit<R: CanisterRuntime>(
state: &mut CkBtcMinterState,
burn_block_index: u64,
mint_block_index: u64,
runtime: &R,
) {
record_event(
EventType::ReimbursedFailedDeposit {
burn_block_index,
mint_block_index,
},
runtime,
);
let reimbursed_tx = state
.pending_reimbursements
.remove(&burn_block_index)
.expect("bug: reimbursement task should be present");
state.reimbursed_transactions.insert(
burn_block_index,
ReimbursedDeposit {
account: reimbursed_tx.account,
amount: reimbursed_tx.amount,
reason: reimbursed_tx.reason,
mint_block_index,
},
);
}
32 changes: 1 addition & 31 deletions rs/bitcoin/ckbtc/minter/src/tasks.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
#[cfg(test)]
mod tests;
use crate::{
distribute_kyt_fees, estimate_fee_per_vbyte, finalize_requests, reimburse_failed_kyt,
submit_pending_requests, CanisterRuntime,
};
use ic_btc_interface::Network;
use crate::{estimate_fee_per_vbyte, finalize_requests, submit_pending_requests, CanisterRuntime};
use scopeguard::guard;
use std::cell::{Cell, RefCell};
use std::collections::{BTreeMap, BTreeSet};
Expand All @@ -19,7 +15,6 @@ thread_local! {
pub enum TaskType {
ProcessLogic,
RefreshFeePercentiles,
DistributeKytFee,
}

#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Debug)]
Expand Down Expand Up @@ -147,7 +142,6 @@ pub(crate) async fn run_task<R: CanisterRuntime>(task: Task, runtime: R) {

submit_pending_requests().await;
finalize_requests().await;
reimburse_failed_kyt().await;
}
TaskType::RefreshFeePercentiles => {
const FEE_ESTIMATE_DELAY: Duration = Duration::from_secs(60 * 60);
Expand All @@ -165,29 +159,5 @@ pub(crate) async fn run_task<R: CanisterRuntime>(task: Task, runtime: R) {
};
let _ = estimate_fee_per_vbyte().await;
}
TaskType::DistributeKytFee => {
const MAINNET_KYT_FEE_DISTRIBUTION_PERIOD: Duration = Duration::from_secs(24 * 60 * 60);

let _enqueue_followup_guard = guard((), |_| {
schedule_after(
MAINNET_KYT_FEE_DISTRIBUTION_PERIOD,
TaskType::DistributeKytFee,
&runtime,
);
});
let _guard = match crate::guard::DistributeKytFeeGuard::new() {
Some(guard) => guard,
None => return,
};

match crate::state::read_state(|s| s.btc_network) {
Network::Mainnet | Network::Testnet => {
distribute_kyt_fees().await;
}
// We use a debug canister build exposing an endpoint
// triggering the fee distribution in tests.
Network::Regtest => {}
}
}
}
}
10 changes: 0 additions & 10 deletions rs/bitcoin/ckbtc/minter/src/tasks/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ async fn should_reschedule_refresh_fees() {
.await;
}

#[tokio::test]
async fn should_reschedule_distribute_kyt_fee() {
test_reschedule(
TaskType::DistributeKytFee,
|| crate::guard::DistributeKytFeeGuard::new().unwrap(),
Duration::from_secs(24 * 60 * 60),
)
.await;
}

async fn test_reschedule<T, G: FnOnce() -> T>(
task_type: TaskType,
guard: G,
Expand Down

0 comments on commit cfd1859

Please sign in to comment.