Skip to content

Commit

Permalink
Add retrieveBTCWithApproval to ckbtc minter canister (#454)
Browse files Browse the repository at this point in the history
# Motivation

We want to use ICRC-2 for BTC withdrawals.

# Changes

1. Add `createRetrieveBtcWithApprovalError` to convert
`RetrieveBtcWithApprovalError`.
2. Add `retrieveBtcWithApproval` which calls
`retrieve_btc_with_approval` on the minter canister.
3. Add unit tests for `retrieveBtcWithApproval`.
4. Unrelated: Correct a comment about the type of `subaccount` on
`getBtcAddress`, which was mentioned as `Principal` but should be
`Uint8Array`.

# Tests

Unit tests pass.
Also tested manually by installing the local changes in nns-dapp.

# Todos

- [x] Add entry to changelog (if necessary).

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
dskloetd and github-actions[bot] authored Oct 17, 2023
1 parent 5cdfbd0 commit 234c232
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- support new fields in the `CreateServiceNervousSystem` proposal action `maximum_direct_participation_icp`, `minimum_direct_participation_icp` and `neurons_fund_participation`.
- support new filter field `omit_large_fields` in `list_proposals`.
- support new field `is_genesis` in `Neuron` and `NeuronInfo`.
- add support for `retrieve_btc_with_approval` in `@dfinity/ckbtc`.

# 2023.10.02-1515Z

Expand Down
43 changes: 35 additions & 8 deletions packages/ckbtc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Parameters:

### :factory: CkBTCMinterCanister

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L33)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L35)

#### Methods

Expand All @@ -87,6 +87,7 @@ Parameters:
- [updateBalance](#gear-updatebalance)
- [getWithdrawalAccount](#gear-getwithdrawalaccount)
- [retrieveBtc](#gear-retrievebtc)
- [retrieveBtcWithApproval](#gear-retrievebtcwithapproval)
- [estimateWithdrawalFee](#gear-estimatewithdrawalfee)
- [getMinterInfo](#gear-getminterinfo)

Expand All @@ -96,7 +97,7 @@ Parameters:
| -------- | ------------------------------------------------------------------------ |
| `create` | `(options: CkBTCMinterCanisterOptions<_SERVICE>) => CkBTCMinterCanister` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L34)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L36)

##### :gear: getBtcAddress

Expand All @@ -114,7 +115,7 @@ Parameters:
- `params.owner`: The owner for which the BTC address should be generated. If not provided, the `caller` will be use instead.
- `params.subaccount`: An optional subaccount to compute the address.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L55)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L57)

##### :gear: updateBalance

Expand All @@ -132,7 +133,7 @@ Parameters:
- `params.owner`: The owner of the address. If not provided, the `caller` will be use instead.
- `params.subaccount`: An optional subaccount of the address.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L74)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L76)

##### :gear: getWithdrawalAccount

Expand All @@ -142,7 +143,7 @@ Returns the account to which the caller should deposit ckBTC before withdrawing
| ---------------------- | ------------------------ |
| `getWithdrawalAccount` | `() => Promise<Account>` |

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

##### :gear: retrieveBtc

Expand All @@ -166,7 +167,33 @@ Parameters:
- `params.address`: The bitcoin address.
- `params.amount`: The ckBTC amount.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L116)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L118)

##### :gear: retrieveBtcWithApproval

Submits a request to convert ckBTC to BTC after making an ICRC-2 approval.

# Note

The BTC retrieval process is slow. Instead of synchronously waiting for a BTC transaction to settle, this method returns a request ([block_index]) that the caller can use to query the request status.

# Preconditions

The caller allowed the minter's principal to spend its funds using
[icrc2_approve] on the ckBTC ledger.

| Method | Type |
| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------- |
| `retrieveBtcWithApproval` | `({ address, amount, fromSubaccount, }: { address: string; amount: bigint; fromSubaccount?: Uint8Array; }) => Promise<RetrieveBtcOk>` |

Parameters:

- `params.address`: The bitcoin address.
- `params.amount`: The ckBTC amount.
- `params.fromSubaccount`: An optional subaccount from which
the ckBTC should be transferred.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L148)

##### :gear: estimateWithdrawalFee

Expand All @@ -182,7 +209,7 @@ Parameters:
- `params.certified`: query or update call
- `params.amount`: The optional amount for which the fee should be estimated.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L135)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L179)

##### :gear: getMinterInfo

Expand All @@ -197,7 +224,7 @@ Parameters:
- `params`: The parameters to get the deposit fee.
- `params.certified`: query or update call

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L149)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/ckbtc/src/minter.canister.ts#L193)

<!-- TSDOC_END -->

Expand Down
27 changes: 25 additions & 2 deletions packages/ckbtc/src/errors/minter.errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { nonNullish } from "@dfinity/utils";
import type { RetrieveBtcError, UpdateBalanceError } from "../../candid/minter";
import type {
RetrieveBtcError,
RetrieveBtcWithApprovalError,
UpdateBalanceError,
} from "../../candid/minter";

