Extension is difficult & other long term design decisions #31
Comments
the-frey
added
help wanted
|
I guess, to sum it up I'm questioning whether the approach of abstraction around contracts vs more of a pure library approach is a more maintainable pattern. There's also the hybrid approach, where you build up a library of functions that take a It's not quite DI, but this kind of client/env/context injection is v common in large functional systems to have a singular representation of state (which is kind of what the current pattern does too, as it closes over that within the contract wrapper abstraction) |
|
Hey @the-frey thanks for opening this issue. Jumping on this a while after you posted but the difficulty of extension you pointed out is becoming more apparent and forcing developers to extend cw721 in non backwards compatible ways. Talis for example updated the Now when my discord bot interacts with nft contracts I have to do some validation to see which type of response I am getting back... https://github.com/GraviDAO/lunar-assistant-bot/blob/main/src/utils/terraHelpers.ts#L4 If a change is going to be made it would be better sooner rather than later like @the-frey said. It would be nice to follow the approach of the Open-closed principle which states that "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification". In the case of |
|
Think #42 goes a long way towards closing this. |
|
I think 42 digs us out of the hole we're currently in with extensions (and is a great piece of work given where we were), but does nothing to address the core issue this was opened to discuss (for my 2cs, anyway). I think there's actually also an overlap here with the authz discussion that was had re: x/wasm that essentially reveals there is:
These things are where the tension lies, I think. |
|
The more I look at the great work @ethanfrey and the Confio folks are doing on the latest mesh-security contracts with Sylvia, the more I'm starting to think that may be our best path forward. Would be a substantial re-write but could also make extension much easier without having to use traits. Would mean some breaking changes, so not to be taken on lightly, but could be promising? Additionally, may also be worth exploring leveraging the new Cosmos SDK NFT module for much the same reasons as Token Factory (though this is a separate discussion). |
I think this is the starting point for a wider conversation. Or just tell me I'm imagining things. Either or.
FWIW I can use and extend this stuff fine now, hence why I will totally accept "nah, it's fine" as an answer.
However, I've found it a bit of an uphill struggle to get to grips with, and am still a little unclear on best practice for extending. I've seen several others taking similar or different approaches to this (on the query and the execute side). Generally, doing roughly what is in:
https://github.com/envoylabs/whoami/blob/main/src/lib.rs#L36 for execute and
cw-nfts/contracts/cw2981-royalties/src/lib.rs
Line 79 in d79bd6f
seems to be kinda how most folk do it (roughly).
I'm not experienced enough at Rust to make a perfect call on this, but my gut feeling is that the way CW721 base is implemented is:
If I didn't have the Rust compiler to help, I'd be relatively lost.
I found myself just now needing to essentially copy-and-paste a big chunk of functionality, and I was thinking "how much of the base am I using and not overriding?" The way it works right now is as a container, and that's fine - because there is base behaviour in there that I am using, but I'm not sure what it is buying me over a module containing functions that I can import as-and-when-needed, if I'm having to implement new functions in a module and dispatch to them, instead of bolting stuff to the side of the container. The default implementations of
query,executeetc I suppose?What I fear is that base behaviour will change but not many contracts will pick it up, because it's being overriden, at which point you have to ask whether this should be a lego-bricks library of helper functions with some enums (closer to how some of the other cw-plus contracts are implemented, I suppose).
I'm sticking my neck out a little bit here, so feel free to say that I'm just not good with traits and I'm missing something, but... yeah. I feel like this is probably difficult to decide via e.g. discord and we need to have a design discussion around this. Even if it's just "the-frey, you're wrong, but X is an issue, let's talk about that."
Usage of these contracts is already exploding and soon we won't have the luxury to radically change
base.Which also brings into focus discussions like #25 - how we extend, what we consider e.g.
baseandplusand how we want to make a modular code base that is ergonomic to use while still being safe.I don't think any of these are easy questions, and I certainly don't want to point fingers - everybody has worked hard to get this library to the place it is today (thanks!) but I see the window for foundational design closing, to some degree.
Maybe this also ties into the wider standards conversation that's happening. We're building the plane on the way down, and that's fine, but a few hours and some discussion now could save us some heartache in the future if we're going to be designing and building modular CW contracts for the next few years around NFTs.
Sorry about the essay and apologies if it's unclear what I'm trying to say. It's been a long day of code and possibly I need to revisit and refine this later.
The text was updated successfully, but these errors were encountered: