fix: follow-up SDK fixes from the 0.14 audit

packages/cashscript/src/TransactionBuilder.ts

@@ -1,5 +1,6 @@
 import {
   binToHex,
+  decodeAuthenticationInstructions,
   decodeTransaction,
   decodeTransactionUnsafe,
   encodeTransaction,
@@ -19,12 +20,15 @@ import {
   StandardUnlockableUtxo,
   VmResourceUsage,
   isContractUnlocker,
+  isP2PKHUnlocker,
   isPlaceholderUnlocker,
+  SignatureAlgorithm,
   BchChangeOutputOptions,
   TokenChangeOutputOptions,
 } from './interfaces.js';
 import { PLACEHOLDER_P2PKH_UNLOCKING_SIZE } from './constants.js';
 import { NetworkProvider } from './network/index.js';
+import { NetworkProviderError } from './network/errors.js';
 import {
   calculateDust,
   cashScriptOutputToLibauthOutput,
@@ -47,6 +51,7 @@ import {
   TransactionTooLargeError,
 } from './Errors.js';
 import { DebugResults } from './debugging.js';
+import SignatureTemplate from './SignatureTemplate.js';
 import { debugLibauthTemplate, getLibauthTemplate, getBitauthUri } from './libauth-template/LibauthTemplate.js';
 import { getWcContractInfo, WcSourceOutput, WcTransactionOptions } from './walletconnect-utils.js';
 import semver from 'semver';
@@ -188,6 +193,7 @@ export class TransactionBuilder {
    *
    * @param chunks - The data chunks to include after the `OP_RETURN` opcode.
    * @returns This builder for chaining.
+   * @throws If a `0x`-prefixed chunk is not a valid hex string.
    */
   addOpReturnOutput(chunks: string[]): this {
     this.addOutput(createOpReturnOutput(chunks));
@@ -211,7 +217,13 @@ export class TransactionBuilder {
     const totalBchOutputAmount = this.outputs.reduce((total, output) => total + output.amount, 0n);
 
     const tentativeSurplus = totalBchInputAmount - totalBchOutputAmount;
-    const tentativeTransactionSize = this.getTransactionSize();
+
+    // ECDSA signatures vary in length, and are generated again once the change output is added, so the fee is
+    // calculated as if every ECDSA signature has its maximum length
+    const tentativeTransaction = this.buildLibauthTransaction(true);
+    const tentativeTransactionSize = BigInt(
+      this.getEncodedTransactionSize(tentativeTransaction) + this.getEcdsaSignatureSizeMargin(tentativeTransaction),
+    );
     const tentativeFee = BigInt(Math.ceil(changeOutputOptions.feeRate * Number(tentativeTransactionSize)));
     const tentativeChangeAmount = tentativeSurplus - tentativeFee;
 
@@ -255,14 +267,16 @@ export class TransactionBuilder {
    * or BCH change output was already added.
    */
   addTokenChangeOutputIfNeeded(changeOutputOptions: TokenChangeOutputOptions): this {
-    const { category, to } = changeOutputOptions;
+    // Token categories are hex strings, which are compared case-insensitively
+    const category = changeOutputOptions.category.toLowerCase();
+    const { to } = changeOutputOptions;
 
     const inputAmount = this.inputs
-      .filter((input) => input.token?.category === category)
+      .filter((input) => input.token?.category.toLowerCase() === category)
       .reduce((total, input) => total + input.token!.amount, 0n);
 
     const outputAmount = this.outputs
-      .filter((output) => output.token?.category === category)
+      .filter((output) => output.token?.category.toLowerCase() === category)
       .reduce((total, output) => total + output.token!.amount, 0n);
 
     const changeAmount = inputAmount - outputAmount;
@@ -300,6 +314,24 @@ export class TransactionBuilder {
     return encodeTransaction(transaction).byteLength + placeholderInputCount * PLACEHOLDER_P2PKH_UNLOCKING_SIZE;
   }
 
+  // The number of bytes that the transaction's ECDSA signatures are shorter than their maximum length
+  private getEcdsaSignatureSizeMargin(transaction: LibauthTransaction): number {
+    // 72-byte DER signature + sighash byte
+    const MAX_ECDSA_SIGNATURE_SIZE = 73;
+
+    return this.inputs.reduce((margin, input, inputIndex) => {
+      const signatureIndices = getEcdsaSignaturePushIndices(input.unlocker);
+      if (signatureIndices.length === 0) return margin;
+
+      const instructions = decodeAuthenticationInstructions(transaction.inputs[inputIndex].unlockingBytecode);
+      return signatureIndices.reduce((total, index) => {
+        const instruction = instructions[index];
+        const signatureSize = instruction && 'data' in instruction ? instruction.data.length : MAX_ECDSA_SIGNATURE_SIZE;
+        return total + MAX_ECDSA_SIGNATURE_SIZE - signatureSize;
+      }, margin);
+    }, 0);
+  }
+
   /**
    * Calculate the transaction fee in satoshis and the fee per byte.
    *
@@ -489,7 +521,8 @@ export class TransactionBuilder {
    * require statements or VM errors surface with descriptive messages.
    *
    * @returns The decoded transaction details (including txid and raw hex).
-   * @throws A `FailedTransactionError` if the network rejects the transaction, or any of the
+   * @throws A `NetworkProviderError` (or one of its subclasses) if the network provider rejects the transaction,
+   *   a `FailedTransactionError` if broadcasting fails otherwise, or any of the
    *   build / local evaluation errors (e.g. fee cap, implicit burn, failing require statement).
    */
   async send(): Promise<TransactionDetails>;
@@ -500,7 +533,8 @@ export class TransactionBuilder {
    *
    * @param raw - Pass `true` to receive the raw transaction hex instead of decoded details.
    * @returns The raw transaction hex as retrieved from the network after broadcast.
-   * @throws A `FailedTransactionError` if the network rejects the transaction, or any of the
+   * @throws A `NetworkProviderError` (or one of its subclasses) if the network provider rejects the transaction,
+   *   a `FailedTransactionError` if broadcasting fails otherwise, or any of the
    *   build / local evaluation errors (e.g. fee cap, implicit burn, failing require statement).
    */
   async send(raw: true): Promise<string>;
@@ -517,6 +551,9 @@ export class TransactionBuilder {
     try {
       txid = await this.provider.sendRawTransaction(tx);
     } catch (e: any) {
+      // Network provider errors describe why the network rejected the transaction, so they are thrown as they are
+      if (e instanceof NetworkProviderError) throw e;
+
       const reason = e.error ?? e.message;
 
       const getBitauthUriWithFallback = (): string => {
@@ -608,14 +645,17 @@ export class TransactionBuilder {
     const tokenInputAmounts: Record<string, bigint> = {};
     const tokenOutputAmounts: Record<string, bigint> = {};
 
+    // Token categories are hex strings, which are compared case-insensitively
     for (const input of this.inputs) {
       if (input.token?.amount) {
-        tokenInputAmounts[input.token.category] = (tokenInputAmounts[input.token.category] || 0n) + input.token.amount;
+        const category = input.token.category.toLowerCase();
+        tokenInputAmounts[category] = (tokenInputAmounts[category] || 0n) + input.token.amount;
       }
     }
     for (const output of this.outputs) {
       if (output.token?.amount) {
-        tokenOutputAmounts[output.token.category] = (tokenOutputAmounts[output.token.category] || 0n) + output.token.amount;
+        const category = output.token.category.toLowerCase();
+        tokenOutputAmounts[category] = (tokenOutputAmounts[category] || 0n) + output.token.amount;
       }
     }
 
@@ -653,3 +693,20 @@ export class TransactionBuilder {
     }
   }
 }
+
+// The positions of the pushes in the unlocking bytecode that hold an ECDSA signature generated by a SignatureTemplate
+function getEcdsaSignaturePushIndices(unlocker: Unlocker): number[] {
+  const isEcdsaTemplate = (param: unknown): boolean => (
+    param instanceof SignatureTemplate && param.signatureAlgorithm === SignatureAlgorithm.ECDSA
+  );
+
+  // P2PKH unlocking bytecode is <signature> <public key>
+  if (isP2PKHUnlocker(unlocker)) return isEcdsaTemplate(unlocker.template) ? [0] : [];
+
+  // Contract function arguments are pushed in reverse order
+  if (isContractUnlocker(unlocker)) {
+    return unlocker.params.flatMap((param, i, params) => (isEcdsaTemplate(param) ? [params.length - 1 - i] : []));
+  }
+
+  return [];
+}

packages/cashscript/src/debugging.ts

@@ -10,8 +10,13 @@ import { VmTarget } from './interfaces.js';
 export type DebugResult = AuthenticationProgramStateCommon[];
 export type DebugResults = Record<string, DebugResult>;
 
-// debugs the template, optionally logging the execution data
-export const debugTemplate = (template: WalletTemplate, artifacts: Artifact[]): DebugResults => {
+// debugs the template, optionally logging the execution data. The artifacts are keyed by the ID of the unlocking script
+// that spends the contract, so every input is debugged with the artifact of its own contract
+export const debugTemplate = (
+  template: WalletTemplate, artifactsByUnlockingScriptId: Record<string, Artifact>,
+): DebugResults => {
+  const artifacts = Object.values(artifactsByUnlockingScriptId);
+
   // If a contract has the same name, but a different bytecode, then it is considered a name collision
   const hasArtifactNameCollision = artifacts.some(
     (artifact) => (
@@ -29,7 +34,7 @@ export const debugTemplate = (template: WalletTemplate, artifacts: Artifact[]):
   for (const unlockingScriptId of unlockingScriptIds) {
     const scenarioIds = (template.scripts[unlockingScriptId] as WalletTemplateScriptUnlocking).passes ?? [];
 
-    const matchingArtifact = artifacts.find((artifact) => unlockingScriptId.startsWith(artifact.contractName));
+    const matchingArtifact = artifactsByUnlockingScriptId[unlockingScriptId];
 
     for (const scenarioId of scenarioIds) {
       results[`${unlockingScriptId}.${scenarioId}`] = debugSingleScenario(template, matchingArtifact, unlockingScriptId, scenarioId);
@@ -72,6 +77,13 @@ const debugSingleScenario = (
     // - multiple log statements may exist for the same ip, so we need to handle all of them.
     const executedLogs = executedDebugSteps
       .flatMap((debugStep, index) => {
+        // A step with an error did not complete its instruction, so the log entries after it were not reached. libauth
+        // also repeats the final state at the end of the trace, which should only be matched once.
+        const previousDebugStep = executedDebugSteps[index - 1];
+        const isRepeatedFinalState = index === executedDebugSteps.length - 1
+          && debugStep.ip === previousDebugStep?.ip && debugStep.instructions === previousDebugStep?.instructions;
+        if (debugStep.error || isRepeatedFinalState) return [];
+
         const frame = resolveFrame(artifact, debugStep);
         const logEntries = frame.logs.filter((log) => log.ip === debugStep.ip);
         if (logEntries.length === 0) return [];
@@ -151,7 +163,11 @@ const debugSingleScenario = (
 
   // Check if the evaluation failed matches any of the possible failure cases
   if (failedFinalVerify(evaluationResult)) {
-    const finalExecutedVerifyIp = getFinalExecutedVerifyIp(executedDebugSteps);
+    // Only the steps of the frame that failed are used, since ips of other frames (e.g. a function body that was
+    // invoked by the final require statement) do not point into this frame
+    const finalExecutedVerifyIp = getFinalExecutedVerifyIp(
+      executedDebugSteps.filter((step) => step.instructions === lastExecutedDebugStep.instructions),
+    );
 
     // The final executed verify instruction points to the "implicit" VERIFY that is added at the end of the script.
     // This instruction does not exist in the sourcemap, so we need to decrement the instruction pointer to get the

packages/cashscript/src/libauth-template/LibauthTemplate.ts

@@ -66,12 +66,15 @@ export const getLibauthTemplate = (
 };
 
 export const debugLibauthTemplate = (template: WalletTemplate, transaction: TransactionBuilder): DebugResults => {
-  const allArtifacts = transaction.inputs
-    .map(input => isContractUnlocker(input.unlocker) ? input.unlocker.contract : undefined)
-    .filter((contract): contract is Contract => Boolean(contract))
-    .map(contract => contract.artifact);
+  // Artifacts are matched to inputs by the exact unlocking script ID (P2PKH inputs do not have an artifact)
+  const artifactEntries = transaction.inputs.flatMap((input, inputIndex): Array<[string, Artifact]> => {
+    if (!isContractUnlocker(input.unlocker)) return [];
 
-  return debugTemplate(template, allArtifacts);
+    const { contract, abiFunction } = input.unlocker;
+    return [[getUnlockScriptName(contract, abiFunction, inputIndex), contract.artifact]];
+  });
+
+  return debugTemplate(template, Object.fromEntries(artifactEntries));
 };
 
 export const getBitauthUri = (template: WalletTemplate): string => {

packages/cashscript/src/network/ElectrumNetworkProvider.ts

@@ -9,6 +9,7 @@ import { SpendableUtxo, Network } from '../interfaces.js';
 import NetworkProvider from './NetworkProvider.js';
 import { addressToLockScript } from '../utils.js';
 import {
+  NetworkProviderError,
   NetworkProviderMissingInputsError,
   NetworkProviderMempoolConflictError,
   NetworkProviderTransactionAlreadySubmittedError,
@@ -249,7 +250,7 @@ const RELATIVE_TIMELOCK_PATTERNS = [
   'non-BIP68-final',
 ];
 
-function classifyNetworkProviderError(errorMessage: string): Error {
+function classifyNetworkProviderError(errorMessage: string): NetworkProviderError {
   if (MISSING_INPUTS_PATTERNS.some((pattern) => errorMessage.includes(pattern))) {
     return new NetworkProviderMissingInputsError(errorMessage);
   }
@@ -270,7 +271,7 @@ function classifyNetworkProviderError(errorMessage: string): Error {
     return new NetworkProviderRelativeTimelockError(errorMessage);
   }
 
-  return new Error(errorMessage);
+  return new NetworkProviderError(errorMessage, errorMessage);
 }
 
 function lockingBytecodeToElectrumScriptHash(lockingBytecode: Uint8Array): string {

packages/cashscript/src/network/MockNetworkProvider.ts

@@ -4,7 +4,7 @@ import { SpendableUtxo, Utxo, Network, VmTarget } from '../interfaces.js';
 import NetworkProvider from './NetworkProvider.js';
 import { addressToLockScript, cashScriptOutputToLibauthOutput, libauthTokenDetailsToCashScriptTokenDetails } from '../utils.js';
 import { createVirtualMachine, DEFAULT_VM_TARGET } from '../libauth-template/utils.js';
-import { NetworkProviderAbsoluteTimelockError } from './errors.js';
+import { NetworkProviderAbsoluteTimelockError, NetworkProviderMissingInputsError } from './errors.js';
 
 /**
  * Options accepted by the `MockNetworkProvider` constructor.
@@ -130,9 +130,8 @@ export default class MockNetworkProvider implements NetworkProvider {
         utxo.txid.toLowerCase() === binToHex(input.outpointTransactionHash) && utxo.vout === input.outpointIndex
       ));
 
-      // TODO: we should check what error a BCHN node throws, so we can throw the same error here
       if (utxoIndex === -1) {
-        throw new Error(`UTXO not found for input ${input.outpointIndex} of transaction ${txid}`);
+        throw new NetworkProviderMissingInputsError(`UTXO not found for input ${input.outpointIndex} of transaction ${txid}`);
       }
 
       return remainingUtxoEntries.splice(utxoIndex, 1)[0];

packages/cashscript/src/transaction-utils.ts

@@ -54,7 +54,7 @@ export function gatherFungibleTokenUtxos<U extends Utxo>(
   utxos: U[], tokenCategory: string, amount: bigint,
 ): GatherUtxosResult<U> {
   const sortedTokenUtxos = utxos
-    .filter((utxo) => isFungibleTokenUtxo(utxo) && utxo.token!.category === tokenCategory)
+    .filter((utxo) => isFungibleTokenUtxo(utxo) && utxo.token!.category.toLowerCase() === tokenCategory.toLowerCase())
     .toSorted((a, b) => Number(b.token!.amount - a.token!.amount));
 
   const targetUtxos: U[] = [];

packages/cashscript/src/utils.ts

@@ -76,7 +76,8 @@ export function validateInput(utxo: Utxo, changeLocks: Record<string, boolean>):
 // A UTXO/unlocker mismatch would be rejected by the network, so we catch it locally with a descriptive error
 export function validateUnlocker(utxo: SpendableUtxo, unlocker: Unlocker, inputIndex: number, network: Network): void {
   const unlockerLockingBytecode = getUnlockerLockingBytecode(unlocker);
-  if (unlockerLockingBytecode === undefined || utxo.lockingBytecode === unlockerLockingBytecode) return;
+  if (unlockerLockingBytecode === undefined) return;
+  if (utxo.lockingBytecode.toLowerCase() === unlockerLockingBytecode.toLowerCase()) return;
 
   throw new UnlockerLockingBytecodeMismatchError(
     inputIndex,
@@ -171,7 +172,8 @@ function validateChangeLocks(changeLocks: Record<string, boolean>, category?: st
     throw new OutputBchChangeLockedError();
   }
 
-  if (category && changeLocks[category]) {
+  // Token categories are hex strings, which are compared case-insensitively
+  if (category && changeLocks[category.toLowerCase()]) {
     throw new OutputTokenChangeLockedError(category);
   }
 }
@@ -317,9 +319,14 @@ export function createOpReturnOutput(
 }
 
 function toBin(output: string): Uint8Array {
-  const data = output.replace(/^0x/, '');
-  const encode = data === output ? utf8ToBin : hexToBin;
-  return encode(data);
+  if (!output.startsWith('0x')) return utf8ToBin(output);
+
+  const hex = output.slice(2);
+  if (!isHex(hex)) {
+    throw new Error(`OP_RETURN chunk should be a valid hex string with an even number of digits, found '${output}'`);
+  }
+
+  return hexToBin(hex);
 }
 
 // BCH consensus requires the fork id flag on every signing serialization