From 5b52dc317ec41fea70ff18ea7b5c574c79b3831b Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 14:05:59 +0200 Subject: [PATCH 01/16] feat(ckbtc-minter): support ICRC-21 consent messages Implement the ICRC-21 canister call consent message standard on the ckBTC minter so that ICRC-21-aware wallets can show the user a human- readable description of `retrieve_btc_with_approval` (and `retrieve_btc`) calls before signing. Both display variants are supported: - GenericDisplay: a Markdown message for software wallets. - FieldsDisplay: a list of (label, Value) fields tailored for hardware wallets like the Ledger Nano S+. Long Text values (Bitcoin addresses, hex subaccounts) are split into chunks of at most 18 characters with a "(N/M)" pagination suffix, and labels are kept short ("BTC address", "From subaccount", "Amount") so they fit on a single hardware-wallet line. Also adds an `icrc10_supported_standards` query that advertises ICRC-10 and ICRC-21. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/ckbtc_minter.did | 77 ++- rs/bitcoin/ckbtc/minter/src/main.rs | 15 + rs/bitcoin/ckbtc/minter/src/updates.rs | 1 + rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 602 ++++++++++++++++++ 4 files changed, 694 insertions(+), 1 deletion(-) create mode 100644 rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs diff --git a/rs/bitcoin/ckbtc/minter/ckbtc_minter.did b/rs/bitcoin/ckbtc/minter/ckbtc_minter.did index 385b47dbb72e..e1cb4afd6b4d 100644 --- a/rs/bitcoin/ckbtc/minter/ckbtc_minter.did +++ b/rs/bitcoin/ckbtc/minter/ckbtc_minter.did @@ -611,7 +611,70 @@ type DecodeLedgerMemoResult = variant { // An error in case the minter was not able to decode the provided memo. This field is `opt`, so that other error // types can be added in the future. Err : opt DecodeLedgerMemoError; -} +}; + +// ICRC-10 supported standards record. +type Icrc10StandardRecord = record { + name : text; + url : text; +}; + +// ICRC-21 Canister Call Consent Message types. +type Icrc21ConsentMessageMetadata = record { + language : text; + utc_offset_minutes : opt int16; +}; + +type Icrc21DeviceSpec = variant { + GenericDisplay; + FieldsDisplay; +}; + +type Icrc21ConsentMessageSpec = record { + metadata : Icrc21ConsentMessageMetadata; + device_spec : opt Icrc21DeviceSpec; +}; + +type Icrc21ConsentMessageRequest = record { + method : text; + arg : blob; + user_preferences : Icrc21ConsentMessageSpec; +}; + +type Icrc21Value = variant { + TokenAmount : record { + decimals : nat8; + amount : nat64; + symbol : text; + }; + TimestampSeconds : record { amount : nat64 }; + DurationSeconds : record { amount : nat64 }; + Text : record { content : text }; +}; + +type Icrc21FieldsDisplay = record { + intent : text; + fields : vec record { text; Icrc21Value }; +}; + +type Icrc21ConsentMessage = variant { + GenericDisplayMessage : text; + FieldsDisplayMessage : Icrc21FieldsDisplay; +}; + +type Icrc21ConsentInfo = record { + consent_message : Icrc21ConsentMessage; + metadata : Icrc21ConsentMessageMetadata; +}; + +type Icrc21ErrorInfo = record { description : text }; + +type Icrc21Error = variant { + UnsupportedCanisterCall : Icrc21ErrorInfo; + ConsentMessageUnavailable : Icrc21ErrorInfo; + InsufficientPayment : Icrc21ErrorInfo; + GenericError : record { error_code : nat; description : text }; +}; service : (minter_arg : MinterArg) -> { // Section "Convert BTC to ckBTC" {{{ @@ -728,4 +791,16 @@ service : (minter_arg : MinterArg) -> { // Returns information related to minter transactions. decode_ledger_memo : (DecodeLedgerMemoArgs) -> (DecodeLedgerMemoResult) query; // }}} + + // Section "ICRC-10 / ICRC-21" {{{ + + // Returns the list of supported ICRC standards. + icrc10_supported_standards : () -> (vec Icrc10StandardRecord) query; + + // Returns a human-readable consent message describing the requested + // canister call. See the ICRC-21 standard for details: + // https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md + icrc21_canister_call_consent_message : (Icrc21ConsentMessageRequest) -> (variant { Ok : Icrc21ConsentInfo; Err : Icrc21Error }); + + // }}} Section "ICRC-10 / ICRC-21" } diff --git a/rs/bitcoin/ckbtc/minter/src/main.rs b/rs/bitcoin/ckbtc/minter/src/main.rs index d692d0866cc2..f4d390340f0d 100644 --- a/rs/bitcoin/ckbtc/minter/src/main.rs +++ b/rs/bitcoin/ckbtc/minter/src/main.rs @@ -30,6 +30,9 @@ use ic_ckbtc_minter::{ }; use ic_http_types::{HttpRequest, HttpResponse}; use icrc_ledger_types::icrc1::account::Account; +use icrc_ledger_types::icrc21::errors::Icrc21Error; +use icrc_ledger_types::icrc21::requests::ConsentMessageRequest; +use icrc_ledger_types::icrc21::responses::ConsentInfo; #[init] fn init(args: MinterArg) { @@ -265,6 +268,18 @@ fn decode_ledger_memo(arg: DecodeLedgerMemoArgs) -> DecodeLedgerMemoResult { ic_ckbtc_minter::queries::decode_ledger_memo(arg) } +#[update] +fn icrc21_canister_call_consent_message( + consent_msg_request: ConsentMessageRequest, +) -> Result { + updates::icrc21::icrc21_canister_call_consent_message(consent_msg_request) +} + +#[query] +fn icrc10_supported_standards() -> Vec { + updates::icrc21::icrc10_supported_standards() +} + #[query(hidden = true)] fn http_request(req: HttpRequest) -> HttpResponse { if ic_cdk::api::in_replicated_execution() { diff --git a/rs/bitcoin/ckbtc/minter/src/updates.rs b/rs/bitcoin/ckbtc/minter/src/updates.rs index 06de24cc38b3..9a2ac30780b5 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates.rs @@ -3,6 +3,7 @@ mod tests; pub mod get_btc_address; pub mod get_withdrawal_account; +pub mod icrc21; pub mod retrieve_btc; pub mod update_balance; diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs new file mode 100644 index 000000000000..451ef972e6d9 --- /dev/null +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -0,0 +1,602 @@ +//! Implementation of the [ICRC-21](https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md) +//! Canister Call Consent Message standard for the ckBTC minter. + +use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs}; +use candid::{CandidType, Decode, Deserialize}; +use icrc_ledger_types::icrc1::account::Subaccount; +use icrc_ledger_types::icrc21::errors::{ErrorInfo, Icrc21Error}; +use icrc_ledger_types::icrc21::requests::{ + ConsentMessageMetadata, ConsentMessageRequest, DisplayMessageType, +}; +use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDisplay, Value}; + +/// The token symbol used in consent messages. The minter burns ckBTC to +/// release the equivalent in BTC. +pub const TOKEN_SYMBOL: &str = "ckBTC"; + +/// The number of decimals used to display token amounts. +/// Both ckBTC and BTC use 8 decimals (1 BTC = 10^8 satoshis). +pub const DECIMALS: u8 = 8; + +/// Maximum number of bytes that an argument to a ckBTC minter method can have +/// when passed through the ICRC-21 endpoint. +pub const MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES: usize = 500; + +/// Maximum number of characters per `Value::Text` field for the FieldsDisplay +/// variant. Hardware wallets like the Ledger Nano S+ have a small display and +/// cannot fit a full Bitcoin address (up to 62 characters for bech32m) or a +/// 64-character hex subaccount on a single screen. Long values are split into +/// multiple fields with a "(N/M)" pagination suffix in the label so each chunk +/// fits on a single screen with the label on top. +/// +/// Field labels are kept short (e.g. "BTC address", "From subaccount") so that +/// even with a "(N/M)" suffix the label still fits on one line at the typical +/// hardware-wallet font size. +pub const FIELDS_DISPLAY_TEXT_CHUNK_LEN: usize = 18; + +/// An entry of the ICRC-10 supported standards list. +#[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] +pub struct StandardRecord { + pub name: String, + pub url: String, +} + +pub fn icrc10_supported_standards() -> Vec { + vec![ + StandardRecord { + name: "ICRC-10".to_string(), + url: "https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-10/ICRC-10.md".to_string(), + }, + StandardRecord { + name: "ICRC-21".to_string(), + url: "https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md".to_string(), + }, + ] +} + +pub fn icrc21_canister_call_consent_message( + consent_msg_request: ConsentMessageRequest, +) -> Result { + if consent_msg_request.arg.len() > MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES { + return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { + description: format!( + "The argument size is too large. The maximum allowed size is \ + {MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES} bytes." + ), + })); + } + + let display_type = consent_msg_request + .user_preferences + .device_spec + .clone() + .unwrap_or(DisplayMessageType::GenericDisplay); + + let consent_message = match consent_msg_request.method.as_str() { + "retrieve_btc_with_approval" => { + let args = Decode!( + consent_msg_request.arg.as_slice(), + RetrieveBtcWithApprovalArgs + ) + .map_err(|e| { + Icrc21Error::UnsupportedCanisterCall(ErrorInfo { + description: format!("Failed to decode RetrieveBtcWithApprovalArgs: {e}"), + }) + })?; + build_retrieve_btc_with_approval_message(&args, &display_type) + } + "retrieve_btc" => { + let args = + Decode!(consent_msg_request.arg.as_slice(), RetrieveBtcArgs).map_err(|e| { + Icrc21Error::UnsupportedCanisterCall(ErrorInfo { + description: format!("Failed to decode RetrieveBtcArgs: {e}"), + }) + })?; + build_retrieve_btc_message(&args, &display_type) + } + method => { + return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { + description: format!( + "The method '{method}' is not supported by the ckBTC minter ICRC-21 endpoint." + ), + })); + } + }; + + // Respond in English regardless of what the client requested for now. + let metadata = ConsentMessageMetadata { + language: "en".to_string(), + utc_offset_minutes: consent_msg_request + .user_preferences + .metadata + .utc_offset_minutes, + }; + + Ok(ConsentInfo { + metadata, + consent_message, + }) +} + +fn build_retrieve_btc_with_approval_message( + args: &RetrieveBtcWithApprovalArgs, + display_type: &DisplayMessageType, +) -> ConsentMessage { + match display_type { + DisplayMessageType::GenericDisplay => { + let mut message = String::new(); + message.push_str("# Convert ckBTC to BTC"); + message.push_str( + "\n\nAuthorize the ckBTC minter to burn ckBTC from your account and \ + send the equivalent amount in BTC (minus network and minter fees) to \ + the Bitcoin address below.", + ); + message.push_str(&format!( + "\n\n**Amount to convert:** `{} ckBTC`", + format_amount(args.amount, DECIMALS) + )); + message.push_str(&format!( + "\n\n**Bitcoin destination address:**\n`{}`", + args.address + )); + if let Some(subaccount) = args.from_subaccount { + message.push_str(&format!( + "\n\n**ckBTC source subaccount:**\n`{}`", + hex::encode(subaccount) + )); + } + ConsentMessage::GenericDisplayMessage(message) + } + DisplayMessageType::FieldsDisplay => { + let mut fields = Vec::new(); + fields.push(( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: args.amount, + symbol: TOKEN_SYMBOL.to_string(), + }, + )); + push_chunked_text(&mut fields, "BTC address", &args.address); + if let Some(subaccount) = args.from_subaccount { + push_chunked_text( + &mut fields, + "From subaccount", + &format_subaccount(&subaccount), + ); + } + ConsentMessage::FieldsDisplayMessage(FieldsDisplay { + intent: "ckBTC to BTC".to_string(), + fields, + }) + } + } +} + +fn build_retrieve_btc_message( + args: &RetrieveBtcArgs, + display_type: &DisplayMessageType, +) -> ConsentMessage { + match display_type { + DisplayMessageType::GenericDisplay => { + let mut message = String::new(); + message.push_str("# Convert ckBTC to BTC"); + message.push_str( + "\n\nWithdraw ckBTC previously deposited to your withdrawal account \ + with the ckBTC minter and send the equivalent amount in BTC \ + (minus network and minter fees) to the Bitcoin address below.", + ); + message.push_str(&format!( + "\n\n**Amount to convert:** `{} ckBTC`", + format_amount(args.amount, DECIMALS) + )); + message.push_str(&format!( + "\n\n**Bitcoin destination address:**\n`{}`", + args.address + )); + ConsentMessage::GenericDisplayMessage(message) + } + DisplayMessageType::FieldsDisplay => { + let mut fields = vec![( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: args.amount, + symbol: TOKEN_SYMBOL.to_string(), + }, + )]; + push_chunked_text(&mut fields, "BTC address", &args.address); + ConsentMessage::FieldsDisplayMessage(FieldsDisplay { + intent: "ckBTC to BTC".to_string(), + fields, + }) + } + } +} + +/// Pushes a `(label, Value::Text)` field, splitting `content` into chunks of at +/// most [`FIELDS_DISPLAY_TEXT_CHUNK_LEN`] characters when needed. When chunked, +/// the label gets a "(i/n)" suffix so a hardware-wallet user can tell where +/// each chunk fits in the whole value. +/// +/// Bitcoin addresses and the chunked subaccount hex are pure ASCII, so chunking +/// on byte boundaries is identical to chunking on character boundaries here. +/// We still split on `char_indices` to keep the helper safe for any future +/// caller that passes non-ASCII text. +fn push_chunked_text(fields: &mut Vec<(String, Value)>, label: &str, content: &str) { + let chunks = chunk_text(content, FIELDS_DISPLAY_TEXT_CHUNK_LEN); + if chunks.len() <= 1 { + fields.push(( + label.to_string(), + Value::Text { + content: content.to_string(), + }, + )); + return; + } + let total = chunks.len(); + for (i, chunk) in chunks.into_iter().enumerate() { + fields.push(( + format!("{label} ({}/{total})", i + 1), + Value::Text { content: chunk }, + )); + } +} + +fn chunk_text(content: &str, max_chars: usize) -> Vec { + assert!(max_chars > 0, "max_chars must be greater than zero"); + if content.is_empty() { + return vec![String::new()]; + } + let mut chunks = Vec::new(); + let mut current = String::new(); + let mut count = 0; + for c in content.chars() { + current.push(c); + count += 1; + if count == max_chars { + chunks.push(std::mem::take(&mut current)); + count = 0; + } + } + if !current.is_empty() { + chunks.push(current); + } + chunks +} + +fn format_subaccount(subaccount: &Subaccount) -> String { + hex::encode(subaccount) +} + +fn format_amount(amount: u64, decimals: u8) -> String { + let divisor = 10_u64.pow(decimals as u32); + let whole = amount / divisor; + let frac = amount % divisor; + if frac == 0 { + format!("{whole}") + } else { + let frac_str = format!("{frac:0width$}", width = decimals as usize); + let trimmed = frac_str.trim_end_matches('0'); + format!("{whole}.{trimmed}") + } +} + +#[cfg(test)] +mod tests { + use super::*; + use candid::Encode; + use icrc_ledger_types::icrc21::requests::ConsentMessageSpec; + + fn make_request( + method: &str, + arg: Vec, + device_spec: Option, + ) -> ConsentMessageRequest { + ConsentMessageRequest { + method: method.to_string(), + arg, + user_preferences: ConsentMessageSpec { + metadata: ConsentMessageMetadata { + language: "en".to_string(), + utc_offset_minutes: None, + }, + device_spec, + }, + } + } + + #[test] + fn test_format_amount() { + assert_eq!(format_amount(0, 8), "0"); + assert_eq!(format_amount(1, 8), "0.00000001"); + assert_eq!(format_amount(100_000_000, 8), "1"); + assert_eq!(format_amount(150_000_000, 8), "1.5"); + assert_eq!(format_amount(123_456_789, 8), "1.23456789"); + } + + #[test] + fn test_unsupported_method() { + let req = make_request("update_balance", vec![], None); + let err = icrc21_canister_call_consent_message(req).unwrap_err(); + assert!(matches!(err, Icrc21Error::UnsupportedCanisterCall(_))); + } + + #[test] + fn test_argument_too_large() { + let req = make_request( + "retrieve_btc_with_approval", + vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES + 1], + None, + ); + let err = icrc21_canister_call_consent_message(req).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!(info.description.contains("argument size is too large")); + } + _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), + } + } + + #[test] + fn test_retrieve_btc_with_approval_generic_display() { + let args = RetrieveBtcWithApprovalArgs { + amount: 150_000, + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let info = icrc21_canister_call_consent_message(req).unwrap(); + assert_eq!(info.metadata.language, "en"); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.starts_with("# Convert ckBTC to BTC")); + assert!(message.contains("0.0015 ckBTC")); + assert!(message.contains("bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq")); + // No subaccount section if from_subaccount is None. + assert!(!message.contains("source subaccount")); + } + + #[test] + fn test_retrieve_btc_with_approval_generic_display_with_subaccount() { + let mut subaccount = [0u8; 32]; + subaccount[31] = 0x42; + let args = RetrieveBtcWithApprovalArgs { + amount: 100_000_000, + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), + from_subaccount: Some(subaccount), + }; + let req = make_request("retrieve_btc_with_approval", Encode!(&args).unwrap(), None); + let info = icrc21_canister_call_consent_message(req).unwrap(); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.contains("1 ckBTC")); + assert!(message.contains(&hex::encode(subaccount))); + } + + #[test] + fn test_retrieve_btc_with_approval_fields_display_short_address() { + // A 16-character address fits in a single chunk so the label is not + // suffixed with "(i/n)". + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: "tb1qexampleaddres".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = icrc21_canister_call_consent_message(req).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckBTC to BTC"); + assert_eq!(fields_display.fields.len(), 2); + assert_eq!( + fields_display.fields[0], + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: 250_000, + symbol: TOKEN_SYMBOL.to_string(), + } + ) + ); + assert_eq!( + fields_display.fields[1], + ( + "BTC address".to_string(), + Value::Text { + content: "tb1qexampleaddres".to_string(), + } + ) + ); + } + + #[test] + fn test_retrieve_btc_with_approval_fields_display_long_address_and_subaccount() { + // Bech32 address (42 chars) and subaccount (64 hex chars) both exceed + // FIELDS_DISPLAY_TEXT_CHUNK_LEN and must be paginated. + let mut subaccount = [0_u8; 32]; + subaccount[0] = 0xab; + subaccount[31] = 0xcd; + let address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(); + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: address.clone(), + from_subaccount: Some(subaccount), + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = icrc21_canister_call_consent_message(req).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckBTC to BTC"); + + // Address: 42 chars / 18 = 3 chunks (18 + 18 + 6). + // Subaccount: 64 chars / 18 = 4 chunks (18 + 18 + 18 + 10). + // Plus 1 amount field => 1 + 3 + 4 = 8 fields. + assert_eq!(fields_display.fields.len(), 8); + + assert_eq!(fields_display.fields[0].0, "Amount"); + assert!(matches!( + fields_display.fields[0].1, + Value::TokenAmount { .. } + )); + + // Address chunks. + assert_eq!(fields_display.fields[1].0, "BTC address (1/3)"); + assert_eq!(fields_display.fields[2].0, "BTC address (2/3)"); + assert_eq!(fields_display.fields[3].0, "BTC address (3/3)"); + let chunk_lens: Vec = (1..=3) + .map(|i| match &fields_display.fields[i].1 { + Value::Text { content } => content.chars().count(), + _ => panic!("expected Text"), + }) + .collect(); + assert_eq!(chunk_lens, vec![18, 18, 6]); + // Reassemble and check the original is preserved exactly. + let mut reassembled = String::new(); + for i in 1..=3 { + if let Value::Text { content } = &fields_display.fields[i].1 { + reassembled.push_str(content); + } + } + assert_eq!(reassembled, address); + + // Subaccount chunks. + for (i, expected_label) in [ + "From subaccount (1/4)", + "From subaccount (2/4)", + "From subaccount (3/4)", + "From subaccount (4/4)", + ] + .iter() + .enumerate() + { + assert_eq!(fields_display.fields[4 + i].0, *expected_label); + } + let mut subaccount_reassembled = String::new(); + for i in 4..8 { + if let Value::Text { content } = &fields_display.fields[i].1 { + subaccount_reassembled.push_str(content); + } + } + assert_eq!(subaccount_reassembled, hex::encode(subaccount)); + } + + #[test] + fn test_retrieve_btc_fields_display() { + // Mainnet legacy address — 34 chars => 2 chunks (18 + 16). + let address = "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(); + let args = RetrieveBtcArgs { + amount: 50_000, + address: address.clone(), + }; + let req = make_request( + "retrieve_btc", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = icrc21_canister_call_consent_message(req).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckBTC to BTC"); + assert_eq!(fields_display.fields.len(), 3); + assert_eq!(fields_display.fields[1].0, "BTC address (1/2)"); + assert_eq!(fields_display.fields[2].0, "BTC address (2/2)"); + let mut reassembled = String::new(); + for i in 1..=2 { + if let Value::Text { content } = &fields_display.fields[i].1 { + reassembled.push_str(content); + assert!(content.chars().count() <= FIELDS_DISPLAY_TEXT_CHUNK_LEN); + } + } + assert_eq!(reassembled, address); + } + + #[test] + fn test_chunk_text() { + assert_eq!(chunk_text("", 5), vec![""]); + assert_eq!(chunk_text("abcde", 5), vec!["abcde"]); + assert_eq!(chunk_text("abcdef", 5), vec!["abcde", "f"]); + assert_eq!(chunk_text("abcdefghij", 5), vec!["abcde", "fghij"]); + // Ensure splitting on char boundaries (multi-byte chars). + let multi_byte = "αβγδε"; + let chunks = chunk_text(multi_byte, 2); + assert_eq!(chunks, vec!["αβ", "γδ", "ε"]); + } + + #[test] + fn test_push_chunked_text_no_chunk_when_short() { + let mut fields = Vec::new(); + push_chunked_text(&mut fields, "Label", "short"); + assert_eq!(fields.len(), 1); + assert_eq!(fields[0].0, "Label"); + assert_eq!( + fields[0].1, + Value::Text { + content: "short".to_string() + } + ); + } + + #[test] + fn test_push_chunked_text_chunks_when_long() { + let mut fields = Vec::new(); + // 36 chars => 2 chunks of 18. + let content: String = (0..36).map(|i| (b'a' + (i % 26) as u8) as char).collect(); + push_chunked_text(&mut fields, "Long", &content); + assert_eq!(fields.len(), 2); + assert_eq!(fields[0].0, "Long (1/2)"); + assert_eq!(fields[1].0, "Long (2/2)"); + } + + #[test] + fn test_retrieve_btc_generic_display() { + let args = RetrieveBtcArgs { + amount: 50_000, + address: "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(), + }; + let req = make_request("retrieve_btc", Encode!(&args).unwrap(), None); + let info = icrc21_canister_call_consent_message(req).unwrap(); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.contains("withdrawal account")); + assert!(message.contains("0.0005 ckBTC")); + assert!(message.contains("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa")); + } + + #[test] + fn test_invalid_args() { + let req = make_request("retrieve_btc_with_approval", vec![1, 2, 3], None); + let err = icrc21_canister_call_consent_message(req).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!(info.description.contains("Failed to decode")); + } + _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), + } + } +} From 9a2df0311bb15fd1434807974ea8a4c731e29fe9 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 14:20:28 +0200 Subject: [PATCH 02/16] feat(ckbtc-minter): derive ICRC-21 token symbols from btc_network MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review feedback on PR #10093: 1. Token symbol was hard-coded to "ckBTC" / "BTC" but the minter is also deployed against testnet/regtest where the same code drives the ckTESTBTC / TESTBTC tokens (see dashboard.rs and update_balance.rs). Introduce a TokenSymbols struct that picks the right symbols based on the configured Network and thread it through both the GenericDisplay markdown and FieldsDisplay fields so consent messages stay accurate on test deployments. 2. Fix a stale doc comment that claimed push_chunked_text iterates via `char_indices` — it iterates `chars()` (the comment intent stands: we use Unicode-scalar iteration to keep the helper safe for any non-ASCII caller). Add unit tests for the testnet/regtest symbol path on both display variants. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 189 ++++++++++++++---- 1 file changed, 150 insertions(+), 39 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 451ef972e6d9..92eb3ab7d7ea 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -1,6 +1,8 @@ //! Implementation of the [ICRC-21](https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md) //! Canister Call Consent Message standard for the ckBTC minter. +use crate::Network; +use crate::state::read_state; use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs}; use candid::{CandidType, Decode, Deserialize}; use icrc_ledger_types::icrc1::account::Subaccount; @@ -10,14 +12,35 @@ use icrc_ledger_types::icrc21::requests::{ }; use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDisplay, Value}; -/// The token symbol used in consent messages. The minter burns ckBTC to -/// release the equivalent in BTC. -pub const TOKEN_SYMBOL: &str = "ckBTC"; - /// The number of decimals used to display token amounts. /// Both ckBTC and BTC use 8 decimals (1 BTC = 10^8 satoshis). pub const DECIMALS: u8 = 8; +/// Token symbols used in consent messages. They depend on the configured +/// Bitcoin network so that test deployments use the test-token names. +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +struct TokenSymbols { + /// The ledger token, e.g. "ckBTC" on mainnet, "ckTESTBTC" otherwise. + ckbtc: &'static str, + /// The native Bitcoin token, e.g. "BTC" on mainnet, "TESTBTC" otherwise. + btc: &'static str, +} + +impl TokenSymbols { + fn for_network(network: Network) -> Self { + match network { + Network::Mainnet => Self { + ckbtc: "ckBTC", + btc: "BTC", + }, + Network::Testnet | Network::Regtest => Self { + ckbtc: "ckTESTBTC", + btc: "TESTBTC", + }, + } + } +} + /// Maximum number of bytes that an argument to a ckBTC minter method can have /// when passed through the ICRC-21 endpoint. pub const MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES: usize = 500; @@ -56,6 +79,14 @@ pub fn icrc10_supported_standards() -> Vec { pub fn icrc21_canister_call_consent_message( consent_msg_request: ConsentMessageRequest, +) -> Result { + let network = read_state(|s| s.btc_network); + build_consent_info(consent_msg_request, network) +} + +fn build_consent_info( + consent_msg_request: ConsentMessageRequest, + network: Network, ) -> Result { if consent_msg_request.arg.len() > MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES { return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { @@ -72,6 +103,8 @@ pub fn icrc21_canister_call_consent_message( .clone() .unwrap_or(DisplayMessageType::GenericDisplay); + let symbols = TokenSymbols::for_network(network); + let consent_message = match consent_msg_request.method.as_str() { "retrieve_btc_with_approval" => { let args = Decode!( @@ -83,7 +116,7 @@ pub fn icrc21_canister_call_consent_message( description: format!("Failed to decode RetrieveBtcWithApprovalArgs: {e}"), }) })?; - build_retrieve_btc_with_approval_message(&args, &display_type) + build_retrieve_btc_with_approval_message(&args, &display_type, symbols) } "retrieve_btc" => { let args = @@ -92,7 +125,7 @@ pub fn icrc21_canister_call_consent_message( description: format!("Failed to decode RetrieveBtcArgs: {e}"), }) })?; - build_retrieve_btc_message(&args, &display_type) + build_retrieve_btc_message(&args, &display_type, symbols) } method => { return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { @@ -121,18 +154,20 @@ pub fn icrc21_canister_call_consent_message( fn build_retrieve_btc_with_approval_message( args: &RetrieveBtcWithApprovalArgs, display_type: &DisplayMessageType, + symbols: TokenSymbols, ) -> ConsentMessage { + let TokenSymbols { ckbtc, btc } = symbols; match display_type { DisplayMessageType::GenericDisplay => { let mut message = String::new(); - message.push_str("# Convert ckBTC to BTC"); - message.push_str( - "\n\nAuthorize the ckBTC minter to burn ckBTC from your account and \ - send the equivalent amount in BTC (minus network and minter fees) to \ - the Bitcoin address below.", - ); + message.push_str(&format!("# Convert {ckbtc} to {btc}")); + message.push_str(&format!( + "\n\nAuthorize the {ckbtc} minter to burn {ckbtc} from your account and \ + send the equivalent amount in {btc} (minus network and minter fees) to \ + the Bitcoin address below." + )); message.push_str(&format!( - "\n\n**Amount to convert:** `{} ckBTC`", + "\n\n**Amount to convert:** `{} {ckbtc}`", format_amount(args.amount, DECIMALS) )); message.push_str(&format!( @@ -141,7 +176,7 @@ fn build_retrieve_btc_with_approval_message( )); if let Some(subaccount) = args.from_subaccount { message.push_str(&format!( - "\n\n**ckBTC source subaccount:**\n`{}`", + "\n\n**{ckbtc} source subaccount:**\n`{}`", hex::encode(subaccount) )); } @@ -154,10 +189,10 @@ fn build_retrieve_btc_with_approval_message( Value::TokenAmount { decimals: DECIMALS, amount: args.amount, - symbol: TOKEN_SYMBOL.to_string(), + symbol: ckbtc.to_string(), }, )); - push_chunked_text(&mut fields, "BTC address", &args.address); + push_chunked_text(&mut fields, &format!("{btc} address"), &args.address); if let Some(subaccount) = args.from_subaccount { push_chunked_text( &mut fields, @@ -166,7 +201,7 @@ fn build_retrieve_btc_with_approval_message( ); } ConsentMessage::FieldsDisplayMessage(FieldsDisplay { - intent: "ckBTC to BTC".to_string(), + intent: format!("{ckbtc} to {btc}"), fields, }) } @@ -176,18 +211,20 @@ fn build_retrieve_btc_with_approval_message( fn build_retrieve_btc_message( args: &RetrieveBtcArgs, display_type: &DisplayMessageType, + symbols: TokenSymbols, ) -> ConsentMessage { + let TokenSymbols { ckbtc, btc } = symbols; match display_type { DisplayMessageType::GenericDisplay => { let mut message = String::new(); - message.push_str("# Convert ckBTC to BTC"); - message.push_str( - "\n\nWithdraw ckBTC previously deposited to your withdrawal account \ - with the ckBTC minter and send the equivalent amount in BTC \ - (minus network and minter fees) to the Bitcoin address below.", - ); + message.push_str(&format!("# Convert {ckbtc} to {btc}")); message.push_str(&format!( - "\n\n**Amount to convert:** `{} ckBTC`", + "\n\nWithdraw {ckbtc} previously deposited to your withdrawal account \ + with the {ckbtc} minter and send the equivalent amount in {btc} \ + (minus network and minter fees) to the Bitcoin address below." + )); + message.push_str(&format!( + "\n\n**Amount to convert:** `{} {ckbtc}`", format_amount(args.amount, DECIMALS) )); message.push_str(&format!( @@ -202,12 +239,12 @@ fn build_retrieve_btc_message( Value::TokenAmount { decimals: DECIMALS, amount: args.amount, - symbol: TOKEN_SYMBOL.to_string(), + symbol: ckbtc.to_string(), }, )]; - push_chunked_text(&mut fields, "BTC address", &args.address); + push_chunked_text(&mut fields, &format!("{btc} address"), &args.address); ConsentMessage::FieldsDisplayMessage(FieldsDisplay { - intent: "ckBTC to BTC".to_string(), + intent: format!("{ckbtc} to {btc}"), fields, }) } @@ -221,8 +258,8 @@ fn build_retrieve_btc_message( /// /// Bitcoin addresses and the chunked subaccount hex are pure ASCII, so chunking /// on byte boundaries is identical to chunking on character boundaries here. -/// We still split on `char_indices` to keep the helper safe for any future -/// caller that passes non-ASCII text. +/// We still iterate on Unicode scalar values via [`char`] so the helper stays +/// safe for any future caller that passes non-ASCII text. fn push_chunked_text(fields: &mut Vec<(String, Value)>, label: &str, content: &str) { let chunks = chunk_text(content, FIELDS_DISPLAY_TEXT_CHUNK_LEN); if chunks.len() <= 1 { @@ -318,7 +355,7 @@ mod tests { #[test] fn test_unsupported_method() { let req = make_request("update_balance", vec![], None); - let err = icrc21_canister_call_consent_message(req).unwrap_err(); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); assert!(matches!(err, Icrc21Error::UnsupportedCanisterCall(_))); } @@ -329,7 +366,7 @@ mod tests { vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES + 1], None, ); - let err = icrc21_canister_call_consent_message(req).unwrap_err(); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); match err { Icrc21Error::UnsupportedCanisterCall(info) => { assert!(info.description.contains("argument size is too large")); @@ -350,7 +387,7 @@ mod tests { Encode!(&args).unwrap(), Some(DisplayMessageType::GenericDisplay), ); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); assert_eq!(info.metadata.language, "en"); let message = match info.consent_message { ConsentMessage::GenericDisplayMessage(m) => m, @@ -373,7 +410,7 @@ mod tests { from_subaccount: Some(subaccount), }; let req = make_request("retrieve_btc_with_approval", Encode!(&args).unwrap(), None); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); let message = match info.consent_message { ConsentMessage::GenericDisplayMessage(m) => m, other => panic!("expected GenericDisplayMessage, got {other:?}"), @@ -396,7 +433,7 @@ mod tests { Encode!(&args).unwrap(), Some(DisplayMessageType::FieldsDisplay), ); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); let fields_display = match info.consent_message { ConsentMessage::FieldsDisplayMessage(f) => f, other => panic!("expected FieldsDisplayMessage, got {other:?}"), @@ -410,7 +447,7 @@ mod tests { Value::TokenAmount { decimals: DECIMALS, amount: 250_000, - symbol: TOKEN_SYMBOL.to_string(), + symbol: "ckBTC".to_string(), } ) ); @@ -443,7 +480,7 @@ mod tests { Encode!(&args).unwrap(), Some(DisplayMessageType::FieldsDisplay), ); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); let fields_display = match info.consent_message { ConsentMessage::FieldsDisplayMessage(f) => f, other => panic!("expected FieldsDisplayMessage, got {other:?}"), @@ -515,7 +552,7 @@ mod tests { Encode!(&args).unwrap(), Some(DisplayMessageType::FieldsDisplay), ); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); let fields_display = match info.consent_message { ConsentMessage::FieldsDisplayMessage(f) => f, other => panic!("expected FieldsDisplayMessage, got {other:?}"), @@ -534,6 +571,80 @@ mod tests { assert_eq!(reassembled, address); } + #[test] + fn test_token_symbols_for_network() { + assert_eq!( + TokenSymbols::for_network(Network::Mainnet), + TokenSymbols { + ckbtc: "ckBTC", + btc: "BTC" + } + ); + assert_eq!( + TokenSymbols::for_network(Network::Testnet), + TokenSymbols { + ckbtc: "ckTESTBTC", + btc: "TESTBTC" + } + ); + assert_eq!( + TokenSymbols::for_network(Network::Regtest), + TokenSymbols { + ckbtc: "ckTESTBTC", + btc: "TESTBTC" + } + ); + } + + #[test] + fn test_retrieve_btc_with_approval_uses_testnet_symbols() { + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: "tb1qexampleaddress".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = build_consent_info(req, Network::Testnet).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckTESTBTC to TESTBTC"); + match &fields_display.fields[0].1 { + Value::TokenAmount { symbol, .. } => assert_eq!(symbol, "ckTESTBTC"), + other => panic!("expected TokenAmount, got {other:?}"), + } + // FieldsDisplay address label uses the testnet native symbol too. + assert_eq!(fields_display.fields[1].0, "TESTBTC address"); + } + + #[test] + fn test_retrieve_btc_with_approval_generic_uses_testnet_symbols() { + let args = RetrieveBtcWithApprovalArgs { + amount: 100_000_000, + address: "tb1qexampleaddress".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let info = build_consent_info(req, Network::Regtest).unwrap(); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.starts_with("# Convert ckTESTBTC to TESTBTC")); + assert!(message.contains("1 ckTESTBTC")); + assert!(message.contains("ckTESTBTC minter")); + assert!(message.contains("equivalent amount in TESTBTC")); + } + #[test] fn test_chunk_text() { assert_eq!(chunk_text("", 5), vec![""]); @@ -578,7 +689,7 @@ mod tests { address: "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(), }; let req = make_request("retrieve_btc", Encode!(&args).unwrap(), None); - let info = icrc21_canister_call_consent_message(req).unwrap(); + let info = build_consent_info(req, Network::Mainnet).unwrap(); let message = match info.consent_message { ConsentMessage::GenericDisplayMessage(m) => m, other => panic!("expected GenericDisplayMessage, got {other:?}"), @@ -591,7 +702,7 @@ mod tests { #[test] fn test_invalid_args() { let req = make_request("retrieve_btc_with_approval", vec![1, 2, 3], None); - let err = icrc21_canister_call_consent_message(req).unwrap_err(); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); match err { Icrc21Error::UnsupportedCanisterCall(info) => { assert!(info.description.contains("Failed to decode")); From e2efdac6e85a637b2d291af1a828e442c352cc3a Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 14:57:27 +0200 Subject: [PATCH 03/16] chore(ckbtc-minter): reuse shared MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES Address Copilot review feedback on PR #10093: replace the local 500-byte copy of the ICRC-21 argument-size limit with the shared constant icrc_ledger_types::icrc21::lib::MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES so the two values cannot drift in the future. The shared constant is a u16, cast to usize at the comparison and test sites. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 92eb3ab7d7ea..53b66f94726f 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -7,6 +7,7 @@ use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs} use candid::{CandidType, Decode, Deserialize}; use icrc_ledger_types::icrc1::account::Subaccount; use icrc_ledger_types::icrc21::errors::{ErrorInfo, Icrc21Error}; +use icrc_ledger_types::icrc21::lib::MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES; use icrc_ledger_types::icrc21::requests::{ ConsentMessageMetadata, ConsentMessageRequest, DisplayMessageType, }; @@ -41,10 +42,6 @@ impl TokenSymbols { } } -/// Maximum number of bytes that an argument to a ckBTC minter method can have -/// when passed through the ICRC-21 endpoint. -pub const MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES: usize = 500; - /// Maximum number of characters per `Value::Text` field for the FieldsDisplay /// variant. Hardware wallets like the Ledger Nano S+ have a small display and /// cannot fit a full Bitcoin address (up to 62 characters for bech32m) or a @@ -88,7 +85,7 @@ fn build_consent_info( consent_msg_request: ConsentMessageRequest, network: Network, ) -> Result { - if consent_msg_request.arg.len() > MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES { + if consent_msg_request.arg.len() > MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES as usize { return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { description: format!( "The argument size is too large. The maximum allowed size is \ @@ -363,7 +360,7 @@ mod tests { fn test_argument_too_large() { let req = make_request( "retrieve_btc_with_approval", - vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES + 1], + vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES as usize + 1], None, ); let err = build_consent_info(req, Network::Mainnet).unwrap_err(); From 94fd5f6cc1b6b6171ae267d494f5dd8fd4b4794f Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 15:44:12 +0200 Subject: [PATCH 04/16] Clippy fix --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 53b66f94726f..90abc13c5e9e 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -399,7 +399,7 @@ mod tests { #[test] fn test_retrieve_btc_with_approval_generic_display_with_subaccount() { - let mut subaccount = [0u8; 32]; + let mut subaccount = [0_u8; 32]; subaccount[31] = 0x42; let args = RetrieveBtcWithApprovalArgs { amount: 100_000_000, From 00f26c77a3722dd84e76f9ad43ef00560f3893ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Tackmann?= <54846571+Dfinity-Bjoern@users.noreply.github.com> Date: Tue, 5 May 2026 18:01:15 +0200 Subject: [PATCH 05/16] Update rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mathias Björkqvist --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 90abc13c5e9e..f47f25b92f82 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -15,7 +15,7 @@ use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDi /// The number of decimals used to display token amounts. /// Both ckBTC and BTC use 8 decimals (1 BTC = 10^8 satoshis). -pub const DECIMALS: u8 = 8; +const DECIMALS: u8 = 8; /// Token symbols used in consent messages. They depend on the configured /// Bitcoin network so that test deployments use the test-token names. From 9cc5fb759620129253377980223fa97f64af0aa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Tackmann?= <54846571+Dfinity-Bjoern@users.noreply.github.com> Date: Tue, 5 May 2026 18:01:29 +0200 Subject: [PATCH 06/16] Update rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mathias Björkqvist --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index f47f25b92f82..9b14b0836639 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -52,7 +52,7 @@ impl TokenSymbols { /// Field labels are kept short (e.g. "BTC address", "From subaccount") so that /// even with a "(N/M)" suffix the label still fits on one line at the typical /// hardware-wallet font size. -pub const FIELDS_DISPLAY_TEXT_CHUNK_LEN: usize = 18; +const FIELDS_DISPLAY_TEXT_CHUNK_LEN: usize = 18; /// An entry of the ICRC-10 supported standards list. #[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] From e96631c6dd81034c9bad15a05667906c20a4b25f Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 18:18:45 +0200 Subject: [PATCH 07/16] refactor(ckbtc-minter): drop ICRC-21 FieldsDisplay value pre-chunking Per the ICRC-21 spec, hardware wallets are responsible for paginating long Value::Text fields across device screens. Verified by inspecting the Ledger ICP app, which reads the raw Text content and calls handle_ui_message(text, message, page) to slice it into device-sized pages with the original label preserved across screens. Pre-chunking on the canister side was actively harmful: it inflated field counts (one logical address turned into 3 separate fields the user had to step through with confusing "(1/3)" / "(2/3)" / "(3/3)" labels) and prevented the device from paginating cleanly. Drop the push_chunked_text / chunk_text helpers and the FIELDS_DISPLAY_TEXT_CHUNK_LEN constant, and emit each long value as a single Value::Text. Update the FieldsDisplay tests accordingly. Short labels ("Amount", "BTC address", "From subaccount", intent "ckBTC to BTC") are kept since titles in the Ledger app are truncated rather than paginated. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 326 ++++++------------ 1 file changed, 99 insertions(+), 227 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 9b14b0836639..14b5e53ba8c3 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -42,18 +42,6 @@ impl TokenSymbols { } } -/// Maximum number of characters per `Value::Text` field for the FieldsDisplay -/// variant. Hardware wallets like the Ledger Nano S+ have a small display and -/// cannot fit a full Bitcoin address (up to 62 characters for bech32m) or a -/// 64-character hex subaccount on a single screen. Long values are split into -/// multiple fields with a "(N/M)" pagination suffix in the label so each chunk -/// fits on a single screen with the label on top. -/// -/// Field labels are kept short (e.g. "BTC address", "From subaccount") so that -/// even with a "(N/M)" suffix the label still fits on one line at the typical -/// hardware-wallet font size. -const FIELDS_DISPLAY_TEXT_CHUNK_LEN: usize = 18; - /// An entry of the ICRC-10 supported standards list. #[derive(Clone, Debug, CandidType, Deserialize, Eq, PartialEq)] pub struct StandardRecord { @@ -180,22 +168,34 @@ fn build_retrieve_btc_with_approval_message( ConsentMessage::GenericDisplayMessage(message) } DisplayMessageType::FieldsDisplay => { - let mut fields = Vec::new(); - fields.push(( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: args.amount, - symbol: ckbtc.to_string(), - }, - )); - push_chunked_text(&mut fields, &format!("{btc} address"), &args.address); + // Long values (Bitcoin addresses, subaccount hex) are sent as a + // single `Value::Text` per the ICRC-21 spec — wallets are + // responsible for paginating them across screens. See e.g. the + // Ledger ICP app, which calls `handle_ui_message` to chunk the + // value into device-sized pages. + let mut fields = vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: args.amount, + symbol: ckbtc.to_string(), + }, + ), + ( + format!("{btc} address"), + Value::Text { + content: args.address.clone(), + }, + ), + ]; if let Some(subaccount) = args.from_subaccount { - push_chunked_text( - &mut fields, - "From subaccount", - &format_subaccount(&subaccount), - ); + fields.push(( + "From subaccount".to_string(), + Value::Text { + content: format_subaccount(&subaccount), + }, + )); } ConsentMessage::FieldsDisplayMessage(FieldsDisplay { intent: format!("{ckbtc} to {btc}"), @@ -230,73 +230,26 @@ fn build_retrieve_btc_message( )); ConsentMessage::GenericDisplayMessage(message) } - DisplayMessageType::FieldsDisplay => { - let mut fields = vec![( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: args.amount, - symbol: ckbtc.to_string(), - }, - )]; - push_chunked_text(&mut fields, &format!("{btc} address"), &args.address); - ConsentMessage::FieldsDisplayMessage(FieldsDisplay { - intent: format!("{ckbtc} to {btc}"), - fields, - }) - } - } -} - -/// Pushes a `(label, Value::Text)` field, splitting `content` into chunks of at -/// most [`FIELDS_DISPLAY_TEXT_CHUNK_LEN`] characters when needed. When chunked, -/// the label gets a "(i/n)" suffix so a hardware-wallet user can tell where -/// each chunk fits in the whole value. -/// -/// Bitcoin addresses and the chunked subaccount hex are pure ASCII, so chunking -/// on byte boundaries is identical to chunking on character boundaries here. -/// We still iterate on Unicode scalar values via [`char`] so the helper stays -/// safe for any future caller that passes non-ASCII text. -fn push_chunked_text(fields: &mut Vec<(String, Value)>, label: &str, content: &str) { - let chunks = chunk_text(content, FIELDS_DISPLAY_TEXT_CHUNK_LEN); - if chunks.len() <= 1 { - fields.push(( - label.to_string(), - Value::Text { - content: content.to_string(), - }, - )); - return; - } - let total = chunks.len(); - for (i, chunk) in chunks.into_iter().enumerate() { - fields.push(( - format!("{label} ({}/{total})", i + 1), - Value::Text { content: chunk }, - )); - } -} - -fn chunk_text(content: &str, max_chars: usize) -> Vec { - assert!(max_chars > 0, "max_chars must be greater than zero"); - if content.is_empty() { - return vec![String::new()]; - } - let mut chunks = Vec::new(); - let mut current = String::new(); - let mut count = 0; - for c in content.chars() { - current.push(c); - count += 1; - if count == max_chars { - chunks.push(std::mem::take(&mut current)); - count = 0; - } - } - if !current.is_empty() { - chunks.push(current); + DisplayMessageType::FieldsDisplay => ConsentMessage::FieldsDisplayMessage(FieldsDisplay { + intent: format!("{ckbtc} to {btc}"), + fields: vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: args.amount, + symbol: ckbtc.to_string(), + }, + ), + ( + format!("{btc} address"), + Value::Text { + content: args.address.clone(), + }, + ), + ], + }), } - chunks } fn format_subaccount(subaccount: &Subaccount) -> String { @@ -417,13 +370,18 @@ mod tests { } #[test] - fn test_retrieve_btc_with_approval_fields_display_short_address() { - // A 16-character address fits in a single chunk so the label is not - // suffixed with "(i/n)". + fn test_retrieve_btc_with_approval_fields_display() { + // Long values (Bitcoin address, subaccount hex) are emitted as a + // single Value::Text — wallets paginate them across screens. The + // number of fields is therefore independent of the value length. + let mut subaccount = [0_u8; 32]; + subaccount[0] = 0xab; + subaccount[31] = 0xcd; + let address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(); let args = RetrieveBtcWithApprovalArgs { amount: 250_000, - address: "tb1qexampleaddres".to_string(), - from_subaccount: None, + address: address.clone(), + from_subaccount: Some(subaccount), }; let req = make_request( "retrieve_btc_with_approval", @@ -436,41 +394,34 @@ mod tests { other => panic!("expected FieldsDisplayMessage, got {other:?}"), }; assert_eq!(fields_display.intent, "ckBTC to BTC"); - assert_eq!(fields_display.fields.len(), 2); - assert_eq!( - fields_display.fields[0], - ( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: 250_000, - symbol: "ckBTC".to_string(), - } - ) - ); assert_eq!( - fields_display.fields[1], - ( - "BTC address".to_string(), - Value::Text { - content: "tb1qexampleaddres".to_string(), - } - ) + fields_display.fields, + vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: 250_000, + symbol: "ckBTC".to_string(), + } + ), + ("BTC address".to_string(), Value::Text { content: address }), + ( + "From subaccount".to_string(), + Value::Text { + content: hex::encode(subaccount) + } + ), + ] ); } #[test] - fn test_retrieve_btc_with_approval_fields_display_long_address_and_subaccount() { - // Bech32 address (42 chars) and subaccount (64 hex chars) both exceed - // FIELDS_DISPLAY_TEXT_CHUNK_LEN and must be paginated. - let mut subaccount = [0_u8; 32]; - subaccount[0] = 0xab; - subaccount[31] = 0xcd; - let address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(); + fn test_retrieve_btc_with_approval_fields_display_no_subaccount() { let args = RetrieveBtcWithApprovalArgs { amount: 250_000, - address: address.clone(), - from_subaccount: Some(subaccount), + address: "tb1qexampleaddres".to_string(), + from_subaccount: None, }; let req = make_request( "retrieve_btc_with_approval", @@ -482,63 +433,18 @@ mod tests { ConsentMessage::FieldsDisplayMessage(f) => f, other => panic!("expected FieldsDisplayMessage, got {other:?}"), }; - assert_eq!(fields_display.intent, "ckBTC to BTC"); - - // Address: 42 chars / 18 = 3 chunks (18 + 18 + 6). - // Subaccount: 64 chars / 18 = 4 chunks (18 + 18 + 18 + 10). - // Plus 1 amount field => 1 + 3 + 4 = 8 fields. - assert_eq!(fields_display.fields.len(), 8); - - assert_eq!(fields_display.fields[0].0, "Amount"); - assert!(matches!( - fields_display.fields[0].1, - Value::TokenAmount { .. } - )); - - // Address chunks. - assert_eq!(fields_display.fields[1].0, "BTC address (1/3)"); - assert_eq!(fields_display.fields[2].0, "BTC address (2/3)"); - assert_eq!(fields_display.fields[3].0, "BTC address (3/3)"); - let chunk_lens: Vec = (1..=3) - .map(|i| match &fields_display.fields[i].1 { - Value::Text { content } => content.chars().count(), - _ => panic!("expected Text"), - }) - .collect(); - assert_eq!(chunk_lens, vec![18, 18, 6]); - // Reassemble and check the original is preserved exactly. - let mut reassembled = String::new(); - for i in 1..=3 { - if let Value::Text { content } = &fields_display.fields[i].1 { - reassembled.push_str(content); - } - } - assert_eq!(reassembled, address); - - // Subaccount chunks. - for (i, expected_label) in [ - "From subaccount (1/4)", - "From subaccount (2/4)", - "From subaccount (3/4)", - "From subaccount (4/4)", - ] - .iter() - .enumerate() - { - assert_eq!(fields_display.fields[4 + i].0, *expected_label); - } - let mut subaccount_reassembled = String::new(); - for i in 4..8 { - if let Value::Text { content } = &fields_display.fields[i].1 { - subaccount_reassembled.push_str(content); - } - } - assert_eq!(subaccount_reassembled, hex::encode(subaccount)); + assert_eq!(fields_display.fields.len(), 2); + // No subaccount field when from_subaccount is None. + assert!( + !fields_display + .fields + .iter() + .any(|(label, _)| label == "From subaccount") + ); } #[test] fn test_retrieve_btc_fields_display() { - // Mainnet legacy address — 34 chars => 2 chunks (18 + 16). let address = "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(); let args = RetrieveBtcArgs { amount: 50_000, @@ -555,17 +461,20 @@ mod tests { other => panic!("expected FieldsDisplayMessage, got {other:?}"), }; assert_eq!(fields_display.intent, "ckBTC to BTC"); - assert_eq!(fields_display.fields.len(), 3); - assert_eq!(fields_display.fields[1].0, "BTC address (1/2)"); - assert_eq!(fields_display.fields[2].0, "BTC address (2/2)"); - let mut reassembled = String::new(); - for i in 1..=2 { - if let Value::Text { content } = &fields_display.fields[i].1 { - reassembled.push_str(content); - assert!(content.chars().count() <= FIELDS_DISPLAY_TEXT_CHUNK_LEN); - } - } - assert_eq!(reassembled, address); + assert_eq!( + fields_display.fields, + vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: 50_000, + symbol: "ckBTC".to_string(), + } + ), + ("BTC address".to_string(), Value::Text { content: address }), + ] + ); } #[test] @@ -642,43 +551,6 @@ mod tests { assert!(message.contains("equivalent amount in TESTBTC")); } - #[test] - fn test_chunk_text() { - assert_eq!(chunk_text("", 5), vec![""]); - assert_eq!(chunk_text("abcde", 5), vec!["abcde"]); - assert_eq!(chunk_text("abcdef", 5), vec!["abcde", "f"]); - assert_eq!(chunk_text("abcdefghij", 5), vec!["abcde", "fghij"]); - // Ensure splitting on char boundaries (multi-byte chars). - let multi_byte = "αβγδε"; - let chunks = chunk_text(multi_byte, 2); - assert_eq!(chunks, vec!["αβ", "γδ", "ε"]); - } - - #[test] - fn test_push_chunked_text_no_chunk_when_short() { - let mut fields = Vec::new(); - push_chunked_text(&mut fields, "Label", "short"); - assert_eq!(fields.len(), 1); - assert_eq!(fields[0].0, "Label"); - assert_eq!( - fields[0].1, - Value::Text { - content: "short".to_string() - } - ); - } - - #[test] - fn test_push_chunked_text_chunks_when_long() { - let mut fields = Vec::new(); - // 36 chars => 2 chunks of 18. - let content: String = (0..36).map(|i| (b'a' + (i % 26) as u8) as char).collect(); - push_chunked_text(&mut fields, "Long", &content); - assert_eq!(fields.len(), 2); - assert_eq!(fields[0].0, "Long (1/2)"); - assert_eq!(fields[1].0, "Long (2/2)"); - } - #[test] fn test_retrieve_btc_generic_display() { let args = RetrieveBtcArgs { From 6285499bb738e286964254408a3535e7b6e911eb Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 18:25:47 +0200 Subject: [PATCH 08/16] feat(ckbtc-minter): validate destination address in ICRC-21 endpoint Address Mathias's review feedback on PR #10093 (Markdown injection nit): the destination address from RetrieveBtc(WithApproval)Args was being interpolated directly into the GenericDisplay Markdown. A crafted value containing newlines, backticks, or '#' could fake additional fields in the consent message (e.g. an "address" of "bc1q...\n# You will receive 100 BTC") and confuse the user. The malicious request would later fail when retrieve_btc[_with_approval] is invoked because the actual call also parses the address, but a bad consent message is itself the issue. Add a validate_address step that parses the address with the same BitcoinAddress::parse used by retrieve_btc[_with_approval]. If parsing fails, return Icrc21Error::UnsupportedCanisterCall before constructing any consent message. This both eliminates the injection vector and guarantees the address shown to the user is actually parseable, so the consent endpoint and the actual call agree on what's accepted. Replace fake test addresses ("tb1qexampleaddress" etc.) with valid network-matching ones, and add test_malformed_address_is_rejected with a few crafted Markdown-injection payloads as well as an address from the wrong network. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 64 +++++++++++++++++-- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 14b5e53ba8c3..8f650e13ca08 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -2,6 +2,7 @@ //! Canister Call Consent Message standard for the ckBTC minter. use crate::Network; +use crate::address::BitcoinAddress; use crate::state::read_state; use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs}; use candid::{CandidType, Decode, Deserialize}; @@ -101,6 +102,7 @@ fn build_consent_info( description: format!("Failed to decode RetrieveBtcWithApprovalArgs: {e}"), }) })?; + validate_address(&args.address, network)?; build_retrieve_btc_with_approval_message(&args, &display_type, symbols) } "retrieve_btc" => { @@ -110,6 +112,7 @@ fn build_consent_info( description: format!("Failed to decode RetrieveBtcArgs: {e}"), }) })?; + validate_address(&args.address, network)?; build_retrieve_btc_message(&args, &display_type, symbols) } method => { @@ -256,6 +259,22 @@ fn format_subaccount(subaccount: &Subaccount) -> String { hex::encode(subaccount) } +/// Verifies that `address` parses as a valid Bitcoin address on the configured +/// network before it gets interpolated into a consent message. This both +/// guarantees the user is shown a meaningful (parseable) destination and rules +/// out Markdown-injection vectors in the GenericDisplay output (e.g. an +/// "address" that contains newlines or backticks crafted to fake additional +/// fields). Uses the same parser as `retrieve_btc[_with_approval]`, so any +/// address the consent endpoint accepts is also accepted by the actual call. +fn validate_address(address: &str, network: Network) -> Result<(), Icrc21Error> { + BitcoinAddress::parse(address, network).map_err(|e| { + Icrc21Error::UnsupportedCanisterCall(ErrorInfo { + description: format!("Invalid Bitcoin destination address: {e}"), + }) + })?; + Ok(()) +} + fn format_amount(amount: u64, decimals: u8) -> String { let divisor = 10_u64.pow(decimals as u32); let whole = amount / divisor; @@ -420,7 +439,7 @@ mod tests { fn test_retrieve_btc_with_approval_fields_display_no_subaccount() { let args = RetrieveBtcWithApprovalArgs { amount: 250_000, - address: "tb1qexampleaddres".to_string(), + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), from_subaccount: None, }; let req = make_request( @@ -506,7 +525,7 @@ mod tests { fn test_retrieve_btc_with_approval_uses_testnet_symbols() { let args = RetrieveBtcWithApprovalArgs { amount: 250_000, - address: "tb1qexampleaddress".to_string(), + address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), from_subaccount: None, }; let req = make_request( @@ -532,7 +551,7 @@ mod tests { fn test_retrieve_btc_with_approval_generic_uses_testnet_symbols() { let args = RetrieveBtcWithApprovalArgs { amount: 100_000_000, - address: "tb1qexampleaddress".to_string(), + address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), from_subaccount: None, }; let req = make_request( @@ -540,7 +559,7 @@ mod tests { Encode!(&args).unwrap(), Some(DisplayMessageType::GenericDisplay), ); - let info = build_consent_info(req, Network::Regtest).unwrap(); + let info = build_consent_info(req, Network::Testnet).unwrap(); let message = match info.consent_message { ConsentMessage::GenericDisplayMessage(m) => m, other => panic!("expected GenericDisplayMessage, got {other:?}"), @@ -579,4 +598,41 @@ mod tests { _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), } } + + #[test] + fn test_malformed_address_is_rejected() { + // The minter must not interpolate an unparseable address into the + // Markdown consent message — that would be a Markdown-injection vector + // (e.g. an "address" containing newlines, backticks, or '#' that fakes + // additional fields). + for bad_address in [ + "not-a-real-address", + "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq\n# You will receive 100 BTC", + "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa`\n\n**Amount:** 100 BTC\n`", + "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx", // valid testnet but on Mainnet + ] { + let args = RetrieveBtcWithApprovalArgs { + amount: 50_000, + address: bad_address.to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!( + info.description + .contains("Invalid Bitcoin destination address"), + "unexpected error description: {}", + info.description + ); + } + other => panic!("expected UnsupportedCanisterCall, got {other:?}"), + } + } + } } From 36684ce1b9e0b256c83fc407f67b46bf04672167 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 18:33:12 +0200 Subject: [PATCH 09/16] refactor(ckbtc-minter): tighten ICRC-21 GenericDisplay construction Two leftover bits from the FieldsDisplay chunking add/remove cycle that no longer earn their keep: - format_subaccount was a one-line wrapper around hex::encode used in a single place. Inline it and drop the now-unused Subaccount import. - The GenericDisplay markdown was being built up with five separate push_str(&format!(...)) calls per builder, each allocating a throwaway String. Collapse into a single format! per builder; the optional subaccount block on the with-approval variant stays as a conditional push_str. Same output, fewer allocations, easier to read the whole template at a glance. No behavior change; all 14 unit tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 58 +++++++------------ 1 file changed, 20 insertions(+), 38 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 8f650e13ca08..21005522c93d 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -6,7 +6,6 @@ use crate::address::BitcoinAddress; use crate::state::read_state; use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs}; use candid::{CandidType, Decode, Deserialize}; -use icrc_ledger_types::icrc1::account::Subaccount; use icrc_ledger_types::icrc21::errors::{ErrorInfo, Icrc21Error}; use icrc_ledger_types::icrc21::lib::MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES; use icrc_ledger_types::icrc21::requests::{ @@ -145,23 +144,18 @@ fn build_retrieve_btc_with_approval_message( symbols: TokenSymbols, ) -> ConsentMessage { let TokenSymbols { ckbtc, btc } = symbols; + let amount = format_amount(args.amount, DECIMALS); match display_type { DisplayMessageType::GenericDisplay => { - let mut message = String::new(); - message.push_str(&format!("# Convert {ckbtc} to {btc}")); - message.push_str(&format!( - "\n\nAuthorize the {ckbtc} minter to burn {ckbtc} from your account and \ + let mut message = format!( + "# Convert {ckbtc} to {btc}\n\n\ + Authorize the {ckbtc} minter to burn {ckbtc} from your account and \ send the equivalent amount in {btc} (minus network and minter fees) to \ - the Bitcoin address below." - )); - message.push_str(&format!( - "\n\n**Amount to convert:** `{} {ckbtc}`", - format_amount(args.amount, DECIMALS) - )); - message.push_str(&format!( - "\n\n**Bitcoin destination address:**\n`{}`", - args.address - )); + the Bitcoin address below.\n\n\ + **Amount to convert:** `{amount} {ckbtc}`\n\n\ + **Bitcoin destination address:**\n`{address}`", + address = args.address, + ); if let Some(subaccount) = args.from_subaccount { message.push_str(&format!( "\n\n**{ckbtc} source subaccount:**\n`{}`", @@ -196,7 +190,7 @@ fn build_retrieve_btc_with_approval_message( fields.push(( "From subaccount".to_string(), Value::Text { - content: format_subaccount(&subaccount), + content: hex::encode(subaccount), }, )); } @@ -214,25 +208,17 @@ fn build_retrieve_btc_message( symbols: TokenSymbols, ) -> ConsentMessage { let TokenSymbols { ckbtc, btc } = symbols; + let amount = format_amount(args.amount, DECIMALS); match display_type { - DisplayMessageType::GenericDisplay => { - let mut message = String::new(); - message.push_str(&format!("# Convert {ckbtc} to {btc}")); - message.push_str(&format!( - "\n\nWithdraw {ckbtc} previously deposited to your withdrawal account \ - with the {ckbtc} minter and send the equivalent amount in {btc} \ - (minus network and minter fees) to the Bitcoin address below." - )); - message.push_str(&format!( - "\n\n**Amount to convert:** `{} {ckbtc}`", - format_amount(args.amount, DECIMALS) - )); - message.push_str(&format!( - "\n\n**Bitcoin destination address:**\n`{}`", - args.address - )); - ConsentMessage::GenericDisplayMessage(message) - } + DisplayMessageType::GenericDisplay => ConsentMessage::GenericDisplayMessage(format!( + "# Convert {ckbtc} to {btc}\n\n\ + Withdraw {ckbtc} previously deposited to your withdrawal account \ + with the {ckbtc} minter and send the equivalent amount in {btc} \ + (minus network and minter fees) to the Bitcoin address below.\n\n\ + **Amount to convert:** `{amount} {ckbtc}`\n\n\ + **Bitcoin destination address:**\n`{address}`", + address = args.address, + )), DisplayMessageType::FieldsDisplay => ConsentMessage::FieldsDisplayMessage(FieldsDisplay { intent: format!("{ckbtc} to {btc}"), fields: vec![ @@ -255,10 +241,6 @@ fn build_retrieve_btc_message( } } -fn format_subaccount(subaccount: &Subaccount) -> String { - hex::encode(subaccount) -} - /// Verifies that `address` parses as a valid Bitcoin address on the configured /// network before it gets interpolated into a consent message. This both /// guarantees the user is shown a meaningful (parseable) destination and rules From f96bd35130454d2de9fdff6b9430ae1e5a2a7e17 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 23:15:31 +0200 Subject: [PATCH 10/16] chore(ckbtc-minter): allow canbench update target on Apple Silicon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mainnet_events.mem.gz genrule that ckbtc_minter_canbench depends on declared its macOS-only constraint as @platforms//cpu:arm, which is the 32-bit ARM (aarch32) constraint — Apple Silicon machines are aarch64 and therefore failed the constraint check, making the _update target incompatible on every Mac. Switch to @platforms//cpu:aarch64 so the target is buildable on Apple Silicon while keeping macOS x86_64 excluded as the surrounding comment intended. Also regenerate canbench/results.yml with the updated instruction counts produced by the new minter Wasm (the ICRC-21 endpoints introduced in this branch shift the layout enough to trip the canbench_test noise threshold). Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/BUILD.bazel | 4 +- rs/bitcoin/ckbtc/minter/canbench/results.yml | 52 ++++++++++---------- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/BUILD.bazel b/rs/bitcoin/ckbtc/minter/BUILD.bazel index 9592d7d5822f..3789641a8c40 100644 --- a/rs/bitcoin/ckbtc/minter/BUILD.bazel +++ b/rs/bitcoin/ckbtc/minter/BUILD.bazel @@ -277,8 +277,10 @@ genrule( $(location :ckbtc_minter_dump_stable_memory) $(location @ckbtc_mainnet_events.gz//file) $@ """, # For some unknown reason this target can take well over an hour to run on MacOS x86_64 so we exclude that platform: + # Note: `@platforms//cpu:arm` is 32-bit ARM. Apple Silicon is `aarch64`, + # so use that here — otherwise this target is incompatible on every Mac. target_compatible_with = select({ - "@platforms//os:osx": ["@platforms//cpu:arm"], + "@platforms//os:osx": ["@platforms//cpu:aarch64"], "//conditions:default": [], }), visibility = ["//visibility:private"], diff --git a/rs/bitcoin/ckbtc/minter/canbench/results.yml b/rs/bitcoin/ckbtc/minter/canbench/results.yml index 90b219fadb6e..52c70b9dad51 100644 --- a/rs/bitcoin/ckbtc/minter/canbench/results.yml +++ b/rs/bitcoin/ckbtc/minter/canbench/results.yml @@ -2,13 +2,13 @@ benches: build_estimate_retrieve_btc_fee_1_50k_sats: total: calls: 1 - instructions: 419639 + instructions: 419559 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -18,24 +18,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413500 + instructions: 413498 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_1_50k_sats: total: calls: 1 - instructions: 421753 + instructions: 421675 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419172 + instructions: 419092 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -45,24 +45,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413500 + instructions: 413498 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_2_100k_sats: total: calls: 1 - instructions: 421799 + instructions: 421721 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419218 + instructions: 419138 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -72,24 +72,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413546 + instructions: 413544 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_3_1m_sats: total: calls: 1 - instructions: 421702 + instructions: 421624 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419121 + instructions: 419041 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -99,24 +99,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413449 + instructions: 413447 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_4_10m_sats: total: calls: 1 - instructions: 422877 + instructions: 422799 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 420296 + instructions: 420216 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -126,24 +126,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 414624 + instructions: 414622 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_5_1_btc: total: calls: 1 - instructions: 422877 + instructions: 422799 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 420296 + instructions: 420216 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3441 + instructions: 3363 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -153,24 +153,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 414624 + instructions: 414622 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_6_10_btc: total: calls: 1 - instructions: 429549 + instructions: 429473 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 426968 + instructions: 426890 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3908 + instructions: 3830 heap_increase: 0 stable_memory_increase: 0 greedy: From b0195c309e48099e414e7459fc91134f21f3b684 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Tue, 5 May 2026 23:15:42 +0200 Subject: [PATCH 11/16] chore(ckbtc-minter): point ICRC-21 standard URL at the dfinity/ICRC repo The ICRC-21 spec is being moved from the wg-identity-authentication repo into the canonical dfinity/ICRC repo (one ICRC per directory under ICRCs/). Update the URL the minter advertises via icrc10_supported_standards, plus the surrounding doc comments in the .did file and the icrc21 Rust module, to point at https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md. This change goes in only after the corresponding ICRC-repo PR is merged and the new path resolves, so wallets that follow the URL always reach a live spec. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/ckbtc_minter.did | 2 +- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/ckbtc_minter.did b/rs/bitcoin/ckbtc/minter/ckbtc_minter.did index e1cb4afd6b4d..5643f401ce37 100644 --- a/rs/bitcoin/ckbtc/minter/ckbtc_minter.did +++ b/rs/bitcoin/ckbtc/minter/ckbtc_minter.did @@ -799,7 +799,7 @@ service : (minter_arg : MinterArg) -> { // Returns a human-readable consent message describing the requested // canister call. See the ICRC-21 standard for details: - // https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md + // https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md icrc21_canister_call_consent_message : (Icrc21ConsentMessageRequest) -> (variant { Ok : Icrc21ConsentInfo; Err : Icrc21Error }); // }}} Section "ICRC-10 / ICRC-21" diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index 21005522c93d..b4132d04eb82 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -1,4 +1,4 @@ -//! Implementation of the [ICRC-21](https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md) +//! Implementation of the [ICRC-21](https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md) //! Canister Call Consent Message standard for the ckBTC minter. use crate::Network; @@ -57,7 +57,7 @@ pub fn icrc10_supported_standards() -> Vec { }, StandardRecord { name: "ICRC-21".to_string(), - url: "https://github.com/dfinity/wg-identity-authentication/blob/main/topics/ICRC-21/icrc_21_consent_msg.md".to_string(), + url: "https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md".to_string(), }, ] } From 75b279dcd91db7c5397ef1e079276ff3be282040 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Wed, 6 May 2026 09:20:27 +0200 Subject: [PATCH 12/16] feat(ckbtc-minter): scope ICRC-21 to retrieve_btc_with_approval only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The non-approval `retrieve_btc` flow requires the user to first deposit ckBTC into a minter-derived withdrawal subaccount, which wallets cannot discover or sign for via standard ICRC-21 — the wallet would render a consent message for a call that, on its own, never moves the user's tokens. Drop ICRC-21 support for that method so the minter only emits a consent message for `retrieve_btc_with_approval`, which is the flow ICRC-21-aware wallets actually drive. Also tighten test_unsupported_method to assert that retrieve_btc (and a few other unrelated methods) explicitly return UnsupportedCanisterCall so future re-additions to the supported set are deliberate. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 118 ++---------------- 1 file changed, 13 insertions(+), 105 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index b4132d04eb82..bb99774e3214 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -4,7 +4,7 @@ use crate::Network; use crate::address::BitcoinAddress; use crate::state::read_state; -use crate::updates::retrieve_btc::{RetrieveBtcArgs, RetrieveBtcWithApprovalArgs}; +use crate::updates::retrieve_btc::RetrieveBtcWithApprovalArgs; use candid::{CandidType, Decode, Deserialize}; use icrc_ledger_types::icrc21::errors::{ErrorInfo, Icrc21Error}; use icrc_ledger_types::icrc21::lib::MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES; @@ -104,16 +104,6 @@ fn build_consent_info( validate_address(&args.address, network)?; build_retrieve_btc_with_approval_message(&args, &display_type, symbols) } - "retrieve_btc" => { - let args = - Decode!(consent_msg_request.arg.as_slice(), RetrieveBtcArgs).map_err(|e| { - Icrc21Error::UnsupportedCanisterCall(ErrorInfo { - description: format!("Failed to decode RetrieveBtcArgs: {e}"), - }) - })?; - validate_address(&args.address, network)?; - build_retrieve_btc_message(&args, &display_type, symbols) - } method => { return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { description: format!( @@ -202,51 +192,12 @@ fn build_retrieve_btc_with_approval_message( } } -fn build_retrieve_btc_message( - args: &RetrieveBtcArgs, - display_type: &DisplayMessageType, - symbols: TokenSymbols, -) -> ConsentMessage { - let TokenSymbols { ckbtc, btc } = symbols; - let amount = format_amount(args.amount, DECIMALS); - match display_type { - DisplayMessageType::GenericDisplay => ConsentMessage::GenericDisplayMessage(format!( - "# Convert {ckbtc} to {btc}\n\n\ - Withdraw {ckbtc} previously deposited to your withdrawal account \ - with the {ckbtc} minter and send the equivalent amount in {btc} \ - (minus network and minter fees) to the Bitcoin address below.\n\n\ - **Amount to convert:** `{amount} {ckbtc}`\n\n\ - **Bitcoin destination address:**\n`{address}`", - address = args.address, - )), - DisplayMessageType::FieldsDisplay => ConsentMessage::FieldsDisplayMessage(FieldsDisplay { - intent: format!("{ckbtc} to {btc}"), - fields: vec![ - ( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: args.amount, - symbol: ckbtc.to_string(), - }, - ), - ( - format!("{btc} address"), - Value::Text { - content: args.address.clone(), - }, - ), - ], - }), - } -} - /// Verifies that `address` parses as a valid Bitcoin address on the configured /// network before it gets interpolated into a consent message. This both /// guarantees the user is shown a meaningful (parseable) destination and rules /// out Markdown-injection vectors in the GenericDisplay output (e.g. an /// "address" that contains newlines or backticks crafted to fake additional -/// fields). Uses the same parser as `retrieve_btc[_with_approval]`, so any +/// fields). Uses the same parser as `retrieve_btc_with_approval`, so any /// address the consent endpoint accepts is also accepted by the actual call. fn validate_address(address: &str, network: Network) -> Result<(), Icrc21Error> { BitcoinAddress::parse(address, network).map_err(|e| { @@ -305,9 +256,17 @@ mod tests { #[test] fn test_unsupported_method() { - let req = make_request("update_balance", vec![], None); - let err = build_consent_info(req, Network::Mainnet).unwrap_err(); - assert!(matches!(err, Icrc21Error::UnsupportedCanisterCall(_))); + // Includes `retrieve_btc` because the minter intentionally only + // supports ICRC-21 for the approval-based flow — wallets calling + // `retrieve_btc` should not get a consent message rendered for them. + for method in ["update_balance", "retrieve_btc", "get_btc_address", ""] { + let req = make_request(method, vec![], None); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + assert!( + matches!(err, Icrc21Error::UnsupportedCanisterCall(_)), + "method {method:?} should be unsupported, got {err:?}" + ); + } } #[test] @@ -444,40 +403,6 @@ mod tests { ); } - #[test] - fn test_retrieve_btc_fields_display() { - let address = "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(); - let args = RetrieveBtcArgs { - amount: 50_000, - address: address.clone(), - }; - let req = make_request( - "retrieve_btc", - Encode!(&args).unwrap(), - Some(DisplayMessageType::FieldsDisplay), - ); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - let fields_display = match info.consent_message { - ConsentMessage::FieldsDisplayMessage(f) => f, - other => panic!("expected FieldsDisplayMessage, got {other:?}"), - }; - assert_eq!(fields_display.intent, "ckBTC to BTC"); - assert_eq!( - fields_display.fields, - vec![ - ( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: 50_000, - symbol: "ckBTC".to_string(), - } - ), - ("BTC address".to_string(), Value::Text { content: address }), - ] - ); - } - #[test] fn test_token_symbols_for_network() { assert_eq!( @@ -552,23 +477,6 @@ mod tests { assert!(message.contains("equivalent amount in TESTBTC")); } - #[test] - fn test_retrieve_btc_generic_display() { - let args = RetrieveBtcArgs { - amount: 50_000, - address: "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa".to_string(), - }; - let req = make_request("retrieve_btc", Encode!(&args).unwrap(), None); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - let message = match info.consent_message { - ConsentMessage::GenericDisplayMessage(m) => m, - other => panic!("expected GenericDisplayMessage, got {other:?}"), - }; - assert!(message.contains("withdrawal account")); - assert!(message.contains("0.0005 ckBTC")); - assert!(message.contains("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa")); - } - #[test] fn test_invalid_args() { let req = make_request("retrieve_btc_with_approval", vec![1, 2, 3], None); From 172b5c72993ba3b00e34f8d2ac4b6e9a72b53f13 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Wed, 6 May 2026 09:38:44 +0200 Subject: [PATCH 13/16] refactor(ckbtc-minter): move ICRC-21 tests into updates/tests.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Group all updates-module tests in the same file (updates/tests.rs) the way update_balance is already organized: keep production code in updates/icrc21.rs and put the tests in a sibling `mod icrc21 {...}` block in updates/tests.rs. Bumps the visibility of the items the tests reach into — DECIMALS, TokenSymbols, TokenSymbols::for_network, format_amount, and build_consent_info — to pub(super), which is the minimum needed for the sibling `crate::updates::tests::icrc21` module to use them while keeping them out of the crate-public API. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs | 320 +----------------- rs/bitcoin/ckbtc/minter/src/updates/tests.rs | 312 +++++++++++++++++ 2 files changed, 319 insertions(+), 313 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs index bb99774e3214..5a594ed426fd 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/icrc21.rs @@ -15,20 +15,20 @@ use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDi /// The number of decimals used to display token amounts. /// Both ckBTC and BTC use 8 decimals (1 BTC = 10^8 satoshis). -const DECIMALS: u8 = 8; +pub(super) const DECIMALS: u8 = 8; /// Token symbols used in consent messages. They depend on the configured /// Bitcoin network so that test deployments use the test-token names. #[derive(Copy, Clone, Debug, Eq, PartialEq)] -struct TokenSymbols { +pub(super) struct TokenSymbols { /// The ledger token, e.g. "ckBTC" on mainnet, "ckTESTBTC" otherwise. - ckbtc: &'static str, + pub(super) ckbtc: &'static str, /// The native Bitcoin token, e.g. "BTC" on mainnet, "TESTBTC" otherwise. - btc: &'static str, + pub(super) btc: &'static str, } impl TokenSymbols { - fn for_network(network: Network) -> Self { + pub(super) fn for_network(network: Network) -> Self { match network { Network::Mainnet => Self { ckbtc: "ckBTC", @@ -69,7 +69,7 @@ pub fn icrc21_canister_call_consent_message( build_consent_info(consent_msg_request, network) } -fn build_consent_info( +pub(super) fn build_consent_info( consent_msg_request: ConsentMessageRequest, network: Network, ) -> Result { @@ -208,7 +208,7 @@ fn validate_address(address: &str, network: Network) -> Result<(), Icrc21Error> Ok(()) } -fn format_amount(amount: u64, decimals: u8) -> String { +pub(super) fn format_amount(amount: u64, decimals: u8) -> String { let divisor = 10_u64.pow(decimals as u32); let whole = amount / divisor; let frac = amount % divisor; @@ -220,309 +220,3 @@ fn format_amount(amount: u64, decimals: u8) -> String { format!("{whole}.{trimmed}") } } - -#[cfg(test)] -mod tests { - use super::*; - use candid::Encode; - use icrc_ledger_types::icrc21::requests::ConsentMessageSpec; - - fn make_request( - method: &str, - arg: Vec, - device_spec: Option, - ) -> ConsentMessageRequest { - ConsentMessageRequest { - method: method.to_string(), - arg, - user_preferences: ConsentMessageSpec { - metadata: ConsentMessageMetadata { - language: "en".to_string(), - utc_offset_minutes: None, - }, - device_spec, - }, - } - } - - #[test] - fn test_format_amount() { - assert_eq!(format_amount(0, 8), "0"); - assert_eq!(format_amount(1, 8), "0.00000001"); - assert_eq!(format_amount(100_000_000, 8), "1"); - assert_eq!(format_amount(150_000_000, 8), "1.5"); - assert_eq!(format_amount(123_456_789, 8), "1.23456789"); - } - - #[test] - fn test_unsupported_method() { - // Includes `retrieve_btc` because the minter intentionally only - // supports ICRC-21 for the approval-based flow — wallets calling - // `retrieve_btc` should not get a consent message rendered for them. - for method in ["update_balance", "retrieve_btc", "get_btc_address", ""] { - let req = make_request(method, vec![], None); - let err = build_consent_info(req, Network::Mainnet).unwrap_err(); - assert!( - matches!(err, Icrc21Error::UnsupportedCanisterCall(_)), - "method {method:?} should be unsupported, got {err:?}" - ); - } - } - - #[test] - fn test_argument_too_large() { - let req = make_request( - "retrieve_btc_with_approval", - vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES as usize + 1], - None, - ); - let err = build_consent_info(req, Network::Mainnet).unwrap_err(); - match err { - Icrc21Error::UnsupportedCanisterCall(info) => { - assert!(info.description.contains("argument size is too large")); - } - _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), - } - } - - #[test] - fn test_retrieve_btc_with_approval_generic_display() { - let args = RetrieveBtcWithApprovalArgs { - amount: 150_000, - address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), - from_subaccount: None, - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::GenericDisplay), - ); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - assert_eq!(info.metadata.language, "en"); - let message = match info.consent_message { - ConsentMessage::GenericDisplayMessage(m) => m, - other => panic!("expected GenericDisplayMessage, got {other:?}"), - }; - assert!(message.starts_with("# Convert ckBTC to BTC")); - assert!(message.contains("0.0015 ckBTC")); - assert!(message.contains("bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq")); - // No subaccount section if from_subaccount is None. - assert!(!message.contains("source subaccount")); - } - - #[test] - fn test_retrieve_btc_with_approval_generic_display_with_subaccount() { - let mut subaccount = [0_u8; 32]; - subaccount[31] = 0x42; - let args = RetrieveBtcWithApprovalArgs { - amount: 100_000_000, - address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), - from_subaccount: Some(subaccount), - }; - let req = make_request("retrieve_btc_with_approval", Encode!(&args).unwrap(), None); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - let message = match info.consent_message { - ConsentMessage::GenericDisplayMessage(m) => m, - other => panic!("expected GenericDisplayMessage, got {other:?}"), - }; - assert!(message.contains("1 ckBTC")); - assert!(message.contains(&hex::encode(subaccount))); - } - - #[test] - fn test_retrieve_btc_with_approval_fields_display() { - // Long values (Bitcoin address, subaccount hex) are emitted as a - // single Value::Text — wallets paginate them across screens. The - // number of fields is therefore independent of the value length. - let mut subaccount = [0_u8; 32]; - subaccount[0] = 0xab; - subaccount[31] = 0xcd; - let address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(); - let args = RetrieveBtcWithApprovalArgs { - amount: 250_000, - address: address.clone(), - from_subaccount: Some(subaccount), - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::FieldsDisplay), - ); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - let fields_display = match info.consent_message { - ConsentMessage::FieldsDisplayMessage(f) => f, - other => panic!("expected FieldsDisplayMessage, got {other:?}"), - }; - assert_eq!(fields_display.intent, "ckBTC to BTC"); - assert_eq!( - fields_display.fields, - vec![ - ( - "Amount".to_string(), - Value::TokenAmount { - decimals: DECIMALS, - amount: 250_000, - symbol: "ckBTC".to_string(), - } - ), - ("BTC address".to_string(), Value::Text { content: address }), - ( - "From subaccount".to_string(), - Value::Text { - content: hex::encode(subaccount) - } - ), - ] - ); - } - - #[test] - fn test_retrieve_btc_with_approval_fields_display_no_subaccount() { - let args = RetrieveBtcWithApprovalArgs { - amount: 250_000, - address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), - from_subaccount: None, - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::FieldsDisplay), - ); - let info = build_consent_info(req, Network::Mainnet).unwrap(); - let fields_display = match info.consent_message { - ConsentMessage::FieldsDisplayMessage(f) => f, - other => panic!("expected FieldsDisplayMessage, got {other:?}"), - }; - assert_eq!(fields_display.fields.len(), 2); - // No subaccount field when from_subaccount is None. - assert!( - !fields_display - .fields - .iter() - .any(|(label, _)| label == "From subaccount") - ); - } - - #[test] - fn test_token_symbols_for_network() { - assert_eq!( - TokenSymbols::for_network(Network::Mainnet), - TokenSymbols { - ckbtc: "ckBTC", - btc: "BTC" - } - ); - assert_eq!( - TokenSymbols::for_network(Network::Testnet), - TokenSymbols { - ckbtc: "ckTESTBTC", - btc: "TESTBTC" - } - ); - assert_eq!( - TokenSymbols::for_network(Network::Regtest), - TokenSymbols { - ckbtc: "ckTESTBTC", - btc: "TESTBTC" - } - ); - } - - #[test] - fn test_retrieve_btc_with_approval_uses_testnet_symbols() { - let args = RetrieveBtcWithApprovalArgs { - amount: 250_000, - address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), - from_subaccount: None, - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::FieldsDisplay), - ); - let info = build_consent_info(req, Network::Testnet).unwrap(); - let fields_display = match info.consent_message { - ConsentMessage::FieldsDisplayMessage(f) => f, - other => panic!("expected FieldsDisplayMessage, got {other:?}"), - }; - assert_eq!(fields_display.intent, "ckTESTBTC to TESTBTC"); - match &fields_display.fields[0].1 { - Value::TokenAmount { symbol, .. } => assert_eq!(symbol, "ckTESTBTC"), - other => panic!("expected TokenAmount, got {other:?}"), - } - // FieldsDisplay address label uses the testnet native symbol too. - assert_eq!(fields_display.fields[1].0, "TESTBTC address"); - } - - #[test] - fn test_retrieve_btc_with_approval_generic_uses_testnet_symbols() { - let args = RetrieveBtcWithApprovalArgs { - amount: 100_000_000, - address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), - from_subaccount: None, - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::GenericDisplay), - ); - let info = build_consent_info(req, Network::Testnet).unwrap(); - let message = match info.consent_message { - ConsentMessage::GenericDisplayMessage(m) => m, - other => panic!("expected GenericDisplayMessage, got {other:?}"), - }; - assert!(message.starts_with("# Convert ckTESTBTC to TESTBTC")); - assert!(message.contains("1 ckTESTBTC")); - assert!(message.contains("ckTESTBTC minter")); - assert!(message.contains("equivalent amount in TESTBTC")); - } - - #[test] - fn test_invalid_args() { - let req = make_request("retrieve_btc_with_approval", vec![1, 2, 3], None); - let err = build_consent_info(req, Network::Mainnet).unwrap_err(); - match err { - Icrc21Error::UnsupportedCanisterCall(info) => { - assert!(info.description.contains("Failed to decode")); - } - _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), - } - } - - #[test] - fn test_malformed_address_is_rejected() { - // The minter must not interpolate an unparseable address into the - // Markdown consent message — that would be a Markdown-injection vector - // (e.g. an "address" containing newlines, backticks, or '#' that fakes - // additional fields). - for bad_address in [ - "not-a-real-address", - "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq\n# You will receive 100 BTC", - "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa`\n\n**Amount:** 100 BTC\n`", - "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx", // valid testnet but on Mainnet - ] { - let args = RetrieveBtcWithApprovalArgs { - amount: 50_000, - address: bad_address.to_string(), - from_subaccount: None, - }; - let req = make_request( - "retrieve_btc_with_approval", - Encode!(&args).unwrap(), - Some(DisplayMessageType::GenericDisplay), - ); - let err = build_consent_info(req, Network::Mainnet).unwrap_err(); - match err { - Icrc21Error::UnsupportedCanisterCall(info) => { - assert!( - info.description - .contains("Invalid Bitcoin destination address"), - "unexpected error description: {}", - info.description - ); - } - other => panic!("expected UnsupportedCanisterCall, got {other:?}"), - } - } - } -} diff --git a/rs/bitcoin/ckbtc/minter/src/updates/tests.rs b/rs/bitcoin/ckbtc/minter/src/updates/tests.rs index 4fa4b1f2ca89..951d781fe3f5 100644 --- a/rs/bitcoin/ckbtc/minter/src/updates/tests.rs +++ b/rs/bitcoin/ckbtc/minter/src/updates/tests.rs @@ -1164,3 +1164,315 @@ mod update_balance { CkBtcEventLogger.events_iter() } } + +mod icrc21 { + use crate::Network; + use crate::updates::icrc21::{DECIMALS, TokenSymbols, build_consent_info, format_amount}; + use crate::updates::retrieve_btc::RetrieveBtcWithApprovalArgs; + use candid::Encode; + use icrc_ledger_types::icrc21::errors::Icrc21Error; + use icrc_ledger_types::icrc21::lib::MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES; + use icrc_ledger_types::icrc21::requests::{ + ConsentMessageMetadata, ConsentMessageRequest, ConsentMessageSpec, DisplayMessageType, + }; + use icrc_ledger_types::icrc21::responses::{ConsentMessage, Value}; + + fn make_request( + method: &str, + arg: Vec, + device_spec: Option, + ) -> ConsentMessageRequest { + ConsentMessageRequest { + method: method.to_string(), + arg, + user_preferences: ConsentMessageSpec { + metadata: ConsentMessageMetadata { + language: "en".to_string(), + utc_offset_minutes: None, + }, + device_spec, + }, + } + } + + #[test] + fn test_format_amount() { + assert_eq!(format_amount(0, 8), "0"); + assert_eq!(format_amount(1, 8), "0.00000001"); + assert_eq!(format_amount(100_000_000, 8), "1"); + assert_eq!(format_amount(150_000_000, 8), "1.5"); + assert_eq!(format_amount(123_456_789, 8), "1.23456789"); + } + + #[test] + fn test_unsupported_method() { + // Includes `retrieve_btc` because the minter intentionally only + // supports ICRC-21 for the approval-based flow — wallets calling + // `retrieve_btc` should not get a consent message rendered for them. + for method in ["update_balance", "retrieve_btc", "get_btc_address", ""] { + let req = make_request(method, vec![], None); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + assert!( + matches!(err, Icrc21Error::UnsupportedCanisterCall(_)), + "method {method:?} should be unsupported, got {err:?}" + ); + } + } + + #[test] + fn test_argument_too_large() { + let req = make_request( + "retrieve_btc_with_approval", + vec![0; MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES as usize + 1], + None, + ); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!(info.description.contains("argument size is too large")); + } + _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), + } + } + + #[test] + fn test_retrieve_btc_with_approval_generic_display() { + let args = RetrieveBtcWithApprovalArgs { + amount: 150_000, + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let info = build_consent_info(req, Network::Mainnet).unwrap(); + assert_eq!(info.metadata.language, "en"); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.starts_with("# Convert ckBTC to BTC")); + assert!(message.contains("0.0015 ckBTC")); + assert!(message.contains("bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq")); + // No subaccount section if from_subaccount is None. + assert!(!message.contains("source subaccount")); + } + + #[test] + fn test_retrieve_btc_with_approval_generic_display_with_subaccount() { + let mut subaccount = [0_u8; 32]; + subaccount[31] = 0x42; + let args = RetrieveBtcWithApprovalArgs { + amount: 100_000_000, + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), + from_subaccount: Some(subaccount), + }; + let req = make_request("retrieve_btc_with_approval", Encode!(&args).unwrap(), None); + let info = build_consent_info(req, Network::Mainnet).unwrap(); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.contains("1 ckBTC")); + assert!(message.contains(&hex::encode(subaccount))); + } + + #[test] + fn test_retrieve_btc_with_approval_fields_display() { + // Long values (Bitcoin address, subaccount hex) are emitted as a + // single Value::Text — wallets paginate them across screens. The + // number of fields is therefore independent of the value length. + let mut subaccount = [0_u8; 32]; + subaccount[0] = 0xab; + subaccount[31] = 0xcd; + let address = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(); + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: address.clone(), + from_subaccount: Some(subaccount), + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = build_consent_info(req, Network::Mainnet).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckBTC to BTC"); + assert_eq!( + fields_display.fields, + vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: DECIMALS, + amount: 250_000, + symbol: "ckBTC".to_string(), + } + ), + ("BTC address".to_string(), Value::Text { content: address }), + ( + "From subaccount".to_string(), + Value::Text { + content: hex::encode(subaccount) + } + ), + ] + ); + } + + #[test] + fn test_retrieve_btc_with_approval_fields_display_no_subaccount() { + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = build_consent_info(req, Network::Mainnet).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.fields.len(), 2); + // No subaccount field when from_subaccount is None. + assert!( + !fields_display + .fields + .iter() + .any(|(label, _)| label == "From subaccount") + ); + } + + #[test] + fn test_token_symbols_for_network() { + assert_eq!( + TokenSymbols::for_network(Network::Mainnet), + TokenSymbols { + ckbtc: "ckBTC", + btc: "BTC" + } + ); + assert_eq!( + TokenSymbols::for_network(Network::Testnet), + TokenSymbols { + ckbtc: "ckTESTBTC", + btc: "TESTBTC" + } + ); + assert_eq!( + TokenSymbols::for_network(Network::Regtest), + TokenSymbols { + ckbtc: "ckTESTBTC", + btc: "TESTBTC" + } + ); + } + + #[test] + fn test_retrieve_btc_with_approval_uses_testnet_symbols() { + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + ); + let info = build_consent_info(req, Network::Testnet).unwrap(); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckTESTBTC to TESTBTC"); + match &fields_display.fields[0].1 { + Value::TokenAmount { symbol, .. } => assert_eq!(symbol, "ckTESTBTC"), + other => panic!("expected TokenAmount, got {other:?}"), + } + // FieldsDisplay address label uses the testnet native symbol too. + assert_eq!(fields_display.fields[1].0, "TESTBTC address"); + } + + #[test] + fn test_retrieve_btc_with_approval_generic_uses_testnet_symbols() { + let args = RetrieveBtcWithApprovalArgs { + amount: 100_000_000, + address: "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx".to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let info = build_consent_info(req, Network::Testnet).unwrap(); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!(message.starts_with("# Convert ckTESTBTC to TESTBTC")); + assert!(message.contains("1 ckTESTBTC")); + assert!(message.contains("ckTESTBTC minter")); + assert!(message.contains("equivalent amount in TESTBTC")); + } + + #[test] + fn test_invalid_args() { + let req = make_request("retrieve_btc_with_approval", vec![1, 2, 3], None); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!(info.description.contains("Failed to decode")); + } + _ => panic!("expected UnsupportedCanisterCall, got {err:?}"), + } + } + + #[test] + fn test_malformed_address_is_rejected() { + // The minter must not interpolate an unparseable address into the + // Markdown consent message — that would be a Markdown-injection vector + // (e.g. an "address" containing newlines, backticks, or '#' that fakes + // additional fields). + for bad_address in [ + "not-a-real-address", + "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq\n# You will receive 100 BTC", + "1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa`\n\n**Amount:** 100 BTC\n`", + "tb1qw508d6qejxtdg4y5r3zarvary0c5xw7kxpjzsx", // valid testnet but on Mainnet + ] { + let args = RetrieveBtcWithApprovalArgs { + amount: 50_000, + address: bad_address.to_string(), + from_subaccount: None, + }; + let req = make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + ); + let err = build_consent_info(req, Network::Mainnet).unwrap_err(); + match err { + Icrc21Error::UnsupportedCanisterCall(info) => { + assert!( + info.description + .contains("Invalid Bitcoin destination address"), + "unexpected error description: {}", + info.description + ); + } + other => panic!("expected UnsupportedCanisterCall, got {other:?}"), + } + } + } +} From 3dc1a91c8cdb6276b78c8527fec2eb636e057c00 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Wed, 6 May 2026 10:54:50 +0200 Subject: [PATCH 14/16] test(ckbtc-minter): smoke integration test for ICRC-10/ICRC-21 endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback on PR #10093: add a state-machine-driven integration test covering the new endpoints end-to-end. The test installs a mainnet minter via the existing CkBtcSetup helper and asserts: * `icrc10_supported_standards` advertises both ICRC-10 and ICRC-21, with the ICRC-21 entry pointing at the canonical `dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md` URL. * `icrc21_canister_call_consent_message` for `retrieve_btc_with_approval` returns a GenericDisplay markdown message with the expected `# Convert ckBTC to BTC` header, amount and address; and a FieldsDisplay structured message with the expected intent and (Amount, BTC address) fields. * Unsupported methods — including `retrieve_btc`, which the minter intentionally does *not* render consent for — return `Icrc21Error::UnsupportedCanisterCall`. The test exercises the full Candid path (encode the request, send via ingress, decode the response), which the unit tests in updates::tests::icrc21 do not. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/tests/tests.rs | 155 +++++++++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/rs/bitcoin/ckbtc/minter/tests/tests.rs b/rs/bitcoin/ckbtc/minter/tests/tests.rs index 355bdb6f36b1..79f0cf550227 100644 --- a/rs/bitcoin/ckbtc/minter/tests/tests.rs +++ b/rs/bitcoin/ckbtc/minter/tests/tests.rs @@ -203,6 +203,161 @@ fn test_install_ckbtc_minter_canister() { install_minter(&env, ledger_id); } +/// Smoke-tests the ICRC-21 / ICRC-10 endpoints exposed by the minter: +/// +/// * `icrc10_supported_standards` advertises both standards with the canonical +/// `dfinity/ICRC` URLs. +/// * `icrc21_canister_call_consent_message` produces both a GenericDisplay +/// markdown message and a FieldsDisplay structured message for +/// `retrieve_btc_with_approval`, with the configured network's token symbols +/// threaded through. +/// * Unsupported methods return `Icrc21Error::UnsupportedCanisterCall`. +#[test] +fn test_icrc21_endpoints_smoke() { + use ic_ckbtc_minter::updates::icrc21::StandardRecord; + use icrc_ledger_types::icrc21::errors::Icrc21Error; + use icrc_ledger_types::icrc21::requests::{ + ConsentMessageMetadata, ConsentMessageRequest, ConsentMessageSpec, DisplayMessageType, + }; + use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, Value}; + + let setup = CkBtcSetup::new(); // mainnet minter; "ckBTC" / "BTC" symbols. + + let consent_message = |req: &ConsentMessageRequest| -> Result { + Decode!( + &assert_reply( + setup + .env + .execute_ingress_as( + setup.caller, + setup.minter_id, + "icrc21_canister_call_consent_message", + Encode!(req).unwrap(), + ) + .expect("icrc21_canister_call_consent_message ingress failed") + ), + Result + ) + .unwrap() + }; + let make_request = |method: &str, + arg: Vec, + device_spec: Option| + -> ConsentMessageRequest { + ConsentMessageRequest { + method: method.to_string(), + arg, + user_preferences: ConsentMessageSpec { + metadata: ConsentMessageMetadata { + language: "en".to_string(), + utc_offset_minutes: None, + }, + device_spec, + }, + } + }; + + // 1. icrc10_supported_standards advertises ICRC-10 and ICRC-21. + let standards = Decode!( + &assert_reply( + setup + .env + .query( + setup.minter_id, + "icrc10_supported_standards", + Encode!().unwrap() + ) + .expect("icrc10_supported_standards query failed") + ), + Vec + ) + .unwrap(); + let names: Vec<_> = standards.iter().map(|s| s.name.as_str()).collect(); + assert!( + names.contains(&"ICRC-10") && names.contains(&"ICRC-21"), + "expected ICRC-10 and ICRC-21 in supported standards, got {names:?}" + ); + let icrc21 = standards + .iter() + .find(|s| s.name == "ICRC-21") + .expect("ICRC-21 entry missing"); + assert_eq!( + icrc21.url, + "https://github.com/dfinity/ICRC/blob/main/ICRCs/ICRC-21/ICRC-21.md" + ); + + // 2a. retrieve_btc_with_approval / GenericDisplay renders the expected + // Markdown sections, with the configured (mainnet) token symbols. + let args = RetrieveBtcWithApprovalArgs { + amount: 250_000, + address: WITHDRAWAL_ADDRESS.to_string(), + from_subaccount: None, + }; + let info = consent_message(&make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::GenericDisplay), + )) + .expect("consent message should be produced for retrieve_btc_with_approval"); + let message = match info.consent_message { + ConsentMessage::GenericDisplayMessage(m) => m, + other => panic!("expected GenericDisplayMessage, got {other:?}"), + }; + assert!( + message.starts_with("# Convert ckBTC to BTC"), + "unexpected message header: {message}" + ); + assert!( + message.contains("0.0025 ckBTC") && message.contains(WITHDRAWAL_ADDRESS), + "unexpected message body: {message}" + ); + + // 2b. retrieve_btc_with_approval / FieldsDisplay renders the structured + // intent + (Amount, BTC address) pair. + let info = consent_message(&make_request( + "retrieve_btc_with_approval", + Encode!(&args).unwrap(), + Some(DisplayMessageType::FieldsDisplay), + )) + .expect("consent message should be produced for retrieve_btc_with_approval"); + let fields_display = match info.consent_message { + ConsentMessage::FieldsDisplayMessage(f) => f, + other => panic!("expected FieldsDisplayMessage, got {other:?}"), + }; + assert_eq!(fields_display.intent, "ckBTC to BTC"); + assert_eq!( + fields_display.fields, + vec![ + ( + "Amount".to_string(), + Value::TokenAmount { + decimals: 8, + amount: 250_000, + symbol: "ckBTC".to_string(), + } + ), + ( + "BTC address".to_string(), + Value::Text { + content: WITHDRAWAL_ADDRESS.to_string(), + } + ), + ] + ); + + // 3. Unsupported methods return UnsupportedCanisterCall. `retrieve_btc` + // is intentionally listed here — the minter only renders consent for + // the approval-based flow. + for method in ["retrieve_btc", "update_balance", "get_btc_address"] { + let err = consent_message(&make_request(method, vec![], None)) + .expect_err("expected UnsupportedCanisterCall"); + assert!( + matches!(err, Icrc21Error::UnsupportedCanisterCall(_)), + "method {method:?} should be rejected as unsupported, got {err:?}" + ); + } +} + #[test] fn test_wrong_upgrade_parameter() { let env = new_state_machine(); From bd563a2513db9557027b96105c0faa4aba1e7c8e Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Wed, 6 May 2026 11:34:20 +0200 Subject: [PATCH 15/16] test(ckbtc-minter): pin full GenericDisplay message in smoke test Replace the loose `starts_with` + `contains` checks in `test_icrc21_endpoints_smoke` with a full `assert_eq!` on the entire GenericDisplay markdown produced for `retrieve_btc_with_approval`. Any future wording tweak now surfaces as a test diff that has to be updated consciously instead of slipping past a substring match. Co-Authored-By: Claude Opus 4.7 (1M context) --- rs/bitcoin/ckbtc/minter/tests/tests.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/tests/tests.rs b/rs/bitcoin/ckbtc/minter/tests/tests.rs index 79f0cf550227..df96e9b07632 100644 --- a/rs/bitcoin/ckbtc/minter/tests/tests.rs +++ b/rs/bitcoin/ckbtc/minter/tests/tests.rs @@ -287,7 +287,9 @@ fn test_icrc21_endpoints_smoke() { ); // 2a. retrieve_btc_with_approval / GenericDisplay renders the expected - // Markdown sections, with the configured (mainnet) token symbols. + // Markdown sections, with the configured (mainnet) token symbols. The + // assertion is a full-string equality so any wording change has to be + // updated here consciously. let args = RetrieveBtcWithApprovalArgs { amount: 250_000, address: WITHDRAWAL_ADDRESS.to_string(), @@ -303,13 +305,16 @@ fn test_icrc21_endpoints_smoke() { ConsentMessage::GenericDisplayMessage(m) => m, other => panic!("expected GenericDisplayMessage, got {other:?}"), }; - assert!( - message.starts_with("# Convert ckBTC to BTC"), - "unexpected message header: {message}" - ); - assert!( - message.contains("0.0025 ckBTC") && message.contains(WITHDRAWAL_ADDRESS), - "unexpected message body: {message}" + assert_eq!( + message, + format!( + "# Convert ckBTC to BTC\n\n\ + Authorize the ckBTC minter to burn ckBTC from your account and \ + send the equivalent amount in BTC (minus network and minter fees) to \ + the Bitcoin address below.\n\n\ + **Amount to convert:** `0.0025 ckBTC`\n\n\ + **Bitcoin destination address:**\n`{WITHDRAWAL_ADDRESS}`" + ) ); // 2b. retrieve_btc_with_approval / FieldsDisplay renders the structured From da14c6d3cec610d23e93176c236354433f2a9078 Mon Sep 17 00:00:00 2001 From: Bjoern Tackmann Date: Wed, 6 May 2026 12:55:40 +0200 Subject: [PATCH 16/16] Update canbench measurement --- rs/bitcoin/ckbtc/minter/canbench/results.yml | 52 ++++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/rs/bitcoin/ckbtc/minter/canbench/results.yml b/rs/bitcoin/ckbtc/minter/canbench/results.yml index 52c70b9dad51..e4dbf561171b 100644 --- a/rs/bitcoin/ckbtc/minter/canbench/results.yml +++ b/rs/bitcoin/ckbtc/minter/canbench/results.yml @@ -2,13 +2,13 @@ benches: build_estimate_retrieve_btc_fee_1_50k_sats: total: calls: 1 - instructions: 419559 + instructions: 419644 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -18,24 +18,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413498 + instructions: 413500 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_1_50k_sats: total: calls: 1 - instructions: 421675 + instructions: 421760 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419092 + instructions: 419177 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -45,24 +45,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413498 + instructions: 413500 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_2_100k_sats: total: calls: 1 - instructions: 421721 + instructions: 421806 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419138 + instructions: 419223 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -72,24 +72,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413544 + instructions: 413546 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_3_1m_sats: total: calls: 1 - instructions: 421624 + instructions: 421709 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 419041 + instructions: 419126 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -99,24 +99,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 413447 + instructions: 413449 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_4_10m_sats: total: calls: 1 - instructions: 422799 + instructions: 422884 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 420216 + instructions: 420301 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -126,24 +126,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 414622 + instructions: 414624 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_5_1_btc: total: calls: 1 - instructions: 422799 + instructions: 422884 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 420216 + instructions: 420301 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3363 + instructions: 3446 heap_increase: 0 stable_memory_increase: 0 greedy: @@ -153,24 +153,24 @@ benches: stable_memory_increase: 0 utxos_selection: calls: 1 - instructions: 414622 + instructions: 414624 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_6_10_btc: total: calls: 1 - instructions: 429473 + instructions: 429556 heap_increase: 0 stable_memory_increase: 0 scopes: build_unsigned_transaction: calls: 1 - instructions: 426890 + instructions: 426973 heap_increase: 0 stable_memory_increase: 0 build_unsigned_transaction_from_inputs: calls: 1 - instructions: 3830 + instructions: 3913 heap_increase: 0 stable_memory_increase: 0 greedy: