Skip to content

Commit

Permalink
FOLLOW-244: Refresh neuron from NeuronDetail page (#5697)
Browse files Browse the repository at this point in the history
# Motivation

Increasing a neurons' stake is a 2 step process:
1. Transfer ICP to the neuron's account.
2. Notify the governance canister to refresh the neurons.

Because this process can be interrupted after step 1, we need to be able
to finish a half completed process.
Currently the nns-dapp canister has a list of neuron accounts for each
user and it watches every ICP ledger transaction to see if it is to a
neuron account and then it refreshes that neuron.
We want to stop keeping this list in the nns-dapp canister and we want
to stop watching every transaction.
So instead we will check from the frontend if a neuron's stake matches
its account's balance and refresh if needed.

In this PR we call the service added in
#5696 from the `NnsNeuronDetail`
component.


# Changes

1. Call `refreshNeuronIfNeeded` from `NnsNeuronDetail` once the neuron
is loaded.

# Tests

1. Add missing `queryNeuron` and `claimOrRefreshNeuron` to fake
governance API.
2. Add unit tests.
3. Manually tested after removing the corresponding logic from the
canister.

# Todos

- [x] Add entry to changelog (if necessary).
  • Loading branch information
dskloetd authored Oct 28, 2024
1 parent f89646b commit 82e31e6
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG-Nns-Dapp-unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ proposal is successful, the changes it released will be moved from this file to

* Provide better error messages when the transaction timestamp is off.
* A link to the imported tokens documentation page.
* Refresh NNS neurons from neuron details page if needed.

#### Changed

Expand Down
10 changes: 9 additions & 1 deletion frontend/src/lib/pages/NnsNeuronDetail.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import { AppPath } from "$lib/constants/routes.constants";
import { pageStore } from "$lib/derived/page.derived";
import NnsNeuronModals from "$lib/modals/neurons/NnsNeuronModals.svelte";
import { listNeurons } from "$lib/services/neurons.services";
import {
listNeurons,
refreshNeuronIfNeeded,
} from "$lib/services/neurons.services";
import { loadLatestRewardEvent } from "$lib/services/nns-reward-event.services";
import { i18n } from "$lib/stores/i18n";
import { neuronsStore } from "$lib/stores/neurons.store";
Expand All @@ -38,6 +41,7 @@
} from "$lib/utils/neuron.utils";
import { Island } from "@dfinity/gix-components";
import type { NeuronId, NeuronInfo } from "@dfinity/nns";
import { nonNullish } from "@dfinity/utils";
import { onMount, setContext } from "svelte";
import { writable } from "svelte/store";
Expand Down Expand Up @@ -139,6 +143,10 @@
setContext<NnsNeuronContext>(NNS_NEURON_CONTEXT_KEY, {
store: selectedNeuronStore,
});
$: if (nonNullish(neuron)) {
refreshNeuronIfNeeded(neuron);
}
</script>

<TestIdWrapper testId="nns-neuron-detail-component">
Expand Down
36 changes: 34 additions & 2 deletions frontend/src/tests/fakes/governance-api.fake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,31 @@ import type {
ApiMergeNeuronsParams,
ApiQueryParams,
} from "$lib/api/governance.api";
import { queryAccountBalance } from "$lib/api/icp-ledger.api";
import { mockIdentity } from "$tests/mocks/auth.store.mock";
import { mockNeuron } from "$tests/mocks/neurons.mock";
import { mockRewardEvent } from "$tests/mocks/nns-reward-event.mock";
import {
installImplAndBlockRest,
makePausable,
} from "$tests/utils/module.test-utils";
import type { Identity } from "@dfinity/agent";
import type { KnownNeuron, NeuronInfo, RewardEvent } from "@dfinity/nns";
import { AnonymousIdentity, type Identity } from "@dfinity/agent";
import type {
KnownNeuron,
NeuronId,
NeuronInfo,
RewardEvent,
} from "@dfinity/nns";
import { NeuronState, NeuronType } from "@dfinity/nns";
import { isNullish, nonNullish } from "@dfinity/utils";

