Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: cleanup for compendium changes #102

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Chore: cleanup for compendium changes #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
630b27d
feat: use EIP712 for the pubkeyRegistrationMessageHash
ChaoticWalrus Dec 12, 2023
ef7147f
chore: add NatSpec to getter function
ChaoticWalrus Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 8 additions & 20 deletions src/BLSApkRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,12 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
* @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key.
* @param operator is the operator for whom the key is being registered
* @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership
* @param pubkeyRegistrationMessageHash is a hash that the operator must sign to prove key ownership
*/
function registerBLSPublicKey(
address operator,
PubkeyRegistrationParams calldata params
PubkeyRegistrationParams calldata params,
BN254.G1Point calldata pubkeyRegistrationMessageHash
) external onlyRegistryCoordinator returns (bytes32 operatorId) {
bytes32 pubkeyHash = BN254.hashG1Point(params.pubkeyG1);
require(
Expand All @@ -115,9 +117,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
"BLSApkRegistry.registerBLSPublicKey: public key already registered"
);

// H(m)
BN254.G1Point memory messageHash = getMessageHash(operator);

// gamma = h(sigma, P, P', H(m))
uint256 gamma = uint256(keccak256(abi.encodePacked(
params.pubkeyRegistrationSignature.X,
Expand All @@ -126,15 +125,15 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
params.pubkeyG1.Y,
params.pubkeyG2.X,
params.pubkeyG2.Y,
messageHash.X,
messageHash.Y
pubkeyRegistrationMessageHash.X,
pubkeyRegistrationMessageHash.Y
))) % BN254.FR_MODULUS;

// e(sigma + P * gamma, [-1]_2) = e(H(m) + [1]_1 * gamma, P')
require(BN254.pairing(
params.pubkeyRegistrationSignature.plus(params.pubkeyG1.scalar_mul(gamma)),
BN254.negGeneratorG2(),
messageHash.plus(BN254.generatorG1().scalar_mul(gamma)),
pubkeyRegistrationMessageHash.plus(BN254.generatorG1().scalar_mul(gamma)),
params.pubkeyG2
), "BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match");

Expand Down Expand Up @@ -215,19 +214,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
return (pubkey, pubkeyHash);
}

/**
* @notice Returns the message hash that an operator must sign to register their BLS public key.
* @param operator is the address of the operator registering their BLS public key
*/
function getMessageHash(address operator) public view returns (BN254.G1Point memory) {
return BN254.hashToG1(keccak256(abi.encodePacked(
operator,
address(this),
block.chainid,
"EigenLayer_BN254_Pubkey_Registration"
)));
}

/**
* @notice Returns the indices of the quorumApks index at `blockNumber` for the provided `quorumNumbers`
* @dev Returns the current indices if `blockNumber >= block.number`
Expand Down Expand Up @@ -294,6 +280,8 @@ contract BLSApkRegistry is BLSApkRegistryStorage {
return pubkeyHashToOperator[pubkeyHash];
}

/// @notice returns the ID used to identify the `operator` within this AVS
/// @dev Returns zero in the event that the `operator` has never registered for the AVS
function getOperatorId(address operator) public view returns (bytes32) {
return operatorToPubkeyHash[operator];
}
Expand Down
18 changes: 16 additions & 2 deletions src/RegistryCoordinator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo
/// @notice The EIP-712 typehash for the `DelegationApproval` struct used by the contract
bytes32 public constant OPERATOR_CHURN_APPROVAL_TYPEHASH =
keccak256("OperatorChurnApproval(bytes32 registeringOperatorId,OperatorKickParam[] operatorKickParams)OperatorKickParam(address operator,bytes32[] operatorIdsToSwap)");
/// @notice The EIP-712 typehash used for registering BLS public keys
bytes32 public constant PUBKEY_REGISTRATION_TYPEHASH = keccak256("BN254PubkeyRegistration(address operator)");
/// @notice The maximum value of a quorum bitmap
uint256 internal constant MAX_QUORUM_BITMAP = type(uint192).max;
/// @notice The basis point denominator
Expand Down Expand Up @@ -161,7 +163,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo
*/
bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender);
if (operatorId == 0) {
operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params);
operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender));
}

