Skip to content

Commit fc1984e

Browse files
authored
fix: enforce ordering of nonsigners in bn254CV (#1615)
**Motivation:** Currently there is no ordering check for non-signers in the BN254CV, which can present some edge cases with duplicate non-signers. **Modifications:** Ordering check for non-signers. **Result:** Addresses minor issue brought up in audit.
1 parent cd5612e commit fc1984e

File tree

3 files changed

+19
-0
lines changed

3 files changed

+19
-0
lines changed

docs/multichain/destination/CertificateVerifier.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,7 @@ The contract supports 3 verification patterns:
427427
* @param signature the G1 signature of the message. The signature is over the signable digest, which is calculated by `calculateCertificateDigest`
428428
* @param apk the G2 aggregate public key
429429
* @param nonSignerWitnesses an array of witnesses of non-signing operators
430+
* @dev Non-signer witnesses MUST be strictly increasing by `operatorIndex`
430431
* @dev The `referenceTimestamp` is used to key into the operatorSet's stake weights. It is NOT the timestamp at which the certificate was generated off-chain
431432
*/
432433
struct BN254Certificate {
@@ -475,6 +476,7 @@ Verifies a BN254 certificate by checking the aggregated signature against the op
475476
* The root at the `referenceTimestamp` MUST not be disabled
476477
* The operator set info MUST exist for the `referenceTimestamp`
477478
* The `operatorIndex` must be valid for the non signer
479+
* The non-signer witnesses MUST be strictly increasing by `operatorIndex`
478480
* All merkle proofs for nonsigners MUST be valid
479481
* The BLS signature MUST verify correctly
480482

src/contracts/interfaces/IBN254CertificateVerifier.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ interface IBN254CertificateVerifierTypes is IOperatorTableCalculatorTypes {
3131
* @param signature the G1 signature of the message. The signature is over the signable digest, which is calculated by `calculateCertificateDigest`
3232
* @param apk the G2 aggregate public key
3333
* @param nonSignerWitnesses an array of witnesses of non-signing operators
34+
* @dev Non-signer witnesses MUST be strictly increasing by `operatorIndex`
3435
* @dev The `referenceTimestamp` is used to key into the operatorSet's stake weights. It is NOT the timestamp at which the certificate was generated off-chain
3536
*/
3637
struct BN254Certificate {
@@ -52,6 +53,11 @@ interface IBN254CertificateVerifierErrors {
5253
/// @dev Error code: 0x03f4a78e
5354
/// @dev We enforce that operator indices are within valid bounds to prevent out-of-bounds access in the merkle tree verification
5455
error InvalidOperatorIndex();
56+
57+
/// @notice thrown when the non-signer witnesses are not strictly increasing by operatorIndex
58+
/// @dev Error code: 0xec6268b8
59+
/// @dev We enforce strictly increasing order to prevent duplicates and ensure deterministic processing
60+
error NonSignerIndicesNotSorted();
5561
}
5662

5763
/// @notice An interface for verifying BN254 certificates
@@ -128,6 +134,7 @@ interface IBN254CertificateVerifier is
128134
* - ReferenceTimestampDoesNotExist: No operator table exists for the referenceTimestamp
129135
* - RootDisabled: The global table root for this timestamp has been disabled
130136
* - InvalidOperatorIndex: Operator index provided in nonSigner witness is invalid
137+
* - NonSignerIndicesNotSorted: Non-signer witnesses are not strictly increasing by operatorIndex
131138
* - VerificationFailed: Merkle proof verification failed or BLS signature verification failed
132139
*/
133140
function verifyCertificate(
@@ -156,6 +163,7 @@ interface IBN254CertificateVerifier is
156163
* - ReferenceTimestampDoesNotExist: No operator table exists for the referenceTimestamp
157164
* - RootDisabled: The global table root for this timestamp has been disabled
158165
* - InvalidOperatorIndex: Operator index provided in nonSigner witness is invalid
166+
* - NonSignerIndicesNotSorted: Non-signer witnesses are not strictly increasing by operatorIndex
159167
* - VerificationFailed: Merkle proof verification failed or BLS signature verification failed
160168
* - ArrayLengthMismatch: signedStakes length does not equal totalStakeProportionThresholds length
161169
*/
@@ -186,6 +194,7 @@ interface IBN254CertificateVerifier is
186194
* - ReferenceTimestampDoesNotExist: No operator table exists for the referenceTimestamp
187195
* - RootDisabled: The global table root for this timestamp has been disabled
188196
* - InvalidOperatorIndex: Operator index provided in nonSigner witness is invalid
197+
* - NonSignerIndicesNotSorted: Non-signer witnesses are not strictly increasing by operatorIndex
189198
* - VerificationFailed: Merkle proof verification failed or BLS signature verification failed
190199
* - ArrayLengthMismatch: signedStakes length does not equal totalStakeNominalThresholds length
191200
*/

src/contracts/multichain/BN254CertificateVerifier.sol

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,16 @@ contract BN254CertificateVerifier is
202202
BN254Certificate memory cert
203203
) internal returns (BN254.G1Point memory nonSignerApk) {
204204
nonSignerApk = BN254.G1Point(0, 0);
205+
uint32 previousOperatorIndex = 0;
205206

206207
for (uint256 i = 0; i < cert.nonSignerWitnesses.length; i++) {
207208
BN254OperatorInfoWitness memory witness = cert.nonSignerWitnesses[i];
208209

210+
if (i > 0) {
211+
// Enforce strictly increasing order of non-signer operator indices
212+
require(witness.operatorIndex > previousOperatorIndex, NonSignerIndicesNotSorted());
213+
}
214+
209215
require(witness.operatorIndex < ctx.operatorSetInfo.numOperators, InvalidOperatorIndex());
210216

211217
BN254OperatorInfo memory operatorInfo =
@@ -219,6 +225,8 @@ contract BN254CertificateVerifier is
219225
ctx.totalSignedStakeWeights[j] -= operatorInfo.weights[j];
220226
}
221227
}
228+
229+
previousOperatorIndex = witness.operatorIndex;
222230
}
223231
}
224232

0 commit comments

Comments
 (0)