const modulePath = "$lib/api/governance.api";
const fakeFunctions = {
queryNeurons,
queryNeuron,
queryKnownNeurons,
queryLastestRewardEvent,
claimOrRefreshNeuron,
startDissolving,
mergeNeurons,
simulateMergeNeurons,
Expand Down Expand Up @@ -68,6 +76,13 @@ async function queryNeurons({
return getNeurons(identity);
}

async function queryNeuron({
neuronId,
identity,
}: ApiManageNeuronParams): Promise<NeuronInfo> {
return getNeuron({ identity, neuronId });
}

async function queryKnownNeurons({
identity: _,
certified: __,
Expand All @@ -82,6 +97,23 @@ async function queryLastestRewardEvent({
return latestRewardEvent;
}

async function claimOrRefreshNeuron({
neuronId,
identity,
}: ApiManageNeuronParams): Promise<NeuronId | undefined> {
const neuron = getNeuron({ identity, neuronId });
if (isNullish(neuron)) {
return undefined;
}
const neuronAccountBalance = await queryAccountBalance({
icpAccountIdentifier: neuron?.fullNeuron.accountIdentifier,
identity: new AnonymousIdentity(),
certified: false,
});
neuron.fullNeuron.cachedNeuronStake = neuronAccountBalance;
return neuronId;
}

async function startDissolving({
neuronId,
identity,
Expand Down
58 changes: 56 additions & 2 deletions frontend/src/tests/lib/pages/NnsNeuronDetail.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
import * as governanceApi from "$lib/api/governance.api";
import * as icpLedgerApi from "$lib/api/icp-ledger.api";
import { OWN_CANISTER_ID } from "$lib/constants/canister-ids.constants";
import NnsNeuronDetail from "$lib/pages/NnsNeuronDetail.svelte";
import { checkedNeuronSubaccountsStore } from "$lib/stores/checked-neurons.store";
import { neuronsStore } from "$lib/stores/neurons.store";
import { voteRegistrationStore } from "$lib/stores/vote-registration.store";
import * as fakeGovernanceApi from "$tests/fakes/governance-api.fake";
import { resetIdentity } from "$tests/mocks/auth.store.mock";
import { mockIdentity, resetIdentity } from "$tests/mocks/auth.store.mock";
import { mockVoteRegistration } from "$tests/mocks/proposal.mock";
import { NnsNeuronDetailPo } from "$tests/page-objects/NnsNeuronDetail.page-object";
import { JestPageObjectElement } from "$tests/page-objects/jest.page-object";
Expand All @@ -23,9 +26,12 @@ describe("NeuronDetail", () => {
fakeGovernanceApi.install();

const neuronId = 314n;
const neuronStake = 300_000_000n;
const latestRewardEventTimestamp = Math.floor(
new Date("1992-05-22T21:00:00").getTime() / 1000
);
const spyQueryAccountBalance = vi.spyOn(icpLedgerApi, "queryAccountBalance");

const renderComponent = async (neuronId: string) => {
const { container } = render(NnsNeuronDetail, {
props: {
Expand All @@ -39,15 +45,18 @@ describe("NeuronDetail", () => {
};

beforeEach(() => {
vi.clearAllMocks();
resetIdentity();
neuronsStore.reset();
voteRegistrationStore.reset();
fakeGovernanceApi.addNeuronWith({ neuronId });
checkedNeuronSubaccountsStore.reset();
fakeGovernanceApi.addNeuronWith({ neuronId, stake: neuronStake });
fakeGovernanceApi.addNeuronWith({ neuronId: 1234n });
fakeGovernanceApi.setLatestRewardEvent({
rounds_since_last_distribution: [3n] as [bigint],
actual_timestamp_seconds: BigInt(latestRewardEventTimestamp),
});
spyQueryAccountBalance.mockResolvedValue(neuronStake);
});

it("renders new sections", async () => {
Expand Down Expand Up @@ -121,4 +130,49 @@ describe("NeuronDetail", () => {
"May 19, 1992"
);
});

it("should not refresh neuron if not needed", async () => {
expect(spyQueryAccountBalance).toBeCalledTimes(0);

await renderComponent(`${neuronId}`);
await runResolvedPromises();

expect(spyQueryAccountBalance).toBeCalledTimes(1);
expect(governanceApi.claimOrRefreshNeuron).toBeCalledTimes(0);

const newNeuron = fakeGovernanceApi.getNeuron({
identity: mockIdentity,
neuronId,
});
expect(newNeuron.fullNeuron.cachedNeuronStake).toEqual(neuronStake);
});

it("should refresh neuron if needed", async () => {
const stakeIncrease = 100_000_000n;
spyQueryAccountBalance.mockResolvedValue(neuronStake + stakeIncrease);

expect(spyQueryAccountBalance).toBeCalledTimes(0);

const oldNeuron = fakeGovernanceApi.getNeuron({
identity: mockIdentity,
neuronId,
});
expect(oldNeuron.fullNeuron.cachedNeuronStake).toEqual(neuronStake);

await renderComponent(`${neuronId}`);
await runResolvedPromises();

// The balance is queried once by the service and once by the fake
// governance API.
expect(spyQueryAccountBalance).toBeCalledTimes(2);
expect(governanceApi.claimOrRefreshNeuron).toBeCalledTimes(1);

const newNeuron = fakeGovernanceApi.getNeuron({
identity: mockIdentity,
neuronId,
});
expect(newNeuron.fullNeuron.cachedNeuronStake).toEqual(
neuronStake + stakeIncrease
);
});
});
5 changes: 4 additions & 1 deletion frontend/src/tests/lib/routes/NeuronDetail.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as agent from "$lib/api/agent.api";
import * as icpLedgerApi from "$lib/api/icp-ledger.api";
import * as icrcLedgerApi from "$lib/api/icrc-ledger.api";
import { OWN_CANISTER_ID_TEXT } from "$lib/constants/canister-ids.constants";
import { AppPath } from "$lib/constants/routes.constants";
Expand All @@ -23,6 +24,7 @@ import { waitFor } from "@testing-library/dom";
import { render } from "@testing-library/svelte";
import { mock } from "vitest-mock-extended";

vi.mock("$lib/api/icp-ledger.api");
vi.mock("$lib/api/icrc-ledger.api");
vi.mock("$lib/api/sns-aggregator.api");
vi.mock("$lib/api/governance.api");
Expand All @@ -37,7 +39,7 @@ const nnsProps = {
neuronId: testNnsNeuronId,
};

const blockedPaths = ["$lib/api/icrc-ledger.api"];
const blockedPaths = ["$lib/api/icrc-ledger.api", "$lib/api/icp-ledger.api"];

describe("NeuronDetail", () => {
blockAllCallsTo(blockedPaths);
Expand All @@ -51,6 +53,7 @@ describe("NeuronDetail", () => {
resetSnsProjects();
vi.spyOn(agent, "createAgent").mockResolvedValue(mock<HttpAgent>());
vi.spyOn(icrcLedgerApi, "queryIcrcBalance").mockResolvedValue(0n);
vi.spyOn(icpLedgerApi, "queryAccountBalance").mockResolvedValue(0n);
});

describe("nns neuron", () => {
Expand Down

0 comments on commit 82e31e6

Please sign in to comment.