Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ckbtc): remove distribute_kyt_fee and reimburse_failed_kyt #3325

Merged
merged 2 commits into from
Jan 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
gregorydemay marked this conversation as resolved.
Show resolved Hide resolved
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(
gregorydemay marked this conversation as resolved.
Show resolved Hide resolved
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() {
gregorydemay marked this conversation as resolved.
Show resolved Hide resolved
let _guard = match ic_ckbtc_minter::guard::DistributeKytFeeGuard::new() {
gregorydemay marked this conversation as resolved.
Show resolved Hide resolved
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
Loading