From c7d2f50caace8a2b6e46f0bce59b68b6457272c7 Mon Sep 17 00:00:00 2001 From: QUAQ Date: Mon, 12 Feb 2024 14:52:42 -0600 Subject: [PATCH 1/2] feat: cancel salt --- src/contracts/core/AVSDirectory.sol | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/contracts/core/AVSDirectory.sol b/src/contracts/core/AVSDirectory.sol index 98f7d9797..9a856da8e 100644 --- a/src/contracts/core/AVSDirectory.sol +++ b/src/contracts/core/AVSDirectory.sol @@ -127,6 +127,14 @@ contract AVSDirectory is emit AVSMetadataURIUpdated(msg.sender, metadataURI); } + /** + * @notice Called by an operator to cancel a salt that has been used to register with an AVS. + * @param salt A unique and single use value associated with the approver signature. + */ + function cancelSalt(bytes32 salt) external { + operatorSaltIsSpent[msg.sender][salt] = true; + } + /******************************************************************************* VIEW FUNCTIONS *******************************************************************************/ From 2f921f0845762c40dc5dbe73efe177a0dea94936 Mon Sep 17 00:00:00 2001 From: wadealexc Date: Tue, 13 Feb 2024 20:52:19 +0000 Subject: [PATCH 2/2] fix: require that salt cannot be cancelled twice --- docs/core/AVSDirectory.md | 16 +++++- src/contracts/core/AVSDirectory.sol | 1 + src/test/unit/AVSDirectoryUnit.t.sol | 81 ++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/docs/core/AVSDirectory.md b/docs/core/AVSDirectory.md index 3f121d856..e025345b5 100644 --- a/docs/core/AVSDirectory.md +++ b/docs/core/AVSDirectory.md @@ -63,4 +63,18 @@ Allows the caller (an AVS) to deregister an `operator` with itself * `operator` MUST already be registered with the AVS *As of M2*: -* Operator registration/deregistration does not have any sort of consequences for the Operator or its shares. Eventually, this will tie into payments for services and slashing for misbehavior. \ No newline at end of file +* Operator registration/deregistration does not have any sort of consequences for the Operator or its shares. Eventually, this will tie into payments for services and slashing for misbehavior. + +#### `cancelSalt` + +```solidity +function cancelSalt(bytes32 salt) external +``` + +Allows the caller (an Operator) to cancel a signature salt before it is used to register for an AVS. + +*Effects*: +* Sets `operatorSaltIsSpent[msg.sender][salt]` to `true` + +*Requirements*: +* Salt MUST NOT already be cancelled \ No newline at end of file diff --git a/src/contracts/core/AVSDirectory.sol b/src/contracts/core/AVSDirectory.sol index 9a856da8e..0391aac6b 100644 --- a/src/contracts/core/AVSDirectory.sol +++ b/src/contracts/core/AVSDirectory.sol @@ -132,6 +132,7 @@ contract AVSDirectory is * @param salt A unique and single use value associated with the approver signature. */ function cancelSalt(bytes32 salt) external { + require(!operatorSaltIsSpent[msg.sender][salt], "AVSDirectory.cancelSalt: cannot cancel spent salt"); operatorSaltIsSpent[msg.sender][salt] = true; } diff --git a/src/test/unit/AVSDirectoryUnit.t.sol b/src/test/unit/AVSDirectoryUnit.t.sol index f37beeca1..d18b98fe9 100644 --- a/src/test/unit/AVSDirectoryUnit.t.sol +++ b/src/test/unit/AVSDirectoryUnit.t.sol @@ -271,4 +271,85 @@ contract AVSDirectoryUnitTests_operatorAVSRegisterationStatus is AVSDirectoryUni avsDirectory.registerOperatorToAVS(operator, operatorSignature); cheats.stopPrank(); } + + /// @notice Checks that cancelSalt updates the operatorSaltIsSpent mapping correctly + function testFuzz_cancelSalt(bytes32 salt) public { + address operator = cheats.addr(delegationSignerPrivateKey); + assertFalse(delegationManager.isOperator(operator), "bad test setup"); + _registerOperatorWithBaseDetails(operator); + + assertFalse(avsDirectory.operatorSaltIsSpent(operator, salt), "bad test setup"); + assertFalse(avsDirectory.operatorSaltIsSpent(defaultAVS, salt), "bad test setup"); + + cheats.prank(operator); + avsDirectory.cancelSalt(salt); + + assertTrue(avsDirectory.operatorSaltIsSpent(operator, salt), "salt was not successfully cancelled"); + assertFalse(avsDirectory.operatorSaltIsSpent(defaultAVS, salt), "salt should only be cancelled for the operator"); + + bytes32 newSalt; + unchecked { newSalt = bytes32(uint(salt) + 1); } + + assertFalse(salt == newSalt, "bad test setup"); + + cheats.prank(operator); + avsDirectory.cancelSalt(newSalt); + + assertTrue(avsDirectory.operatorSaltIsSpent(operator, salt), "original salt should still be cancelled"); + assertTrue(avsDirectory.operatorSaltIsSpent(operator, newSalt), "new salt should be cancelled"); + } + + /// @notice Verifies that registration fails when the salt has been cancelled via cancelSalt + function testFuzz_revert_whenRegisteringWithCancelledSalt(bytes32 salt) public { + address operator = cheats.addr(delegationSignerPrivateKey); + assertFalse(delegationManager.isOperator(operator), "bad test setup"); + _registerOperatorWithBaseDetails(operator); + + uint256 expiry = type(uint256).max; + ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature = + _getOperatorSignature(delegationSignerPrivateKey, operator, defaultAVS, salt, expiry); + + cheats.prank(operator); + avsDirectory.cancelSalt(salt); + + cheats.expectRevert("AVSDirectory.registerOperatorToAVS: salt already spent"); + cheats.prank(defaultAVS); + avsDirectory.registerOperatorToAVS(operator, operatorSignature); + } + + /// @notice Verifies that an operator cannot cancel the same salt twice + function testFuzz_revert_whenSaltCancelledTwice(bytes32 salt) public { + address operator = cheats.addr(delegationSignerPrivateKey); + assertFalse(delegationManager.isOperator(operator), "bad test setup"); + _registerOperatorWithBaseDetails(operator); + + uint256 expiry = type(uint256).max; + ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature = + _getOperatorSignature(delegationSignerPrivateKey, operator, defaultAVS, salt, expiry); + + cheats.startPrank(operator); + avsDirectory.cancelSalt(salt); + + cheats.expectRevert("AVSDirectory.cancelSalt: cannot cancel spent salt"); + avsDirectory.cancelSalt(salt); + cheats.stopPrank(); + } + + /// @notice Verifies that an operator cannot cancel the same salt twice + function testFuzz_revert_whenCancellingSaltUsedToRegister(bytes32 salt) public { + address operator = cheats.addr(delegationSignerPrivateKey); + assertFalse(delegationManager.isOperator(operator), "bad test setup"); + _registerOperatorWithBaseDetails(operator); + + uint256 expiry = type(uint256).max; + ISignatureUtils.SignatureWithSaltAndExpiry memory operatorSignature = + _getOperatorSignature(delegationSignerPrivateKey, operator, defaultAVS, salt, expiry); + + cheats.prank(defaultAVS); + avsDirectory.registerOperatorToAVS(operator, operatorSignature); + + cheats.prank(operator); + cheats.expectRevert("AVSDirectory.cancelSalt: cannot cancel spent salt"); + avsDirectory.cancelSalt(salt); + } }