Work around list_neuron not working with HW (#673)

# Motivation

`list_neurons` is broken with the Ledger HW (current version 2.4.9)
because it does not accept a `ListNeurons` with the
`include_empty_neurons_readable_by_caller` even if the field itself is
not set.

The (temporary?) work-around added in this PR is to keep a service which
uses an old version of the `ListNeurons` type to use when the field is
not set anyway.

# Changes

1. Add `old_list_neurons_service.certified.idl.js` which just has what's
necessary for the `list_neurons` method without
`include_empty_neurons_readable_by_caller`.
2. Use this service when calling `list_neurons` with `certified: true`
and `include_empty_neurons_readable_by_caller: undefined`.

# Tests

1. Unit tests added/adapted.
2. Tested manually at
https://qsgjb-riaaa-aaaaa-aaaga-cai.dskloet-ingress.devenv.dfinity.network/

# Todos

- [x] Add entry to changelog (if necessary).

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

CHANGELOG.md

@@ -9,6 +9,7 @@
 ## Fix
 
 - `updateNeuron` to not change the neuron subaccount.
+- `list_neurons` to use old `ListNeurons` type for hardware wallet compatibility.
 
 # 2024.06.11-1630Z
 

packages/nns/candid/old_list_neurons_service.certified.idl.d.ts

@@ -0,0 +1,2 @@
+import type { IDL } from "@dfinity/candid";
+export const idlFactory: IDL.InterfaceFactory;

packages/nns/candid/old_list_neurons_service.certified.idl.js

@@ -0,0 +1,78 @@
+// This file was created manually by taking governance.certified.idl.js and
+// removing everything that isn't needed for `list_neurons` and then removing
+// `include_empty_neurons_readable_by_caller` from `ListNeurons`.
+// The Ledger hardware wallet doesn't support the
+// `include_empty_neurons_readable_by_caller` field, even when it's not set, so
+// we use this service for compatibility with the hardware wallet.
+export const idlFactory = ({ IDL }) => {
+  const NeuronId = IDL.Record({ id: IDL.Nat64 });
+  const Followees = IDL.Record({ followees: IDL.Vec(NeuronId) });
+  const KnownNeuronData = IDL.Record({
+    name: IDL.Text,
+    description: IDL.Opt(IDL.Text),
+  });
+  const NeuronStakeTransfer = IDL.Record({
+    to_subaccount: IDL.Vec(IDL.Nat8),
+    neuron_stake_e8s: IDL.Nat64,
+    from: IDL.Opt(IDL.Principal),
+    memo: IDL.Nat64,
+    from_subaccount: IDL.Vec(IDL.Nat8),
+    transfer_timestamp: IDL.Nat64,
+    block_height: IDL.Nat64,
+  });
+  const BallotInfo = IDL.Record({
+    vote: IDL.Int32,
+    proposal_id: IDL.Opt(NeuronId),
+  });
+  const DissolveState = IDL.Variant({
+    DissolveDelaySeconds: IDL.Nat64,
+    WhenDissolvedTimestampSeconds: IDL.Nat64,
+  });
+  const Neuron = IDL.Record({
+    id: IDL.Opt(NeuronId),
+    staked_maturity_e8s_equivalent: IDL.Opt(IDL.Nat64),
+    controller: IDL.Opt(IDL.Principal),
+    recent_ballots: IDL.Vec(BallotInfo),
+    kyc_verified: IDL.Bool,
+    neuron_type: IDL.Opt(IDL.Int32),
+    not_for_profit: IDL.Bool,
+    maturity_e8s_equivalent: IDL.Nat64,
+    cached_neuron_stake_e8s: IDL.Nat64,
+    created_timestamp_seconds: IDL.Nat64,
+    auto_stake_maturity: IDL.Opt(IDL.Bool),
+    aging_since_timestamp_seconds: IDL.Nat64,
+    hot_keys: IDL.Vec(IDL.Principal),
+    account: IDL.Vec(IDL.Nat8),
+    joined_community_fund_timestamp_seconds: IDL.Opt(IDL.Nat64),
+    dissolve_state: IDL.Opt(DissolveState),
+    followees: IDL.Vec(IDL.Tuple(IDL.Int32, Followees)),
+    neuron_fees_e8s: IDL.Nat64,
+    transfer: IDL.Opt(NeuronStakeTransfer),
+    known_neuron_data: IDL.Opt(KnownNeuronData),
+    spawn_at_timestamp_seconds: IDL.Opt(IDL.Nat64),
+  });
+  const NeuronInfo = IDL.Record({
+    dissolve_delay_seconds: IDL.Nat64,
+    recent_ballots: IDL.Vec(BallotInfo),
+    neuron_type: IDL.Opt(IDL.Int32),
+    created_timestamp_seconds: IDL.Nat64,
+    state: IDL.Int32,
+    stake_e8s: IDL.Nat64,
+    joined_community_fund_timestamp_seconds: IDL.Opt(IDL.Nat64),
+    retrieved_at_timestamp_seconds: IDL.Nat64,
+    known_neuron_data: IDL.Opt(KnownNeuronData),
+    voting_power: IDL.Nat64,
+    age_seconds: IDL.Nat64,
+  });
+  const ListNeurons = IDL.Record({
+    neuron_ids: IDL.Vec(IDL.Nat64),
+    include_neurons_readable_by_caller: IDL.Bool,
+  });
+  const ListNeuronsResponse = IDL.Record({
+    neuron_infos: IDL.Vec(IDL.Tuple(IDL.Nat64, NeuronInfo)),
+    full_neurons: IDL.Vec(Neuron),
+  });
+  return IDL.Service({
+    list_neurons: IDL.Func([ListNeurons], [ListNeuronsResponse], []),
+  });
+};

packages/nns/src/governance.canister.spec.ts

@@ -425,31 +425,39 @@ describe("GovernanceCanister", () => {
   describe("GovernanceCanister.listNeurons", () => {
     it("list user neurons", async () => {
       const service = mock<ActorSubclass<GovernanceService>>();
-      service.list_neurons.mockResolvedValue(mockListNeuronsResponse);
+      const certifiedService = mock<ActorSubclass<GovernanceService>>();
+      const oldService = mock<ActorSubclass<GovernanceService>>();
+      certifiedService.list_neurons.mockResolvedValue(mockListNeuronsResponse);
 
       const governance = GovernanceCanister.create({
-        certifiedServiceOverride: service,
+        certifiedServiceOverride: certifiedService,
         serviceOverride: service,
+        oldListNeuronsServiceOverride: oldService,
       });
       const neurons = await governance.listNeurons({
         certified: true,
+        includeEmptyNeurons: true,
       });
-      expect(service.list_neurons).toBeCalledWith({
+      expect(certifiedService.list_neurons).toBeCalledWith({
         neuron_ids: new BigUint64Array(),
         include_neurons_readable_by_caller: true,
-        include_empty_neurons_readable_by_caller: [],
+        include_empty_neurons_readable_by_caller: [true],
       });
-      expect(service.list_neurons).toBeCalledTimes(1);
+      expect(certifiedService.list_neurons).toBeCalledTimes(1);
       expect(neurons.length).toBe(1);
+      expect(service.list_neurons).not.toBeCalled();
+      expect(oldService.list_neurons).not.toBeCalled();
     });
 
     it("list user neurons excluding empty neurons", async () => {
       const service = mock<ActorSubclass<GovernanceService>>();
+      const oldService = mock<ActorSubclass<GovernanceService>>();
       service.list_neurons.mockResolvedValue(mockListNeuronsResponse);
 
       const governance = GovernanceCanister.create({
         certifiedServiceOverride: service,
         serviceOverride: service,
+        oldListNeuronsServiceOverride: oldService,
       });
       const neurons = await governance.listNeurons({
         certified: true,
@@ -463,6 +471,55 @@ describe("GovernanceCanister", () => {
       expect(service.list_neurons).toBeCalledTimes(1);
       expect(neurons.length).toBe(1);
     });
+
+    it("should use old service when not excluding empty neurons", async () => {
+      const service = mock<ActorSubclass<GovernanceService>>();
+      const oldService = mock<ActorSubclass<GovernanceService>>();
+      oldService.list_neurons.mockResolvedValue(mockListNeuronsResponse);
+
+      const governance = GovernanceCanister.create({
+        certifiedServiceOverride: service,
+        serviceOverride: service,
+        oldListNeuronsServiceOverride: oldService,
+      });
+      const neurons = await governance.listNeurons({
+        certified: true,
+      });
+      expect(oldService.list_neurons).toBeCalledWith({
+        neuron_ids: new BigUint64Array(),
+        include_neurons_readable_by_caller: true,
+        // The field is present in the argument but ignored by the old service.
+        include_empty_neurons_readable_by_caller: [],
+      });
+      expect(oldService.list_neurons).toBeCalledTimes(1);
+      expect(neurons.length).toBe(1);
+      expect(service.list_neurons).not.toBeCalled();
+    });
+
+    it("should not use old service when not certified", async () => {
+      const certifiedService = mock<ActorSubclass<GovernanceService>>();
+      const service = mock<ActorSubclass<GovernanceService>>();
+      const oldService = mock<ActorSubclass<GovernanceService>>();
+      service.list_neurons.mockResolvedValue(mockListNeuronsResponse);
+
+      const governance = GovernanceCanister.create({
+        certifiedServiceOverride: certifiedService,
+        serviceOverride: service,
+        oldListNeuronsServiceOverride: oldService,
+      });
+      const neurons = await governance.listNeurons({
+        certified: false,
+      });
+      expect(service.list_neurons).toBeCalledWith({
+        neuron_ids: new BigUint64Array(),
+        include_neurons_readable_by_caller: true,
+        include_empty_neurons_readable_by_caller: [],
+      });
+      expect(service.list_neurons).toBeCalledTimes(1);
+      expect(neurons.length).toBe(1);
+      expect(oldService.list_neurons).not.toBeCalled();
+      expect(certifiedService.list_neurons).not.toBeCalled();
+    });
   });
 
   describe("GovernanceCanister.listProposals", () => {
@@ -613,6 +670,7 @@ describe("GovernanceCanister", () => {
       const governance = GovernanceCanister.create({
         certifiedServiceOverride: service,
         serviceOverride: service,
+        oldListNeuronsServiceOverride: service,
       });
 
       service.list_neurons.mockResolvedValue(

packages/nns/src/governance.canister.ts

@@ -1,4 +1,5 @@
 import type { ActorSubclass, Agent } from "@dfinity/agent";
+import { Actor } from "@dfinity/agent";
 import type { LedgerCanister } from "@dfinity/ledger-icp";
 import {
   AccountIdentifier,
@@ -30,6 +31,7 @@ import type {
 } from "../candid/governance";
 import { idlFactory as certifiedIdlFactory } from "../candid/governance.certified.idl";
 import { idlFactory } from "../candid/governance.idl";
+import { idlFactory as oldListNeuronsCertifiedIdlFactory } from "../candid/old_list_neurons_service.certified.idl";
 import {
   fromClaimOrRefreshNeuronRequest,
   fromListNeurons,
@@ -93,11 +95,13 @@ export class GovernanceCanister {
     private readonly canisterId: Principal,
     private readonly service: ActorSubclass<GovernanceService>,
     private readonly certifiedService: ActorSubclass<GovernanceService>,
+    private readonly oldListNeuronsCertifiedService: ActorSubclass<GovernanceService>,
     private readonly agent: Agent,
   ) {
     this.canisterId = canisterId;
     this.service = service;
     this.certifiedService = certifiedService;
+    this.oldListNeuronsCertifiedService = oldListNeuronsCertifiedService;
     this.agent = agent;
   }
 
@@ -115,7 +119,20 @@ export class GovernanceCanister {
         certifiedIdlFactory,
       });
 
-    return new GovernanceCanister(canisterId, service, certifiedService, agent);
+    const oldListNeuronsCertifiedService =
+      options.oldListNeuronsServiceOverride ??
+      Actor.createActor<GovernanceService>(oldListNeuronsCertifiedIdlFactory, {
+        agent,
+        canisterId,
+      });
+
+    return new GovernanceCanister(
+      canisterId,
+      service,
+      certifiedService,
+      oldListNeuronsCertifiedService,
+      agent,
+    );
   }
 
   /**
@@ -138,8 +155,16 @@ export class GovernanceCanister {
     includeEmptyNeurons?: boolean;
   }): Promise<NeuronInfo[]> => {
     const rawRequest = fromListNeurons({ neuronIds, includeEmptyNeurons });
-    const raw_response =
-      await this.getGovernanceService(certified).list_neurons(rawRequest);
+    // The Ledger app version 2.4.9 does not support
+    // include_empty_neurons_readable_by_caller, even when the field is absent,
+    // so we use the old service (which does not have this field) if possible,
+    // in case the call will be signed by the Ledger device. We only have a
+    // certified version of the old service.
+    const useOldMethod = isNullish(includeEmptyNeurons) && certified;
+    const service = useOldMethod
+      ? this.oldListNeuronsCertifiedService
+      : this.getGovernanceService(certified);
+    const raw_response = await service.list_neurons(rawRequest);
     return toArrayOfNeuronInfo({
       response: raw_response,
       canisterId: this.canisterId,

packages/nns/src/types/governance.options.ts

@@ -1,3 +1,4 @@
+import type { ActorSubclass } from "@dfinity/agent";
 import type { CanisterOptions } from "@dfinity/utils";
 import type { _SERVICE as GovernanceService } from "../../candid/governance";
 
@@ -6,4 +7,5 @@ export interface GovernanceCanisterOptions
   // Ledger IC App needs requests built with Protobuf.
   // This flag ensures that the methods use Protobuf.
   hardwareWallet?: boolean;
+  oldListNeuronsServiceOverride?: ActorSubclass<GovernanceService>;
 }