From 1e27ee5ed84a6662ba672d83652c808d8f38b560 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 12:53:40 -0800 Subject: [PATCH 1/8] fix: actually use the `operator` input to `_getOrCreateOperatorId` also reduce memory copying operations (I think, at least) in the `registerOperator` function specifically, only copy the `numOperatorsPerQuorum` returned by the internal `_registerOperator` function --- src/RegistryCoordinator.sol | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 3a84a0f9..d4e2f930 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -178,25 +178,23 @@ contract RegistryCoordinator is bytes32 operatorId = _getOrCreateOperatorId(msg.sender, params); // Register the operator in each of the registry contracts - RegisterResults memory results = _registerOperator({ + uint32[] memory numOperatorsPerQuorum = _registerOperator({ operator: msg.sender, operatorId: operatorId, quorumNumbers: quorumNumbers, socket: socket, operatorSignature: operatorSignature - }); + }).numOperatorsPerQuorum; for (uint256 i = 0; i < quorumNumbers.length; i++) { uint8 quorumNumber = uint8(quorumNumbers[i]); - - OperatorSetParam memory operatorSetParams = _quorumParams[quorumNumber]; - + /** * The new operator count for each quorum may not exceed the configured maximum * If it does, use `registerOperatorWithChurn` instead. */ require( - results.numOperatorsPerQuorum[i] <= operatorSetParams.maxOperatorCount, + numOperatorsPerQuorum[i] <= _quorumParams[quorumNumber].maxOperatorCount, "RegistryCoordinator.registerOperator: operator count exceeds maximum" ); } @@ -517,9 +515,9 @@ contract RegistryCoordinator is address operator, IBLSApkRegistry.PubkeyRegistrationParams calldata params ) internal returns (bytes32 operatorId) { - operatorId = blsApkRegistry.getOperatorId(msg.sender); + operatorId = blsApkRegistry.getOperatorId(operator); if (operatorId == 0) { - operatorId = blsApkRegistry.registerBLSPublicKey(msg.sender, params, pubkeyRegistrationMessageHash(msg.sender)); + operatorId = blsApkRegistry.registerBLSPublicKey(operator, params, pubkeyRegistrationMessageHash(operator)); } return operatorId; } From e729464988f84793cddb32cbf3f88e18dfe6f9d5 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:11:25 -0800 Subject: [PATCH 2/8] feat: add optimizer runs count to foundry config --- foundry.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/foundry.toml b/foundry.toml index 6a48adda..5c1f5469 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,4 +6,7 @@ fs_permissions = [{ access = "read-write", path = "./" }] ffi = true +# The number of optimizer runs +optimizer_runs = 200 + # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options From 43ed47465190675425dc3268ad98532aba308d52 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:18:12 -0800 Subject: [PATCH 3/8] chore: remove redundant check and return data The `operatorId` in this check is already fetched from the `blsApkRegistry` With the other changes, this return data is no longer necessary at all --- src/BLSApkRegistry.sol | 4 +--- src/RegistryCoordinator.sol | 3 +-- src/interfaces/IBLSApkRegistry.sol | 2 +- test/unit/BLSApkRegistryUnit.t.sol | 8 +++----- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index 71e3a77b..fe16c7fa 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -32,7 +32,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { * @notice Registers the `operator`'s pubkey for the specified `quorumNumbers`. * @param operator The address of the operator to register. * @param quorumNumbers The quorum numbers the operator is registering for, where each byte is an 8 bit integer quorumNumber. - * @return pubkeyHash of the operator's pubkey * @dev access restricted to the RegistryCoordinator * @dev Preconditions (these are assumed, not validated in this contract): * 1) `quorumNumbers` has no duplicates @@ -43,7 +42,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { function registerOperator( address operator, bytes memory quorumNumbers - ) public virtual onlyRegistryCoordinator returns (bytes32) { + ) public virtual onlyRegistryCoordinator { // Get the operator's pubkey. Reverts if they have not registered a key (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = getRegisteredPubkey(operator); @@ -52,7 +51,6 @@ contract BLSApkRegistry is BLSApkRegistryStorage { // Return pubkeyHash, which will become the operator's unique id emit OperatorAddedToQuorums(operator, quorumNumbers); - return pubkeyHash; } /** diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index d4e2f930..810ef6cb 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -498,8 +498,7 @@ contract RegistryCoordinator is /** * Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry */ - bytes32 registeredId = blsApkRegistry.registerOperator(operator, quorumNumbers); - require(registeredId == operatorId, "RegistryCoordinator._registerOperator: operatorId mismatch"); + blsApkRegistry.registerOperator(operator, quorumNumbers); (uint96[] memory operatorStakes, uint96[] memory totalStakes) = stakeRegistry.registerOperator(operator, operatorId, quorumNumbers); uint32[] memory numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); diff --git a/src/interfaces/IBLSApkRegistry.sol b/src/interfaces/IBLSApkRegistry.sol index 916320ec..62a3e6b1 100644 --- a/src/interfaces/IBLSApkRegistry.sol +++ b/src/interfaces/IBLSApkRegistry.sol @@ -60,7 +60,7 @@ interface IBLSApkRegistry is IRegistry { * 3) `quorumNumbers` is ordered in ascending order * 4) the operator is not already registered */ - function registerOperator(address operator, bytes calldata quorumNumbers) external returns(bytes32); + function registerOperator(address operator, bytes calldata quorumNumbers) external; /** * @notice Deregisters the `operator`'s pubkey for the specified `quorumNumbers`. diff --git a/test/unit/BLSApkRegistryUnit.t.sol b/test/unit/BLSApkRegistryUnit.t.sol index 0d8ed07a..81c02654 100644 --- a/test/unit/BLSApkRegistryUnit.t.sol +++ b/test/unit/BLSApkRegistryUnit.t.sol @@ -92,13 +92,11 @@ contract BLSApkRegistryUnitTests is Test { bytes memory quorumNumbers = new bytes(1); quorumNumbers[0] = bytes1(defaultQuorumNumber); - cheats.startPrank(address(registryCoordinator)); - bytes32 registeredpkHash = blsApkRegistry.registerOperator(operator, quorumNumbers); - cheats.stopPrank(); - + cheats.prank(address(registryCoordinator)); + blsApkRegistry.registerOperator(operator, quorumNumbers); + (, bytes32 registeredpkHash) = blsApkRegistry.getRegisteredPubkey(operator); require(registeredpkHash == pkHash, "registeredpkHash not set correctly"); - emit log("ehey"); return pkHash; } From b205bded62c27e6b3b6f6f0cafbfa8a182f40ee6 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:20:35 -0800 Subject: [PATCH 4/8] chore: fewer memory operations(?) I believe this change cuts down on the memory copying being done here --- src/RegistryCoordinator.sol | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 810ef6cb..041e7586 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -458,7 +458,7 @@ contract RegistryCoordinator is bytes calldata quorumNumbers, string memory socket, SignatureWithSaltAndExpiry memory operatorSignature - ) internal virtual returns (RegisterResults memory) { + ) internal virtual returns (RegisterResults memory results) { /** * Get bitmap of quorums to register for and operator's current bitmap. Validate that: * - we're trying to register for at least 1 quorum @@ -499,15 +499,11 @@ contract RegistryCoordinator is * Register the operator with the BLSApkRegistry, StakeRegistry, and IndexRegistry */ blsApkRegistry.registerOperator(operator, quorumNumbers); - (uint96[] memory operatorStakes, uint96[] memory totalStakes) = + (results.operatorStakes, results.totalStakes) = stakeRegistry.registerOperator(operator, operatorId, quorumNumbers); - uint32[] memory numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); + results.numOperatorsPerQuorum = indexRegistry.registerOperator(operatorId, quorumNumbers); - return RegisterResults({ - numOperatorsPerQuorum: numOperatorsPerQuorum, - operatorStakes: operatorStakes, - totalStakes: totalStakes - }); + return results; } function _getOrCreateOperatorId( From c2f5719ea565409d273194a339efde4ad55b9641 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:25:26 -0800 Subject: [PATCH 5/8] fix: reduce optimizer runs to meet contract code size limits Likely not the ideal fix, but it gets the job done for the moment, at least. --- foundry.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/foundry.toml b/foundry.toml index 5c1f5469..f8f557c1 100644 --- a/foundry.toml +++ b/foundry.toml @@ -7,6 +7,6 @@ fs_permissions = [{ access = "read-write", path = "./" }] ffi = true # The number of optimizer runs -optimizer_runs = 200 +optimizer_runs = 100 # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options From caf9960d9808484afb8ffa7682d996f5e89f1dd1 Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:39:13 -0800 Subject: [PATCH 6/8] chore: rename file to reflect it only being used in tests `RegistryCoordinatorHarness.sol` -> `RegistryCoordinatorHarness.t.sol` --- src/RegistryCoordinator.sol | 2 +- ...yCoordinatorHarness.sol => RegistryCoordinatorHarness.t.sol} | 0 test/unit/StakeRegistryUnit.t.sol | 2 +- test/utils/MockAVSDeployer.sol | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename test/harnesses/{RegistryCoordinatorHarness.sol => RegistryCoordinatorHarness.t.sol} (100%) diff --git a/src/RegistryCoordinator.sol b/src/RegistryCoordinator.sol index 041e7586..b86f9dc6 100644 --- a/src/RegistryCoordinator.sol +++ b/src/RegistryCoordinator.sol @@ -432,7 +432,7 @@ contract RegistryCoordinator is /** * @notice Sets the metadata URI for the AVS * @param _metadataURI is the metadata URI for the AVS - * @dev only callable by the service manager owner + * @dev only callable by the owner */ function setMetadataURI(string memory _metadataURI) external onlyOwner { delegationManager.updateAVSMetadataURI(_metadataURI); diff --git a/test/harnesses/RegistryCoordinatorHarness.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol similarity index 100% rename from test/harnesses/RegistryCoordinatorHarness.sol rename to test/harnesses/RegistryCoordinatorHarness.t.sol diff --git a/test/unit/StakeRegistryUnit.t.sol b/test/unit/StakeRegistryUnit.t.sol index c0ae719b..766e5fe3 100644 --- a/test/unit/StakeRegistryUnit.t.sol +++ b/test/unit/StakeRegistryUnit.t.sol @@ -25,7 +25,7 @@ import {SlasherMock} from "eigenlayer-contracts/src/test/mocks/SlasherMock.sol"; import {StakeRegistryHarness} from "test/harnesses/StakeRegistryHarness.sol"; import {StakeRegistry} from "src/StakeRegistry.sol"; -import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.sol"; +import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.t.sol"; import "forge-std/Test.sol"; diff --git a/test/utils/MockAVSDeployer.sol b/test/utils/MockAVSDeployer.sol index c7f5a757..118cbd03 100644 --- a/test/utils/MockAVSDeployer.sol +++ b/test/utils/MockAVSDeployer.sol @@ -14,7 +14,7 @@ import {BN254} from "src/libraries/BN254.sol"; import {OperatorStateRetriever} from "src/OperatorStateRetriever.sol"; import {RegistryCoordinator} from "src/RegistryCoordinator.sol"; -import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.sol"; +import {RegistryCoordinatorHarness} from "test/harnesses/RegistryCoordinatorHarness.t.sol"; import {BLSApkRegistry} from "src/BLSApkRegistry.sol"; import {StakeRegistry} from "src/StakeRegistry.sol"; import {IndexRegistry} from "src/IndexRegistry.sol"; From b659b2f74d661bbffd94bab8f7558e69f0e68b6c Mon Sep 17 00:00:00 2001 From: ChaoticWalrus <93558947+ChaoticWalrus@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:40:22 -0800 Subject: [PATCH 7/8] chore: delete unused (memory) variable --- src/BLSApkRegistry.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/BLSApkRegistry.sol b/src/BLSApkRegistry.sol index fe16c7fa..a732a44a 100644 --- a/src/BLSApkRegistry.sol +++ b/src/BLSApkRegistry.sol @@ -44,7 +44,7 @@ contract BLSApkRegistry is BLSApkRegistryStorage { bytes memory quorumNumbers ) public virtual onlyRegistryCoordinator { // Get the operator's pubkey. Reverts if they have not registered a key - (BN254.G1Point memory pubkey, bytes32 pubkeyHash) = getRegisteredPubkey(operator); + (BN254.G1Point memory pubkey, ) = getRegisteredPubkey(operator); // Update each quorum's aggregate pubkey _processQuorumApkUpdate(quorumNumbers, pubkey); From d03039fbaf6d41330b29af0b0c26ddf9ba035a6e Mon Sep 17 00:00:00 2001 From: wadealexc Date: Wed, 13 Dec 2023 21:55:09 +0000 Subject: [PATCH 8/8] fix: have the harness import Test so it gets ignored in build sizes --- test/harnesses/RegistryCoordinatorHarness.t.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/harnesses/RegistryCoordinatorHarness.t.sol b/test/harnesses/RegistryCoordinatorHarness.t.sol index 246ea705..f3ce41a7 100644 --- a/test/harnesses/RegistryCoordinatorHarness.t.sol +++ b/test/harnesses/RegistryCoordinatorHarness.t.sol @@ -3,8 +3,10 @@ pragma solidity =0.8.12; import "src/RegistryCoordinator.sol"; +import "forge-std/Test.sol"; + // wrapper around the RegistryCoordinator contract that exposes the internal functions for unit testing. -contract RegistryCoordinatorHarness is RegistryCoordinator { +contract RegistryCoordinatorHarness is RegistryCoordinator, Test { constructor( IDelegationManager _delegationManager, ISlasher _slasher,