search

Developer-DAO / pixel-avatars

A Polygon-based minting projects for Developer DAO members who own an Devs for Revolution NFT
https://pixel-devs.developerdao.com/
ISC License
133 stars 36 forks source link

Future proofing the contract before launch #79

Closed rasmuscnielsen closed 2 years ago

rasmuscnielsen commented 2 years ago

On the subject of preparing for future games etc. I have been pondering if we could do anything to future-proof our smart contract further. While it can be difficult to foresee every possible future idea I have thought of an example use-case of offering special mints where people choose their own traits (outside of the 8000 reserved combinations) - potentially branding it as another kind of token that doesn't hold the D_D necklace but some other visual identifier.

Basically I think it would offer a lot of flexibility if we could adjust these parameters:

Authorization / eligibility Different mints requires different pre-requisites. This is already achieved today by server signature as we can change the server code as we please.

Mint price Different kinds of mints / variations could trigger different prices. This would be easily achievable by tweaking the current contract/server integration. Basically I propose we continue to enforce a minimum mint-price in the contract which should be the cost of minting the current DAO member NFT's. However additionally we could allow the server to also enforce a "tipping fee" or similar which would be useful in case we wanted to offer other variations that incurs extra fees.

Trait combinations It would be very powerful if we could also add more variations / meta data beyond the existing traits and their 8000 existing combinations. I believe this should already be possible after launching the contract but honestly I'm still not completely familiar with the full palette of the ERC 721 contract such as meta data and uploading assets to IPFS. If we were ever to allow people to make their own combinations I suppose it would have to be the server that generated & uploaded the image to IPFS + issues that token with proper meta data to the contract. I'm not sure whether this is a bad idea or even possible, but thought I'd share it and get a conversation going. As long as the current contract would allow such an extension we don't have to work on this right now.

The alternative to all of the above of course is to just create new contracts every time we hit a roadblock with the current one. While that's possible too it might make it more cumbersome to create an ecosystem of games etc. around the avatars if they are all spread across different contracts.

With the flexibility the server gives us I thought it might give us an advantage in terms of adding new functionality, and perhaps allow us to have everything hosted on the same contract by using the server to incorporate business logic for the minting part.

Interested to hear your thoughts!

goldzulu commented 2 years ago

Hi @rasmuscnielsen great ideas on future proofing work. Probably warrant some discussion in the weekly call.

While thinking of additional enhancements we need to bear in mind that adding server in the background as a dependency will typically means that there is a coupling between the smart contract and the server. Should the server go down then we need ensure that the smart contract can continue to function. One of the other fundamental principle of the block chain is the fact that nobody can change data as and when we want and if it did, it's in a transparent manner, if we make these data generation dynamic and on the fly, one can argue that we have broken this core fundamental behaviour. Oracles etc are built for that matter to make sure any external interaction to the smart contract be done in a fair and predictable manner.

Because of the above, smart contracts functions are sometimes exposed and can be accessed directly. We would need to unsure if this occurs then we are not giving some users and unfair advantage, e.g. minting a token that. we have prevented in the web3 controls but not in the smart contract.

Now having said all this, the direction that we can take is via using the Diamonds.. a way to have allow for flexible upgrading. Here https://dev.to/mudgen/understanding-diamonds-on-ethereum-1fb Takes a bit of a learning curve but would cater for future upgradability. Thoughts?

rasmuscnielsen commented 2 years ago

@goldzulu Thanks for weighing in with your thoughts. Highly appreciate it.

While thinking of additional enhancements we need to bear in mind that adding server in the background as a dependency will typically means that there is a coupling between the smart contract and the server. Should the server go down then we need ensure that the smart contract can continue to function.

It's a given that with the "Authorization server" strategy new NFT's can only be minted through permission of the server. I don't this is something that can change without completely changing lanes and abandon the server approach. It's worth mentioning that "the server" is basically a stateless cloud function and can at any time be moved or change identity by the contract-owner (the DAO). Similarly I also want to stress that any dependency would only apply to minting new NFT's - not the ones already minted.

I have actually been busy writing a blog post about launching the Derivate project that touches on this and many other considerations. If you'd like I can send you a preview? Would love to get your thoughts on the article before publishing.

One of the other fundamental principle of the block chain is the fact that nobody can change data as and when we want and if it did, it's in a transparent manner, if we make these data generation dynamic and on the fly, one can argue that we have broken this core fundamental behaviour. Oracles etc are built for that matter to make sure any external interaction to the smart contract be done in a fair and predictable manner.

Is this related to the possibility of introducing eg new variants for a different pricing scheme? I guess I don't personally see it as changing data or not being transparent. Just a business introducing new products. Let's pick it up on Wednesday :-)

Because of the above, smart contracts functions are sometimes exposed and can be accessed directly. We would need to unsure if this occurs then we are not giving some users and unfair advantage, e.g. minting a token that. we have prevented in the web3 controls but not in the smart contract.

