search

CosmWasm / sylvia

CosmWasm smart contract framework
Apache License 2.0
93 stars 15 forks source link

Demo: add cw-orch support #375

Closed ethanfrey closed 1 month ago

ethanfrey commented 5 months ago

This is a demo PR to show the possibility of implementing #374

It has some short-comings, but in the current state, it allows crates to optionally enable cw_orch derives in sylvia and use it in their crate. While leaving crates that don't opt-in with the same code-gen as before (meaning it is potentially non-API breaking).

In this case, @kayanski modified the cw20-base sylvia example to play well with the cw-orch framework and we run a multi-test on the cw-orch generated code to show it works.

I will highlight some limitations of the current PR that need to be fixed before such could be merged, but this should provide a concrete example to move any potential cw-orch and sylvia collaboration forward

ethanfrey commented 4 months ago

Providing a way to simply forward attributes to messages doesn't create artificial dependency corelations.

I support this approach 100% and was struggling to remove the import, which I couldn't (and signalled as an issue).

I have no idea how one can extend remote macros. But if there is a way, and it can cover the code points here, it would be great. And this serves as an example of what places would need to be extended.

Also if #376 is implement, the only points left to extend are just adding some derive macros on top of the Execute, Query, Sudo messages, which should be a reasonable target. And a lot more clear than "allow others to extend".

Very happy to close this PR and use another approach if it can acheive this outcome

CyberHoward commented 4 months ago

This adds a lot of potential maintenance work - because, at some point, some bug might occur, saying, "I use Sylvia x.y.z with orchestrator a.b.c, and it doesn't work.". Providing a way to simply forward attributes to messages doesn't create artificial dependency correlations.

This is a great point that we identified a while ago. To solve this we've stabilized the core traits and types used in cw-orch. See the cw-orch-core crate.

This change essentially means that an interface with cw-orch v0.20.x is compatible with a version of v0.21.x and above until the next breaking change to core (which should be rare). This means that not all crates have to be on the same version of cw-orch.

Lmk if you think we're missing something with this or if there's a better way for us to make this clear to the crate consumer!

Edit: will spend some time on this PR this weekend :)

kulikthebird commented 3 months ago

Hi Ethan,

Would you agree to close this demo PR? We already have two issues that specify the problem demonstrated in this PR:

The first of these is already addressed by https://github.com/CosmWasm/sylvia/pull/388.

jawoznia commented 1 month ago

Hi Ethan,

Would you agree to close this demo PR? We already have two issues that specify the problem demonstrated in this PR:

* [Allow sylvia crates to use cw-orchestrator as well #374](https://github.com/CosmWasm/sylvia/issues/374)

* [Add impl From<VARIANTS> for Contract{Exec,Query,Sudo}Msg #376](https://github.com/CosmWasm/sylvia/issues/376)

The first of these is already addressed by #388.

@ethanfrey, did you check if the attributes forwarding satisfy the cw-orch support?

jawoznia commented 1 month ago

Closing as this PR should be resolved by the #374 and #376.

If there will be missing support for attributes forwarding please create an issue.