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

feat(ckbtc): Use the new KYT canister in ckbtc withdrawal flow #2240

Merged
merged 16 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions rs/bitcoin/ckbtc/minter/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ LIB_DEPS = [
"//packages/icrc-ledger-client-cdk:icrc_ledger_client_cdk",
"//packages/icrc-ledger-types:icrc_ledger_types",
"//rs/bitcoin/ckbtc/kyt",
"//rs/bitcoin/kyt:btc_kyt_lib",
"//rs/crypto/getrandom_for_wasm",
"//rs/crypto/secp256k1",
"//rs/crypto/sha2",
Expand Down Expand Up @@ -143,6 +144,7 @@ rust_ic_test(
"CARGO_MANIFEST_DIR": "rs/bitcoin/ckbtc/minter",
"IC_CKBTC_MINTER_WASM_PATH": "$(rootpath :ckbtc_minter_debug.wasm)",
"IC_ICRC1_LEDGER_WASM_PATH": "$(rootpath //rs/ledger_suite/icrc1/ledger:ledger_canister)",
"IC_BTC_KYT_WASM_PATH": "$(rootpath //rs/bitcoin/kyt:btc_kyt_canister)",
"IC_CKBTC_KYT_WASM_PATH": "$(rootpath //rs/bitcoin/ckbtc/kyt:kyt_canister)",
"IC_BITCOIN_CANISTER_MOCK_WASM_PATH": "$(rootpath //rs/bitcoin/mock:bitcoin_canister_mock)",
},
Expand All @@ -151,6 +153,7 @@ rust_ic_test(
":ckbtc_minter_lib",
"//packages/icrc-ledger-types:icrc_ledger_types",
"//rs/bitcoin/ckbtc/kyt",
"//rs/bitcoin/kyt:btc_kyt_lib",
"//rs/bitcoin/mock",
"//rs/config",
"//rs/ledger_suite/icrc1",
Expand Down
1 change: 1 addition & 0 deletions rs/bitcoin/ckbtc/minter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ ciborium = { workspace = true }
hex = { workspace = true }
ic-base-types = { path = "../../../types/base_types" }
ic-btc-interface = { workspace = true }
ic-btc-kyt = { path = "../../kyt" }
ic-canister-log = { path = "../../../rust_canisters/canister_log" }
ic-canisters-http-types = { path = "../../../rust_canisters/http_types" }
ic-cdk = { workspace = true }
Expand Down
11 changes: 11 additions & 0 deletions rs/bitcoin/ckbtc/minter/ckbtc_minter.did
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ type InitArgs = record {

/// The canister id of the KYT canister.
kyt_principal: opt principal;

/// The canister id of the new KYT canister.
new_kyt_principal: opt principal;
ninegua marked this conversation as resolved.
Show resolved Hide resolved
};

// The upgrade parameters of the minter canister.
Expand All @@ -216,6 +219,9 @@ type UpgradeArgs = record {

/// The principal of the KYT canister.
kyt_principal : opt principal;

/// The principal of the new KYT canister.
new_kyt_principal : opt principal;
};

type RetrieveBtcStatus = variant {
Expand Down Expand Up @@ -368,6 +374,11 @@ type Event = variant {
uuid : text;
block_index : nat64;
};
retrieve_btc_ofac_failed : record {
ninegua marked this conversation as resolved.
Show resolved Hide resolved
address : text;
amount : nat64;
owner : principal;
};
schedule_deposit_reimbursement : record {
account : Account;
burn_block_index : nat64;
Expand Down
1 change: 1 addition & 0 deletions rs/bitcoin/ckbtc/minter/src/guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ mod tests {
max_time_in_queue_nanos: 0,
min_confirmations: None,
mode: crate::state::Mode::GeneralAvailability,
new_kyt_principal: Some(CanisterId::from(0)),
kyt_principal: Some(CanisterId::from(0)),
kyt_fee: None,
}
Expand Down
5 changes: 2 additions & 3 deletions rs/bitcoin/ckbtc/minter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1255,11 +1255,10 @@ pub fn tx_vsize_estimate(input_count: u64, output_count: u64) -> u64 {
/// * `available_utxos` - the list of UTXOs available to the minter.
/// * `maybe_amount` - the withdrawal amount.
/// * `median_fee_millisatoshi_per_vbyte` - the median network fee, in millisatoshi per vbyte.
pub fn estimate_fee(
pub fn estimate_retrieve_btc_fee(
available_utxos: &BTreeSet<Utxo>,
maybe_amount: Option<u64>,
median_fee_millisatoshi_per_vbyte: u64,
kyt_fee: u64,
) -> WithdrawalFee {
const DEFAULT_INPUT_COUNT: u64 = 2;
// One output for the caller and one for the change.
Expand Down Expand Up @@ -1293,7 +1292,7 @@ pub fn estimate_fee(
vsize * median_fee_millisatoshi_per_vbyte / 1000 / (DEFAULT_OUTPUT_COUNT - 1).max(1);
let minter_fee = minter_fee / (DEFAULT_OUTPUT_COUNT - 1).max(1);
WithdrawalFee {
minter_fee: kyt_fee + minter_fee,
minter_fee,
bitcoin_fee,
}
}
5 changes: 5 additions & 0 deletions rs/bitcoin/ckbtc/minter/src/lifecycle/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ pub struct InitArgs {
/// 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")]
pub new_kyt_principal: Option<CanisterId>,
}

pub fn init(args: InitArgs) {
Expand Down
3 changes: 3 additions & 0 deletions rs/bitcoin/ckbtc/minter/src/lifecycle/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ pub struct UpgradeArgs {

#[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>,
}

pub fn post_upgrade(upgrade_args: Option<UpgradeArgs>) {
Expand Down
3 changes: 1 addition & 2 deletions rs/bitcoin/ckbtc/minter/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,10 @@ async fn get_canister_status() -> ic_cdk::api::management_canister::main::Canist
#[query]
fn estimate_withdrawal_fee(arg: EstimateFeeArg) -> WithdrawalFee {
read_state(|s| {
ic_ckbtc_minter::estimate_fee(
ic_ckbtc_minter::estimate_retrieve_btc_fee(
&s.available_utxos,
arg.amount,
s.last_fee_per_vbyte[50],
s.kyt_fee,
gregorydemay marked this conversation as resolved.
Show resolved Hide resolved
)
})
}
Expand Down
19 changes: 19 additions & 0 deletions rs/bitcoin/ckbtc/minter/src/management.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use ic_btc_interface::{
Address, GetCurrentFeePercentilesRequest, GetUtxosRequest, GetUtxosResponse,
MillisatoshiPerByte, Network, Utxo, UtxosFilterInRequest,
};
use ic_btc_kyt::{CheckAddressArgs, CheckAddressResponse};
use ic_canister_log::log;
use ic_cdk::api::call::RejectionCode;
use ic_ckbtc_kyt::{DepositRequest, Error as KytError, FetchAlertsResponse, WithdrawalAttempt};
Expand Down Expand Up @@ -347,3 +348,21 @@ pub async fn fetch_withdrawal_alerts(
})?;
Ok(res)
}

/// Check if the given Bitcoin address is blocked.
pub async fn check_withdrawal_destination_address(
kyt_principal: Principal,
address: String,
) -> Result<CheckAddressResponse, CallError> {
let (res,): (CheckAddressResponse,) = ic_cdk::api::call::call(
kyt_principal,
"check_address",
(CheckAddressArgs { address },),
)
.await
.map_err(|(code, message)| CallError {
method: "check_address".to_string(),
reason: Reason::from_reject(code, message),
})?;
Ok(res)
}
17 changes: 16 additions & 1 deletion rs/bitcoin/ckbtc/minter/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,9 @@ pub struct CkBtcMinterState {
/// The principal of the KYT canister.
pub kyt_principal: Option<CanisterId>,

/// The new principal of the KYT canister.
pub new_kyt_principal: Option<CanisterId>,

/// The set of UTXOs unused in pending transactions.
pub available_utxos: BTreeSet<Utxo>,

Expand Down Expand Up @@ -427,6 +430,7 @@ impl CkBtcMinterState {
mode,
kyt_fee,
kyt_principal,
new_kyt_principal,
}: InitArgs,
) {
self.btc_network = btc_network.into();
Expand All @@ -436,6 +440,7 @@ impl CkBtcMinterState {
self.max_time_in_queue_nanos = max_time_in_queue_nanos;
self.mode = mode;
self.kyt_principal = kyt_principal;
self.new_kyt_principal = new_kyt_principal;
if let Some(kyt_fee) = kyt_fee {
self.kyt_fee = kyt_fee;
}
Expand All @@ -451,6 +456,7 @@ impl CkBtcMinterState {
max_time_in_queue_nanos,
min_confirmations,
mode,
new_kyt_principal,
kyt_principal,
kyt_fee,
}: UpgradeArgs,
Expand All @@ -476,6 +482,9 @@ impl CkBtcMinterState {
if let Some(mode) = mode {
self.mode = mode;
}
if let Some(new_kyt_principal) = new_kyt_principal {
self.new_kyt_principal = Some(new_kyt_principal);
}
if let Some(kyt_principal) = kyt_principal {
self.kyt_principal = Some(kyt_principal);
}
Expand All @@ -494,6 +503,9 @@ impl CkBtcMinterState {
if self.kyt_principal.is_none() {
ic_cdk::trap("KYT principal is not set");
}
if self.new_kyt_principal.is_none() {
ic_cdk::trap("New KYT principal is not set");
}
}

pub fn check_invariants(&self) -> Result<(), String> {
Expand Down Expand Up @@ -1087,7 +1099,9 @@ impl CkBtcMinterState {
kyt_provider,
kyt_fee,
} => {
*self.owed_kyt_amount.entry(kyt_provider).or_insert(0) += kyt_fee;
if kyt_fee > 0 {
*self.owed_kyt_amount.entry(kyt_provider).or_insert(0) += kyt_fee;
}
}
ReimbursementReason::CallFailed => {}
}
Expand Down Expand Up @@ -1250,6 +1264,7 @@ impl From<InitArgs> for CkBtcMinterState {
tokens_burned: 0,
ledger_id: args.ledger_id,
kyt_principal: args.kyt_principal,
new_kyt_principal: args.new_kyt_principal,
available_utxos: Default::default(),
outpoint_account: Default::default(),
utxos_state_addresses: Default::default(),
Expand Down
16 changes: 2 additions & 14 deletions rs/bitcoin/ckbtc/minter/src/state/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,12 @@ pub fn distributed_kyt_fee(
state.distribute_kyt_fee(kyt_provider, amount)
}

pub fn retrieve_btc_kyt_failed(
state: &mut CkBtcMinterState,
owner: Principal,
address: String,
amount: u64,
kyt_provider: Principal,
uuid: String,
block_index: u64,
) {
record_event(&Event::RetrieveBtcKytFailed {
pub fn retrieve_btc_ofac_failed(owner: Principal, address: String, amount: u64) {
record_event(&Event::RetrieveBtcOfacFailed {
owner,
address,
amount,
kyt_provider,
uuid,
block_index,
});
*state.owed_kyt_amount.entry(kyt_provider).or_insert(0) += state.kyt_fee;
}

pub fn schedule_deposit_reimbursement(
Expand Down
12 changes: 12 additions & 0 deletions rs/bitcoin/ckbtc/minter/src/state/eventlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,17 @@ pub enum Event {
block_index: u64,
},

/// Indicates that the KYT check for the specified address failed.
#[serde(rename = "retrieve_btc_ofac_failed")]
RetrieveBtcOfacFailed {
/// The owner of the address.
owner: Principal,
/// The address that failed the KYT check.
address: String,
/// The amount associated with the failed KYT check.
amount: u64,
ninegua marked this conversation as resolved.
Show resolved Hide resolved
},

/// Indicates a reimbursement.
#[serde(rename = "schedule_deposit_reimbursement")]
ScheduleDepositReimbursement {
Expand Down Expand Up @@ -336,6 +347,7 @@ pub fn replay(mut events: impl Iterator<Item = Event>) -> Result<CkBtcMinterStat
Event::RetrieveBtcKytFailed { kyt_provider, .. } => {
*state.owed_kyt_amount.entry(kyt_provider).or_insert(0) += state.kyt_fee;
}
Event::RetrieveBtcOfacFailed { .. } => {}
ninegua marked this conversation as resolved.
Show resolved Hide resolved
Event::ScheduleDepositReimbursement {
account,
amount,
Expand Down
22 changes: 13 additions & 9 deletions rs/bitcoin/ckbtc/minter/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::MINTER_FEE_CONSTANT;
use crate::{
address::BitcoinAddress, build_unsigned_transaction, estimate_fee, fake_sign, greedy,
signature::EncodedSignature, tx, BuildTxError,
address::BitcoinAddress, build_unsigned_transaction, estimate_retrieve_btc_fee, fake_sign,
greedy, signature::EncodedSignature, tx, BuildTxError,
};
use crate::{
lifecycle::init::InitArgs,
Expand Down Expand Up @@ -685,8 +685,8 @@ proptest! {

let target = total_value / 2;

let fee_estimate = estimate_fee(&utxos, Some(target), fee_per_vbyte, crate::lifecycle::init::DEFAULT_KYT_FEE);
let fee_estimate = fee_estimate.minter_fee + fee_estimate.bitcoin_fee - crate::lifecycle::init::DEFAULT_KYT_FEE;
let fee_estimate = estimate_retrieve_btc_fee(&utxos, Some(target), fee_per_vbyte);
let fee_estimate = fee_estimate.minter_fee + fee_estimate.bitcoin_fee;

let (unsigned_tx, _, _) = build_unsigned_transaction(
&mut utxos,
Expand Down Expand Up @@ -838,7 +838,8 @@ proptest! {
min_confirmations: None,
mode: Mode::GeneralAvailability,
kyt_fee: None,
kyt_principal: None
kyt_principal: None,
new_kyt_principal: None
});
for (utxo, acc_idx) in utxos_acc_idx {
state.add_utxos(accounts[acc_idx], vec![utxo]);
Expand All @@ -862,7 +863,8 @@ proptest! {
min_confirmations: None,
mode: Mode::GeneralAvailability,
kyt_fee: None,
kyt_principal: None
kyt_principal: None,
new_kyt_principal: None
});

let mut available_amount = 0;
Expand Down Expand Up @@ -905,7 +907,8 @@ proptest! {
min_confirmations: None,
mode: Mode::GeneralAvailability,
kyt_fee: None,
kyt_principal: None
kyt_principal: None,
new_kyt_principal: None
});

for (utxo, acc_idx) in utxos_acc_idx {
Expand Down Expand Up @@ -1107,9 +1110,8 @@ proptest! {
) {
const SMALLEST_TX_SIZE_VBYTES: u64 = 140; // one input, two outputs
const MIN_MINTER_FEE: u64 = 312;
let kyt_fee: u64 = crate::lifecycle::init::DEFAULT_KYT_FEE;

let estimate = estimate_fee(&utxos, amount, fee_per_vbyte, kyt_fee);
let estimate = estimate_retrieve_btc_fee(&utxos, amount, fee_per_vbyte);
let lower_bound = MIN_MINTER_FEE + SMALLEST_TX_SIZE_VBYTES * fee_per_vbyte / 1000;
let estimate_amount = estimate.minter_fee + estimate.bitcoin_fee;
prop_assert!(
Expand All @@ -1133,6 +1135,7 @@ fn can_form_a_batch_conditions() {
mode: Mode::GeneralAvailability,
kyt_fee: None,
kyt_principal: None,
new_kyt_principal: None,
});
// no request, can't form a batch, fail.
assert!(!state.can_form_a_batch(1, 0));
Expand Down Expand Up @@ -1190,6 +1193,7 @@ fn test_build_account_to_utxos_table_pagination() {
mode: Mode::GeneralAvailability,
kyt_fee: None,
kyt_principal: None,
new_kyt_principal: None,
});
let account1 = Account::from(
Principal::from_str("gjfkw-yiolw-ncij7-yzhg2-gq6ec-xi6jy-feyni-g26f4-x7afk-thx6z-6ae")
Expand Down
Loading