diff --git a/src/contracts/interfaces/IECDSACertificateVerifier.sol b/src/contracts/interfaces/IECDSACertificateVerifier.sol index 1adb83933d..f6bd27082c 100644 --- a/src/contracts/interfaces/IECDSACertificateVerifier.sol +++ b/src/contracts/interfaces/IECDSACertificateVerifier.sol @@ -51,11 +51,12 @@ interface IECDSACertificateVerifier is IECDSACertificateVerifierEvents, IBaseCer * @param cert a certificate * @return signedStakes amount of stake that signed the certificate for each stake * type. Each index corresponds to a stake type in the `weights` array in the `ECDSAOperatorInfo` + * @return signers array of addresses that signed the certificate */ function verifyCertificate( OperatorSet calldata operatorSet, ECDSACertificate memory cert - ) external returns (uint256[] memory signedStakes); + ) external returns (uint256[] memory signedStakes, address[] memory signers); /** * @notice verifies a certificate and makes sure that the signed stakes meet @@ -65,12 +66,13 @@ interface IECDSACertificateVerifier is IECDSACertificateVerifierEvents, IBaseCer * the signed stake of the certificate should meet. Each index corresponds to * a stake type in the `weights` array in the `ECDSAOperatorInfo` * @return Whether or not the certificate is valid and meets thresholds + * @return signers array of addresses that signed the certificate */ function verifyCertificateProportion( OperatorSet calldata operatorSet, ECDSACertificate memory cert, uint16[] memory totalStakeProportionThresholds - ) external returns (bool); + ) external returns (bool, address[] memory signers); /** * @notice verifies a certificate and makes sure that the signed stakes meet @@ -80,12 +82,13 @@ interface IECDSACertificateVerifier is IECDSACertificateVerifierEvents, IBaseCer * the signed stake of the certificate should meet. Each index corresponds to * a stake type in the `weights` array in the `ECDSAOperatorInfo` * @return Whether or not the certificate is valid and meets thresholds + * @return signers array of addresses that signed the certificate */ function verifyCertificateNominal( OperatorSet calldata operatorSet, ECDSACertificate memory cert, uint256[] memory totalStakeNominalThresholds - ) external returns (bool); + ) external returns (bool, address[] memory signers); /** * @notice Get operator infos for a timestamp diff --git a/src/contracts/multichain/ECDSACertificateVerifier.sol b/src/contracts/multichain/ECDSACertificateVerifier.sol index 00d897092c..f2668f7d9b 100644 --- a/src/contracts/multichain/ECDSACertificateVerifier.sol +++ b/src/contracts/multichain/ECDSACertificateVerifier.sol @@ -74,8 +74,9 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor function verifyCertificate( OperatorSet calldata operatorSet, ECDSACertificate calldata cert - ) external view returns (uint256[] memory) { - return _verifyECDSACertificate(operatorSet, cert); + ) external view returns (uint256[] memory, address[] memory) { + (uint256[] memory signedStakes, address[] memory signers) = _verifyECDSACertificate(operatorSet, cert); + return (signedStakes, signers); } ///@inheritdoc IECDSACertificateVerifier @@ -83,17 +84,17 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor OperatorSet calldata operatorSet, ECDSACertificate calldata cert, uint16[] calldata totalStakeProportionThresholds - ) external view returns (bool) { - uint256[] memory signedStakes = _verifyECDSACertificate(operatorSet, cert); + ) external view returns (bool, address[] memory) { + (uint256[] memory signedStakes, address[] memory signers) = _verifyECDSACertificate(operatorSet, cert); uint256[] memory totalStakes = getTotalStakes(operatorSet, cert.referenceTimestamp); require(signedStakes.length == totalStakeProportionThresholds.length, ArrayLengthMismatch()); for (uint256 i = 0; i < signedStakes.length; i++) { uint256 threshold = (totalStakes[i] * totalStakeProportionThresholds[i]) / 10_000; if (signedStakes[i] < threshold) { - return false; + return (false, signers); } } - return true; + return (true, signers); } ///@inheritdoc IECDSACertificateVerifier @@ -101,15 +102,15 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor OperatorSet calldata operatorSet, ECDSACertificate calldata cert, uint256[] memory totalStakeNominalThresholds - ) external view returns (bool) { - uint256[] memory signedStakes = _verifyECDSACertificate(operatorSet, cert); + ) external view returns (bool, address[] memory) { + (uint256[] memory signedStakes, address[] memory signers) = _verifyECDSACertificate(operatorSet, cert); require(signedStakes.length == totalStakeNominalThresholds.length, ArrayLengthMismatch()); for (uint256 i = 0; i < signedStakes.length; i++) { if (signedStakes[i] < totalStakeNominalThresholds[i]) { - return false; + return (false, signers); } } - return true; + return (true, signers); } /** @@ -120,7 +121,7 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor function _verifyECDSACertificate( OperatorSet calldata operatorSet, ECDSACertificate calldata cert - ) internal view returns (uint256[] memory) { + ) internal view returns (uint256[] memory, address[] memory) { bytes32 operatorSetKey = operatorSet.key(); // Assert that reference timestamp is not stale @@ -171,7 +172,7 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor } } - return signedStakes; + return (signedStakes, signers); } /** @@ -182,7 +183,7 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor /** * @notice Parse signatures from the concatenated signature bytes - * @param messageHash The message hash that was signed + * @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 @@ -190,9 +191,9 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor * @dev This does not support smart contract based signatures for multichain */ function _parseSignatures( - bytes32 messageHash, + bytes32 signableDigest, bytes memory signatures - ) internal view returns (address[] memory signers, bool valid) { + ) internal pure returns (address[] memory signers, bool valid) { // Each ECDSA signature is 65 bytes: r (32 bytes) + s (32 bytes) + v (1 byte) require(signatures.length > 0 && signatures.length % 65 == 0, InvalidSignatureLength()); @@ -206,7 +207,7 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor } // Recover the signer - (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(messageHash, signature); + (address recovered, ECDSA.RecoverError error) = ECDSA.tryRecover(signableDigest, signature); if (error != ECDSA.RecoverError.NoError || recovered == address(0)) { return (signers, false); } @@ -216,9 +217,6 @@ contract ECDSACertificateVerifier is Initializable, ECDSACertificateVerifierStor return (signers, false); } - // Verify that the recovered address actually signed the message - _checkIsValidSignatureNow(recovered, messageHash, signature, type(uint256).max); - signers[i] = recovered; } diff --git a/src/test/unit/ECDSACertificateVerifierUnit.t.sol b/src/test/unit/ECDSACertificateVerifierUnit.t.sol index e420e02ffa..27c5953e58 100644 --- a/src/test/unit/ECDSACertificateVerifierUnit.t.sol +++ b/src/test/unit/ECDSACertificateVerifierUnit.t.sol @@ -441,7 +441,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate IECDSACertificateVerifierTypes.ECDSACertificate memory cert = _createCertificate(referenceTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory signers) = verifier.verifyCertificate(defaultOperatorSet, cert); // Calculate total stakes uint[] memory totalStakes = new uint[](2); @@ -470,7 +470,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate IECDSACertificateVerifierTypes.ECDSACertificate memory cert = _createCertificate(referenceTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory signers) = verifier.verifyCertificate(defaultOperatorSet, cert); // Check that the signed stakes are correct assertEq(signedStakes.length, 2, "Wrong number of stake types"); @@ -525,7 +525,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate IECDSACertificateVerifierTypes.ECDSACertificate memory cert = _createCertificate(secondTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory signers) = verifier.verifyCertificate(defaultOperatorSet, cert); // Verify all stakes are signed uint[] memory totalStakes = new uint[](2); @@ -565,7 +565,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificate is ECDSACertificate IECDSACertificateVerifierTypes.ECDSACertificate memory cert = _createCertificate(secondTimestamp, defaultMsgHash, nonSignerIndices, secondOperators, signerPrivKeys); - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory signers) = verifier.verifyCertificate(defaultOperatorSet, cert); // Verify only signers' stakes are counted uint[] memory expectedSignedStakes = new uint[](2); @@ -800,7 +800,7 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateProportion is ECDSAC thresholds[0] = 6000; // 60% thresholds[1] = 6000; // 60% - bool meetsThresholds = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); + (bool meetsThresholds, address[] memory signers) = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); // Calculate expected result based on the number of signers uint signedPercentage = (numSigners * 10_000) / numOperators; @@ -829,10 +829,10 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateProportion is ECDSAC thresholds[0] = 9000; // 90% thresholds[1] = 9000; // 90% - bool meetsThresholds = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); + (bool meetsThresholds, address[] memory signers) = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); // Calculate percentage of signed stakes to determine if it should meet threshold - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory certSigners) = verifier.verifyCertificate(defaultOperatorSet, cert); uint[] memory totalStakes = verifier.getTotalStakes(defaultOperatorSet, referenceTimestamp); uint signedPercentage0 = (signedStakes[0] * 10_000) / totalStakes[0]; uint signedPercentage1 = (signedStakes[1] * 10_000) / totalStakes[1]; @@ -864,10 +864,10 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateProportion is ECDSAC thresholds[0] = threshold0; thresholds[1] = threshold1; - bool meetsThresholds = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); + (bool meetsThresholds, address[] memory signers) = verifier.verifyCertificateProportion(defaultOperatorSet, cert, thresholds); // Calculate expected result - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory certSigners) = verifier.verifyCertificate(defaultOperatorSet, cert); uint[] memory totalStakes = verifier.getTotalStakes(defaultOperatorSet, referenceTimestamp); bool expectedResult = true; @@ -927,14 +927,14 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateNominal is ECDSACert _createCertificate(referenceTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); // Get the signed stakes for reference - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory certSigners) = verifier.verifyCertificate(defaultOperatorSet, cert); // Test with thresholds lower than signed stakes (should pass) uint[] memory passThresholds = new uint[](2); passThresholds[0] = signedStakes[0] > 10 ? signedStakes[0] - 10 : 0; passThresholds[1] = signedStakes[1] > 10 ? signedStakes[1] - 10 : 0; - bool meetsThresholds = verifier.verifyCertificateNominal(defaultOperatorSet, cert, passThresholds); + (bool meetsThresholds, address[] memory signers) = verifier.verifyCertificateNominal(defaultOperatorSet, cert, passThresholds); assertTrue(meetsThresholds, "Certificate should meet nominal thresholds"); } @@ -954,14 +954,14 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateNominal is ECDSACert _createCertificate(referenceTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); // Get the signed stakes for reference - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes, address[] memory certSigners) = verifier.verifyCertificate(defaultOperatorSet, cert); // Test with thresholds higher than signed stakes (should fail) uint[] memory failThresholds = new uint[](2); failThresholds[0] = signedStakes[0] + 10; failThresholds[1] = signedStakes[1] + 10; - bool meetsThresholds = verifier.verifyCertificateNominal(defaultOperatorSet, cert, failThresholds); + (bool meetsThresholds, address[] memory signers) = verifier.verifyCertificateNominal(defaultOperatorSet, cert, failThresholds); assertFalse(meetsThresholds, "Certificate should not meet impossible nominal thresholds"); } @@ -982,20 +982,20 @@ contract ECDSACertificateVerifierUnitTests_verifyCertificateNominal is ECDSACert _createCertificate(referenceTimestamp, defaultMsgHash, nonSignerIndices, operators, signerPrivKeys); // Get the signed stakes for reference - uint[] memory signedStakes = verifier.verifyCertificate(defaultOperatorSet, cert); + (uint[] memory signedStakes,) = verifier.verifyCertificate(defaultOperatorSet, cert); // Bound thresholds to reasonable values - threshold0 = bound(threshold0, 0, signedStakes[0] * 2); - threshold1 = bound(threshold1, 0, signedStakes[1] * 2); + uint boundedThreshold0 = bound(threshold0, 0, signedStakes[0] * 2); + uint boundedThreshold1 = bound(threshold1, 0, signedStakes[1] * 2); uint[] memory thresholds = new uint[](2); - thresholds[0] = threshold0; - thresholds[1] = threshold1; + thresholds[0] = boundedThreshold0; + thresholds[1] = boundedThreshold1; - bool meetsThresholds = verifier.verifyCertificateNominal(defaultOperatorSet, cert, thresholds); + (bool meetsThresholds,) = verifier.verifyCertificateNominal(defaultOperatorSet, cert, thresholds); // Expected result - bool expectedResult = (signedStakes[0] >= threshold0) && (signedStakes[1] >= threshold1); + bool expectedResult = (signedStakes[0] >= boundedThreshold0) && (signedStakes[1] >= boundedThreshold1); assertEq(meetsThresholds, expectedResult, "Nominal threshold check mismatch"); }