Merge pull request #932 from ava-labs/master

chore: release
ava-labs · Nov 29, 2024 · c1641d9 · c1641d9
2 parents 31f4c62 + dd5cd7e
commit c1641d9
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 109 deletions.
diff --git a/src/utils/addressMap.test.ts b/src/utils/addressMap.test.ts
@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { beforeEach, describe, expect, it, test, vi } from 'vitest';
 import { address } from '../fixtures/common';
 import { Address } from '../serializable/fxs/common';
 import { AddressMap, AddressMaps, matchOwners } from './addressMap';
@@ -245,67 +245,76 @@ describe('AddressMaps', () => {
 });
 
 describe('matchOwners', () => {
-  it('matches owners', () => {
-    const owner1 = address();
-    const owner2 = Address.fromHex('7db97c7cece249c2b98bdc0226cc4c2a57bf52fc');
-    const ownerAddresses: Uint8Array[] = [owner1.toBytes(), owner2.toBytes()];
-    const goodOwner = OutputOwners.fromNative(ownerAddresses, 0n, 1);
-    const threasholdTooHigh = OutputOwners.fromNative(ownerAddresses, 0n, 5);
-    const wrongOwner = OutputOwners.fromNative(
-      [hexToBuffer('0x12345123451234512345')],
-      0n,
-      5,
-    );
-    const locked = OutputOwners.fromNative(
-      ownerAddresses,
-      9999999999999999999999999999999999n,
-      5,
-    );
+  const owner1 = address();
+  const owner2 = Address.fromHex('7db97c7cece249c2b98bdc0226cc4c2a57bf52fc');
+  const ownerAddresses: Uint8Array[] = [owner1.toBytes(), owner2.toBytes()];
+  // NOTE: the ownerAddresses will be sorted in the OutputOwners -- owner2 is at index 0.
+  const goodOwner = OutputOwners.fromNative(ownerAddresses, 0n, 1);
+  const goodOwnerMultisig = OutputOwners.fromNative(ownerAddresses, 0n, 2);
+  const threasholdTooHigh = OutputOwners.fromNative(ownerAddresses, 0n, 5);
+  const wrongOwner = OutputOwners.fromNative(
+    [hexToBuffer('0x12345123451234512345')],
+    0n,
+    5,
+  );
+  const locked = OutputOwners.fromNative(
+    ownerAddresses,
+    9999999999999999999999999999999999n,
+    5,
+  );
 
-    const specs = [
-      {
-        testCase: goodOwner,
-        expectedSigIndices: [0],
-        expectedAddressMap: new AddressMap([[owner1, 0]]),
-      },
-      {
-        testCase: threasholdTooHigh,
-        expectedSigIndices: undefined,
-        expectedAddressMap: undefined,
-      },
-      {
-        testCase: locked,
-        expectedSigIndices: undefined,
-        expectedAddressMap: undefined,
-      },
-      {
-        testCase: wrongOwner,
-        expectedSigIndices: undefined,
-        expectedAddressMap: undefined,
-      },
-      {
-        testCase: goodOwner,
-        sigindices: [1],
-        expectedSigIndices: [1],
-        expectedAddressMap: new AddressMap([[owner2, 1]]),
-      },
-      {
-        testCase: goodOwner,
-        sigindices: [2],
-        expectedSigIndices: undefined,
-        expectedAddressMap: undefined,
-      },
-    ];
+  const specs = [
+    {
+      testCase: goodOwner,
+      expectedSigIndices: [0],
+      expectedAddressMap: new AddressMap([[owner2, 0]]),
+    },
+    {
+      testCase: threasholdTooHigh,
+      expectedSigIndices: undefined,
+      expectedAddressMap: undefined,
+    },
+    {
+      testCase: locked,
+      expectedSigIndices: undefined,
+      expectedAddressMap: undefined,
+    },
+    {
+      testCase: wrongOwner,
+      expectedSigIndices: undefined,
+      expectedAddressMap: undefined,
+    },
+    {
+      testCase: goodOwner,
+      sigindices: [1],
+      expectedSigIndices: [1],
+      expectedAddressMap: new AddressMap([[owner1, 1]]),
+    },
+    {
+      testCase: goodOwnerMultisig,
+      sigindices: [0, 1],
+      expectedSigIndices: [0, 1],
+      expectedAddressMap: new AddressMap([
+        [owner2, 0],
+        [owner1, 1],
+      ]),
+    },
+    {
+      testCase: goodOwner,
+      sigindices: [2],
+      expectedSigIndices: undefined,
+      expectedAddressMap: undefined,
+    },
+  ];
 
-    specs.forEach((spec) => {
-      const result = matchOwners(
-        spec.testCase,
-        addressesFromBytes(ownerAddresses),
-        50n,
-        spec.sigindices,
-      );
-      expect(result?.sigIndicies).toEqual(spec.expectedSigIndices);
-      expect(result?.addressMap).toEqual(spec.expectedAddressMap);
-    });
+  test.each(specs)('matchOwners($testCase, $sigIndices)', (spec) => {
+    const result = matchOwners(
+      spec.testCase,
+      addressesFromBytes(ownerAddresses),
+      50n,
+      spec.sigindices,
+    );
+    expect(result?.sigIndicies).toEqual(spec.expectedSigIndices);
+    expect(result?.addressMap).toEqual(spec.expectedAddressMap);
   });
 });
diff --git a/src/utils/addressesFromBytes.ts b/src/utils/addressesFromBytes.ts
@@ -1,5 +1,7 @@
 import { Address } from '../serializable/fxs/common';
+import { bytesCompare } from './bytesCompare';
 
 export function addressesFromBytes(bytes: readonly Uint8Array[]): Address[] {
-  return bytes.map((b) => new Address(b));
+  const sortedBytes = bytes.toSorted(bytesCompare);
+  return sortedBytes.map((b) => new Address(b));
 }
diff --git a/src/vms/pvm/etna-builder/builder.test.ts b/src/vms/pvm/etna-builder/builder.test.ts
@@ -308,6 +308,13 @@ describe('./src/vms/pvm/etna-builder/builder.test.ts', () => {
       );
 
       expectTxs(unsignedTx.getTx(), expectedTx);
+
+      // Ensure that the unsigned tx utxos are the filtered utxos,
+      // and not the inputUtxos registered in the spend helper.
+      // This is only relevant for the ImportTx.
+      expect(unsignedTx.utxos).toHaveLength(1);
+      expect(unsignedTx.utxos).not.toContain(utxos[0]);
+      expect(unsignedTx.utxos).not.toContain(utxos[1]);
     });
 
     test('newExportTx', () => {

diff --git a/src/vms/pvm/etna-builder/builder.ts b/src/vms/pvm/etna-builder/builder.ts
@@ -302,50 +302,52 @@ export const newImportTx: TxBuilderFn<NewImportTxProps> = (
 ) => {
   const fromAddresses = addressesFromBytes(fromAddressesBytes);
 
-  const { importedInputs, importedAmounts } = utxos
-    .filter(
-      (utxo): utxo is Utxo<TransferOutput> =>
-        isTransferOut(utxo.output) &&
-        // Currently - only AVAX is allowed to be imported to the P-Chain
-        utxo.assetId.toString() === context.avaxAssetID,
-    )
-    .reduce<{
-      importedInputs: TransferableInput[];
-      importedAmounts: Record<string, bigint>;
-    }>(
-      (acc, utxo) => {
-        const { sigIndicies: inputSigIndices } =
-          matchOwners(utxo.getOutputOwners(), fromAddresses, minIssuanceTime) ||
-          {};
-
-        if (inputSigIndices === undefined) {
-          // We couldn't spend this UTXO, so we skip to the next one.
-          return acc;
-        }
-
-        const assetId = utxo.getAssetId();
-
-        return {
-          importedInputs: [
-            ...acc.importedInputs,
-            new TransferableInput(
-              utxo.utxoId,
-              utxo.assetId,
-              new TransferInput(
-                utxo.output.amt,
-                new Input(inputSigIndices.map((value) => new Int(value))),
-              ),
+  const filteredUtxos = utxos.filter(
+    (utxo): utxo is Utxo<TransferOutput> =>
+      isTransferOut(utxo.output) &&
+      // Currently - only AVAX is allowed to be imported to the P-Chain
+      utxo.assetId.toString() === context.avaxAssetID,
+  );
+
+  const { importedInputs, importedAmounts, inputUtxos } = filteredUtxos.reduce<{
+    importedInputs: TransferableInput[];
+    importedAmounts: Record<string, bigint>;
+    inputUtxos: Utxo[];
+  }>(
+    (acc, utxo) => {
+      const { sigIndicies: inputSigIndices } =
+        matchOwners(utxo.getOutputOwners(), fromAddresses, minIssuanceTime) ||
+        {};
+
+      if (inputSigIndices === undefined) {
+        // We couldn't spend this UTXO, so we skip to the next one.
+        return acc;
+      }
+
+      const assetId = utxo.getAssetId();
+
+      return {
+        importedInputs: [
+          ...acc.importedInputs,
+          new TransferableInput(
+            utxo.utxoId,
+            utxo.assetId,
+            new TransferInput(
+              utxo.output.amt,
+              new Input(inputSigIndices.map((value) => new Int(value))),
             ),
-          ],
-          importedAmounts: {
-            ...acc.importedAmounts,
-            [assetId]:
-              (acc.importedAmounts[assetId] ?? 0n) + utxo.output.amount(),
-          },
-        };
-      },
-      { importedInputs: [], importedAmounts: {} },
-    );
+          ),
+        ],
+        importedAmounts: {
+          ...acc.importedAmounts,
+          [assetId]:
+            (acc.importedAmounts[assetId] ?? 0n) + utxo.output.amount(),
+        },
+        inputUtxos: [...acc.inputUtxos, utxo],
+      };
+    },
+    { importedInputs: [], importedAmounts: {}, inputUtxos: [] },
+  );
 
   if (importedInputs.length === 0) {
     throw new Error('no UTXOs available to import');
@@ -355,7 +357,7 @@ export const newImportTx: TxBuilderFn<NewImportTxProps> = (
 
   const addressMaps = AddressMaps.fromTransferableInputs(
     importedInputs,
-    utxos,
+    filteredUtxos,
     minIssuanceTime,
     fromAddressesBytes,
   );
@@ -391,13 +393,16 @@ export const newImportTx: TxBuilderFn<NewImportTxProps> = (
       fromAddresses,
       initialComplexity: complexity,
       minIssuanceTime,
-      utxos,
+      utxos: filteredUtxos,
     },
     [useUnlockedUTXOs],
     context,
   );
 
-  const { changeOutputs, inputs, inputUTXOs } = spendResults;
+  // Note: We don't use the `inputUTXOs` from `spendResults`
+  // for the `UnsignedTx` because we want to use the original
+  // UTXOs that were imported.
+  const { changeOutputs, inputs } = spendResults;
 
   return new UnsignedTx(
     new ImportTx(
@@ -411,7 +416,7 @@ export const newImportTx: TxBuilderFn<NewImportTxProps> = (
       Id.fromString(sourceChainId),
       importedInputs.sort(TransferableInput.compare),
     ),
-    inputUTXOs,
+    inputUtxos,
     addressMaps,
   );
 };