refactor: cleaner reverts for ECDSACertificateVerifier (#1521)

**Motivation:**

`parseSignatures` returned false for several errors, making it harder to
debug

**Modifications:**

- Revert in `_parseSignatures` right when error occurs

**Result:**

Cleaner code

Author: ypatil12

src/contracts/interfaces/IECDSACertificateVerifier.sol

@@ -7,15 +7,18 @@ import "./IOperatorTableCalculator.sol";
 
 interface IECDSACertificateVerifierTypes is IOperatorTableCalculatorTypes {
     // Errors
+    /// @notice Thrown when the signature length is invalid
     error InvalidSignatureLength();
-
+    /// @notice Thrown when the signatures are not ordered by signer address
+    error SignersNotOrdered();
     /**
      * @notice A ECDSA Certificate
      * @param referenceTimestamp the timestamp at which the certificate was created,
      *        which corresponds to a reference timestamp of the operator table update
      * @param messageHash the hash of the message that was signed by operators
      * @param sig the concatenated signature of each signing operator
      */
+
     struct ECDSACertificate {
         uint32 referenceTimestamp;
         bytes32 messageHash;

src/contracts/multichain/ECDSACertificateVerifier.sol

@@ -141,8 +141,7 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor
         bytes32 signableDigest = calculateCertificateDigest(cert.referenceTimestamp, cert.messageHash);
 
         // Parse the signatures
-        (address[] memory signers, bool validSignatures) = _parseSignatures(signableDigest, cert.sig);
-        require(validSignatures, VerificationFailed());
+        address[] memory signers = _parseSignatures(signableDigest, cert.sig);
 
         // Process each recovered signer
         for (uint256 i = 0; i < signers.length; i++) {
@@ -186,14 +185,13 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor
      * @param signableDigest The signable digest that was signed
      * @param signatures The concatenated signatures
      * @return signers Array of addresses that signed the message
-     * @return valid Whether all signatures are valid
      * @dev Signatures must be ordered by signer address (ascending)
      * @dev This does not support smart contract based signatures for multichain
      */
     function _parseSignatures(
         bytes32 signableDigest,
         bytes memory signatures
-    ) internal pure returns (address[] memory signers, bool valid) {
+    ) internal pure returns (address[] memory signers) {
         // Each ECDSA signature is 65 bytes: r (32 bytes) + s (32 bytes) + v (1 byte)
         require(signatures.length > 0 && signatures.length % 65 == 0, InvalidSignatureLength());
 
@@ -207,20 +205,16 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor
             }
 
             // Recover the signer
-            (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(signableDigest, signature);
-            if (error != ECDSA.RecoverError.NoError || recovered == address(0)) {
-                return (signers, false);
-            }
+            (address recovered, ECDSA.RecoverError err) = ECDSA.tryRecover(signableDigest, signature);
+            require(err == ECDSA.RecoverError.NoError, InvalidSignature());
 
             // Check that signatures are ordered by signer address
-            if (i > 0 && recovered <= signers[i - 1]) {
-                return (signers, false);
-            }
+            require(i == 0 || recovered > signers[i - 1], SignersNotOrdered());
 
             signers[i] = recovered;
         }
 
-        return (signers, true);
+        return signers;
     }
 
     /**

src/test/unit/ECDSACertificateVerifierUnit.t.sol

@@ -376,7 +376,23 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate
             sig: "" // Empty signatures
         });
 
-        vm.expectRevert(InvalidSignatureLength.selector);
+        vm.expectRevert(IECDSACertificateVerifierTypes.InvalidSignatureLength.selector);
+        verifier.verifyCertificate(defaultOperatorSet, cert);
+    }
+
+    function test_revert_invalidSignatureLength() public {
+        uint32 referenceTimestamp = _initializeOperatorTableBase();
+
+        // Create certificate with wrong signature length (not a multiple of 65)
+        bytes memory invalidLengthSig = new bytes(64); // Should be 65 bytes for ECDSA
+
+        IECDSACertificateVerifierTypes.ECDSACertificate memory cert = IECDSACertificateVerifierTypes.ECDSACertificate({
+            referenceTimestamp: referenceTimestamp,
+            messageHash: defaultMsgHash,
+            sig: invalidLengthSig
+        });
+
+        vm.expectRevert(IECDSACertificateVerifierTypes.InvalidSignatureLength.selector);
         verifier.verifyCertificate(defaultOperatorSet, cert);
     }
 
@@ -423,8 +439,9 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate
         // Modify the certificate to use a different message hash
         cert.messageHash = keccak256("different message");
 
-        // Verification should fail
-        vm.expectRevert(VerificationFailed.selector);
+        // Verification should fail - expect SignersNotOrdered because signature recovery
+        // with wrong message hash produces different addresses that break ordering
+        vm.expectRevert(IECDSACertificateVerifierTypes.SignersNotOrdered.selector);
         verifier.verifyCertificate(defaultOperatorSet, cert);
     }
 
@@ -678,7 +695,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate
         });
 
         // Should revert because signatures are not ordered by address
-        vm.expectRevert(VerificationFailed.selector);
+        vm.expectRevert(IECDSACertificateVerifierTypes.SignersNotOrdered.selector);
         verifier.verifyCertificate(defaultOperatorSet, cert);
     }
 
@@ -713,7 +730,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate
         });
 
         // Should revert because signature recovery returns an error
-        vm.expectRevert(VerificationFailed.selector);
+        vm.expectRevert(ISignatureUtilsMixinErrors.InvalidSignature.selector);
         verifier.verifyCertificate(defaultOperatorSet, cert);
     }
 
@@ -748,7 +765,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate
             sig: zeroRecoverySignature
         });
 
-        // Should revert because recovered address is zero
+        // Should revert because recovered address is zero - now throws VerificationFailed
         vm.expectRevert(VerificationFailed.selector);
         verifier.verifyCertificate(defaultOperatorSet, cert);
     }