From 9176de02b8fa5b6c4bd3d49995072471e888825c Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 21 Mar 2022 16:10:44 -0400 Subject: [PATCH 1/4] IERC721Roles --- .../token/ERC721/extensions/ERC721Roles.sol | 67 +++++++++++++ .../token/ERC721/extensions/IERC721Roles.sol | 94 +++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 contracts/token/ERC721/extensions/ERC721Roles.sol create mode 100644 contracts/token/ERC721/extensions/IERC721Roles.sol diff --git a/contracts/token/ERC721/extensions/ERC721Roles.sol b/contracts/token/ERC721/extensions/ERC721Roles.sol new file mode 100644 index 00000000000..0be40bd8b39 --- /dev/null +++ b/contracts/token/ERC721/extensions/ERC721Roles.sol @@ -0,0 +1,67 @@ +pragma solidity ^0.8.0; + +import "../ERC721.sol"; +import "./IERC721Roles.sol"; + +contract ERC721Roles is ERC721, IERC721Roles { + // The address authorized to change the _roleManagement contract. + address private _rolesManager; + // The contract holding the logic defining the roles access. + IERC721RolesManagement private _rolesManagement; + // A record of the registered and active roles by tokenId. + mapping(uint256 => mapping(address => mapping(bytes4 => bool))) private _tokenIdRegisteredRoles; + + constructor(string memory name_, string memory symbol_, address rolesManager_) ERC721(name_, symbol_){ + _rolesManager = rolesManager_; + } + + /// @inheritdoc IERC721Roles + function setRolesManager(address rolesManager_) external { + require(_msgSender()==_rolesManager, "ERC721: caller is not the rolesManager"); + _rolesManager = rolesManager_; + } + + /// @inheritdoc IERC721Roles + function rolesManager() public view returns (address) { + return _rolesManager; + } + + /// @inheritdoc IERC721Roles + function setRolesManagement(IERC721RolesManagement rolesManagement_) external { + require(_msgSender()==_rolesManager, "ERC721: caller is not the rolesManager"); + if (address(_rolesManagement) != address(0)) { + _rolesManagement.afterRolesManagementRemoved(); + } + + _rolesManagement = rolesManagement_; + } + + /// @inheritdoc IERC721Roles + function rolesManagement() public view returns (IERC721RolesManagement){ + return _rolesManagement; + } + + /// @inheritdoc IERC721Roles + function roleGranted(address user, uint256 tokenId, bytes4 roleId) external view returns (bool) { + return _tokenIdRegisteredRoles[tokenId][user][roleId]; + } + + /// @inheritdoc IERC721Roles + function addRole(address forAddress, uint256 tokenId, bytes4 roleId) external { + _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = true; + // Callback to the roles management contract. + _rolesManagement.afterRoleAdded(forAddress, tokenId, roleId); + } + + /// @inheritdoc IERC721Roles + function revokeRole(address forAddress, uint256 tokenId, bytes4 roleId) external { + _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = false; + // Callback to the roles management contract. + _rolesManagement.afterRoleRevoked(forAddress, tokenId, roleId); + } + + /// @dev See {IERC165-supportsInterface}. + function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC721) returns (bool) { + return interfaceId == type(IERC721Roles).interfaceId || super.supportsInterface(interfaceId); + } +} \ No newline at end of file diff --git a/contracts/token/ERC721/extensions/IERC721Roles.sol b/contracts/token/ERC721/extensions/IERC721Roles.sol new file mode 100644 index 00000000000..9a3b28f9544 --- /dev/null +++ b/contracts/token/ERC721/extensions/IERC721Roles.sol @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "../IERC721.sol"; +import "../../../utils/introspection/IERC165.sol"; + +/// @title ERC721 token roles management interface +/// +/// Defines the interface that a role management contract should support to be used by +/// `IERC721Roles`. +interface IERC721RolesManagement is IERC165 { + + /// Function called at the end of `IERC721Roles.setRolesManagement` on the roles management contract + /// currently set for the token, if one exists. + /// + /// @dev Allows the roles management to cancel the change by reverting if it deems it + /// necessary. The `IERC721Roles` is calling this function, so all information needed + /// can be queried through the `msg.sender`. This event is only called when their are no specific roles + /// set as it is not allowed to change the role manager when there are active + function afterRolesManagementRemoved() external; + + /// Function called at the end of `IERC721Roles.revokeRole` + /// + /// @dev Allows the roles management to cancel role withdrawal by reverting if it deems it + /// necessary. The `IERC721Roles` is calling this function, so all information needed + /// can be queried through the `msg.sender`. This event is not called if a rental is + /// not in progress. + function afterRoleRevoked(address forAddress, uint256 tokenId, bytes4 roleId) external; + + /// Function called at the end of `IERC721Roles.addRole`. + /// + /// @dev Allows the roles management to prevent adding a new role if it deems it + /// necessary. The `IERC721Roles` is calling this function, so all information needed + /// can be queried through the `msg.sender`. + /// + /// @param forAddress The address that called `IERC721Roles.addRole` + function afterRoleAdded(address forAddress, uint256 tokenId, bytes4 roleId) external; +} + +/// @title ERC721 token roles interface +/// +/// Defines the optional interface that allows setting roles for users by tokenId. +/// It delegates the logic of adding and revoking roles to a roles management contract implementing the +/// `IERC721RolesManagement` interface. +/// A user can hold multiple roles and multiple users can be granted the same role. It's the +/// responsability of the roles management contract to allow such interactions. +/// +/// The roles manager is allowed to change the roles Management contract, +/// if authorized by the currently set RoleManagement contract. +/// +/// A role is defined similarly to functions' methodId by the first 4 bytes of its hash. +/// For example, the renter role will be defined by bytes4(sha3("renter")) + +interface IERC721Roles is IERC721 { + /// Set the role management contract. + /// + /// A previously set roles management contract must accept the change. + /// The caller must be the roles manager, the address authorized make such a change + /// + /// @dev If a roles management was already set before this call, calls its + /// `IERC721RolesManagement.afterRolesManagementRemoved` at the end of the call. + /// + /// @param rolesManagement The roles management contract. Set to 0 to remove the current roles management. + function setRolesManagement(IERC721RolesManagement rolesManagement) external; + + /// @return the address of the roles management, or 0 if there is no roles management set. + function rolesManagement() external returns (IERC721RolesManagement); + + /// Set the address authorized to change the roles management contract. + /// @param rolesManager the new roles manager. + function setRolesManager(address rolesManager) external; + + /// @return the roles manager. + function rolesManager() external view returns(address); + + /// @return true if the role has been granted to the user. + function roleGranted(address user, uint256 tokenId, bytes4 roleId) external view returns (bool); + + /// Set the role for the address and the token. + /// + /// The token must have a roles management contract set. + /// The roles management contract must accept the new role for the address + /// + /// @dev Calls `IERC721RolesManagement.afterRoleAdded` at the end of the call. + function addRole(address forAddress, uint256 tokenId, bytes4 roleId) external; + + /// Revoke the role for the token and the address. + /// + /// The token must have a roles management contract set. + /// The roles management contract must accept the role withdrawal for this address. + /// + /// @dev Calls `IERC721RolesManagement.afterRoleRevoked` at the end of the call. + function revokeRole(address forAddress, uint256 tokenId, bytes4 roleId) external; +} From 1c6af356c00fdb03e34e101578f6c94e74326da5 Mon Sep 17 00:00:00 2001 From: Arthur Date: Mon, 21 Mar 2022 19:27:39 -0400 Subject: [PATCH 2/4] add implementations --- .../ERC721/ERC721RolesRentalAgreement.sol | 168 ++++++++++++++++++ .../token/ERC721/extensions/ERC721Roles.sol | 4 +- .../token/ERC721/extensions/IERC721Roles.sol | 12 +- 3 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 contracts/token/ERC721/ERC721RolesRentalAgreement.sol diff --git a/contracts/token/ERC721/ERC721RolesRentalAgreement.sol b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol new file mode 100644 index 00000000000..3c411850758 --- /dev/null +++ b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol @@ -0,0 +1,168 @@ +pragma solidity ^0.8.0; + +import "../../utils/Context.sol"; +import "./extensions/IERC721Roles.sol"; +import "../../utils/introspection/ERC165.sol"; + +contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { + enum RentalStatus { + pending, + active, + finished + } + + struct RentalAgreement { + uint32 rentalDuration; + uint32 expirationDate; + uint32 startTime; + uint256 rentalFees; + RentalStatus rentalStatus; + } + + IERC721Roles public erc721Contract; + mapping(uint256 => RentalAgreement) public tokenIdToRentalAgreement; + uint256[] public tokenRented; + + // Mapping owners address to balances; + mapping(address => uint256) public balances; + + bytes4 public renterRoleId = bytes4(keccak256("ERC721Role::Renter")); + + constructor(IERC721Roles _erc721Contract) { + erc721Contract = _erc721Contract; + } + + // ===== Modifiers ====== // + modifier onlyErc721Contract() { + require( + _msgSender() == address(erc721Contract), + "ERC721RolesRentalAgreement: only erc721Contract contract can modify state" + ); + _; + } + + function afterRolesManagementRemoved() external onlyErc721Contract { + // The roles management contract can be updated only if all rentals are finished. + for (uint256 i = 0; i < tokenRented.length; i++) { + RentalStatus rentalStatus = tokenIdToRentalAgreement[tokenRented[i]].rentalStatus; + require(rentalStatus == RentalStatus.finished, "ERC721RolesRentalAgreement: rental is still active"); + } + } + + // Allow the token holder or operator to set up a new rental agreement. + function setRentalAgreement( + uint256 tokenId, + uint32 duration, + uint32 expirationDate, + uint256 fees + ) public { + require( + _isOwnerOrApproved(tokenId, _msgSender()), + "ERC721RolesRentalAgreement: only owner or approver can set up a rental agreement" + ); + + RentalAgreement existing_agreement = tokenIdToRentalAgreement[token_id]; + require( + existing_agreement.rentalStatus != RentalStatus.active, + "ERC721RolesRentalAgreement: can't update rental agreement if there is an active one already" + ); + + RentalAgreement rentalAgreement = RentalAgreement( + duration, + expirationDate, + uint32(block.timestamp), + fees, + RentalStatus.pending + ); + tokenIdToRentalAgreement[token_id] = rentalAgreement; + } + + function startRental(address forAddress, uint256 tokenId) public payable { + RentalAgreement rentalAgreement = tokenIdToRentalAgreement[token_id]; + require(now <= rentalAgreement.expirationDate, "ERC721RolesRentalAgreement: rental agreement expired"); + require( + rentalAgreement.rentalStatus != rentalStatus.active, + "ERC721RolesRentalAgreement: rental already in progress" + ); + + uint256 rentalFees = rentalAgreement.rentalFees; + require(msg.value >= rentalFees, "ERC721RolesRentalAgreement: value below the rental fees"); + + address owner = erc721Contract.ownerOf(tokenId); + // Credit the fees to the owner's balance. + balances[owner] += rentalFees; + // Credit the remaining value to the sender's balance. + balances[_msgSender()] += msg.value - rentalFees; + + // Start the rental + rentalAgreement.rentalStatus = RentalStatus.active; + rentalAgreement.startTime = now; + + // Mark the token as rented. + tokenRented.push(tokenId); + + // Reflect the role in the ERC721 token. + erc721Contract.addRole(forAddress, tokenId, renterRoleId); + } + + function stopRental(address forAddress, uint256 tokenId) public { + RentalAgreement rentalAgreement = tokenIdToRentalAgreement[token_id]; + require(rentalAgreement.rentalStatus == rentalStatus.active, "ERC721RolesRentalAgreement: rental not active"); + require( + now - rentalAgreement.startTime >= rentalAgreement.rentalDuration, + "ERC721RolesRentalAgreement: rental still ongoing" + ); + + // Stop the rental + rentalAgreement.rentalStatus = finished; + + // Revoke the renter role + erc721Contract.revokeRole(forAddress, tokenId, renterRoleId); + } + + function afterRoleAdded( + address fromAddress, + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external onlyErc721Contract { + require(fromAddress == address(this), "ERC721RolesRentalAgreement: only this contract can set up renter roles"); + } + + function afterRoleRevoked( + address fromAddress, + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external onlyErc721Contract { + require(fromAddress == address(this), "ERC721RolesRentalAgreement: only this contract can revoke renter roles"); + } + + function _isOwnerOrApproved(uint256 tokenId, address sender) internal view returns (bool) { + address owner = erc721Contract.ownerOf(tokenId); + return + owner == sender || + erc721Contract.getApproved(tokenId) == sender || + erc721Contract.isApprovedForAll(owner, sender); + } + + function redeemFunds(uint256 _value) public { + require(_value <= balances[_msgSender()], "ERC721RolesRentalAgreement: not enough funds to redeem"); + + balances[_msgSender()] -= _value; + + // Check if the transfer is successful. + require(_attemptETHTransfer(_msgSender(), _value), "ERC721RolesRentalAgreement: ETH transfer failed"); + + // Emit an event. + emit FundsRedeemed(_msgSender(), _value, balances[_msgSender()]); + } + + function _attemptETHTransfer(address _to, uint256 _value) internal returns (bool) { + // Here increase the gas limit a reasonable amount above the default, and try + // to send ETH to the recipient. + // NOTE: This might allow the recipient to attempt a limited reentrancy attack. + (bool success, ) = _to.call{value: _value, gas: 30000}(""); + return success; + } +} diff --git a/contracts/token/ERC721/extensions/ERC721Roles.sol b/contracts/token/ERC721/extensions/ERC721Roles.sol index 0be40bd8b39..a5fff6c4404 100644 --- a/contracts/token/ERC721/extensions/ERC721Roles.sol +++ b/contracts/token/ERC721/extensions/ERC721Roles.sol @@ -50,14 +50,14 @@ contract ERC721Roles is ERC721, IERC721Roles { function addRole(address forAddress, uint256 tokenId, bytes4 roleId) external { _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = true; // Callback to the roles management contract. - _rolesManagement.afterRoleAdded(forAddress, tokenId, roleId); + _rolesManagement.afterRoleAdded(msg.sender, forAddress, tokenId, roleId); } /// @inheritdoc IERC721Roles function revokeRole(address forAddress, uint256 tokenId, bytes4 roleId) external { _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = false; // Callback to the roles management contract. - _rolesManagement.afterRoleRevoked(forAddress, tokenId, roleId); + _rolesManagement.afterRoleRevoked(msg.sender, forAddress, tokenId, roleId); } /// @dev See {IERC165-supportsInterface}. diff --git a/contracts/token/ERC721/extensions/IERC721Roles.sol b/contracts/token/ERC721/extensions/IERC721Roles.sol index 9a3b28f9544..38369b1480d 100644 --- a/contracts/token/ERC721/extensions/IERC721Roles.sol +++ b/contracts/token/ERC721/extensions/IERC721Roles.sol @@ -23,9 +23,9 @@ interface IERC721RolesManagement is IERC165 { /// /// @dev Allows the roles management to cancel role withdrawal by reverting if it deems it /// necessary. The `IERC721Roles` is calling this function, so all information needed - /// can be queried through the `msg.sender`. This event is not called if a rental is - /// not in progress. - function afterRoleRevoked(address forAddress, uint256 tokenId, bytes4 roleId) external; + /// can be queried through the `msg.sender`. + /// @param fromAddress The address that called `IERC721Roles.revokeRole` + function afterRoleRevoked(address fromAddress, address forAddress, uint256 tokenId, bytes4 roleId) external; /// Function called at the end of `IERC721Roles.addRole`. /// @@ -33,8 +33,8 @@ interface IERC721RolesManagement is IERC165 { /// necessary. The `IERC721Roles` is calling this function, so all information needed /// can be queried through the `msg.sender`. /// - /// @param forAddress The address that called `IERC721Roles.addRole` - function afterRoleAdded(address forAddress, uint256 tokenId, bytes4 roleId) external; + /// @param fromAddress The address that called `IERC721Roles.addRole` + function afterRoleAdded(address fromAddress, address forAddress, uint256 tokenId, bytes4 roleId) external; } /// @title ERC721 token roles interface @@ -49,7 +49,7 @@ interface IERC721RolesManagement is IERC165 { /// if authorized by the currently set RoleManagement contract. /// /// A role is defined similarly to functions' methodId by the first 4 bytes of its hash. -/// For example, the renter role will be defined by bytes4(sha3("renter")) +/// For example, the renter role will be defined by bytes4(keccak256("renter")) interface IERC721Roles is IERC721 { /// Set the role management contract. From 7c4f147bf75ee57995d1f454b007929179dd6e7d Mon Sep 17 00:00:00 2001 From: Arthur Date: Wed, 23 Mar 2022 23:57:24 -0400 Subject: [PATCH 3/4] define roles manager contract at tokenId level --- .../ERC721/ERC721RolesRentalAgreement.sol | 96 ++++++++------- .../token/ERC721/extensions/ERC721Roles.sol | 81 +++++++------ .../token/ERC721/extensions/IERC721Roles.sol | 111 ++++++++++-------- 3 files changed, 163 insertions(+), 125 deletions(-) diff --git a/contracts/token/ERC721/ERC721RolesRentalAgreement.sol b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol index 3c411850758..b2205862955 100644 --- a/contracts/token/ERC721/ERC721RolesRentalAgreement.sol +++ b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol @@ -4,29 +4,38 @@ import "../../utils/Context.sol"; import "./extensions/IERC721Roles.sol"; import "../../utils/introspection/ERC165.sol"; -contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { +contract ERC721RolesRentalAgreement is Context, IERC721RolesManager, ERC165 { + // The status of the rent enum RentalStatus { pending, active, finished } + // A representation of rental agreement terms struct RentalAgreement { + // The duration of the rent, in seconds uint32 rentalDuration; + // The timestamp after which the rental agreement has expired and is no longer valid uint32 expirationDate; + // The timestamp corresponding to the start of the rental period uint32 startTime; + // The fees in wei that a renter needs to pay to start the rent uint256 rentalFees; RentalStatus rentalStatus; } + // The ERC721Roles token for which this contract can be used as a roles manager IERC721Roles public erc721Contract; + + // Mapping from tokenId to rental agreement mapping(uint256 => RentalAgreement) public tokenIdToRentalAgreement; - uint256[] public tokenRented; - // Mapping owners address to balances; + // Mapping addresses to balances mapping(address => uint256) public balances; - bytes4 public renterRoleId = bytes4(keccak256("ERC721Role::Renter")); + // The identifier of the role Renter + bytes4 public renterRoleId = bytes4(keccak256("ERC721Roles::Renter")); constructor(IERC721Roles _erc721Contract) { erc721Contract = _erc721Contract; @@ -41,15 +50,15 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { _; } - function afterRolesManagementRemoved() external onlyErc721Contract { - // The roles management contract can be updated only if all rentals are finished. - for (uint256 i = 0; i < tokenRented.length; i++) { - RentalStatus rentalStatus = tokenIdToRentalAgreement[tokenRented[i]].rentalStatus; - require(rentalStatus == RentalStatus.finished, "ERC721RolesRentalAgreement: rental is still active"); - } + function afterRolesManagerRemoved(uint256 tokenId) external view onlyErc721Contract { + RentalAgreement memory agreement = tokenIdToRentalAgreement[tokenId]; + require( + agreement.rentalStatus != RentalStatus.active, + "ERC721RolesRentalAgreement: can't remove the roles manager contract if there is an active rental" + ); } - // Allow the token holder or operator to set up a new rental agreement. + // Allow the token's owner or operator to set up a new rental agreement. function setRentalAgreement( uint256 tokenId, uint32 duration, @@ -61,27 +70,32 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { "ERC721RolesRentalAgreement: only owner or approver can set up a rental agreement" ); - RentalAgreement existing_agreement = tokenIdToRentalAgreement[token_id]; + RentalAgreement memory currentAgreement = tokenIdToRentalAgreement[tokenId]; require( - existing_agreement.rentalStatus != RentalStatus.active, + currentAgreement.rentalStatus != RentalStatus.active, "ERC721RolesRentalAgreement: can't update rental agreement if there is an active one already" ); - RentalAgreement rentalAgreement = RentalAgreement( + RentalAgreement memory rentalAgreement = RentalAgreement( duration, expirationDate, - uint32(block.timestamp), + 0, fees, RentalStatus.pending ); - tokenIdToRentalAgreement[token_id] = rentalAgreement; + // Set the rental agreement + tokenIdToRentalAgreement[tokenId] = rentalAgreement; } + // startRental allows an address to start the rent by paying the fees function startRental(address forAddress, uint256 tokenId) public payable { - RentalAgreement rentalAgreement = tokenIdToRentalAgreement[token_id]; - require(now <= rentalAgreement.expirationDate, "ERC721RolesRentalAgreement: rental agreement expired"); + RentalAgreement memory rentalAgreement = tokenIdToRentalAgreement[tokenId]; + require( + block.timestamp <= rentalAgreement.expirationDate, + "ERC721RolesRentalAgreement: rental agreement expired" + ); require( - rentalAgreement.rentalStatus != rentalStatus.active, + rentalAgreement.rentalStatus != RentalStatus.active, "ERC721RolesRentalAgreement: rental already in progress" ); @@ -89,52 +103,52 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { require(msg.value >= rentalFees, "ERC721RolesRentalAgreement: value below the rental fees"); address owner = erc721Contract.ownerOf(tokenId); - // Credit the fees to the owner's balance. + // Credit the fees to the owner's balance balances[owner] += rentalFees; - // Credit the remaining value to the sender's balance. + // Credit the remaining value to the sender's balance balances[_msgSender()] += msg.value - rentalFees; // Start the rental rentalAgreement.rentalStatus = RentalStatus.active; - rentalAgreement.startTime = now; + rentalAgreement.startTime = uint32(block.timestamp); - // Mark the token as rented. - tokenRented.push(tokenId); - - // Reflect the role in the ERC721 token. + // Reflect the role in the ERC721 token erc721Contract.addRole(forAddress, tokenId, renterRoleId); } + // stopRental stops the rental function stopRental(address forAddress, uint256 tokenId) public { - RentalAgreement rentalAgreement = tokenIdToRentalAgreement[token_id]; - require(rentalAgreement.rentalStatus == rentalStatus.active, "ERC721RolesRentalAgreement: rental not active"); + RentalAgreement memory rentalAgreement = tokenIdToRentalAgreement[tokenId]; + require(rentalAgreement.rentalStatus == RentalStatus.active, "ERC721RolesRentalAgreement: rental not active"); require( - now - rentalAgreement.startTime >= rentalAgreement.rentalDuration, + block.timestamp - rentalAgreement.startTime >= rentalAgreement.rentalDuration, "ERC721RolesRentalAgreement: rental still ongoing" ); // Stop the rental - rentalAgreement.rentalStatus = finished; + rentalAgreement.rentalStatus = RentalStatus.finished; - // Revoke the renter role + // Revoke the renter's role erc721Contract.revokeRole(forAddress, tokenId, renterRoleId); } + // afterRoleAdded will be called back in `erc721Contract.addRole` function afterRoleAdded( address fromAddress, - address forAddress, - uint256 tokenId, - bytes4 roleId - ) external onlyErc721Contract { + address, + uint256, + bytes4 + ) external view onlyErc721Contract { require(fromAddress == address(this), "ERC721RolesRentalAgreement: only this contract can set up renter roles"); } + // afterRoleRevoked will be called back in `erc721Contract.revokeRole` function afterRoleRevoked( address fromAddress, - address forAddress, - uint256 tokenId, - bytes4 roleId - ) external onlyErc721Contract { + address, + uint256, + bytes4 + ) external view onlyErc721Contract { require(fromAddress == address(this), "ERC721RolesRentalAgreement: only this contract can revoke renter roles"); } @@ -146,6 +160,7 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { erc721Contract.isApprovedForAll(owner, sender); } + // Allow addresses to redeem their funds function redeemFunds(uint256 _value) public { require(_value <= balances[_msgSender()], "ERC721RolesRentalAgreement: not enough funds to redeem"); @@ -153,9 +168,6 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManagement, ERC165 { // Check if the transfer is successful. require(_attemptETHTransfer(_msgSender(), _value), "ERC721RolesRentalAgreement: ETH transfer failed"); - - // Emit an event. - emit FundsRedeemed(_msgSender(), _value, balances[_msgSender()]); } function _attemptETHTransfer(address _to, uint256 _value) internal returns (bool) { diff --git a/contracts/token/ERC721/extensions/ERC721Roles.sol b/contracts/token/ERC721/extensions/ERC721Roles.sol index a5fff6c4404..6d57bfe1846 100644 --- a/contracts/token/ERC721/extensions/ERC721Roles.sol +++ b/contracts/token/ERC721/extensions/ERC721Roles.sol @@ -4,64 +4,77 @@ import "../ERC721.sol"; import "./IERC721Roles.sol"; contract ERC721Roles is ERC721, IERC721Roles { - // The address authorized to change the _roleManagement contract. - address private _rolesManager; - // The contract holding the logic defining the roles access. - IERC721RolesManagement private _rolesManagement; - // A record of the registered and active roles by tokenId. - mapping(uint256 => mapping(address => mapping(bytes4 => bool))) private _tokenIdRegisteredRoles; - - constructor(string memory name_, string memory symbol_, address rolesManager_) ERC721(name_, symbol_){ - _rolesManager = rolesManager_; - } + // Mapping from token ID to roles management contract + mapping(uint256 => IERC721RolesManager) private _rolesManager; - /// @inheritdoc IERC721Roles - function setRolesManager(address rolesManager_) external { - require(_msgSender()==_rolesManager, "ERC721: caller is not the rolesManager"); - _rolesManager = rolesManager_; - } + // A record of the registered and active roles by tokenId + mapping(uint256 => mapping(address => mapping(bytes4 => bool))) private _tokenIdRegisteredRoles; /// @inheritdoc IERC721Roles - function rolesManager() public view returns (address) { - return _rolesManager; - } + function setRolesManager(uint256 tokenId, IERC721RolesManager tokenRolesManager) external { + require( + _isApprovedOrOwner(_msgSender(), tokenId), + "ERC721Roles: only owner or approver can change roles manager" + ); - /// @inheritdoc IERC721Roles - function setRolesManagement(IERC721RolesManagement rolesManagement_) external { - require(_msgSender()==_rolesManager, "ERC721: caller is not the rolesManager"); - if (address(_rolesManagement) != address(0)) { - _rolesManagement.afterRolesManagementRemoved(); + // If there is an existing roles manager contract, call `IERC721RolesManager.afterRolesManagerRemoved` + IERC721RolesManager currentRolesManager = _rolesManager[tokenId]; + if (address(currentRolesManager) != address(0)) { + currentRolesManager.afterRolesManagerRemoved(tokenId); } - _rolesManagement = rolesManagement_; + // Update the roles manager contrac + _rolesManager[tokenId] = tokenRolesManager; } /// @inheritdoc IERC721Roles - function rolesManagement() public view returns (IERC721RolesManagement){ - return _rolesManagement; + function rolesManager(uint256 tokenId) public view returns (IERC721RolesManager) { + return _rolesManager[tokenId]; } /// @inheritdoc IERC721Roles - function roleGranted(address user, uint256 tokenId, bytes4 roleId) external view returns (bool) { + function roleGranted( + address user, + uint256 tokenId, + bytes4 roleId + ) external view returns (bool) { return _tokenIdRegisteredRoles[tokenId][user][roleId]; } /// @inheritdoc IERC721Roles - function addRole(address forAddress, uint256 tokenId, bytes4 roleId) external { + function addRole( + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external { + IERC721RolesManager manager = _rolesManager[tokenId]; + require(address(manager) != address(0), "ERC721Roles: no roles manager set up"); + + // Register the new role _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = true; - // Callback to the roles management contract. - _rolesManagement.afterRoleAdded(msg.sender, forAddress, tokenId, roleId); + + // Callback to the roles manager contract + manager.afterRoleAdded(_msgSender(), forAddress, tokenId, roleId); } /// @inheritdoc IERC721Roles - function revokeRole(address forAddress, uint256 tokenId, bytes4 roleId) external { + function revokeRole( + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external { + IERC721RolesManager manager = _rolesManager[tokenId]; + require(address(manager) != address(0), "ERC721Roles: no roles manager set up"); + + // De-register the role _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = false; - // Callback to the roles management contract. - _rolesManagement.afterRoleRevoked(msg.sender, forAddress, tokenId, roleId); + + // Callback to the roles manager contract + manager.afterRoleRevoked(_msgSender(), forAddress, tokenId, roleId); } /// @dev See {IERC165-supportsInterface}. function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC721) returns (bool) { return interfaceId == type(IERC721Roles).interfaceId || super.supportsInterface(interfaceId); } -} \ No newline at end of file +} diff --git a/contracts/token/ERC721/extensions/IERC721Roles.sol b/contracts/token/ERC721/extensions/IERC721Roles.sol index 38369b1480d..e45f90481ed 100644 --- a/contracts/token/ERC721/extensions/IERC721Roles.sol +++ b/contracts/token/ERC721/extensions/IERC721Roles.sol @@ -4,91 +4,104 @@ pragma solidity ^0.8.0; import "../IERC721.sol"; import "../../../utils/introspection/IERC165.sol"; -/// @title ERC721 token roles management interface +/// @title ERC721 token roles manager interface /// -/// Defines the interface that a role management contract should support to be used by +/// Defines the interface that a roles manager contract should support to be used by /// `IERC721Roles`. -interface IERC721RolesManagement is IERC165 { - - /// Function called at the end of `IERC721Roles.setRolesManagement` on the roles management contract +interface IERC721RolesManager is IERC165 { + /// Function called at the end of `IERC721Roles.setRolesManager` on the roles manager contract /// currently set for the token, if one exists. /// - /// @dev Allows the roles management to cancel the change by reverting if it deems it + /// @dev Allows the roles manager to cancel the change by reverting if it deems it /// necessary. The `IERC721Roles` is calling this function, so all information needed - /// can be queried through the `msg.sender`. This event is only called when their are no specific roles - /// set as it is not allowed to change the role manager when there are active - function afterRolesManagementRemoved() external; + /// can be queried through the `msg.sender`. + function afterRolesManagerRemoved(uint256 tokenId) external; - /// Function called at the end of `IERC721Roles.revokeRole` - /// - /// @dev Allows the roles management to cancel role withdrawal by reverting if it deems it + /// Function called at the end of `IERC721Roles.revokeRole` + /// + /// @dev Allows the roles manager to cancel role withdrawal by reverting if it deems it /// necessary. The `IERC721Roles` is calling this function, so all information needed /// can be queried through the `msg.sender`. /// @param fromAddress The address that called `IERC721Roles.revokeRole` - function afterRoleRevoked(address fromAddress, address forAddress, uint256 tokenId, bytes4 roleId) external; + function afterRoleRevoked( + address fromAddress, + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external; /// Function called at the end of `IERC721Roles.addRole`. /// - /// @dev Allows the roles management to prevent adding a new role if it deems it + /// @dev Allows the roles manager to prevent adding a new role if it deems it /// necessary. The `IERC721Roles` is calling this function, so all information needed /// can be queried through the `msg.sender`. /// /// @param fromAddress The address that called `IERC721Roles.addRole` - function afterRoleAdded(address fromAddress, address forAddress, uint256 tokenId, bytes4 roleId) external; + function afterRoleAdded( + address fromAddress, + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external; } /// @title ERC721 token roles interface /// -/// Defines the optional interface that allows setting roles for users by tokenId. -/// It delegates the logic of adding and revoking roles to a roles management contract implementing the -/// `IERC721RolesManagement` interface. -/// A user can hold multiple roles and multiple users can be granted the same role. It's the -/// responsability of the roles management contract to allow such interactions. +/// Defines the optional interface that allows setting roles for users by tokenId. +/// It delegates the logic of adding and revoking roles to a roles manager contract implementing the +/// `IERC721RolesManager` interface. +/// A user could hold multiple roles and multiple users could be granted the same role. It's the +/// responsability of the roles manager contract to allow such permissions. /// -/// The roles manager is allowed to change the roles Management contract, -/// if authorized by the currently set RoleManagement contract. +/// Only the token's owner or an approver is able to change the roles manager contract, +/// if authorized by the currently set RolesManager contract. /// /// A role is defined similarly to functions' methodId by the first 4 bytes of its hash. -/// For example, the renter role will be defined by bytes4(keccak256("renter")) +/// For example, the renter role will be defined by bytes4(keccak256("ERC721Roles::Renter")) interface IERC721Roles is IERC721 { - /// Set the role management contract. + /// Set the roles manager contract for a token. /// - /// A previously set roles management contract must accept the change. - /// The caller must be the roles manager, the address authorized make such a change + /// A previously set roles manager contract must accept the change. + /// The caller must be the token's owner or operator. /// - /// @dev If a roles management was already set before this call, calls its - /// `IERC721RolesManagement.afterRolesManagementRemoved` at the end of the call. + /// @dev If a roles manager contract was already set before this call, calls its + /// `IERC721RolesManager.afterRolesManagerRemoved` at the end of the call. /// - /// @param rolesManagement The roles management contract. Set to 0 to remove the current roles management. - function setRolesManagement(IERC721RolesManagement rolesManagement) external; - - /// @return the address of the roles management, or 0 if there is no roles management set. - function rolesManagement() external returns (IERC721RolesManagement); - - /// Set the address authorized to change the roles management contract. - /// @param rolesManager the new roles manager. - function setRolesManager(address rolesManager) external; + /// @param rolesManager The roles manager contract. Set to 0 to remove the current roles manager. + function setRolesManager(uint256 tokenId, IERC721RolesManager rolesManager) external; - /// @return the roles manager. - function rolesManager() external view returns(address); + /// @return the address of the roles manager, or 0 if there is no roles manager set. + function rolesManager(uint256 tokenid) external returns (IERC721RolesManager); /// @return true if the role has been granted to the user. - function roleGranted(address user, uint256 tokenId, bytes4 roleId) external view returns (bool); + function roleGranted( + address user, + uint256 tokenId, + bytes4 roleId + ) external view returns (bool); - /// Set the role for the address and the token. + /// Set the role for the address and the token. /// - /// The token must have a roles management contract set. - /// The roles management contract must accept the new role for the address + /// The token must have a roles manager contract set. + /// The roles manager contract must accept the new role for the address /// - /// @dev Calls `IERC721RolesManagement.afterRoleAdded` at the end of the call. - function addRole(address forAddress, uint256 tokenId, bytes4 roleId) external; + /// @dev Calls `IERC721RolesManager.afterRoleAdded` at the end of the call. + function addRole( + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external; /// Revoke the role for the token and the address. /// - /// The token must have a roles management contract set. - /// The roles management contract must accept the role withdrawal for this address. + /// The token must have a roles manager contract set. + /// The roles manager contract must accept the role withdrawal for this address. /// - /// @dev Calls `IERC721RolesManagement.afterRoleRevoked` at the end of the call. - function revokeRole(address forAddress, uint256 tokenId, bytes4 roleId) external; + /// @dev Calls `IERC721RolesManager.afterRoleRevoked` at the end of the call. + function revokeRole( + address forAddress, + uint256 tokenId, + bytes4 roleId + ) external; } From 2f3f49d7b56f48a6095bb35b2d56ea3ad03624b9 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 24 Mar 2022 12:44:20 -0400 Subject: [PATCH 4/4] addRole -> grantRole --- contracts/token/ERC721/ERC721RolesRentalAgreement.sol | 6 +++--- contracts/token/ERC721/extensions/ERC721Roles.sol | 6 +++--- contracts/token/ERC721/extensions/IERC721Roles.sol | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC721/ERC721RolesRentalAgreement.sol b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol index b2205862955..3c7bc531bae 100644 --- a/contracts/token/ERC721/ERC721RolesRentalAgreement.sol +++ b/contracts/token/ERC721/ERC721RolesRentalAgreement.sol @@ -113,7 +113,7 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManager, ERC165 { rentalAgreement.startTime = uint32(block.timestamp); // Reflect the role in the ERC721 token - erc721Contract.addRole(forAddress, tokenId, renterRoleId); + erc721Contract.grantRole(forAddress, tokenId, renterRoleId); } // stopRental stops the rental @@ -132,8 +132,8 @@ contract ERC721RolesRentalAgreement is Context, IERC721RolesManager, ERC165 { erc721Contract.revokeRole(forAddress, tokenId, renterRoleId); } - // afterRoleAdded will be called back in `erc721Contract.addRole` - function afterRoleAdded( + // afterRoleGranted will be called back in `erc721Contract.grantRole` + function afterRoleGranted( address fromAddress, address, uint256, diff --git a/contracts/token/ERC721/extensions/ERC721Roles.sol b/contracts/token/ERC721/extensions/ERC721Roles.sol index 6d57bfe1846..97098ec5325 100644 --- a/contracts/token/ERC721/extensions/ERC721Roles.sol +++ b/contracts/token/ERC721/extensions/ERC721Roles.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.0; import "../ERC721.sol"; import "./IERC721Roles.sol"; -contract ERC721Roles is ERC721, IERC721Roles { +abstract contract ERC721Roles is ERC721, IERC721Roles { // Mapping from token ID to roles management contract mapping(uint256 => IERC721RolesManager) private _rolesManager; @@ -42,7 +42,7 @@ contract ERC721Roles is ERC721, IERC721Roles { } /// @inheritdoc IERC721Roles - function addRole( + function grantRole( address forAddress, uint256 tokenId, bytes4 roleId @@ -54,7 +54,7 @@ contract ERC721Roles is ERC721, IERC721Roles { _tokenIdRegisteredRoles[tokenId][forAddress][roleId] = true; // Callback to the roles manager contract - manager.afterRoleAdded(_msgSender(), forAddress, tokenId, roleId); + manager.afterRoleGranted(_msgSender(), forAddress, tokenId, roleId); } /// @inheritdoc IERC721Roles diff --git a/contracts/token/ERC721/extensions/IERC721Roles.sol b/contracts/token/ERC721/extensions/IERC721Roles.sol index e45f90481ed..25a441d7430 100644 --- a/contracts/token/ERC721/extensions/IERC721Roles.sol +++ b/contracts/token/ERC721/extensions/IERC721Roles.sol @@ -30,14 +30,14 @@ interface IERC721RolesManager is IERC165 { bytes4 roleId ) external; - /// Function called at the end of `IERC721Roles.addRole`. + /// Function called at the end of `IERC721Roles.grantRole`. /// /// @dev Allows the roles manager to prevent adding a new role if it deems it /// necessary. The `IERC721Roles` is calling this function, so all information needed /// can be queried through the `msg.sender`. /// - /// @param fromAddress The address that called `IERC721Roles.addRole` - function afterRoleAdded( + /// @param fromAddress The address that called `IERC721Roles.grantRole` + function afterRoleGranted( address fromAddress, address forAddress, uint256 tokenId, @@ -86,8 +86,8 @@ interface IERC721Roles is IERC721 { /// The token must have a roles manager contract set. /// The roles manager contract must accept the new role for the address /// - /// @dev Calls `IERC721RolesManager.afterRoleAdded` at the end of the call. - function addRole( + /// @dev Calls `IERC721RolesManager.afterRoleGranted` at the end of the call. + function grantRole( address forAddress, uint256 tokenId, bytes4 roleId