export class MinterGenericError extends Error {}
export class MinterTemporaryUnavailableError extends MinterGenericError {}
Expand All @@ -12,9 +16,10 @@ export class MinterRetrieveBtcError extends MinterGenericError {}
export class MinterMalformedAddressError extends MinterRetrieveBtcError {}
export class MinterAmountTooLowError extends MinterRetrieveBtcError {}
export class MinterInsufficientFundsError extends MinterRetrieveBtcError {}
export class MinterInsufficientAllowanceError extends MinterRetrieveBtcError {}

const mapGenericError = (
Err: UpdateBalanceError | RetrieveBtcError,
Err: UpdateBalanceError | RetrieveBtcError | RetrieveBtcWithApprovalError,
): MinterGenericError | undefined => {
if ("GenericError" in Err) {
const {
Expand Down Expand Up @@ -79,3 +84,21 @@ export const createRetrieveBtcError = (
`Unsupported response type in minter.retrieveBtc ${JSON.stringify(Err)}`,
);
};

export const createRetrieveBtcWithApprovalError = (
Err: RetrieveBtcWithApprovalError,
): MinterGenericError => {
const error = mapGenericError(Err);

if (nonNullish(error)) {
return error;
}

if ("InsufficientAllowance" in Err) {
return new MinterInsufficientAllowanceError(
`${Err.InsufficientAllowance.allowance}`,
);
}

return createRetrieveBtcError(Err);
};
181 changes: 181 additions & 0 deletions packages/ckbtc/src/minter.canister.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
MinterAlreadyProcessingError,
MinterAmountTooLowError,
MinterGenericError,
MinterInsufficientAllowanceError,
MinterInsufficientFundsError,
MinterMalformedAddressError,
MinterNoNewUtxosError,
Expand Down Expand Up @@ -407,6 +408,186 @@ describe("ckBTC minter canister", () => {
});
});

describe("Retrieve BTC with approval", () => {
const success: RetrieveBtcOk = {
block_index: 1n,
};
const ok = { Ok: success };

const params = {
address: bitcoinAddressMock,
amount: 123_000n,
};

it("should return Ok", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();
service.retrieve_btc_with_approval.mockResolvedValue(ok);

const canister = minter(service);

const res = await canister.retrieveBtcWithApproval(params);

expect(service.retrieve_btc_with_approval).toBeCalledTimes(1);
expect(service.retrieve_btc_with_approval).toBeCalledWith({
...params,
from_subaccount: [],
});
expect(res).toEqual(success);
});

it("should return Ok with fromSubaccount", async () => {
const fromSubaccount = new Uint8Array([3, 4, 5]);
const service = mock<ActorSubclass<CkBTCMinterService>>();
service.retrieve_btc_with_approval.mockResolvedValue(ok);

const canister = minter(service);

const res = await canister.retrieveBtcWithApproval({
...params,
fromSubaccount,
});

expect(service.retrieve_btc_with_approval).toBeCalledTimes(1);
expect(service.retrieve_btc_with_approval).toBeCalledWith({
...params,
from_subaccount: [fromSubaccount],
});
expect(res).toEqual(success);
});

it("should throw MinterGenericError", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = {
Err: { GenericError: { error_message: "message", error_code: 1n } },
};
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterGenericError(
`${error.Err.GenericError.error_message} (${error.Err.GenericError.error_code})`,
),
);
});

it("should throw MinterTemporarilyUnavailable", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { TemporarilyUnavailable: "unavailable" } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterTemporaryUnavailableError(error.Err.TemporarilyUnavailable),
);
});

it("should throw MinterAlreadyProcessingError", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { AlreadyProcessing: null } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterAlreadyProcessingError(),
);
});

it("should throw MinterMalformedAddress", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { MalformedAddress: "malformated" } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterMalformedAddressError(error.Err.MalformedAddress),
);
});

it("should throw MinterAmountTooLowError", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { AmountTooLow: 123n } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterAmountTooLowError(`${error.Err.AmountTooLow}`),
);
});

it("should throw MinterInsufficientFundsError", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { InsufficientFunds: { balance: 123n } } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterInsufficientFundsError(
`${error.Err.InsufficientFunds.balance}`,
),
);
});

it("should throw MinterInsufficientAllowanceError", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { InsufficientAllowance: { allowance: 123n } } };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterInsufficientAllowanceError(
`${error.Err.InsufficientAllowance.allowance}`,
),
);
});

it("should throw unsupported response", async () => {
const service = mock<ActorSubclass<CkBTCMinterService>>();

const error = { Err: { Test: null } as unknown as RetrieveBtcError };
service.retrieve_btc_with_approval.mockResolvedValue(error);

const canister = minter(service);

const call = () => canister.retrieveBtcWithApproval(params);

await expect(call).rejects.toThrowError(
new MinterRetrieveBtcError(
`Unsupported response type in minter.retrieveBtc ${JSON.stringify(
error.Err,
)}`,
),
);
});
});

describe("Estimate Withdrawal Fee", () => {
it("should return estimated fee", async () => {
const result = { minter_fee: 123n, bitcoin_fee: 456n };
Expand Down
Loading

0 comments on commit 234c232

Please sign in to comment.