chore(gateway-contracts): add checks and comments for code quality (#1255)

Author: melanciani

gateway-contracts/contracts/CiphertextCommits.sol

@@ -202,7 +202,13 @@ contract CiphertextCommits is ICiphertextCommits, UUPSUpgradeableEmptyProxy, Gat
     function getCiphertextMaterials(
         bytes32[] calldata ctHandles
     ) external view virtual returns (CiphertextMaterial[] memory ctMaterials) {
+        // Check that the list of handles is not empty
+        if (ctHandles.length == 0) {
+            revert EmptyCtHandles();
+        }
+
         CiphertextCommitsStorage storage $ = _getCiphertextCommitsStorage();
+
         ctMaterials = new CiphertextMaterial[](ctHandles.length);
 
         for (uint256 i = 0; i < ctHandles.length; i++) {
@@ -233,6 +239,11 @@ contract CiphertextCommits is ICiphertextCommits, UUPSUpgradeableEmptyProxy, Gat
     function getSnsCiphertextMaterials(
         bytes32[] calldata ctHandles
     ) external view virtual returns (SnsCiphertextMaterial[] memory snsCtMaterials) {
+        // Check that the list of handles is not empty
+        if (ctHandles.length == 0) {
+            revert EmptyCtHandles();
+        }
+
         CiphertextCommitsStorage storage $ = _getCiphertextCommitsStorage();
         snsCtMaterials = new SnsCiphertextMaterial[](ctHandles.length);
 

gateway-contracts/contracts/Decryption.sol

@@ -547,6 +547,11 @@ contract Decryption is
         bytes32[] calldata ctHandles,
         bytes calldata /* extraData */
     ) external view virtual returns (bool) {
+        // Return false if the list of handles is empty
+        if (ctHandles.length == 0) {
+            return false;
+        }
+
         // For each handle, check that it is allowed for public decryption and that the ciphertext
         // material represented by it has been added.
         for (uint256 i = 0; i < ctHandles.length; i++) {
@@ -568,6 +573,11 @@ contract Decryption is
         CtHandleContractPair[] calldata ctHandleContractPairs,
         bytes calldata /* extraData */
     ) external view virtual returns (bool) {
+        // Return false if the list of handles is empty
+        if (ctHandleContractPairs.length == 0) {
+            return false;
+        }
+
         // For each handle, check that the user and contracts accounts have access to it and that the
         // ciphertext material represented by it has been added.
         for (uint256 i = 0; i < ctHandleContractPairs.length; i++) {

gateway-contracts/contracts/GatewayConfig.sol

@@ -610,11 +610,27 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Register the new KMS nodes
         for (uint256 i = 0; i < newKmsNodes.length; i++) {
-            $.isKmsTxSender[newKmsNodes[i].txSenderAddress] = true;
-            $.kmsNodes[newKmsNodes[i].txSenderAddress] = newKmsNodes[i];
-            $.kmsTxSenderAddresses.push(newKmsNodes[i].txSenderAddress);
-            $.isKmsSigner[newKmsNodes[i].signerAddress] = true;
-            $.kmsSignerAddresses.push(newKmsNodes[i].signerAddress);
+            address newKmsTxSenderAddress = newKmsNodes[i].txSenderAddress;
+            address newKmsSignerAddress = newKmsNodes[i].signerAddress;
+
+            // Check for KMS transaction sender and signer duplicates
+            if ($.isKmsTxSender[newKmsTxSenderAddress]) {
+                revert KmsTxSenderAlreadyRegistered(newKmsTxSenderAddress);
+            }
+            if ($.isKmsSigner[newKmsSignerAddress]) {
+                revert KmsSignerAlreadyRegistered(newKmsSignerAddress);
+            }
+
+            // Register transaction sender
+            $.isKmsTxSender[newKmsTxSenderAddress] = true;
+            $.kmsTxSenderAddresses.push(newKmsTxSenderAddress);
+
+            // Register KMS node
+            $.kmsNodes[newKmsTxSenderAddress] = newKmsNodes[i];
+
+            // Register signer
+            $.isKmsSigner[newKmsSignerAddress] = true;
+            $.kmsSignerAddresses.push(newKmsSignerAddress);
         }
 
         // Setting the thresholds should be done after the KMS nodes have been registered as the functions
@@ -642,11 +658,27 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Register the new coprocessors
         for (uint256 i = 0; i < newCoprocessors.length; i++) {
-            $.isCoprocessorTxSender[newCoprocessors[i].txSenderAddress] = true;
-            $.coprocessors[newCoprocessors[i].txSenderAddress] = newCoprocessors[i];
-            $.coprocessorTxSenderAddresses.push(newCoprocessors[i].txSenderAddress);
-            $.isCoprocessorSigner[newCoprocessors[i].signerAddress] = true;
-            $.coprocessorSignerAddresses.push(newCoprocessors[i].signerAddress);
+            address newCoprocessorTxSenderAddress = newCoprocessors[i].txSenderAddress;
+            address newCoprocessorSignerAddress = newCoprocessors[i].signerAddress;
+
+            // Check for coprocessor transaction sender and signer duplicates
+            if ($.isCoprocessorTxSender[newCoprocessorTxSenderAddress]) {
+                revert CoprocessorTxSenderAlreadyRegistered(newCoprocessorTxSenderAddress);
+            }
+            if ($.isCoprocessorSigner[newCoprocessorSignerAddress]) {
+                revert CoprocessorSignerAlreadyRegistered(newCoprocessorSignerAddress);
+            }
+
+            // Register coprocessor transaction sender
+            $.isCoprocessorTxSender[newCoprocessorTxSenderAddress] = true;
+            $.coprocessorTxSenderAddresses.push(newCoprocessorTxSenderAddress);
+
+            // Register coprocessor
+            $.coprocessors[newCoprocessorTxSenderAddress] = newCoprocessors[i];
+
+            // Register coprocessor signer
+            $.isCoprocessorSigner[newCoprocessorSignerAddress] = true;
+            $.coprocessorSignerAddresses.push(newCoprocessorSignerAddress);
         }
 
         // Setting the coprocessor threshold should be done after the coprocessors have been
@@ -667,11 +699,27 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Register the new custodians
         for (uint256 i = 0; i < newCustodians.length; i++) {
-            $.custodians[newCustodians[i].txSenderAddress] = newCustodians[i];
-            $.custodianTxSenderAddresses.push(newCustodians[i].txSenderAddress);
-            $.isCustodianTxSender[newCustodians[i].txSenderAddress] = true;
-            $.custodianSignerAddresses.push(newCustodians[i].signerAddress);
-            $.isCustodianSigner[newCustodians[i].signerAddress] = true;
+            address newCustodianTxSenderAddress = newCustodians[i].txSenderAddress;
+            address newCustodianSignerAddress = newCustodians[i].signerAddress;
+
+            // Check for custodian transaction sender and signer duplicates
+            if ($.isCustodianTxSender[newCustodianTxSenderAddress]) {
+                revert CustodianTxSenderAlreadyRegistered(newCustodianTxSenderAddress);
+            }
+            if ($.isCustodianSigner[newCustodianSignerAddress]) {
+                revert CustodianSignerAlreadyRegistered(newCustodianSignerAddress);
+            }
+
+            // Register custodian transaction sender
+            $.isCustodianTxSender[newCustodianTxSenderAddress] = true;
+            $.custodianTxSenderAddresses.push(newCustodianTxSenderAddress);
+
+            // Register custodian
+            $.custodians[newCustodianTxSenderAddress] = newCustodians[i];
+
+            // Register custodian signer
+            $.isCustodianSigner[newCustodianSignerAddress] = true;
+            $.custodianSignerAddresses.push(newCustodianSignerAddress);
         }
     }
 
@@ -703,7 +751,7 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Check that the public decryption threshold `t` is valid. It must verify:
         // - `t >= 1` : the public decryption consensus should require at least one vote
-        // - `t <= n` : it should be less than the number of registered KMS nodes
+        // - `t <= n` : it should be less than or equal to the number of registered KMS nodes
         if (newPublicDecryptionThreshold == 0) {
             revert InvalidNullPublicDecryptionThreshold();
         }
@@ -724,7 +772,7 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Check that the user decryption threshold `t` is valid. It must verify:
         // - `t >= 1` : the user decryption consensus should require at least one vote
-        // - `t <= n` : it should be less than the number of registered KMS nodes
+        // - `t <= n` : it should be less than or equal to the number of registered KMS nodes
         if (newUserDecryptionThreshold == 0) {
             revert InvalidNullUserDecryptionThreshold();
         }
@@ -745,7 +793,7 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Check that the coprocessor threshold `t` is valid. It must verify:
         // - `t >= 1` : the coprocessor consensus should require at least one vote
-        // - `t <= n` : it should be less than the number of registered coprocessors
+        // - `t <= n` : it should be less than or equal to the number of registered coprocessors
         if (newCoprocessorThreshold == 0) {
             revert InvalidNullCoprocessorThreshold();
         }
@@ -766,7 +814,7 @@ contract GatewayConfig is IGatewayConfig, Ownable2StepUpgradeable, UUPSUpgradeab
 
         // Check that the key and CRS generation threshold `t` is valid. It must verify:
         // - `t >= 1` : the key and CRS generation consensus should require at least one vote
-        // - `t <= n` : it should be less than the number of registered KMS nodes
+        // - `t <= n` : it should be less than or equal to the number of registered KMS nodes
         if (newKmsGenThreshold == 0) {
             revert InvalidNullKmsGenThreshold();
         }

gateway-contracts/contracts/InputVerification.sol

@@ -206,6 +206,9 @@ contract InputVerification is
      * @dev See {IInputVerification-verifyProofResponse}.
      * We restrict this call to coprocessor transaction senders because, in case of reorgs, we need to
      * prevent anyone else from copying the signature and sending it to trigger a consensus.
+     * Also, a user is currently allowed to send an empty list of packed ciphertexts for the input
+     * proof request, meaning the coprocessors will respond with an empty list of handles. We thus
+     * don't revert if the `ctHandles` list is empty on purpose.
      */
     function verifyProofResponse(
         uint256 zkProofId,

gateway-contracts/contracts/interfaces/ICiphertextCommits.sol

@@ -48,6 +48,11 @@ interface ICiphertextCommits {
      */
     error CoprocessorAlreadyAdded(bytes32 ctHandle, address txSender);
 
+    /**
+     * @notice Error indicating that the list of handles is empty.
+     */
+    error EmptyCtHandles();
+
     /**
      * @notice Error indicating that the given ciphertext material represented by the given handle has not
      * been added in the contract.

gateway-contracts/contracts/interfaces/IGatewayConfig.sol

@@ -127,16 +127,52 @@ interface IGatewayConfig {
      */
     error EmptyKmsNodes();
 
+    /**
+     * @notice Error emitted when the KMS transaction sender is already registered.
+     * @param kmsTxSenderAddress The KMS transaction sender address.
+     */
+    error KmsTxSenderAlreadyRegistered(address kmsTxSenderAddress);
+
+    /**
+     * @notice Error emitted when the KMS signer is already registered.
+     * @param kmsSignerAddress The KMS signer address.
+     */
+    error KmsSignerAlreadyRegistered(address kmsSignerAddress);
+
     /**
      * @notice Error emitted when the coprocessors list is empty.
      */
     error EmptyCoprocessors();
 
+    /**
+     * @notice Error emitted when the coprocessor transaction sender is already registered.
+     * @param coprocessorTxSenderAddress The coprocessor transaction sender address.
+     */
+    error CoprocessorTxSenderAlreadyRegistered(address coprocessorTxSenderAddress);
+
+    /**
+     * @notice Error emitted when the coprocessor signer is already registered.
+     * @param coprocessorSignerAddress The coprocessor signer address.
+     */
+    error CoprocessorSignerAlreadyRegistered(address coprocessorSignerAddress);
+
     /**
      * @notice Error emitted when the custodians list is empty.
      */
     error EmptyCustodians();
 
+    /**
+     * @notice Error emitted when the custodian transaction sender is already registered.
+     * @param custodianTxSenderAddress The custodian transaction sender address.
+     */
+    error CustodianTxSenderAlreadyRegistered(address custodianTxSenderAddress);
+
+    /**
+     * @notice Error emitted when the custodian signer is already registered.
+     * @param custodianSignerAddress The custodian signer address.
+     */
+    error CustodianSignerAlreadyRegistered(address custodianSignerAddress);
+
     /**
      * @notice Error emitted when the MPC threshold is greater or equal to the number of KMS nodes.
      * @param mpcThreshold The MPC threshold.