Skip to content

Commit

Permalink
Fix not loaded balances on failed imported (#5555)
Browse files Browse the repository at this point in the history
# Motivation

When testing under real network conditions (not locally), there’s a high
chance that when a call fails, some unrelated balances in the tokens
table will display a loading state, even though all of them were
successfully loaded. The reason for this behaviour is that, on a failed
call (e.g. failed imported token) in the `loadAccounts` function, the
whole accounts store gets reset.

To reproduce, there should be at least one failed imported token in the
table.

Presumably, this was implemented this way because initially, [only a
single ICRC1
token](https://github.com/dfinity/nns-dapp/pull/1918/files#diff-4be3667b0d4ed46a702674d48f6e1e4c2b18ef0bf68e2e89170d78ea1bf22758R48)
was supported by the NNS dapp.

To avoid this, we now reset the store only for the failed account.

# Changes

- Added `resetUniverse` to `icrcAccountsStore`.
- Use `resetUniverse` instead of `reset` on failed account call.

# Tests

- Updated.

# Todos

- [ ] Add entry to changelog (if necessary).
Not necessary.
  • Loading branch information
mstrasinskis authored Oct 1, 2024
1 parent 6f0e85f commit 0521511
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 1 deletion.
2 changes: 1 addition & 1 deletion frontend/src/lib/services/icrc-accounts.services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export const loadAccounts = async ({
}

// hide unproven data
icrcAccountsStore.reset();
icrcAccountsStore.resetUniverse(ledgerCanisterId);
icrcTransactionsStore.resetUniverse(ledgerCanisterId);

if (
Expand Down
11 changes: 11 additions & 0 deletions frontend/src/lib/stores/icrc-accounts.store.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { Account } from "$lib/types/account";
import { removeKeys } from "$lib/utils/utils";
import type { Principal } from "@dfinity/principal";
import type { Readable } from "svelte/store";
import { writable } from "svelte/store";
Expand All @@ -21,6 +22,7 @@ export interface IcrcAccountsStore extends Readable<IcrcAccountStoreData> {
accounts: IcrcAccounts;
ledgerCanisterId: Principal;
}) => void;
resetUniverse: (ledgerCanisterId: Principal) => void;
reset: () => void;
}

Expand Down Expand Up @@ -72,6 +74,15 @@ const initIcrcAccountsStore = (): IcrcAccountsStore => {
}));
},

resetUniverse: (ledgerCanisterId: Principal) => {
update((currentState: IcrcAccountStoreData) =>
removeKeys({
obj: currentState,
keysToRemove: [ledgerCanisterId.toText()],
})
);
},

reset: () => set(initialAccounts),
};
};
Expand Down
40 changes: 40 additions & 0 deletions frontend/src/tests/lib/services/icrc-accounts.services.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,46 @@ describe("icrc-accounts-services", () => {
).toBeUndefined();
});

it("should remove only failed account from the store", async () => {
vi.spyOn(ledgerApi, "queryIcrcBalance")
.mockResolvedValueOnce(mockCkBTCMainAccount.balanceUlps)
.mockRejectedValueOnce(new Error());
icrcAccountsStore.set({
accounts: {
accounts: [mockCkBTCMainAccount],
certified: true,
},
ledgerCanisterId: ledgerCanisterId,
});
icrcAccountsStore.set({
accounts: {
accounts: [mockCkBTCMainAccount],
certified: false,
},
ledgerCanisterId: ledgerCanisterId2,
});

expect(get(icrcAccountsStore)).toEqual({
[ledgerCanisterId.toText()]: {
accounts: [mockCkBTCMainAccount],
certified: true,
},
[ledgerCanisterId2.toText()]: {
accounts: [mockCkBTCMainAccount],
certified: false,
},
});

await loadAccounts({ ledgerCanisterId });

expect(get(icrcAccountsStore)).toEqual({
[ledgerCanisterId2.toText()]: {
accounts: [mockCkBTCMainAccount],
certified: false,
},
});
});

it("displays a toast on error", async () => {
vi.spyOn(ledgerApi, "queryIcrcBalance").mockRejectedValue(
new Error("test")
Expand Down
43 changes: 43 additions & 0 deletions frontend/src/tests/lib/stores/icrc-accounts.store.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
mockSnsMainAccount,
mockSnsSubAccount,
} from "$tests/mocks/sns-accounts.mock";
import { principal } from "$tests/mocks/sns-projects.mock";
import { get } from "svelte/store";

describe("icrc Accounts store", () => {
Expand Down Expand Up @@ -88,4 +89,46 @@ describe("icrc Accounts store", () => {
certified: true,
});
});

it("should reset for a project", () => {
const ledgerCanisterId1 = principal(0);
const ledgerCanisterId2 = principal(1);
const accounts: Account[] = [mockSnsMainAccount, mockSnsSubAccount];
icrcAccountsStore.set({
accounts: {
accounts,
certified: true,
},
ledgerCanisterId: ledgerCanisterId1,
});
icrcAccountsStore.set({
accounts: {
accounts,
certified: false,
},
ledgerCanisterId: ledgerCanisterId2,
});

expect(get(icrcAccountsStore)).toEqual({
[ledgerCanisterId1.toText()]: {
accounts,
certified: true,
},
[ledgerCanisterId2.toText()]: {
accounts,
certified: false,
},
});

icrcAccountsStore.resetUniverse(ledgerCanisterId1);
expect(get(icrcAccountsStore)).toEqual({
[ledgerCanisterId2.toText()]: {
accounts,
certified: false,
},
});

icrcAccountsStore.resetUniverse(ledgerCanisterId2);
expect(get(icrcAccountsStore)).toEqual({});
});
});

0 comments on commit 0521511

Please sign in to comment.