Skip to content

Commit

Permalink
Remove ICRC-1 neuron staking logic (#3946)
Browse files Browse the repository at this point in the history
# 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).
  • Loading branch information
dskloetd committed Dec 4, 2023
1 parent d90e93f commit 9630944
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 164 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 7 additions & 24 deletions frontend/src/lib/services/neurons.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
140 changes: 0 additions & 140 deletions frontend/src/tests/lib/services/neurons.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<LedgerCanister>()
);

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<LedgerCanister>()
);

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<LedgerCanister>()
);

// 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", () => {
Expand Down

0 comments on commit 9630944

Please sign in to comment.