I think we might be talking about two different things here :-) I 100% agree that all business rules must be enforced in a way that they cannot be bypassed by invoking functions on the smart contract directly.

Now having said all this, the direction that we can take is via using the Diamonds.. a way to have allow for flexible upgrading. Here https://dev.to/mudgen/understanding-diamonds-on-ethereum-1fb Takes a bit of a learning curve but would cater for future upgradability. Thoughts?

Thanks for sharing the article. Diamonds seem like an interesting concept that I haven't come across before. The article gives a good high level explanation but not that many concrete examples. I wouldn't know where to start with the PixelAvatar contract but would be very interested to see someone implement the concept in a meaningful way for our use case.

I think it might be helpful to write up 2-5 example use-cases for how we might want to evolve the Derivates in the next 6 months and then talk about how that could be achieved exploring different paths :-)

Lots of good stuff to talk about Wednesday.

briangershon commented 2 years ago

First, thanks @rasmuscnielsen and @goldzulu for great discussion and topic!

Potential concern with server approach

One concern with the server-flexible approach (that I was hearing about recently) is around fraud. I think it relates to the feedback @goldzulu is giving. If a server is an unknown box, a user won't know if they can trust due to lack of transparency, so who knows what the server is really doing -- hence opening doors to fraud that might not exist if using contract directly. Our code-base is open-source (so is more transparent) but I wonder if this implies some bias against using this practice.

And if that's true, we may want to put more emphasis on the contract side upgradeability.

Contract Upgrade options

I was curious if "Diamonds" was the same method that OpenZeppelin used for their Upgradeable contracts.

Turns out contract upgrades are a big topic with lots of options. This article talks about a lot of the tradeoffs: https://blog.openzeppelin.com/the-state-of-smart-contract-upgrades/

In there, they mention Diamonds as well as Transparent Proxies (which OpenZeppelin uses).

Thoughts In summary

briangershon commented 2 years ago

Though one big plus of the server-flexible approach is it might be a lot easier to reason with. The intro paragraph from "state of smart contracts article" says "there has also been a great deal of controversy around smart contract upgrades, due to the technical complexities they introduce and the fact that they can be a threat for true decentralization."

So an overly complex solution (as well as a solution that uses a lot of redirection/call delegation in contracts) is risky too. A server-flexible approach would likely be more straight-forward.

rasmuscnielsen commented 2 years ago

@briangershon Great notes and very informative article you sent. I didn't make it all the way through yet, but read the part of of proxies / delegate calls. Also thanks for getting involved in this discussion. I think it's super important that we discuss this one as a team and figure out a good solution :-)

A few initial thoughts to your points.

One concern with the server-flexible approach (that I was hearing about recently) is around fraud. I think it relates to the feedback @goldzulu is giving. If a server is an unknown box, a user won't know if they can trust due to lack of transparency, so who knows what the server is really doing -- hence opening doors to fraud that might not exist if using contract directly.

The counter argument here is that the user actually is interacting directly with the smart contract. For all practical purposes the only message being passed from the server is a boolean true (ie: "Yes, this user can mint the avatar (on these conditions)").

The only logic we can change is that related to the requirements of the minting. All communication is being passed through the user's own call to the smart contract. The server has no way of communicating directly to the smart contract (at least currently).

I ponder whether being able to swap logic in the smart contract (ie through proxies or diamonds) is more transparent (?). At least currently the implementation of a call "mint call" can't suddenly change meaning? Being able to write new code to the contract in some way seems more controversial to me, but of course it depends on the implementation.

We can then hopefully answer where we should add flexibility (server-side, or contract side). My web2 experience likes the server-side, but I'm not 100% sure from pure web3 perspective. Albeit we may want to mix-and-match options on both server and contract. (We're already doing a bit of mix-and-match now -- safety in the contract, with business logic on server)

Agree on everything here.

Though one big plus of the server-flexible approach is it might be a lot easier to reason with

Yeah. My suggested tweak of adding mint-price could be introduced in a couple hours work, and would get us a long way.

I'm not against contract upgradability as such but I have some concerns around what it will do to the project scope. Who has time and experience to take on that project, and how do we decide on which parts should be upgradable vs which shouldn't? I think that's something that needs to be addressed early on if we're to go down that road.

briangershon commented 2 years ago

I'm done with the heavy-lifting modifications to our contract to support OpenZeppelin Upgradeability.

PR: https://github.com/Developer-DAO/pixel-avatars/pull/81

I'm still working on improving docs and doing a bit more testing -- but everything is working end-to-end.

This PR ready for others to:

rasmuscnielsen commented 2 years ago

@briangershon With your PR on upgradeability honestly I don't think my previous suggestion of adding minting price to server is necessary right now. With the option to upgrade the contract we could either write the logic directly in the contract (as suggested earlier by @goldzulu ) or just make the tweak to the server-signature when needed.

As such I think we can close this issue now 💪 Thanks for working on the contract 🙏