Feat: generate parameters for checkSignatures by referencing OperatorStateRetriever on-chain #461
Conversation
Co-authored-by: Tomás Grüner <[email protected]>
refactor: import path refactor: rename format: forge fmt
**Motivation:** Computation of the parameters needed for signature verification is typically done through the Eigen go or rust services. However, the rust services are not reliable at the current moment and some projects might want to have signature verifications without having to rely on these services. This PR aims to make the computation of the `NonSignerStakesAndSignature` parameter easier and accessible through implementing it in `OperatorStateRetriever` contract **Modifications:** - added `getNonSignerStakesAndSignature` to `OperatorStateRetriever` - added BN256G2 solidity library dependency **Result:** Computing the data necessary for performing signature verification is more accessible
|
lgtm! |
1 similar comment
|
lgtm! |
| * @param p The point to check, in G1 | ||
| * @return true if the point lies on the curve, false otherwise | ||
| */ | ||
| function _isOnCurve( |
There was a problem hiding this comment.
should be a library method
| ISlashingRegistryCoordinator registryCoordinator, | ||
| bytes calldata quorumNumbers, | ||
| BN254.G1Point calldata sigma, | ||
| address[] calldata operators, |
There was a problem hiding this comment.
i think people want to call this with operatorIds, not addresses?
There was a problem hiding this comment.
I highly doubt that they will... we can duplicate this method to add another entrypoint with operator ids though
|
|
||
| // Trim the nonSignerOperatorIds array to the actual count | ||
| bytes32[] memory trimmedNonSignerOperatorIds = new bytes32[](nonSignerOperatorsCount); | ||
| for (uint256 i = 0; i < nonSignerOperatorsCount; i++) { |
There was a problem hiding this comment.
combine this loop and the next
| .getQuorumBitmapAtBlockNumberByIndex( | ||
| m.signingOperatorIds[i], blockNumber, signingOperatorQuorumBitmapIndices[i] | ||
| ); | ||
| require(signingOperatorQuorumBitmap != 0, OperatorNotRegistered()); |
There was a problem hiding this comment.
this should make sure they intersect with quorumNumbers
| address operator = registryCoordinator.getOperatorFromId(operatorIds[i]); | ||
| BN254.G1Point memory operatorPk; | ||
| (operatorPk.X, operatorPk.Y) = blsApkRegistry.operatorToPubkey(operator); | ||
| apk = BN254.plus(apk, operatorPk); |
fix: make quorumNotCreated tests have valid quorumNumber lists
chore: `BN254.plus` -> `apk.plus`
|
I opened #469 because I don't have write access to this branch |
|
About the CI errors: Run if diff --unified pr target; then
Only in pr: BLSSigCheckOperatorStateRetriever.md
Only in pr: BN256G2.md
Error: Differences found between PR and target branch storage layouts
Error: Process completed with exit code 1. |
|
Also, only now I noticed the commitlint CI check fails. |
|
Don't worry about the storage issues or CI errors on this one. @rubydusa |
Fixes for BLSSigCheckOperatorStateRetriever
|
cc @gpsanant for re-review |
| * @param p The point to check, in G1 | ||
| * @return true if the point lies on the curve, false otherwise | ||
| */ | ||
| function isOnCurve( |
There was a problem hiding this comment.
should be part of the BN254.sol library? or was this done due to lack of audit?
There was a problem hiding this comment.
Lack of audit, didn't want to change BN254
Motivation:
Computation of the parameters needed for signature verification is typically done through the Eigen go or rust services. However, the rust services are not reliable at the current moment and some projects might want to have signature verifications without having to rely on these services.
This PR aims to make the computation of the NonSignerStakesAndSignature parameter easier and accessible through implementing it in OperatorStateRetriever contract
Modifications:
added getNonSignerStakesAndSignature to OperatorStateRetriever
added BN256G2 solidity library dependency
Result:
Computing the data necessary for performing signature verification is more accessible