Skip to content

Commit

Permalink
Add keepers for newbieVilla (#175)
Browse files Browse the repository at this point in the history 
* feat: add keepers for newbieVilla

* feat: add test cases

* fix: typo

* feat: update testWithdrawNewbieOutFail

* feat: update docs & newbieVilla storage layout

* feat: update permission for tips
  • Loading branch information
Albert authored Oct 16, 2023
1 parent 90ffcd8 commit e87afca
Show file tree
Hide file tree
Showing 6 changed files with 259 additions and 113 deletions.
61 changes: 43 additions & 18 deletions contracts/misc/NewbieVilla.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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.
Expand Down Expand Up @@ -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>
Expand All @@ -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
Expand All @@ -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>
Expand All @@ -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.
Expand All @@ -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"
);

Expand All @@ -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>
Expand All @@ -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.
Expand All @@ -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

Expand All @@ -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.
*
Expand All @@ -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,
Expand Down Expand Up @@ -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));
}
}
2 changes: 1 addition & 1 deletion scripts/Deployer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
70 changes: 70 additions & 0 deletions test/UpgradeNewbieVilla.t.sol
View file
Original file line number Diff line number Diff line change
@@ -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)))
);
}
}
2 changes: 1 addition & 1 deletion test/helpers/CommonTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
Loading

0 comments on commit e87afca

Please sign in to comment.