Add keepers for newbieVilla

contracts/misc/NewbieVilla.sol

@@ -14,7 +14,7 @@ import {IERC777} from "@openzeppelin/contracts/token/ERC777/IERC777.sol";
 import {IERC1820Registry} from "@openzeppelin/contracts/utils/introspection/IERC1820Registry.sol";
 
 /**
- * @dev Implementation of a contract to keep characters for others. The address with
+ * @dev Implementation of a contract to keep characters for others. The keepers and addresses with
  * the ADMIN_ROLE are expected to issue the proof to users. Then users could use the
  * proof to withdraw the corresponding character.
  */
@@ -33,6 +33,8 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
     mapping(uint256 => uint256) internal _balances;
     address internal _tips; // tips contract
 
+    mapping(uint256 characterId => address keeper) private _keepers;
+
     // events
     /**
      * @dev Emitted when the web3Entry character nft is withdrawn.
@@ -158,7 +160,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
     /**
      * @notice  Withdraw character#`characterId` to `to` using the nonce, expires and the proof. <br>
      * Emits the `Withdraw` event. <br>
-     * @dev Proof is the signature from someone with the ADMIN_ROLE. The message to sign is
+     * @dev Proof is the signature from character keepers or someone with the ADMIN_ROLE. The message to sign is
      * the packed data of this contract's address, `characterId`, `nonce` and `expires`. <br>
      *
      * Here's an example to generate a proof: <br>
@@ -192,13 +194,21 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
         bytes32 signedData = ECDSA.toEthSignedMessageHash(
             keccak256(abi.encodePacked(address(this), characterId, nonce, expires))
         );
+
+        // check proof
+        address signer = ECDSA.recover(signedData, proof);
+        address keeper = _keepers[characterId];
         require(
-            hasRole(ADMIN_ROLE, ECDSA.recover(signedData, proof)),
+            (keeper == signer) || (keeper == address(0) && hasRole(ADMIN_ROLE, signer)),
             "NewbieVilla: unauthorized withdraw"
         );
 
+        // update balance
         uint256 amount = _balances[characterId];
         _balances[characterId] = 0;
+        // update keeper
+        delete _keepers[characterId];
+
         // send token
         IERC777(_token).send(to, amount, ""); // solhint-disable-line check-send-result
 
@@ -230,9 +240,11 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
     ) external override returns (bytes4) {
         // Only character nft could be received, other nft, e.g. mint nft would be reverted
         require(msg.sender == web3Entry, "NewbieVilla: receive unknown token");
-        // Only admin role could send character to this contract
-        require(hasRole(ADMIN_ROLE, operator), "NewbieVilla: receive unknown character");
 
+        // set keeper for tokenId
+        _keepers[tokenId] = operator;
+
+        // grant operator permissions
         if (data.length == 0) {
             IWeb3Entry(web3Entry).grantOperatorPermissions(
                 tokenId,
@@ -295,6 +307,15 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
         return _balances[characterId];
     }
 
+    /**
+     * @notice Returns the address of keeper by `characterId`.
+     * @param characterId The character ID to query.
+     * @return address The address of the keeper.
+     */
+    function getKeeper(uint256 characterId) external view returns (address) {
+        return _keepers[characterId];
+    }
+
     /**
      * @notice Returns the address of mira token contract.
      * @return The address of mira token contract.

scripts/Deployer.sol

@@ -334,7 +334,7 @@ abstract contract Deployer is Script {
         return string(res);
     }
 
-    /// @notice Returns the constructor arguent of a deployment transaction given a transaction json.
+    /// @notice Returns the constructor arguments of a deployment transaction given a transaction json.
     function getDeployTransactionConstructorArguments(
         string memory _transaction
     ) internal returns (string[] memory) {

test/UpgradeNewbieVilla.t.sol

@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: MIT
+// solhint-disable comprehensive-interface
+pragma solidity 0.8.18;
+
+import {CommonTest} from "./helpers/CommonTest.sol";
+import {NewbieVilla} from "../contracts/misc/NewbieVilla.sol";
+import {
+    TransparentUpgradeableProxy
+} from "../contracts/upgradeability/TransparentUpgradeableProxy.sol";
+import {IERC721} from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
+
+contract UpgradeNewbieVillaTest is CommonTest {
+    // test upgradeability of NewbieVilla from crossbell fork
+    address internal _web3Entry = address(0xa6f969045641Cf486a747A2688F3a5A6d43cd0D8);
+    address internal constant _token = 0xAfB95CC0BD320648B3E8Df6223d9CDD05EbeDC64;
+    address payable internal _newbieVilla =
+        payable(address(0xD0c83f0BB2c61D55B3d33950b70C59ba2f131caA));
+    address internal _proxyAdmin = address(0x5f603895B48F0C451af39bc7e0c587aE15718e4d);
+
+    function setUp() public {
+        // create and select a fork from crossbell at block 46718115
+        vm.createSelectFork(vm.envString("CROSSBELL_RPC_URL"), 46718115);
+    }
+
+    function testCheckSetupState() public {
+        assertEq(NewbieVilla(_newbieVilla).web3Entry(), _web3Entry);
+        assertEq(NewbieVilla(_newbieVilla).getToken(), _token);
+    }
+
+    function testUpgradeNewbieVilla() public {
+        NewbieVilla newImpl = new NewbieVilla();
+        // upgrade and initialize
+        vm.prank(_proxyAdmin);
+        TransparentUpgradeableProxy(_newbieVilla).upgradeTo(address(newImpl));
+        // check newImpl
+        vm.prank(_proxyAdmin);
+        assertEq(TransparentUpgradeableProxy(_newbieVilla).implementation(), address(newImpl));
+        // check state
+        assertEq(NewbieVilla(_newbieVilla).web3Entry(), _web3Entry);
+        assertEq(NewbieVilla(_newbieVilla).getToken(), _token);
+    }
+
+    function testUpgradeNewbieVillaWithStorageCheck() public {
+        // create and select a fork from crossbell at block 46718115
+        vm.createSelectFork(vm.envString("CROSSBELL_RPC_URL"), 46718115);
+
+        NewbieVilla newImpl = new NewbieVilla();
+        // upgrade
+        vm.prank(_proxyAdmin);
+        TransparentUpgradeableProxy(_newbieVilla).upgradeTo(address(newImpl));
+
+        NewbieVilla newbieVilla = NewbieVilla(_newbieVilla);
+
+        // transfer character to newbieVilla
+        address owner = 0xC8b960D09C0078c18Dcbe7eB9AB9d816BcCa8944;
+        vm.prank(owner);
+        IERC721(_web3Entry).safeTransferFrom(owner, _newbieVilla, 10);
+        assertEq(newbieVilla.getKeeper(10), owner);
+
+        // check storage
+        assertEq(newbieVilla.web3Entry(), _web3Entry);
+        assertEq(newbieVilla.getToken(), _token);
+        assertEq(newbieVilla.hasRole(ADMIN_ROLE, 0x51e2368D60Bc329DBd5834370C1e633bE60C1d6D), true);
+
+        assertEq(
+            vm.load(address(newbieVilla), bytes32(uint256(7))),
+            bytes32(uint256(uint160(0x0058be0845952D887D1668B5545de995E12e8783)))
+        );
+    }
+}

test/helpers/CommonTest.sol

@@ -44,7 +44,7 @@ contract CommonTest is Utils {
     address public constant admin = address(0x999999999999999999999999999999);
 
     address public constant xsyncOperator = address(0xffff4444);
-    uint256 public constant newbieAdminPrivateKey = 1;
+    uint256 public constant newbieAdminPrivateKey = 1234567;
     address public newbieAdmin = vm.addr(newbieAdminPrivateKey);
     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
 

test/misc/NewbieVilla.t.sol

@@ -287,34 +287,29 @@ contract NewbieVillaTest is CommonTest {
         );
     }
 
-    function testNewbieCreateCharacterFail() public {
-        // bob has no mint role, so he can't send character to newbieVilla contract
-        vm.prank(bob);
-        vm.expectRevert(abi.encodePacked("NewbieVilla: receive unknown character"));
-        web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, address(newbieVilla)));
-    }
-
     // transfer character to newbieVilla contract
     function testTransferNewbieIn() public {
-        web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, alice));
-        vm.prank(alice);
-        web3Entry.safeTransferFrom(alice, address(newbieVilla), FIRST_CHARACTER_ID);
+        web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, bob));
+        vm.prank(bob);
+        web3Entry.safeTransferFrom(bob, address(newbieVilla), FIRST_CHARACTER_ID);
         // check operators
         address[] memory operators = web3Entry.getOperators(FIRST_CHARACTER_ID);
-        assertEq(operators[0], alice);
+        assertEq(operators[0], bob);
         assertEq(operators[1], xsyncOperator);
 
         // check operator permission bitmap
-        // alice(NewbieVilla admin) has DEFAULT_PERMISSION_BITMAP.
+        // bob(character's keeper) has DEFAULT_PERMISSION_BITMAP.
         assertEq(
-            web3Entry.getOperatorPermissions(FIRST_CHARACTER_ID, alice),
+            web3Entry.getOperatorPermissions(FIRST_CHARACTER_ID, bob),
             OP.DEFAULT_PERMISSION_BITMAP
         );
         // xsyncOperator has POST_NOTE_DEFAULT_PERMISSION_BITMAP
         assertEq(
             web3Entry.getOperatorPermissions(FIRST_CHARACTER_ID, xsyncOperator),
             OP.POST_NOTE_DEFAULT_PERMISSION_BITMAP
         );
+        // check keeper
+        assertEq(newbieVilla.getKeeper(FIRST_CHARACTER_ID), bob);
     }
 
     // transfer character to newbieVilla contract with data
@@ -347,15 +342,12 @@ contract NewbieVillaTest is CommonTest {
             web3Entry.getOperatorPermissions(FIRST_CHARACTER_ID, xsyncOperator),
             OP.POST_NOTE_DEFAULT_PERMISSION_BITMAP
         );
+        // check keeper
+        assertEq(newbieVilla.getKeeper(FIRST_CHARACTER_ID), alice);
     }
 
-    function testTransferNewbieInFail() public {
-        web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, bob));
-
-        vm.expectRevert(abi.encodePacked("NewbieVilla: receive unknown character"));
-        vm.prank(bob);
-        web3Entry.safeTransferFrom(bob, address(newbieVilla), FIRST_CHARACTER_ID);
-
+    function testTransferNewbieInFailWithNonCharacterNFT() public {
+        // transfer non-character nft to newbieVilla contract
         NFT nft = new NFT();
         nft.mint(bob);
 
@@ -396,6 +388,83 @@ contract NewbieVillaTest is CommonTest {
         assertEq(newbieVilla.balanceOf(characterId), 0);
         assertEq(web3Entry.ownerOf(characterId), carol);
         assertEq(token.balanceOf(carol), amount);
+        // check keeper
+        assertEq(newbieVilla.getKeeper(characterId), address(0));
+    }
+
+    function testWithdrawNewbieOutWithKeeper(uint256 amount) public {
+        vm.assume(amount > 0 && amount < 10 ether);
+        address to = carol;
+        uint256 nonce = 1;
+        uint256 expires = block.timestamp + 10 minutes;
+
+        uint256 keeperPrivateKey = 1;
+        address keeper = vm.addr(keeperPrivateKey);
+
+        // 1. create and transfer web3Entry nft to newbieVilla
+        uint256 characterId = web3Entry.createCharacter(
+            makeCharacterData(CHARACTER_HANDLE, keeper)
+        );
+        vm.prank(keeper);
+        web3Entry.safeTransferFrom(keeper, address(newbieVilla), characterId);
+
+        // 2. send some token to web3Entry nft in newbieVilla
+        vm.prank(alice);
+        token.send(address(newbieVilla), amount, abi.encode(2, characterId));
+
+        // 3. withdraw web3Entry nft
+        bytes32 digest = keccak256(
+            abi.encodePacked(
+                "\x19Ethereum Signed Message:\n32",
+                keccak256(abi.encodePacked(address(newbieVilla), characterId, nonce, expires))
+            )
+        );
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(keeperPrivateKey, digest);
+        // withdraw
+        vm.prank(to);
+        newbieVilla.withdraw(to, characterId, nonce, expires, abi.encodePacked(r, s, v));
+
+        // check state
+        assertEq(newbieVilla.balanceOf(characterId), 0);
+        assertEq(web3Entry.ownerOf(characterId), carol);
+        assertEq(token.balanceOf(carol), amount);
+        // check keeper
+        assertEq(newbieVilla.getKeeper(characterId), address(0));
+    }
+
+    // newbieVilla admin can't withdraw characters deposited by keeper
+    function testWithdrawNewbieOutFail(uint256 amount) public {
+        vm.assume(amount > 0 && amount < 10 ether);
+        address to = carol;
+        uint256 nonce = 1;
+        uint256 expires = block.timestamp + 10 minutes;
+
+        uint256 keeperPrivateKey = newbieAdminPrivateKey + 1;
+        address keeper = vm.addr(keeperPrivateKey);
+
+        // 1.  create and transfer web3Entry nft to newbieVilla
+        uint256 characterId = web3Entry.createCharacter(
+            makeCharacterData(CHARACTER_HANDLE, keeper)
+        );
+        vm.prank(keeper);
+        web3Entry.safeTransferFrom(keeper, address(newbieVilla), characterId);
+
+        // 2. send some token to web3Entry nft in newbieVilla
+        vm.prank(alice);
+        token.send(address(newbieVilla), amount, abi.encode(2, characterId));
+
+        // 3. withdraw web3Entry nft by newbieVilla admin
+        bytes32 digest = keccak256(
+            abi.encodePacked(
+                "\x19Ethereum Signed Message:\n32",
+                keccak256(abi.encodePacked(address(newbieVilla), characterId, nonce, expires))
+            )
+        );
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(newbieAdminPrivateKey, digest);
+        // withdraw
+        vm.expectRevert("NewbieVilla: unauthorized withdraw");
+        vm.prank(to);
+        newbieVilla.withdraw(to, characterId, nonce, expires, abi.encodePacked(r, s, v));
     }
 
     function testTokensReceived(uint256 amount) public {