msg(exec) macro conditional on feature / config

[maurolacy]

It would be nice if the msg(exec) macro could be enabled based on a feature or config option.

That is, something like #[cfg(test, msg(exec))], or a similar syntax.

This would be very useful to add handlers that are only available / used for testing.

See
https://github.com/osmosis-labs/mesh-security/blob/70872078c80b8019f2da6220b47222dab753baad/contracts/provider/external-staking/src/contract.rs#L230C6-L244

for an example where this would be useful.

[hashedone]

First of all - I am pretty sure something is wrong with the testing approach here. My hot take: having test-only API is a code smell. In the test, you want to verify states which are possible in the real world, so you should be able to achieve the tested state with a non-test-only API.

What I assume you are doing here is making some shortcuts to achieve some state you assume should be possible with some flow, but you don't include the fact that achieving such a state might lead to additional changes happening around which our test does not consider. And the worst part of that is - maybe now those additional things do not happen, but it might change in the future, and you will not consider those changes because your test still uses the shortcut.

So this is a reason why I am strongly against providing a special solution for that.

Now the proper answer - the solution already exists in Rust itself - it is a cfg_attr. In your case: #[cfg_attr(test, msg(exec))] - however I still strongly advice to not structure test this way.

[maurolacy]

First of all - I am pretty sure something is wrong with the testing approach here. My hot take: having test-only API is a code smell. In the test, you want to verify states which are possible in the real world, so you should be able to achieve the tested state with a non-test-only API.

I know. We're currently forced to do this, because of limitations of the testing framework (no IBC support in mt).

...
Now the proper answer - the solution already exists in Rust itself - it is a cfg_attr. In your case: #[cfg_attr(test, msg(exec))] - however I still strongly advise to not structure test this way.

When I do this it compiles, but then fails when compiling tests:

$ cargo test
...
error: cannot find attribute `msg` in this scope
   --> contracts/provider/external-staking/src/contract.rs:233:22
    |
233 |     #[cfg_attr(test, msg(exec))]

Is there a way around it, perhaps qualifying or importing the required macro?

[maurolacy]

Re-opening, as after macro expansion we can confirm the cfg_attr(test, msg(exec)) is not being processed / understood by the contract macro.

[hashedone]

As I am not happy about this testing approach, I still see value in gating sylvia attributes with features using cfg_attr - we should at least refine where there is a problem and document it.

[kulikthebird]

I played a little bit with cfg_attr, and it seems that it doesn't work for the attributes within the impl block for contracts and interfaces. On the other hand, there is no problem when the conditional Sylvia attributes are used for the whole impl blocks. It seems that the attributes inside the mentioned blocks are not processed by the Rust compiler for some reason before the macro procedure is called.

Since cfg_attr are not handled by the compiler beforehand, to keep our attributes convention we can adding a new Sylvia attribute called #[sv::cfg({...})]. The {...} part could be forwarded to every enum's variant, structure and method as a #[cfg({...})] attribute. Once it's implemented, users could write:

impl Contract {
    #[sv::msg(exec)]
    #[sv::cfg(test)]
    fn exec_method(...) {}
}

In such a case, every variant/struct/method related to exec_method will be visible only when the test flag is set.

I'm happy to get your opinion on this solution.