Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(security): expose certified flag for ICP to XDR conversion rate #830

Merged
merged 8 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# YYYY.MM.DD-HHMMZ

# Features
## Breaking Changes

- Default `getIcpToCyclesConversionRate` to an update call while providing a `certified` parameter for queries.

## Features

- Support `NervousSystemParameters.automatically_advance_target_version` in `@dfinity/sns`.

Expand Down
23 changes: 14 additions & 9 deletions packages/cmc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,18 @@ const rate = await getIcpToCyclesConversionRate();

##### :gear: getIcpToCyclesConversionRate

Returns conversion rate of ICP to Cycles
Returns conversion rate of ICP to Cycles. It can be called as query or update.

| Method | Type |
| ------------------------------ | ----------------------- |
| `getIcpToCyclesConversionRate` | `() => Promise<bigint>` |
| Method | Type |
| ------------------------------ | --------------------------------------------------- |
| `getIcpToCyclesConversionRate` | `({ certified, }?: QueryParams) => Promise<bigint>` |

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

- `params`: - The parameters for the call.
- `params.certified`: - Determines whether the response should be certified (default: non-certified)

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

##### :gear: notifyCreateCanister

Expand All @@ -93,7 +98,7 @@ It returns the new canister principal.
| ---------------------- | ---------------------------------------------------------- |
| `notifyCreateCanister` | `(request: NotifyCreateCanisterArg) => Promise<Principal>` |

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

##### :gear: notifyTopUp

Expand All @@ -104,7 +109,7 @@ It returns the new Cycles of the canister.
| ------------- | ---------------------------------------------- |
| `notifyTopUp` | `(request: NotifyTopUpArg) => Promise<bigint>` |

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L77)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L85)

##### :gear: getDefaultSubnets

Expand All @@ -121,7 +126,7 @@ Parameters:
- `params.certified`: - Determines whether the response should be certified
(default: non-certified if not specified).

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L102)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L110)

##### :gear: getSubnetTypesToSubnets

Expand All @@ -139,6 +144,6 @@ Parameters:
- `params.certified`: - Specifies whether the response should be certified.
If not provided, the response defaults to non-certified.

[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L121)
[:link: Source](https://github.com/dfinity/ic-js/tree/main/packages/cmc/src/cmc.canister.ts#L129)

<!-- TSDOC_END -->
40 changes: 38 additions & 2 deletions packages/cmc/src/cmc.canister.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe("CyclesMintingCanister", () => {
};

describe("CMCCanister.getIcpToCyclesConversionRate", () => {
it("should returns the conversion rate from ICP to cycles", async () => {
it("should returns the conversion rate from ICP to cycles for a query", async () => {
const exchangeRate = BigInt(10_000);
const response: IcpXdrConversionRateResponse = {
certificate: arrayOfNumberToUint8Array([]),
Expand All @@ -49,10 +49,46 @@ describe("CyclesMintingCanister", () => {
service.get_icp_xdr_conversion_rate.mockResolvedValue(response);

const cmc = await createCMC(service);
const callerSpy = jest.spyOn(
cmc as unknown as {
caller: (params: QueryParams) => Promise<CMCService>;
},
"caller",
);

const res = await cmc.getIcpToCyclesConversionRate({ certified: false });

expect(res).toEqual(exchangeRate);
expect(service.get_icp_xdr_conversion_rate).toHaveBeenCalledTimes(1);
expect(callerSpy).toHaveBeenCalledWith({ certified: false });
});

it("should returns the conversion rate from ICP to cycles for an update", async () => {
const exchangeRate = BigInt(10_000);
const response: IcpXdrConversionRateResponse = {
certificate: arrayOfNumberToUint8Array([]),
data: {
xdr_permyriad_per_icp: exchangeRate,
timestamp_seconds: BigInt(10),
},
hash_tree: arrayOfNumberToUint8Array([]),
};
const service = mock<CMCService>();
service.get_icp_xdr_conversion_rate.mockResolvedValue(response);

const cmc = await createCMC(service);
const callerSpy = jest.spyOn(
cmc as unknown as {
caller: (params: QueryParams) => Promise<CMCService>;
},
"caller",
);

const res = await cmc.getIcpToCyclesConversionRate();
const res = await cmc.getIcpToCyclesConversionRate({ certified: true });

expect(res).toEqual(exchangeRate);
expect(service.get_icp_xdr_conversion_rate).toHaveBeenCalledTimes(1);
expect(callerSpy).toHaveBeenCalledWith({ certified: true });
});
});

Expand Down
16 changes: 12 additions & 4 deletions packages/cmc/src/cmc.canister.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,22 @@ export class CMCCanister extends Canister<CMCCanisterService> {
}

/**
* Returns conversion rate of ICP to Cycles
* Returns conversion rate of ICP to Cycles. It can be called as query or update.
*
* @param {Object} [params] - The parameters for the call.
* @param {boolean} [params.certified] - Determines whether the response should be certified (default: non-certified)
*
* @returns Promise<BigInt>
*/
public getIcpToCyclesConversionRate = async (): Promise<bigint> => {
const { data } = await this.service.get_icp_xdr_conversion_rate();
public getIcpToCyclesConversionRate = async ({
certified,
peterpeterparker marked this conversation as resolved.
Show resolved Hide resolved
}: QueryParams = {}): Promise<bigint> => {
const { data } = await this.caller({
certified,
}).get_icp_xdr_conversion_rate();

// TODO: validate the certificate in the response - https://dfinity.atlassian.net/browse/FOLLOW-223
// TODO: validate the certificate in the response - https://dfinity.atlassian.net/browse/GIX-150
// Example: https://github.com/dfinity/response-verification/tree/main/examples/certification/certified-counter
return data.xdr_permyriad_per_icp;
};

Expand Down