Allow to restrict minters #4
Conversation
| __MintControl_init_unchained(); | ||
| } | ||
|
|
||
| function __MintControl_init_unchained() internal initializer { |
There was a problem hiding this comment.
I'm not sure how these init functions work, but I guess we should ensure that if there is an update, we can still keep the same state _minters and minterControlEnabled. I remember some "hacks" with a gap variable to make sure we can do updates.
Can we have some tests for that even just manual tests
There was a problem hiding this comment.
- Is it possible to initialize the smart contract with the
minterControlEnabledenabled and the owner/deployer as minter? Will it create a breaking changes (is it possible to have optional function parameter in solidity?) - Could be nice to have a function to enable the minter control and set the owner as minter so in one transaction, the owner could turn on minter control with a basic config.
By default now, there is not minting restriction when creating the contract so it have side effects i think.
It's not really a standard way to do, since we manage a list of minters. |
I'm good with 2 transactions then. |
| function mintAndTransfer(LibERC1155LazyMint.Mint1155Data memory data, address to, uint256 _amount) public override virtual { | ||
| function mintAndTransfer(LibERC1155LazyMint.Mint1155Data memory data, address to, uint256 _amount) public override virtual validateMinter { | ||
| address minter = address(data.tokenId >> 96); |
There was a problem hiding this comment.
The minter is this function is not _msgSender() but address(data.tokenId >> 96).
The modifier will not work. Maybe replace the modifier by its require to give more flexibility?
There was a problem hiding this comment.
I think the same issue apply to ERC721Lazy
There was a problem hiding this comment.
This is related to the lazy minting process where Alice signs the NFT payload, but Bob sent it later on. In that case, Bob is the sender and mint for Alice.
In that case, who should be authorized? Bob because he's the one pushing the creation or Alice because she's the one that initially creates it? Both?
It feels more natural that Alice is the one that should be authorized and in that case yes this needs to be changed. If Bob should be the one authorized then it's valid
|
Closing this PR to create it on rarible repo rarible#105 |
See #2