From 64e94236e6cecff56253355fea4e19a70d8f420c Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:10:22 +0400 Subject: [PATCH 01/15] fix:statemind- Open access SharedVaults in self-register middlewares --- .../SelfRegisterEd25519Middleware.sol | 14 +++++++++----- .../SelfRegisterMiddleware.sol | 14 +++++++++----- .../ForcePauseSelfRegisterOperators.sol | 19 +++++++++++++++++++ .../IForcePauseSelfRegisterOperators.sol | 16 ++++++++++++++++ 4 files changed, 53 insertions(+), 10 deletions(-) diff --git a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol index 0bda53b..8f64090 100644 --- a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol +++ b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol @@ -5,7 +5,7 @@ import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; import {SharedVaults} from "../../extensions/SharedVaults.sol"; import {SelfRegisterOperators} from "../../extensions/operators/SelfRegisterOperators.sol"; -import {NoAccessManager} from "../../extensions/managers/access/NoAccessManager.sol"; +import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; import {TimestampCapture} from "../../extensions/managers/capture-timestamps/TimestampCapture.sol"; import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStakePower.sol"; import {KeyManager256} from "../../extensions/managers/keys/KeyManager256.sol"; @@ -16,7 +16,7 @@ contract SelfRegisterEd25519Middleware is SelfRegisterOperators, KeyManager256, EdDSASig, - NoAccessManager, + OwnableAccessManager, TimestampCapture, EqualStakePower { @@ -28,6 +28,7 @@ contract SelfRegisterEd25519Middleware is * @param operatorRegistry The address of the operator registry * @param operatorNetOptin The address of the operator network opt-in service * @param reader The address of the reader contract used for delegatecall + * @param owner The address of the owner */ constructor( address network, @@ -35,9 +36,10 @@ contract SelfRegisterEd25519Middleware is address vaultRegistry, address operatorRegistry, address operatorNetOptin, - address reader + address reader, + address owner ) { - initialize(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptin, reader); + initialize(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptin, reader, owner); } function initialize( @@ -46,9 +48,11 @@ contract SelfRegisterEd25519Middleware is address vaultRegistry, address operatorRegistry, address operatorNetOptin, - address reader + address reader, + address owner ) internal initializer { __BaseMiddleware_init(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptin, reader); __SelfRegisterOperators_init("SelfRegisterEd25519Middleware"); + __OwnableAccessManager_init(owner); } } diff --git a/src/examples/self-register-network/SelfRegisterMiddleware.sol b/src/examples/self-register-network/SelfRegisterMiddleware.sol index a10bea4..5617c8e 100644 --- a/src/examples/self-register-network/SelfRegisterMiddleware.sol +++ b/src/examples/self-register-network/SelfRegisterMiddleware.sol @@ -6,7 +6,7 @@ import {SharedVaults} from "../../extensions/SharedVaults.sol"; import {SelfRegisterOperators} from "../../extensions/operators/SelfRegisterOperators.sol"; import {ECDSASig} from "../../extensions/managers/sigs/ECDSASig.sol"; -import {NoAccessManager} from "../../extensions/managers/access/NoAccessManager.sol"; +import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; import {TimestampCapture} from "../../extensions/managers/capture-timestamps/TimestampCapture.sol"; import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStakePower.sol"; import {KeyManager256} from "../../extensions/managers/keys/KeyManager256.sol"; @@ -16,7 +16,7 @@ contract SelfRegisterMiddleware is SelfRegisterOperators, KeyManager256, ECDSASig, - NoAccessManager, + OwnableAccessManager, TimestampCapture, EqualStakePower { @@ -28,6 +28,7 @@ contract SelfRegisterMiddleware is * @param operatorRegistry The address of the operator registry * @param operatorNetOptin The address of the operator network opt-in service * @param reader The address of the reader contract used for delegatecall + * @param owner The address of the owner */ constructor( address network, @@ -35,9 +36,10 @@ contract SelfRegisterMiddleware is address vaultRegistry, address operatorRegistry, address operatorNetOptin, - address reader + address reader, + address owner ) { - initialize(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptin, reader); + initialize(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptin, reader, owner); } function initialize( @@ -46,9 +48,11 @@ contract SelfRegisterMiddleware is address vaultRegistry, address operatorRegistry, address operatorNetOptIn, - address reader + address reader, + address owner ) internal initializer { __BaseMiddleware_init(network, slashingWindow, vaultRegistry, operatorRegistry, operatorNetOptIn, reader); __SelfRegisterOperators_init("SelfRegisterMiddleware"); + __OwnableAccessManager_init(owner); } } diff --git a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol index 814a716..36315bf 100644 --- a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol +++ b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol @@ -53,6 +53,17 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor _unpauseOperator(operator); } + /** + * @inheritdoc IForcePauseSelfRegisterOperators + */ + function forceUnregisterOperator( + address operator + ) public checkAccess { + _beforeUnregisterOperator(operator); + _unregisterOperator(operator); + } + + /** * @inheritdoc IForcePauseSelfRegisterOperators */ @@ -73,6 +84,14 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor _unpauseOperatorVault(operator, vault); } + /** + * @inheritdoc IForcePauseSelfRegisterOperators + */ + function forceUnregisterOperatorVault(address operator, address vault) public checkAccess { + _beforeUnregisterOperatorVault(operator, vault); + _unregisterOperatorVault(operator, vault); + } + /** * @notice Override to prevent unpausing force-paused operators * @param operator The operator address diff --git a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol index 1a09b5f..09b1fe1 100644 --- a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol +++ b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol @@ -27,6 +27,14 @@ interface IForcePauseSelfRegisterOperators is ISelfRegisterOperators { address operator ) external; + /** + * @notice Forces an operator to be unregistered + * @param operator The address of the operator to unregister + */ + function forceUnregisterOperator( + address operator + ) external; + /** * @notice Forces a specific operator-vault pair to be paused * @param operator The address of the operator @@ -40,4 +48,12 @@ interface IForcePauseSelfRegisterOperators is ISelfRegisterOperators { * @param vault The address of the vault */ function forceUnpauseOperatorVault(address operator, address vault) external; + + /** + /** + * @notice Forces a specific operator-vault pair to be unregistered + * @param operator The address of the operator + * @param vault The address of the vault + */ + function forceUnregisterOperatorVault(address operator, address vault) external; } From 7a70b59547c338dce74570a3f97566c2f11f07dc Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:10:40 +0400 Subject: [PATCH 02/15] fix: tests --- test/SigTests.t.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/SigTests.t.sol b/test/SigTests.t.sol index 072b867..2a4764c 100644 --- a/test/SigTests.t.sol +++ b/test/SigTests.t.sol @@ -51,7 +51,8 @@ contract SigTests is POCBaseTest { address(vaultFactory), address(operatorRegistry), address(operatorNetworkOptInService), - readHelper + readHelper, + alice ); ed25519Middleware = new SelfRegisterEd25519Middleware( @@ -60,7 +61,8 @@ contract SigTests is POCBaseTest { address(vaultFactory), address(operatorRegistry), address(operatorNetworkOptInService), - readHelper + readHelper, + alice ); _registerNetwork(address(0x123), address(middleware)); From 1ff339b4966799b74c8a3d18b770013ff0df828d Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:14:55 +0400 Subject: [PATCH 03/15] fix: statemind - Empty key is always considered to be valid --- src/extensions/operators/SelfRegisterOperators.sol | 2 +- src/managers/extendable/KeyManager.sol | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extensions/operators/SelfRegisterOperators.sol b/src/extensions/operators/SelfRegisterOperators.sol index 411b7ab..b1eb424 100644 --- a/src/extensions/operators/SelfRegisterOperators.sol +++ b/src/extensions/operators/SelfRegisterOperators.sol @@ -298,7 +298,7 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg /** * @notice Verifies a key signature * @param operator The address of the operator - * @param key The public key to verify + * @param key The public key to verify (zero key is allowed for deletion) * @param signature The signature to verify */ function _verifyKey(address operator, bytes memory key, bytes memory signature) internal { diff --git a/src/managers/extendable/KeyManager.sol b/src/managers/extendable/KeyManager.sol index f71b305..86dd3fa 100644 --- a/src/managers/extendable/KeyManager.sol +++ b/src/managers/extendable/KeyManager.sol @@ -12,7 +12,7 @@ abstract contract KeyManager is SlashingWindowStorage, CaptureTimestampManager { /** * @notice Updates the key associated with an operator * @param operator The address of the operator - * @param key The key to update + * @param key The key to update, or empty bytes to delete the key */ function _updateKey(address operator, bytes memory key) internal virtual; From 7b4545df7c7cba75ec139d466d500e9f02390306 Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:15:39 +0400 Subject: [PATCH 04/15] fix: statemind - Usage of EIP712 version number as decimal --- src/extensions/operators/SelfRegisterOperators.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/extensions/operators/SelfRegisterOperators.sol b/src/extensions/operators/SelfRegisterOperators.sol index b1eb424..3b1f40a 100644 --- a/src/extensions/operators/SelfRegisterOperators.sol +++ b/src/extensions/operators/SelfRegisterOperators.sol @@ -61,7 +61,7 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg function __SelfRegisterOperators_init( string memory name ) internal onlyInitializing { - __EIP712_init(name, "1.0"); + __EIP712_init(name, "1"); } /** From 09812fe1329b48daae218e30f2846e44cbd9f461 Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:22:18 +0400 Subject: [PATCH 05/15] fix: statemind-Incorrect address derivation through public key leads to registration inoperability --- src/extensions/managers/sigs/ECDSASig.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/extensions/managers/sigs/ECDSASig.sol b/src/extensions/managers/sigs/ECDSASig.sol index 6efc7c9..66940fe 100644 --- a/src/extensions/managers/sigs/ECDSASig.sol +++ b/src/extensions/managers/sigs/ECDSASig.sol @@ -8,7 +8,8 @@ import {IECDSASig} from "../../../interfaces/extensions/managers/sigs/IECDSASig. /** * @title ECDSASig * @notice Contract for verifying ECDSA signatures against operator keys - * @dev Implements SigManager interface using OpenZeppelin's ECDSA library + * @dev Implements SigManager interface using OpenZeppelin's ECDSA library. Instead of using public keys directly, + * this implementation uses Ethereum addresses derived from the public keys as operator keys. */ abstract contract ECDSASig is SigManager, IECDSASig { uint64 public constant ECDSASig_VERSION = 1; @@ -18,7 +19,7 @@ abstract contract ECDSASig is SigManager, IECDSASig { /** * @notice Verifies that a signature was created by the owner of a key * @param operator The address of the operator that owns the key - * @param key_ The public key to verify against, encoded as bytes + * @param key_ The address derived from the public key, encoded as bytes * @param signature The ECDSA signature to verify * @return True if the signature was created by the key owner, false otherwise * @dev The key is expected to be a bytes32 that can be converted to an Ethereum address From 9ac3df098deb5235202651ea37060c61410c77e1 Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:32:17 +0400 Subject: [PATCH 06/15] fix: statemind-The SelfRegisterOperators contract functionality breaks the forced pause --- .../operators/ForcePauseSelfRegisterOperators.sol | 9 ++++++--- .../operators/IForcePauseSelfRegisterOperators.sol | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol index 36315bf..8e01056 100644 --- a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol +++ b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol @@ -38,7 +38,9 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePaused[operator] = true; _beforePauseOperator(operator); - _pauseOperator(operator); + if (_operatorWasActiveAt(_now(), operator)) { + _pauseOperator(operator); + } } /** @@ -63,7 +65,6 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor _unregisterOperator(operator); } - /** * @inheritdoc IForcePauseSelfRegisterOperators */ @@ -71,7 +72,9 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePausedVault[operator][vault] = true; _beforePauseOperatorVault(operator, vault); - _pauseOperatorVault(operator, vault); + if (_operatorVaultWasActiveAt(_now(), operator, vault)) { + _pauseOperatorVault(operator, vault); + } } /** diff --git a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol index 09b1fe1..d8c7a00 100644 --- a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol +++ b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol @@ -50,7 +50,7 @@ interface IForcePauseSelfRegisterOperators is ISelfRegisterOperators { function forceUnpauseOperatorVault(address operator, address vault) external; /** - /** + * /** * @notice Forces a specific operator-vault pair to be unregistered * @param operator The address of the operator * @param vault The address of the vault From 4e960be131b4e31db9f211757cab9ea79ea4d26b Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:41:58 +0400 Subject: [PATCH 07/15] fix: statemind - Open access variables may lead to DOS --- .../SelfRegisterEd25519Middleware.sol | 24 ++++++++++++++++ .../SelfRegisterMiddleware.sol | 20 +++++++++++++ .../SelfRegisterSqrtTaskMiddleware.sol | 28 +++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol index 8f64090..be2dfdb 100644 --- a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol +++ b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol @@ -20,6 +20,12 @@ contract SelfRegisterEd25519Middleware is TimestampCapture, EqualStakePower { + error TooManyOperators(); + error TooManyOperatorVaults(); + + uint256 public constant MAX_OPERATORS = 100; + uint256 public constant MAX_OPERATOR_VAULTS = 40; + /** * @notice Constructor for initializing the SelfRegisterEd25519Middleware contract * @param network The address of the network @@ -55,4 +61,22 @@ contract SelfRegisterEd25519Middleware is __SelfRegisterOperators_init("SelfRegisterEd25519Middleware"); __OwnableAccessManager_init(owner); } + + /// @notice Prevents DOS by limiting total number of operators that can be registered + /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators + function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { + super._beforeRegisterOperator(operator, key, vault); + if (_operatorsLength() >= MAX_OPERATORS) { + revert TooManyOperators(); + } + } + + /// @notice Prevents DOS by limiting number of vaults per operator + /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults + function _beforeRegisterOperatorVault(address operator, address vault) internal virtual override { + super._beforeRegisterOperatorVault(operator, vault); + if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { + revert TooManyOperatorVaults(); + } + } } diff --git a/src/examples/self-register-network/SelfRegisterMiddleware.sol b/src/examples/self-register-network/SelfRegisterMiddleware.sol index 5617c8e..0bdde2c 100644 --- a/src/examples/self-register-network/SelfRegisterMiddleware.sol +++ b/src/examples/self-register-network/SelfRegisterMiddleware.sol @@ -20,6 +20,12 @@ contract SelfRegisterMiddleware is TimestampCapture, EqualStakePower { + error TooManyOperators(); + error TooManyOperatorVaults(); + + uint256 public constant MAX_OPERATORS = 100; + uint256 public constant MAX_OPERATOR_VAULTS = 40; + /** * @notice Constructor for initializing the SelfRegisterMiddleware contract * @param network The address of the network @@ -55,4 +61,18 @@ contract SelfRegisterMiddleware is __SelfRegisterOperators_init("SelfRegisterMiddleware"); __OwnableAccessManager_init(owner); } + + function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { + super._beforeRegisterOperator(operator, key, vault); + if (_operatorsLength() >= MAX_OPERATORS) { + revert TooManyOperators(); + } + } + + function _beforeRegisterOperatorVault(address operator, address vault) internal virtual override { + super._beforeRegisterOperatorVault(operator, vault); + if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { + revert TooManyOperatorVaults(); + } + } } diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index bbebf4a..50c45ca 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -35,6 +35,9 @@ contract SelfRegisterSqrtTaskMiddleware is error InvalidHints(); error TaskCompleted(); + error TooManyOperators(); + error TooManyOperatorVaults(); + error TooManySharedVaults(); event CreateTask(uint256 indexed taskIndex); event CompleteTask(uint256 indexed taskIndex, bool isValidAnswer); @@ -48,6 +51,10 @@ contract SelfRegisterSqrtTaskMiddleware is bytes32 private constant COMPLETE_TASK_TYPEHASH = keccak256("CompleteTask(uint256 taskIndex,uint256 answer)"); + uint256 public constant MAX_OPERATORS = 100; + uint256 public constant MAX_OPERATOR_VAULTS = 40; + uint256 public constant MAX_SHARED_VAULTS = 60; + Task[] public tasks; constructor( @@ -192,13 +199,34 @@ contract SelfRegisterSqrtTaskMiddleware is _slashVault(epochStart, vault, subnetwork, operator, amount, hints); } + /// @notice Prevents DOS by limiting total number of shared vaults that can be registered + /// @dev MAX_SHARED_VAULTS constant prevents unbounded iteration when looping through shared vaults function _beforeRegisterSharedVault( address sharedVault ) internal override { + super._beforeRegisterSharedVault(sharedVault); + if (_sharedVaultsLength() >= MAX_SHARED_VAULTS) { + revert TooManySharedVaults(); + } IBaseDelegator(IVault(sharedVault).delegator()).setMaxNetworkLimit(DEFAULT_SUBNETWORK, type(uint256).max); } + /// @notice Prevents DOS by limiting number of vaults per operator + /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults function _beforeRegisterOperatorVault(address operator, address vault) internal override { + super._beforeRegisterOperatorVault(operator, vault); + if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { + revert TooManyOperatorVaults(); + } IBaseDelegator(IVault(vault).delegator()).setMaxNetworkLimit(DEFAULT_SUBNETWORK, type(uint256).max); } + + /// @notice Prevents DOS by limiting total number of operators that can be registered + /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators + function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { + super._beforeRegisterOperator(operator, key, vault); + if (_operatorsLength() >= MAX_OPERATORS) { + revert TooManyOperators(); + } + } } From 84e1faee35b3f238061e4464915686e7d92b2b77 Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 17:50:24 +0400 Subject: [PATCH 08/15] fix: lint --- .../sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index 50c45ca..4817ee4 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -200,7 +200,7 @@ contract SelfRegisterSqrtTaskMiddleware is } /// @notice Prevents DOS by limiting total number of shared vaults that can be registered - /// @dev MAX_SHARED_VAULTS constant prevents unbounded iteration when looping through shared vaults + /// @dev MAX_SHARED_VAULTS constant prevents unbounded iteration when looping through shared vaults function _beforeRegisterSharedVault( address sharedVault ) internal override { From 4a97095328f22fbc96f5dafcaf2dd2856fb940ec Mon Sep 17 00:00:00 2001 From: alrxy Date: Mon, 16 Dec 2024 18:00:57 +0400 Subject: [PATCH 09/15] fix: rm docstrings --- .../extensions/operators/IForcePauseSelfRegisterOperators.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol index d8c7a00..1d864b9 100644 --- a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol +++ b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol @@ -50,7 +50,6 @@ interface IForcePauseSelfRegisterOperators is ISelfRegisterOperators { function forceUnpauseOperatorVault(address operator, address vault) external; /** - * /** * @notice Forces a specific operator-vault pair to be unregistered * @param operator The address of the operator * @param vault The address of the vault From a387792b62485510f81396e0bb867e75df8b39ed Mon Sep 17 00:00:00 2001 From: alrxy Date: Tue, 17 Dec 2024 18:01:25 +0400 Subject: [PATCH 10/15] fix: statemind - Incorrect address derivation through public key leads to registration inoperability --- .../SelfRegisterMiddleware.sol | 8 +- .../SelfRegisterSqrtTaskMiddleware.sol | 10 +- .../managers/keys/KeyManagerAddress.sol | 118 ++++++++++++++++++ src/extensions/managers/sigs/ECDSASig.sol | 5 +- src/libraries/PauseableEnumerableSet.sol | 19 +++ test/SigTests.t.sol | 24 ++-- 6 files changed, 161 insertions(+), 23 deletions(-) create mode 100644 src/extensions/managers/keys/KeyManagerAddress.sol diff --git a/src/examples/self-register-network/SelfRegisterMiddleware.sol b/src/examples/self-register-network/SelfRegisterMiddleware.sol index 0bdde2c..9a00f6c 100644 --- a/src/examples/self-register-network/SelfRegisterMiddleware.sol +++ b/src/examples/self-register-network/SelfRegisterMiddleware.sol @@ -9,12 +9,12 @@ import {ECDSASig} from "../../extensions/managers/sigs/ECDSASig.sol"; import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; import {TimestampCapture} from "../../extensions/managers/capture-timestamps/TimestampCapture.sol"; import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStakePower.sol"; -import {KeyManager256} from "../../extensions/managers/keys/KeyManager256.sol"; +import {KeyManagerAddress} from "../../extensions/managers/keys/KeyManagerAddress.sol"; contract SelfRegisterMiddleware is SharedVaults, SelfRegisterOperators, - KeyManager256, + KeyManagerAddress, ECDSASig, OwnableAccessManager, TimestampCapture, @@ -62,6 +62,8 @@ contract SelfRegisterMiddleware is __OwnableAccessManager_init(owner); } + /// @notice Prevents DOS by limiting total number of operators that can be registered + /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { super._beforeRegisterOperator(operator, key, vault); if (_operatorsLength() >= MAX_OPERATORS) { @@ -69,6 +71,8 @@ contract SelfRegisterMiddleware is } } + /// @notice Prevents DOS by limiting number of vaults per operator + /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults function _beforeRegisterOperatorVault(address operator, address vault) internal virtual override { super._beforeRegisterOperatorVault(operator, vault); if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index 4817ee4..8f2b6fe 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -17,7 +17,7 @@ import {SelfRegisterOperators} from "../../extensions/operators/SelfRegisterOper import {ECDSASig} from "../../extensions/managers/sigs/ECDSASig.sol"; import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; -import {KeyManager256} from "../../extensions/managers/keys/KeyManager256.sol"; +import {KeyManagerAddress} from "../../extensions/managers/keys/KeyManagerAddress.sol"; import {TimestampCapture} from "../../extensions/managers/capture-timestamps/TimestampCapture.sol"; import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStakePower.sol"; @@ -25,7 +25,7 @@ contract SelfRegisterSqrtTaskMiddleware is SharedVaults, SelfRegisterOperators, ECDSASig, - KeyManager256, + KeyManagerAddress, OwnableAccessManager, TimestampCapture, EqualStakePower @@ -39,7 +39,7 @@ contract SelfRegisterSqrtTaskMiddleware is error TooManyOperatorVaults(); error TooManySharedVaults(); - event CreateTask(uint256 indexed taskIndex); + event CreateTask(uint256 indexed taskIndex, address indexed operator); event CompleteTask(uint256 indexed taskIndex, bool isValidAnswer); struct Task { @@ -51,7 +51,7 @@ contract SelfRegisterSqrtTaskMiddleware is bytes32 private constant COMPLETE_TASK_TYPEHASH = keccak256("CompleteTask(uint256 taskIndex,uint256 answer)"); - uint256 public constant MAX_OPERATORS = 100; + uint256 public constant MAX_OPERATORS = 300; uint256 public constant MAX_OPERATOR_VAULTS = 40; uint256 public constant MAX_SHARED_VAULTS = 60; @@ -99,7 +99,7 @@ contract SelfRegisterSqrtTaskMiddleware is taskIndex = tasks.length; tasks.push(Task({captureTimestamp: getCaptureTimestamp(), value: value, operator: operator, completed: false})); - emit CreateTask(taskIndex); + emit CreateTask(taskIndex, operator); } function completeTask( diff --git a/src/extensions/managers/keys/KeyManagerAddress.sol b/src/extensions/managers/keys/KeyManagerAddress.sol new file mode 100644 index 0000000..fe889dc --- /dev/null +++ b/src/extensions/managers/keys/KeyManagerAddress.sol @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +import {KeyManager} from "../../../managers/extendable/KeyManager.sol"; +import {PauseableEnumerableSet} from "../../../libraries/PauseableEnumerableSet.sol"; + +/** + * @title KeyManagerAddress + * @notice Manages storage and validation of operator keys using address values + * @dev Extends KeyManager to provide key management functionality + */ +abstract contract KeyManagerAddress is KeyManager { + uint64 public constant KeyManagerAddress_VERSION = 1; + + using PauseableEnumerableSet for PauseableEnumerableSet.AddressSet; + + error DuplicateKey(); + error MaxDisabledKeysReached(); + + uint256 private constant MAX_DISABLED_KEYS = 1; + + struct KeyManagerAddressStorage { + /// @notice Mapping from operator addresses to their keys + mapping(address => PauseableEnumerableSet.AddressSet) _keys; + /// @notice Mapping from keys to operator addresses + mapping(address => address) _keyToOperator; + } + + // keccak256(abi.encode(uint256(keccak256("symbiotic.storage.KeyManagerAddress")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant KeyManagerAddressStorageLocation = + 0x3da47716e6090d5a5545e03387f4dac112d37cd069a5573bb81de8579bd9dc00; + + function _getKeyManagerAddressStorage() internal pure returns (KeyManagerAddressStorage storage s) { + bytes32 location = KeyManagerAddressStorageLocation; + assembly { + s.slot := location + } + } + + /** + * @notice Gets the operator address associated with a key + * @param key The key to lookup + * @return The operator address that owns the key, or zero address if none + */ + function operatorByKey( + bytes memory key + ) public view override returns (address) { + KeyManagerAddressStorage storage $ = _getKeyManagerAddressStorage(); + return $._keyToOperator[abi.decode(key, (address))]; + } + + /** + * @notice Gets an operator's active key at the current capture timestamp + * @param operator The operator address to lookup + * @return The operator's active key encoded as bytes, or encoded zero bytes if none + */ + function operatorKey( + address operator + ) public view override returns (bytes memory) { + KeyManagerAddressStorage storage $ = _getKeyManagerAddressStorage(); + address[] memory active = $._keys[operator].getActive(getCaptureTimestamp()); + if (active.length == 0) { + return abi.encode(address(0)); + } + return abi.encode(active[0]); + } + + /** + * @notice Checks if a key was active at a specific timestamp + * @param timestamp The timestamp to check + * @param key_ The key to check + * @return True if the key was active at the timestamp, false otherwise + */ + function keyWasActiveAt(uint48 timestamp, bytes memory key_) public view override returns (bool) { + KeyManagerAddressStorage storage $ = _getKeyManagerAddressStorage(); + address key = abi.decode(key_, (address)); + return $._keys[$._keyToOperator[key]].wasActiveAt(timestamp, key); + } + + /** + * @notice Updates an operator's key + * @dev Handles key rotation by disabling old key and registering new one + * @param operator The operator address to update + * @param key_ The new key to register, encoded as bytes + */ + function _updateKey(address operator, bytes memory key_) internal override { + KeyManagerAddressStorage storage $ = _getKeyManagerAddressStorage(); + address key = abi.decode(key_, (address)); + uint48 timestamp = _now(); + + if ($._keyToOperator[key] != address(0)) { + revert DuplicateKey(); + } + + // check if we have reached the max number of disabled keys + // this allow us to limit the number times we can change the key + if (key != address(0) && $._keys[operator].length() > MAX_DISABLED_KEYS + 1) { + revert MaxDisabledKeysReached(); + } + + if ($._keys[operator].length() > 0) { + // try to remove disabled keys + address prevKey = address($._keys[operator].set.array[0].value); + if ($._keys[operator].checkUnregister(timestamp, _SLASHING_WINDOW(), prevKey)) { + $._keys[operator].unregister(timestamp, _SLASHING_WINDOW(), prevKey); + delete $._keyToOperator[prevKey]; + } else if ($._keys[operator].wasActiveAt(timestamp, prevKey)) { + $._keys[operator].pause(timestamp, prevKey); + } + } + + if (key != address(0)) { + // register the new key + $._keys[operator].register(timestamp, key); + $._keyToOperator[key] = operator; + } + } +} diff --git a/src/extensions/managers/sigs/ECDSASig.sol b/src/extensions/managers/sigs/ECDSASig.sol index 66940fe..ebdf09f 100644 --- a/src/extensions/managers/sigs/ECDSASig.sol +++ b/src/extensions/managers/sigs/ECDSASig.sol @@ -29,11 +29,10 @@ abstract contract ECDSASig is SigManager, IECDSASig { bytes memory key_, bytes memory signature ) internal pure override returns (bool) { - bytes32 key = abi.decode(key_, (bytes32)); + address key = abi.decode(key_, (address)); bytes32 hash = keccak256(abi.encodePacked(operator, key)); address signer = recover(hash, signature); - address keyAddress = address(uint160(uint256(key))); - return signer == keyAddress && signer != address(0); + return signer == key && signer != address(0); } /** diff --git a/src/libraries/PauseableEnumerableSet.sol b/src/libraries/PauseableEnumerableSet.sol index d086d7f..6c0b423 100644 --- a/src/libraries/PauseableEnumerableSet.sol +++ b/src/libraries/PauseableEnumerableSet.sol @@ -268,6 +268,25 @@ library PauseableEnumerableSet { self.set.unpause(timestamp, immutablePeriod, uint160(addr)); } + /** + * @notice Checks if an address can be unregistered + * @param self The AddressSet to query + * @param timestamp The current timestamp + * @param immutablePeriod The required waiting period after disabling + * @param value The address to check + * @return bool Whether the address can be unregistered + */ + function checkUnregister( + AddressSet storage self, + uint48 timestamp, + uint48 immutablePeriod, + address value + ) internal view returns (bool) { + uint256 pos = self.set.positions[uint160(value)]; + if (pos == 0) return false; + return self.set.array[pos - 1].status.checkUnregister(timestamp, immutablePeriod); + } + /** * @notice Unregisters an address * @param self The AddressSet to modify diff --git a/test/SigTests.t.sol b/test/SigTests.t.sol index 2a4764c..401d82a 100644 --- a/test/SigTests.t.sol +++ b/test/SigTests.t.sol @@ -25,7 +25,6 @@ contract SigTests is POCBaseTest { SelfRegisterEd25519Middleware internal ed25519Middleware; uint256 internal operatorPrivateKey; address internal operator; - bytes32 internal operatorPublicKey; string internal constant ED25519_TEST_DATA = "test/helpers/ed25519TestData.json"; address internal ed25519Operator; address internal vault; @@ -37,7 +36,6 @@ contract SigTests is POCBaseTest { super.setUp(); (operator, operatorPrivateKey) = makeAddrAndKey("operator"); - operatorPublicKey = bytes32(uint256(uint160(operator))); string memory json = vm.readFile(ED25519_TEST_DATA); ed25519Operator = abi.decode(vm.parseJson(json, ".operator"), (address)); @@ -135,27 +133,27 @@ contract SigTests is POCBaseTest { function testSelfRegisterOperator() public { // Create registration message - bytes32 messageHash = keccak256(abi.encodePacked(operator, operatorPublicKey)); + bytes32 messageHash = keccak256(abi.encodePacked(operator, operator)); // Sign message with operator's private key (uint8 v, bytes32 r, bytes32 s) = vm.sign(operatorPrivateKey, messageHash); bytes memory signature = abi.encodePacked(r, s, v); // Register operator using their own signature vm.prank(operator); - middleware.registerOperator(abi.encode(operatorPublicKey), address(vault), signature); + middleware.registerOperator(abi.encode(operator), address(vault), signature); // Verify operator is registered correctly assertTrue(IBaseMiddlewareReader(address(middleware)).isOperatorRegistered(operator)); - assertEq(abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (bytes32)), bytes32(0)); + assertEq(abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (address)), address(0)); vm.warp(vm.getBlockTimestamp() + 100); assertEq( - abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (bytes32)), operatorPublicKey + abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (address)), operator ); } function testSelxfRegisterOperatorInvalidSignature() public { // Create registration message with wrong key - bytes32 wrongKey = bytes32(uint256(1)); + address wrongKey = address(uint160(uint256(1))); bytes32 messageHash = keccak256(abi.encodePacked(operator, wrongKey)); // Sign message (uint8 v, bytes32 r, bytes32 s) = vm.sign(operatorPrivateKey, messageHash); @@ -164,12 +162,12 @@ contract SigTests is POCBaseTest { // Attempt to register with mismatched key should fail vm.prank(operator); vm.expectRevert(); - middleware.registerOperator(abi.encode(operatorPublicKey), address(vault), signature); + middleware.registerOperator(abi.encode(operator), address(vault), signature); } function testSelfxRegisterOperatorWrongSender() public { // Create valid registration message - bytes32 messageHash = keccak256(abi.encodePacked(operator, operatorPublicKey)); + bytes32 messageHash = keccak256(abi.encodePacked(operator, operator)); // Sign message with operator's key (uint8 v, bytes32 r, bytes32 s) = vm.sign(operatorPrivateKey, messageHash); bytes memory signature = abi.encodePacked(r, s, v); @@ -177,23 +175,23 @@ contract SigTests is POCBaseTest { // Attempt to register from different address should fail vm.prank(alice); vm.expectRevert(); - middleware.registerOperator(abi.encode(operatorPublicKey), address(vault), signature); + middleware.registerOperator(abi.encode(operator), address(vault), signature); } function testSelxfRegisterOperatorAlreadyRegistered() public { // Create registration message - bytes32 messageHash = keccak256(abi.encodePacked(operator, operatorPublicKey)); + bytes32 messageHash = keccak256(abi.encodePacked(operator, operator)); // Sign message (uint8 v, bytes32 r, bytes32 s) = vm.sign(operatorPrivateKey, messageHash); bytes memory signature = abi.encodePacked(r, s, v); // Register operator first time vm.prank(operator); - middleware.registerOperator(abi.encode(operatorPublicKey), address(vault), signature); + middleware.registerOperator(abi.encode(operator), address(vault), signature); // Attempt to register again should fail vm.prank(operator); vm.expectRevert(); - middleware.registerOperator(abi.encode(operatorPublicKey), address(vault), signature); + middleware.registerOperator(abi.encode(operator), address(vault), signature); } function testEd25519RegisterOperatorMismatchedKeyAndSignature() public { From ea9dc6801b7ea936ab740e5cd88a5053f9b8169e Mon Sep 17 00:00:00 2001 From: alrxy Date: Tue, 17 Dec 2024 19:24:15 +0400 Subject: [PATCH 11/15] fix: statemind - Open access variables may lead to DOS --- README.md | 9 +- .../SelfRegisterEd25519Middleware.sol | 24 --- .../SelfRegisterMiddleware.sol | 24 --- .../SelfRegisterSqrtTaskMiddleware.sol | 80 ++++----- .../sqrt-task-network/SqrtTaskMiddleware.sol | 4 +- .../operators/ApprovalRegisterOperators.sol | 103 +++++++++++ src/extensions/operators/Operators.sol | 60 ++++++- .../operators/SelfRegisterOperators.sol | 167 ++++-------------- .../operators/IApprovalRegisterOperators.sol | 59 +++++++ .../IForcePauseSelfRegisterOperators.sol | 4 +- .../operators/ISelfRegisterOperators.sol | 1 - test/SigTests.t.sol | 4 +- 12 files changed, 287 insertions(+), 252 deletions(-) create mode 100644 src/extensions/operators/ApprovalRegisterOperators.sol create mode 100644 src/interfaces/extensions/operators/IApprovalRegisterOperators.sol diff --git a/README.md b/README.md index f31a238..2f12492 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ This repository provides a framework for developing middleware in a modular and - **Operators**: Manages operator registration and operator's vault. - - **KeyManager**: Manages operator keys. Variants include `KeyManager256`, `KeyManagerBytes`, and `NoKeyManager`. + - **KeyManager**: Manages operator keys. Variants include `KeyManagerAddress`, `KeyManager256`, `KeyManagerBytes`, and `NoKeyManager`. - **AccessManager**: Controls access to restricted functions. Implementations include `OwnableAccessManager`, `OzAccessControl`, `OzAccessManaged`, and `NoAccessManager`. @@ -63,7 +63,7 @@ Features: #### SelfRegisterMiddleware ```solidity -contract SelfRegisterMiddleware is SharedVaults, SelfRegisterOperators, KeyManager256, ECDSASig, NoAccessManager, TimestampCapture, EqualStakePower { +contract SelfRegisterMiddleware is SharedVaults, SelfRegisterOperators, KeyManagerAddress, ECDSASig, NoAccessManager, TimestampCapture, EqualStakePower { // Implementation details... } ``` @@ -89,7 +89,7 @@ Features: #### SelfRegisterSqrtTaskMiddleware ```solidity -contract SelfRegisterSqrtTaskMiddleware is SharedVaults, SelfRegisterOperators, KeyManager256, ECDSASig, OwnableAccessManager, TimestampCapture, EqualStakePower { +contract SelfRegisterSqrtTaskMiddleware is SharedVaults, SelfRegisterOperators, KeyManagerAddress, ECDSASig, OwnableAccessManager, TimestampCapture, EqualStakePower { // Implementation details... } ``` @@ -209,10 +209,9 @@ contract MyCustomMiddleware is BaseMiddleware, Operators, KeyStorage256, Ownable 1. Granting roles to addresses using `grantRole(bytes32 role, address account)` 2. Setting role admins with `_setRoleAdmin(bytes32 role, bytes32 adminRole)` 3. Assigning roles to function selectors via `_setSelectorRole(bytes4 selector, bytes32 role)` - - `OzAccessManaged`: Wraps OpenZeppelin's AccessManaged contract to integrate with external access control systems - `OzAccessManaged`: Wraps OpenZeppelin's AccessManaged contract to integrate with external access control systems. This allows for more complex access control scenarios where permissions are managed externally, providing flexibility and scalability in managing roles and permissions. -- **Key Manager**: Choose a `KeyManager` implementation that suits your key management needs. Use `KeyManager256` for managing 256-bit keys, `KeyManagerBytes` for handling arbitrary-length keys, or `NoKeyManager` if key management is not required. +- **Key Manager**: Choose a `KeyManager` implementation that suits your key management needs. Use `KeyManagerAddress` for managing address keys, `KeyManager256` for managing 256-bit keys, `KeyManagerBytes` for handling arbitrary-length keys, or `NoKeyManager` if key management is not required. This framework provides flexibility in building middleware by allowing you to mix and match various extensions based on your requirements. By following the modular approach and best practices outlined, you can develop robust middleware solutions that integrate seamlessly with the network. diff --git a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol index be2dfdb..8f64090 100644 --- a/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol +++ b/src/examples/self-register-network/SelfRegisterEd25519Middleware.sol @@ -20,12 +20,6 @@ contract SelfRegisterEd25519Middleware is TimestampCapture, EqualStakePower { - error TooManyOperators(); - error TooManyOperatorVaults(); - - uint256 public constant MAX_OPERATORS = 100; - uint256 public constant MAX_OPERATOR_VAULTS = 40; - /** * @notice Constructor for initializing the SelfRegisterEd25519Middleware contract * @param network The address of the network @@ -61,22 +55,4 @@ contract SelfRegisterEd25519Middleware is __SelfRegisterOperators_init("SelfRegisterEd25519Middleware"); __OwnableAccessManager_init(owner); } - - /// @notice Prevents DOS by limiting total number of operators that can be registered - /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators - function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { - super._beforeRegisterOperator(operator, key, vault); - if (_operatorsLength() >= MAX_OPERATORS) { - revert TooManyOperators(); - } - } - - /// @notice Prevents DOS by limiting number of vaults per operator - /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults - function _beforeRegisterOperatorVault(address operator, address vault) internal virtual override { - super._beforeRegisterOperatorVault(operator, vault); - if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { - revert TooManyOperatorVaults(); - } - } } diff --git a/src/examples/self-register-network/SelfRegisterMiddleware.sol b/src/examples/self-register-network/SelfRegisterMiddleware.sol index 9a00f6c..d205782 100644 --- a/src/examples/self-register-network/SelfRegisterMiddleware.sol +++ b/src/examples/self-register-network/SelfRegisterMiddleware.sol @@ -20,12 +20,6 @@ contract SelfRegisterMiddleware is TimestampCapture, EqualStakePower { - error TooManyOperators(); - error TooManyOperatorVaults(); - - uint256 public constant MAX_OPERATORS = 100; - uint256 public constant MAX_OPERATOR_VAULTS = 40; - /** * @notice Constructor for initializing the SelfRegisterMiddleware contract * @param network The address of the network @@ -61,22 +55,4 @@ contract SelfRegisterMiddleware is __SelfRegisterOperators_init("SelfRegisterMiddleware"); __OwnableAccessManager_init(owner); } - - /// @notice Prevents DOS by limiting total number of operators that can be registered - /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators - function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { - super._beforeRegisterOperator(operator, key, vault); - if (_operatorsLength() >= MAX_OPERATORS) { - revert TooManyOperators(); - } - } - - /// @notice Prevents DOS by limiting number of vaults per operator - /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults - function _beforeRegisterOperatorVault(address operator, address vault) internal virtual override { - super._beforeRegisterOperatorVault(operator, vault); - if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { - revert TooManyOperatorVaults(); - } - } } diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index 8f2b6fe..da702f8 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -35,25 +35,22 @@ contract SelfRegisterSqrtTaskMiddleware is error InvalidHints(); error TaskCompleted(); - error TooManyOperators(); error TooManyOperatorVaults(); - error TooManySharedVaults(); + error InactiveValidator(); - event CreateTask(uint256 indexed taskIndex, address indexed operator); + event CreateTask(uint256 indexed taskIndex, address indexed validator); event CompleteTask(uint256 indexed taskIndex, bool isValidAnswer); struct Task { uint48 captureTimestamp; uint256 value; - address operator; + address validator; bool completed; } bytes32 private constant COMPLETE_TASK_TYPEHASH = keccak256("CompleteTask(uint256 taskIndex,uint256 answer)"); - uint256 public constant MAX_OPERATORS = 300; - uint256 public constant MAX_OPERATOR_VAULTS = 40; - uint256 public constant MAX_SHARED_VAULTS = 60; + uint256 public constant MAX_OPERATOR_VAULTS = 20; Task[] public tasks; @@ -84,22 +81,21 @@ contract SelfRegisterSqrtTaskMiddleware is __SelfRegisterOperators_init("SelfRegisterSqrtTaskMiddleware"); } - // allow anyone to register shared vaults - function _checkAccess() internal view override(AccessManager, OwnableAccessManager) { - if ( - msg.sig == this.registerSharedVault.selector || msg.sig == this.unregisterSharedVault.selector - || msg.sig == this.pauseSharedVault.selector - ) { - return; + function createTask(uint256 value, address validator) external returns (uint256 taskIndex) { + bytes memory key = abi.encode(validator); + address operator = operatorByKey(key); + if (!keyWasActiveAt(getCaptureTimestamp(), key)) { + revert InactiveValidator(); + } + if (!_isOperatorRegistered(operator)) { + revert OperatorNotRegistered(); } - OwnableAccessManager._checkAccess(); - } - - function createTask(uint256 value, address operator) external returns (uint256 taskIndex) { taskIndex = tasks.length; - tasks.push(Task({captureTimestamp: getCaptureTimestamp(), value: value, operator: operator, completed: false})); + tasks.push( + Task({captureTimestamp: getCaptureTimestamp(), value: value, validator: validator, completed: false}) + ); - emit CreateTask(taskIndex, operator); + emit CreateTask(taskIndex, validator); } function completeTask( @@ -133,7 +129,7 @@ contract SelfRegisterSqrtTaskMiddleware is bytes32 hash_ = _hashTypedDataV4(keccak256(abi.encode(COMPLETE_TASK_TYPEHASH, taskIndex, answer))); - if (!SignatureChecker.isValidSignatureNow(task.operator, hash_, signature)) { + if (!SignatureChecker.isValidSignatureNow(task.validator, hash_, signature)) { revert InvalidSignature(); } } @@ -166,7 +162,17 @@ contract SelfRegisterSqrtTaskMiddleware is function _slash(uint256 taskIndex, bytes[] calldata stakeHints, bytes[] calldata slashHints) private { Task storage task = tasks[taskIndex]; - address[] memory vaults = _activeVaultsAt(task.captureTimestamp, task.operator); + address operator = operatorByKey(abi.encode(task.validator)); + _slashOperator(task.captureTimestamp, operator, stakeHints, slashHints); + } + + function _slashOperator( + uint48 captureTimestamp, + address operator, + bytes[] calldata stakeHints, + bytes[] calldata slashHints + ) private { + address[] memory vaults = _activeVaultsAt(captureTimestamp, operator); uint256 vaultsLength = vaults.length; if (stakeHints.length != slashHints.length || stakeHints.length != vaultsLength) { @@ -176,15 +182,14 @@ contract SelfRegisterSqrtTaskMiddleware is bytes32 subnetwork = _NETWORK().subnetwork(0); for (uint256 i; i < vaultsLength; ++i) { address vault = vaults[i]; - uint256 slashAmount = IBaseDelegator(IVault(vault).delegator()).stakeAt( - subnetwork, task.operator, task.captureTimestamp, stakeHints[i] - ); + uint256 slashAmount = + IBaseDelegator(IVault(vault).delegator()).stakeAt(subnetwork, operator, captureTimestamp, stakeHints[i]); if (slashAmount == 0) { continue; } - _slashVault(task.captureTimestamp, vault, subnetwork, task.operator, slashAmount, slashHints[i]); + _slashVault(captureTimestamp, vault, subnetwork, operator, slashAmount, slashHints[i]); } } @@ -199,20 +204,6 @@ contract SelfRegisterSqrtTaskMiddleware is _slashVault(epochStart, vault, subnetwork, operator, amount, hints); } - /// @notice Prevents DOS by limiting total number of shared vaults that can be registered - /// @dev MAX_SHARED_VAULTS constant prevents unbounded iteration when looping through shared vaults - function _beforeRegisterSharedVault( - address sharedVault - ) internal override { - super._beforeRegisterSharedVault(sharedVault); - if (_sharedVaultsLength() >= MAX_SHARED_VAULTS) { - revert TooManySharedVaults(); - } - IBaseDelegator(IVault(sharedVault).delegator()).setMaxNetworkLimit(DEFAULT_SUBNETWORK, type(uint256).max); - } - - /// @notice Prevents DOS by limiting number of vaults per operator - /// @dev MAX_OPERATOR_VAULTS constant prevents unbounded iteration when looping through an operator's vaults function _beforeRegisterOperatorVault(address operator, address vault) internal override { super._beforeRegisterOperatorVault(operator, vault); if (_operatorVaultsLength(operator) >= MAX_OPERATOR_VAULTS) { @@ -220,13 +211,4 @@ contract SelfRegisterSqrtTaskMiddleware is } IBaseDelegator(IVault(vault).delegator()).setMaxNetworkLimit(DEFAULT_SUBNETWORK, type(uint256).max); } - - /// @notice Prevents DOS by limiting total number of operators that can be registered - /// @dev MAX_OPERATORS constant prevents unbounded iteration when looping through operators - function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual override { - super._beforeRegisterOperator(operator, key, vault); - if (_operatorsLength() >= MAX_OPERATORS) { - revert TooManyOperators(); - } - } } diff --git a/src/examples/sqrt-task-network/SqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SqrtTaskMiddleware.sol index b15c36a..faeb767 100644 --- a/src/examples/sqrt-task-network/SqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SqrtTaskMiddleware.sol @@ -34,7 +34,7 @@ contract SqrtTaskMiddleware is error InvalidSignature(); error TaskCompleted(); - event CreateTask(uint256 indexed taskIndex); + event CreateTask(uint256 indexed taskIndex, address indexed operator); event CompleteTask(uint256 indexed taskIndex, bool isValidAnswer); struct Task { @@ -77,7 +77,7 @@ contract SqrtTaskMiddleware is taskIndex = tasks.length; tasks.push(Task({captureTimestamp: getCaptureTimestamp(), value: value, operator: operator, completed: false})); - emit CreateTask(taskIndex); + emit CreateTask(taskIndex, operator); } function completeTask( diff --git a/src/extensions/operators/ApprovalRegisterOperators.sol b/src/extensions/operators/ApprovalRegisterOperators.sol new file mode 100644 index 0000000..d3444b2 --- /dev/null +++ b/src/extensions/operators/ApprovalRegisterOperators.sol @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +import {SelfRegisterOperators} from "./SelfRegisterOperators.sol"; +import {IApprovalRegisterOperators} from "../../interfaces/extensions/operators/IApprovalRegisterOperators.sol"; +/** + * @title ApprovalRegisterOperators + * @notice Extends SelfRegisterOperators to add approval-based registration + */ +abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalRegisterOperators { + uint64 public constant ApprovalRegisterOperators_VERSION = 1; + + struct ApprovalRegisterOperatorsStorage { + RegistrationRequest[] requests; + } + + // keccak256(abi.encode(uint256(keccak256("symbiotic.storage.ApprovalRegisterOperators")) - 1)) & ~bytes32(uint256(0xff)) + bytes32 private constant ApprovalRegisterOperators_STORAGE_LOCATION = + 0x8d3c0d900c3fcfbc53470fac03a90d5cf6aa7b77c3f1ed10e6c6bd4d192eaf00; + + function _getApprovalRegisterOperatorsStorage() private pure returns (ApprovalRegisterOperatorsStorage storage $) { + bytes32 location = ApprovalRegisterOperators_STORAGE_LOCATION; + assembly { + $.slot := location + } + } + + /** + * @inheritdoc IApprovalRegisterOperators + */ + function getRegistrationRequestCount() public view returns (uint256) { + return _getApprovalRegisterOperatorsStorage().requests.length; + } + + /** + * @inheritdoc IApprovalRegisterOperators + */ + function getRegistrationRequest( + uint256 index + ) public view returns (RegistrationRequest memory) { + return _getApprovalRegisterOperatorsStorage().requests[index]; + } + + /** + * @inheritdoc IApprovalRegisterOperators + */ + function registerOperator( + uint256 requestIndex + ) public checkAccess { + RegistrationRequest memory request = getRegistrationRequest(requestIndex); + _registerOperatorImpl(request.operator, request.key, request.vault); + ApprovalRegisterOperatorsStorage storage $ = _getApprovalRegisterOperatorsStorage(); + uint256 lastIndex = $.requests.length - 1; + if (requestIndex != lastIndex) { + $.requests[requestIndex] = $.requests[lastIndex]; + } + $.requests.pop(); + } + + /** + * @notice Override to prevent direct registration + */ + function registerOperator(bytes memory, address, bytes memory) public override { + revert DirectRegistrationNotAllowed(); + } + + /** + * @notice Override to prevent direct registration + */ + function registerOperator(address, bytes memory, address, bytes memory, bytes memory) public override { + revert DirectRegistrationNotAllowed(); + } + + /** + * @inheritdoc IApprovalRegisterOperators + */ + function requestRegisterOperator(bytes memory key, address vault, bytes memory signature) public { + _verifyKey(msg.sender, key, signature); + ApprovalRegisterOperatorsStorage storage $ = _getApprovalRegisterOperatorsStorage(); + $.requests.push(RegistrationRequest({operator: msg.sender, vault: vault, key: key})); + } + + /** + * @inheritdoc IApprovalRegisterOperators + */ + function requestRegisterOperator( + address operator, + bytes memory key, + address vault, + bytes memory signature, + bytes memory keySignature + ) public { + SelfRegisterOperatorsStorage storage s = _getSelfRegisterOperatorsStorage(); + _verifyEIP712( + operator, + keccak256(abi.encode(REGISTER_OPERATOR_TYPEHASH, operator, keccak256(key), vault, s.nonces[operator]++)), + signature + ); + _verifyKey(operator, key, keySignature); + ApprovalRegisterOperatorsStorage storage $ = _getApprovalRegisterOperatorsStorage(); + $.requests.push(RegistrationRequest({operator: operator, vault: vault, key: key})); + } +} diff --git a/src/extensions/operators/Operators.sol b/src/extensions/operators/Operators.sol index cf71726..3e90d6e 100644 --- a/src/extensions/operators/Operators.sol +++ b/src/extensions/operators/Operators.sol @@ -15,7 +15,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function registerOperator(address operator, bytes memory key, address vault) public checkAccess { + function registerOperator(address operator, bytes memory key, address vault) external virtual checkAccess { + _registerOperatorImpl(operator, key, vault); + } + + function _registerOperatorImpl(address operator, bytes memory key, address vault) internal virtual { _beforeRegisterOperator(operator, key, vault); _registerOperator(operator); _updateKey(operator, key); @@ -30,7 +34,13 @@ abstract contract Operators is BaseMiddleware, IOperators { */ function unregisterOperator( address operator - ) public checkAccess { + ) external virtual checkAccess { + _unregisterOperatorImpl(operator); + } + + function _unregisterOperatorImpl( + address operator + ) internal virtual { _beforeUnregisterOperator(operator); _unregisterOperator(operator); } @@ -40,7 +50,13 @@ abstract contract Operators is BaseMiddleware, IOperators { */ function pauseOperator( address operator - ) public checkAccess { + ) external virtual checkAccess { + _pauseOperatorImpl(operator); + } + + function _pauseOperatorImpl( + address operator + ) internal virtual { _beforePauseOperator(operator); _pauseOperator(operator); } @@ -50,7 +66,13 @@ abstract contract Operators is BaseMiddleware, IOperators { */ function unpauseOperator( address operator - ) public checkAccess { + ) external virtual checkAccess { + _unpauseOperatorImpl(operator); + } + + function _unpauseOperatorImpl( + address operator + ) internal virtual { _beforeUnpauseOperator(operator); _unpauseOperator(operator); } @@ -58,7 +80,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function updateOperatorKey(address operator, bytes memory key) public checkAccess { + function updateOperatorKey(address operator, bytes memory key) external virtual checkAccess { + _updateOperatorKeyImpl(operator, key); + } + + function _updateOperatorKeyImpl(address operator, bytes memory key) internal virtual { _beforeUpdateOperatorKey(operator, key); _updateKey(operator, key); } @@ -66,7 +92,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function registerOperatorVault(address operator, address vault) public checkAccess { + function registerOperatorVault(address operator, address vault) external virtual checkAccess { + _registerOperatorVaultImpl(operator, vault); + } + + function _registerOperatorVaultImpl(address operator, address vault) internal virtual { if (!_isOperatorRegistered(operator)) { revert OperatorNotRegistered(); } @@ -77,7 +107,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function unregisterOperatorVault(address operator, address vault) public checkAccess { + function unregisterOperatorVault(address operator, address vault) external virtual checkAccess { + _unregisterOperatorVaultImpl(operator, vault); + } + + function _unregisterOperatorVaultImpl(address operator, address vault) internal virtual { _beforeUnregisterOperatorVault(operator, vault); _unregisterOperatorVault(operator, vault); } @@ -85,7 +119,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function pauseOperatorVault(address operator, address vault) public checkAccess { + function pauseOperatorVault(address operator, address vault) external virtual checkAccess { + _pauseOperatorVaultImpl(operator, vault); + } + + function _pauseOperatorVaultImpl(address operator, address vault) internal virtual { _beforePauseOperatorVault(operator, vault); _pauseOperatorVault(operator, vault); } @@ -93,7 +131,11 @@ abstract contract Operators is BaseMiddleware, IOperators { /** * @inheritdoc IOperators */ - function unpauseOperatorVault(address operator, address vault) public checkAccess { + function unpauseOperatorVault(address operator, address vault) external virtual checkAccess { + _unpauseOperatorVaultImpl(operator, vault); + } + + function _unpauseOperatorVaultImpl(address operator, address vault) internal virtual { _beforeUnpauseOperatorVault(operator, vault); _unpauseOperatorVault(operator, vault); } diff --git a/src/extensions/operators/SelfRegisterOperators.sol b/src/extensions/operators/SelfRegisterOperators.sol index 3b1f40a..f7d3b50 100644 --- a/src/extensions/operators/SelfRegisterOperators.sol +++ b/src/extensions/operators/SelfRegisterOperators.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.25; -import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; +import {Operators} from "./Operators.sol"; import {SigManager} from "../../managers/extendable/SigManager.sol"; import {EIP712Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/cryptography/EIP712Upgradeable.sol"; import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; @@ -11,12 +11,13 @@ import {ISelfRegisterOperators} from "../../interfaces/extensions/operators/ISel * @title SelfRegisterOperators * @notice Contract for self-registration and management of operators with signature verification * @dev Extends BaseMiddleware, SigManager, and EIP712Upgradeable to provide signature-based operator management + * @dev CAUTION: If activeOperators functionality is needed, use ApprovalRegisterOperators instead to prevent DOS attacks */ -abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upgradeable, ISelfRegisterOperators { +abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradeable, ISelfRegisterOperators { uint64 public constant SelfRegisterOperators_VERSION = 1; // EIP-712 TypeHash constants - bytes32 private constant REGISTER_OPERATOR_TYPEHASH = + bytes32 internal constant REGISTER_OPERATOR_TYPEHASH = keccak256("RegisterOperator(address operator,bytes key,address vault,uint256 nonce)"); bytes32 private constant UNREGISTER_OPERATOR_TYPEHASH = keccak256("UnregisterOperator(address operator,uint256 nonce)"); @@ -41,7 +42,7 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg bytes32 private constant SelfResgisterOperators_STORAGE_LOCATION = 0x7c1bcd600c3fcfbc53470fac03a90d5cf6aa7b77c3f1ed10e6c6bd4d192eaf00; - function _getSelfRegisterOperatorsStorage() private pure returns (SelfRegisterOperatorsStorage storage $) { + function _getSelfRegisterOperatorsStorage() internal pure returns (SelfRegisterOperatorsStorage storage $) { bytes32 location = SelfResgisterOperators_STORAGE_LOCATION; assembly { $.slot := location @@ -67,16 +68,9 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg /** * @inheritdoc ISelfRegisterOperators */ - function registerOperator(bytes memory key, address vault, bytes memory signature) public { + function registerOperator(bytes memory key, address vault, bytes memory signature) public virtual override { _verifyKey(msg.sender, key, signature); - _beforeRegisterOperator(msg.sender, key, vault); - _registerOperator(msg.sender); - _beforeUpdateOperatorKey(msg.sender, key); - _updateKey(msg.sender, key); - if (vault != address(0)) { - _beforeRegisterOperatorVault(msg.sender, vault); - _registerOperatorVault(msg.sender, vault); - } + _registerOperatorImpl(msg.sender, key, vault); } /** @@ -88,91 +82,77 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg address vault, bytes memory signature, bytes memory keySignature - ) public { + ) public virtual { + _verifyKey(operator, key, keySignature); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(REGISTER_OPERATOR_TYPEHASH, operator, keccak256(key), vault, $.nonces[operator]++)), signature ); - _verifyKey(operator, key, keySignature); - _beforeRegisterOperator(operator, key, vault); - _registerOperator(operator); - _beforeUpdateOperatorKey(operator, key); - _updateKey(operator, key); - if (vault != address(0)) { - _beforeRegisterOperatorVault(operator, vault); - _registerOperatorVault(operator, vault); - } + _registerOperatorImpl(operator, key, vault); } /** * @inheritdoc ISelfRegisterOperators */ - function unregisterOperator() public { - _beforeUnregisterOperator(msg.sender); - _unregisterOperator(msg.sender); + function unregisterOperator() public override { + _unregisterOperatorImpl(msg.sender); } /** * @inheritdoc ISelfRegisterOperators */ function unregisterOperator(address operator, bytes memory signature) public { - _beforeUnregisterOperator(operator); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(UNREGISTER_OPERATOR_TYPEHASH, operator, $.nonces[operator]++)), signature ); - _unregisterOperator(operator); + _unregisterOperatorImpl(operator); } /** * @inheritdoc ISelfRegisterOperators */ - function pauseOperator() public { - _beforePauseOperator(msg.sender); - _pauseOperator(msg.sender); + function pauseOperator() public override { + _pauseOperatorImpl(msg.sender); } /** * @inheritdoc ISelfRegisterOperators */ function pauseOperator(address operator, bytes memory signature) public { - _beforePauseOperator(operator); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(PAUSE_OPERATOR_TYPEHASH, operator, $.nonces[operator]++)), signature ); - _pauseOperator(operator); + _pauseOperatorImpl(operator); } /** * @inheritdoc ISelfRegisterOperators */ - function unpauseOperator() public { - _beforeUnpauseOperator(msg.sender); - _unpauseOperator(msg.sender); + function unpauseOperator() public override { + _unpauseOperatorImpl(msg.sender); } /** * @inheritdoc ISelfRegisterOperators */ function unpauseOperator(address operator, bytes memory signature) public { - _beforeUnpauseOperator(operator); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(UNPAUSE_OPERATOR_TYPEHASH, operator, $.nonces[operator]++)), signature ); - _unpauseOperator(operator); + _unpauseOperatorImpl(operator); } /** * @inheritdoc ISelfRegisterOperators */ - function updateOperatorKey(bytes memory key, bytes memory signature) public { + function updateOperatorKey(bytes memory key, bytes memory signature) public override { _verifyKey(msg.sender, key, signature); - _beforeUpdateOperatorKey(msg.sender, key); - _updateKey(msg.sender, key); + _updateOperatorKeyImpl(msg.sender, key); } /** @@ -184,15 +164,14 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg bytes memory signature, bytes memory keySignature ) public { + _verifyKey(operator, key, keySignature); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(UPDATE_OPERATOR_KEY_TYPEHASH, operator, keccak256(key), $.nonces[operator]++)), signature ); - _verifyKey(operator, key, keySignature); - _beforeUpdateOperatorKey(operator, key); - _updateKey(operator, key); + _updateOperatorKeyImpl(operator, key); } /** @@ -200,10 +179,8 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg */ function registerOperatorVault( address vault - ) public { - require(_isOperatorRegistered(msg.sender), "Operator not registered"); - _beforeRegisterOperatorVault(msg.sender, vault); - _registerOperatorVault(msg.sender, vault); + ) public override { + _registerOperatorVaultImpl(msg.sender, vault); } /** @@ -213,14 +190,13 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg if (!_isOperatorRegistered(operator)) { revert OperatorNotRegistered(); } - _beforeRegisterOperatorVault(operator, vault); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(REGISTER_OPERATOR_VAULT_TYPEHASH, operator, vault, $.nonces[operator]++)), signature ); - _registerOperatorVault(operator, vault); + _registerOperatorVaultImpl(operator, vault); } /** @@ -228,23 +204,21 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg */ function unregisterOperatorVault( address vault - ) public { - _beforeUnregisterOperatorVault(msg.sender, vault); - _unregisterOperatorVault(msg.sender, vault); + ) public override { + _unregisterOperatorVaultImpl(msg.sender, vault); } /** * @inheritdoc ISelfRegisterOperators */ function unregisterOperatorVault(address operator, address vault, bytes memory signature) public { - _beforeUnregisterOperatorVault(operator, vault); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(UNREGISTER_OPERATOR_VAULT_TYPEHASH, operator, vault, $.nonces[operator]++)), signature ); - _unregisterOperatorVault(operator, vault); + _unregisterOperatorVaultImpl(operator, vault); } /** @@ -252,23 +226,21 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg */ function pauseOperatorVault( address vault - ) public { - _beforePauseOperatorVault(msg.sender, vault); - _pauseOperatorVault(msg.sender, vault); + ) public override { + _pauseOperatorVaultImpl(msg.sender, vault); } /** * @inheritdoc ISelfRegisterOperators */ function pauseOperatorVault(address operator, address vault, bytes memory signature) public { - _beforePauseOperatorVault(operator, vault); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(PAUSE_OPERATOR_VAULT_TYPEHASH, operator, vault, $.nonces[operator]++)), signature ); - _pauseOperatorVault(operator, vault); + _pauseOperatorVaultImpl(operator, vault); } /** @@ -276,23 +248,21 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg */ function unpauseOperatorVault( address vault - ) public { - _beforeUnpauseOperatorVault(msg.sender, vault); - _unpauseOperatorVault(msg.sender, vault); + ) public override { + _unpauseOperatorVaultImpl(msg.sender, vault); } /** * @inheritdoc ISelfRegisterOperators */ function unpauseOperatorVault(address operator, address vault, bytes memory signature) public { - _beforeUnpauseOperatorVault(operator, vault); SelfRegisterOperatorsStorage storage $ = _getSelfRegisterOperatorsStorage(); _verifyEIP712( operator, keccak256(abi.encode(UNPAUSE_OPERATOR_VAULT_TYPEHASH, operator, vault, $.nonces[operator]++)), signature ); - _unpauseOperatorVault(operator, vault); + _unpauseOperatorVaultImpl(operator, vault); } /** @@ -318,71 +288,4 @@ abstract contract SelfRegisterOperators is BaseMiddleware, SigManager, EIP712Upg revert InvalidSignature(); } } - - /** - * @notice Hook called before updating an operator's key - * @param operator The operator address - * @param key The new key - */ - function _beforeUpdateOperatorKey(address operator, bytes memory key) internal virtual {} - - /** - * @notice Hook called before registering an operator - * @param operator The operator address - * @param key The operator's key - * @param vault Optional vault address - */ - function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual {} - - /** - * @notice Hook called before unregistering an operator - * @param operator The operator address - */ - function _beforeUnregisterOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before pausing an operator - * @param operator The operator address - */ - function _beforePauseOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before unpausing an operator - * @param operator The operator address - */ - function _beforeUnpauseOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before registering an operator vault - * @param operator The operator address - * @param vault The vault address - */ - function _beforeRegisterOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before unregistering an operator vault - * @param operator The operator address - * @param vault The vault address - */ - function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before pausing an operator vault - * @param operator The operator address - * @param vault The vault address - */ - function _beforePauseOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before unpausing an operator vault - * @param operator The operator address - * @param vault The vault address - */ - function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual {} } diff --git a/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol b/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol new file mode 100644 index 0000000..12c1a40 --- /dev/null +++ b/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +/** + * @title IApprovalRegisterOperators + * @notice Interface for approval-based operator registration + */ +interface IApprovalRegisterOperators { + struct RegistrationRequest { + address operator; + address vault; + bytes key; + } + + error DirectRegistrationNotAllowed(); + + /** + * @notice Get the total number of pending registration requests + * @return The number of requests + */ + function getRegistrationRequestCount() external view returns (uint256); + + /** + * @notice Get a specific registration request by index + * @param index The index of the request to retrieve + * @return The registration request details + */ + function getRegistrationRequest(uint256 index) external view returns (RegistrationRequest memory); + + /** + * @notice Register an operator based on a pending request + * @param requestIndex The index of the request to register + */ + function registerOperator(uint256 requestIndex) external; + + /** + * @notice Request registration as an operator + * @param key The operator's public key + * @param vault Optional vault address to associate + * @param signature Signature proving ownership of the key + */ + function requestRegisterOperator(bytes memory key, address vault, bytes memory signature) external; + + /** + * @notice Request registration on behalf of another operator + * @param operator The address of the operator to register + * @param key The operator's public key + * @param vault Optional vault address to associate + * @param signature EIP712 signature authorizing registration + * @param keySignature Signature proving ownership of the key + */ + function requestRegisterOperator( + address operator, + bytes memory key, + address vault, + bytes memory signature, + bytes memory keySignature + ) external; +} diff --git a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol index 1d864b9..2de7027 100644 --- a/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol +++ b/src/interfaces/extensions/operators/IForcePauseSelfRegisterOperators.sol @@ -1,13 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.25; -import {ISelfRegisterOperators} from "./ISelfRegisterOperators.sol"; - /** * @title IForcePauseSelfRegisterOperators * @notice Interface for force pausing operators and operator-vault pairs */ -interface IForcePauseSelfRegisterOperators is ISelfRegisterOperators { +interface IForcePauseSelfRegisterOperators { error OperatorForcePaused(); error OperatorVaultForcePaused(); diff --git a/src/interfaces/extensions/operators/ISelfRegisterOperators.sol b/src/interfaces/extensions/operators/ISelfRegisterOperators.sol index a103452..9a6fe8b 100644 --- a/src/interfaces/extensions/operators/ISelfRegisterOperators.sol +++ b/src/interfaces/extensions/operators/ISelfRegisterOperators.sol @@ -7,7 +7,6 @@ pragma solidity ^0.8.25; */ interface ISelfRegisterOperators { error InvalidSignature(); - error OperatorNotRegistered(); /** * @notice Returns the nonce for an operator address diff --git a/test/SigTests.t.sol b/test/SigTests.t.sol index 401d82a..102443b 100644 --- a/test/SigTests.t.sol +++ b/test/SigTests.t.sol @@ -146,9 +146,7 @@ contract SigTests is POCBaseTest { assertEq(abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (address)), address(0)); vm.warp(vm.getBlockTimestamp() + 100); - assertEq( - abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (address)), operator - ); + assertEq(abi.decode(IBaseMiddlewareReader(address(middleware)).operatorKey(operator), (address)), operator); } function testSelxfRegisterOperatorInvalidSignature() public { From ee799a0dc5ab9eeacc04c9af9e0d549e2e06a6ad Mon Sep 17 00:00:00 2001 From: alrxy Date: Tue, 17 Dec 2024 19:24:30 +0400 Subject: [PATCH 12/15] fix: lint --- src/extensions/operators/ApprovalRegisterOperators.sol | 1 + .../extensions/operators/IApprovalRegisterOperators.sol | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/extensions/operators/ApprovalRegisterOperators.sol b/src/extensions/operators/ApprovalRegisterOperators.sol index d3444b2..9bdd00f 100644 --- a/src/extensions/operators/ApprovalRegisterOperators.sol +++ b/src/extensions/operators/ApprovalRegisterOperators.sol @@ -7,6 +7,7 @@ import {IApprovalRegisterOperators} from "../../interfaces/extensions/operators/ * @title ApprovalRegisterOperators * @notice Extends SelfRegisterOperators to add approval-based registration */ + abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalRegisterOperators { uint64 public constant ApprovalRegisterOperators_VERSION = 1; diff --git a/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol b/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol index 12c1a40..5aa416d 100644 --- a/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol +++ b/src/interfaces/extensions/operators/IApprovalRegisterOperators.sol @@ -25,13 +25,17 @@ interface IApprovalRegisterOperators { * @param index The index of the request to retrieve * @return The registration request details */ - function getRegistrationRequest(uint256 index) external view returns (RegistrationRequest memory); + function getRegistrationRequest( + uint256 index + ) external view returns (RegistrationRequest memory); /** * @notice Register an operator based on a pending request * @param requestIndex The index of the request to register */ - function registerOperator(uint256 requestIndex) external; + function registerOperator( + uint256 requestIndex + ) external; /** * @notice Request registration as an operator From e3120c6825583a789f692e4ac843a13e37723dbf Mon Sep 17 00:00:00 2001 From: alrxy Date: Wed, 18 Dec 2024 02:17:34 +0400 Subject: [PATCH 13/15] chore: docstring in selfregsitersqrtmiddleware --- .../sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index da702f8..ae6ed35 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -21,6 +21,13 @@ import {KeyManagerAddress} from "../../extensions/managers/keys/KeyManagerAddres import {TimestampCapture} from "../../extensions/managers/capture-timestamps/TimestampCapture.sol"; import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStakePower.sol"; +/** + * @title SelfRegisterSqrtTaskMiddleware + * @notice Middleware for managing sqrt computation tasks with self-registering operators + * @dev Uses SelfRegisterOperators for operator management because it allows permissionless registration. + * Task validation is done by a single validator chosen at task creation time because this avoids + * having to iterate over all operators, making it more gas efficient and avoiding potential DOS attacks. + */ contract SelfRegisterSqrtTaskMiddleware is SharedVaults, SelfRegisterOperators, From fb1d0a04704adf05ca9f74ff121c8f56ff112696 Mon Sep 17 00:00:00 2001 From: alrxy Date: Wed, 18 Dec 2024 03:02:19 +0400 Subject: [PATCH 14/15] feat: add forcepauseapprovalregisteroperators --- .../SelfRegisterSqrtTaskMiddleware.sol | 4 +- .../operators/ApprovalRegisterOperators.sol | 14 +- src/extensions/operators/BaseOperators.sol | 183 ++++++++++++++++++ .../ForcePauseApprovalRegisterOperators.sol | 50 +++++ .../ForcePauseSelfRegisterOperators.sol | 20 +- src/extensions/operators/Operators.sol | 136 +------------ .../operators/SelfRegisterOperators.sol | 22 +-- .../extensions/operators/IOperators.sol | 2 - 8 files changed, 271 insertions(+), 160 deletions(-) create mode 100644 src/extensions/operators/BaseOperators.sol create mode 100644 src/extensions/operators/ForcePauseApprovalRegisterOperators.sol diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index ae6ed35..6caff0f 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -13,7 +13,7 @@ import {AccessManager} from "../../managers/extendable/AccessManager.sol"; import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; import {SharedVaults} from "../../extensions/SharedVaults.sol"; -import {SelfRegisterOperators} from "../../extensions/operators/SelfRegisterOperators.sol"; +import {ForcePauseApprovalRegisterOperators} from "../../extensions/operators/ForcePauseApprovalRegisterOperators.sol"; import {ECDSASig} from "../../extensions/managers/sigs/ECDSASig.sol"; import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; @@ -30,7 +30,7 @@ import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStake */ contract SelfRegisterSqrtTaskMiddleware is SharedVaults, - SelfRegisterOperators, + ForcePauseApprovalRegisterOperators, ECDSASig, KeyManagerAddress, OwnableAccessManager, diff --git a/src/extensions/operators/ApprovalRegisterOperators.sol b/src/extensions/operators/ApprovalRegisterOperators.sol index 9bdd00f..339262d 100644 --- a/src/extensions/operators/ApprovalRegisterOperators.sol +++ b/src/extensions/operators/ApprovalRegisterOperators.sol @@ -47,7 +47,7 @@ abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalR */ function registerOperator( uint256 requestIndex - ) public checkAccess { + ) external checkAccess { RegistrationRequest memory request = getRegistrationRequest(requestIndex); _registerOperatorImpl(request.operator, request.key, request.vault); ApprovalRegisterOperatorsStorage storage $ = _getApprovalRegisterOperatorsStorage(); @@ -61,21 +61,27 @@ abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalR /** * @notice Override to prevent direct registration */ - function registerOperator(bytes memory, address, bytes memory) public override { + function registerOperator(bytes memory key, address vault, bytes memory signature) external override virtual { revert DirectRegistrationNotAllowed(); } /** * @notice Override to prevent direct registration */ - function registerOperator(address, bytes memory, address, bytes memory, bytes memory) public override { + function registerOperator( + address operator, + bytes memory key, + address vault, + bytes memory signature, + bytes memory keySignature + ) public override virtual { revert DirectRegistrationNotAllowed(); } /** * @inheritdoc IApprovalRegisterOperators */ - function requestRegisterOperator(bytes memory key, address vault, bytes memory signature) public { + function requestRegisterOperator(bytes memory key, address vault, bytes memory signature) external { _verifyKey(msg.sender, key, signature); ApprovalRegisterOperatorsStorage storage $ = _getApprovalRegisterOperatorsStorage(); $.requests.push(RegistrationRequest({operator: msg.sender, vault: vault, key: key})); diff --git a/src/extensions/operators/BaseOperators.sol b/src/extensions/operators/BaseOperators.sol new file mode 100644 index 0000000..bb79ceb --- /dev/null +++ b/src/extensions/operators/BaseOperators.sol @@ -0,0 +1,183 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; + +/** + * @title BaseOperators + * @notice Base contract for managing operator registration, keys, and vault relationships + * @dev Provides core operator management functionality with hooks for customization + */ +abstract contract BaseOperators is BaseMiddleware { + error OperatorNotRegistered(); + + /** + * @notice Internal implementation for registering an operator + * @param operator The operator address to register + * @param key The operator's public key + * @param vault Optional vault address to associate with operator + */ + function _registerOperatorImpl(address operator, bytes memory key, address vault) internal virtual { + _beforeRegisterOperator(operator, key, vault); + _registerOperator(operator); + _updateKey(operator, key); + if (vault != address(0)) { + _beforeRegisterOperatorVault(operator, vault); + _registerOperatorVault(operator, vault); + } + } + + /** + * @notice Internal implementation for unregistering an operator + * @param operator The operator address to unregister + */ + function _unregisterOperatorImpl( + address operator + ) internal virtual { + _beforeUnregisterOperator(operator); + _unregisterOperator(operator); + } + + /** + * @notice Internal implementation for pausing an operator + * @param operator The operator address to pause + */ + function _pauseOperatorImpl( + address operator + ) internal virtual { + _beforePauseOperator(operator); + _pauseOperator(operator); + } + + /** + * @notice Internal implementation for unpausing an operator + * @param operator The operator address to unpause + */ + function _unpauseOperatorImpl( + address operator + ) internal virtual { + _beforeUnpauseOperator(operator); + _unpauseOperator(operator); + } + + /** + * @notice Internal implementation for updating an operator's key + * @param operator The operator address + * @param key The new public key + */ + function _updateOperatorKeyImpl(address operator, bytes memory key) internal virtual { + _beforeUpdateOperatorKey(operator, key); + _updateKey(operator, key); + } + + /** + * @notice Internal implementation for registering an operator-vault pair + * @param operator The operator address + * @param vault The vault address to associate + * @dev Reverts if operator is not registered + */ + function _registerOperatorVaultImpl(address operator, address vault) internal virtual { + if (!_isOperatorRegistered(operator)) { + revert OperatorNotRegistered(); + } + _beforeRegisterOperatorVault(operator, vault); + _registerOperatorVault(operator, vault); + } + + /** + * @notice Internal implementation for unregistering an operator-vault pair + * @param operator The operator address + * @param vault The vault address to unregister + */ + function _unregisterOperatorVaultImpl(address operator, address vault) internal virtual { + _beforeUnregisterOperatorVault(operator, vault); + _unregisterOperatorVault(operator, vault); + } + + /** + * @notice Internal implementation for pausing an operator-vault pair + * @param operator The operator address + * @param vault The vault address to pause + */ + function _pauseOperatorVaultImpl(address operator, address vault) internal virtual { + _beforePauseOperatorVault(operator, vault); + _pauseOperatorVault(operator, vault); + } + + /** + * @notice Internal implementation for unpausing an operator-vault pair + * @param operator The operator address + * @param vault The vault address to unpause + */ + function _unpauseOperatorVaultImpl(address operator, address vault) internal virtual { + _beforeUnpauseOperatorVault(operator, vault); + _unpauseOperatorVault(operator, vault); + } + + /** + * @notice Hook called before updating an operator's key + * @param operator The operator address + * @param key The new key + */ + function _beforeUpdateOperatorKey(address operator, bytes memory key) internal virtual {} + + /** + * @notice Hook called before registering an operator + * @param operator The operator address + * @param key The operator's key + * @param vault Optional vault address + */ + function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual {} + + /** + * @notice Hook called before unregistering an operator + * @param operator The operator address + */ + function _beforeUnregisterOperator( + address operator + ) internal virtual {} + + /** + * @notice Hook called before pausing an operator + * @param operator The operator address + */ + function _beforePauseOperator( + address operator + ) internal virtual {} + + /** + * @notice Hook called before unpausing an operator + * @param operator The operator address + */ + function _beforeUnpauseOperator( + address operator + ) internal virtual {} + + /** + * @notice Hook called before registering an operator-vault pair + * @param operator The operator address + * @param vault The vault address + */ + function _beforeRegisterOperatorVault(address operator, address vault) internal virtual {} + + /** + * @notice Hook called before unregistering an operator-vault pair + * @param operator The operator address + * @param vault The vault address + */ + function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual {} + + /** + * @notice Hook called before pausing an operator-vault pair + * @param operator The operator address + * @param vault The vault address + */ + function _beforePauseOperatorVault(address operator, address vault) internal virtual {} + + /** + * @notice Hook called before unpausing an operator-vault pair + * @param operator The operator address + * @param vault The vault address + */ + function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual {} +} diff --git a/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol b/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol new file mode 100644 index 0000000..83a2697 --- /dev/null +++ b/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.25; + +import {ApprovalRegisterOperators} from "./ApprovalRegisterOperators.sol"; +import {SelfRegisterOperators} from "./SelfRegisterOperators.sol"; +import {ForcePauseSelfRegisterOperators} from "./ForcePauseSelfRegisterOperators.sol"; +import {BaseOperators} from "./BaseOperators.sol"; +/** + * @title ForcePauseSelfRegisterOperators + * @notice Extension of SelfRegisterOperators that allows authorized addresses to forcefully pause operators + * @dev Implements force pause functionality and prevents unpausing of force-paused operators + */ +abstract contract ForcePauseApprovalRegisterOperators is ForcePauseSelfRegisterOperators, ApprovalRegisterOperators { + uint64 public constant ForcePauseApprovalRegisterOperators_VERSION = 1; + + function registerOperator( + bytes memory key, + address vault, + bytes memory signature + ) external override(ApprovalRegisterOperators, SelfRegisterOperators) { + revert DirectRegistrationNotAllowed(); + } + + // Override the registerOperator function to resolve ambiguity + function registerOperator( + address operator, + bytes memory key, + address vault, + bytes memory signature, + bytes memory keySignature + ) public override(ApprovalRegisterOperators, SelfRegisterOperators) { + revert DirectRegistrationNotAllowed(); + } + + function _beforeUnpauseOperator(address operator) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + super._beforeUnpauseOperator(operator); + } + + function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + super._beforeUnpauseOperatorVault(operator, vault); + } + + function _beforeUnregisterOperator(address operator) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + super._beforeUnregisterOperator(operator); + } + + function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual override(BaseOperators, ForcePauseSelfRegisterOperators) { + super._beforeUnregisterOperatorVault(operator, vault); + } +} diff --git a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol index 8e01056..d8c474a 100644 --- a/src/extensions/operators/ForcePauseSelfRegisterOperators.sol +++ b/src/extensions/operators/ForcePauseSelfRegisterOperators.sol @@ -34,7 +34,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor */ function forcePauseOperator( address operator - ) public checkAccess { + ) external checkAccess { ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePaused[operator] = true; _beforePauseOperator(operator); @@ -48,7 +48,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor */ function forceUnpauseOperator( address operator - ) public checkAccess { + ) external checkAccess { ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePaused[operator] = false; _beforeUnpauseOperator(operator); @@ -60,7 +60,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor */ function forceUnregisterOperator( address operator - ) public checkAccess { + ) external checkAccess { _beforeUnregisterOperator(operator); _unregisterOperator(operator); } @@ -68,7 +68,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor /** * @inheritdoc IForcePauseSelfRegisterOperators */ - function forcePauseOperatorVault(address operator, address vault) public checkAccess { + function forcePauseOperatorVault(address operator, address vault) external checkAccess { ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePausedVault[operator][vault] = true; _beforePauseOperatorVault(operator, vault); @@ -80,7 +80,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor /** * @inheritdoc IForcePauseSelfRegisterOperators */ - function forceUnpauseOperatorVault(address operator, address vault) public checkAccess { + function forceUnpauseOperatorVault(address operator, address vault) external checkAccess { ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); $.forcePausedVault[operator][vault] = false; _beforeUnpauseOperatorVault(operator, vault); @@ -90,7 +90,7 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor /** * @inheritdoc IForcePauseSelfRegisterOperators */ - function forceUnregisterOperatorVault(address operator, address vault) public checkAccess { + function forceUnregisterOperatorVault(address operator, address vault) external checkAccess { _beforeUnregisterOperatorVault(operator, vault); _unregisterOperatorVault(operator, vault); } @@ -102,9 +102,9 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor function _beforeUnpauseOperator( address operator ) internal virtual override { + super._beforeUnpauseOperator(operator); ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); if ($.forcePaused[operator]) revert OperatorForcePaused(); - super._beforeUnpauseOperator(operator); } /** @@ -114,9 +114,9 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor function _beforeUnregisterOperator( address operator ) internal virtual override { + super._beforeUnregisterOperator(operator); ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); if ($.forcePaused[operator]) revert OperatorForcePaused(); - super._beforeUnregisterOperator(operator); } /** @@ -125,9 +125,9 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor * @param vault The vault address */ function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual override { + super._beforeUnpauseOperatorVault(operator, vault); ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); if ($.forcePausedVault[operator][vault]) revert OperatorVaultForcePaused(); - super._beforeUnpauseOperatorVault(operator, vault); } /** @@ -136,8 +136,8 @@ abstract contract ForcePauseSelfRegisterOperators is SelfRegisterOperators, IFor * @param vault The vault address */ function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual override { + super._beforeUnregisterOperatorVault(operator, vault); ForcePauseSelfRegisterOperatorsStorage storage $ = _getForcePauseStorage(); if ($.forcePausedVault[operator][vault]) revert OperatorVaultForcePaused(); - super._beforeUnregisterOperatorVault(operator, vault); } } diff --git a/src/extensions/operators/Operators.sol b/src/extensions/operators/Operators.sol index 3e90d6e..cf459eb 100644 --- a/src/extensions/operators/Operators.sol +++ b/src/extensions/operators/Operators.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.25; -import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; +import {BaseOperators} from "./BaseOperators.sol"; import {IOperators} from "../../interfaces/extensions/operators/IOperators.sol"; /** @@ -9,42 +9,25 @@ import {IOperators} from "../../interfaces/extensions/operators/IOperators.sol"; * @notice Base contract for managing operator registration, keys, and vault relationships * @dev Provides core operator management functionality with hooks for customization */ -abstract contract Operators is BaseMiddleware, IOperators { +abstract contract Operators is BaseOperators, IOperators { uint64 public constant Operators_VERSION = 1; /** * @inheritdoc IOperators */ - function registerOperator(address operator, bytes memory key, address vault) external virtual checkAccess { + function registerOperator(address operator, bytes memory key, address vault) external checkAccess { _registerOperatorImpl(operator, key, vault); } - function _registerOperatorImpl(address operator, bytes memory key, address vault) internal virtual { - _beforeRegisterOperator(operator, key, vault); - _registerOperator(operator); - _updateKey(operator, key); - if (vault != address(0)) { - _beforeRegisterOperatorVault(operator, vault); - _registerOperatorVault(operator, vault); - } - } - /** * @inheritdoc IOperators */ function unregisterOperator( address operator - ) external virtual checkAccess { + ) external checkAccess { _unregisterOperatorImpl(operator); } - function _unregisterOperatorImpl( - address operator - ) internal virtual { - _beforeUnregisterOperator(operator); - _unregisterOperator(operator); - } - /** * @inheritdoc IOperators */ @@ -54,13 +37,6 @@ abstract contract Operators is BaseMiddleware, IOperators { _pauseOperatorImpl(operator); } - function _pauseOperatorImpl( - address operator - ) internal virtual { - _beforePauseOperator(operator); - _pauseOperator(operator); - } - /** * @inheritdoc IOperators */ @@ -70,40 +46,20 @@ abstract contract Operators is BaseMiddleware, IOperators { _unpauseOperatorImpl(operator); } - function _unpauseOperatorImpl( - address operator - ) internal virtual { - _beforeUnpauseOperator(operator); - _unpauseOperator(operator); - } - /** * @inheritdoc IOperators */ function updateOperatorKey(address operator, bytes memory key) external virtual checkAccess { _updateOperatorKeyImpl(operator, key); } - - function _updateOperatorKeyImpl(address operator, bytes memory key) internal virtual { - _beforeUpdateOperatorKey(operator, key); - _updateKey(operator, key); - } - /** * @inheritdoc IOperators */ + function registerOperatorVault(address operator, address vault) external virtual checkAccess { _registerOperatorVaultImpl(operator, vault); } - function _registerOperatorVaultImpl(address operator, address vault) internal virtual { - if (!_isOperatorRegistered(operator)) { - revert OperatorNotRegistered(); - } - _beforeRegisterOperatorVault(operator, vault); - _registerOperatorVault(operator, vault); - } - /** * @inheritdoc IOperators */ @@ -111,11 +67,6 @@ abstract contract Operators is BaseMiddleware, IOperators { _unregisterOperatorVaultImpl(operator, vault); } - function _unregisterOperatorVaultImpl(address operator, address vault) internal virtual { - _beforeUnregisterOperatorVault(operator, vault); - _unregisterOperatorVault(operator, vault); - } - /** * @inheritdoc IOperators */ @@ -123,87 +74,10 @@ abstract contract Operators is BaseMiddleware, IOperators { _pauseOperatorVaultImpl(operator, vault); } - function _pauseOperatorVaultImpl(address operator, address vault) internal virtual { - _beforePauseOperatorVault(operator, vault); - _pauseOperatorVault(operator, vault); - } - /** * @inheritdoc IOperators */ function unpauseOperatorVault(address operator, address vault) external virtual checkAccess { _unpauseOperatorVaultImpl(operator, vault); } - - function _unpauseOperatorVaultImpl(address operator, address vault) internal virtual { - _beforeUnpauseOperatorVault(operator, vault); - _unpauseOperatorVault(operator, vault); - } - - /** - * @notice Hook called before updating an operator's key - * @param operator The operator address - * @param key The new key - */ - function _beforeUpdateOperatorKey(address operator, bytes memory key) internal virtual {} - - /** - * @notice Hook called before registering an operator - * @param operator The operator address - * @param key The operator's key - * @param vault Optional vault address - */ - function _beforeRegisterOperator(address operator, bytes memory key, address vault) internal virtual {} - - /** - * @notice Hook called before unregistering an operator - * @param operator The operator address - */ - function _beforeUnregisterOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before pausing an operator - * @param operator The operator address - */ - function _beforePauseOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before unpausing an operator - * @param operator The operator address - */ - function _beforeUnpauseOperator( - address operator - ) internal virtual {} - - /** - * @notice Hook called before registering an operator-vault pair - * @param operator The operator address - * @param vault The vault address - */ - function _beforeRegisterOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before unregistering an operator-vault pair - * @param operator The operator address - * @param vault The vault address - */ - function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before pausing an operator-vault pair - * @param operator The operator address - * @param vault The vault address - */ - function _beforePauseOperatorVault(address operator, address vault) internal virtual {} - - /** - * @notice Hook called before unpausing an operator-vault pair - * @param operator The operator address - * @param vault The vault address - */ - function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual {} } diff --git a/src/extensions/operators/SelfRegisterOperators.sol b/src/extensions/operators/SelfRegisterOperators.sol index f7d3b50..385bd7d 100644 --- a/src/extensions/operators/SelfRegisterOperators.sol +++ b/src/extensions/operators/SelfRegisterOperators.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.25; -import {Operators} from "./Operators.sol"; +import {BaseOperators} from "./BaseOperators.sol"; import {SigManager} from "../../managers/extendable/SigManager.sol"; import {EIP712Upgradeable} from "@openzeppelin/contracts-upgradeable/utils/cryptography/EIP712Upgradeable.sol"; import {SignatureChecker} from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; @@ -13,7 +13,7 @@ import {ISelfRegisterOperators} from "../../interfaces/extensions/operators/ISel * @dev Extends BaseMiddleware, SigManager, and EIP712Upgradeable to provide signature-based operator management * @dev CAUTION: If activeOperators functionality is needed, use ApprovalRegisterOperators instead to prevent DOS attacks */ -abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradeable, ISelfRegisterOperators { +abstract contract SelfRegisterOperators is BaseOperators, SigManager, EIP712Upgradeable, ISelfRegisterOperators { uint64 public constant SelfRegisterOperators_VERSION = 1; // EIP-712 TypeHash constants @@ -68,7 +68,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea /** * @inheritdoc ISelfRegisterOperators */ - function registerOperator(bytes memory key, address vault, bytes memory signature) public virtual override { + function registerOperator(bytes memory key, address vault, bytes memory signature) external virtual { _verifyKey(msg.sender, key, signature); _registerOperatorImpl(msg.sender, key, vault); } @@ -96,7 +96,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea /** * @inheritdoc ISelfRegisterOperators */ - function unregisterOperator() public override { + function unregisterOperator() external override { _unregisterOperatorImpl(msg.sender); } @@ -114,7 +114,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea /** * @inheritdoc ISelfRegisterOperators */ - function pauseOperator() public override { + function pauseOperator() external override { _pauseOperatorImpl(msg.sender); } @@ -132,7 +132,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea /** * @inheritdoc ISelfRegisterOperators */ - function unpauseOperator() public override { + function unpauseOperator() external override { _unpauseOperatorImpl(msg.sender); } @@ -150,7 +150,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea /** * @inheritdoc ISelfRegisterOperators */ - function updateOperatorKey(bytes memory key, bytes memory signature) public override { + function updateOperatorKey(bytes memory key, bytes memory signature) external override { _verifyKey(msg.sender, key, signature); _updateOperatorKeyImpl(msg.sender, key); } @@ -179,7 +179,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea */ function registerOperatorVault( address vault - ) public override { + ) external override { _registerOperatorVaultImpl(msg.sender, vault); } @@ -204,7 +204,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea */ function unregisterOperatorVault( address vault - ) public override { + ) external override { _unregisterOperatorVaultImpl(msg.sender, vault); } @@ -226,7 +226,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea */ function pauseOperatorVault( address vault - ) public override { + ) external override { _pauseOperatorVaultImpl(msg.sender, vault); } @@ -248,7 +248,7 @@ abstract contract SelfRegisterOperators is Operators, SigManager, EIP712Upgradea */ function unpauseOperatorVault( address vault - ) public override { + ) external override { _unpauseOperatorVaultImpl(msg.sender, vault); } diff --git a/src/interfaces/extensions/operators/IOperators.sol b/src/interfaces/extensions/operators/IOperators.sol index 1cf4280..81c5a30 100644 --- a/src/interfaces/extensions/operators/IOperators.sol +++ b/src/interfaces/extensions/operators/IOperators.sol @@ -6,8 +6,6 @@ pragma solidity ^0.8.25; * @notice Interface for managing operator registration, keys, and vault relationships */ interface IOperators { - error OperatorNotRegistered(); - /** * @notice Registers a new operator with an optional vault association * @param operator The address of the operator to register From a7907c48c6e6945c3d1e796211136b0bd343835d Mon Sep 17 00:00:00 2001 From: alrxy Date: Wed, 18 Dec 2024 03:03:10 +0400 Subject: [PATCH 15/15] fix: lint and revert operators in selfregistersqrtaskmiddleware --- .../SelfRegisterSqrtTaskMiddleware.sol | 4 ++-- .../operators/ApprovalRegisterOperators.sol | 4 ++-- .../ForcePauseApprovalRegisterOperators.sol | 19 +++++++++++++++---- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol index 6caff0f..ae6ed35 100644 --- a/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol +++ b/src/examples/sqrt-task-network/SelfRegisterSqrtTaskMiddleware.sol @@ -13,7 +13,7 @@ import {AccessManager} from "../../managers/extendable/AccessManager.sol"; import {BaseMiddleware} from "../../middleware/BaseMiddleware.sol"; import {SharedVaults} from "../../extensions/SharedVaults.sol"; -import {ForcePauseApprovalRegisterOperators} from "../../extensions/operators/ForcePauseApprovalRegisterOperators.sol"; +import {SelfRegisterOperators} from "../../extensions/operators/SelfRegisterOperators.sol"; import {ECDSASig} from "../../extensions/managers/sigs/ECDSASig.sol"; import {OwnableAccessManager} from "../../extensions/managers/access/OwnableAccessManager.sol"; @@ -30,7 +30,7 @@ import {EqualStakePower} from "../../extensions/managers/stake-powers/EqualStake */ contract SelfRegisterSqrtTaskMiddleware is SharedVaults, - ForcePauseApprovalRegisterOperators, + SelfRegisterOperators, ECDSASig, KeyManagerAddress, OwnableAccessManager, diff --git a/src/extensions/operators/ApprovalRegisterOperators.sol b/src/extensions/operators/ApprovalRegisterOperators.sol index 339262d..dda290e 100644 --- a/src/extensions/operators/ApprovalRegisterOperators.sol +++ b/src/extensions/operators/ApprovalRegisterOperators.sol @@ -61,7 +61,7 @@ abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalR /** * @notice Override to prevent direct registration */ - function registerOperator(bytes memory key, address vault, bytes memory signature) external override virtual { + function registerOperator(bytes memory key, address vault, bytes memory signature) external virtual override { revert DirectRegistrationNotAllowed(); } @@ -74,7 +74,7 @@ abstract contract ApprovalRegisterOperators is SelfRegisterOperators, IApprovalR address vault, bytes memory signature, bytes memory keySignature - ) public override virtual { + ) public virtual override { revert DirectRegistrationNotAllowed(); } diff --git a/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol b/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol index 83a2697..5ec1e29 100644 --- a/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol +++ b/src/extensions/operators/ForcePauseApprovalRegisterOperators.sol @@ -10,6 +10,7 @@ import {BaseOperators} from "./BaseOperators.sol"; * @notice Extension of SelfRegisterOperators that allows authorized addresses to forcefully pause operators * @dev Implements force pause functionality and prevents unpausing of force-paused operators */ + abstract contract ForcePauseApprovalRegisterOperators is ForcePauseSelfRegisterOperators, ApprovalRegisterOperators { uint64 public constant ForcePauseApprovalRegisterOperators_VERSION = 1; @@ -32,19 +33,29 @@ abstract contract ForcePauseApprovalRegisterOperators is ForcePauseSelfRegisterO revert DirectRegistrationNotAllowed(); } - function _beforeUnpauseOperator(address operator) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + function _beforeUnpauseOperator( + address operator + ) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { super._beforeUnpauseOperator(operator); } - function _beforeUnpauseOperatorVault(address operator, address vault) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + function _beforeUnpauseOperatorVault( + address operator, + address vault + ) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { super._beforeUnpauseOperatorVault(operator, vault); } - function _beforeUnregisterOperator(address operator) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { + function _beforeUnregisterOperator( + address operator + ) internal virtual override(ForcePauseSelfRegisterOperators, BaseOperators) { super._beforeUnregisterOperator(operator); } - function _beforeUnregisterOperatorVault(address operator, address vault) internal virtual override(BaseOperators, ForcePauseSelfRegisterOperators) { + function _beforeUnregisterOperatorVault( + address operator, + address vault + ) internal virtual override(BaseOperators, ForcePauseSelfRegisterOperators) { super._beforeUnregisterOperatorVault(operator, vault); } }