Skip to content

Commit

Permalink
Add icrc1Transfer method to ICP ledger canister (#419)
Browse files Browse the repository at this point in the history
# Motivation

We want to transfer ICP using the ICRC-1 API because then we don't have
to compute account identifiers in the client.
Passing the ICP ledger canister ID to the ICRC-1 API works but the ICP
ledger canister has its own ICRC-1 interface so calling it directly on
the ICP ledger API is cleaner.

*Note*: I started with a commit that duplicates the non-ICRC-1 code.
That way you can see how the ICRC-1 code differs from the existing code
by reviewing the second commit.

# Changes

1. Added `icrc1Transfer` canister method.
2. Added `toIcrc1TransferRawRequest` request converter.
3. Added `mapIcrc1TransferError` error converter.
4. Added `Icrc1TransferRequest` request type.

# Tests

Copied the existing tests for `transfer` and applied them to
`icrc1Transfer` as far as they made sense. The new method has less
custom code for hardware wallets so some of those test have not been
duplicated.

Tested manually in NNS dapp by using the new method when making ICRC-1
transfers and then testing an ICRC-1 transfer.

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
dskloetd and github-actions[bot] authored Sep 20, 2023
1 parent 4c8dd0b commit 1f2e7d9
Show file tree
Hide file tree
Showing 7 changed files with 482 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

- add support for `icrc2_transfer_from`, `icrc2_approve` and `icrc2_allowance` in `@dfinity/ledger`
- update index did definitions in ledger which provides more information in the transactions
- add support for icrc1_transfer on the ICP ledger canister in `@dfinity/nss`

## Build

Expand Down
22 changes: 17 additions & 5 deletions packages/nns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -259,22 +259,23 @@ Parameters:

### :factory: LedgerCanister

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L27)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L32)

#### Methods

- [create](#gear-create)
- [accountBalance](#gear-accountbalance)
- [transactionFee](#gear-transactionfee)
- [transfer](#gear-transfer)
- [icrc1Transfer](#gear-icrc1transfer)

##### :gear: create

| Method | Type |
| -------- | ----------------------------------------------------- |
| `create` | `(options?: LedgerCanisterOptions) => LedgerCanister` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L38)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L43)

##### :gear: accountBalance

Expand All @@ -287,7 +288,7 @@ it is fetched using a query call.
| ---------------- | ------------------------------------------------------------------------------------------------------------------------ |
| `accountBalance` | `({ accountIdentifier, certified, }: { accountIdentifier: AccountIdentifier; certified?: boolean; }) => Promise<bigint>` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L70)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L75)

##### :gear: transactionFee

Expand All @@ -297,7 +298,7 @@ Returns the transaction fee of the ledger canister
| ---------------- | ----------------------- |
| `transactionFee` | `() => Promise<bigint>` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L94)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L99)

##### :gear: transfer

Expand All @@ -308,7 +309,18 @@ Returns the index of the block containing the tx if it was successful.
| ---------- | ----------------------------------------------- |
| `transfer` | `(request: TransferRequest) => Promise<bigint>` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L107)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L112)

##### :gear: icrc1Transfer

Transfer ICP from the caller to the destination `Account`.
Returns the index of the block containing the tx if it was successful.

| Method | Type |
| --------------- | ---------------------------------------------------- |
| `icrc1Transfer` | `(request: Icrc1TransferRequest) => Promise<bigint>` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/nns/src/ledger.canister.ts#L137)

### :factory: GovernanceCanister

Expand Down
25 changes: 23 additions & 2 deletions packages/nns/src/canisters/ledger/ledger.request.converts.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import type { ICPTs, Subaccount } from "@dfinity/nns-proto";
import { arrayOfNumberToUint8Array } from "@dfinity/utils";
import { arrayOfNumberToUint8Array, toNullable } from "@dfinity/utils";
import type {
Tokens,
TransferArg as Icrc1TransferRawRequest,
TransferArgs as TransferRawRequest,
} from "../../../candid/ledger";
import { TRANSACTION_FEE } from "../../constants/constants";
import type { TransferRequest } from "../../types/ledger_converters";
import type {
Icrc1TransferRequest,
TransferRequest,
} from "../../types/ledger_converters";
import { importNnsProto } from "../../utils/proto.utils";

export const subAccountNumbersToSubaccount = async (
Expand Down Expand Up @@ -47,3 +51,20 @@ export const toTransferRawRequest = ({
? []
: [arrayOfNumberToUint8Array(fromSubAccount)],
});

export const toIcrc1TransferRawRequest = ({
fromSubAccount,
to,
amount,
fee,
memo,
createdAt,
}: Icrc1TransferRequest): Icrc1TransferRawRequest => ({
to,
fee: toNullable(fee ?? TRANSACTION_FEE),
amount,
// Always explicitly set the memo for compatibility with ledger wallet - hardware wallet
memo: toNullable(memo ?? new Uint8Array()),
created_at_time: toNullable(createdAt),
from_subaccount: toNullable(fromSubAccount),
});
33 changes: 31 additions & 2 deletions packages/nns/src/errors/ledger.errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { convertStringToE8s } from "@dfinity/utils";
import type { TransferError as RawTransferError } from "../../candid/ledger";
import type {
Icrc1TransferError as RawIcrc1TransferError,
TransferError as RawTransferError,
} from "../../candid/ledger";
import type { BlockHeight } from "../types/common";

export class TransferError extends Error {}
Expand All @@ -13,7 +16,7 @@ export class InsufficientFundsError extends TransferError {
}

export class TxTooOldError extends TransferError {
constructor(public readonly allowed_window_secs: number) {
constructor(public readonly allowed_window_secs?: number | undefined) {
super();
}
}
Expand Down Expand Up @@ -60,6 +63,32 @@ export const mapTransferError = (
);
};

export const mapIcrc1TransferError = (
rawTransferError: RawIcrc1TransferError,
): TransferError => {
if ("Duplicate" in rawTransferError) {
return new TxDuplicateError(rawTransferError.Duplicate.duplicate_of);
}
if ("InsufficientFunds" in rawTransferError) {
return new InsufficientFundsError(
rawTransferError.InsufficientFunds.balance,
);
}
if ("CreatedInFuture" in rawTransferError) {
return new TxCreatedInFutureError();
}
if ("TooOld" in rawTransferError) {
return new TxTooOldError();
}
if ("BadFee" in rawTransferError) {
return new BadFeeError(rawTransferError.BadFee.expected_fee);
}
// Edge case
return new TransferError(
`Unknown error type ${JSON.stringify(rawTransferError)}`,
);
};

export const mapTransferProtoError = (responseBytes: Error): TransferError => {
const { message } = responseBytes;

Expand Down
Loading

0 comments on commit 1f2e7d9

Please sign in to comment.