From 96309445519b58833ab5f3f853a481b7e95c3f8a Mon Sep 17 00:00:00 2001 From: David de Kloet <122978264+dskloetd@users.noreply.github.com> Date: Mon, 4 Dec 2023 13:27:00 +0100 Subject: [PATCH] Remove ICRC-1 neuron staking logic (#3946) # Motivation ICRC-1 transfers use a different memo. We use the memo when staking an NNS neuron to make sure we can complete the transaction. So currently, using ICRC-1 for neuron staking wouldn't be safe. We should move directly to ICRC-2. # Changes Remove the logic for using ICRC-1 when staking a neuron. The feature flag itself will be clean up, together with some other unused feature flags, in a separate PR. # Tests Corresponding tests have been removed. # Todos - [x] Add entry to changelog (if necessary). --- CHANGELOG-Nns-Dapp-unreleased.md | 2 + frontend/src/lib/services/neurons.services.ts | 31 +--- .../lib/services/neurons.services.spec.ts | 140 ------------------ 3 files changed, 9 insertions(+), 164 deletions(-) diff --git a/CHANGELOG-Nns-Dapp-unreleased.md b/CHANGELOG-Nns-Dapp-unreleased.md index 2e827fdcbd1..bdf8a8b620f 100644 --- a/CHANGELOG-Nns-Dapp-unreleased.md +++ b/CHANGELOG-Nns-Dapp-unreleased.md @@ -20,6 +20,8 @@ proposal is successful, the changes it released will be moved from this file to #### Removed +* Remove logic for using ICRC-1 when staking a neuron. + #### Fixed #### Security diff --git a/frontend/src/lib/services/neurons.services.ts b/frontend/src/lib/services/neurons.services.ts index 57fbae85b54..7f1baf733a3 100644 --- a/frontend/src/lib/services/neurons.services.ts +++ b/frontend/src/lib/services/neurons.services.ts @@ -11,7 +11,6 @@ import { FORCE_CALL_STRATEGY } from "$lib/constants/mockable.constants"; import type { LedgerIdentity } from "$lib/identities/ledger.identity"; import { getLedgerIdentityProxy } from "$lib/proxy/icp-ledger.services.proxy"; import { startBusy, stopBusy } from "$lib/stores/busy.store"; -import { ENABLE_STAKE_NEURON_ICRC1 } from "$lib/stores/feature-flags.store"; import { icpAccountsStore } from "$lib/stores/icp-accounts.store"; import { definedNeuronsStore, neuronsStore } from "$lib/stores/neurons.store"; import { @@ -53,7 +52,6 @@ import { numberToE8s } from "$lib/utils/token.utils"; import { AnonymousIdentity, type Identity } from "@dfinity/agent"; import { Topic, type NeuronId, type NeuronInfo } from "@dfinity/nns"; import { Principal } from "@dfinity/principal"; -import { isNullish } from "@dfinity/utils"; import { get } from "svelte/store"; import { getAuthenticatedIdentity } from "./auth.services"; import { @@ -215,28 +213,13 @@ export const stakeNeuron = async ({ const { ledgerCanisterIdentity, controller, fromSubAccount, identity } = getStakeNeuronPropsByAccount({ account, accountIdentity }); - let newNeuronId: NeuronId; - - // Ledger HW app currently (2023-09-21) doesn't support staking with ICRC-1. - if (get(ENABLE_STAKE_NEURON_ICRC1) && !isHardwareWallet) { - newNeuronId = await governanceApiService.stakeNeuronIcrc1({ - stake, - identity, - ledgerCanisterIdentity, - controller, - fromSubAccount: isNullish(fromSubAccount) - ? undefined - : new Uint8Array(fromSubAccount), - }); - } else { - newNeuronId = await governanceApiService.stakeNeuron({ - stake, - identity, - ledgerCanisterIdentity, - controller, - fromSubAccount, - }); - } + const newNeuronId = await governanceApiService.stakeNeuron({ + stake, + identity, + ledgerCanisterIdentity, + controller, + fromSubAccount, + }); if (loadNeuron) { await getAndLoadNeuron(newNeuronId); diff --git a/frontend/src/tests/lib/services/neurons.services.spec.ts b/frontend/src/tests/lib/services/neurons.services.spec.ts index 2fd999f7028..76bc6e5471f 100644 --- a/frontend/src/tests/lib/services/neurons.services.spec.ts +++ b/frontend/src/tests/lib/services/neurons.services.spec.ts @@ -342,146 +342,6 @@ describe("neurons-services", () => { expect(spyStakeNeuronIcrc1).not.toBeCalled(); }); }); - - describe("with ENABLE_STAKE_NEURON_ICRC1 enabled", () => { - beforeEach(() => { - overrideFeatureFlagsStore.setFlag("ENABLE_STAKE_NEURON_ICRC1", true); - }); - - it("should stake a neuron from main account", async () => { - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - const newNeuronId = await stakeNeuron({ - amount: 10, - account: mockMainAccount, - }); - - expect(spyStakeNeuronIcrc1).toBeCalledWith({ - controller: mockIdentity.getPrincipal(), - fromSubAccount: undefined, - identity: mockIdentity, - ledgerCanisterIdentity: mockIdentity, - stake: BigInt(10 * E8S_PER_ICP), - }); - expect(spyStakeNeuronIcrc1).toBeCalledTimes(1); - expect(newNeuronId).toEqual(mockNeuron.neuronId); - expect(spyStakeNeuron).not.toBeCalled(); - }); - - it("should stake and load a neuron from subaccount", async () => { - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - const newNeuronId = await stakeNeuron({ - amount: 10, - account: mockSubAccount, - }); - - expect(spyStakeNeuronIcrc1).toBeCalledWith({ - controller: mockIdentity.getPrincipal(), - fromSubAccount: new Uint8Array(mockSubAccount.subAccount), - identity: mockIdentity, - ledgerCanisterIdentity: mockIdentity, - stake: BigInt(10 * E8S_PER_ICP), - }); - expect(spyStakeNeuronIcrc1).toBeCalledTimes(1); - expect(newNeuronId).toEqual(mockNeuron.neuronId); - expect(spyStakeNeuron).not.toBeCalled(); - }); - - it("should stake neuron from hardware wallet", async () => { - const mockHardkwareWalletIdentity = { - getPrincipal: () => mockHardwareWalletAccount.principal, - } as unknown as Identity; - setAccountIdentity(mockHardkwareWalletIdentity); - - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - - const newNeuronId = await stakeNeuron({ - amount: 10, - account: mockHardwareWalletAccount, - }); - - // HW doesn't support ICRC-1 staking yet so we expect the non-ICRC1 - // function to be called. - expect(spyStakeNeuron).toBeCalledWith({ - controller: mockHardwareWalletAccount.principal, - identity: new AnonymousIdentity(), - fromSubAccount: undefined, - ledgerCanisterIdentity: mockHardkwareWalletIdentity, - stake: BigInt(10 * E8S_PER_ICP), - }); - expect(spyStakeNeuron).toBeCalledTimes(1); - expect(newNeuronId).toEqual(mockNeuron.neuronId); - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - }); - - it(`stakeNeuron return undefined if amount less than ${ - E8S_PER_ICP / E8S_PER_ICP - } ICP`, async () => { - vi.spyOn(LedgerCanister, "create").mockImplementation(() => - mock() - ); - - const response = await stakeNeuron({ - amount: 0.1, - account: mockMainAccount, - }); - - expect(response).toBeUndefined(); - expectToastError(en.error.amount_not_enough_stake_neuron); - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - expect(spyStakeNeuron).not.toBeCalled(); - }); - - it("stake neuron should return undefined if amount not valid", async () => { - vi.spyOn(LedgerCanister, "create").mockImplementation(() => - mock() - ); - - const response = await stakeNeuron({ - amount: NaN, - account: mockMainAccount, - }); - - expect(response).toBeUndefined(); - expectToastError("Invalid number NaN"); - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - expect(spyStakeNeuron).not.toBeCalled(); - }); - - it("stake neuron should return undefined if not enough funds in account", async () => { - vi.spyOn(LedgerCanister, "create").mockImplementation(() => - mock() - ); - - // 10 ICPs - const amount = 10; - const response = await stakeNeuron({ - amount, - account: { - ...mockMainAccount, - balanceE8s: BigInt(amount - 1), - }, - }); - - expect(response).toBeUndefined(); - expectToastError(en.error.insufficient_funds); - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - expect(spyStakeNeuron).not.toBeCalled(); - }); - - it("should not stake neuron if no identity", async () => { - setNoAccountIdentity(); - - const response = await stakeNeuron({ - amount: 10, - account: mockMainAccount, - }); - - expect(response).toBeUndefined(); - expectToastError("Cannot read properties of null"); - expect(spyStakeNeuronIcrc1).not.toBeCalled(); - expect(spyStakeNeuron).not.toBeCalled(); - }); - }); }); describe("list neurons", () => {