From 3d5fce83f36e04867783c60cbe1a2806a716e5c8 Mon Sep 17 00:00:00 2001 From: David de Kloet Date: Tue, 26 Sep 2023 10:17:56 +0200 Subject: [PATCH] Rename ICRC-1 memo to icrc1Memo --- .../src/converters/ledger.converters.ts | 17 +++++---- packages/ledger/src/types/ledger.params.ts | 13 ++++--- .../ledger/ledger.request.converts.ts | 9 +++-- packages/nns/src/governance.canister.ts | 4 ++- packages/nns/src/ledger.canister.spec.ts | 36 +++++++++---------- packages/nns/src/ledger.canister.ts | 5 +++ packages/nns/src/types/ledger_converters.ts | 7 +++- packages/sns/src/sns.wrapper.ts | 2 +- 8 files changed, 60 insertions(+), 33 deletions(-) diff --git a/packages/ledger/src/converters/ledger.converters.ts b/packages/ledger/src/converters/ledger.converters.ts index eb4f0bc25..b9b16e323 100644 --- a/packages/ledger/src/converters/ledger.converters.ts +++ b/packages/ledger/src/converters/ledger.converters.ts @@ -10,16 +10,21 @@ import type { TransferParams, } from "../types/ledger.params"; +// WARNING: When using the ICRC-1 interface of the ICP ledger, there is no +// relationship between the memo and the icrc1Memo of a transaction. The ICRC-1 +// interface simply cannot set the memo field and the non-ICRC-1 interface +// cannot set the icrc1Memo field, even though the icrc1Memo field is called +// just "memo" in canister method params. export const toTransferArg = ({ from_subaccount, fee, created_at_time, - memo, + icrc1Memo, ...rest }: TransferParams): TransferArg => ({ ...rest, fee: toNullable(fee), - memo: toNullable(memo), + memo: toNullable(icrc1Memo), from_subaccount: toNullable(from_subaccount), created_at_time: toNullable(created_at_time), }); @@ -28,12 +33,12 @@ export const toTransferFromArgs = ({ spender_subaccount, fee, created_at_time, - memo, + icrc1Memo, ...rest }: TransferFromParams): TransferFromArgs => ({ ...rest, fee: toNullable(fee), - memo: toNullable(memo), + memo: toNullable(icrc1Memo), spender_subaccount: toNullable(spender_subaccount), created_at_time: toNullable(created_at_time), }); @@ -41,7 +46,7 @@ export const toTransferFromArgs = ({ export const toApproveArgs = ({ fee, created_at_time, - memo, + icrc1Memo, from_subaccount, expected_allowance, expires_at, @@ -49,7 +54,7 @@ export const toApproveArgs = ({ }: ApproveParams): ApproveArgs => ({ ...rest, fee: toNullable(fee), - memo: toNullable(memo), + memo: toNullable(icrc1Memo), from_subaccount: toNullable(from_subaccount), created_at_time: toNullable(created_at_time), expected_allowance: toNullable(expected_allowance), diff --git a/packages/ledger/src/types/ledger.params.ts b/packages/ledger/src/types/ledger.params.ts index 095624a83..38b09dd17 100644 --- a/packages/ledger/src/types/ledger.params.ts +++ b/packages/ledger/src/types/ledger.params.ts @@ -13,13 +13,18 @@ import type { IcrcAccount } from "./ledger.responses"; */ export type BalanceParams = IcrcAccount & QueryParams; +// WARNING: When using the ICRC-1 interface of the ICP ledger, there is no +// relationship between the memo and the icrc1Memo of a transaction. The ICRC-1 +// interface simply cannot set the memo field and the non-ICRC-1 interface +// cannot set the icrc1Memo field, even though the icrc1Memo field is called +// just "memo" in canister method params. /** * Params to make a transfer in an ICRC-1 ledger * * @param {Account} to The account to transfer tokens to. * @param {Tokens} amount The Amount of tokens to transfer. * @param {Subaccount?} from_subaccount The subaccount to transfer tokens to. - * @param {Uint8Array?} memo Transfer memo. + * @param {Uint8Array?} icrc1Memo Transfer memo. * @param {Timestamp?} created_at_time nanoseconds since unix epoc to trigger deduplication and avoid other issues * See the link for more details on deduplication * https://github.com/dfinity/ICRC-1/blob/main/standards/ICRC-1/README.md#transaction_deduplication @@ -28,7 +33,7 @@ export type BalanceParams = IcrcAccount & QueryParams; export interface TransferParams { to: Account; fee?: Tokens; - memo?: Uint8Array; + icrc1Memo?: Uint8Array; from_subaccount?: Subaccount; created_at_time?: Timestamp; amount: Tokens; @@ -41,7 +46,7 @@ export interface TransferParams { * @param {Account} from The account to transfer tokens from. * @param {Subaccount?} spender_subaccount A spender subaccount. * @param {Tokens} amount The Amount of tokens to transfer. - * @param {Uint8Array?} memo Transfer memo. + * @param {Uint8Array?} icrc1Memo Transfer memo. * @param {Timestamp?} created_at_time nanoseconds since unix epoc to trigger deduplication and avoid other issues * @param {Tokens?} fee The fee of the transfer when it's not the default fee. */ @@ -56,7 +61,7 @@ export type TransferFromParams = Omit & { * @param {Account} spender The account of the spender. * @param {Tokens} amount The Amount of tokens to approve. * @param {Subaccount?} from_subaccount The subaccount to transfer tokens from. - * @param {Uint8Array?} memo Transfer memo. + * @param {Uint8Array?} icrc1Memo Transfer memo. * @param {Timestamp?} created_at_time nanoseconds since unix epoc to trigger deduplication and avoid other issues * @param {Tokens?} fee The fee of the transfer when it's not the default fee. * @param {Tokens?} expected_allowance The optional allowance expected. If the expected_allowance field is set, the ledger MUST ensure that the current allowance for the spender from the caller's account is equal to the given value and return the AllowanceChanged error otherwise. diff --git a/packages/nns/src/canisters/ledger/ledger.request.converts.ts b/packages/nns/src/canisters/ledger/ledger.request.converts.ts index c50c60b0c..e1bde1c04 100644 --- a/packages/nns/src/canisters/ledger/ledger.request.converts.ts +++ b/packages/nns/src/canisters/ledger/ledger.request.converts.ts @@ -52,18 +52,23 @@ export const toTransferRawRequest = ({ : [arrayOfNumberToUint8Array(fromSubAccount)], }); +// WARNING: When using the ICRC-1 interface of the ICP ledger, there is no +// relationship between the memo and the icrc1Memo of a transaction. The ICRC-1 +// interface simply cannot set the memo field and the non-ICRC-1 interface +// cannot set the icrc1Memo field, even though the icrc1Memo field is called +// just "memo" in canister method params. export const toIcrc1TransferRawRequest = ({ fromSubAccount, to, amount, fee, - memo, + icrc1Memo, createdAt, }: Icrc1TransferRequest): Icrc1TransferRawRequest => ({ to, fee: toNullable(fee ?? TRANSACTION_FEE), amount, - memo: toNullable(memo), + memo: toNullable(icrc1Memo), created_at_time: toNullable(createdAt), from_subaccount: toNullable(fromSubAccount), }); diff --git a/packages/nns/src/governance.canister.ts b/packages/nns/src/governance.canister.ts index f6d7408a3..7ba52a019 100644 --- a/packages/nns/src/governance.canister.ts +++ b/packages/nns/src/governance.canister.ts @@ -335,7 +335,9 @@ export class GovernanceCanister { // Send amount to the ledger. await ledgerCanister.icrc1Transfer({ - memo: nonceBytes, + // WARNING: This does not set the same memo field as the stakeNeuron + // function above and would need to be handled separately from that field. + icrc1Memo: nonceBytes, amount: stake, fromSubAccount, to: { diff --git a/packages/nns/src/ledger.canister.spec.ts b/packages/nns/src/ledger.canister.spec.ts index 65bb98fd7..79ad5e3ed 100644 --- a/packages/nns/src/ledger.canister.spec.ts +++ b/packages/nns/src/ledger.canister.spec.ts @@ -651,7 +651,7 @@ describe("LedgerCanister", () => { Ok: BigInt(1234), }); const fee = BigInt(10_000); - const memo = new Uint8Array([3, 4, 5, 6]); + const icrc1Memo = new Uint8Array([3, 4, 5, 6]); const ledger = LedgerCanister.create({ certifiedServiceOverride: service, }); @@ -659,14 +659,14 @@ describe("LedgerCanister", () => { to, amount, fee, - memo, + icrc1Memo, }); expect(service.icrc1_transfer).toBeCalledWith({ to, fee: [fee], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [], from_subaccount: [], }); @@ -703,7 +703,7 @@ describe("LedgerCanister", () => { Ok: BigInt(1234), }); const fee = BigInt(10_000); - const memo = new Uint8Array([3, 4, 5, 6]); + const icrc1Memo = new Uint8Array([3, 4, 5, 6]); const ledger = LedgerCanister.create({ certifiedServiceOverride: service, }); @@ -712,7 +712,7 @@ describe("LedgerCanister", () => { to, amount, fee, - memo, + icrc1Memo, createdAt, }); @@ -720,7 +720,7 @@ describe("LedgerCanister", () => { to, fee: [fee], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [createdAt], from_subaccount: [], }); @@ -732,7 +732,7 @@ describe("LedgerCanister", () => { Ok: BigInt(1234), }); const fee = BigInt(10_000); - const memo = new Uint8Array(); + const icrc1Memo = new Uint8Array(); const fromSubAccount = new Uint8Array([ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, @@ -744,7 +744,7 @@ describe("LedgerCanister", () => { to, amount, fee, - memo, + icrc1Memo, fromSubAccount, }); @@ -752,7 +752,7 @@ describe("LedgerCanister", () => { to, fee: [fee], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [], from_subaccount: [fromSubAccount], }); @@ -764,7 +764,7 @@ describe("LedgerCanister", () => { Ok: BigInt(1234), }); const fee = BigInt(10_000); - const memo = new Uint8Array(); + const icrc1Memo = new Uint8Array(); const toSubAccount = new Uint8Array([ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, @@ -779,7 +779,7 @@ describe("LedgerCanister", () => { }, amount, fee, - memo, + icrc1Memo, }); expect(service.icrc1_transfer).toBeCalledWith({ @@ -789,7 +789,7 @@ describe("LedgerCanister", () => { }, fee: [fee], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [], from_subaccount: [], }); @@ -925,11 +925,11 @@ describe("LedgerCanister", () => { hardwareWallet: true, }); - const memo = new Uint8Array(); + const icrc1Memo = new Uint8Array(); await ledger.icrc1Transfer({ to, amount, - memo, + icrc1Memo, }); expect(service.transfer_fee).not.toBeCalled(); @@ -938,7 +938,7 @@ describe("LedgerCanister", () => { to, fee: [BigInt(10000)], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [], from_subaccount: [], }); @@ -956,12 +956,12 @@ describe("LedgerCanister", () => { }); const fee = BigInt(990_000); - const memo = new Uint8Array(); + const icrc1Memo = new Uint8Array(); await ledger.icrc1Transfer({ to, amount, fee, - memo, + icrc1Memo, }); expect(service.transfer_fee).not.toBeCalled(); @@ -970,7 +970,7 @@ describe("LedgerCanister", () => { to, fee: [fee], amount, - memo: [memo], + memo: [icrc1Memo], created_at_time: [], from_subaccount: [], }); diff --git a/packages/nns/src/ledger.canister.ts b/packages/nns/src/ledger.canister.ts index 095352d9e..6a9f3e752 100644 --- a/packages/nns/src/ledger.canister.ts +++ b/packages/nns/src/ledger.canister.ts @@ -128,6 +128,11 @@ export class LedgerCanister { return response.Ok; }; + // WARNING: When using the ICRC-1 interface of the ICP ledger, there is no + // relationship between the memo and the icrc1Memo of a transaction. The + // ICRC-1 interface simply cannot set the memo field and the non-ICRC-1 + // interface cannot set the icrc1Memo field, even though the icrc1Memo field + // is called just "memo" in canister method params. /** * Transfer ICP from the caller to the destination `Account`. * Returns the index of the block containing the tx if it was successful. diff --git a/packages/nns/src/types/ledger_converters.ts b/packages/nns/src/types/ledger_converters.ts index 901f080f6..ade1ade77 100644 --- a/packages/nns/src/types/ledger_converters.ts +++ b/packages/nns/src/types/ledger_converters.ts @@ -20,10 +20,15 @@ export type TransferRequest = { createdAt?: bigint; }; +// WARNING: When using the ICRC-1 interface of the ICP ledger, there is no +// relationship between the memo and the icrc1Memo of a transaction. The ICRC-1 +// interface simply cannot set the memo field and the non-ICRC-1 interface +// cannot set the icrc1Memo field, even though the icrc1Memo field is called +// just "memo" in canister method params. export type Icrc1TransferRequest = { to: Account; amount: Icrc1Tokens; - memo?: Uint8Array; + icrc1Memo?: Uint8Array; fee?: Icrc1Tokens; fromSubAccount?: SubAccount; // Nanoseconds since unix epoc to trigger deduplication and avoid other issues diff --git a/packages/sns/src/sns.wrapper.ts b/packages/sns/src/sns.wrapper.ts index d471fe450..e0024cc1b 100644 --- a/packages/sns/src/sns.wrapper.ts +++ b/packages/sns/src/sns.wrapper.ts @@ -276,7 +276,7 @@ export class SnsWrapper { subaccount: toNullable(neuronAccount.subaccount), }, from_subaccount: source.subaccount, - memo: bigIntToUint8Array(index), + icrc1Memo: bigIntToUint8Array(index), created_at_time: createdAt, fee, });