From 716f278a7e0ff53430c310307069cadce74b222d Mon Sep 17 00:00:00 2001
From: Jay Paik <jay@alchemy.com>
Date: Sun, 7 Jan 2024 17:27:24 -0500
Subject: [PATCH] fix: [spearbit-66] remove redundant 0 check

---
 src/plugins/session/ISessionKeyPlugin.sol      |  1 -
 src/plugins/session/SessionKeyPlugin.sol       | 14 +++++---------
 .../SessionKeyPluginWithMultiOwner.t.sol       | 18 ++++++++++++++++++
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/plugins/session/ISessionKeyPlugin.sol b/src/plugins/session/ISessionKeyPlugin.sol
index 3eaf95cb..6f71cf95 100644
--- 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
index ce803de3..dfaa8803 100644
--- 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
index 3a5b57f4..98ba248e 100644
--- 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");