From b3cdc0bcddd36bfbd3f5a540d40e690ebbc3d365 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 16 Jun 2025 12:37:07 +0000 Subject: [PATCH 01/28] fix generic message --- packages/icrc-ledger-types/src/icrc21/lib.rs | 19 ++++++++++--------- rs/ledger_suite/tests/sm-tests/src/lib.rs | 19 ++++++++++--------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index d656b8398428..6cf30e4e30a5 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -150,7 +150,12 @@ impl ConsentMessageBuilder { }; match self.function { Icrc21Function::Transfer => { - message.push_str("# Approve the transfer of funds"); + let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token Symbol must be specified.".to_owned(), + })?; + message.push_str(&format!("# Send {}", token_symbol)); + message.push_str("\n\nYou are approving a transfer of funds from your account."); let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -166,10 +171,6 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Token Symbol must be specified.".to_owned(), - })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -178,17 +179,17 @@ impl ConsentMessageBuilder { self.decimals, )?; - message.push_str(&format!("\n\n**Amount:**\n{} {}", amount, token_symbol)); if from_account.owner == Principal::anonymous() { message.push_str(&format!( "\n\n**From subaccount:**\n{}", extract_subaccount(from_account)? )); } else { - message.push_str(&format!("\n\n**From:**\n{}", from_account)); + message.push_str(&format!("\n\n**From:**\n`{}`", from_account)); } - message.push_str(&format!("\n\n**To:**\n{}", receiver_account)); - message.push_str(&format!("\n\n**Fee:**\n{} {}", fee, token_symbol)); + message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); + message.push_str(&format!("\n\n**To:**\n`{}`", receiver_account)); + message.push_str(&format!("\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", fee, token_symbol)); } Icrc21Function::Approve => { message.push_str("# Authorize another address to withdraw from your account"); diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index c43bf7c0b6c9..97f1f2067fdb 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4595,19 +4595,20 @@ fn test_icrc21_transfer_message( }, }; - let expected_transfer_message = "# Approve the transfer of funds + let expected_transfer_message = "# Send XTST -**Amount:** -0.01 XTST +You are approving a transfer of funds from your account. **From:** -d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101 +`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101` + +**Amount:** `0.01 XTST` **To:** -6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202 +`6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202` -**Fee:** -0.0001 XTST +**Fees:** `0.0001 XTST` +Charged for processing the transfer. **Memo:** test_bytes"; @@ -4672,7 +4673,7 @@ test_bytes"; .unwrap() .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**From subaccount:**\n101010101010101010101010101010101010101010101010101010101010101" ); + let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n101010101010101010101010101010101010101010101010101010101010101" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4691,7 +4692,7 @@ test_bytes"; .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**From subaccount:**\n0000000000000000000000000000000000000000000000000000000000000000" ); + let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n0000000000000000000000000000000000000000000000000000000000000000" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", From 60941645bb37b36c1faca6837f0032b6263fe0f6 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 08:53:35 +0000 Subject: [PATCH 02/28] clippy --- packages/icrc-ledger-types/src/icrc21/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 6cf30e4e30a5..ccb553759316 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -189,7 +189,10 @@ impl ConsentMessageBuilder { } message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); message.push_str(&format!("\n\n**To:**\n`{}`", receiver_account)); - message.push_str(&format!("\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", fee, token_symbol)); + message.push_str(&format!( + "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", + fee, token_symbol + )); } Icrc21Function::Approve => { message.push_str("# Authorize another address to withdraw from your account"); From cad65b7ca0efd56c99ef336d90542446ac2bcb3e Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 09:05:53 +0000 Subject: [PATCH 03/28] quote memo --- packages/icrc-ledger-types/src/icrc21/lib.rs | 2 +- rs/ledger_suite/tests/sm-tests/src/lib.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index ccb553759316..2dd7ec905f5c 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -347,7 +347,7 @@ impl ConsentMessageBuilder { if let Some(memo) = self.memo { message.push_str(&format!( - "\n\n**Memo:**\n{}", + "\n\n**Memo:**\n`{}`", // Check if the memo is a valid UTF-8 string and display it as such if it is. &match std::str::from_utf8(memo.as_slice()) { Ok(valid_str) => valid_str.to_string(), diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 97f1f2067fdb..937010de8b8d 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4611,7 +4611,7 @@ You are approving a transfer of funds from your account. Charged for processing the transfer. **Memo:** -test_bytes"; +`test_bytes`"; let consent_info = icrc21_consent_message(env, canister_id, from_account.owner, args.clone()).unwrap(); @@ -4640,7 +4640,7 @@ test_bytes"; .unwrap() .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**Memo:**\ntest_bytes", ""); + let expected_message = expected_transfer_message.replace("\n\n**Memo:**\n`test_bytes`", ""); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4768,7 +4768,7 @@ Thu, 06 May 2021 20:17:10 +0000 d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101 **Memo:** -test_bytes"; +`test_bytes`"; let mut args = ConsentMessageRequest { method: "icrc2_approve".to_owned(), @@ -4934,7 +4934,7 @@ djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.30303030303030303030303030303030303030303030 0.0001 XTST **Memo:** -test_bytes"; +`test_bytes`"; let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, spender_account.owner, args.clone()) From 779b70f109d3a1c0c6447faa5fe76c6299cc0cdc Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 09:30:53 +0000 Subject: [PATCH 04/28] fix subaccount --- packages/icrc-ledger-types/src/icrc21/lib.rs | 12 +++++++----- rs/ledger_suite/tests/sm-tests/src/lib.rs | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 2dd7ec905f5c..038ce11d60c6 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -2,7 +2,7 @@ use super::errors::ErrorInfo; use super::requests::ConsentMessageRequest; use super::responses::{ConsentInfo, ConsentMessage}; use super::{requests::DisplayMessageType, responses::LineDisplayPage}; -use crate::icrc1::account::Account; +use crate::icrc1::account::{Account, DEFAULT_SUBACCOUNT}; use crate::icrc1::transfer::TransferArg; use crate::icrc2::approve::ApproveArgs; use crate::icrc2::transfer_from::TransferFromArgs; @@ -180,10 +180,12 @@ impl ConsentMessageBuilder { )?; if from_account.owner == Principal::anonymous() { - message.push_str(&format!( - "\n\n**From subaccount:**\n{}", - extract_subaccount(from_account)? - )); + if from_account.effective_subaccount() != DEFAULT_SUBACCOUNT { + message.push_str(&format!( + "\n\n**From subaccount:**\n`{}`", + extract_subaccount(from_account)? + )); + } } else { message.push_str(&format!("\n\n**From:**\n`{}`", from_account)); } diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 937010de8b8d..f3dcc1d2de88 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4673,7 +4673,7 @@ Charged for processing the transfer. .unwrap() .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n101010101010101010101010101010101010101010101010101010101010101" ); + let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4692,7 +4692,7 @@ Charged for processing the transfer. .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n0000000000000000000000000000000000000000000000000000000000000000" ); + let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", From 478433c7b50bc359e6b0f0220c4ed32a7bad7c96 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 11:43:09 +0000 Subject: [PATCH 05/28] approve --- packages/icrc-ledger-types/src/icrc21/lib.rs | 55 +++++++++++--------- rs/ledger_suite/tests/sm-tests/src/lib.rs | 46 ++++++++-------- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 038ce11d60c6..52eb30594bbc 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -197,7 +197,10 @@ impl ConsentMessageBuilder { )); } Icrc21Function::Approve => { - message.push_str("# Authorize another address to withdraw from your account"); + message.push_str("# Approve spending"); + message.push_str( + "\n\nYou are authorizing another address to withdraw funds from your account.", + ); let approver_account = self.approver.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "Approver account has to be specified.".to_owned(), @@ -256,37 +259,41 @@ impl ConsentMessageBuilder { }) .unwrap_or("No expiration.".to_owned()); - message.push_str(&format!( - "\n\n**The following address is allowed to withdraw from your account:**\n{}", - spender_account - )); if approver_account.owner == Principal::anonymous() { - message.push_str(&format!( - "\n\n**Your subaccount:**\n{}", - extract_subaccount(approver_account)? - )); + if approver_account.effective_subaccount() != DEFAULT_SUBACCOUNT { + message.push_str(&format!( + "\n\n**From subaccount:**\n`{}`", + extract_subaccount(approver_account)? + )); + } } else { - message.push_str(&format!("\n\n**Your account:**\n{}", approver_account)); + message.push_str(&format!("\n\n**From:**\n`{}`", approver_account)); } message.push_str(&format!( - "\n\n**Requested withdrawal allowance:**\n{} {}", + "\n\n**Approve to spender:**\n`{}`", + spender_account + )); + message.push_str(&format!( + "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", amount, token_symbol )); - message.push_str(&match self.expected_allowance{ - Some(expected_allowance) => format!("\n\n**Current withdrawal allowance:**\n{} {}", convert_tokens_to_string_representation(expected_allowance,self.decimals)?,token_symbol), - None => format!("\n\u{26A0} The allowance will be set to {} {} independently of any previous allowance. Until this transaction has been executed the spender can still exercise the previous allowance (if any) to it's full amount.",amount,token_symbol)}); - message.push_str(&format!("\n\n**Expiration date:**\n{}", expires_at)); - message.push_str(&format!("\n\n**Approval fee:**\n{} {}", fee, token_symbol)); + if let Some(expected_allowance) = self.expected_allowance { + message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", convert_tokens_to_string_representation(expected_allowance,self.decimals)?,token_symbol)); + } + message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); + message.push_str(&format!( + "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", + fee, token_symbol + )); if approver_account.owner == Principal::anonymous() { - message.push_str(&format!( - "\n\n**Transaction fees to be paid by your subaccount:**\n{}", - extract_subaccount(approver_account)? - )); + if approver_account.effective_subaccount() != DEFAULT_SUBACCOUNT { + message.push_str(&format!( + "\n\n**Fees paid by your subaccount:**\n`{}`", + extract_subaccount(approver_account)? + )); + } } else { - message.push_str(&format!( - "\n\n**Transaction fees to be paid by:**\n{}", - approver_account - )); + message.push_str(&format!("\n\n**Fees paid by:**\n`{}`", approver_account)); } } Icrc21Function::TransferFrom => { diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index f3dcc1d2de88..95f94c486a87 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4744,28 +4744,30 @@ fn test_icrc21_approve_message( memo: Some(Memo::from(b"test_bytes".to_vec())), }; assert_eq!(spender_account.to_string(), "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303"); - let expected_approve_message = "# Authorize another address to withdraw from your account + let expected_approve_message = "# Approve spending -**The following address is allowed to withdraw from your account:** -djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303 +You are authorizing another address to withdraw funds from your account. -**Your account:** -d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101 +**From:** +`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101` -**Requested withdrawal allowance:** -0.01 XTST +**Approve to spender:** +`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303` -**Current withdrawal allowance:** -0.01 XTST +**Requested allowance:** `0.01 XTST` +This is the withdrawal limit that will apply upon approval. + +**Existing allowance:** `0.01 XTST` +Until approval, this allowance remains in effect. -**Expiration date:** +**Approval expiration:** Thu, 06 May 2021 20:17:10 +0000 -**Approval fee:** -0.0001 XTST +**Approval fees:** `0.0001 XTST` +Charged for processing the approval. -**Transaction fees to be paid by:** -d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101 +**Fees paid by:** +`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101` **Memo:** `test_bytes`"; @@ -4801,11 +4803,9 @@ d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.10101010101010101010101010101010101010101010 .unwrap() .consent_message, ); - // When the expected allowance is not set, a warning should be displayed. - let expected_message = expected_approve_message.replace( - "\n\n**Current withdrawal allowance:**\n0.01 XTST", - "\n\u{26A0} The allowance will be set to 0.01 XTST independently of any previous allowance. Until this transaction has been executed the spender can still exercise the previous allowance (if any) to it's full amount.", - ); + // When the expected allowance is not set, it should be skipped. + let expected_message = + expected_approve_message.replace("\n\n**Existing allowance:** `0.01 XTST`\nUntil approval, this allowance remains in effect.", ""); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4839,8 +4839,8 @@ d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.10101010101010101010101010101010101010101010 .consent_message, ); let expected_message = expected_approve_message - .replace("\n\n**Transaction fees to be paid by:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**Transaction fees to be paid by your subaccount:**\n101010101010101010101010101010101010101010101010101010101010101" ) - .replace("\n\n**Your account:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**Your subaccount:**\n101010101010101010101010101010101010101010101010101010101010101"); + .replace("\n\n**Fees paid by:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**Fees paid by your subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`" ) + .replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`"); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4877,8 +4877,8 @@ d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.10101010101010101010101010101010101010101010 .consent_message, ); - let expected_message = expected_approve_message.replace("\n\n**Your account:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**Your subaccount:**\n0000000000000000000000000000000000000000000000000000000000000000" ) - .replace("\n\n**Transaction fees to be paid by:**\nd2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101","\n\n**Transaction fees to be paid by your subaccount:**\n0000000000000000000000000000000000000000000000000000000000000000" ); + let expected_message = expected_approve_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ) + .replace("\n\n**Fees paid by:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", From a459b318678e3cd1617670057a5015cc67e1d382 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 11:53:49 +0000 Subject: [PATCH 06/28] add comment --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 95f94c486a87..eb241d49fedf 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4680,6 +4680,7 @@ Charged for processing the transfer. expected_message, message ); + // If from_subaccount is not specified, no from information should be included. args.arg = Encode!(&TransferArg { from_subaccount: None, ..transfer_args.clone() From 0e400c335470885b32c23dea65519553db72482f Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 12:00:25 +0000 Subject: [PATCH 07/28] fix no expiration msg --- packages/icrc-ledger-types/src/icrc21/lib.rs | 2 +- rs/ledger_suite/tests/sm-tests/src/lib.rs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 52eb30594bbc..056f32facd36 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -257,7 +257,7 @@ impl ConsentMessageBuilder { Err(_) => format!("Invalid timestamp: {}", ts), } }) - .unwrap_or("No expiration.".to_owned()); + .unwrap_or("This approval does not have an expiration.".to_owned()); if approver_account.owner == Principal::anonymous() { if approver_account.effective_subaccount() != DEFAULT_SUBACCOUNT { diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index eb241d49fedf..5fa0f39923b7 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4824,8 +4824,10 @@ Charged for processing the approval. .unwrap() .consent_message, ); - let expected_message = - expected_approve_message.replace("Thu, 06 May 2021 20:17:10 +0000", "No expiration."); + let expected_message = expected_approve_message.replace( + "Thu, 06 May 2021 20:17:10 +0000", + "This approval does not have an expiration.", + ); assert_eq!( message, expected_message, "Expected: {}, got: {}", From 9c8ec95581bacb5c64d1df6ac85534490131d717 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 12:12:01 +0000 Subject: [PATCH 08/28] test memo not included --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 5fa0f39923b7..7f7761cdb103 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4887,6 +4887,26 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + + // If memo is not specified it should not be included. + args.arg = Encode!(&ApproveArgs { + memo: None, + ..approve_args.clone() + }) + .unwrap(); + + let message = extract_icrc21_message_string( + &icrc21_consent_message(env, canister_id, from_account.owner, args.clone()) + .unwrap() + .consent_message, + ); + + let expected_message = expected_approve_message.replace("\n\n**Memo:**\n`test_bytes`", ""); + assert_eq!( + message, expected_message, + "Expected: {}, got: {}", + expected_message, message + ); } fn test_icrc21_transfer_from_message( From 6247efa1fe57ca83e2e8c93679aab077e8c4cb2f Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 12:16:40 +0000 Subject: [PATCH 09/28] add comments --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 7f7761cdb103..ed40b63f1cc9 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4813,6 +4813,7 @@ Charged for processing the approval. expected_message, message ); + // Test approval without an expiration. args.arg = Encode!(&ApproveArgs { expires_at: None, ..approve_args.clone() @@ -4867,6 +4868,7 @@ Charged for processing the approval. expected_message, message ); + // If from_subaccount is not specified, the from information should be skipped. args.arg = Encode!(&ApproveArgs { from_subaccount: None, ..approve_args.clone() From 179d2cd4f31aa3beefda93942f9cb9034ac71c21 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 12:50:37 +0000 Subject: [PATCH 10/28] transfer from --- packages/icrc-ledger-types/src/icrc21/lib.rs | 40 ++++++++++---------- rs/ledger_suite/tests/sm-tests/src/lib.rs | 29 +++++++------- 2 files changed, 34 insertions(+), 35 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 056f32facd36..b1175f77e987 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -297,7 +297,14 @@ impl ConsentMessageBuilder { } } Icrc21Function::TransferFrom => { - message.push_str("# Transfer from a withdrawal account"); + let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token symbol must be specified.".to_owned(), + })?; + message.push_str(&format!("# Spend {}", token_symbol)); + message.push_str( + "\n\nYou are approving a transfer of funds from a withdrawal account.", + ); let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -318,10 +325,6 @@ impl ConsentMessageBuilder { self.decimals, )?; - let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Token symbol must be specified.".to_owned(), - })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -329,26 +332,21 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - - message.push_str(&format!("\n\n**Withdrawal account:**\n{}", from_account)); + message.push_str(&format!("\n\n**From:**\n`{}`", from_account)); + message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); if spender_account.owner == Principal::anonymous() { - message.push_str(&format!( - "\n\n**Subaccount sending the transfer request:**\n{}", - extract_subaccount(spender_account)? - )); + if spender_account.effective_subaccount() != DEFAULT_SUBACCOUNT { + message.push_str(&format!( + "\n\n**Spender subaccount:**\n`{}`", + extract_subaccount(spender_account)? + )); + } } else { - message.push_str(&format!( - "\n\n**Account sending the transfer request:**\n{}", - spender_account - )); + message.push_str(&format!("\n\n**Spender:**\n`{}`", spender_account)); } + message.push_str(&format!("\n\n**To:**\n`{}`", receiver_account)); message.push_str(&format!( - "\n\n**Amount to withdraw:**\n{} {}", - amount, token_symbol - )); - message.push_str(&format!("\n\n**To:**\n{}", receiver_account)); - message.push_str(&format!( - "\n\n**Fee paid by withdrawal account:**\n{} {}", + "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", fee, token_symbol )); } diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index ed40b63f1cc9..951f597ea0e4 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4941,22 +4941,23 @@ fn test_icrc21_transfer_from_message( }, }; - let expected_transfer_from_message = "# Transfer from a withdrawal account + let expected_transfer_from_message = "# Spend XTST -**Withdrawal account:** -d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101 +You are approving a transfer of funds from a withdrawal account. -**Account sending the transfer request:** -djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303 +**From:** +`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101` + +**Amount:** `0.01 XTST` -**Amount to withdraw:** -0.01 XTST +**Spender:** +`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303` **To:** -6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202 +`6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202` -**Fee paid by withdrawal account:** -0.0001 XTST +**Fees:** `0.0001 XTST` +Charged for processing the transfer. **Memo:** `test_bytes`"; @@ -4980,8 +4981,8 @@ djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.30303030303030303030303030303030303030303030 .consent_message, ); let expected_message = expected_transfer_from_message.replace( - "\n\n**Account sending the transfer request:**\ndjduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303", - "\n\n**Subaccount sending the transfer request:**\n303030303030303030303030303030303030303030303030303030303030303", + "\n\n**Spender:**\n`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303`", + "\n\n**Spender subaccount:**\n`303030303030303030303030303030303030303030303030303030303030303`", ); assert_eq!( message, expected_message, @@ -5002,8 +5003,8 @@ djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.30303030303030303030303030303030303030303030 ); let expected_message = expected_transfer_from_message.replace( - "\n\n**Account sending the transfer request:**\ndjduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303", - "\n\n**Subaccount sending the transfer request:**\n0000000000000000000000000000000000000000000000000000000000000000" ); + "\n\n**Spender:**\n`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303`", + "" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", From f4028a3725e755627e10b08e3caf6cd94471e158 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 12:51:57 +0000 Subject: [PATCH 11/28] add comment --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 951f597ea0e4..f61702174484 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4990,6 +4990,7 @@ Charged for processing the transfer. expected_message, message ); + // If the spender is anonymous and spender_subaccount is default, the spender information should be skipped. args.arg = Encode!(&TransferFromArgs { spender_subaccount: None, ..transfer_from_args.clone() From 5efab6858493ecbc66db144455fca681987bd7c5 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Tue, 17 Jun 2025 13:09:11 +0000 Subject: [PATCH 12/28] check no memo --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index f61702174484..5d236d1a95ab 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -5011,6 +5011,27 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + + // If memo is not specified it should not be included. + args.arg = Encode!(&TransferFromArgs { + memo: None, + ..transfer_from_args.clone() + }) + .unwrap(); + + let message = extract_icrc21_message_string( + &icrc21_consent_message(env, canister_id, spender_account.owner, args.clone()) + .unwrap() + .consent_message, + ); + + let expected_message = + expected_transfer_from_message.replace("\n\n**Memo:**\n`test_bytes`", ""); + assert_eq!( + message, expected_message, + "Expected: {}, got: {}", + expected_message, message + ); } fn extract_icrc21_message_string(consent_message: &ConsentMessage) -> String { From 94c8adaaf9fbde1246bef7782b55fd1eb97f7a38 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 12:46:41 +0000 Subject: [PATCH 13/28] remove line display, add generation to the type, remove account always if anonymous --- packages/icrc-ledger-types/src/icrc21/lib.rs | 206 +++--------------- .../icrc-ledger-types/src/icrc21/requests.rs | 5 +- .../icrc-ledger-types/src/icrc21/responses.rs | 116 +++++++++- rs/ledger_suite/tests/sm-tests/src/lib.rs | 80 +------ 4 files changed, 153 insertions(+), 254 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index b1175f77e987..9314fcf337e7 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -1,16 +1,16 @@ use super::errors::ErrorInfo; use super::requests::ConsentMessageRequest; +use super::requests::DisplayMessageType; use super::responses::{ConsentInfo, ConsentMessage}; -use super::{requests::DisplayMessageType, responses::LineDisplayPage}; -use crate::icrc1::account::{Account, DEFAULT_SUBACCOUNT}; +use crate::icrc1::account::Account; use crate::icrc1::transfer::TransferArg; use crate::icrc2::approve::ApproveArgs; use crate::icrc2::transfer_from::TransferFromArgs; use crate::icrc21::errors::Icrc21Error; use crate::icrc21::requests::ConsentMessageMetadata; +use crate::icrc21::responses::Intent; use candid::Decode; use candid::{Nat, Principal}; -use itertools::Itertools; use num_traits::{Pow, ToPrimitive}; use serde_bytes::ByteBuf; use strum::{self, IntoEnumIterator}; @@ -133,20 +133,11 @@ impl ConsentMessageBuilder { } pub fn build(self) -> Result { - let mut message = "".to_string(); - let extract_subaccount = |account: Account| -> Result { - Ok(match account.subaccount { - None => hex::encode(account.effective_subaccount().as_slice()), - Some(_) => account - .to_string() - .split('.') - .next_back() - .ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Subaccount has an unexpected format.".to_owned(), - })? - .to_string(), - }) + let mut message = match self.display_type { + Some(DisplayMessageType::GenericDisplay) | None => { + ConsentMessage::GenericDisplayMessage("".to_string()) + } + Some(DisplayMessageType::FieldsDisplay) => todo!(), }; match self.function { Icrc21Function::Transfer => { @@ -154,8 +145,6 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token Symbol must be specified.".to_owned(), })?; - message.push_str(&format!("# Send {}", token_symbol)); - message.push_str("\n\nYou are approving a transfer of funds from your account."); let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -179,28 +168,15 @@ impl ConsentMessageBuilder { self.decimals, )?; - if from_account.owner == Principal::anonymous() { - if from_account.effective_subaccount() != DEFAULT_SUBACCOUNT { - message.push_str(&format!( - "\n\n**From subaccount:**\n`{}`", - extract_subaccount(from_account)? - )); - } - } else { - message.push_str(&format!("\n\n**From:**\n`{}`", from_account)); + message.add_intent(Intent::Transfer, token_symbol.clone()); + if from_account.owner != Principal::anonymous() { + message.add_account("From", from_account); } - message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); - message.push_str(&format!("\n\n**To:**\n`{}`", receiver_account)); - message.push_str(&format!( - "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", - fee, token_symbol - )); + message.add_amount(amount, token_symbol.clone()); + message.add_account("To", receiver_account); + message.add_fee(fee, token_symbol.clone(), Intent::Transfer); } Icrc21Function::Approve => { - message.push_str("# Approve spending"); - message.push_str( - "\n\nYou are authorizing another address to withdraw funds from your account.", - ); let approver_account = self.approver.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "Approver account has to be specified.".to_owned(), @@ -259,41 +235,22 @@ impl ConsentMessageBuilder { }) .unwrap_or("This approval does not have an expiration.".to_owned()); - if approver_account.owner == Principal::anonymous() { - if approver_account.effective_subaccount() != DEFAULT_SUBACCOUNT { - message.push_str(&format!( - "\n\n**From subaccount:**\n`{}`", - extract_subaccount(approver_account)? - )); - } - } else { - message.push_str(&format!("\n\n**From:**\n`{}`", approver_account)); + message.add_intent(Intent::Approve, "NONE".to_string()); + if approver_account.owner != Principal::anonymous() { + message.add_account("From", approver_account); } - message.push_str(&format!( - "\n\n**Approve to spender:**\n`{}`", - spender_account - )); - message.push_str(&format!( - "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", - amount, token_symbol - )); + message.add_account("Approve to spender", spender_account); + message.add_allowance(amount, token_symbol.clone()); if let Some(expected_allowance) = self.expected_allowance { - message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", convert_tokens_to_string_representation(expected_allowance,self.decimals)?,token_symbol)); + message.add_existing_allowance( + convert_tokens_to_string_representation(expected_allowance, self.decimals)?, + token_symbol.clone(), + ); } - message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); - message.push_str(&format!( - "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", - fee, token_symbol - )); - if approver_account.owner == Principal::anonymous() { - if approver_account.effective_subaccount() != DEFAULT_SUBACCOUNT { - message.push_str(&format!( - "\n\n**Fees paid by your subaccount:**\n`{}`", - extract_subaccount(approver_account)? - )); - } - } else { - message.push_str(&format!("\n\n**Fees paid by:**\n`{}`", approver_account)); + message.add_expiration(expires_at); + message.add_fee(fee, token_symbol.clone(), Intent::Approve); + if approver_account.owner != Principal::anonymous() { + message.add_account("Fees paid by", approver_account); } } Icrc21Function::TransferFrom => { @@ -301,10 +258,6 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token symbol must be specified.".to_owned(), })?; - message.push_str(&format!("# Spend {}", token_symbol)); - message.push_str( - "\n\nYou are approving a transfer of funds from a withdrawal account.", - ); let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -332,99 +285,23 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - message.push_str(&format!("\n\n**From:**\n`{}`", from_account)); - message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); - if spender_account.owner == Principal::anonymous() { - if spender_account.effective_subaccount() != DEFAULT_SUBACCOUNT { - message.push_str(&format!( - "\n\n**Spender subaccount:**\n`{}`", - extract_subaccount(spender_account)? - )); - } - } else { - message.push_str(&format!("\n\n**Spender:**\n`{}`", spender_account)); + message.add_intent(Intent::TransferFrom, token_symbol.clone()); + message.add_account("From", from_account); + message.add_amount(amount, token_symbol.clone()); + if spender_account.owner != Principal::anonymous() { + message.add_account("Spender", spender_account); } - message.push_str(&format!("\n\n**To:**\n`{}`", receiver_account)); - message.push_str(&format!( - "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", - fee, token_symbol - )); + message.add_account("To", receiver_account); + message.add_fee(fee, token_symbol.clone(), Intent::TransferFrom); } }; if let Some(memo) = self.memo { - message.push_str(&format!( - "\n\n**Memo:**\n`{}`", - // Check if the memo is a valid UTF-8 string and display it as such if it is. - &match std::str::from_utf8(memo.as_slice()) { - Ok(valid_str) => valid_str.to_string(), - Err(_) => hex::encode(memo.as_slice()), - } - )); - } - - match self.display_type { - Some(DisplayMessageType::GenericDisplay) | None => { - Ok(ConsentMessage::GenericDisplayMessage(message)) - } - Some(DisplayMessageType::LineDisplay { - lines_per_page, - characters_per_line, - }) => { - let pages = consent_msg_text_pages(&message, characters_per_line, lines_per_page); - Ok(ConsentMessage::LineDisplayMessage { pages }) - } + message.add_memo(memo); } - } -} -/// This function was taken from the reference implementation: https://github.com/dfinity/wg-identity-authentication/blob/3ed140225b283c0a1cc88344d0cfb9912aec73cd/reference-implementations/ICRC-21/src/lib.rs#L73 -pub fn consent_msg_text_pages( - message: &str, - characters_per_line: u16, - lines_per_page: u16, -) -> Vec { - if characters_per_line == 0 || lines_per_page == 0 { - return vec![]; + Ok(message) } - - // Split text into word chunks that fit on a line (breaking long words) - let words = message.split_whitespace().flat_map(|word| { - word.chars() - .chunks(characters_per_line as usize) - .into_iter() - .map(|chunk| chunk.collect::()) - .collect::>() - }); - - // Add words to lines until the line is full - let mut lines = vec![]; - let mut current_line = "".to_string(); - for word in words { - if current_line.is_empty() { - // all words are guaranteed to fit on a line - current_line = word; - continue; - } - if current_line.len() + word.len() < characters_per_line as usize { - current_line.push(' '); - current_line.push_str(word.as_str()); - } else { - lines.push(current_line); - current_line = word; - } - } - lines.push(current_line); - - // Group lines into pages - lines - .into_iter() - .chunks(lines_per_page as usize) - .into_iter() - .map(|page| LineDisplayPage { - lines: page.collect(), - }) - .collect() } pub fn build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( @@ -466,17 +343,6 @@ pub fn build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( } if let Some(display_type) = consent_msg_request.user_preferences.device_spec { - if let DisplayMessageType::LineDisplay { - lines_per_page, - characters_per_line, - } = display_type - { - if lines_per_page == 0 || characters_per_line == 0 { - return Err(Icrc21Error::UnsupportedCanisterCall(ErrorInfo { - description: "Invalid display type. Lines per page and characters per line must be greater than 0.".to_string() - })); - } - } display_message_builder = display_message_builder.with_display_type(display_type); } diff --git a/packages/icrc-ledger-types/src/icrc21/requests.rs b/packages/icrc-ledger-types/src/icrc21/requests.rs index 0d4ded56749e..e95c27007f38 100644 --- a/packages/icrc-ledger-types/src/icrc21/requests.rs +++ b/packages/icrc-ledger-types/src/icrc21/requests.rs @@ -10,10 +10,7 @@ pub struct ConsentMessageMetadata { #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum DisplayMessageType { GenericDisplay, - LineDisplay { - characters_per_line: u16, - lines_per_page: u16, - }, + FieldsDisplay, } #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index 6ffdc31777dd..7b7f92d98607 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -1,16 +1,13 @@ +use crate::icrc1::account::Account; + use super::requests::ConsentMessageMetadata; use candid::{CandidType, Deserialize}; use serde::Serialize; - -#[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct LineDisplayPage { - pub lines: Vec, -} +use serde_bytes::ByteBuf; #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum ConsentMessage { GenericDisplayMessage(String), - LineDisplayMessage { pages: Vec }, } #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -18,3 +15,110 @@ pub struct ConsentInfo { pub consent_message: ConsentMessage, pub metadata: ConsentMessageMetadata, } + +pub enum Intent { + Transfer, + Approve, + TransferFrom, +} + +impl ConsentMessage { + pub fn add_intent(&mut self, intent: Intent, token_symbol: String) { + match self { + ConsentMessage::GenericDisplayMessage(message) => match intent { + Intent::Transfer => { + message.push_str(&format!("# Send {}", token_symbol)); + message + .push_str("\n\nYou are approving a transfer of funds from your account."); + } + Intent::Approve => { + message.push_str("# Approve spending"); + message.push_str( + "\n\nYou are authorizing another address to withdraw funds from your account.", + ); + } + Intent::TransferFrom => { + message.push_str(&format!("# Spend {}", token_symbol)); + message.push_str( + "\n\nYou are approving a transfer of funds from a withdrawal account.", + ); + } + }, + } + } + + pub fn add_account(&mut self, name: &str, account: Account) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!("\n\n**{}:**\n`{}`", name, account)); + } + } + } + + pub fn add_amount(&mut self, amount: String, token_symbol: String) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); + } + } + } + + pub fn add_fee(&mut self, fee: String, token_symbol: String, intent: Intent) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + match intent { + Intent::Approve => message.push_str(&format!( + "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", + fee, token_symbol + )), + _ => message.push_str(&format!( + "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", + fee, token_symbol + )), + }; + } + } + } + + pub fn add_allowance(&mut self, amount: String, token_symbol: String) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!( + "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", + amount, token_symbol + )); + } + } + } + + pub fn add_existing_allowance(&mut self, expected_allowance: String, token_symbol: String) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", expected_allowance, token_symbol)); + } + } + } + + pub fn add_expiration(&mut self, expires_at: String) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); + } + } + } + + pub fn add_memo(&mut self, memo: ByteBuf) { + match self { + ConsentMessage::GenericDisplayMessage(message) => { + message.push_str(&format!( + "\n\n**Memo:**\n`{}`", + // Check if the memo is a valid UTF-8 string and display it as such if it is. + &match std::str::from_utf8(memo.as_slice()) { + Ok(valid_str) => valid_str.to_string(), + Err(_) => hex::encode(memo.as_slice()), + } + )); + } + } + } +} diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index e7dc65c06ddd..0f56ebf8d768 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4673,33 +4673,13 @@ Charged for processing the transfer. expected_message, message ); - // If the from account is anonymous, the message should not include the from account but only the from subaccount. + // If the from account is anonymous, the message should not include the account information. args.arg = Encode!(&transfer_args.clone()).unwrap(); let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) .unwrap() .consent_message, ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`" ); - assert_eq!( - message, expected_message, - "Expected: {}, got: {}", - expected_message, message - ); - - // If from_subaccount is not specified, no from information should be included. - args.arg = Encode!(&TransferArg { - from_subaccount: None, - ..transfer_args.clone() - }) - .unwrap(); - - let message = extract_icrc21_message_string( - &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) - .unwrap() - .consent_message, - ); - let expected_message = expected_transfer_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ); assert_eq!( message, expected_message, @@ -4842,7 +4822,7 @@ Charged for processing the approval. expected_message, message ); - // If the approver is anonymous, the message should not include the approver account but only the approver subaccount. + // If the approver is anonymous, the message should not include the approver information. args.arg = Encode!(&approve_args.clone()).unwrap(); let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) @@ -4850,8 +4830,8 @@ Charged for processing the approval. .consent_message, ); let expected_message = expected_approve_message - .replace("\n\n**Fees paid by:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**Fees paid by your subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`" ) - .replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","\n\n**From subaccount:**\n`101010101010101010101010101010101010101010101010101010101010101`"); + .replace("\n\n**Fees paid by:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ) + .replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`",""); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -4874,29 +4854,8 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - - // If from_subaccount is not specified, the from information should be skipped. - args.arg = Encode!(&ApproveArgs { - from_subaccount: None, - ..approve_args.clone() - }) - .unwrap(); args.user_preferences.metadata.utc_offset_minutes = None; - let message = extract_icrc21_message_string( - &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) - .unwrap() - .consent_message, - ); - - let expected_message = expected_approve_message.replace("\n\n**From:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ) - .replace("\n\n**Fees paid by:**\n`d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101`","" ); - assert_eq!( - message, expected_message, - "Expected: {}, got: {}", - expected_message, message - ); - // If memo is not specified it should not be included. args.arg = Encode!(&ApproveArgs { memo: None, @@ -4980,7 +4939,7 @@ Charged for processing the transfer. expected_transfer_from_message, message ); - // If the spender is anonymous, the message should not include the spender account but only the spender subaccount. + // If the spender is anonymous, the message should not include the spender account information. args.arg = Encode!(&transfer_from_args.clone()).unwrap(); let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) @@ -4989,30 +4948,8 @@ Charged for processing the transfer. ); let expected_message = expected_transfer_from_message.replace( "\n\n**Spender:**\n`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303`", - "\n\n**Spender subaccount:**\n`303030303030303030303030303030303030303030303030303030303030303`", - ); - assert_eq!( - message, expected_message, - "Expected: {}, got: {}", - expected_message, message - ); - - // If the spender is anonymous and spender_subaccount is default, the spender information should be skipped. - args.arg = Encode!(&TransferFromArgs { - spender_subaccount: None, - ..transfer_from_args.clone() - }) - .unwrap(); - - let message = extract_icrc21_message_string( - &icrc21_consent_message(env, canister_id, Principal::anonymous(), args.clone()) - .unwrap() - .consent_message, + "", ); - - let expected_message = expected_transfer_from_message.replace( - "\n\n**Spender:**\n`djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303`", - "" ); assert_eq!( message, expected_message, "Expected: {}, got: {}", @@ -5044,11 +4981,6 @@ Charged for processing the transfer. fn extract_icrc21_message_string(consent_message: &ConsentMessage) -> String { match consent_message { ConsentMessage::GenericDisplayMessage(message) => message.to_string(), - ConsentMessage::LineDisplayMessage { pages } => pages - .iter() - .map(|page| page.lines.join("")) - .collect::>() - .join(""), } } From 1e8f229de86ba0d4e13d91b7ff57d0a283595e45 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 13:01:04 +0000 Subject: [PATCH 14/28] put token symbol back --- packages/icrc-ledger-types/src/icrc21/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 9314fcf337e7..26dd7768947e 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -141,10 +141,6 @@ impl ConsentMessageBuilder { }; match self.function { Icrc21Function::Transfer => { - let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Token Symbol must be specified.".to_owned(), - })?; let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -160,6 +156,10 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; + let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token Symbol must be specified.".to_owned(), + })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -254,10 +254,6 @@ impl ConsentMessageBuilder { } } Icrc21Function::TransferFrom => { - let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Token symbol must be specified.".to_owned(), - })?; let from_account = self.from.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "From account has to be specified.".to_owned(), @@ -278,6 +274,10 @@ impl ConsentMessageBuilder { self.decimals, )?; + let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token symbol must be specified.".to_owned(), + })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), From 52819a8b4e33c6bec91b7171123c672a994449cc Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 13:14:14 +0000 Subject: [PATCH 15/28] refactor --- packages/icrc-ledger-types/src/icrc21/lib.rs | 43 ++++++++++--------- .../icrc-ledger-types/src/icrc21/responses.rs | 14 +++--- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 26dd7768947e..7730e00d2770 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -168,13 +168,13 @@ impl ConsentMessageBuilder { self.decimals, )?; - message.add_intent(Intent::Transfer, token_symbol.clone()); + message.add_intent(Intent::Transfer, &token_symbol); if from_account.owner != Principal::anonymous() { - message.add_account("From", from_account); + message.add_account("From", &from_account); } - message.add_amount(amount, token_symbol.clone()); - message.add_account("To", receiver_account); - message.add_fee(fee, token_symbol.clone(), Intent::Transfer); + message.add_amount(&amount, &token_symbol); + message.add_account("To", &receiver_account); + message.add_fee(Intent::Transfer, &fee, &token_symbol); } Icrc21Function::Approve => { let approver_account = self.approver.ok_or(Icrc21Error::GenericError { @@ -235,22 +235,25 @@ impl ConsentMessageBuilder { }) .unwrap_or("This approval does not have an expiration.".to_owned()); - message.add_intent(Intent::Approve, "NONE".to_string()); + message.add_intent(Intent::Approve, &token_symbol); if approver_account.owner != Principal::anonymous() { - message.add_account("From", approver_account); + message.add_account("From", &approver_account); } - message.add_account("Approve to spender", spender_account); - message.add_allowance(amount, token_symbol.clone()); + message.add_account("Approve to spender", &spender_account); + message.add_allowance(&amount, &token_symbol); if let Some(expected_allowance) = self.expected_allowance { message.add_existing_allowance( - convert_tokens_to_string_representation(expected_allowance, self.decimals)?, - token_symbol.clone(), + &convert_tokens_to_string_representation( + expected_allowance, + self.decimals, + )?, + &token_symbol, ); } - message.add_expiration(expires_at); - message.add_fee(fee, token_symbol.clone(), Intent::Approve); + message.add_expiration(&expires_at); + message.add_fee(Intent::Approve, &fee, &token_symbol); if approver_account.owner != Principal::anonymous() { - message.add_account("Fees paid by", approver_account); + message.add_account("Fees paid by", &approver_account); } } Icrc21Function::TransferFrom => { @@ -285,14 +288,14 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - message.add_intent(Intent::TransferFrom, token_symbol.clone()); - message.add_account("From", from_account); - message.add_amount(amount, token_symbol.clone()); + message.add_intent(Intent::TransferFrom, &token_symbol); + message.add_account("From", &from_account); + message.add_amount(&amount, &token_symbol); if spender_account.owner != Principal::anonymous() { - message.add_account("Spender", spender_account); + message.add_account("Spender", &spender_account); } - message.add_account("To", receiver_account); - message.add_fee(fee, token_symbol.clone(), Intent::TransferFrom); + message.add_account("To", &receiver_account); + message.add_fee(Intent::TransferFrom, &fee, &token_symbol); } }; diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index 7b7f92d98607..e4dc785e5fd6 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -23,7 +23,7 @@ pub enum Intent { } impl ConsentMessage { - pub fn add_intent(&mut self, intent: Intent, token_symbol: String) { + pub fn add_intent(&mut self, intent: Intent, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => match intent { Intent::Transfer => { @@ -47,7 +47,7 @@ impl ConsentMessage { } } - pub fn add_account(&mut self, name: &str, account: Account) { + pub fn add_account(&mut self, name: &str, account: &Account) { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**{}:**\n`{}`", name, account)); @@ -55,7 +55,7 @@ impl ConsentMessage { } } - pub fn add_amount(&mut self, amount: String, token_symbol: String) { + pub fn add_amount(&mut self, amount: &String, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); @@ -63,7 +63,7 @@ impl ConsentMessage { } } - pub fn add_fee(&mut self, fee: String, token_symbol: String, intent: Intent) { + pub fn add_fee(&mut self, intent: Intent, fee: &String, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { match intent { @@ -80,7 +80,7 @@ impl ConsentMessage { } } - pub fn add_allowance(&mut self, amount: String, token_symbol: String) { + pub fn add_allowance(&mut self, amount: &String, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!( @@ -91,7 +91,7 @@ impl ConsentMessage { } } - pub fn add_existing_allowance(&mut self, expected_allowance: String, token_symbol: String) { + pub fn add_existing_allowance(&mut self, expected_allowance: &String, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", expected_allowance, token_symbol)); @@ -99,7 +99,7 @@ impl ConsentMessage { } } - pub fn add_expiration(&mut self, expires_at: String) { + pub fn add_expiration(&mut self, expires_at: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); From 3a05d018e6d591064db717f0c5a8974f0072bf2c Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 15:02:40 +0000 Subject: [PATCH 16/28] add fields display --- .../icrc-ledger-types/src/icrc21/responses.rs | 75 +++++++++++++++---- rs/ledger_suite/tests/sm-tests/src/lib.rs | 1 + 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index e4dc785e5fd6..8a5057efcc2d 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -5,9 +5,16 @@ use candid::{CandidType, Deserialize}; use serde::Serialize; use serde_bytes::ByteBuf; +#[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct FieldsDisplay { + pub intent: String, + pub fields: Vec<(String, String)>, +} + #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum ConsentMessage { GenericDisplayMessage(String), + FieldsDisplayMessage(FieldsDisplay), } #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -34,8 +41,8 @@ impl ConsentMessage { Intent::Approve => { message.push_str("# Approve spending"); message.push_str( - "\n\nYou are authorizing another address to withdraw funds from your account.", - ); + "\n\nYou are authorizing another address to withdraw funds from your account.", + ); } Intent::TransferFrom => { message.push_str(&format!("# Spend {}", token_symbol)); @@ -44,14 +51,28 @@ impl ConsentMessage { ); } }, + ConsentMessage::FieldsDisplayMessage(fields_display) => match intent { + Intent::Transfer => { + fields_display.intent = format!("Send {}", token_symbol); + } + Intent::Approve => { + fields_display.intent = "Approve spending".to_string(); + } + Intent::TransferFrom => { + fields_display.intent = format!("Spend {}", token_symbol); + } + }, } } pub fn add_account(&mut self, name: &str, account: &Account) { match self { ConsentMessage::GenericDisplayMessage(message) => { - message.push_str(&format!("\n\n**{}:**\n`{}`", name, account)); + message.push_str(&format!("\n\n**{}:**\n`{}`", name, account)) } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display + .fields + .push((name.to_string(), account.to_string())), } } @@ -60,6 +81,9 @@ impl ConsentMessage { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display + .fields + .push(("Amount".to_string(), format!("{} {}", amount, token_symbol))), } } @@ -77,6 +101,17 @@ impl ConsentMessage { )), }; } + ConsentMessage::FieldsDisplayMessage(fields_display) => { + match intent { + Intent::Approve => fields_display.fields.push(( + "Approval fees".to_string(), + format!("{} {}", fee, token_symbol), + )), + _ => fields_display + .fields + .push(("Fees".to_string(), format!("{} {}", fee, token_symbol))), + }; + } } } @@ -84,10 +119,14 @@ impl ConsentMessage { match self { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!( - "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", - amount, token_symbol - )); + "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", + amount, token_symbol + )); } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( + "Requested allowance".to_string(), + format!("{} {}", amount, token_symbol), + )), } } @@ -96,6 +135,10 @@ impl ConsentMessage { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", expected_allowance, token_symbol)); } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( + "Existing allowance".to_string(), + format!("{} {}", expected_allowance, token_symbol), + )), } } @@ -104,20 +147,24 @@ impl ConsentMessage { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display + .fields + .push(("Approval expiration".to_string(), expires_at.to_string())), } } pub fn add_memo(&mut self, memo: ByteBuf) { + // Check if the memo is a valid UTF-8 string and display it as such if it is. + let memo_str = match std::str::from_utf8(memo.as_slice()) { + Ok(valid_str) => valid_str.to_string(), + Err(_) => hex::encode(memo.as_slice()), + }; match self { ConsentMessage::GenericDisplayMessage(message) => { - message.push_str(&format!( - "\n\n**Memo:**\n`{}`", - // Check if the memo is a valid UTF-8 string and display it as such if it is. - &match std::str::from_utf8(memo.as_slice()) { - Ok(valid_str) => valid_str.to_string(), - Err(_) => hex::encode(memo.as_slice()), - } - )); + message.push_str(&format!("\n\n**Memo:**\n`{}`", memo_str)); + } + ConsentMessage::FieldsDisplayMessage(fields_display) => { + fields_display.fields.push(("Memo".to_string(), memo_str)) } } } diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 0f56ebf8d768..1edb5e951551 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4981,6 +4981,7 @@ Charged for processing the transfer. fn extract_icrc21_message_string(consent_message: &ConsentMessage) -> String { match consent_message { ConsentMessage::GenericDisplayMessage(message) => message.to_string(), + ConsentMessage::FieldsDisplayMessage(_) => panic!("cannot convert to string"), } } From c756f4f229b0ca50eaae38c7f9c86cbea1883e39 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 15:17:01 +0000 Subject: [PATCH 17/28] fix icp ledger did file --- rs/ledger_suite/icp/ledger.did | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/rs/ledger_suite/icp/ledger.did b/rs/ledger_suite/icp/ledger.did index 7466333461c5..90677996581e 100644 --- a/rs/ledger_suite/icp/ledger.did +++ b/rs/ledger_suite/icp/ledger.did @@ -431,10 +431,7 @@ type icrc21_consent_message_spec = record { metadata: icrc21_consent_message_metadata; device_spec: opt variant { GenericDisplay; - LineDisplay: record { - characters_per_line: nat16; - lines_per_page: nat16; - }; + FieldsDisplay; }; }; @@ -444,13 +441,14 @@ type icrc21_consent_message_request = record { user_preferences: icrc21_consent_message_spec; }; +type FieldsDisplay = record { + intent: text; + fields: vec record { text; text }; +}; + type icrc21_consent_message = variant { GenericDisplayMessage: text; - LineDisplayMessage: record { - pages: vec record { - lines: vec text; - }; - }; + FieldsDisplayMessage: FieldsDisplay; }; type icrc21_consent_info = record { From 42688c5d65ce57277aa8dc52961616e3bf582481 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Wed, 18 Jun 2025 15:19:44 +0000 Subject: [PATCH 18/28] fix icrc ledger did file --- rs/ledger_suite/icrc1/ledger/ledger.did | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/rs/ledger_suite/icrc1/ledger/ledger.did b/rs/ledger_suite/icrc1/ledger/ledger.did index 7eaadf46a376..b1ed05aa9067 100644 --- a/rs/ledger_suite/icrc1/ledger/ledger.did +++ b/rs/ledger_suite/icrc1/ledger/ledger.did @@ -432,10 +432,7 @@ type icrc21_consent_message_spec = record { metadata: icrc21_consent_message_metadata; device_spec: opt variant { GenericDisplay; - LineDisplay: record { - characters_per_line: nat16; - lines_per_page: nat16; - }; + FieldsDisplay; }; }; @@ -445,13 +442,14 @@ type icrc21_consent_message_request = record { user_preferences: icrc21_consent_message_spec; }; +type FieldsDisplay = record { + intent: text; + fields: vec record { text; text }; +}; + type icrc21_consent_message = variant { GenericDisplayMessage: text; - LineDisplayMessage: record { - pages: vec record { - lines: vec text; - }; - }; + FieldsDisplayMessage: FieldsDisplay; }; type icrc21_consent_info = record { From 080771e69248688f970816e83e1b5faef962d406 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Thu, 19 Jun 2025 12:24:11 +0000 Subject: [PATCH 19/28] add fields display test --- packages/icrc-ledger-types/src/icrc21/lib.rs | 6 +- .../icrc-ledger-types/src/icrc21/responses.rs | 2 +- rs/ledger_suite/tests/sm-tests/src/lib.rs | 178 +++++++++++++++++- 3 files changed, 182 insertions(+), 4 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 7730e00d2770..0e165e8f34c1 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -135,9 +135,11 @@ impl ConsentMessageBuilder { pub fn build(self) -> Result { let mut message = match self.display_type { Some(DisplayMessageType::GenericDisplay) | None => { - ConsentMessage::GenericDisplayMessage("".to_string()) + ConsentMessage::GenericDisplayMessage(Default::default()) + } + Some(DisplayMessageType::FieldsDisplay) => { + ConsentMessage::FieldsDisplayMessage(Default::default()) } - Some(DisplayMessageType::FieldsDisplay) => todo!(), }; match self.function { Icrc21Function::Transfer => { diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index 8a5057efcc2d..aca06002f96d 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -5,7 +5,7 @@ use candid::{CandidType, Deserialize}; use serde::Serialize; use serde_bytes::ByteBuf; -#[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct FieldsDisplay { pub intent: String, pub fields: Vec<(String, String)>, diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 1edb5e951551..60a8c25d6689 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -43,7 +43,7 @@ use icrc_ledger_types::icrc21::requests::ConsentMessageMetadata; use icrc_ledger_types::icrc21::requests::{ ConsentMessageRequest, ConsentMessageSpec, DisplayMessageType, }; -use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage}; +use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDisplay}; use icrc_ledger_types::icrc3; use icrc_ledger_types::icrc3::archive::ArchiveInfo; use icrc_ledger_types::icrc3::blocks::{ @@ -4574,6 +4574,28 @@ pub fn test_icrc1_test_suite( } } +fn convert_to_fields_args(args: &ConsentMessageRequest) -> ConsentMessageRequest { + let mut fields_args = args.clone(); + fields_args.user_preferences.device_spec = Some(DisplayMessageType::FieldsDisplay); + fields_args +} + +fn modify_field(fields_message: &FieldsDisplay, field_name: String, new_value: String) -> FieldsDisplay { + let mut result: FieldsDisplay = Default::default(); + result.intent = fields_message.intent.clone(); + for (f_name, f_value) in &fields_message.fields { + if *f_name == field_name { + if new_value == "".to_string() { + continue; + } + result.fields.push((f_name.to_string(), new_value.clone())); + } else { + result.fields.push((f_name.to_string(), f_value.to_string())); + } + } + result +} + fn test_icrc21_transfer_message( env: &StateMachine, canister_id: CanisterId, @@ -4620,6 +4642,15 @@ Charged for processing the transfer. **Memo:** `test_bytes`"; + let expected_fields_message = FieldsDisplay { + intent: "Send XTST".to_string(), + fields: vec![ + ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), + ("Amount".to_string(), "0.01 XTST".to_string()), + ("To".to_string(), "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()), + ("Fees".to_string(), "0.0001 XTST".to_string()), + ("Memo".to_string(), "test_bytes".to_string())]}; + let consent_info = icrc21_consent_message(env, canister_id, from_account.owner, args.clone()).unwrap(); assert_eq!(consent_info.metadata.language, "en"); @@ -4633,6 +4664,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_transfer_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + assert_eq!( + fields_message, expected_fields_message, + "Expected: {:?}, got: {:?}", + expected_fields_message, fields_message + ); + // Make sure the accounts are formatted correctly. assert_eq!(from_account.to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101"); assert_eq!(receiver_account.to_string(), "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202"); @@ -4653,6 +4693,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // If the memo is not a valid UTF string, it should be hex encoded. args.arg = Encode!(&TransferArg { @@ -4672,6 +4721,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(),hex::encode(vec![0, 159, 146, 150])); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // If the from account is anonymous, the message should not include the account information. args.arg = Encode!(&transfer_args.clone()).unwrap(); @@ -4686,6 +4744,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "From".to_string(),"".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); } fn test_icrc21_approve_message( @@ -4760,6 +4827,18 @@ Charged for processing the approval. **Memo:** `test_bytes`"; + let expected_fields_message = FieldsDisplay { + intent: "Approve spending".to_string(), + fields: vec![ + ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), + ("Approve to spender".to_string(), "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()), + ("Requested allowance".to_string(), "0.01 XTST".to_string()), + ("Existing allowance".to_string(), "0.01 XTST".to_string()), + ("Approval expiration".to_string(), "Thu, 06 May 2021 20:17:10 +0000".to_string()), + ("Approval fees".to_string(), "0.0001 XTST".to_string()), + ("Fees paid by".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), + ("Memo".to_string(), "test_bytes".to_string())]}; + let mut args = ConsentMessageRequest { method: "icrc2_approve".to_owned(), arg: Encode!(&approve_args).unwrap(), @@ -4781,6 +4860,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_approve_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + assert_eq!( + fields_message, expected_fields_message, + "Expected: {:?}, got: {:?}", + expected_fields_message, fields_message + ); + args.arg = Encode!(&ApproveArgs { expected_allowance: None, ..approve_args.clone() @@ -4799,6 +4887,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Existing allowance".to_string(), "".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // Test approval without an expiration. args.arg = Encode!(&ApproveArgs { @@ -4821,6 +4918,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Approval expiration".to_string(), "This approval does not have an expiration.".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // If the approver is anonymous, the message should not include the approver information. args.arg = Encode!(&approve_args.clone()).unwrap(); @@ -4837,6 +4943,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "From".to_string(),"".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // If we set the offset to 1 hour the expiration date should be 1 hour ahead. args.user_preferences.metadata.utc_offset_minutes = Some(60); @@ -4854,6 +4969,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Approval expiration".to_string(), "Thu, 06 May 2021 21:17:10 +0100".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); args.user_preferences.metadata.utc_offset_minutes = None; // If memo is not specified it should not be included. @@ -4875,6 +4999,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); } fn test_icrc21_transfer_from_message( @@ -4928,6 +5061,16 @@ Charged for processing the transfer. **Memo:** `test_bytes`"; + let expected_fields_message = FieldsDisplay { + intent: "Spend XTST".to_string(), + fields: vec![ + ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), + ("Amount".to_string(), "0.01 XTST".to_string()), + ("Spender".to_string(), "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()), + ("To".to_string(), "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()), + ("Fees".to_string(), "0.0001 XTST".to_string()), + ("Memo".to_string(), "test_bytes".to_string())]}; + let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, spender_account.owner, args.clone()) .unwrap() @@ -4938,6 +5081,14 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_transfer_from_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, spender_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + assert_eq!( + fields_message, expected_fields_message, + "Expected: {:?}, got: {:?}", + expected_fields_message, fields_message + ); // If the spender is anonymous, the message should not include the spender account information. args.arg = Encode!(&transfer_from_args.clone()).unwrap(); @@ -4955,6 +5106,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Spender".to_string(), "".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); // If memo is not specified it should not be included. args.arg = Encode!(&TransferFromArgs { @@ -4976,6 +5136,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); + let fields_consent_info = + icrc21_consent_message(env, canister_id, spender_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); + let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + assert_eq!( + fields_message, new_exp_fields_message, + "Expected: {:?}, got: {:?}", + new_exp_fields_message, fields_message + ); } fn extract_icrc21_message_string(consent_message: &ConsentMessage) -> String { @@ -4985,6 +5154,13 @@ fn extract_icrc21_message_string(consent_message: &ConsentMessage) -> String { } } +fn extract_icrc21_fields_message(consent_message: &ConsentMessage) -> FieldsDisplay { + match consent_message { + ConsentMessage::GenericDisplayMessage(_) => panic!("should not be a string"), + ConsentMessage::FieldsDisplayMessage(message) => message.clone(), + } +} + pub fn test_icrc21_standard(ledger_wasm: Vec, encode_init_args: fn(InitArgs) -> T) where T: CandidType, From c0249f37ac6a947dc73c359d450f610454851d5b Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Thu, 19 Jun 2025 12:27:58 +0000 Subject: [PATCH 20/28] clippy --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 164 +++++++++++++++++----- 1 file changed, 126 insertions(+), 38 deletions(-) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 60a8c25d6689..b33883c810f2 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4580,7 +4580,11 @@ fn convert_to_fields_args(args: &ConsentMessageRequest) -> ConsentMessageRequest fields_args } -fn modify_field(fields_message: &FieldsDisplay, field_name: String, new_value: String) -> FieldsDisplay { +fn modify_field( + fields_message: &FieldsDisplay, + field_name: String, + new_value: String, +) -> FieldsDisplay { let mut result: FieldsDisplay = Default::default(); result.intent = fields_message.intent.clone(); for (f_name, f_value) in &fields_message.fields { @@ -4590,7 +4594,9 @@ fn modify_field(fields_message: &FieldsDisplay, field_name: String, new_value: S } result.fields.push((f_name.to_string(), new_value.clone())); } else { - result.fields.push((f_name.to_string(), f_value.to_string())); + result + .fields + .push((f_name.to_string(), f_value.to_string())); } } result @@ -4642,7 +4648,7 @@ Charged for processing the transfer. **Memo:** `test_bytes`"; - let expected_fields_message = FieldsDisplay { + let expected_fields_message = FieldsDisplay { intent: "Send XTST".to_string(), fields: vec![ ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), @@ -4664,8 +4670,13 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_transfer_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); assert_eq!( fields_message, expected_fields_message, @@ -4693,8 +4704,13 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); assert_eq!( @@ -4721,10 +4737,19 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(),hex::encode(vec![0, 159, 146, 150])); + let new_exp_fields_message = modify_field( + &fields_message, + "Memo".to_string(), + hex::encode(vec![0, 159, 146, 150]), + ); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4744,10 +4769,15 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + Principal::anonymous(), + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "From".to_string(),"".to_string()); + let new_exp_fields_message = modify_field(&fields_message, "From".to_string(), "".to_string()); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4827,7 +4857,7 @@ Charged for processing the approval. **Memo:** `test_bytes`"; - let expected_fields_message = FieldsDisplay { + let expected_fields_message = FieldsDisplay { intent: "Approve spending".to_string(), fields: vec![ ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), @@ -4860,8 +4890,13 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_approve_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); assert_eq!( fields_message, expected_fields_message, @@ -4887,10 +4922,19 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Existing allowance".to_string(), "".to_string()); + let new_exp_fields_message = modify_field( + &fields_message, + "Existing allowance".to_string(), + "".to_string(), + ); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4918,10 +4962,19 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Approval expiration".to_string(), "This approval does not have an expiration.".to_string()); + let new_exp_fields_message = modify_field( + &fields_message, + "Approval expiration".to_string(), + "This approval does not have an expiration.".to_string(), + ); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4943,10 +4996,15 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + Principal::anonymous(), + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "From".to_string(),"".to_string()); + let new_exp_fields_message = modify_field(&fields_message, "From".to_string(), "".to_string()); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4969,10 +5027,19 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Approval expiration".to_string(), "Thu, 06 May 2021 21:17:10 +0100".to_string()); + let new_exp_fields_message = modify_field( + &fields_message, + "Approval expiration".to_string(), + "Thu, 06 May 2021 21:17:10 +0100".to_string(), + ); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4999,8 +5066,13 @@ Charged for processing the approval. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, from_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + from_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); assert_eq!( @@ -5061,7 +5133,7 @@ Charged for processing the transfer. **Memo:** `test_bytes`"; - let expected_fields_message = FieldsDisplay { + let expected_fields_message = FieldsDisplay { intent: "Spend XTST".to_string(), fields: vec![ ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), @@ -5081,8 +5153,13 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_transfer_from_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, spender_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + spender_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); assert_eq!( fields_message, expected_fields_message, @@ -5106,10 +5183,16 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, Principal::anonymous(), convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + Principal::anonymous(), + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Spender".to_string(), "".to_string()); + let new_exp_fields_message = + modify_field(&fields_message, "Spender".to_string(), "".to_string()); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -5136,8 +5219,13 @@ Charged for processing the transfer. "Expected: {}, got: {}", expected_message, message ); - let fields_consent_info = - icrc21_consent_message(env, canister_id, spender_account.owner, convert_to_fields_args(&args)).unwrap(); + let fields_consent_info = icrc21_consent_message( + env, + canister_id, + spender_account.owner, + convert_to_fields_args(&args), + ) + .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); assert_eq!( From da6d2e3b6dde4cc7ef81eb806cf35ab3a5528ee3 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Thu, 19 Jun 2025 12:39:43 +0000 Subject: [PATCH 21/28] clippy --- rs/ledger_suite/tests/sm-tests/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index da4a4d1401dc..afcb3f222f46 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4615,11 +4615,13 @@ fn modify_field( field_name: String, new_value: String, ) -> FieldsDisplay { - let mut result: FieldsDisplay = Default::default(); - result.intent = fields_message.intent.clone(); + let mut result = FieldsDisplay { + intent: fields_message.intent.clone(), + ..Default::default() + }; for (f_name, f_value) in &fields_message.fields { if *f_name == field_name { - if new_value == "".to_string() { + if new_value == *"" { continue; } result.fields.push((f_name.to_string(), new_value.clone())); From c1b506ce9dcf962b400375f8776274e9ae89fc10 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 23 Jun 2025 11:59:36 +0000 Subject: [PATCH 22/28] use token name in the title --- packages/icrc-ledger-types/src/icrc21/lib.rs | 25 ++++++++++++++++--- .../icrc-ledger-types/src/icrc21/responses.rs | 14 +++++++---- rs/ledger_suite/icp/ledger/src/main.rs | 2 ++ rs/ledger_suite/icrc1/ledger/src/main.rs | 2 ++ rs/ledger_suite/tests/sm-tests/src/lib.rs | 8 +++--- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 0e165e8f34c1..8fcb6fff85c6 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -38,6 +38,7 @@ pub struct ConsentMessageBuilder { receiver: Option, amount: Option, token_symbol: Option, + token_name: Option, ledger_fee: Option, memo: Option, expected_allowance: Option, @@ -63,6 +64,7 @@ impl ConsentMessageBuilder { receiver: None, amount: None, token_symbol: None, + token_name: None, ledger_fee: None, utc_offset_minutes: None, memo: None, @@ -102,6 +104,11 @@ impl ConsentMessageBuilder { self } + pub fn with_token_name(mut self, token_name: String) -> Self { + self.token_name = Some(token_name); + self + } + pub fn with_ledger_fee(mut self, ledger_fee: Nat) -> Self { self.ledger_fee = Some(ledger_fee); self @@ -162,6 +169,10 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token Symbol must be specified.".to_owned(), })?; + let token_name = self.token_name.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token Name must be specified.".to_owned(), + })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -170,7 +181,7 @@ impl ConsentMessageBuilder { self.decimals, )?; - message.add_intent(Intent::Transfer, &token_symbol); + message.add_intent(Intent::Transfer, Some(token_name)); if from_account.owner != Principal::anonymous() { message.add_account("From", &from_account); } @@ -237,7 +248,7 @@ impl ConsentMessageBuilder { }) .unwrap_or("This approval does not have an expiration.".to_owned()); - message.add_intent(Intent::Approve, &token_symbol); + message.add_intent(Intent::Approve, None); if approver_account.owner != Principal::anonymous() { message.add_account("From", &approver_account); } @@ -283,6 +294,10 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token symbol must be specified.".to_owned(), })?; + let token_name = self.token_name.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Token Name must be specified.".to_owned(), + })?; let amount = convert_tokens_to_string_representation( self.amount.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -290,7 +305,7 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - message.add_intent(Intent::TransferFrom, &token_symbol); + message.add_intent(Intent::TransferFrom, Some(token_name)); message.add_account("From", &from_account); message.add_amount(&amount, &token_symbol); if spender_account.owner != Principal::anonymous() { @@ -314,6 +329,7 @@ pub fn build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( caller_principal: Principal, ledger_fee: Nat, token_symbol: String, + token_name: String, decimals: u8, ) -> Result { if consent_msg_request.arg.len() > MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES as usize { @@ -337,7 +353,8 @@ pub fn build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( let mut display_message_builder = ConsentMessageBuilder::new(&consent_msg_request.method, decimals)? .with_ledger_fee(ledger_fee) - .with_token_symbol(token_symbol); + .with_token_symbol(token_symbol) + .with_token_name(token_name); if let Some(offset) = consent_msg_request .user_preferences diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index aca06002f96d..d0cf50081282 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -30,11 +30,12 @@ pub enum Intent { } impl ConsentMessage { - pub fn add_intent(&mut self, intent: Intent, token_symbol: &String) { + pub fn add_intent(&mut self, intent: Intent, token_name: Option) { match self { ConsentMessage::GenericDisplayMessage(message) => match intent { Intent::Transfer => { - message.push_str(&format!("# Send {}", token_symbol)); + assert!(token_name.is_some()); + message.push_str(&format!("# Send {}", token_name.unwrap())); message .push_str("\n\nYou are approving a transfer of funds from your account."); } @@ -45,7 +46,8 @@ impl ConsentMessage { ); } Intent::TransferFrom => { - message.push_str(&format!("# Spend {}", token_symbol)); + assert!(token_name.is_some()); + message.push_str(&format!("# Spend {}", token_name.unwrap())); message.push_str( "\n\nYou are approving a transfer of funds from a withdrawal account.", ); @@ -53,13 +55,15 @@ impl ConsentMessage { }, ConsentMessage::FieldsDisplayMessage(fields_display) => match intent { Intent::Transfer => { - fields_display.intent = format!("Send {}", token_symbol); + assert!(token_name.is_some()); + fields_display.intent = format!("Send {}", token_name.unwrap()); } Intent::Approve => { fields_display.intent = "Approve spending".to_string(); } Intent::TransferFrom => { - fields_display.intent = format!("Spend {}", token_symbol); + assert!(token_name.is_some()); + fields_display.intent = format!("Spend {}", token_name.unwrap()); } }, } diff --git a/rs/ledger_suite/icp/ledger/src/main.rs b/rs/ledger_suite/icp/ledger/src/main.rs index 17a418e1ac37..426a15c82258 100644 --- a/rs/ledger_suite/icp/ledger/src/main.rs +++ b/rs/ledger_suite/icp/ledger/src/main.rs @@ -1654,6 +1654,7 @@ fn icrc21_canister_call_consent_message( let caller_principal = caller(); let ledger_fee = Nat::from(LEDGER.read().unwrap().transfer_fee.get_e8s()); let token_symbol = LEDGER.read().unwrap().token_symbol.clone(); + let token_name = LEDGER.read().unwrap().token_symbol.clone(); let decimals = ic_ledger_core::tokens::DECIMAL_PLACES as u8; build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( @@ -1661,6 +1662,7 @@ fn icrc21_canister_call_consent_message( caller_principal, ledger_fee, token_symbol, + token_name, decimals, ) } diff --git a/rs/ledger_suite/icrc1/ledger/src/main.rs b/rs/ledger_suite/icrc1/ledger/src/main.rs index 13e050ee5141..1febd3dda337 100644 --- a/rs/ledger_suite/icrc1/ledger/src/main.rs +++ b/rs/ledger_suite/icrc1/ledger/src/main.rs @@ -1437,6 +1437,7 @@ fn icrc21_canister_call_consent_message( let caller_principal = ic_cdk::api::caller(); let ledger_fee = icrc1_fee(); let token_symbol = icrc1_symbol(); + let token_name = icrc1_name(); let decimals = icrc1_decimals(); build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( @@ -1444,6 +1445,7 @@ fn icrc21_canister_call_consent_message( caller_principal, ledger_fee, token_symbol, + token_name, decimals, ) } diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index afcb3f222f46..2c599fc94e2a 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -4662,7 +4662,7 @@ fn test_icrc21_transfer_message( }, }; - let expected_transfer_message = "# Send XTST + let expected_transfer_message = "# Send Test Token You are approving a transfer of funds from your account. @@ -4681,7 +4681,7 @@ Charged for processing the transfer. `test_bytes`"; let expected_fields_message = FieldsDisplay { - intent: "Send XTST".to_string(), + intent: "Send Test Token".to_string(), fields: vec![ ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), ("Amount".to_string(), "0.01 XTST".to_string()), @@ -5144,7 +5144,7 @@ fn test_icrc21_transfer_from_message( }, }; - let expected_transfer_from_message = "# Spend XTST + let expected_transfer_from_message = "# Spend Test Token You are approving a transfer of funds from a withdrawal account. @@ -5166,7 +5166,7 @@ Charged for processing the transfer. `test_bytes`"; let expected_fields_message = FieldsDisplay { - intent: "Spend XTST".to_string(), + intent: "Spend Test Token".to_string(), fields: vec![ ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), ("Amount".to_string(), "0.01 XTST".to_string()), From 6ebd49a6cedcd594747d08722f39209f8af5458e Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 23 Jun 2025 13:22:52 +0000 Subject: [PATCH 23/28] fix token name in icp --- rs/ledger_suite/icp/ledger/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/ledger_suite/icp/ledger/src/main.rs b/rs/ledger_suite/icp/ledger/src/main.rs index 426a15c82258..a9c4a9cc5612 100644 --- a/rs/ledger_suite/icp/ledger/src/main.rs +++ b/rs/ledger_suite/icp/ledger/src/main.rs @@ -1654,7 +1654,7 @@ fn icrc21_canister_call_consent_message( let caller_principal = caller(); let ledger_fee = Nat::from(LEDGER.read().unwrap().transfer_fee.get_e8s()); let token_symbol = LEDGER.read().unwrap().token_symbol.clone(); - let token_name = LEDGER.read().unwrap().token_symbol.clone(); + let token_name = LEDGER.read().unwrap().token_name.clone(); let decimals = ic_ledger_core::tokens::DECIMAL_PLACES as u8; build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( From eaa0b08d424495f1db5d4a36965a94ce6a549d50 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 23 Jun 2025 14:09:01 +0000 Subject: [PATCH 24/28] list intents explicitly --- packages/icrc-ledger-types/src/icrc21/responses.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index d0cf50081282..ac2b8d1588b2 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -99,7 +99,7 @@ impl ConsentMessage { "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", fee, token_symbol )), - _ => message.push_str(&format!( + Intent::Transfer | Intent::TransferFrom => message.push_str(&format!( "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", fee, token_symbol )), @@ -111,7 +111,7 @@ impl ConsentMessage { "Approval fees".to_string(), format!("{} {}", fee, token_symbol), )), - _ => fields_display + Intent::Transfer | Intent::TransferFrom => fields_display .fields .push(("Fees".to_string(), format!("{} {}", fee, token_symbol))), }; From 5cdf205662295741da3d67fe8fdf643b66b9da7d Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 23 Jun 2025 14:23:07 +0000 Subject: [PATCH 25/28] use Icrc21Function instead of Intent --- packages/icrc-ledger-types/src/icrc21/lib.rs | 15 ++++----- .../icrc-ledger-types/src/icrc21/responses.rs | 32 ++++++++----------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index 8fcb6fff85c6..dcb70e548bf7 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -8,7 +8,6 @@ use crate::icrc2::approve::ApproveArgs; use crate::icrc2::transfer_from::TransferFromArgs; use crate::icrc21::errors::Icrc21Error; use crate::icrc21::requests::ConsentMessageMetadata; -use crate::icrc21::responses::Intent; use candid::Decode; use candid::{Nat, Principal}; use num_traits::{Pow, ToPrimitive}; @@ -20,7 +19,7 @@ use strum_macros::{Display, EnumIter, EnumString}; pub const MAX_CONSENT_MESSAGE_ARG_SIZE_BYTES: u16 = 500; #[derive(Debug, EnumString, EnumIter, Display)] -enum Icrc21Function { +pub enum Icrc21Function { #[strum(serialize = "icrc1_transfer")] Transfer, #[strum(serialize = "icrc2_approve")] @@ -181,13 +180,13 @@ impl ConsentMessageBuilder { self.decimals, )?; - message.add_intent(Intent::Transfer, Some(token_name)); + message.add_intent(Icrc21Function::Transfer, Some(token_name)); if from_account.owner != Principal::anonymous() { message.add_account("From", &from_account); } message.add_amount(&amount, &token_symbol); message.add_account("To", &receiver_account); - message.add_fee(Intent::Transfer, &fee, &token_symbol); + message.add_fee(Icrc21Function::Transfer, &fee, &token_symbol); } Icrc21Function::Approve => { let approver_account = self.approver.ok_or(Icrc21Error::GenericError { @@ -248,7 +247,7 @@ impl ConsentMessageBuilder { }) .unwrap_or("This approval does not have an expiration.".to_owned()); - message.add_intent(Intent::Approve, None); + message.add_intent(Icrc21Function::Approve, None); if approver_account.owner != Principal::anonymous() { message.add_account("From", &approver_account); } @@ -264,7 +263,7 @@ impl ConsentMessageBuilder { ); } message.add_expiration(&expires_at); - message.add_fee(Intent::Approve, &fee, &token_symbol); + message.add_fee(Icrc21Function::Approve, &fee, &token_symbol); if approver_account.owner != Principal::anonymous() { message.add_account("Fees paid by", &approver_account); } @@ -305,14 +304,14 @@ impl ConsentMessageBuilder { })?, self.decimals, )?; - message.add_intent(Intent::TransferFrom, Some(token_name)); + message.add_intent(Icrc21Function::TransferFrom, Some(token_name)); message.add_account("From", &from_account); message.add_amount(&amount, &token_symbol); if spender_account.owner != Principal::anonymous() { message.add_account("Spender", &spender_account); } message.add_account("To", &receiver_account); - message.add_fee(Intent::TransferFrom, &fee, &token_symbol); + message.add_fee(Icrc21Function::TransferFrom, &fee, &token_symbol); } }; diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index ac2b8d1588b2..89c2688806aa 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -1,4 +1,4 @@ -use crate::icrc1::account::Account; +use crate::{icrc1::account::Account, icrc21::lib::Icrc21Function}; use super::requests::ConsentMessageMetadata; use candid::{CandidType, Deserialize}; @@ -23,29 +23,23 @@ pub struct ConsentInfo { pub metadata: ConsentMessageMetadata, } -pub enum Intent { - Transfer, - Approve, - TransferFrom, -} - impl ConsentMessage { - pub fn add_intent(&mut self, intent: Intent, token_name: Option) { + pub fn add_intent(&mut self, intent: Icrc21Function, token_name: Option) { match self { ConsentMessage::GenericDisplayMessage(message) => match intent { - Intent::Transfer => { + Icrc21Function::Transfer => { assert!(token_name.is_some()); message.push_str(&format!("# Send {}", token_name.unwrap())); message .push_str("\n\nYou are approving a transfer of funds from your account."); } - Intent::Approve => { + Icrc21Function::Approve => { message.push_str("# Approve spending"); message.push_str( "\n\nYou are authorizing another address to withdraw funds from your account.", ); } - Intent::TransferFrom => { + Icrc21Function::TransferFrom => { assert!(token_name.is_some()); message.push_str(&format!("# Spend {}", token_name.unwrap())); message.push_str( @@ -54,14 +48,14 @@ impl ConsentMessage { } }, ConsentMessage::FieldsDisplayMessage(fields_display) => match intent { - Intent::Transfer => { + Icrc21Function::Transfer => { assert!(token_name.is_some()); fields_display.intent = format!("Send {}", token_name.unwrap()); } - Intent::Approve => { + Icrc21Function::Approve => { fields_display.intent = "Approve spending".to_string(); } - Intent::TransferFrom => { + Icrc21Function::TransferFrom => { assert!(token_name.is_some()); fields_display.intent = format!("Spend {}", token_name.unwrap()); } @@ -91,15 +85,15 @@ impl ConsentMessage { } } - pub fn add_fee(&mut self, intent: Intent, fee: &String, token_symbol: &String) { + pub fn add_fee(&mut self, intent: Icrc21Function, fee: &String, token_symbol: &String) { match self { ConsentMessage::GenericDisplayMessage(message) => { match intent { - Intent::Approve => message.push_str(&format!( + Icrc21Function::Approve => message.push_str(&format!( "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", fee, token_symbol )), - Intent::Transfer | Intent::TransferFrom => message.push_str(&format!( + Icrc21Function::Transfer | Icrc21Function::TransferFrom => message.push_str(&format!( "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", fee, token_symbol )), @@ -107,11 +101,11 @@ impl ConsentMessage { } ConsentMessage::FieldsDisplayMessage(fields_display) => { match intent { - Intent::Approve => fields_display.fields.push(( + Icrc21Function::Approve => fields_display.fields.push(( "Approval fees".to_string(), format!("{} {}", fee, token_symbol), )), - Intent::Transfer | Intent::TransferFrom => fields_display + Icrc21Function::Transfer | Icrc21Function::TransferFrom => fields_display .fields .push(("Fees".to_string(), format!("{} {}", fee, token_symbol))), }; From 55eaaf68e5ac1c572e89e61433a6ac2a1df2b2f5 Mon Sep 17 00:00:00 2001 From: Maciej Modelski Date: Mon, 23 Jun 2025 14:32:19 +0000 Subject: [PATCH 26/28] clippy --- packages/icrc-ledger-types/src/icrc21/responses.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index 89c2688806aa..c852f264bea1 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -93,10 +93,12 @@ impl ConsentMessage { "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", fee, token_symbol )), - Icrc21Function::Transfer | Icrc21Function::TransferFrom => message.push_str(&format!( - "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", - fee, token_symbol - )), + Icrc21Function::Transfer | Icrc21Function::TransferFrom => { + message.push_str(&format!( + "\n\n**Fees:** `{} {}`\nCharged for processing the transfer.", + fee, token_symbol + )) + } }; } ConsentMessage::FieldsDisplayMessage(fields_display) => { From eff50506759a0e618f6d08d9c777e9f25b6e1f8f Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 5 Aug 2025 13:01:03 +0000 Subject: [PATCH 27/28] use new value type for fields display --- packages/icrc-ledger-types/src/icrc21/lib.rs | 123 +++-------- .../icrc-ledger-types/src/icrc21/responses.rs | 202 +++++++++++++++--- rs/ledger_suite/tests/sm-tests/src/lib.rs | 95 ++++---- 3 files changed, 250 insertions(+), 170 deletions(-) diff --git a/packages/icrc-ledger-types/src/icrc21/lib.rs b/packages/icrc-ledger-types/src/icrc21/lib.rs index dcb70e548bf7..b5bca185146e 100644 --- a/packages/icrc-ledger-types/src/icrc21/lib.rs +++ b/packages/icrc-ledger-types/src/icrc21/lib.rs @@ -10,7 +10,6 @@ use crate::icrc21::errors::Icrc21Error; use crate::icrc21::requests::ConsentMessageMetadata; use candid::Decode; use candid::{Nat, Principal}; -use num_traits::{Pow, ToPrimitive}; use serde_bytes::ByteBuf; use strum::{self, IntoEnumIterator}; use strum_macros::{Display, EnumIter, EnumString}; @@ -157,13 +156,7 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Receiver account has to be specified.".to_owned(), })?; - let fee = convert_tokens_to_string_representation( - self.ledger_fee.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Ledger fee must be specified.".to_owned(), - })?, - self.decimals, - )?; + let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "Token Symbol must be specified.".to_owned(), @@ -172,21 +165,19 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token Name must be specified.".to_owned(), })?; - let amount = convert_tokens_to_string_representation( - self.amount.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Amount has to be specified.".to_owned(), - })?, - self.decimals, - )?; message.add_intent(Icrc21Function::Transfer, Some(token_name)); if from_account.owner != Principal::anonymous() { message.add_account("From", &from_account); } - message.add_amount(&amount, &token_symbol); + message.add_amount(self.amount, self.decimals, &token_symbol)?; message.add_account("To", &receiver_account); - message.add_fee(Icrc21Function::Transfer, &fee, &token_symbol); + message.add_fee( + Icrc21Function::Transfer, + self.ledger_fee, + self.decimals, + &token_symbol, + )?; } Icrc21Function::Approve => { let approver_account = self.approver.ok_or(Icrc21Error::GenericError { @@ -197,73 +188,31 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Spender account has to be specified.".to_owned(), })?; - let fee = convert_tokens_to_string_representation( - self.ledger_fee.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Ledger fee must be specified.".to_owned(), - })?, - self.decimals, - )?; let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), description: "Token symbol must be specified.".to_owned(), })?; - let amount = convert_tokens_to_string_representation( - self.amount.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Amount has to be specified.".to_owned(), - })?, - self.decimals, - )?; - let expires_at = self - .expires_at - .map(|ts| { - let seconds = (ts as i64) / 10_i64.pow(9); - let nanos = ((ts as i64) % 10_i64.pow(9)) as u32; - - let utc_dt = match (match time::OffsetDateTime::from_unix_timestamp(seconds) - { - Ok(dt) => dt, - Err(_) => return format!("Invalid timestamp: {}", ts), - }) - .replace_nanosecond(nanos) - { - Ok(dt) => dt, - Err(_) => return format!("Invalid nanosecond: {}", nanos), - }; - - // Apply the offset minutes - let offset = time::UtcOffset::from_whole_seconds( - (self.utc_offset_minutes.unwrap_or(0) * 60).into(), - ) - .expect("Invalid offset"); - let offset_dt = utc_dt.to_offset(offset); - - // Format as a string including the offset - match offset_dt.format(&time::format_description::well_known::Rfc2822) { - Ok(formatted) => formatted, - Err(_) => format!("Invalid timestamp: {}", ts), - } - }) - .unwrap_or("This approval does not have an expiration.".to_owned()); message.add_intent(Icrc21Function::Approve, None); if approver_account.owner != Principal::anonymous() { message.add_account("From", &approver_account); } message.add_account("Approve to spender", &spender_account); - message.add_allowance(&amount, &token_symbol); + message.add_allowance(self.amount, self.decimals, &token_symbol)?; if let Some(expected_allowance) = self.expected_allowance { message.add_existing_allowance( - &convert_tokens_to_string_representation( - expected_allowance, - self.decimals, - )?, + expected_allowance, + self.decimals, &token_symbol, - ); + )?; } - message.add_expiration(&expires_at); - message.add_fee(Icrc21Function::Approve, &fee, &token_symbol); + message.add_expiration(self.expires_at, self.utc_offset_minutes); + message.add_fee( + Icrc21Function::Approve, + self.ledger_fee, + self.decimals, + &token_symbol, + )?; if approver_account.owner != Principal::anonymous() { message.add_account("Fees paid by", &approver_account); } @@ -281,13 +230,6 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Spender account has to be specified.".to_owned(), })?; - let fee = convert_tokens_to_string_representation( - self.ledger_fee.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Ledger fee must be specified.".to_owned(), - })?, - self.decimals, - )?; let token_symbol = self.token_symbol.ok_or(Icrc21Error::GenericError { error_code: Nat::from(500u64), @@ -297,21 +239,19 @@ impl ConsentMessageBuilder { error_code: Nat::from(500u64), description: "Token Name must be specified.".to_owned(), })?; - let amount = convert_tokens_to_string_representation( - self.amount.ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Amount has to be specified.".to_owned(), - })?, - self.decimals, - )?; message.add_intent(Icrc21Function::TransferFrom, Some(token_name)); message.add_account("From", &from_account); - message.add_amount(&amount, &token_symbol); + message.add_amount(self.amount, self.decimals, &token_symbol)?; if spender_account.owner != Principal::anonymous() { message.add_account("Spender", &spender_account); } message.add_account("To", &receiver_account); - message.add_fee(Icrc21Function::TransferFrom, &fee, &token_symbol); + message.add_fee( + Icrc21Function::TransferFrom, + self.ledger_fee, + self.decimals, + &token_symbol, + )?; } }; @@ -471,14 +411,3 @@ pub fn build_icrc21_consent_info_for_icrc1_and_icrc2_endpoints( consent_message, }) } - -fn convert_tokens_to_string_representation( - tokens: Nat, - decimals: u8, -) -> Result { - let tokens = tokens.0.to_f64().ok_or(Icrc21Error::GenericError { - error_code: Nat::from(500u64), - description: "Failed to convert tokens to u64".to_owned(), - })?; - Ok(format!("{}", tokens / 10_f64.pow(decimals))) -} diff --git a/packages/icrc-ledger-types/src/icrc21/responses.rs b/packages/icrc-ledger-types/src/icrc21/responses.rs index c852f264bea1..fe5ceb38baef 100644 --- a/packages/icrc-ledger-types/src/icrc21/responses.rs +++ b/packages/icrc-ledger-types/src/icrc21/responses.rs @@ -1,14 +1,36 @@ -use crate::{icrc1::account::Account, icrc21::lib::Icrc21Function}; +use crate::{ + icrc1::account::Account, + icrc21::{errors::Icrc21Error, lib::Icrc21Function}, +}; use super::requests::ConsentMessageMetadata; -use candid::{CandidType, Deserialize}; +use candid::{CandidType, Deserialize, Nat}; +use num_traits::{Pow, ToPrimitive}; use serde::Serialize; use serde_bytes::ByteBuf; +#[derive(CandidType, Deserialize, Eq, PartialEq, Debug, Serialize, Clone)] +pub enum Value { + TokenAmount { + decimals: u8, + amount: u64, + symbol: String, + }, + TimestampSeconds { + amount: u64, + }, + DurationSeconds { + amount: u64, + }, + Text { + content: String, + }, +} + #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] pub struct FieldsDisplay { pub intent: String, - pub fields: Vec<(String, String)>, + pub fields: Vec<(String, Value)>, } #[derive(Debug, CandidType, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -68,26 +90,56 @@ impl ConsentMessage { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**{}:**\n`{}`", name, account)) } - ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display - .fields - .push((name.to_string(), account.to_string())), + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( + name.to_string(), + Value::Text { + content: account.to_string(), + }, + )), } } - pub fn add_amount(&mut self, amount: &String, token_symbol: &String) { + pub fn add_amount( + &mut self, + amount: Option, + decimals: u8, + token_symbol: &String, + ) -> Result<(), Icrc21Error> { + let amount = amount.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Amount has to be specified.".to_owned(), + })?; match self { ConsentMessage::GenericDisplayMessage(message) => { + let amount = convert_tokens_to_string_representation(amount, decimals)?; message.push_str(&format!("\n\n**Amount:** `{} {}`", amount, token_symbol)); } - ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display - .fields - .push(("Amount".to_string(), format!("{} {}", amount, token_symbol))), + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( + "Amount".to_string(), + Value::TokenAmount { + decimals, + amount: nat_to_u64(amount)?, + symbol: token_symbol.to_string(), + }, + )), } + Ok(()) } - pub fn add_fee(&mut self, intent: Icrc21Function, fee: &String, token_symbol: &String) { + pub fn add_fee( + &mut self, + intent: Icrc21Function, + amount: Option, + decimals: u8, + token_symbol: &String, + ) -> Result<(), Icrc21Error> { + let amount = amount.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Amount has to be specified.".to_owned(), + })?; match self { ConsentMessage::GenericDisplayMessage(message) => { + let fee = convert_tokens_to_string_representation(amount, decimals)?; match intent { Icrc21Function::Approve => message.push_str(&format!( "\n\n**Approval fees:** `{} {}`\nCharged for processing the approval.", @@ -102,22 +154,37 @@ impl ConsentMessage { }; } ConsentMessage::FieldsDisplayMessage(fields_display) => { + let token_amount = Value::TokenAmount { + decimals, + amount: nat_to_u64(amount)?, + symbol: token_symbol.to_string(), + }; match intent { - Icrc21Function::Approve => fields_display.fields.push(( - "Approval fees".to_string(), - format!("{} {}", fee, token_symbol), - )), + Icrc21Function::Approve => fields_display + .fields + .push(("Approval fees".to_string(), token_amount)), Icrc21Function::Transfer | Icrc21Function::TransferFrom => fields_display .fields - .push(("Fees".to_string(), format!("{} {}", fee, token_symbol))), + .push(("Fees".to_string(), token_amount)), }; } } + Ok(()) } - pub fn add_allowance(&mut self, amount: &String, token_symbol: &String) { + pub fn add_allowance( + &mut self, + amount: Option, + decimals: u8, + token_symbol: &String, + ) -> Result<(), Icrc21Error> { + let amount = amount.ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Amount has to be specified.".to_owned(), + })?; match self { ConsentMessage::GenericDisplayMessage(message) => { + let amount = convert_tokens_to_string_representation(amount, decimals)?; message.push_str(&format!( "\n\n**Requested allowance:** `{} {}`\nThis is the withdrawal limit that will apply upon approval.", amount, token_symbol @@ -125,31 +192,94 @@ impl ConsentMessage { } ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( "Requested allowance".to_string(), - format!("{} {}", amount, token_symbol), + Value::TokenAmount { + decimals, + amount: nat_to_u64(amount)?, + symbol: token_symbol.to_string(), + }, )), } + Ok(()) } - pub fn add_existing_allowance(&mut self, expected_allowance: &String, token_symbol: &String) { + pub fn add_existing_allowance( + &mut self, + expected_allowance: Nat, + decimals: u8, + token_symbol: &String, + ) -> Result<(), Icrc21Error> { match self { ConsentMessage::GenericDisplayMessage(message) => { + let expected_allowance = + convert_tokens_to_string_representation(expected_allowance, decimals)?; message.push_str(&format!("\n\n**Existing allowance:** `{} {}`\nUntil approval, this allowance remains in effect.", expected_allowance, token_symbol)); } ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display.fields.push(( "Existing allowance".to_string(), - format!("{} {}", expected_allowance, token_symbol), + Value::TokenAmount { + decimals, + amount: nat_to_u64(expected_allowance)?, + symbol: token_symbol.to_string(), + }, )), } + Ok(()) } - pub fn add_expiration(&mut self, expires_at: &String) { + pub fn add_expiration(&mut self, expires_at: Option, utc_offset_minutes: Option) { match self { ConsentMessage::GenericDisplayMessage(message) => { + let expires_at = expires_at + .map(|ts| { + let seconds = (ts as i64) / 10_i64.pow(9); + let nanos = ((ts as i64) % 10_i64.pow(9)) as u32; + + let utc_dt = match (match time::OffsetDateTime::from_unix_timestamp(seconds) + { + Ok(dt) => dt, + Err(_) => return format!("Invalid timestamp: {}", ts), + }) + .replace_nanosecond(nanos) + { + Ok(dt) => dt, + Err(_) => return format!("Invalid nanosecond: {}", nanos), + }; + + // Apply the offset minutes + let offset = time::UtcOffset::from_whole_seconds( + (utc_offset_minutes.unwrap_or(0) * 60).into(), + ) + .expect("Invalid offset"); + let offset_dt = utc_dt.to_offset(offset); + + // Format as a string including the offset + match offset_dt.format(&time::format_description::well_known::Rfc2822) { + Ok(formatted) => formatted, + Err(_) => format!("Invalid timestamp: {}", ts), + } + }) + .unwrap_or("This approval does not have an expiration.".to_owned()); message.push_str(&format!("\n\n**Approval expiration:**\n{}", expires_at)); } - ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display - .fields - .push(("Approval expiration".to_string(), expires_at.to_string())), + ConsentMessage::FieldsDisplayMessage(fields_display) => { + match expires_at { + Some(expires_at) => { + let seconds = (expires_at as i64) / 10_i64.pow(9); + fields_display.fields.push(( + "Approval expiration".to_string(), + Value::TimestampSeconds { + amount: seconds as u64, + }, + )) + } + None => fields_display.fields.push(( + "Approval expiration".to_string(), + Value::Text { + content: "This approval does not have an expiration.".to_string(), + }, + )), + }; + } } } @@ -163,9 +293,27 @@ impl ConsentMessage { ConsentMessage::GenericDisplayMessage(message) => { message.push_str(&format!("\n\n**Memo:**\n`{}`", memo_str)); } - ConsentMessage::FieldsDisplayMessage(fields_display) => { - fields_display.fields.push(("Memo".to_string(), memo_str)) - } + ConsentMessage::FieldsDisplayMessage(fields_display) => fields_display + .fields + .push(("Memo".to_string(), Value::Text { content: memo_str })), } } } + +fn convert_tokens_to_string_representation( + tokens: Nat, + decimals: u8, +) -> Result { + let tokens = tokens.0.to_f64().ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Failed to convert tokens to u64".to_owned(), + })?; + Ok(format!("{}", tokens / 10_f64.pow(decimals))) +} + +fn nat_to_u64(tokens: Nat) -> Result { + tokens.0.to_u64().ok_or(Icrc21Error::GenericError { + error_code: Nat::from(500u64), + description: "Failed to convert tokens to u64".to_owned(), + }) +} diff --git a/rs/ledger_suite/tests/sm-tests/src/lib.rs b/rs/ledger_suite/tests/sm-tests/src/lib.rs index 63f7155a9845..aaf61bbdea52 100644 --- a/rs/ledger_suite/tests/sm-tests/src/lib.rs +++ b/rs/ledger_suite/tests/sm-tests/src/lib.rs @@ -43,7 +43,9 @@ use icrc_ledger_types::icrc21::requests::ConsentMessageMetadata; use icrc_ledger_types::icrc21::requests::{ ConsentMessageRequest, ConsentMessageSpec, DisplayMessageType, }; -use icrc_ledger_types::icrc21::responses::{ConsentInfo, ConsentMessage, FieldsDisplay}; +use icrc_ledger_types::icrc21::responses::{ + ConsentInfo, ConsentMessage, FieldsDisplay, Value as Icrc21Value, +}; use icrc_ledger_types::icrc3; use icrc_ledger_types::icrc3::archive::ArchiveInfo; use icrc_ledger_types::icrc3::blocks::{ @@ -4626,7 +4628,7 @@ fn convert_to_fields_args(args: &ConsentMessageRequest) -> ConsentMessageRequest fn modify_field( fields_message: &FieldsDisplay, field_name: String, - new_value: String, + new_value: Option, ) -> FieldsDisplay { let mut result = FieldsDisplay { intent: fields_message.intent.clone(), @@ -4634,14 +4636,13 @@ fn modify_field( }; for (f_name, f_value) in &fields_message.fields { if *f_name == field_name { - if new_value == *"" { - continue; + if new_value.is_some() { + result + .fields + .push((f_name.to_string(), new_value.clone().unwrap().clone())); } - result.fields.push((f_name.to_string(), new_value.clone())); } else { - result - .fields - .push((f_name.to_string(), f_value.to_string())); + result.fields.push((f_name.to_string(), f_value.clone())); } } result @@ -4694,13 +4695,14 @@ Charged for processing the transfer. `test_bytes`"; let expected_fields_message = FieldsDisplay { - intent: "Send Test Token".to_string(), + intent: "Send Test Token".to_string(), fields: vec![ - ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), - ("Amount".to_string(), "0.01 XTST".to_string()), - ("To".to_string(), "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()), - ("Fees".to_string(), "0.0001 XTST".to_string()), - ("Memo".to_string(), "test_bytes".to_string())]}; + ("From".to_string(), Icrc21Value::Text{content: "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()}), + ("Amount".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 1000000, symbol: "XTST".to_string()}), // "0.01 XTST".to_string()), + ("To".to_string(), Icrc21Value::Text{content: "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()}), + ("Fees".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 10000, symbol: "XTST".to_string()}), + ("Memo".to_string(), Icrc21Value::Text{content: "test_bytes".to_string()})], + }; let consent_info = icrc21_consent_message(env, canister_id, from_account.owner, args.clone()).unwrap(); @@ -4757,7 +4759,7 @@ Charged for processing the transfer. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + let new_exp_fields_message = modify_field(&expected_fields_message, "Memo".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4791,9 +4793,11 @@ Charged for processing the transfer. .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field( - &fields_message, + &expected_fields_message, "Memo".to_string(), - hex::encode(vec![0, 159, 146, 150]), + Some(Icrc21Value::Text { + content: hex::encode(vec![0, 159, 146, 150]), + }), ); assert_eq!( fields_message, new_exp_fields_message, @@ -4822,7 +4826,7 @@ Charged for processing the transfer. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "From".to_string(), "".to_string()); + let new_exp_fields_message = modify_field(&expected_fields_message, "From".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -4905,14 +4909,14 @@ Charged for processing the approval. let expected_fields_message = FieldsDisplay { intent: "Approve spending".to_string(), fields: vec![ - ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), - ("Approve to spender".to_string(), "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()), - ("Requested allowance".to_string(), "0.01 XTST".to_string()), - ("Existing allowance".to_string(), "0.01 XTST".to_string()), - ("Approval expiration".to_string(), "Thu, 06 May 2021 20:17:10 +0000".to_string()), - ("Approval fees".to_string(), "0.0001 XTST".to_string()), - ("Fees paid by".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), - ("Memo".to_string(), "test_bytes".to_string())]}; + ("From".to_string(), Icrc21Value::Text{content: "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()}), + ("Approve to spender".to_string(), Icrc21Value::Text{content: "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()}), + ("Requested allowance".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 1000000, symbol: "XTST".to_string()}), + ("Existing allowance".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 1000000, symbol: "XTST".to_string()}), + ("Approval expiration".to_string(), Icrc21Value::TimestampSeconds { amount: 1620332230 }), + ("Approval fees".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 10000, symbol: "XTST".to_string()}), + ("Fees paid by".to_string(), Icrc21Value::Text{content: "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()}), + ("Memo".to_string(), Icrc21Value::Text{content: "test_bytes".to_string()})]}; let mut args = ConsentMessageRequest { method: "icrc2_approve".to_owned(), @@ -4976,9 +4980,9 @@ Charged for processing the approval. .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field( - &fields_message, + &expected_fields_message, "Existing allowance".to_string(), - "".to_string(), + None, ); assert_eq!( fields_message, new_exp_fields_message, @@ -5016,9 +5020,11 @@ Charged for processing the approval. .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = modify_field( - &fields_message, + &expected_fields_message, "Approval expiration".to_string(), - "This approval does not have an expiration.".to_string(), + Some(Icrc21Value::Text { + content: "This approval does not have an expiration.".to_string(), + }), ); assert_eq!( fields_message, new_exp_fields_message, @@ -5049,7 +5055,9 @@ Charged for processing the approval. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "From".to_string(), "".to_string()); + let new_exp_fields_message = modify_field(&expected_fields_message, "From".to_string(), None); + let new_exp_fields_message = + modify_field(&new_exp_fields_message, "Fees paid by".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -5080,13 +5088,8 @@ Charged for processing the approval. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field( - &fields_message, - "Approval expiration".to_string(), - "Thu, 06 May 2021 21:17:10 +0100".to_string(), - ); assert_eq!( - fields_message, new_exp_fields_message, + fields_message, expected_fields_message, "Expected: {:?}, got: {:?}", new_exp_fields_message, fields_message ); @@ -5119,7 +5122,7 @@ Charged for processing the approval. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + let new_exp_fields_message = modify_field(&expected_fields_message, "Memo".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -5181,12 +5184,12 @@ Charged for processing the transfer. let expected_fields_message = FieldsDisplay { intent: "Spend Test Token".to_string(), fields: vec![ - ("From".to_string(), "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()), - ("Amount".to_string(), "0.01 XTST".to_string()), - ("Spender".to_string(), "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()), - ("To".to_string(), "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()), - ("Fees".to_string(), "0.0001 XTST".to_string()), - ("Memo".to_string(), "test_bytes".to_string())]}; + ("From".to_string(), Icrc21Value::Text{content: "d2zjj-uyaaa-aaaaa-aaaap-4ai-qmfzyha.101010101010101010101010101010101010101010101010101010101010101".to_string()}), + ("Amount".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 1000000, symbol: "XTST".to_string()}), + ("Spender".to_string(), Icrc21Value::Text{content: "djduj-3qcaa-aaaaa-aaaap-4ai-5r7aoqy.303030303030303030303030303030303030303030303030303030303030303".to_string()}), + ("To".to_string(), Icrc21Value::Text{content: "6fyp7-3ibaa-aaaaa-aaaap-4ai-v57emui.202020202020202020202020202020202020202020202020202020202020202".to_string()}), + ("Fees".to_string(), Icrc21Value::TokenAmount {decimals: 8, amount: 10000, symbol: "XTST".to_string()}), + ("Memo".to_string(), Icrc21Value::Text{content: "test_bytes".to_string()})]}; let message = extract_icrc21_message_string( &icrc21_consent_message(env, canister_id, spender_account.owner, args.clone()) @@ -5237,7 +5240,7 @@ Charged for processing the transfer. .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); let new_exp_fields_message = - modify_field(&fields_message, "Spender".to_string(), "".to_string()); + modify_field(&expected_fields_message, "Spender".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", @@ -5272,7 +5275,7 @@ Charged for processing the transfer. ) .unwrap(); let fields_message = extract_icrc21_fields_message(&fields_consent_info.consent_message); - let new_exp_fields_message = modify_field(&fields_message, "Memo".to_string(), "".to_string()); + let new_exp_fields_message = modify_field(&expected_fields_message, "Memo".to_string(), None); assert_eq!( fields_message, new_exp_fields_message, "Expected: {:?}, got: {:?}", From d2d33d290e9664410e50e83db9b269a3cef09c94 Mon Sep 17 00:00:00 2001 From: Maciej Dfinity Date: Tue, 5 Aug 2025 13:39:45 +0000 Subject: [PATCH 28/28] fix did files --- rs/ledger_suite/icp/ledger.did | 19 ++++++++++++++++++- rs/ledger_suite/icrc1/ledger/ledger.did | 19 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/rs/ledger_suite/icp/ledger.did b/rs/ledger_suite/icp/ledger.did index 7b3ff6e5929c..9693528a5f0e 100644 --- a/rs/ledger_suite/icp/ledger.did +++ b/rs/ledger_suite/icp/ledger.did @@ -441,9 +441,26 @@ type icrc21_consent_message_request = record { user_preferences: icrc21_consent_message_spec; }; +type Icrc21Value = variant { + TokenAmount: record { + decimals: nat8; + amount: nat64; + symbol: text; + }; + TimestampSeconds: record { + amount: nat64; + }; + DurationSeconds: record { + amount: nat64; + }; + Text: record { + content: text; + }; +}; + type FieldsDisplay = record { intent: text; - fields: vec record { text; text }; + fields: vec record { text; Icrc21Value }; }; type icrc21_consent_message = variant { diff --git a/rs/ledger_suite/icrc1/ledger/ledger.did b/rs/ledger_suite/icrc1/ledger/ledger.did index b1ed05aa9067..774d7f94a00e 100644 --- a/rs/ledger_suite/icrc1/ledger/ledger.did +++ b/rs/ledger_suite/icrc1/ledger/ledger.did @@ -442,9 +442,26 @@ type icrc21_consent_message_request = record { user_preferences: icrc21_consent_message_spec; }; +type Icrc21Value = variant { + TokenAmount: record { + decimals: nat8; + amount: nat64; + symbol: text; + }; + TimestampSeconds: record { + amount: nat64; + }; + DurationSeconds: record { + amount: nat64; + }; + Text: record { + content: text; + }; +}; + type FieldsDisplay = record { intent: text; - fields: vec record { text; text }; + fields: vec record { text; Icrc21Value }; }; type icrc21_consent_message = variant {