fix(wallet): dynamicChangeResolver gives preference to lower derivati…

…on indices
input-output-hk · Sep 11, 2024 · efc9572 · efc9572
1 parent 7c2d694
commit efc9572
Show file tree

Hide file tree

Showing 3 changed files with 135 additions and 5 deletions.
diff --git a/packages/wallet/src/services/ChangeAddress/DynamicChangeAddressResolver.ts b/packages/wallet/src/services/ChangeAddress/DynamicChangeAddressResolver.ts
@@ -284,6 +284,33 @@ export const delegationMatchesPortfolio = (
 /** Gets the current delegation portfolio. */
 export type GetDelegationPortfolio = () => Promise<Cardano.Cip17DelegationPortfolio | null>;
 
+/**
+ * Sorts an array of addresses by their primary index and, if available, by the
+ * index of their stakeKeyDerivationPath.
+ *
+ * @param addresses - The array of addresses to sort.
+ * @returns A new sorted array of addresses.
+ */
+const sortAddresses = (addresses: GroupedAddress[]): GroupedAddress[] =>
+  [...addresses].sort((a, b) => {
+    if (a.index !== b.index) {
+      return a.index - b.index;
+    }
+
+    if (a.stakeKeyDerivationPath && b.stakeKeyDerivationPath) {
+      return a.stakeKeyDerivationPath.index - b.stakeKeyDerivationPath.index;
+    }
+
+    if (a.stakeKeyDerivationPath && !b.stakeKeyDerivationPath) {
+      return -1;
+    }
+    if (!a.stakeKeyDerivationPath && b.stakeKeyDerivationPath) {
+      return 1;
+    }
+
+    return 0;
+  });
+
 /** Resolves the address to be used for change. */
 export class DynamicChangeAddressResolver implements ChangeAddressResolver {
   readonly #getDelegationPortfolio: GetDelegationPortfolio;
@@ -315,7 +342,8 @@ export class DynamicChangeAddressResolver implements ChangeAddressResolver {
   async resolve(selection: Selection): Promise<Array<Cardano.TxOut>> {
     const delegationDistribution = [...(await firstValueFrom(this.#delegationDistribution)).values()];
     let portfolio = await this.#getDelegationPortfolio();
-    const addresses = await firstValueFrom(this.#addresses$);
+    const addresses = sortAddresses(await firstValueFrom(this.#addresses$));
+
     let updatedChange = [...selection.change];
 
     if (addresses.length === 0) throw new InvalidStateError('The wallet has no known addresses.');

diff --git a/packages/wallet/test/services/ChangeAddress/DynamicChangeAddressResolver.test.ts b/packages/wallet/test/services/ChangeAddress/DynamicChangeAddressResolver.test.ts
@@ -25,8 +25,8 @@ import {
   rewardAccount_0,
   rewardAccount_1,
   rewardAccount_2,
-  rewardAccount_3
-} from './testData';
+  rewardAccount_3, unorderedKnownAddresses$
+} from "./testData";
 import { logger } from '@cardano-sdk/util-dev';
 
 describe('delegationMatchesPortfolio', () => {
@@ -154,7 +154,7 @@ describe('DynamicChangeAddressResolver', () => {
 
   it('adds all change outputs at payment_stake address 0 if the wallet is currently not delegating to any pool', async () => {
     const changeAddressResolver = new DynamicChangeAddressResolver(
-      knownAddresses$,
+      unorderedKnownAddresses$,
       createMockDelegateTracker(new Map<Cardano.PoolId, DelegatedStake>([])).distribution$,
       getNullDelegationPortfolio,
       logger
@@ -191,7 +191,7 @@ describe('DynamicChangeAddressResolver', () => {
 
   it('distributes change equally between the currently delegated addresses if no portfolio is given, ', async () => {
     const changeAddressResolver = new DynamicChangeAddressResolver(
-      knownAddresses$,
+      unorderedKnownAddresses$,
       createMockDelegateTracker(
         new Map<Cardano.PoolId, DelegatedStake>([
           [

diff --git a/packages/wallet/test/services/ChangeAddress/testData.ts b/packages/wallet/test/services/ChangeAddress/testData.ts
@@ -212,6 +212,108 @@ export const knownAddresses$ = new BehaviorSubject<GroupedAddress[]>([
   }
 ]);
 
+export const unorderedKnownAddresses$ = new BehaviorSubject<GroupedAddress[]>([
+  {
+    accountIndex: 0,
+    address: address_5_0,
+    index: 5,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_1_0,
+    index: 1,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_3,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_3,
+    stakeKeyDerivationPath: { index: 3, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_1,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_1,
+    stakeKeyDerivationPath: { index: 1, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_2,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_2,
+    stakeKeyDerivationPath: { index: 2, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_2_0,
+    index: 2,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_4,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_4,
+    stakeKeyDerivationPath: { index: 4, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_3_0,
+    index: 3,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_5,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_5,
+    stakeKeyDerivationPath: { index: 5, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_0_0,
+    index: 0,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  },
+  {
+    accountIndex: 0,
+    address: address_4_0,
+    index: 4,
+    networkId: Cardano.NetworkId.Testnet,
+    rewardAccount: rewardAccount_0,
+    stakeKeyDerivationPath: { index: 0, role: KeyRole.Stake },
+    type: AddressType.External
+  }
+]);
+
 export const emptyKnownAddresses$ = new BehaviorSubject<GroupedAddress[]>([]);
 
 export const createMockDelegateTracker = (delegatedStake: Map<Cardano.PoolId, DelegatedStake>) => ({