// Register the operator in each of the registry contracts
Expand Down Expand Up @@ -213,7 +215,7 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo
*/
bytes32 operatorId = blsApkRegistry.getOperatorId(msg.sender);
if (operatorId == 0) {
operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params);
operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender));
}
Comment on lines 216 to 219
Copy link
Collaborator

@wadealexc wadealexc Dec 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could consider tossing these lines into a helper, since we use them twice.

bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params);

I don't feel strongly about this though.


// Verify the churn approver's signature for the registering operator and kick params
Expand Down Expand Up @@ -760,4 +762,16 @@ contract RegistryCoordinator is EIP712, Initializable, IRegistryCoordinator, ISo
// calculate the digest hash
return _hashTypedDataV4(keccak256(abi.encode(OPERATOR_CHURN_APPROVAL_TYPEHASH, registeringOperatorId, operatorKickParams, salt, expiry)));
}

/**
* @notice Returns the message hash that an operator must sign to register their BLS public key.
* @param operator is the address of the operator registering their BLS public key
*/
function pubkeyRegistrationMessageHash(address operator) public view returns (BN254.G1Point memory) {
return BN254.hashToG1(
_hashTypedDataV4(
keccak256(abi.encode(PUBKEY_REGISTRATION_TYPEHASH, operator))
)
);
}
}
12 changes: 5 additions & 7 deletions src/interfaces/IBLSApkRegistry.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,12 @@ interface IBLSApkRegistry is IRegistry {
* @notice Called by the RegistryCoordinator register an operator as the owner of a BLS public key.
* @param operator is the operator for whom the key is being registered
* @param params contains the G1 & G2 public keys of the operator, and a signature proving their ownership
* @param pubkeyRegistrationMessageHash is a hash that the operator must sign to prove key ownership
*/
function registerBLSPublicKey(
address operator,
PubkeyRegistrationParams calldata params
PubkeyRegistrationParams calldata params,
BN254.G1Point calldata pubkeyRegistrationMessageHash
) external returns (bytes32 operatorId);

/**
Expand All @@ -111,12 +113,6 @@ interface IBLSApkRegistry is IRegistry {
*/
function getRegisteredPubkey(address operator) external view returns (BN254.G1Point memory, bytes32);

/**
* @notice Returns the message hash that an operator must sign to register their BLS public key.
* @param operator is the address of the operator registering their BLS public key
*/
function getMessageHash(address operator) external view returns (BN254.G1Point memory);

/// @notice Returns the current APK for the provided `quorumNumber `
function getApk(uint8 quorumNumber) external view returns (BN254.G1Point memory);

Expand All @@ -138,5 +134,7 @@ interface IBLSApkRegistry is IRegistry {
*/
function getApkHashAtBlockNumberAndIndex(uint8 quorumNumber, uint32 blockNumber, uint256 index) external view returns (bytes24);

/// @notice returns the ID used to identify the `operator` within this AVS.
/// @dev Returns zero in the event that the `operator` has never registered for the AVS
function getOperatorId(address operator) external view returns (bytes32);
}
7 changes: 7 additions & 0 deletions src/interfaces/IRegistryCoordinator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity =0.8.12;
import {IBLSApkRegistry} from "src/interfaces/IBLSApkRegistry.sol";
import {IStakeRegistry} from "src/interfaces/IStakeRegistry.sol";
import {IIndexRegistry} from "src/interfaces/IIndexRegistry.sol";
import {BN254} from "src/libraries/BN254.sol";

/**
* @title Interface for a contract that coordinates between various registries for an AVS.
Expand Down Expand Up @@ -134,4 +135,10 @@ interface IRegistryCoordinator {

/// @notice Returns the number of registries
function numRegistries() external view returns (uint256);

/**
* @notice Returns the message hash that an operator must sign to register their BLS public key.
* @param operator is the address of the operator registering their BLS public key
*/
function pubkeyRegistrationMessageHash(address operator) external view returns (BN254.G1Point memory);
}
4 changes: 2 additions & 2 deletions test/ffi/BLSPubKeyCompendiumFFI.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract BLSApkRegistryFFITests is G2Operations {
pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice);

