chore(abstract-utxo): address review comments

TICKET: BTC-1582

Author: davidkaplanbitgo

modules/abstract-utxo/src/abstractUtxoCoin.ts

@@ -76,7 +76,15 @@ import ScriptType2Of3 = utxolib.bitgo.outputScripts.ScriptType2Of3;
 import { isReplayProtectionUnspent } from './replayProtection';
 import { signAndVerifyPsbt, signAndVerifyWalletTransaction } from './sign';
 import { supportedCrossChainRecoveries } from './config';
-import { explainPsbt, explainTx, getPsbtTxInputs, getTxInputs } from './transaction';
+import {
+  assertValidTransactionRecipient,
+  explainPsbt,
+  explainTx,
+  fromExtendedAddressFormat,
+  getPsbtTxInputs,
+  getTxInputs,
+  isExtendedAddressFormat,
+} from './transaction';
 import { assertDescriptorWalletAddress } from './descriptor/assertDescriptorWalletAddress';
 
 type UtxoCustomSigningFunction<TNumber extends number | bigint> = {
@@ -104,7 +112,6 @@ type UtxoCustomSigningFunction<TNumber extends number | bigint> = {
   }): Promise<SignedTransaction>;
 };
 
-export const ScriptRecipientPrefix = 'scriptPubkey:';
 const { getExternalChainCode, isChainCode, scriptTypeForChain, outputScripts } = bitgo;
 type Unspent<TNumber extends number | bigint = number> = bitgo.Unspent<TNumber>;
 
@@ -453,11 +460,8 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
       params.recipients =
         params.recipients instanceof Array
           ? params?.recipients?.map((recipient) => {
-              if (recipient.address.startsWith(ScriptRecipientPrefix)) {
-                const { address, ...rest } = recipient;
-                return { ...rest, script: address.replace(ScriptRecipientPrefix, '') };
-              }
-              return recipient;
+              const { address, ...rest } = recipient;
+              return { ...rest, ...fromExtendedAddressFormat(address) };
             })
           : params.recipients;
     }
@@ -478,12 +482,8 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
   }
 
   checkRecipient(recipient: { address: string; amount: number | string }): void {
-    if (recipient.address.startsWith(ScriptRecipientPrefix)) {
-      const amount = BigInt(recipient.amount);
-      if (amount !== BigInt(0)) {
-        throw new Error('Only zero amounts allowed for non-encodeable scriptPubkeys');
-      }
-    } else {
+    assertValidTransactionRecipient(recipient);
+    if (!isExtendedAddressFormat(recipient.address)) {
       super.checkRecipient(recipient);
     }
   }
@@ -540,6 +540,18 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
     return utxolib.bitgo.createTransactionFromHex<TNumber>(hex, this.network, this.amountType);
   }
 
+  toCanonicalTransactionRecipient(output: { valueString: string; address?: string }): {
+    amount: bigint;
+    address?: string;
+  } {
+    const amount = BigInt(output.valueString);
+    assertValidTransactionRecipient({ amount, address: output.address });
+    if (!output.address) {
+      return { amount };
+    }
+    return { amount, address: this.canonicalAddress(output.address) };
+  }
+
   /**
    * Extract and fill transaction details such as internal/change spend, external spend (explicit vs. implicit), etc.
    * @param params
@@ -602,15 +614,7 @@ export abstract class AbstractUtxoCoin extends BaseCoin {
           if (output.wallet === wallet.id()) {
             return [];
           }
-          // In the case that this is an OP_RETURN output or another non-encodable scriptPubkey, we dont have an address.
-          // We will verify that the amount is zero, and if it isnt then we will throw an error.
-          if (!output.address) {
-            if (output.valueString !== '0') {
-              throw new Error(`Only zero amounts allowed for non-encodeable scriptPubkeys: ${JSON.stringify(output)}`);
-            }
-            return [{ amount: BigInt(0) }];
-          }
-          return [{ amount: BigInt(output.valueString), address: this.canonicalAddress(output.address) }];
+          return [this.toCanonicalTransactionRecipient(output)];
         }
       );
     } else {

modules/abstract-utxo/src/transaction.ts

@@ -8,12 +8,42 @@ import {
   TransactionExplanation,
   TransactionPrebuild,
   Output,
-  ScriptRecipientPrefix,
 } from './abstractUtxoCoin';
 import { bip32, BIP32Interface, bitgo } from '@bitgo/utxo-lib';
 
-// https://github.com/bitcoin/bitcoin/blob/5961b23898ee7c0af2626c46d5d70e80136578d3/src/script/script.h#L47
-const OP_RETURN_IDENTIFIER = Buffer.from('6a', 'hex');
+const ScriptRecipientPrefix = 'scriptPubkey:';
+
+/**
+ * An extended address is one that encodes either a regular address or a hex encoded script with the prefix `scriptPubkey:`.
+ * This function converts the extended address format to either a script or an address.
+ * @param extendedAddress
+ */
+export function fromExtendedAddressFormat(extendedAddress: string): { address: string } | { script: string } {
+  if (extendedAddress.startsWith(ScriptRecipientPrefix)) {
+    return { script: extendedAddress.replace(ScriptRecipientPrefix, '') };
+  }
+  return { address: extendedAddress };
+}
+
+export function toExtendedAddressFormat(script: Buffer, network: utxolib.Network): string {
+  return script[0] === utxolib.opcodes.OP_RETURN
+    ? `${ScriptRecipientPrefix}${script.toString('hex')}`
+    : utxolib.address.fromOutputScript(script, network);
+}
+
+export function isExtendedAddressFormat(address: string): boolean {
+  return address.startsWith(ScriptRecipientPrefix);
+}
+
+export function assertValidTransactionRecipient(output: { amount: bigint | number | string; address?: string }): void {
+  // In the case that this is an OP_RETURN output or another non-encodable scriptPubkey, we dont have an address.
+  // We will verify that the amount is zero, and if it isnt then we will throw an error.
+  if (!output.address || output.address.startsWith(ScriptRecipientPrefix)) {
+    if (output.amount.toString() !== '0') {
+      throw new Error(`Only zero amounts allowed for non-encodeable scriptPubkeys: ${JSON.stringify(output)}`);
+    }
+  }
+}
 
 /**
  * Get the inputs for a psbt from a prebuild.
@@ -112,12 +142,7 @@ function explainCommon<TNumber extends number | bigint>(
   tx.outs.forEach((currentOutput) => {
     // Try to encode the script pubkey with an address. If it fails, try to parse it as an OP_RETURN output with the prefix.
     // If that fails, then it is an unrecognized scriptPubkey and should fail
-    let currentAddress: string;
-    if (currentOutput.script.subarray(0, 1).equals(OP_RETURN_IDENTIFIER)) {
-      currentAddress = `${ScriptRecipientPrefix}${currentOutput.script.toString('hex')}`;
-    } else {
-      currentAddress = utxolib.address.fromOutputScript(currentOutput.script, network);
-    }
+    const currentAddress = toExtendedAddressFormat(currentOutput.script, network);
     const currentAmount = BigInt(currentOutput.value);
 
     if (changeAddresses.includes(currentAddress)) {