Skip to content

Commit

Permalink
Use icrc1Transfer on ICP ledger canister (#3360)
Browse files Browse the repository at this point in the history
# Motivation

When you use an ICRC-1 address, we make an ICRC-1 transfer but currently
we use the generic ICRC-1 ledger interface for that, specifying the ICP
ledger canister ID.
But the ICP ledger interface has its own ICRC-1 transfer method.

# Changes

Use the direct `icrc1Transfer` method on the ICP ledger API instead of
using the generic ICRC-1 ledger API.

# Tests

Unit test updated.
Tested manually and noticed that for HW transactions, an empty memo is
rendered as 422710814078008.
This will be fixed with dfinity/ic-js#423

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Sep 22, 2023
1 parent e1a2b04 commit c1096f0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The NNS Dapp is released through proposals in the Network Nervous System. Theref
* Allow setting a dissolve delay that's shorter than what's required for voting power.
* Improve contrast of token selector's logo in light theme.
* Remove the "Project" leading word in the SNS Project card.
* Use ICRC-1 transfer on ICP ledger canister instead of generic ICRC-1 ledger canister.

#### Deprecated
#### Removed
Expand Down
17 changes: 8 additions & 9 deletions frontend/src/lib/services/icp-accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ import {
getTransactions,
renameSubAccount as renameSubAccountApi,
} from "$lib/api/accounts.api";
import { queryAccountBalance, sendICP } from "$lib/api/icp-ledger.api";
import { icrcTransfer } from "$lib/api/icrc-ledger.api";
import {
queryAccountBalance,
sendICP,
sendIcpIcrc1,
} from "$lib/api/icp-ledger.api";
import { addAccount, queryAccount } from "$lib/api/nns-dapp.api";
import { AccountNotFoundError } from "$lib/canisters/nns-dapp/nns-dapp.errors";
import type {
Expand All @@ -17,7 +20,6 @@ import {
SYNC_ACCOUNTS_RETRY_MAX_ATTEMPTS,
SYNC_ACCOUNTS_RETRY_SECONDS,
} from "$lib/constants/accounts.constants";
import { LEDGER_CANISTER_ID } from "$lib/constants/canister-ids.constants";
import { DEFAULT_TRANSACTION_PAGE_LIMIT } from "$lib/constants/constants";
import { FORCE_CALL_STRATEGY } from "$lib/constants/mockable.constants";
import { nnsAccountsListStore } from "$lib/derived/accounts-list.derived";
Expand Down Expand Up @@ -45,7 +47,6 @@ import {
invalidIcrcAddress,
toIcpAccountIdentifier,
} from "$lib/utils/accounts.utils";
import { nowInBigIntNanoSeconds } from "$lib/utils/date.utils";
import {
isForceCallStrategy,
notForceCallStrategy,
Expand Down Expand Up @@ -316,13 +317,11 @@ export const transferICP = async ({
const feeE8s = get(mainTransactionFeeE8sStore);

await (validIcrcAddress
? icrcTransfer({
? sendIcpIcrc1({
identity,
to: decodeIcrcAccount(to),
fromSubAccount: subAccount,
amount: tokenAmount.toE8s(),
canisterId: LEDGER_CANISTER_ID,
createdAt: nowInBigIntNanoSeconds(),
fromSubAccount: subAccount && new Uint8Array(subAccount),
amount: tokenAmount,
fee: feeE8s,
})
: sendICP({
Expand Down
7 changes: 2 additions & 5 deletions frontend/src/tests/lib/services/icp-accounts.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import * as nnsdappApi from "$lib/api/nns-dapp.api";
import { AccountNotFoundError } from "$lib/canisters/nns-dapp/nns-dapp.errors";
import type { AccountDetails } from "$lib/canisters/nns-dapp/nns-dapp.types";
import { SYNC_ACCOUNTS_RETRY_SECONDS } from "$lib/constants/accounts.constants";
import { LEDGER_CANISTER_ID } from "$lib/constants/canister-ids.constants";
import { DEFAULT_TRANSACTION_PAGE_LIMIT } from "$lib/constants/constants";
import { getLedgerIdentityProxy } from "$lib/proxy/icp-ledger.services.proxy";
import * as authServices from "$lib/services/auth.services";
Expand Down Expand Up @@ -824,7 +823,7 @@ describe("icp-accounts.services", () => {

it("should transfer ICP using an Icrc destination address", async () => {
const spy = jest
.spyOn(icrcLedgerApi, "icrcTransfer")
.spyOn(ledgerApi, "sendIcpIcrc1")
.mockResolvedValue(BigInt(1));

await transferICP({
Expand All @@ -840,9 +839,7 @@ describe("icp-accounts.services", () => {
amount: TokenAmount.fromNumber({
amount: transferICPParams.amount,
token: ICPToken,
}).toE8s(),
canisterId: LEDGER_CANISTER_ID,
createdAt: expect.any(BigInt),
}),
fee: feeE8s,
fromSubAccount: undefined,
identity: mockIdentity,
Expand Down

0 comments on commit c1096f0

Please sign in to comment.