Skip to content

Commit

Permalink
Store SNS neurons after querying for actionable proposals (#5182)
Browse files Browse the repository at this point in the history
# Motivation

We load neurons for all SNSes for actionable proposals. If we put them
in the store they can be used to display on the neurons table etc.
There was a comment saying we shouldn't do this because it prevents
`syncSnsNeurons` from checking neurons but
1. It doesn't actually prevent anything, and
2. We no longer check neurons in `syncSnsNeurons`.

# Changes

1. Rename `loadNeurons` to `loadSnsNeurons` so the context is clearer
when the function is used in another file.
2. Use `loadSnsNeurons` to load neurons into the store instead of just
querying them in `actionable-sns-proposals.services.ts`.
3. Drive-by: Remove a comment from `syncSnsNeurons` about checking
neurons which it no longer does.

# Tests

Unit test adapted to check that the `snsNeuronsStore` is populated.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary
  • Loading branch information
dskloetd authored Jul 11, 2024
1 parent f159c85 commit de6e4c0
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 33 deletions.
34 changes: 12 additions & 22 deletions frontend/src/lib/services/actionable-sns-proposals.services.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { queryProposals, querySnsNeurons } from "$lib/api/sns-governance.api";
import { queryProposals } from "$lib/api/sns-governance.api";
import { MAX_ACTIONABLE_REQUEST_COUNT } from "$lib/constants/constants";
import { DEFAULT_SNS_PROPOSALS_PAGE_SIZE } from "$lib/constants/sns-proposals.constants";
import { snsProjectsCommittedStore } from "$lib/derived/sns/sns-projects.derived";
import { getAuthenticatedIdentity } from "$lib/services/auth.services";
import { loadSnsNeurons } from "$lib/services/sns-neurons.services";
import {
actionableSnsProposalsStore,
failedActionableSnsesStore,
Expand All @@ -18,7 +19,7 @@ import { Principal } from "@dfinity/principal";
import type { SnsNeuron } from "@dfinity/sns";
import { SnsProposalRewardStatus } from "@dfinity/sns";
import type { ProposalData } from "@dfinity/sns/dist/candid/sns_governance";
import { fromNullable, nonNullish } from "@dfinity/utils";
import { fromNullable, isNullish } from "@dfinity/utils";
import { get } from "svelte/store";

export const loadActionableSnsProposals = async () => {
Expand Down Expand Up @@ -57,14 +58,9 @@ export const loadActionableProposalsForSns = async (
return;
}

const neurons =
get(snsNeuronsStore)[rootCanisterIdText]?.neurons ??
// Fetch neurons if they are not in the store, but do not populate the store.
// Otherwise, it will skip calling of the `syncSnsNeurons` function to check neurons stake against the balance of the subaccount.
(await queryNeurons({
rootCanisterId: rootCanisterIdText,
identity,
}));
const neurons = await queryNeurons({
rootCanisterId,
});

// It's not possible to filter out votable proposals w/o ballots
const votableProposals = includeBallotsByCaller
Expand Down Expand Up @@ -93,21 +89,15 @@ export const loadActionableProposalsForSns = async (

const queryNeurons = async ({
rootCanisterId,
identity,
}: {
rootCanisterId: string;
identity: Identity;
rootCanisterId: Principal;
}): Promise<SnsNeuron[]> => {
const storeNeurons = get(snsNeuronsStore)[rootCanisterId];
if (nonNullish(storeNeurons?.neurons)) {
return storeNeurons.neurons;
const getStoreNeurons = () =>
get(snsNeuronsStore)[rootCanisterId.toText()]?.neurons;
if (isNullish(getStoreNeurons())) {
await loadSnsNeurons({ rootCanisterId, certified: false });
}

return await querySnsNeurons({
identity,
rootCanisterId: Principal.fromText(rootCanisterId),
certified: false,
});
return getStoreNeurons();
};

/** Fetches proposals that accept votes */
Expand Down
12 changes: 4 additions & 8 deletions frontend/src/lib/services/sns-neurons.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,9 @@ import { loadSnsAccounts } from "./sns-accounts.services";
import { queryAndUpdate } from "./utils.services";

/**
* Loads sns neurons in store and checks neurons's stake against the balance of the subaccount.
* Loads sns neurons in store.
* (Loads sns parameters when not already in the store)
*
* On update, it will check whether there are neurons that need to be refreshed or claimed.
* A neuron needs to be refreshed if the balance of the subaccount doesn't match the stake of the neuron.
* A neuron needs to be claimed if there is a subaccount with balance and no neuron.
*
* @param {Principal} rootCanisterId
* @returns {void}
*/
Expand Down Expand Up @@ -116,7 +112,7 @@ export const syncSnsNeurons = async (
return Promise.all([snsParametersRequest, syncSnsNeuronsRequest]).then();
};

export const loadNeurons = async ({
export const loadSnsNeurons = async ({
rootCanisterId,
certified,
}: {
Expand Down Expand Up @@ -321,7 +317,7 @@ export const splitNeuron = async ({
// TODO: Get identity depending on account to support HW accounts
const identity = await getSnsNeuronIdentity();
// reload neurons (should be actual for nextMemo calculation)
await loadNeurons({
await loadSnsNeurons({
rootCanisterId,
certified: true,
});
Expand Down Expand Up @@ -531,7 +527,7 @@ export const stakeNeuron = async ({
});
await Promise.all([
loadSnsAccounts({ rootCanisterId }),
loadNeurons({ rootCanisterId, certified: true }),
loadSnsNeurons({ rootCanisterId, certified: true }),
]);
return { success: true };
} catch (err) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
failedActionableSnsesStore,
} from "$lib/stores/actionable-sns-proposals.store";
import { authStore } from "$lib/stores/auth.store";
import { snsNeuronsStore } from "$lib/stores/sns-neurons.store";
import { enumValues } from "$lib/utils/enum.utils";
import { getSnsNeuronIdAsHexString } from "$lib/utils/sns-neuron.utils";
import { snsProposalId } from "$lib/utils/sns-proposals.utils";
Expand Down Expand Up @@ -39,6 +40,7 @@ describe("actionable-sns-proposals.services", () => {
beforeEach(() => {
vi.restoreAllMocks();
failedActionableSnsesStore.resetForTesting();
snsNeuronsStore.reset();
});

describe("loadActionableProposalsForSns", () => {
Expand Down Expand Up @@ -166,6 +168,8 @@ describe("actionable-sns-proposals.services", () => {
mockSnsProjectsCommittedStore([rootCanisterId1, rootCanisterId2]);
expect(spyQuerySnsNeurons).not.toHaveBeenCalled();

expect(get(snsNeuronsStore)).toEqual({});

await loadActionableSnsProposals();

expect(spyQuerySnsNeurons).toHaveBeenCalledTimes(2);
Expand All @@ -179,6 +183,17 @@ describe("actionable-sns-proposals.services", () => {
rootCanisterId: rootCanisterId2,
certified: false,
});

expect(get(snsNeuronsStore)).toEqual({
[rootCanisterId1.toText()]: {
certified: false,
neurons: [neuron],
},
[rootCanisterId2.toText()]: {
certified: false,
neurons: [neuron],
},
});
});

it("should query proposals per sns with accept rewards status only", async () => {
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/tests/lib/services/sns-neurons.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const {
removeHotkey,
splitNeuron,
stakeNeuron,
loadNeurons,
loadSnsNeurons,
addFollowee,
} = services;

Expand Down Expand Up @@ -194,7 +194,7 @@ describe("sns-neurons-services", () => {
});
});

describe("loadNeurons", () => {
describe("loadSnsNeurons", () => {
beforeEach(() => {
snsNeuronsStore.reset();
vi.spyOn(console, "error").mockImplementation(() => undefined);
Expand All @@ -204,7 +204,7 @@ describe("sns-neurons-services", () => {
const spyQuery = vi
.spyOn(governanceApi, "querySnsNeurons")
.mockImplementation(() => Promise.resolve([neuron]));
await loadNeurons({ rootCanisterId: mockPrincipal, certified: true });
await loadSnsNeurons({ rootCanisterId: mockPrincipal, certified: true });

await tick();
const store = get(snsNeuronsStore);
Expand Down

0 comments on commit de6e4c0

Please sign in to comment.