Skip to content

Commit 2f11818

Browse files
authored
Merge pull request #469 from BreadchainCoop/on-chain-check-signatures
Fixes for BLSSigCheckOperatorStateRetriever
2 parents 84fb838 + a4801d7 commit 2f11818

File tree

3 files changed

+93
-27
lines changed

3 files changed

+93
-27
lines changed

src/unaudited/BLSSigCheckOperatorStateRetriever.sol

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,18 @@ import {BitmapUtils} from "../libraries/BitmapUtils.sol";
1010
import {BN254} from "../libraries/BN254.sol";
1111
import {BN256G2} from "./BN256G2.sol";
1212
import {OperatorStateRetriever} from "../OperatorStateRetriever.sol";
13+
import {ECUtils} from "./ECUtils.sol";
1314

1415
/**
1516
* @title BLSSigCheckOperatorStateRetriever with view functions that allow to retrieve the state of an AVSs registry system.
1617
* @dev This contract inherits from OperatorStateRetriever and adds the getNonSignerStakesAndSignature function.
1718
* @author Bread coop
1819
*/
1920
contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
21+
using ECUtils for BN254.G1Point;
22+
using BN254 for BN254.G1Point;
23+
using BitmapUtils for uint256;
24+
2025
/// @dev Thrown when the signature is not on the curve.
2126
error InvalidSigma();
2227
// avoid stack too deep
@@ -61,7 +66,7 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
6166
m.blsApkRegistry = registryCoordinator.blsApkRegistry();
6267

6368
// Safe guard AVSs from generating NonSignerStakesAndSignature with invalid sigma
64-
require(_isOnCurve(sigma), InvalidSigma());
69+
require(sigma.isOnCurve(), InvalidSigma());
6570

6671
// Compute the g2 APK of the signing operator set
6772
m.signingOperatorIds = new bytes32[](operators.length);
@@ -84,13 +89,17 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
8489
{
8590
uint32[] memory signingOperatorQuorumBitmapIndices = registryCoordinator
8691
.getQuorumBitmapIndicesAtBlockNumber(blockNumber, m.signingOperatorIds);
92+
uint256 bitmap = BitmapUtils.orderedBytesArrayToBitmap(quorumNumbers);
8793
// Check that all operators are registered (this is like the check in getCheckSignaturesIndices, but we check against _signing_ operators)
8894
for (uint256 i = 0; i < operators.length; i++) {
8995
uint192 signingOperatorQuorumBitmap = registryCoordinator
9096
.getQuorumBitmapAtBlockNumberByIndex(
9197
m.signingOperatorIds[i], blockNumber, signingOperatorQuorumBitmapIndices[i]
9298
);
93-
require(signingOperatorQuorumBitmap != 0, OperatorNotRegistered());
99+
require(
100+
!uint256(signingOperatorQuorumBitmap).noBitsInCommon(bitmap),
101+
OperatorNotRegistered()
102+
);
94103
}
95104
}
96105

@@ -141,12 +150,9 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
141150

142151
// Trim the nonSignerOperatorIds array to the actual count
143152
bytes32[] memory trimmedNonSignerOperatorIds = new bytes32[](nonSignerOperatorsCount);
144-
for (uint256 i = 0; i < nonSignerOperatorsCount; i++) {
145-
trimmedNonSignerOperatorIds[i] = nonSignerOperatorIds[i];
146-
}
147-
148153
BN254.G1Point[] memory nonSignerPubkeys = new BN254.G1Point[](nonSignerOperatorsCount);
149154
for (uint256 i = 0; i < nonSignerOperatorsCount; i++) {
155+
trimmedNonSignerOperatorIds[i] = nonSignerOperatorIds[i];
150156
address nonSignerOperator =
151157
registryCoordinator.getOperatorFromId(trimmedNonSignerOperatorIds[i]);
152158
(nonSignerPubkeys[i],) = m.blsApkRegistry.getRegisteredPubkey(nonSignerOperator);
@@ -184,24 +190,8 @@ contract BLSSigCheckOperatorStateRetriever is OperatorStateRetriever {
184190
address operator = registryCoordinator.getOperatorFromId(operatorIds[i]);
185191
BN254.G1Point memory operatorPk;
186192
(operatorPk.X, operatorPk.Y) = blsApkRegistry.operatorToPubkey(operator);
187-
apk = BN254.plus(apk, operatorPk);
193+
apk = apk.plus(operatorPk);
188194
}
189195
return apk;
190196
}
191-
192-
/**
193-
* @notice Checks if a point lies on the BN254 elliptic curve
194-
* @dev The curve equation is y^2 = x^3 + 3 (mod p)
195-
* @param p The point to check, in G1
196-
* @return true if the point lies on the curve, false otherwise
197-
*/
198-
function _isOnCurve(
199-
BN254.G1Point memory p
200-
) internal pure returns (bool) {
201-
uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS);
202-
uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS);
203-
uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS);
204-
uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS);
205-
return y2 == rhs;
206-
}
207197
}

