Merge pull request #5658 from BitGo/BTC-1826.fix-descriptoroutput

refactor(utxo-core): improve descriptor handling

Author: OttoAllmendinger

modules/utxo-core/src/Output.ts

@@ -0,0 +1,72 @@
+export type Output<TValue = bigint> = {
+  script: Buffer;
+  value: TValue;
+};
+export type MaxOutput = Output<'max'>;
+type ValueBigInt = { value: bigint };
+type ValueMax = { value: 'max' };
+
+/**
+ * @return true if the output is a max output
+ */
+export function isMaxOutput<A extends ValueBigInt, B extends ValueMax>(output: A | B): output is B {
+  return output.value === 'max';
+}
+
+/**
+ * @return the max output if there is one
+ * @throws if there are multiple max outputs
+ */
+export function getMaxOutput<A extends ValueBigInt, B extends ValueMax>(outputs: (A | B)[]): B | undefined {
+  const max = outputs.filter(isMaxOutput<A, B>);
+  if (max.length === 0) {
+    return undefined;
+  }
+  if (max.length > 1) {
+    throw new Error('Multiple max outputs');
+  }
+  return max[0];
+}
+
+/**
+ * @return the sum of the outputs
+ */
+export function getOutputSum(outputs: ValueBigInt[]): bigint {
+  return outputs.reduce((sum, output) => sum + output.value, 0n);
+}
+
+/**
+ * @return the sum of the outputs that are not 'max'
+ */
+export function getFixedOutputSum(outputs: (ValueBigInt | ValueMax)[]): bigint {
+  return getOutputSum(outputs.filter((o): o is Output => !isMaxOutput(o)));
+}
+
+/**
+ * @param outputs
+ * @param params
+ * @return the outputs with the 'max' output replaced with the max amount
+ */
+export function toFixedOutputs<A extends ValueBigInt, B extends ValueMax>(
+  outputs: (A | B)[],
+  params: { maxAmount: bigint }
+): A[] {
+  // assert that there is at most one max output
+  const maxOutput = getMaxOutput<A, B>(outputs);
+  return outputs.map((output): A => {
+    if (isMaxOutput(output)) {
+      if (output !== maxOutput) {
+        throw new Error('illegal state');
+      }
+      return { ...output, value: params.maxAmount };
+    } else {
+      return output;
+    }
+  });
+}
+
+export type PrevOutput = {
+  hash: string;
+  index: number;
+  witnessUtxo: Output;
+};

modules/utxo-core/src/descriptor/Output.ts

@@ -1,76 +1,11 @@
-import { Descriptor } from '@bitgo/wasm-miniscript';
-
-import { DescriptorMap } from './DescriptorMap';
-import { createScriptPubKeyFromDescriptor } from './address';
-
-export type Output<TValue = bigint> = {
-  script: Buffer;
-  value: TValue;
-};
-
-export type MaxOutput = Output<'max'>;
-
-type ValueBigInt = { value: bigint };
-type ValueMax = { value: 'max' };
-
-/**
- * @return true if the output is a max output
- */
-export function isMaxOutput<A extends ValueBigInt, B extends ValueMax>(output: A | B): output is B {
-  return output.value === 'max';
-}
-
-/**
- * @return the max output if there is one
- * @throws if there are multiple max outputs
- */
-export function getMaxOutput<A extends ValueBigInt, B extends ValueMax>(outputs: (A | B)[]): B | undefined {
-  const max = outputs.filter(isMaxOutput<A, B>);
-  if (max.length === 0) {
-    return undefined;
-  }
-  if (max.length > 1) {
-    throw new Error('Multiple max outputs');
-  }
-  return max[0];
-}
+import assert from 'assert';
 
-/**
- * @return the sum of the outputs
- */
-export function getOutputSum(outputs: ValueBigInt[]): bigint {
-  return outputs.reduce((sum, output) => sum + output.value, 0n);
-}
+import { Descriptor } from '@bitgo/wasm-miniscript';
 
-/**
- * @return the sum of the outputs that are not 'max'
- */
-export function getFixedOutputSum(outputs: (ValueBigInt | ValueMax)[]): bigint {
-  return getOutputSum(outputs.filter((o): o is Output => !isMaxOutput(o)));
-}
+import { getFixedOutputSum, MaxOutput, Output, PrevOutput } from '../Output';
 
-/**
- * @param outputs
- * @param params
- * @return the outputs with the 'max' output replaced with the max amount
- */
-export function toFixedOutputs<A extends ValueBigInt, B extends ValueMax>(
-  outputs: (A | B)[],
-  params: { maxAmount: bigint }
-): A[] {
-  // assert that there is at most one max output
-  const maxOutput = getMaxOutput<A, B>(outputs);
-  return outputs.map((output): A => {
-    if (isMaxOutput(output)) {
-      if (output !== maxOutput) {
-        throw new Error('illegal state');
-      }
-      return { ...output, value: params.maxAmount };
-    } else {
-      return output;
-    }
-  });
-}
+import { DescriptorMap } from './DescriptorMap';
+import { getDescriptorAtIndexCheckScript } from './derive';
 
 export type WithDescriptor<T> = T & {
   descriptor: Descriptor;
@@ -96,15 +31,9 @@ export function getExternalFixedAmount(outputs: WithOptDescriptor<Output | MaxOu
   return getFixedOutputSum(outputs.filter(isExternalOutput));
 }
 
-export type PrevOutput = {
-  hash: string;
-  index: number;
-  witnessUtxo: Output;
-};
-
 export type DescriptorWalletOutput = PrevOutput & {
   descriptorName: string;
-  descriptorIndex: number;
+  descriptorIndex: number | undefined;
 };
 
 export type DerivedDescriptorWalletOutput = WithDescriptor<PrevOutput>;
@@ -117,17 +46,17 @@ export function toDerivedDescriptorWalletOutput(
   if (!descriptor) {
     throw new Error(`Descriptor not found: ${output.descriptorName}`);
   }
-  const derivedDescriptor = descriptor.atDerivationIndex(output.descriptorIndex);
-  const script = createScriptPubKeyFromDescriptor(derivedDescriptor);
-  if (!script.equals(output.witnessUtxo.script)) {
-    throw new Error(
-      `Script mismatch: descriptor ${output.descriptorName} ${descriptor.toString()} script=${script.toString('hex')}`
-    );
-  }
+  assert(descriptor instanceof Descriptor);
+  const descriptorAtIndex = getDescriptorAtIndexCheckScript(
+    descriptor,
+    output.descriptorIndex,
+    output.witnessUtxo.script,
+    output.descriptorName
+  );
   return {
     hash: output.hash,
     index: output.index,
     witnessUtxo: output.witnessUtxo,
-    descriptor: descriptor.atDerivationIndex(output.descriptorIndex),
+    descriptor: descriptorAtIndex,
   };
 }

modules/utxo-core/src/descriptor/address.ts

@@ -1,11 +1,11 @@
 import { Descriptor } from '@bitgo/wasm-miniscript';
 import * as utxolib from '@bitgo/utxo-lib';
 
-export function createScriptPubKeyFromDescriptor(descriptor: Descriptor, index?: number): Buffer {
+export function createScriptPubKeyFromDescriptor(descriptor: Descriptor, index: number | undefined): Buffer {
   if (index === undefined) {
     return Buffer.from(descriptor.scriptPubkey());
   }
-  return createScriptPubKeyFromDescriptor(descriptor.atDerivationIndex(index));
+  return createScriptPubKeyFromDescriptor(descriptor.atDerivationIndex(index), undefined);
 }
 
 export function createAddressFromDescriptor(

modules/utxo-core/src/descriptor/derive.ts

@@ -0,0 +1,44 @@
+import assert from 'assert';
+
+import { Descriptor } from '@bitgo/wasm-miniscript';
+
+/**
+ * Get a descriptor at a specific derivation index.
+ * For wildcard descriptors (containing '*'), the index is required and used for derivation.
+ * For definite descriptors (not containing '*'), no index should be provided.
+ * @param descriptor - The descriptor to derive from
+ * @param index - The derivation index for wildcard descriptors
+ * @returns A new descriptor at the specified index for wildcard descriptors, or the original descriptor for definite ones
+ * @throws {Error} If index is undefined for a wildcard descriptor or if index is provided for a definite descriptor
+ */
+export function getDescriptorAtIndex(descriptor: Descriptor, index: number | undefined): Descriptor {
+  assert(descriptor instanceof Descriptor);
+  Descriptor.fromString(descriptor.toString(), 'derivable');
+  descriptor = Descriptor.fromStringDetectType(descriptor.toString());
+  if (descriptor.hasWildcard()) {
+    if (index === undefined) {
+      throw new Error('Derivable descriptor requires an index');
+    }
+    return descriptor.atDerivationIndex(index);
+  } else {
+    if (index !== undefined) {
+      throw new Error('Definite descriptor cannot be derived with index');
+    }
+    return descriptor;
+  }
+}
+
+export function getDescriptorAtIndexCheckScript(
+  descriptor: Descriptor,
+  index: number | undefined,
+  script: Buffer,
+  descriptorString = descriptor.toString()
+): Descriptor {
+  assert(descriptor instanceof Descriptor);
+  const descriptorAtIndex = getDescriptorAtIndex(descriptor, index);
+  if (!script.equals(descriptorAtIndex.scriptPubkey())) {
+    throw new Error(`Script mismatch: descriptor ${descriptorString} script=${script.toString('hex')}`);
+  }
+  assert(descriptorAtIndex instanceof Descriptor);
+  return descriptorAtIndex;
+}

modules/utxo-core/src/descriptor/index.ts

@@ -3,3 +3,6 @@ export * from './address';
 export * from './DescriptorMap';
 export * from './Output';
 export * from './VirtualSize';
+
+/** @deprecated - import from @bitgo/utxo-core directly instead */
+export * from '../Output';

modules/utxo-core/src/descriptor/psbt/createPsbt.ts

@@ -1,7 +1,8 @@
 import * as utxolib from '@bitgo/utxo-lib';
 import { Descriptor } from '@bitgo/wasm-miniscript';
 
-import { DerivedDescriptorWalletOutput, Output, WithOptDescriptor } from '../Output';
+import { DerivedDescriptorWalletOutput, WithOptDescriptor } from '../Output';
+import { Output } from '../../Output';
 
 import { toUtxoPsbt, toWrappedPsbt } from './wrap';
 import { assertSatisfiable } from './assertSatisfiable';

modules/utxo-core/src/index.ts

@@ -2,3 +2,4 @@ export * as bip65 from './bip65';
 export * as descriptor from './descriptor';
 export * as testutil from './testutil';
 export * from './dustThreshold';
+export * from './Output';

modules/utxo-core/src/testutil/descriptor/mock.utils.ts

@@ -33,7 +33,7 @@ export function mockDerivedDescriptorWalletOutput(
     hash,
     index: vout,
     witnessUtxo: {
-      script: createScriptPubKeyFromDescriptor(descriptor),
+      script: createScriptPubKeyFromDescriptor(descriptor, undefined),
       value,
     },
     descriptor,
@@ -67,7 +67,7 @@ export function mockPsbt(
     outputs.map((o) => {
       const derivedDescriptor = tryDeriveAtIndex(o.descriptor, o.index);
       return {
-        script: createScriptPubKeyFromDescriptor(derivedDescriptor),
+        script: createScriptPubKeyFromDescriptor(derivedDescriptor, undefined),
         value: o.value,
         descriptor: o.external ? undefined : derivedDescriptor,
       };

modules/utxo-core/test/Output.ts

@@ -0,0 +1,41 @@
+import * as assert from 'assert';
+
+import { getFixedOutputSum, getMaxOutput, getOutputSum, isMaxOutput, toFixedOutputs } from '../src';
+
+describe('Output', function () {
+  const oBigInt = { value: 1n };
+  const oBigInt2 = { value: 2n };
+  const oMax = { value: 'max' } as const;
+
+  it('getMaxOutput returns expected values', function () {
+    assert.strictEqual(getMaxOutput([oBigInt]), undefined);
+    assert.strictEqual(getMaxOutput([oBigInt, oBigInt]), undefined);
+    assert.strictEqual(getMaxOutput([oBigInt, oMax]), oMax);
+    assert.throws(() => getMaxOutput([oMax, oMax]), /Multiple max outputs/);
+  });
+
+  it('isMaxOutput correctly identifies max outputs', function () {
+    assert.strictEqual(isMaxOutput(oBigInt), false);
+    assert.strictEqual(isMaxOutput(oMax), true);
+  });
+
+  it('getOutputSum calculates sum correctly', function () {
+    assert.strictEqual(getOutputSum([]), 0n);
+    assert.strictEqual(getOutputSum([oBigInt]), 1n);
+    assert.strictEqual(getOutputSum([oBigInt, oBigInt2]), 3n);
+  });
+
+  it('getFixedOutputSum handles mixed outputs', function () {
+    assert.strictEqual(getFixedOutputSum([]), 0n);
+    assert.strictEqual(getFixedOutputSum([oBigInt]), 1n);
+    assert.strictEqual(getFixedOutputSum([oBigInt, oMax]), 1n);
+    assert.strictEqual(getFixedOutputSum([oBigInt, oBigInt2, oMax]), 3n);
+  });
+
+  it('toFixedOutputs converts max outputs correctly', function () {
+    const maxAmount = 10n;
+    assert.deepStrictEqual(toFixedOutputs([oBigInt], { maxAmount }), [oBigInt]);
+    assert.deepStrictEqual(toFixedOutputs([oMax], { maxAmount }), [{ ...oMax, value: maxAmount }]);
+    assert.throws(() => toFixedOutputs([oMax, oMax], { maxAmount }), /Multiple max outputs/);
+  });
+});