Loosen restrictions on setContractURI for Account contracts (#519)

* Loosen restrictions on setContractURI for Account contracts * add unit tests * test: account signer not approved for setContractURI --------- Co-authored-by: Krishang <[email protected]>
thirdweb-dev · Oct 3, 2023 · 781a781 · 781a781
1 parent 6e260f2
commit 781a781
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 2 deletions.
diff --git a/contracts/prebuilts/account/non-upgradeable/Account.sol b/contracts/prebuilts/account/non-upgradeable/Account.sol
@@ -142,6 +142,6 @@ contract Account is AccountCore, ContractMetadata, ERC1271, ERC721Holder, ERC115
 
     /// @dev Returns whether contract metadata can be set in the given execution context.
     function _canSetContractURI() internal view virtual override returns (bool) {
-        return isAdmin(msg.sender);
+        return isAdmin(msg.sender) || msg.sender == address(this);
     }
 }
diff --git a/contracts/prebuilts/account/utils/AccountExtension.sol b/contracts/prebuilts/account/utils/AccountExtension.sol
@@ -144,7 +144,7 @@ contract AccountExtension is ContractMetadata, ERC1271, AccountPermissions, ERC7
 
     /// @dev Returns whether contract metadata can be set in the given execution context.
     function _canSetContractURI() internal view virtual override returns (bool) {
-        return isAdmin(msg.sender);
+        return isAdmin(msg.sender) || msg.sender == address(this);
     }
 
     function _afterSignerPermissionsUpdate(SignerPermissionRequest calldata _req) internal virtual override {}

diff --git a/src/test/smart-wallet/Account.t.sol b/src/test/smart-wallet/Account.t.sol
@@ -567,4 +567,61 @@ contract SimpleAccountTest is BaseTest {
 
         assertEq(erc1155.balanceOf(account, 0), 1);
     }
+
+    /*///////////////////////////////////////////////////////////////
+                Test: setting contract metadata
+    //////////////////////////////////////////////////////////////*/
+
+    /// @dev Set contract metadata via admin or entrypoint.
+    function test_state_contractMetadata() public {
+        _setup_executeTransaction();
+        address account = accountFactory.getAddress(accountAdmin, bytes(""));
+
+        vm.prank(accountAdmin);
+        SimpleAccount(payable(account)).setContractURI("https://example.com");
+        assertEq(SimpleAccount(payable(account)).contractURI(), "https://example.com");
+
+        UserOperation[] memory userOp = _setupUserOpExecute(
+            accountAdminPKey,
+            bytes(""),
+            address(account),
+            0,
+            abi.encodeWithSignature("setContractURI(string)", "https://thirdweb.com")
+        );
+
+        EntryPoint(entrypoint).handleOps(userOp, beneficiary);
+        assertEq(SimpleAccount(payable(account)).contractURI(), "https://thirdweb.com");
+
+        address[] memory targets = new address[](0);
+        uint256[] memory values = new uint256[](0);
+        bytes[] memory callData = new bytes[](0);
+
+        address[] memory approvedTargets = new address[](0);
+
+        IAccountPermissions.SignerPermissionRequest memory permissionsReq = IAccountPermissions.SignerPermissionRequest(
+            accountSigner,
+            approvedTargets,
+            1 ether,
+            0,
+            type(uint128).max,
+            0,
+            type(uint128).max,
+            uidCache
+        );
+
+        vm.prank(accountAdmin);
+        bytes memory sig = _signSignerPermissionRequest(permissionsReq);
+        SimpleAccount(payable(account)).setPermissionsForSigner(permissionsReq, sig);
+
+        UserOperation[] memory userOpViaSigner = _setupUserOpExecute(
+            accountSignerPKey,
+            bytes(""),
+            address(account),
+            0,
+            abi.encodeWithSignature("setContractURI(string)", "https://thirdweb.com")
+        );
+
+        vm.expectRevert();
+        EntryPoint(entrypoint).handleOps(userOpViaSigner, beneficiary);
+    }
 }