vm.prank(address(registryCoordinator));
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, registryCoordinator.pubkeyRegistrationMessageHash(alice));

assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1),
"pubkey hash not stored correctly");
Expand All @@ -45,7 +45,7 @@ contract BLSApkRegistryFFITests is G2Operations {
}

function _signMessage(address signer) internal view returns(BN254.G1Point memory) {
BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer);
BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(signer);
return BN254.scalar_mul(messageHash, privKey);
}
}
6 changes: 6 additions & 0 deletions test/mocks/RegistryCoordinatorMock.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,10 @@ contract RegistryCoordinatorMock is IRegistryCoordinator {
function registerOperator(bytes memory quorumNumbers, bytes calldata) external {}

function deregisterOperator(bytes calldata quorumNumbers, bytes calldata) external {}

function pubkeyRegistrationMessageHash(address operator) public view returns (BN254.G1Point memory) {
return BN254.hashToG1(
keccak256(abi.encode(operator))
);
}
}
17 changes: 11 additions & 6 deletions test/unit/BLSApkRegistryUnit.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ contract BLSApkRegistryUnitTests is Test {
// TODO: better organize / integrate tests migrated from `BLSPublicKeyCompendium` unit tests
function testRegisterBLSPublicKey() public {
pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(alice);
BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice);
vm.prank(address(registryCoordinator));
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash);

assertEq(blsApkRegistry.operatorToPubkeyHash(alice), BN254.hashG1Point(pubkeyRegistrationParams.pubkeyG1),
"pubkey hash not stored correctly");
Expand All @@ -307,38 +308,42 @@ contract BLSApkRegistryUnitTests is Test {

pubkeyRegistrationParams.pubkeyG1 = badPubkeyG1;

BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice);
vm.prank(address(registryCoordinator));
vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"));
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash);
}

function testRegisterBLSPublicKey_BadSig_Reverts() public {
pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // sign with wrong private key

BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice);
vm.prank(address(registryCoordinator));
vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: either the G1 signature is wrong, or G1 and G2 private key do not match"));
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash);
}

function testRegisterBLSPublicKey_OpRegistered_Reverts() public {
testRegisterBLSPublicKey(); // register alice

BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(alice);
vm.prank(address(registryCoordinator));
vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: operator already registered pubkey"));
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(alice, pubkeyRegistrationParams, messageHash);
}

function testRegisterBLSPublicKey_PkRegistered_Reverts() public {
testRegisterBLSPublicKey();
pubkeyRegistrationParams.pubkeyRegistrationSignature = _signMessage(bob); // same private key different operator

BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(bob);
vm.prank(address(registryCoordinator));
vm.expectRevert(bytes("BLSApkRegistry.registerBLSPublicKey: public key already registered"));
blsApkRegistry.registerBLSPublicKey(bob, pubkeyRegistrationParams);
blsApkRegistry.registerBLSPublicKey(bob, pubkeyRegistrationParams, messageHash);
}

function _signMessage(address signer) internal view returns(BN254.G1Point memory) {
BN254.G1Point memory messageHash = blsApkRegistry.getMessageHash(signer);
BN254.G1Point memory messageHash = registryCoordinator.pubkeyRegistrationMessageHash(signer);
return BN254.scalar_mul(messageHash, privKey);
}

Expand Down