I want to propose to add the following functionality to the cycles minting canister
refund method that allows a user to reclaim ICP (for example when a transaction is send with the wrong memo / subaccount)Maybe @icme has some other suggestions as wel.
The Governance team at DFINITY has resolved to do a couple of things:
If these measures do not address the problem(s), please, lmk. Otherwise, expect these features to be forthcoming.
Your continued support of the Internet Computer is greatly appreciated!
Thanks! This would already be really helpful in preventing wrong transactions and making the ICRC2 flow partially possible!
wen publish these measures
Not sure. Letâs pencil in January?
I was thinking little endian should be assumed, because thatâs what WASM does. Hope this works for everyone. If not, please lmk. TY!
The first feature (icrc1) went out on Monday. (implementation)
I think I will have time to work on the second one (auto refund) this week. Barring unforeseen issues, I estimate that it will not be so difficult.
The second of the proposed features (auto refund) will very very likely be out on Monday.
To be precise, the code is in the master branch. The feature was implemented here, and enabled shortly after that here.
Bon appetit! We really hope you enjoy these features.
As always, your continued support of the Internet Computer is greatly appreciated!
I was trying to implement the cycle topup flow based on doing a icrc2_approve on the frontend and an icrc2_transfer_from on the canister to handle the topup. While the icrc2_transfer_from uses the icrc1_memo field as shown in this transaction it throws the following error
"Refunded: block_index: Some(20385278), reason: "Memo (0x000000) in the incoming ICP transfer does not correspond to any of the operations that the Cycles Minting canister offers.""
I tried a bunch of things like;
icrc2_approveicrc_ledger_types memo and the ic_ledger_types memo but so far no luckSo this is the code iâm working with, its based on the working code i had for just a regular transfer, what am i doing wrong? Maybe only icrc1_transferâs are allowed?
static async approve(to: Icrc1Account, amount: bigint) {
return this.actor(false).icrc2_approve({
amount,
from_subaccount: [],
spender: {
owner: to.owner,
subaccount: to.subaccount
},
fee: [],
memo: [],
created_at_time: [],
expected_allowance: [],
expires_at: [BigInt(Date.now() * 1_000_000 + 5 * 60 * 1_000_000_000)]
});
}
pub async fn top_up_cycles_by_approve(
icp_amount: u64,
from: Principal,
canister: Principal,
) -> CanisterResult<u64> {
let amount = Tokens::from_e8s(icp_amount - ICP_TRANSACTION_FEE);
let args = TransferFromArgs {
// memo: Some(Memo::from(0x50555054), //
memo: Some(Memo::from(vec![0x50, 0x55, 0x50, 0x54])), // removes prefixed 0's attempt
amount: amount.e8s().into(),
fee: None,
to: Account {
owner: MAINNET_CYCLES_MINTING_CANISTER_ID,
subaccount: Some(Subaccount::from(canister).0),
},
created_at_time: None,
spender_subaccount: Some(Subaccount::from(from).0),
from: Account {
owner: from,
subaccount: None,
},
};
let (result,): (Result<Nat, TransferFromError>,) =
ic_cdk::call(MAINNET_LEDGER_CANISTER_ID, "icrc2_transfer_from", (args,))
.await
.map_err(|err| {
ApiError::external_service_error(&format!("{:?}", err))
.add_method_name("top_up_cycles_by_approve")
.add_source("toolkit_utils")
})?;
match result {
Ok(block_index) => Ok(nat_to_u64(&block_index)),
Err(err) => Err(ApiError::external_service_error(&format!("{:?}", err))
.add_method_name("top_up_cycles_by_approve")
.add_source("toolkit_utils")),
}
}
pub async fn notify_top_up_cycles(block_index: u64, canister_id: Principal) -> CanisterResult<Nat> {
let method_name = "notify_top_up_cycles";
let source = "toolkit_utils";
match CyclesMintingService(MAINNET_CYCLES_MINTING_CANISTER_ID)
.notify_top_up(NotifyTopUpArg {
block_index,
canister_id,
})
.await
{
Ok((result,)) => match result {
NotifyTopUpResult::Ok(cycles) => Ok(cycles),
NotifyTopUpResult::Err(err) => match err {
NotifyError::Refunded {
block_index,
reason,
} => Err(ApiError::external_service_error(&format!(
"Refunded: block_index: {:?}, reason: {:?}",
block_index, reason
))
.add_method_name(method_name)
.add_source(source)),
NotifyError::InvalidTransaction(value) => Err(ApiError::external_service_error(
&format!("InvalidTransaction: {:?}", value),
)
.add_method_name(method_name)
.add_source(source)),
NotifyError::Other {
error_message,
error_code,
} => Err(ApiError::external_service_error(&format!(
"Other: error_message: {}, error_code: {:?}",
error_message, error_code
))
.add_method_name(method_name)
.add_source(source)),
NotifyError::Processing => Err(ApiError::external_service_error("Processing")),
NotifyError::TransactionTooOld(value) => Err(ApiError::external_service_error(
&format!("TransactionTooOld: {:?}", value),
)
.add_method_name(method_name)
.add_source(source)),
},
},
Err((_, err)) => Err(ApiError::external_service_error(&err)
.add_method_name(method_name)
.add_source(source)),
}
}
Hi, rem.codes. Sorry itâs not working for you.
The magic memo you need is
Whereas, your transactionâs icrc1_memo is âPUPTâ. Therefore, IIUC, you need to
(Rationale: CMC translates from a blob to an unsigned 64-bit integer, and uses little endian convention (like WASM).)
Hope this works for you. LMK if you still have problems.
Doesnât the old memo use big endian? I seem to remember being huddled in the floor in a puddle of tears when I realized I needed to be encoding PUPT instead of the expected TPUP.
This is confusing to have in two different formats AND two different orders. I guess good docs will helpâŚjust give me all the 32 byte arrays I need for each icrc action paired with the Nat32s I need with transfer in a nice neat table in the reader and online docs. Paging @Jessie
We might be confused about what âthe old memoâ is, but before icrc1, the memo was not a blob. It was (effectively) just a nat64. Therefore, there is no endianness there.
However, what I later found is that our Rust libraries have a conversion from u64 to ircr1 Memo that uses big endian. I did not know about that when I implemented this feature.
IMHO, that conversion (from u64 to icrc1 Memo) should not exist, because from ledgerâs point of view, memo is an opaque blob. It has no idea that a blob of length 8 contains an unsigned integer, and therefore, it cannot know what endianness convention is in use between the sender and receiver of the ICP.
Instead, it is up to the sender and receiver to agree on a protocol for how memo is used. In this case, the receiver is CMC, and the protocol is specified where I said âI was thinking little endian should be assumedâ earlier in this thread. FWIW, I went with little endian, because that is what WASM uses (and WASM uses little endian, because this is what hardware most commonly supports).
ofc, it is very unfortunate that the Rust conversion from u64 to icrc1 Memo uses big endian while CMCâs convention is little endian. This is admittedly extremely confusing to clients.
One thing CMC can do is extend its protocol to also accept big endian. I am mildly against this, but if we find that confusion is rife, then, Iâd agree that also accepting big endian would be worth it. It would be fairly easy to implement; it would probably just require a few additional lines here.
HmmâŚI think the nat 64 was the the term utf8 encoded and then converted to a nat 64.
(But this is ultimately not important).
Your idea may be helpful.