CosmWasm / sylvia

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

Make `: custom(msg, query)` in `sv::messages` attribute redundant #352

Open jawoznia opened 3 months ago

jawoznia commented 3 months ago

158 Introduced mechanism to convert Response<Empty> to Response<SomeMsg>. This allowed us to make implementation of the Interfaces that do not support CustomMsgs either because the author didn't provide the ExecC associated type for users to implement their own CustomMsg or because it was deliberately chosen not to allow chain specific logic via #[sv::custom(msg=Empty, query=Empty)] attribute.

We exposed this conversion via addition of : custom(msg, query) at the end of #[sv::messages(...)] attribute. Provided this parameter contract macro was able to determine that the Response returned from the Interfaces method has to be converted using the IntoResponse interface and analogical thing should happen to the Deps.

Because of above we don't want to remove the conversion mechanism and rather this issue aims to find out a way to make the mechanism automatic and obsolete the : custom parameter.

Obsolete the : custom() parameter

To start let's list all the possible scenarios supported by Sylvia:

  1. Empty Interface implemented on Empty Contract.
  2. Empty Interface implemented on CustomMsg Contract.
  3. CustomMsg Interface implemented on CustomMsg Contract.

IntoResponse implementation aims to make "2" possible.

To make the conversion mechanism automatic we could either make Sylvia deduce that IntoResponse has to be called or make it work in all three scenarios. Automatic detection is impossible as in minimal scenario all contract macro knows about the Interface implemented is the path to the Interface (hence the : custom() parameter).

#[sv::messages(path::to::interface)]

Implement IntoResponse for all scenarios

Current implementation is as follows:

impl<T> IntoResponse<T> for Response<Empty> {
    fn into_response(self) -> StdResult<Response<T>> {
        ...
    }
}

Important thing is that it's implemented only on the Response<Empty>. This is because we cannot implement conversion from Response<CustomMsg> to some other Response<> as it might loose the information in case the CosmosMsg::Custom is sent. This is not an issue in case of Response<Empty> as CosmosMsg::Custom(Empty) doesn't carry any information and in fact shouldn't be constructed.

Specialization

One approach to remove the need for : custom parameter would be to use the IntoResponse always. In such case we would have to provide missing implementation for the "3" scenario. It's not possible unfortunately because of overlapping implementations.

/// Allow conversion to every `Response<T>` for `Response<Empty>`
impl<T> IntoResponse<T> for Response<Empty> {
    fn into_response(self) -> StdResult<Response<T>> {
        ...
    }
}

/// For every [T] that implements the `CustomMsg` implement identity conversion to itself
impl<T> IntoResponse<T> for Response<T>
where
    T: crate::types::CustomMsg,
{
    fn into_response(self) -> StdResult<Response<T>> {
        Ok(self)
    }
}

Both of these implementations cover the IntoResponse<Empty> for Response<Empty>. This might be possible in the future with specialization feature implemented in Rust https://github.com/rust-lang/rust/issues/31844, but it's unfortunately nowhere near being available.

Negative impls

This would be a workaround in providing specialization as we could make some marker trait that would be implemented on each type other than Empty, and then providing an old implementation for Response<Empty> and identity conversion in case of every Response<T> where T: Marker. This is unfortunately also not yet available, but we can track it and consider in the future https://github.com/rust-lang/rust/issues/68318.

Remove blanket implementation

Thanks to the sv::custom attribute we have access to potential CustomMsg used by the Contract. We can then generate a local IntoResponse trait (has to be generated due to the orphan rule) and implement it on all the scenarios, and in case of lack of sv::custom attribute either implement it only for the scenario "1" or just remove the IntoResponse usage. No blanket implementation, no collisions.

This approach might be great in case of generics as while implementing the Empty Interface on Contract with generic type set in place of CustomMsg there should always be : custom parameter added in case future user would apply their CustomMsg type in place of this generic type causing issue with lack of conversion not detected during the implementation of our Contract. This lack of concrete type however would require IntoResponse implementation to also be implemented on some generic T which would turn into a blanket implementation leading us back to the original issue.

Summary

At this moment I unfortunately don't see the solution to removal : custom attribute.

kulikthebird commented 2 months ago

There is a workaround to implement IntoResponse in such a way it meets all the scenarios:

use std::marker::PhantomData;
use std::any::{Any, TypeId};

struct Empty;
struct CustomMsg;
struct Response<T>(pub PhantomData<T>);
trait IntoResponse<T> {
    fn into(self) -> Result<Response<T>, String>;
}
impl<X: Any, Y: Any> IntoResponse<X> for Response<Y> {
    fn into(self) -> Result<Response<X>, String> {
        if TypeId::of::<X>() == TypeId::of::<Y>() {
            Ok(Response(PhantomData)) // identity relation
        } else if TypeId::of::<X>() == TypeId::of::<Empty>() {
            Ok(Response(PhantomData)) // Empty ==> Custom
        } else {
            Err("not_implemented!".to_string())
        }
    }
}

fn main() {}

It requires to use std::any::{Any, TypeId} types.