Enhancement of isValidSigner Function in IPAccount

contracts/IPAccountImpl.sol

@@ -59,15 +59,26 @@ contract IPAccountImpl is ERC6551, IPAccountStorage, IIPAccount {
         return super.token();
     }
 
-    /// @notice Checks if the signer is valid for the given data
+    /// @notice Checks if the signer is valid for executing specific actions on behalf of the IP Account.
     /// @param signer The signer to check
-    /// @param data The data to check against
-    /// @return The function selector if the signer is valid, 0 otherwise
+    /// @param data The data to be checked. The data should be encoded as `abi.encode(address to, bytes calldata)`,
+    /// where `address to` is the recipient and `bytes calldata` is the calldata passed to the recipient.
+    /// If `data.length == 0`, it is also considered valid, implying that the signer is valid for all actions.
+    /// @return result The function selector if the signer is valid, 0 otherwise
     function isValidSigner(
         address signer,
         bytes calldata data
-    ) public view override(ERC6551, IIPAccount) returns (bytes4) {
-        return super.isValidSigner(signer, data);
+    ) public view override(ERC6551, IIPAccount) returns (bytes4 result) {
+        result = bytes4(0);
+        address to = address(0);
+        bytes memory callData = "";
+        if (data.length > 0) {
+            if (data.length < 32) revert Errors.IPAccount__InvalidCalldata();
+            (to, callData) = abi.decode(data, (address, bytes));
+        }
+        if (this.isValidSigner(signer, to, callData)) {
+            result = IERC6551Account.isValidSigner.selector;
+        }
     }
 
     /// @notice Returns the owner of the IP Account.
@@ -86,7 +97,7 @@ contract IPAccountImpl is ERC6551, IPAccountStorage, IIPAccount {
     /// @param to The recipient of the transaction
     /// @param data The calldata to check against
     /// @return bool is true if the signer is valid, false otherwise
-    function _isValidSigner(address signer, address to, bytes calldata data) internal view returns (bool) {
+    function isValidSigner(address signer, address to, bytes calldata data) public view returns (bool) {
         if (data.length > 0 && data.length < 4) {
             revert Errors.IPAccount__InvalidCalldata();
         }
@@ -193,7 +204,7 @@ contract IPAccountImpl is ERC6551, IPAccountStorage, IIPAccount {
         uint256 value,
         bytes calldata data
     ) internal returns (bytes memory result) {
-        require(_isValidSigner(signer, to, data), "Invalid signer");
+        require(isValidSigner(signer, to, data), "Invalid signer");
 
         bool success;
         (success, result) = to.call{ value: value }(data);
@@ -228,7 +239,7 @@ contract IPAccountImpl is ERC6551, IPAccountStorage, IIPAccount {
         bytes32 extraData,
         bytes calldata context
     ) internal view override returns (bool) {
-        return _isValidSigner(signer, address(uint160(uint256(extraData))), context);
+        return isValidSigner(signer, address(uint160(uint256(extraData))), context);
     }
 
     /// @dev Override Solady EIP712 function and return EIP712 domain name for IPAccount.

test/foundry/IPAccount.t.sol

@@ -116,6 +116,88 @@ contract IPAccountTest is BaseTest {
         assertEq(ipAccount.state(), expectedState);
     }
 
+    function test_IPAccount_isValidSigner() public {
+        address owner = vm.addr(1);
+        uint256 tokenId = 100;
+
+        mockNFT.mintId(owner, tokenId);
+
+        address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
+
+        IIPAccount ipAccount = IIPAccount(payable(account));
+
+        // test isValidSigner with owner and empty data
+        assertEq(ipAccount.isValidSigner(owner, ""), IERC6551Account.isValidSigner.selector);
+
+        // test isValidSigner with owner and encoded "to" address and "calldata" as data
+        bytes memory data = abi.encode(address(module), abi.encodeWithSignature("executeSuccessfully(string)", "test"));
+        assertEq(ipAccount.isValidSigner(owner, data), IERC6551Account.isValidSigner.selector);
+
+        assertEq(
+            ipAccount.isValidSigner(owner, abi.encode(address(module), "")),
+            IERC6551Account.isValidSigner.selector
+        );
+
+        // Transfer token to new owner and make sure account owner changes
+        address nonOwner = vm.addr(2);
+        vm.prank(owner);
+        mockNFT.transferFrom(owner, nonOwner, tokenId);
+        assertEq(ipAccount.isValidSigner(nonOwner, data), IERC6551Account.isValidSigner.selector);
+    }
+
+    function test_IPAccount_isValidSigner_revert_InvalidInputs() public {
+        address owner = vm.addr(1);
+        uint256 tokenId = 100;
+
+        mockNFT.mintId(owner, tokenId);
+
+        address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId);
+
+        IIPAccount ipAccount = IIPAccount(payable(account));
+
+        vm.expectRevert(Errors.IPAccount__InvalidCalldata.selector);
+        ipAccount.isValidSigner(address(module), bytes("123"));
+
+        vm.expectRevert(Errors.IPAccount__InvalidCalldata.selector);
+        ipAccount.isValidSigner(address(module), bytes("123456789"));
+
+        vm.expectRevert();
+        ipAccount.isValidSigner(address(module), abi.encode(address(0x123)));
+
+        vm.expectRevert();
+        ipAccount.isValidSigner(address(module), abi.encodeWithSignature("executeSuccessfully(string)", "test"));
+
+        vm.expectRevert(Errors.IPAccount__InvalidCalldata.selector);
+        ipAccount.isValidSigner(owner, abi.encode(address(module), bytes("123")));
+
+        // test isValidSigner with non-owner and empty data
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                Errors.AccessController__PermissionDenied.selector,
+                address(ipAccount),
+                address(module),
+                address(0),
+                bytes4(0)
+            )
+        );
+        ipAccount.isValidSigner(address(module), "");
+
+        // test isValidSigner with non-owner and encoded "to" address and "calldata" as data
+        bytes memory data = abi.encode(address(module), abi.encodeWithSignature("executeSuccessfully(string)", "test"));
+        address nonOwner = vm.addr(2);
+        vm.prank(nonOwner);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                Errors.AccessController__PermissionDenied.selector,
+                address(ipAccount),
+                nonOwner,
+                address(module),
+                module.executeSuccessfully.selector
+            )
+        );
+        ipAccount.isValidSigner(nonOwner, data);
+    }
+
     function test_IPAccount_revert_NonOwnerNoPermissionToExecute() public {
         address owner = vm.addr(1);
         uint256 tokenId = 100;