Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jawoznia opened this issue Apr 15, 2024 · 1 comment
Open

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

jawoznia opened this issue Apr 15, 2024 · 1 comment
Labels

Comments

@jawoznia
Copy link
Collaborator

jawoznia commented Apr 15, 2024

#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 rust-lang/rust#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 rust-lang/rust#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.

@jawoznia jawoznia added the idea label Apr 15, 2024
@jawoznia jawoznia changed the title Make , custom(msg, query) in sv::messages attribute redundant Make : custom(msg, query) in sv::messages attribute redundant Apr 16, 2024
@kulikthebird
Copy link
Contributor

kulikthebird commented Apr 22, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants