fix: [spearbit-66] remove redundant 0 check

alchemyplatform · Jan 8, 2024 · 801d978 · 801d978
1 parent 8b20450
commit 801d978
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 10 deletions.
diff --git a/src/plugins/session/ISessionKeyPlugin.sol b/src/plugins/session/ISessionKeyPlugin.sol
@@ -11,7 +11,6 @@ interface ISessionKeyPlugin {
 
     error InvalidSessionKey(address sessionKey);
     error NotAuthorized(address caller);
-    error SessionKeyAlreadyExists(address sessionKey);
     error SessionKeyNotFound(address sessionKey);
     error UnableToRemove(address sessionKey);
 

diff --git a/src/plugins/session/SessionKeyPlugin.sol b/src/plugins/session/SessionKeyPlugin.sol
@@ -105,8 +105,9 @@ contract SessionKeyPlugin is BasePlugin, ISessionKeyPlugin {
 
         length = sessionKeysToAdd.length;
         for (uint256 i = 0; i < length;) {
+            // This also checks that sessionKeysToAdd[i] is not zero.
             if (!_sessionKeys.tryAdd(msg.sender, CastLib.toSetValue(sessionKeysToAdd[i]))) {
-                revert SessionKeyAlreadyExists(sessionKeysToAdd[i]);
+                revert InvalidSessionKey(sessionKeysToAdd[i]);
             }
 
             unchecked {
@@ -184,14 +185,9 @@ contract SessionKeyPlugin is BasePlugin, ISessionKeyPlugin {
 
         uint256 length = sessionKeysToAdd.length;
         for (uint256 i = 0; i < length;) {
-            address sessionKey = sessionKeysToAdd[i];
-
-            if (sessionKey == address(0)) {
-                revert InvalidSessionKey(sessionKey);
-            }
-
-            if (!_sessionKeys.tryAdd(msg.sender, CastLib.toSetValue(sessionKey))) {
-                revert SessionKeyAlreadyExists(sessionKeysToAdd[i]);
+            // This also checks that sessionKeysToAdd[i] is not zero.
+            if (!_sessionKeys.tryAdd(msg.sender, CastLib.toSetValue(sessionKeysToAdd[i]))) {
+                revert InvalidSessionKey(sessionKeysToAdd[i]);
             }
 
             unchecked {

diff --git a/test/plugin/session/SessionKeyPluginWithMultiOwner.t.sol b/test/plugin/session/SessionKeyPluginWithMultiOwner.t.sol
@@ -97,6 +97,24 @@ contract SessionKeyPluginWithMultiOwnerTest is Test {
         assertEq(sessionKeys[0], sessionKeysToAdd[0]);
     }
 
+    function test_sessionKey_addKeyFailure() public {
+        vm.startPrank(owner1);
+
+        address[] memory sessionKeysToAdd = new address[](1);
+        vm.expectRevert(abi.encodeWithSelector(ISessionKeyPlugin.InvalidSessionKey.selector, address(0)));
+        SessionKeyPlugin(address(account1)).updateSessionKeys(
+            sessionKeysToAdd, new SessionKeyPlugin.SessionKeyToRemove[](0)
+        );
+
+        sessionKeysToAdd = new address[](2);
+        sessionKeysToAdd[0] = makeAddr("sessionKey1");
+        sessionKeysToAdd[1] = sessionKeysToAdd[0];
+        vm.expectRevert(abi.encodeWithSelector(ISessionKeyPlugin.InvalidSessionKey.selector, sessionKeysToAdd[0]));
+        SessionKeyPlugin(address(account1)).updateSessionKeys(
+            sessionKeysToAdd, new SessionKeyPlugin.SessionKeyToRemove[](0)
+        );
+    }
+
     function test_sessionKey_addAndRemoveKeys() public {
         address[] memory sessionKeysToAdd = new address[](2);
         sessionKeysToAdd[0] = makeAddr("sessionKey1");