Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 25, 2020

Conversation

3esmit
Copy link
Member

@3esmit 3esmit commented Sep 25, 2020

No description provided.

@3esmit
Copy link
Member Author

3esmit commented Sep 25, 2020

This is mostly a solidity update, and a small improvement requested by Crytic (#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 OpenZeppelin/openzeppelin-contracts#2373)

@3esmit 3esmit changed the title Update solidity to 0.6.2 Update solidity to 0.6.2 and to openzeppelin-contracts 3.2.0 Sep 25, 2020
@3esmit
Copy link
Member Author

3esmit commented Sep 25, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant