status-im / ens-usernames

DApp to register usernames for Status Network
MIT License
19 stars 11 forks source link

Update solidity to 0.6.2 and to openzeppelin-contracts 3.2.0 #119

Closed 3esmit closed 3 years ago

3esmit commented 3 years ago

This is mostly a solidity update, and a small improvement requested by Crytic (https://github.com/status-im/ens-usernames/issues/114) that needed new solidity, so as the new feature (try-catch) is there in solidity its now used.

Other than that, the surrounding changes were to apply with the new solidity. A new contract had to be added, ERC721, because Openzeppelin implementation is incompatible with EthRegistrarImplementation (see https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2373)

3esmit commented 3 years ago

There were also changes in BaseRegistrarImplementation.sol related to the introspection interface (ERC165). The BaseRegistrarImplementation was overriding the method instead of using the internal function to register it's interface. With the new version of solidity, the parent contracts can define if a function is able to be overwritten. This function was not market virtual by OpenZeppelin's implementation of ERC165 with the purpose of use it's internal mechanism. Therefore, this change does not affect the behavior of the contract, but changes the gas cost in the supportInterface() function, which is not likely used in-chain for BaseRegistrarImplementation, and even if it was, it would be using the official deployed version, not the changed contract in this commit. BaseRegistrarImplementation is not being tested or developed in this system, it was included on the source code only for integration tests related to the reclaim function.