馃 Motivation
ERC20Mintable is available for ERC20 tokens. An ERC777Mintable contract would help the adoption of ERC777.
馃摑 Details
We will need to implement ERC777Mintable.sol in a similar fashion to ERC20Mintable.sol by passing the constructor values to the _mint function properly.
Here is an example implementation that could be used:
import "./ERC777.sol";
import "../../access/roles/MinterRole.sol";
/**
* @dev Extension of {ERC777} that adds a set of accounts with the {MinterRole},
* which have permission to mint (create) new tokens as they see fit.
*
* At construction, the deployer of the contract is the only minter.
*/
contract ERC777Mintable is ERC777, MinterRole {
/**
* @dev See {ERC777-_mint}.
*
* Requirements:
*
* - the caller must have the {MinterRole}.
*/
function mint(
address operator,
address account,
uint256 amount,
bytes memory userData,
bytes memory operatorData
) public onlyMinter returns (bool) {
_mint(operator, account, amount, userData, operatorData);
return true;
}
}
This implementation works as expected, even with the standard Crowsale and MintedCrowdsale contracts, so long as the _deliverTokens function is overridden to call _mint properly, like so:
/**
* @dev Overrides delivery by minting tokens upon purchase.
* @param beneficiary Token purchaser
* @param tokenAmount Number of tokens to be minted
*/
function _deliverTokens(address beneficiary, uint256 tokenAmount) internal {
require(
ERC777Mintable(address(token())).mint(beneficiary, beneficiary, tokenAmount, "", ""),
"MintedCrowdsale: minting failed"
);
}
It may be worth a separate PR to make MintedCrowdsale work with either token standard, or to migrate functionality for crowdsales completely over to ERC777 (not sure of the OpenZeppelin way of handling such upgrades).
Are you creating an ERC777 token @awantoch?
I wonder if there might be a need for a mint and an operatorMint function.
@abcoathup No intention of an actual token sale (all within test environments), but I'd like to help with the momentum of ERC777 and when trying to implement the crowdsale with ERC777 I realized there was not yet official support for MintedCrowdsale.
It makes sense to separate the mint and operatorMint functions logically, but I think case of ERC777Mintable, if I'm understanding correctly, we won't need to since the operator and account parameters are simply passed through from mint to _mint. This is a similar structure to the way the unit tests are currently structured as well, using mintInternal to test ERC777's underlying _mint.
For the MintedCrowdsale, since the intention is for the beneficiary to claim whole ownership of the tokens, it would make sense to use mint(beneficiary, beneficiary, tokenAmount, "", ""), setting the beneficiary to be the operator as well as the account owner within the _deliverTokens function, correct?
Another thought -- would it be better to migrate the MintedCrowdsale and Crowdsale contracts completely over to ERC777 as well (leveraging the overridden _deliverTokens function above)? MintedCrowdsale is the only type of emission implemented in the library, making the change drop-in with Crowdsale. We should only need to change the interfaces from IERC20 to IERC777 (even if not, the interface is backwards compatible anyway so the crowdsale still works even if MintedCrowdsale is the only file changed). New crowdsales should probably be using the newer replacement standard as well.
I'm willing to implement all of the changes necessary, just need community confirmation on the design.
Hopefully I'm following correctly. I appreciate the collaboration 馃檪
@awantoch great to hear that you would like to help with the momentum of ERC777.
I would 鉂わ笍 to see more adoption.
I suggest we get input to the discussion from @frangio and @nventuro on the best way to approach.
Thanks for the suggestion @awantoch! I support adding an ERC777Mintable contract. Please go ahead and submit a pull request for that!
Regarding crowdsales, we haven't seen much demand for crowdsale contracts for quite a while, so I don't think it's a worthwhile investment at the moment!
Sounds good @frangio will do!
Hello @awantoch, thank you for your interest in contributing!
Over the last couple days we've been working on our roadmap for the next couple of weeks (you can read more about it), and one of the goals we set out is to reduce the complexity of the library. This will include removing contracts such as ERC20Mintable, ERC20Pausable, and ERC777Mintable if it were to be included.
The reason why we're doing this is because we want to decouple our Access Control solution (Roles and MinterRole in this case) from these contracts: we are working on a better Access Control system that will make it easier for users of the library to create their own systems. We'd love to get your input on this discussion!
That said, we will most likely still provide some form of a mintable ERC777 token as an example of library usage, which will also be usable for quick prototyping by being deployable as-is with no need to write Solidity code.
Given this, I'm closing this issue to prevent people from spending time on it, which we'll tackle over the following weeks. If you want to look for ideas on how you can contribute to the OpenZeppelin Contracts, head to the roadmap, where there's a section for features we're counting on the community to help with.
Thanks a lot!
Most helpful comment
Thanks for the suggestion @awantoch! I support adding an
ERC777Mintablecontract. Please go ahead and submit a pull request for that!Regarding crowdsales, we haven't seen much demand for crowdsale contracts for quite a while, so I don't think it's a worthwhile investment at the moment!