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.
@@ -82,7 +84,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
 
     /**
      * @notice Tips a character by transferring `amount` tokens
-     * from account with `ADMIN_ROLE` to `Tips` contract. <br>
+     * from account with required permission to `Tips` contract. <br>
      *
      * Admin will call `send` erc777 token to the Tips contract, with `fromCharacterId`
      * and `toCharacterId` encoded in the `data`. <br>
@@ -93,14 +95,17 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
      * [AbiCoder-encode](https://docs.ethers.org/v5/api/utils/abi/coder/#AbiCoder-encode).<br>
      *
      * <b> Requirements: </b>
-     * - The `msg.sender` must have `ADMIN_ROLE`.
+     * - The `msg.sender` must be character's keeper or have `ADMIN_ROLE` but not character's keeper.
      * @param fromCharacterId The token ID of character that calls this contract.
      * @param toCharacterId The token ID of character that will receive the token.
      * @param amount Amount of token.
      */
     function tipCharacter(uint256 fromCharacterId, uint256 toCharacterId, uint256 amount) external {
-        // check admin role
-        require(hasRole(ADMIN_ROLE, msg.sender), "NewbieVilla: unauthorized role for tipCharacter");
+        // check permission
+        require(
+            _hasPermission(msg.sender, fromCharacterId),
+            "NewbieVilla: unauthorized role for tipCharacter"
+        );
 
         // newbievilla's balance - tip amount
         // will fail if balance is insufficient
@@ -115,7 +120,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
 
     /**
      * @notice Tips a character's note by transferring `amount` tokens
-     * from account with `ADMIN_ROLE` to `Tips` contract. <br>
+     * from account with required permission to `Tips` contract. <br>
      *
      * Admin will call `send` erc777 token to the Tips contract, with `fromCharacterId`,
      * `toCharacterId` and `toNoteId` encoded in the `data`. <br>
@@ -126,7 +131,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
      * [AbiCoder-encode](https://docs.ethers.org/v5/api/utils/abi/coder/#AbiCoder-encode).<br>
      *
      * <b> Requirements: </b>
-     * - The `msg.sender` must have `ADMIN_ROLE`.
+     * - The `msg.sender` must be character's keeper or have `ADMIN_ROLE` but not character's keeper.
      * @param fromCharacterId The token ID of character that calls this contract.
      * @param toCharacterId The token ID of character that will receive the token.
      * @param toNoteId The note ID.
@@ -138,9 +143,9 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
         uint256 toNoteId,
         uint256 amount
     ) external {
-        // check admin role
+        // check permission
         require(
-            hasRole(ADMIN_ROLE, msg.sender),
+            _hasPermission(msg.sender, fromCharacterId),
             "NewbieVilla: unauthorized role for tipCharacterForNote"
         );
 
@@ -158,7 +163,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>
@@ -174,7 +179,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
      *
      * <b> Requirements: </b>:
      * - `expires` is greater than the current timestamp
-     * - `proof` is signed by the one with the ADMIN_ROLE
+     * - `proof` is signed by the keeper or address with the ADMIN_ROLE
      *
      * @param   to  Receiver of the withdrawn character.
      * @param   characterId  The token id of the character to withdraw.
@@ -192,13 +197,17 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
         bytes32 signedData = ECDSA.toEthSignedMessageHash(
             keccak256(abi.encodePacked(address(this), characterId, nonce, expires))
         );
-        require(
-            hasRole(ADMIN_ROLE, ECDSA.recover(signedData, proof)),
-            "NewbieVilla: unauthorized withdraw"
-        );
 
+        // check proof
+        address signer = ECDSA.recover(signedData, proof);
+        require(_hasPermission(signer, characterId), "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
 
@@ -217,7 +226,6 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver,
      * <b> Requirements: </b>:
      *
      * - `msg.sender` must be address of Web3Entry.
-     * - `operator` must has ADMIN_ROLE.
      *
      * @param data bytes encoded from the operator address to set for the incoming character.
      *
@@ -230,9 +238,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,11 +305,26 @@ 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.
      */
     function getToken() external view returns (address) {
         return _token;
     }
+
+    /// @dev It will return true if `account` is character's keeper or has `ADMIN_ROLE` but not character's keeper.
+    function _hasPermission(address account, uint256 characterId) internal view returns (bool) {
+        address keeper = _keepers[characterId];
+        return (keeper == account) || (keeper == address(0) && hasRole(ADMIN_ROLE, account));
+    }
 }

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");