fixup! Refactor signing validation

src/commandExecutor.ts

@@ -129,7 +129,7 @@ const CommandExecutor = async () => {
     console.log(scriptHashHex)
   }
 
-  const createTxWitness = async (args: ParsedTransactionWitnessArguments) => {
+  const createTxWitnesses = async (args: ParsedTransactionWitnessArguments) => {
     const unsignedTxParsed = parseUnsignedTx(args.txBodyFileData.cborHex)
     const signingParameters = {
       signingMode: determineSigningMode(unsignedTxParsed, args.hwSigningFileData),
@@ -214,7 +214,7 @@ const CommandExecutor = async () => {
     createVerificationKeyFile,
     createSignedTx,
     createTxPolicyId,
-    createTxWitness,
+    createTxWitnesses,
     createNodeSigningKeyFiles,
     createSignedOperationalCertificate,
     createCatalystVotingKeyRegistrationMetadata,

src/crypto-providers/signingValidation.ts

@@ -13,68 +13,69 @@ import {
   filterSigningFiles,
 } from './util'
 
-const _validateNonMultisigTx = (unsignedTxParsed: _UnsignedTxParsed) => {
-  // We require non-multisig transactions to use certificates and withdrawals
-  // only with AddrKeyHash stake credentials
-  const certificatesOk = certificatesWithStakeCredentials(unsignedTxParsed.certificates).every(
-    (cert) => cert.stakeCredential.type === StakeCredentialType.ADDR_KEY_HASH,
+const _validateStakeCredentialsAreAllOfType = (
+  unsignedTxParsed: _UnsignedTxParsed, stakeCredentailType: StakeCredentialType, errorType: Errors,
+) => {
+  const certificatesHaveCorrectType = certificatesWithStakeCredentials(unsignedTxParsed.certificates).every(
+    (cert) => cert.stakeCredential.type === stakeCredentailType,
   )
-  const withdrawalsOk = unsignedTxParsed.withdrawals.every(
-    (withdrawal) => withdrawal.stakeCredential.type === StakeCredentialType.ADDR_KEY_HASH,
+  const withdrawalsHaveCorrectType = unsignedTxParsed.withdrawals.every(
+    (withdrawal) => withdrawal.stakeCredential.type === stakeCredentailType,
   )
-  if (!certificatesOk || !withdrawalsOk) {
-    throw Error(Errors.ScriptStakeCredentialInOrdinaryTx)
+  if (!certificatesHaveCorrectType || !withdrawalsHaveCorrectType) {
+    throw Error(errorType)
   }
 }
 
-const _countWithdrawalAndCertificateWitnesses = (unsignedTxParsed: _UnsignedTxParsed) => {
-  let numStakeWitnesses = unsignedTxParsed.withdrawals.length
-  let numPoolColdWitnesses = 0
+const _countWitnessableItems = (unsignedTxParsed: _UnsignedTxParsed) => {
+  // we count stake registrations separately because they don't necessarily require a staking witness
+  let numStakeRegistrationItems = 0
+  let numStakeOtherItems = unsignedTxParsed.withdrawals.length
+  let numPoolColdItems = 0
   unsignedTxParsed.certificates.forEach((cert) => {
     switch (cert.type) {
       case TxCertificateKeys.STAKING_KEY_REGISTRATION:
+        numStakeRegistrationItems += 1
+        break
+
       case TxCertificateKeys.STAKING_KEY_DEREGISTRATION:
       case TxCertificateKeys.DELEGATION:
-        numStakeWitnesses += 1
+        numStakeOtherItems += 1
         break
 
       case TxCertificateKeys.STAKEPOOL_RETIREMENT:
-        numPoolColdWitnesses += 1
+        numPoolColdItems += 1
         break
 
       default:
         break
     }
   })
-  return { numStakeWitnesses, numPoolColdWitnesses }
+  return { numStakeRegistrationItems, numStakeOtherItems, numPoolColdItems }
 }
 
 const validateOrdinaryTx = (unsignedTxParsed: _UnsignedTxParsed) => {
-  _validateNonMultisigTx(unsignedTxParsed)
-
-  if (unsignedTxParsed.inputs.length === 0) {
-    throw Error(Errors.MissingInputError)
-  }
+  _validateStakeCredentialsAreAllOfType(
+    unsignedTxParsed, StakeCredentialType.ADDR_KEY_HASH, Errors.ScriptStakeCredentialInOrdinaryTx,
+  )
 }
 
-// TODO we don't count unique witnesses
 const validateOrdinaryWitnesses = (params: SigningParameters) => {
   const {
     stakeSigningFiles, poolColdSigningFiles, mintSigningFiles, multisigSigningFiles,
   } = filterSigningFiles(params.hwSigningFileData)
 
   const {
-    numStakeWitnesses, numPoolColdWitnesses,
-  } = _countWithdrawalAndCertificateWitnesses(params.unsignedTxParsed)
-  const numMintWitnesses = params.unsignedTxParsed.mint?.length || 0
+    numStakeRegistrationItems, numStakeOtherItems, numPoolColdItems,
+  } = _countWitnessableItems(params.unsignedTxParsed)
 
-  if (stakeSigningFiles.length > numStakeWitnesses) {
+  if (numStakeRegistrationItems + numStakeOtherItems === 0 && stakeSigningFiles.length > 0) {
     throw Error(Errors.TooManyStakeSigningFilesError)
   }
-  if (poolColdSigningFiles.length > numPoolColdWitnesses) {
+  if (numPoolColdItems === 0 && poolColdSigningFiles.length > 0) {
     throw Error(Errors.TooManyPoolColdSigningFilesError)
   }
-  if (mintSigningFiles.length > numMintWitnesses) {
+  if (!params.unsignedTxParsed.mint && mintSigningFiles.length > 0) {
     throw Error(Errors.TooManyMintSigningFilesError)
   }
   if (multisigSigningFiles.length > 0) {
@@ -83,38 +84,35 @@ const validateOrdinaryWitnesses = (params: SigningParameters) => {
 }
 
 const validateOrdinarySigning = (params: SigningParameters) => {
+  validateOrdinaryWitnesses(params)
   // these checks should be performed only with createSignedTx call (when the signing file set is
   // expected to be complete) on top of validateOrdinaryWitnesses
+
   const {
-    paymentSigningFiles, stakeSigningFiles, poolColdSigningFiles, mintSigningFiles,
+    paymentSigningFiles, stakeSigningFiles, poolColdSigningFiles,
   } = filterSigningFiles(params.hwSigningFileData)
 
   if (paymentSigningFiles.length === 0) {
     throw Error(Errors.MissingPaymentSigningFileError)
   }
 
   const {
-    numStakeWitnesses, numPoolColdWitnesses,
-  } = _countWithdrawalAndCertificateWitnesses(params.unsignedTxParsed)
-  const numMintWitnesses = params.unsignedTxParsed.mint?.length || 0
+    numStakeOtherItems, numPoolColdItems,
+  } = _countWitnessableItems(params.unsignedTxParsed)
 
-  if (numStakeWitnesses > 0 && (stakeSigningFiles.length === 0)) {
+  if (numStakeOtherItems > 0 && (stakeSigningFiles.length === 0)) {
     throw Error(Errors.MissingStakeSigningFileError)
   }
-  if (numPoolColdWitnesses > 0 && (poolColdSigningFiles.length === 0)) {
+  if (numPoolColdItems > 0 && (poolColdSigningFiles.length === 0)) {
     throw Error(Errors.MissingPoolColdSigningFileError)
   }
-  if (numMintWitnesses > 0 && (mintSigningFiles.length === 0)) {
-    throw Error(Errors.TooManyMintSigningFilesError)
-  }
 }
 
 const validatePoolRegistrationTx = (unsignedTxParsed: _UnsignedTxParsed) => {
-  _validateNonMultisigTx(unsignedTxParsed)
+  _validateStakeCredentialsAreAllOfType(
+    unsignedTxParsed, StakeCredentialType.ADDR_KEY_HASH, Errors.ScriptStakeCredentialInOrdinaryTx,
+  )
 
-  if (unsignedTxParsed.inputs.length === 0) {
-    throw Error(Errors.MissingInputError)
-  }
   if (unsignedTxParsed.certificates.length !== 1) {
     throw Error(Errors.MultipleCertificatesWithPoolRegError)
   }
@@ -135,6 +133,7 @@ const validatePoolOwnerWitnesses = (params: SigningParameters) => {
     throw Error(Errors.MissingStakeSigningFileError)
   }
   if (stakeSigningFiles.length > 1) {
+    // we need exactly one signing file in order to unambiguously determine the owner to be witnessed
     throw Error(Errors.TooManyStakeSigningFilesError)
   }
   if (poolColdSigningFiles.length > 0) {
@@ -153,33 +152,53 @@ const validatePoolOperatorWitnesses = (params: SigningParameters) => {
     stakeSigningFiles, poolColdSigningFiles, multisigSigningFiles,
   } = filterSigningFiles(params.hwSigningFileData)
 
-  if (poolColdSigningFiles.length === 0) {
-    throw Error(Errors.MissingPoolColdSigningFileError)
-  }
   if (stakeSigningFiles.length > 0) {
     throw Error(Errors.TooManyStakeSigningFilesError)
   }
+  if (poolColdSigningFiles.length === 0) {
+    throw Error(Errors.MissingPoolColdSigningFileError)
+  }
   if (multisigSigningFiles.length > 0) {
     throw Error(Errors.TooManyMultisigSigningFilesError)
   }
 }
 
 const validateMultisigTx = (unsignedTxParsed: _UnsignedTxParsed) => {
-  // We require multisig transactions to use certificates and withdrawals
-  // only with ScriptHash stake credentials
-  const certificatesOk = certificatesWithStakeCredentials(unsignedTxParsed.certificates).every(
-    (cert) => cert.stakeCredential.type === StakeCredentialType.SCRIPT_HASH,
-  )
-  const withdrawalsOk = unsignedTxParsed.withdrawals.every(
-    (withdrawal) => withdrawal.stakeCredential.type === StakeCredentialType.SCRIPT_HASH,
+  _validateStakeCredentialsAreAllOfType(
+    unsignedTxParsed, StakeCredentialType.SCRIPT_HASH, Errors.KeyHashStakeCredentialInMultisigTx,
   )
-  if (!certificatesOk || !withdrawalsOk) {
-    throw Error(Errors.NonScriptStakeCredentialInMultisigTx)
+}
+
+const validateMultisigWitnesses = (params: SigningParameters) => {
+  const {
+    paymentSigningFiles, stakeSigningFiles, poolColdSigningFiles, mintSigningFiles,
+  } = filterSigningFiles(params.hwSigningFileData)
+
+  if (paymentSigningFiles.length > 0) {
+    throw Error(Errors.TooManyPaymentSigningFilesError)
+  }
+  if (stakeSigningFiles.length > 0) {
+    throw Error(Errors.TooManyStakeSigningFilesError)
+  }
+  if (poolColdSigningFiles.length > 0) {
+    throw Error(Errors.TooManyPoolColdSigningFilesError)
   }
+  if (!params.unsignedTxParsed.mint && mintSigningFiles.length > 0) {
+    throw Error(Errors.TooManyMintSigningFilesError)
+  }
+}
+
+const validateMultisigSigning = (params: SigningParameters) => {
+  validateMultisigWitnesses(params)
+  // there are no checks that can be done (except those in validateMultisigWitnesses)
 }
 
 const validateWitnessing = (params: SigningParameters): void => {
   // verifies whether the tx and signing files correspond to to each other and to the signing mode
+  if (params.unsignedTxParsed.inputs.length === 0) {
+    throw Error(Errors.MissingInputError)
+  }
+
   switch (params.signingMode) {
     case SigningMode.ORDINARY_TRANSACTION:
       validateOrdinaryTx(params.unsignedTxParsed)
@@ -198,6 +217,7 @@ const validateWitnessing = (params: SigningParameters): void => {
 
     case SigningMode.MULTISIG_TRANSACTION:
       validateMultisigTx(params.unsignedTxParsed)
+      validateMultisigWitnesses(params)
       break
 
     default:
@@ -206,10 +226,13 @@ const validateWitnessing = (params: SigningParameters): void => {
 }
 
 const validateSigning = (params: SigningParameters): void => {
+  if (params.unsignedTxParsed.inputs.length === 0) {
+    throw Error(Errors.MissingInputError)
+  }
+
   switch (params.signingMode) {
     case SigningMode.ORDINARY_TRANSACTION:
       validateOrdinaryTx(params.unsignedTxParsed)
-      validateOrdinaryWitnesses(params)
       validateOrdinarySigning(params)
       break
 
@@ -219,6 +242,7 @@ const validateSigning = (params: SigningParameters): void => {
 
     case SigningMode.MULTISIG_TRANSACTION:
       validateMultisigTx(params.unsignedTxParsed)
+      validateMultisigSigning(params)
       break
 
     default:

src/index.ts

@@ -39,7 +39,7 @@ const executeCommand = async (): Promise<void> => {
       await commandExecutor.createTxPolicyId(parsedArgs)
       break
     case (CommandType.WITNESS_TRANSACTION):
-      await commandExecutor.createTxWitness(parsedArgs)
+      await commandExecutor.createTxWitnesses(parsedArgs)
       break
     case (CommandType.NODE_KEY_GEN):
       await commandExecutor.createNodeSigningKeyFiles(parsedArgs)