From d9a6095f93560f040f49385b808937a3af6cea1b Mon Sep 17 00:00:00 2001 From: Ilan Olkies Date: Thu, 21 Nov 2019 00:50:09 -0300 Subject: [PATCH 1/4] Document RSKOwner --- contracts/RSKOwner.sol | 142 ++++++++++++++++++++++++++++++++++------- 1 file changed, 120 insertions(+), 22 deletions(-) diff --git a/contracts/RSKOwner.sol b/contracts/RSKOwner.sol index 730b23f..f5861a0 100644 --- a/contracts/RSKOwner.sol +++ b/contracts/RSKOwner.sol @@ -14,9 +14,6 @@ contract RSKOwner is ERC721, Ownable { AbstractRNS private rns; bytes32 private rootNode; - Roles.Role registrars; - Roles.Role renewers; - mapping (uint256 => uint) public expirationTime; event ExpirationChanged(uint256 tokenId, uint expirationTime); @@ -46,11 +43,18 @@ contract RSKOwner is ERC721, Ownable { rootNode = _rootNode; } + /// @notice Gets the owner of the specified domain. + /// @param tokenId keccak256 of the domain label. + /// @return domain owner. function ownerOf(uint256 tokenId) public view returns (address) { require(expirationTime[tokenId] > now, "ERC721: owner query for nonexistent token"); return super.ownerOf(tokenId); } + /// @notice Check if a domain is available to be registered. + /// @dev The name must be registered via account with registrar role. + /// @param tokenId keccak256 of the domain label. + /// @return true if the specified domain can be registered. function available(uint256 tokenId) public view returns(bool) { return ( expirationTime[tokenId] < now && @@ -58,7 +62,43 @@ contract RSKOwner is ERC721, Ownable { ); } - // Auction migration + /////////////////// + // RSK TLD ADMIN // + /////////////////// + + /* + This contract owns a node in RNS, so it is capable to + change it's resolution and ttl. + */ + + /// @notice set root node resolver in RNS. + /// @param resolver to be set. + function setRootResolver (address resolver) external onlyOwner { + rns.setResolver(rootNode, resolver); + } + + /// @notice set root node ttl in RNS. + /// @param ttl to be set. + function setRootTTL (uint64 ttl) external onlyOwner { + rns.setTTL(rootNode, ttl); + } + + /////////////////////// + // AUCTION MIGRATION // + /////////////////////// + + /* + A domain registered with previous registrar (auction) + should be transfered before it can be renewed. If the + domain is not transfered and it expires, it might be + registered by someone else. + */ + + /// @notice Accept domain transfer from previous registrar. + /// @dev Use it via tokenRegistrar.trnasferRegistrars(label). + /// All locked tokens in Deed are returned. + /// @param label Accepted domain label. + /// @param deed Deed contract address holding tokens. function acceptRegistrarTransfer(bytes32 label, TokenDeed deed, uint) external onlyPreviousRegistrar { uint256 tokenId = uint256(label); expirationTime[tokenId] = deed.expirationDate(); @@ -66,20 +106,44 @@ contract RSKOwner is ERC721, Ownable { deed.closeDeed(1000); } - // Registrar role + ////////////////// + // REGISTRATION // + ////////////////// + + /* + Only available domains can be registered. Once a domain is + registered, it cannot be revoked until expiration. + */ + + // An account with registrar role can register domains. + Roles.Role registrars; + + /// @notice Give an account access to registrar role. + /// @dev Only owner. + /// @param registrar new registrar. function addRegistrar(address registrar) external onlyOwner { registrars.add(registrar); } + /// @notice Check if an account has registrar role. + /// @param registrar to query if has registrar role. + /// @return true if it has registrar role. function isRegistrar(address registrar) external view returns (bool) { return registrars.has(registrar); } + /// @notice Remove an account's access to registrar role. + /// @dev Only owner + /// @param registrar registrar to remove from registrar role. function removeRegistrar(address registrar) external onlyOwner { registrars.remove(registrar); } - // Registration + /// @notice Registers a domain in RNS for a given duration. + /// @dev Only accounts with registrar role. + /// @param label keccak256 of the domain label to register. + /// @param tokenOwner account that will own the registered domain. + /// @param duration time to register the domain for. function register(bytes32 label, address tokenOwner, uint duration) external onlyRegistrar { uint256 tokenId = uint256(label); @@ -97,26 +161,62 @@ contract RSKOwner is ERC721, Ownable { rns.setSubnodeOwner(rootNode, label, tokenOwner); } - // Reclaim - function reclaim(uint256 id, address newOwner) external { - require(_isApprovedOrOwner(msg.sender, id), "Not approved or owner"); - rns.setSubnodeOwner(rootNode, bytes32(id), newOwner); + //////////////// + // RECLAIMING // + //////////////// + + /* + Names might be lost by transferring to contracts, or by + error. This allows any owner (or approved) to reclaim the + domain ownership in RNS. + */ + + /// @notice Reclaim ownership of a domain in RNS. + /// @dev Only owner or approved for the domain. + /// @param tokenId keccak256 of the domain + /// @param newOwner the owner to set in RNS. + function reclaim(uint256 tokenId, address newOwner) external { + require(_isApprovedOrOwner(msg.sender, tokenId), "Not approved or owner"); + rns.setSubnodeOwner(rootNode, bytes32(tokenId), newOwner); } - // Renewer role + //////////////// + // RENOVATION // + //////////////// + + /* + Only owned domains can be renewed. A renovation extends + domain ownership. + */ + + // An account with renewer role can extend domain expirations. + Roles.Role renewers; + + /// @notice Give an account access to renewer role. + /// @dev Only owner + /// @param renewer new renewer. function addRenewer(address renewer) external onlyOwner { renewers.add(renewer); } + /// @notice Check if an account has renewer role. + /// @param renewer to query if has renewer role. + /// @return true if it has renewer role. function isRenewer(address renewer) external view returns (bool) { return renewers.has(renewer); } + /// @notice Remove an account's access to renewer role. + /// @dev Only owner + /// @param renewer renewer to remove from renewer role. function removeRenewer(address renewer) external onlyOwner { renewers.remove(renewer); } - // Renovation + /// @notice Renew a domain for a given duraiton. + /// @dev Only accounts with renewer role. + /// @param label keccak256 of the domain label to renew. + /// @param time to extend the duration for. function renew (bytes32 label, uint time) external onlyRenewer { uint256 tokenId = uint256(label); require(expirationTime[tokenId] > now, "Name already expired"); @@ -125,7 +225,14 @@ contract RSKOwner is ERC721, Ownable { emit ExpirationChanged(tokenId, newExpirationTime); } - // After expiration + ////////////////////// + // AFTER EXPIRATION // + ////////////////////// + + /// @notice This method removes expired domains. + /// @dev Use this to set 0 address in RNS ownership + /// and burn the domains to keep balance up to date. + /// @param tokenIds keccak256s of the domain labels to remove. function removeExpired(uint256[] calldata tokenIds) external { uint256 tokenId; bytes32 label; @@ -143,13 +250,4 @@ contract RSKOwner is ERC721, Ownable { } } } - - // rsk admin - function setRootResolver (address resolver) external onlyOwner { - rns.setResolver(rootNode, resolver); - } - - function setRootTTL (uint64 ttl) external onlyOwner { - rns.setTTL(rootNode, ttl); - } } From c7fe187403f305536c307142825177effef21a84 Mon Sep 17 00:00:00 2001 From: Ilan Olkies Date: Thu, 21 Nov 2019 01:04:36 -0300 Subject: [PATCH 2/4] Document FIFS --- contracts/FIFSRegistrar.sol | 161 ++++++++++++++++++++---------------- 1 file changed, 91 insertions(+), 70 deletions(-) diff --git a/contracts/FIFSRegistrar.sol b/contracts/FIFSRegistrar.sol index 4c7413d..afcb75c 100644 --- a/contracts/FIFSRegistrar.sol +++ b/contracts/FIFSRegistrar.sol @@ -9,11 +9,9 @@ import "@ensdomains/ethregistrar/contracts/StringUtils.sol"; import "./testing/ERC677Receiver.sol"; import "./BytesUtils.sol"; -/// @title First-in first-served registrar +/// @title First-in first-served registrar. /// @notice You can use this contract to register .rsk names in RNS. -/// First make a commitment of the name to be registered, wait 1 -/// minute, and proceed to register the name. -/// @dev This contract has permission to register in RSK Owner +/// @dev This contract has permission to register in RSK Owner. contract FIFSRegistrar is PricedContract, ERC677Receiver { using SafeMath for uint256; using StringUtils for string; @@ -42,73 +40,111 @@ contract FIFSRegistrar is PricedContract, ERC677Receiver { pool = _pool; } - /// @notice Create a commitment for register action - /// @dev Don't use this method on-chain when commiting - /// @param label keccak256 of the name to be registered - /// @param nameOwner Owner of the name to be registered - /// @param secret Secret to protect the name to be registered - /// @return The commitment hash + /////////////////// + // COMMIT-REVEAL // + /////////////////// + + /* + 0. Caclulate makeCommitment hash of the domain to be registered (off-chain) + 1. Commit the calculated hash + 2. Wait minCommitmentAge + 3. Execute registration via: + - ERC-20 with approve() + register() + - ERC-677 with transferAndCall() + The price of a domain is given by name price contract. + */ + + // 0. + /// @notice Create a commitment for register action. + /// @dev Don't use this method on-chain when commiting. + /// @param label keccak256 of the name to be registered. + /// @param nameOwner Owner of the name to be registered. + /// @param secret Secret to protect the name to be registered. + /// @return The commitment hash. function makeCommitment (bytes32 label, address nameOwner, bytes32 secret) public pure returns (bytes32) { return keccak256(abi.encodePacked(label, nameOwner, secret)); } - /// @notice Commit before registring a name - /// @dev A valid commitment can be calculated using makeCommitment off-chain - /// @param commitment A valid commitment hash + // 1. + /// @notice Commit before registring a name. + /// @dev A valid commitment can be calculated using makeCommitment off-chain. + /// @param commitment A valid commitment hash. function commit(bytes32 commitment) external { require(commitmentRevealTime[commitment] < 1, "Existent commitment"); commitmentRevealTime[commitment] = now.add(minCommitmentAge); } - /// @notice Ensure the commitment is ready to be revealed - /// @param commitment Commitment to be queried - /// @return Wether the commitment can be revealed or not + // 2. + /// @notice Ensure the commitment is ready to be revealed. + /// @dev This method can be polled to ensure registration. + /// @param commitment Commitment to be queried. + /// @return Wether the commitment can be revealed or not. function canReveal(bytes32 commitment) public view returns (bool) { uint revealTime = commitmentRevealTime[commitment]; return 0 < revealTime && revealTime <= now; } - /// @notice Registers a .rsk name in RNS - /// @dev This method must be called after commiting - /// @param name The name to register - /// @param nameOwner The owner of the name to regiter - /// @param secret The secret used to make the commitment - /// @param duration Time to register in years + // 3. + + // - Via ERC-20 + /// @notice Registers a .rsk name in RNS. + /// @dev This method must be called after commiting. + /// @param name The name to register. + /// @param nameOwner The owner of the name to regiter. + /// @param secret The secret used to make the commitment. + /// @param duration Time to register in years. function register(string calldata name, address nameOwner, bytes32 secret, uint duration) external { uint cost = executeRegistration(name, nameOwner, secret, duration); require(rif.transferFrom(msg.sender, pool, cost), "Token transfer failed"); } - /// @notice Change required commitment maturity - /// @dev Only owner - /// @param newMinCommitmentAge The new maturity required - function setMinCommitmentAge (uint newMinCommitmentAge) external onlyOwner { - minCommitmentAge = newMinCommitmentAge; - } + // - Via ERC-677 + /* Encoding: + | signature | 4 bytes - offset 0 + | owner | 20 bytes - offset 4 + | secret | 32 bytes - offest 24 + | duration | 32 bytes - offset 56 + | name | variable size - offset 88 + */ - /// @notice Change disbaled names - /// @dev Only owner - /// @param newMinLength The new minimum length enabled - function setMinLength (uint newMinLength) external onlyOwner { - minLength = newMinLength; + /// @notice ERC-677 token fallback function. + /// @dev Follow 'Register encoding' to execute a one-transaction regitration. + /// @param from token sender. + /// @param value amount of tokens sent. + /// @param data data associated with transaction. + /// @return true if successfull. + function tokenFallback(address from, uint value, bytes calldata data) external returns (bool) { + require(msg.sender == address(rif), "Only RIF token"); + require(data.length > 88, "Invalid data"); + + bytes4 signature = data.toBytes4(0); + + require(signature == REGISTER_SIGNATURE, "Invalid signature"); + + address nameOwner = data.toAddress(4); + bytes32 secret = data.toBytes32(24); + uint duration = data.toUint(56); + string memory name = data.toString(88, data.length.sub(88)); + + registerWithToken(name, nameOwner, secret, duration, from, value); + + return true; } function registerWithToken(string memory name, address nameOwner, bytes32 secret, uint duration, address from, uint amount) private { uint cost = executeRegistration(name, nameOwner, secret, duration); require(amount >= cost, "Not enough tokens"); require(rif.transfer(pool, cost), "Token transfer failed"); - // Calculated twise because the common case is the exact amount is sent. No variables. if (amount.sub(cost) > 0) require(rif.transfer(from, amount.sub(cost)), "Token transfer failed"); } - /// @notice Executes registration without any payments. - /// @dev This method is used to abstract from payment method. - /// @param name The name to register - /// @param nameOwner The owner of the name to regiter - /// @param secret The secret used to make the commitment - /// @param duration Time to register in years - /// @return price Price of the name to register + /// @notice Executes registration abstracted from payment method. + /// @param name The name to register. + /// @param nameOwner The owner of the name to regiter. + /// @param secret The secret used to make the commitment. + /// @param duration Time to register in years. + /// @return price Price of the name to register. function executeRegistration (string memory name, address nameOwner, bytes32 secret, uint duration) private returns (uint) { bytes32 label = keccak256(abi.encodePacked(name)); @@ -123,36 +159,21 @@ contract FIFSRegistrar is PricedContract, ERC677Receiver { return price(name, rskOwner.expirationTime(uint(label)), duration); } - /** - Register encoding: - | signature | 4 bytes - offset 0 - | owner | 20 bytes - offset 4 - | secret | 32 bytes - offest 24 - | duration | 32 bytes - offset 56 - | name | variable size - offset 88 - */ - - /// @notice ERC-677 token fallback function - /// @dev Follow 'Register encoding' to execute a one-transaction regitration. - /// @param from token sender - /// @param value amount of tokens sent - /// @param data data associated with transaction - /// @return true if successfull - function tokenFallback(address from, uint value, bytes calldata data) external returns (bool) { - require(msg.sender == address(rif), "Only RIF token"); - require(data.length > 88, "Invalid data"); - - bytes4 signature = data.toBytes4(0); - - require(signature == REGISTER_SIGNATURE, "Invalid signature"); - - address nameOwner = data.toAddress(4); - bytes32 secret = data.toBytes32(24); - uint duration = data.toUint(56); - string memory name = data.toString(88, data.length.sub(88)); + ///////////////////// + // REGISTRAR ADMIN // + ///////////////////// - registerWithToken(name, nameOwner, secret, duration, from, value); + /// @notice Change required commitment maturity. + /// @dev Only owner. + /// @param newMinCommitmentAge The new maturity required. + function setMinCommitmentAge (uint newMinCommitmentAge) external onlyOwner { + minCommitmentAge = newMinCommitmentAge; + } - return true; + /// @notice Change disbaled names. + /// @dev Only owner. + /// @param newMinLength The new minimum length enabled. + function setMinLength (uint newMinLength) external onlyOwner { + minLength = newMinLength; } } From 05acb332ed8544a9479a97aafafa239e6cda67e6 Mon Sep 17 00:00:00 2001 From: Ilan Olkies Date: Thu, 21 Nov 2019 01:45:12 -0300 Subject: [PATCH 3/4] Update readme --- README.md | 75 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index ff2f05c..9f45e58 100644 --- a/README.md +++ b/README.md @@ -1,19 +1,80 @@ -# rns-fifs -First-in first-served registrar for rsk top level domain. +# RSK Registrar +RNS Registrar for _rsk_ top level domain.1 -## Run tests +## Contracts + +The registrar is separated into several components for simplicity, modularity, and privilege minimization. + +### RSK Owner + +Owner of _rsk_ top level domain. It can `setSubdomainOwner` in RNS. + +- It represents domain ownership implementing ERC-721c1 non-fungible token standard. This standard provides basic functionality to track and transfer NFTs2. +- Stores domains' expiration time. The expiration time determines wether a domain is owned or note. +- Determines if a domain is available to be purchased. +- Accepts domain ownership clamming from previous _rsk_ registrar. +- Grants access to other contracts for registering new domains (registrar role)2. +- Grants access to other contracts for renewing domains (renewer role)2. +- Allows to reclaim ownership in RNS of owned domains. +- It has an owner that can2 + - Change _rsk_ tld resolver and ttl. + - Add/remove registrar contracts. + - Add/remove renewer contracts. + +### FIFS Registrar + +Has registration role in `RSK Owner`. + +- Defines a commit-reveal process to avoid front-running. +- Accepts payments via + - ERC-20 `approve()` + `register()`.3 + - ERC-721 `transferAndCall()`.4 +- Calculates price using `NamePrice` contract. +- It has an owner that can2 + - Set minimum commitment age. + - Set minimum registration name length available. + - Change name price contract. + +The registration must be performed as follows: +0. Calculate `makeCommitment` hash of the domain to be registered (off-chain). +1. Commit the calculated hash using `commit`. +2. Wait `minCommitmentAge` seconds. +3. Execute registration via ERC-20 (with approval) or ERC-677. + +> Find `transferAndCall()` encoder in `utils/index.js` + +### Name Price + +Determines the price of a domain. + +| Years | Price | +| - | - | +| 1 | 2 RIF | +| 2 | 4 RIF | +| 2+k | 4+k RIF | + +> For example, 5 years cost 7 RIF. + +## Setup -Unit tests: ``` -truffle test +npm install ``` -> Get truffle: https://www.trufflesuite.com/ +## Run tests -Static analyzes: ``` +truffle test slither . ``` +> Get truffle: https://www.trufflesuite.com/ > Get slither:https://github.com/crytic/slither + +## References + +1. Strongly based on https://github.com/ensdomains/ethregistrar. +2. https://github.com/OpenZeppelin/openzeppelin-contracts implementation. +3. https://eips.ethereum.org/EIPS/eip-20 +4. https://github.com/ethereum/EIPs/issues/677 \ No newline at end of file From 1e5f87c6ed0627d9064d9df81cbba793f797af78 Mon Sep 17 00:00:00 2001 From: Ilan <36084092+ilanolkies@users.noreply.github.com> Date: Thu, 21 Nov 2019 02:12:48 -0300 Subject: [PATCH 4/4] Update README.md --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9f45e58..6d827d8 100644 --- a/README.md +++ b/README.md @@ -10,8 +10,8 @@ The registrar is separated into several components for simplicity, modularity, a Owner of _rsk_ top level domain. It can `setSubdomainOwner` in RNS. -- It represents domain ownership implementing ERC-721c1 non-fungible token standard. This standard provides basic functionality to track and transfer NFTs2. -- Stores domains' expiration time. The expiration time determines wether a domain is owned or note. +- It represents domain ownership implementing ERC-7211 non-fungible token standard. This standard provides basic functionality to track and transfer NFTs2. +- Stores domains' expiration time. The expiration time determines wether a domain is owned or not. - Determines if a domain is available to be purchased. - Accepts domain ownership clamming from previous _rsk_ registrar. - Grants access to other contracts for registering new domains (registrar role)2. @@ -37,6 +37,7 @@ Has registration role in `RSK Owner`. - Change name price contract. The registration must be performed as follows: + 0. Calculate `makeCommitment` hash of the domain to be registered (off-chain). 1. Commit the calculated hash using `commit`. 2. Wait `minCommitmentAge` seconds. @@ -77,4 +78,4 @@ slither . 1. Strongly based on https://github.com/ensdomains/ethregistrar. 2. https://github.com/OpenZeppelin/openzeppelin-contracts implementation. 3. https://eips.ethereum.org/EIPS/eip-20 -4. https://github.com/ethereum/EIPs/issues/677 \ No newline at end of file +4. https://github.com/ethereum/EIPs/issues/677