After discussion with Hadrien, we decide to implement it the following way:

• Reuse MinterAccessControl smart contract. It already implement the enable/disable and a list of authorized minters.
• Add the operator signature at the end of the MintXXXData.signatures
• If there is one more signatures than the number of creators
  • then check this signature is signed by a minter from MinterAccessControl
  • bypass check previously done by MinterAccessControl.isValidMinter
  • Remove the signature from the signatures payload so the contract checks can continue to work
• Else, execute the existing logic from MinterAccessControl.isValidMinter

Not to do in the same PR but nice to have:
The function to validate the signature is not optimised for the use we are going to do. Would be nice to check how much less gas we would get with like 3 creators and 5 minters

• If there is one more signatures than the number of creators
  • then check this signature is signed by a minter from MinterAccessControl

Unfortunately, it's not possible to iterate mapping (address => bool) private _minters;
Should we go back to a separate class implementation than MinterAccessControl with only 1 address operator to check the signature against ?

Should we go back to a separate class implementation than MinterAccessControl with only 1 address operator to check the signature against ?

What about using an iterable map? Open zeppelin has one.
The new pr could change the structure in MinterAccessControl

Another idea: use directly hash.recover to return signer address:

signerFromSig = hash.recover(signature);

source: https://github.com/rarible/protocol-contracts/blob/master/tokens/contracts/erc-1271/ERC1271Validator.sol

Less code reusability, but much simpler and gas-efficient

signerFromSig = hash.recover(signature);

won't work for a contract, see https://github.com/rarible/protocol-contracts/blob/b1b1593686fc78ceecab123124059acd908fafe1/tokens/contracts/erc-1271/ERC1271Validator.sol#L24 requires the signer to be passed as parameter.

EnumerableMap can be a good idea but is a breaking change that will be hard to handle if the first PR is deployed before this one. It will be almost impossible to migrate the configuration from the mapping since we can't enumerate the values.
Can we block the first PR to switch to an EnumerableMap on MinterAccessControl before the first release ?

EnumerableMap can be a good idea but is a breaking change that will be hard to handle if the first PR is deployed before this one. It will be almost impossible to migrate the configuration from the mapping since we can't enumerate the values. Can we block the first PR to switch to an EnumerableMap on MinterAccessControl before the first release ?

Good point.
I think the first PR will not be merged or review before you finish this new PR.
Implement it in a new PR with a nice dedicated commit that changes the structure and I will push it back to the PR later

Waiting for #2 to be done first