Arbitrary Minting Logic Minter

[scyclow]

It would be great if you had a contract where the precise rules for whether a user can successfully mint were determined by another contract that the artist specifies.

[jakerockland]

It would be great if you had a contract where the precise rules for whether a user can successfully mint were determined by another contract that the artist specifies.

This would definitely be an easy one to implement–though would concern me a bit from a security perspective. What would be the intended use-case from an artist perspective here? Trying to think about how this would be functionally better than just swapping out the actual minter used itself.

[scyclow]

I don't think it would be functionally better, but it might decrease some of the logistical overhead. For example, you'd know that AB would always get their 10% cut and the artist wouldn't have to worry about implementing payment logic.

I think the use case would just be to give the artist more leeway with how they conduct the drops. Maybe they only want to allow transactions with low gas bids, or only accept transactions when USD<>ETH is at a certain level, or only if it's Tuesday, or only if fewer than 50 addresses have minted that day, etc. etc.

I can't think of any security vulnerabilities off the top of my head, but curious what possibilities there are there.

[jakerockland]

I don't think it would be functionally better, but it might decrease some of the logistical overhead. For example, you'd know that AB would always get their 10% cut and the artist wouldn't have to worry about implementing payment logic.

I think the use case would just be to give the artist more leeway with how they conduct the drops. Maybe they only want to allow transactions with low gas bids, or only accept transactions when USD<>ETH is at a certain level, or only if it's Tuesday, or only if fewer than 50 addresses have minted that day, etc. etc.

This makes sense to me–I am supportive of this idea, though I would think we probably will want to implement it such that there is an allow listing mechanism that needs to be executed by the CoreContract controller (AB multi-sig in the case of Art Blocks) to allow list a specific "arbitrary execution gate" contract to be "plugged in" on this more agnosticly-architected minter.

This would be to prevent the vulnerability of say, someone introducing an "arbitrary execution gate" that is just completely broken in some way, or intentionally malicious in some way (e.g. an artist works with a dev to make a plugin but that dev is malicious and creates some weird obfuscated execution gate that actually steals a users funds), etc.

Generally though, the idea of creating yet another abstraction of plugin-ism makes sense to me, that is cordoned off specifically to plugging in the actual minting gate itself.

WDYT about this idea @ryley-o ?

[ryley-o]

(edit - see below, not too concerning from a security perspective!)
From a security perspective, this would be concerning. We are essentially executing arbitrary code on an unowned contract during (actually just after due to checks-effects-interactions) a user's purchase. Allowlisting the contracts that could be run the arbitrary execution gate would be a minimum requirement in my opinion.

I'm like the concept, but from a purchaser perspective, I would want any arbitrary execution gate contract to be audited by Art Blocks, available in this repository for some time before the drop, and a clear warning and notice of which code will be ran during the purchase. At that point, I start to think that if an artist wanted to conduct their drop in some new way, perhaps the more secure route is to just submit a new minter to the minter suite 😄 . We already have a pretty minimal implementation in the MinterSetPriceV1 minter, and it could be easily forked to add any extra restrictions (mint on Tuesday, lower gas prices, etc.).

Thoughts?

[scyclow]

If you're going to audit it anyhow, then I agree that there probably isn't much value in having an arbitrary execution gate contract vs an artists writing their own minter suite contract. So if that's an option you're okay with, then it sounds good to me!

I'm curious though what sort of vulnerabilities this would present that couldn't be protected against by a reentrancy guard.

[jakerockland]

If you're going to audit it anyhow, then I agree that there probably isn't much value in having an arbitrary execution gate contract vs an artists writing their own minter suite contract.

Yeah fair enough, well said @ryley-o 🙏

So if that's an option you're okay with, then it sounds good to me!

We're definitely OK with artists writing their own minter suite-compatible minters, strongly encouraged 😄 🙌

I'm curious though what sort of vulnerabilities this would present that couldn't be protected against by a reentrancy guard.

The first place my mind goes from a malicious perspective is a set of logic that tries to get token spend approvals for the arbitrary code contract initially and then after doing so will arbitrarily take your ERC721s/ERC20s on subsequent mint calls.

A user would be able to tell this was happening of course, if they were paying attention carefully to the TXs before signing/confirming them, but allowing the potential for that type of thing to only be caught at the user-level concerns me from a design perspective with regards to anything we would explicitly add to the minter suite with the Art Blocks experience.

There are probably more creative accidental or intentional vulnerabilities one could dream up as well 😅

[scyclow]

I might not understand some of nuances of these attack vectors, but
a) I don't think you could get the user to approve token spends because the msg.sender will be the contract, not the user's address.
b) I wonder if there are any attacks that this architecture would enable that you couldn't already do by creating a malicious payable function on a contract, and setting that contract address as the payee.

[jakerockland]

Yeah good points–hrm, I would need to think on this a lot more to be quite honest; could be that my initial paranoia is not fully warranted...

[ryley-o]

b) I wonder if there are any attacks that this architecture would enable that you couldn't already do by creating a malicious payable function on a contract, and setting that contract address as the payee.

@scyclow great point! Such a great point, in fact, that I think this type of minter would essentially introduce no additional security risks than we already have today. ("security risks" being limited to a malicious artist that uses some kind of logic to selectively revert purchase transactions in some unfair manner)

I think it would make sense to allowlist the logic contracts and include them in this repository though. The reasoning there would be to prevent artists from accidentally creating a bug related to minting. You are correct that a malicious artist that wants to set their additional payee to a malicious contract could do so. "Malicious" secondary payee contract in that case really just limited to a contract that has the ability to revert the tx and prevent the mint (no funds could be stolen, etc.). Setting the additional payee to a malicious contract would be quite obvious though, and the artist would likely be identified as malicious quickly.

[ryley-o]

Also, just to note, forking the MinterSetPriceV1 minter would be slightly more gas-performant. If we delegate logic to another contract, that would introduce the gas costs associated with https://eips.ethereum.org/EIPS/eip-2929. That being said, the convenience of deploying a simple logic contract might outweigh the gas benefit of forking.

[jakerockland]

Yeah IMHO forking MinterSetPriceV1 is always the more user-centric decision from the perspective of collectors (who are the ones that I think minters should be optimized for, given that they are the ones doing the minting most often 😄), so I'm not sure I see huge benefit to the arbitrary-gating-logic minter–that said, @scyclow if you feel strongly that this would be much more beneficial than just forking MinterSetPriceV1, if you sent us a PR implementing this type of minter we would be happy to review and consider incorporating it into the suite–we also can loop back on implementing this ourselves though I would say that I think we will weigh this lower priority for now than the other more-concrete minters that we're working on currently.