CosmWasm / sylvia

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

Consider implementing `impl <E, Q> cw_multi_test::Contract<E, Q>` when no `custom` macro is provided #401

Open hashedone opened 1 month ago

hashedone commented 1 month ago

There is a misconception about what it means that the contract is not using the custom messages. The intuition inherited from CosmWasm is that it would be a chain-agnostic contract. While it would be a working assumption, it overcomplicates testing such a contract with MT when the App uses custom messages. This is caused by embedding custom messages in the typesystem. Non-Sylvia contracts are tactically worked around by casting around the ContractWrapper from one using Empty types to any custom type. Still, with the generated helpers, the custom message information is maintained everywhere, and it would be a very hacky solution.

Instead, we decided to keep the strict typing and contracts to be chain-agnostic are preferred to use the generics for custom messages - this way they can easily be used on any chain (also in strongly typed MT environment).

However, this approach requires naming generic Q and E in every Ctx and Response. That is not so much a big deal, but it might be considered redundant for simple cases. The technically possible solution might be to implement impl<E, Q> Contract<E, Q>... for any Sylvia contract that do not have sv::custom attribute defined - this guarantees defaulting customs to Empty, so it is safe to use it on non-empty Apps. This might be an easier UX, but it also seems to be somewhat breaking Rust idioms. Also, it is a question of how much it complicates the wrapper generation.

Ultimately, there is this unstable Rust features https://github.com/rust-lang/rust/issues/8995 that would allow the generation of the type Response = ... inside the Contract type in a way that already embeds the proper customs. The strong upside of this solution is that it would be universal across all the contracts - we could then advise using Self::Response and Self::InstantiateCtx everywhere instead of providing the customs everywhere. This would be already possible in traits.

The downside is that it would be breaking - it would cause problems if the user already defined such an embedded type. We could solve it by either:

Also unfortunately it requires us to wait for an unstable Rust feature.