OpenZeppelin Contracts v3.0 Pending Changes

[nventuro]

This is a non-comprehensive list of changes we want to introduce in the v3.0, the upcoming major release. We will use this opportunity to introduce some breaking changes by removing contracts we don't see much demand for (such as Crowdsales), and improving others we could can be done better (like Roles and Access Control in general).

For more information about the v3.0 release, head to our roadmap.

• Migrate to Solidity v0.6.x [Migrate Contracts to Solidity v0.6 #2080]
• Design a new access control solution (see the discussion in this forum post)
• Include optional extensions to some of our standard implementations (such as ERC20Detailed, possibly ERC721Metadata and ERC721Enumerable)
• Review contracts in drafts and either remove or promote out of there
• Make IERC721 contracts interfaces instead (see here)
• Remove contracts or libraries made obsolete by v0.6, such as Address.toPayable() (Address.toPayable no longer needed since Solidity 0.6.0 #2118)
• Review naming conventions, including usage of leading underscores in internal functions in contracts but not in libraries
• Consider marking some contracts that should only be used via inheritance abstract (such as Ownable and ERC20)
• Close all issues on the v3.0 milestone
• Perform some directory and naming cleanup
• Fix documentation website build (requires fixing Add support for Solidity 0.6 syntax solidity-docgen#133)
• Review naming of libraries: singular vs plural, Strings, Counters, EnumerableSet...
• Review malleability protections in ECDSA.recover

[frangio]

We should review the situation around ECDSA.recover and the measures against malleability. There's been many issues with Ganache because of that change.

[gitpusha]

Hi @frangio and @nventuro . Some while ago I mentioned some prospective issues with regards to solc 0.6 new try/catch statement for external function calls and how it clashes with using e.g. the SafeERC20 libs internal methods. See my comment in #2028.

I noticed that SafeERC20 has been upgraded to solc 0.6, but the methods are still internal.

Do you intend to release/deploy a SafeERC20 library that is ready for use with try/catch? If not, what is your recommended way of making use of SafeERC20 with try/catch. Should I modify the method to have external visibility and then deploy the library myself and link my contract code to it?

[frangio]

@gitpusha That's interesting. SafeERC20 was designed for the opposite purpose of using it when you don't need to handle a failure. We can't make those functions external because it would affect everyone that uses them. However, it would be possible to build another library that returns booleans instead of reverting. It wouldn't be used with try/catch though but with if statements. Is there a reason why you want try/catch specifically? Can you open an issue for the feature explaining your use case so we can consider adding it? Thanks.

[frangio]

#2118 should be added to the list IMO was added to the list.

[frangio]

Is #1745 something that may require breaking changes? It seems that we can either add a new separate contract (not breaking) or modify the existing ERC721 in possibly breaking ways.

[nventuro]

I believe it can be implemented as an extension in a non-breaking way, so I don't think we need to worry about it now.

[frangio]

I want to review the naming of libraries. It is currently inconsistent, some are singular and others are plural. Strings, Counters, EnumerableSet...

Should it be plural or singular?

[gitpusha]

Is there a reason why you want try/catch specifically? Can you open an issue for the feature explaining your use case so we can consider adding it? Thanks.

Hi @frangio. Sorry for never replying to this. I lost track of this issue. The reasons for using try catch is

1. We want to prevent reverts from happening because a third-party is paying for the transaction and they should still get their gas refund if something went wrong with the ERC20.

2. In our system several actions are performed in sequence and it often helps debugging when you can catch the external call failure and add some custom error message or logic to it.

However, you are right. There are several other ways of achieving the same without changing this library for everyone. I will pursue one of them myself and might open an issue about it.

Thank you!

[frangio]

Closing in favor of the milestone.