refactor(abstract-utxo): use better keychain types

Issue: BTC-1450

Author: OttoAllmendinger

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -34,7 +34,6 @@ import {
   isTriple,
   ITransactionExplanation as BaseTransactionExplanation,
   IWallet,
-  Keychain,
   KeychainsTriplet,
   KeyIndices,
   P2shP2wshUnsupportedError,
@@ -75,7 +74,7 @@ import { assertDescriptorWalletAddress, getDescriptorMapFromWallet, isDescriptor
 
 import { getChainFromNetwork, getFamilyFromNetwork, getFullNameFromNetwork } from './names';
 import { CustomChangeOptions } from './transaction/fixedScript';
-import { NamedKeychains, toBip32Triple } from './keychains';
+import { toBip32Triple, UtxoKeychain, UtxoNamedKeychains } from './keychains';
 
 const debug = debugLib('bitgo:v2:utxo');
 
@@ -225,7 +224,7 @@ export interface ParseTransactionOptions<TNumber extends number | bigint = numbe
 }
 
 export type ParsedTransaction<TNumber extends number | bigint = number> = {
-  keychains: NamedKeychains;
+  keychains: UtxoNamedKeychains;
   keySignatures: {
     backupPub?: string;
     bitgoPub?: string;
@@ -338,7 +337,7 @@ export interface VerifyKeySignaturesOptions {
 }
 
 export interface VerifyUserPublicKeyOptions {
-  userKeychain?: Keychain;
+  userKeychain?: UtxoKeychain;
   disableNetworking: boolean;
   txParams: TransactionParams;
 }

modules/abstract-utxo/src/index.ts

@@ -5,6 +5,3 @@ export * from './replayProtection';
 export * from './sign';
 
 export * as descriptor from './descriptor';
-export { fetchKeychains } from './keychains';
-export { toKeychainTriple } from './keychains';
-export { NamedKeychains } from './keychains';

modules/abstract-utxo/src/keychains.ts

@@ -1,19 +1,46 @@
+import * as t from 'io-ts';
 import * as utxolib from '@bitgo/utxo-lib';
-import { IRequestTracer, IWallet, Keychain, KeyIndices, promiseProps, Triple } from '@bitgo/sdk-core';
+import { IRequestTracer, IWallet, KeyIndices, promiseProps, Triple } from '@bitgo/sdk-core';
 
 import { AbstractUtxoCoin } from './abstractUtxoCoin';
+import assert from 'assert';
 
-export type NamedKeychains = {
-  user?: Keychain;
-  backup?: Keychain;
-  bitgo?: Keychain;
-};
+/*
 
-export function toKeychainTriple(keychains: NamedKeychains): Triple<Keychain> {
+The standard Keychain type from sdk-core requires a bunch of uninteresting parameters like `id` and `type` and leaves
+important fields like `pub` and `prv` optional.
+
+This is a more focused type that only includes the fields we care about.
+ */
+
+/**
+ * A keychain for a UTXO wallet.
+ */
+export const UtxoKeychain = t.intersection(
+  [
+    t.type({
+      pub: t.string,
+    }),
+    t.partial({
+      prv: t.string,
+      encryptedPrv: t.string,
+    }),
+  ],
+  'UtxoKeychain'
+);
+
+export type UtxoKeychain = t.TypeOf<typeof UtxoKeychain>;
+
+export const UtxoNamedKeychains = t.type({
+  user: UtxoKeychain,
+  backup: UtxoKeychain,
+  bitgo: UtxoKeychain,
+});
+
+export type UtxoNamedKeychains = t.TypeOf<typeof UtxoNamedKeychains>;
+
+export function toKeychainTriple(keychains: UtxoNamedKeychains): Triple<UtxoKeychain> {
   const { user, backup, bitgo } = keychains;
-  if (!user || !backup || !bitgo) {
-    throw new Error('keychains must include user, backup, and bitgo');
-  }
   return [user, backup, bitgo];
 }
 
@@ -28,10 +55,12 @@ export async function fetchKeychains(
   coin: AbstractUtxoCoin,
   wallet: IWallet,
   reqId?: IRequestTracer
-): Promise<NamedKeychains> {
-  return promiseProps({
+): Promise<UtxoNamedKeychains> {
+  const result = await promiseProps({
     user: coin.keychains().get({ id: wallet.keyIds()[KeyIndices.USER], reqId }),
     backup: coin.keychains().get({ id: wallet.keyIds()[KeyIndices.BACKUP], reqId }),
     bitgo: coin.keychains().get({ id: wallet.keyIds()[KeyIndices.BITGO], reqId }),
   });
+  assert(UtxoNamedKeychains.is(result));
+  return result;
 }

modules/abstract-utxo/src/transaction/fixedScript/parseOutput.ts

@@ -5,11 +5,11 @@ import {
   IRequestTracer,
   InvalidAddressDerivationPropertyError,
   IWallet,
-  Keychain,
   TransactionPrebuild,
   UnexpectedAddressError,
   VerificationOptions,
   ITransactionRecipient,
+  Triple,
 } from '@bitgo/sdk-core';
 import { AbstractUtxoCoin, Output, isWalletOutput } from '../../abstractUtxoCoin';
 
@@ -182,16 +182,16 @@ async function fetchAddressDetails({
 }
 
 export interface CustomChangeOptions {
-  keys: [Keychain, Keychain, Keychain];
-  signatures: [string, string, string];
+  keys: Triple<{ pub: string }>;
+  signatures: Triple<string>;
 }
 
 export interface ParseOutputOptions {
   currentOutput: Output;
   coin: AbstractUtxoCoin;
   txPrebuild: TransactionPrebuild;
   verification: VerificationOptions;
-  keychainArray: [Keychain, Keychain, Keychain];
+  keychainArray: Triple<{ pub: string }>;
   wallet: IWallet;
   txParams: {
     recipients: ITransactionRecipient[];
@@ -248,7 +248,7 @@ export async function parseOutput({
       if (isWalletOutput(currentOutput)) {
         const res = await coin.isWalletAddress({
           addressType: AbstractUtxoCoin.inferAddressType({ chain: currentOutput.chain }) || undefined,
-          keychains: keychainArray as { pub: string; commonKeychain?: string | undefined }[],
+          keychains: keychainArray,
           address: currentAddress,
           chain: currentOutput.chain,
           index: currentOutput.index,

modules/abstract-utxo/src/transaction/fixedScript/parseTransaction.ts

@@ -1,6 +1,6 @@
 import assert from 'assert';
 import _ from 'lodash';
-import { Keychain, Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core';
+import { Triple, VerificationOptions, Wallet } from '@bitgo/sdk-core';
 import { CustomChangeOptions, parseOutput } from './parseOutput';
 import * as utxolib from '@bitgo/utxo-lib';
 import {
@@ -11,7 +11,7 @@ import {
   ParsedTransaction,
   ParseTransactionOptions,
 } from '../../abstractUtxoCoin';
-import { fetchKeychains, toKeychainTriple } from '../../keychains';
+import { fetchKeychains, toKeychainTriple, UtxoKeychain, UtxoNamedKeychains } from '../../keychains';
 
 /**
  * @param first
@@ -44,19 +44,19 @@ export async function parseTransaction<TNumber extends bigint | number>(
   const disableNetworking = verification.disableNetworking;
 
   // obtain the keychains and key signatures
-  let keychains: VerificationOptions['keychains'] | undefined = verification.keychains;
+  let keychains: UtxoNamedKeychains | VerificationOptions['keychains'] | undefined = verification.keychains;
   if (!keychains) {
     if (disableNetworking) {
       throw new Error('cannot fetch keychains without networking');
     }
     keychains = await fetchKeychains(coin, wallet, reqId);
   }
 
-  if (!keychains || !keychains.user || !keychains.backup || !keychains.bitgo) {
-    throw new Error('keychains are required, but could not be fetched');
+  if (!UtxoNamedKeychains.is(keychains)) {
+    throw new Error('invalid keychains');
   }
 
-  const keychainArray: Triple<Keychain> = [keychains.user, keychains.backup, keychains.bitgo];
+  const keychainArray: Triple<UtxoKeychain> = toKeychainTriple(keychains);
 
   if (_.isUndefined(txPrebuild.txHex)) {
     throw new Error('missing required txPrebuild property txHex');
@@ -130,7 +130,7 @@ export async function parseTransaction<TNumber extends bigint | number>(
     }
 
     if (customChangeKeys.user && customChangeKeys.backup && customChangeKeys.bitgo && customChangeWallet) {
-      const customChangeKeychains: [Keychain, Keychain, Keychain] = [
+      const customChangeKeychains: Triple<UtxoKeychain> = [
         customChangeKeys.user,
         customChangeKeys.backup,
         customChangeKeys.bitgo,

modules/abstract-utxo/src/verifyKey.ts

@@ -8,8 +8,10 @@ import assert from 'assert';
 import * as utxolib from '@bitgo/utxo-lib';
 import { bip32 } from '@bitgo/utxo-lib';
 import * as bitcoinMessage from 'bitcoinjs-message';
-import { BitGoBase, decryptKeychainPrivateKey, Keychain, KeyIndices } from '@bitgo/sdk-core';
+import { BitGoBase, decryptKeychainPrivateKey, KeyIndices } from '@bitgo/sdk-core';
 import { ParsedTransaction, VerifyKeySignaturesOptions, VerifyUserPublicKeyOptions } from './abstractUtxoCoin';
+import { UtxoKeychain } from './keychains';
+
 const debug = buildDebug('bitgo:abstract-utxo:verifyKey');
 
 /**
@@ -69,7 +71,7 @@ export function verifyKeySignature(params: VerifyKeySignaturesOptions): boolean
  */
 export function verifyCustomChangeKeySignatures<TNumber extends number | bigint>(
   tx: ParsedTransaction<TNumber>,
-  userKeychain: Keychain
+  userKeychain: UtxoKeychain
 ): boolean {
   if (!tx.customChange) {
     throw new Error('parsed transaction is missing required custom change verification data');
@@ -88,13 +90,7 @@ export function verifyCustomChangeKeySignatures<TNumber extends number | bigint>
     if (!keySignature) {
       throw new Error(`missing required custom change ${KeyIndices[keyIndex].toLowerCase()} keychain signature`);
     }
-    if (
-      !verifyKeySignature({
-        userKeychain: userKeychain as { pub: string },
-        keychainToVerify: keychainToVerify as { pub: string },
-        keySignature,
-      })
-    ) {
+    if (!verifyKeySignature({ userKeychain, keychainToVerify, keySignature })) {
       debug('failed to verify custom change %s key signature!', KeyIndices[keyIndex].toLowerCase());
       return false;
     }

modules/bitgo/test/v2/unit/coins/abstractUtxoCoin.ts

@@ -418,7 +418,7 @@ describe('Abstract UTXO Coin:', () => {
 
     it('should not allow more than 150 basis points of implicit external outputs (for paygo outputs)', async () => {
       const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
-        keychains: {},
+        keychains: {} as any,
         keySignatures: {},
         outputs: [],
         missingOutputs: [],
@@ -445,7 +445,7 @@ describe('Abstract UTXO Coin:', () => {
 
     it('should allow 150 basis points of implicit external outputs (for paygo outputs)', async () => {
       const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
-        keychains: {},
+        keychains: {} as any,
         keySignatures: {},
         outputs: [],
         missingOutputs: [],
@@ -479,7 +479,7 @@ describe('Abstract UTXO Coin:', () => {
 
     it('should not allow any implicit external outputs if paygo outputs are disallowed', async () => {
       const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
-        keychains: {},
+        keychains: {} as any,
         keySignatures: {},
         outputs: [],
         missingOutputs: [],
@@ -511,7 +511,7 @@ describe('Abstract UTXO Coin:', () => {
 
     it('should allow paygo outputs if empty verification object is passed', async () => {
       const coinMock = sinon.stub(coin, 'parseTransaction').resolves({
-        keychains: {},
+        keychains: {} as any,
         keySignatures: {},
         outputs: [],
         missingOutputs: [],
@@ -549,7 +549,7 @@ describe('Abstract UTXO Coin:', () => {
       const bigintCoin = bitgo.coin('tdoge') as AbstractUtxoCoin;
 
       const coinMock = sinon.stub(bigintCoin, 'parseTransaction').resolves({
-        keychains: {},
+        keychains: {} as any,
         keySignatures: {},
         outputs: [],
         missingOutputs: [],