src/unaudited/ECUtils.sol

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// SPDX-License-Identifier: BUSL-1.1
2+
pragma solidity ^0.8.12;
3+
4+
import {BN254} from "../libraries/BN254.sol";
5+
6+
/**
7+
* @title ECUtils
8+
* @notice Library containing utility functions for elliptic curve operations
9+
*/
10+
library ECUtils {
11+
/**
12+
* @notice Checks if a point lies on the BN254 elliptic curve
13+
* @dev The curve equation is y^2 = x^3 + 3 (mod p)
14+
* @param p The point to check, in G1
15+
* @return true if the point lies on the curve, false otherwise
16+
*/
17+
function isOnCurve(
18+
BN254.G1Point memory p
19+
) internal pure returns (bool) {
20+
uint256 y2 = mulmod(p.Y, p.Y, BN254.FP_MODULUS);
21+
uint256 x2 = mulmod(p.X, p.X, BN254.FP_MODULUS);
22+
uint256 x3 = mulmod(p.X, x2, BN254.FP_MODULUS);
23+
uint256 rhs = addmod(x3, 3, BN254.FP_MODULUS);
24+
return y2 == rhs;
25+
}
26+
}

test/unit/BLSSigCheckOperatorStateRetriever.t.sol

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,9 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
526526
signingOperators[0] = defaultOperator;
527527

528528
// Try to query for a non-existent quorum (quorum 9)
529-
bytes memory invalidQuorumNumbers = new bytes(1);
530-
invalidQuorumNumbers[0] = bytes1(uint8(9)); // Invalid quorum number
529+
bytes memory invalidQuorumNumbers = new bytes(2);
530+
invalidQuorumNumbers[0] = bytes1(uint8(0));
531+
invalidQuorumNumbers[1] = bytes1(uint8(9)); // Invalid quorum number
531532

532533
// Should revert because quorum 9 doesn't exist, but with a different error message
533534
cheats.expectRevert(
@@ -593,8 +594,9 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
593594
signingOperators[0] = defaultOperator;
594595

595596
// Try to query for the newly created quorum but at a historical block
596-
bytes memory newQuorumNumbers = new bytes(1);
597-
newQuorumNumbers[0] = bytes1(uint8(numQuorums));
597+
bytes memory newQuorumNumbers = new bytes(2);
598+
newQuorumNumbers[0] = bytes1(uint8(0));
599+
newQuorumNumbers[1] = bytes1(uint8(8));
598600

599601
// Should revert when querying for the newly created quorum at a block before it was created
600602
cheats.expectRevert(
@@ -607,6 +609,54 @@ contract BLSSigCheckOperatorStateRetrieverUnitTests is
607609
);
608610
}
609611

612+
function test_getNonSignerStakesAndSignature_revert_operatorRegisteredToIrrelevantQuorum()
613+
public
614+
{
615+
// setup
616+
uint256 quorumBitmapOne = 1;
617+
cheats.roll(registrationBlockNumber);
618+
619+
_registerOperatorWithCoordinator(defaultOperator, quorumBitmapOne, defaultPubKey);
620+
621+
address otherOperator = _incrementAddress(defaultOperator, 1);
622+
BN254.G1Point memory otherPubKey = BN254.G1Point(1, 2);
623+
_registerOperatorWithCoordinator(
624+
otherOperator, quorumBitmapOne, otherPubKey, defaultStake - 1
625+
);
626+
627+
// Generate actual G2 pubkeys
628+
BN254.G2Point memory op1G2 = _makeG2Point(2);
629+
BN254.G2Point memory op2G2 = _makeG2Point(3);
630+
631+
// Mock the registry calls so the contract sees those G2 points
632+
vm.mockCall(
633+
address(blsApkRegistry),
634+
abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, defaultOperator),
635+
abi.encode(op1G2)
636+
);
637+
vm.mockCall(
638+
address(blsApkRegistry),
639+
abi.encodeWithSelector(IBLSApkRegistry.getOperatorPubkeyG2.selector, otherOperator),
640+
abi.encode(op2G2)
641+
);
642+
643+
// Prepare inputs
644+
BN254.G1Point memory dummySigma = BN254.scalar_mul_tiny(BN254.generatorG1(), 123);
645+
address[] memory signingOperators = new address[](2);
646+
signingOperators[0] = defaultOperator;
647+
signingOperators[1] = otherOperator;
648+
649+
bytes memory quorumNumbers = new bytes(1);
650+
quorumNumbers[0] = bytes1(uint8(2));
651+
652+
// Call the function under test
653+
vm.expectRevert(OperatorStateRetriever.OperatorNotRegistered.selector);
654+
IBLSSignatureCheckerTypes.NonSignerStakesAndSignature memory result =
655+
sigCheckOperatorStateRetriever.getNonSignerStakesAndSignature(
656+
registryCoordinator, quorumNumbers, dummySigma, signingOperators, uint32(block.number)
657+
);
658+
}
659+
610660
function _getApkAtBlocknumber(
611661
ISlashingRegistryCoordinator registryCoordinator,
612662
uint8 quorumNumber,

0 commit comments

Comments
 (0)