diff --git a/contracts/misc/NewbieVilla.sol b/contracts/misc/NewbieVilla.sol index 5690771b..b1b7d09f 100644 --- a/contracts/misc/NewbieVilla.sol +++ b/contracts/misc/NewbieVilla.sol @@ -84,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.
+ * from account with required permission to `Tips` contract.
* * Admin will call `send` erc777 token to the Tips contract, with `fromCharacterId` * and `toCharacterId` encoded in the `data`.
@@ -95,14 +95,17 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver, * [AbiCoder-encode](https://docs.ethers.org/v5/api/utils/abi/coder/#AbiCoder-encode).
* * Requirements: - * - 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 @@ -117,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.
+ * from account with required permission to `Tips` contract.
* * Admin will call `send` erc777 token to the Tips contract, with `fromCharacterId`, * `toCharacterId` and `toNoteId` encoded in the `data`.
@@ -128,7 +131,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver, * [AbiCoder-encode](https://docs.ethers.org/v5/api/utils/abi/coder/#AbiCoder-encode).
* * Requirements: - * - 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. @@ -140,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" ); @@ -197,11 +200,7 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver, // check proof address signer = ECDSA.recover(signedData, proof); - address keeper = _keepers[characterId]; - require( - (keeper == signer) || (keeper == address(0) && hasRole(ADMIN_ROLE, signer)), - "NewbieVilla: unauthorized withdraw" - ); + require(_hasPermission(signer, characterId), "NewbieVilla: unauthorized withdraw"); // update balance uint256 amount = _balances[characterId]; @@ -322,4 +321,10 @@ contract NewbieVilla is Initializable, AccessControlEnumerable, IERC721Receiver, 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)); + } } diff --git a/test/misc/NewbieVilla.t.sol b/test/misc/NewbieVilla.t.sol index bac87cec..d212c0a1 100644 --- a/test/misc/NewbieVilla.t.sol +++ b/test/misc/NewbieVilla.t.sol @@ -67,31 +67,30 @@ contract NewbieVillaTest is CommonTest { function testNewbieTipCharacter(uint256 amount) public { vm.assume(amount > 0 && amount < 10 ether); - // 1. admin create and transfer web3Entry nft to newbieVilla - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, newbieAdmin)); - vm.prank(newbieAdmin); - web3Entry.safeTransferFrom(newbieAdmin, address(newbieVilla), FIRST_CHARACTER_ID); + // 1. create and transfer character to newbieVilla + uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, alice); + vm.prank(alice); + web3Entry.safeTransferFrom(alice, address(newbieVilla), newbieCharacterId); - // 2. user create web3Entity nft - vm.prank(bob); - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE2, bob)); + // 2.create character for bob + uint256 bobCharacterId = _createCharacter(CHARACTER_HANDLE2, bob); - // 3. send some token to web3Entry nft in newbieVilla + // 3. send some token to newbieVilla for newbieCharacter vm.prank(alice); - token.send(address(newbieVilla), amount, abi.encode(2, FIRST_CHARACTER_ID)); + token.send(address(newbieVilla), amount, abi.encode(2, newbieCharacterId)); // 4. check balance and state before tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); + assertEq(newbieVilla.balanceOf(newbieCharacterId), amount); assertEq(token.balanceOf(bob), initialBalance); // 5. tip another character for certain amount vm.prank(alice); - newbieVilla.tipCharacter(FIRST_CHARACTER_ID, SECOND_CHARACTER_ID, amount); + newbieVilla.tipCharacter(newbieCharacterId, bobCharacterId, amount); // 6. check balance and state after tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), 0); + assertEq(newbieVilla.balanceOf(newbieCharacterId), 0); assertEq(token.balanceOf(bob), initialBalance + amount); } @@ -130,10 +129,10 @@ contract NewbieVillaTest is CommonTest { function testNewbieTipCharacterInsufficientBalanceFail(uint256 amount) public { vm.assume(amount > 0 && amount < 10 ether); - // 1. admin create and transfer web3Entry nft to newbieVilla - uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, newbieAdmin); - vm.prank(newbieAdmin); - web3Entry.safeTransferFrom(newbieAdmin, address(newbieVilla), newbieCharacterId); + // 1. create and transfer web3Entry nft to newbieVilla + uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, alice); + vm.prank(alice); + web3Entry.safeTransferFrom(alice, address(newbieVilla), newbieCharacterId); // 3. send some token to newbieCharacter in newbieVilla contract vm.prank(alice); @@ -146,7 +145,6 @@ contract NewbieVillaTest is CommonTest { // 5. tip another character for certain amount uint256 bobCharacterId = _createCharacter(CHARACTER_HANDLE2, bob); vm.expectRevert(stdError.arithmeticError); - // alice has no permission to tip newbieCharacter // newbieCharacter only has `amount` token in newbieVilla , so it will overflow vm.prank(alice); newbieVilla.tipCharacter(newbieCharacterId, bobCharacterId, amount + 1); @@ -159,110 +157,92 @@ contract NewbieVillaTest is CommonTest { function testNewbieTipCharacterForNote(uint256 amount) public { vm.assume(amount > 0 && amount < 10 ether); - // 1. admin create and transfer web3Entry nft to newbieVilla - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, newbieAdmin)); - vm.prank(newbieAdmin); - web3Entry.safeTransferFrom(newbieAdmin, address(newbieVilla), FIRST_CHARACTER_ID); + // 1. create and transfer web3Entry nft to newbieVilla + uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, alice); + vm.prank(alice); + web3Entry.safeTransferFrom(alice, address(newbieVilla), newbieCharacterId); - // 2. user create web3Entity nft - vm.prank(bob); - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE2, bob)); + // 2. create character for bob + uint256 bobCharacterId = _createCharacter(CHARACTER_HANDLE2, bob); - // 3. send some token to web3Entry nft in newbieVilla + // 3. send some token to newbieVilla for newbieCharacter vm.prank(alice); - token.send(address(newbieVilla), amount, abi.encode(2, FIRST_CHARACTER_ID)); + token.send(address(newbieVilla), amount, abi.encode(2, newbieCharacterId)); // 4. check balance and state before tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); + assertEq(newbieVilla.balanceOf(newbieCharacterId), amount); assertEq(token.balanceOf(bob), initialBalance); // 5. tip another character's note for certain amount vm.prank(alice); - newbieVilla.tipCharacterForNote( - FIRST_CHARACTER_ID, - SECOND_CHARACTER_ID, - FIRST_NOTE_ID, - amount - ); + newbieVilla.tipCharacterForNote(newbieCharacterId, bobCharacterId, FIRST_NOTE_ID, amount); // 6. check balance and state after tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), 0); + assertEq(newbieVilla.balanceOf(newbieCharacterId), 0); assertEq(token.balanceOf(bob), initialBalance + amount); } function testNewbieTipCharacterForNoteNotAuthorizedFail(uint256 amount) public { vm.assume(amount > 0 && amount < 10 ether); - // 1. admin create and transfer web3Entry nft to newbieVilla - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, newbieAdmin)); - vm.prank(newbieAdmin); - web3Entry.safeTransferFrom(newbieAdmin, address(newbieVilla), FIRST_CHARACTER_ID); - - // 2. user create web3Entity nft - vm.prank(bob); - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE2, bob)); + // 1. create characters + uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, alice); + uint256 bobCharacterId = _createCharacter(CHARACTER_HANDLE2, bob); - // 3. send some token to web3Entry nft in newbieVilla + // 2. alice sends character to newbieVilla vm.prank(alice); - token.send(address(newbieVilla), amount, abi.encode(2, FIRST_CHARACTER_ID)); + web3Entry.safeTransferFrom(alice, address(newbieVilla), newbieCharacterId); - // 4. check balance and state before tip - assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); - assertEq(token.balanceOf(bob), initialBalance); + // 3. send some token to newbieVilla for newbieCharacter + vm.prank(alice); + token.send(address(newbieVilla), amount, abi.encode(2, newbieCharacterId)); - // 5. tip another character's note for certain amount - vm.prank(bob); + // case 1: expect revert with no permission vm.expectRevert(abi.encodePacked("NewbieVilla: unauthorized role for tipCharacterForNote")); - newbieVilla.tipCharacterForNote( - FIRST_CHARACTER_ID, - SECOND_CHARACTER_ID, - FIRST_NOTE_ID, - amount - ); + vm.prank(bob); + newbieVilla.tipCharacterForNote(newbieCharacterId, bobCharacterId, 1, amount); - // 6. check balance and state after tip - assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); - assertEq(token.balanceOf(bob), initialBalance); + // case 2: expect revert with no permission + vm.expectRevert(abi.encodePacked("NewbieVilla: unauthorized role for tipCharacterForNote")); + vm.prank(newbieAdmin); + newbieVilla.tipCharacterForNote(newbieCharacterId, bobCharacterId, 1, amount); } function testNewbieTipCharacterForNoteInsufficientBalanceFail(uint256 amount) public { vm.assume(amount > 0 && amount < 10 ether); - // 1. admin create and transfer web3Entry nft to newbieVilla - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE, newbieAdmin)); - vm.prank(newbieAdmin); - web3Entry.safeTransferFrom(newbieAdmin, address(newbieVilla), FIRST_CHARACTER_ID); + // 1. create and transfer web3Entry nft to newbieVilla + uint256 newbieCharacterId = _createCharacter(CHARACTER_HANDLE, alice); + vm.prank(alice); + web3Entry.safeTransferFrom(alice, address(newbieVilla), newbieCharacterId); - // 2. user create web3Entity nft - vm.prank(bob); - web3Entry.createCharacter(makeCharacterData(CHARACTER_HANDLE2, bob)); + // 2. create character for bob + uint256 bobCharacterId = _createCharacter(CHARACTER_HANDLE2, bob); - // 3. send some token to web3Entry nft in newbieVilla + // 3. send some token to newbieVilla for newbieCharacter vm.prank(alice); - token.send(address(newbieVilla), amount, abi.encode(2, FIRST_CHARACTER_ID)); + token.send(address(newbieVilla), amount, abi.encode(2, newbieCharacterId)); // 4. check balance and state before tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); + assertEq(newbieVilla.balanceOf(newbieCharacterId), amount); assertEq(token.balanceOf(bob), initialBalance); // 5. tip another character's note for certain amount vm.prank(alice); vm.expectRevert(stdError.arithmeticError); newbieVilla.tipCharacterForNote( - FIRST_CHARACTER_ID, - SECOND_CHARACTER_ID, + newbieCharacterId, + bobCharacterId, FIRST_NOTE_ID, amount + 1 ); // 6. check balance and state after tip assertEq(token.balanceOf(alice), initialBalance - amount); - assertEq(newbieVilla.balanceOf(FIRST_CHARACTER_ID), amount); + assertEq(newbieVilla.balanceOf(newbieCharacterId), amount); assertEq(token.balanceOf(bob), initialBalance); }