Skip to content

Commit

Permalink
Rename ICRC-1 memo to icrc1Memo
Browse files Browse the repository at this point in the history
  • Loading branch information
dskloetd committed Sep 26, 2023
1 parent d9cf048 commit 3d5fce8
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 33 deletions.
17 changes: 11 additions & 6 deletions packages/ledger/src/converters/ledger.converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
Expand All @@ -28,28 +33,28 @@ 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),
});

export const toApproveArgs = ({
fee,
created_at_time,
memo,
icrc1Memo,
from_subaccount,
expected_allowance,
expires_at,
...rest
}: 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),
Expand Down
13 changes: 9 additions & 4 deletions packages/ledger/src/types/ledger.params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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.
*/
Expand All @@ -56,7 +61,7 @@ export type TransferFromParams = Omit<TransferParams, "from_subaccount"> & {
* @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.
Expand Down
9 changes: 7 additions & 2 deletions packages/nns/src/canisters/ledger/ledger.request.converts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
4 changes: 3 additions & 1 deletion packages/nns/src/governance.canister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
36 changes: 18 additions & 18 deletions packages/nns/src/ledger.canister.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,22 +651,22 @@ 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,
});
await ledger.icrc1Transfer({
to,
amount,
fee,
memo,
icrc1Memo,
});

expect(service.icrc1_transfer).toBeCalledWith({
to,
fee: [fee],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [],
from_subaccount: [],
});
Expand Down Expand Up @@ -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,
});
Expand All @@ -712,15 +712,15 @@ describe("LedgerCanister", () => {
to,
amount,
fee,
memo,
icrc1Memo,
createdAt,
});

expect(service.icrc1_transfer).toBeCalledWith({
to,
fee: [fee],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [createdAt],
from_subaccount: [],
});
Expand All @@ -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,
Expand All @@ -744,15 +744,15 @@ describe("LedgerCanister", () => {
to,
amount,
fee,
memo,
icrc1Memo,
fromSubAccount,
});

expect(service.icrc1_transfer).toBeCalledWith({
to,
fee: [fee],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [],
from_subaccount: [fromSubAccount],
});
Expand All @@ -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,
Expand All @@ -779,7 +779,7 @@ describe("LedgerCanister", () => {
},
amount,
fee,
memo,
icrc1Memo,
});

expect(service.icrc1_transfer).toBeCalledWith({
Expand All @@ -789,7 +789,7 @@ describe("LedgerCanister", () => {
},
fee: [fee],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [],
from_subaccount: [],
});
Expand Down Expand Up @@ -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();
Expand All @@ -938,7 +938,7 @@ describe("LedgerCanister", () => {
to,
fee: [BigInt(10000)],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [],
from_subaccount: [],
});
Expand All @@ -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();
Expand All @@ -970,7 +970,7 @@ describe("LedgerCanister", () => {
to,
fee: [fee],
amount,
memo: [memo],
memo: [icrc1Memo],
created_at_time: [],
from_subaccount: [],
});
Expand Down
5 changes: 5 additions & 0 deletions packages/nns/src/ledger.canister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 6 additions & 1 deletion packages/nns/src/types/ledger_converters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/sns/src/sns.wrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down

0 comments on commit 3d5fce8

Please sign in to comment.