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

Add semantical replies support #29

Closed
hashedone opened this issue Aug 26, 2022 · 5 comments
Closed

Add semantical replies support #29

hashedone opened this issue Aug 26, 2022 · 5 comments
Assignees
Labels

Comments

@hashedone
Copy link
Collaborator

In the vast majority of cases, the reply is handled by assigning the reply id to a specific handling path and matching that in a top-level reply. We can make it easier by incorporating this into Sylvia, leaving a space to pass some custom data with the id - just reducing it to 32bits instead of 64.

Idea is to treat the lower 32 bits of reply id as some data to pass, and the higher 32 bits as reply handler id. We could then generate a different handler id for all reply functions in Sylvia, and then access them in implementation. Additionally, we can try to improve the arguments there. Here is an example:

struct FooContract

#[contract]
impl FooContract {
  #[msg(exec)]
  fn call_me(&self) -> Result<Response, ContractError> {
    let resp = Response::new().add_submessage(SubMsg::always(/* some_msg */, self.reply_foo_id(0)));
    Ok(resp)
  }

  #[msg(reply)]
  fn reply_foo(
    &self,
    ctx: ReplyCtx,
    reply: Result<(Vec<Event>, ExpectedData), String>,
  ) -> Result<Response, ContractError>;
  
  #[msg(reply, on_success)]
  fn reply_bar(
    &self,
    ctx: ReplyCtx,
    events: Vec<Event>,
    data: ExpectedData
  ) -> Result<Response, ContractError>;
}

To achieve this, we can take advantage of the fact, that we can actually modify the trait attributed with #[contract] so we can add an additional functionality. To pass some additional data, it takes u32 argument and ors it with the code id.

Then we can go even better - it is an obvious move to just into_result the underlying reply structure. But we can also automatically parse the underlying data - I would introduce FromBinary trait in sylvia, defined as follows:

  • identity for Binary
  • implemented for all the Deserialize types forwarding to from_binary
  • implemented for some utility types - for example, protobuf response for instantiation

This way it would be way more natural to work with data (still being able to use raw Binary type here).

The last thing is the on_success thin on msg - it could be also on_failure. It suggests sylvia that this handler should forward the failure/success implementation to default. Basically, on_success replies would forward errors if there is any direction, and on_failure would do nothing on success. Obviously, on_failure will get just an error string instead of events and data.

@hashedone hashedone added the idea label Aug 26, 2022
@hashedone hashedone added this to the 0.4 milestone Jan 10, 2023
@hashedone hashedone assigned hashedone and unassigned hashedone Apr 28, 2023
@hashedone hashedone modified the milestones: 0.4, 0.5 Apr 28, 2023
@jawoznia jawoznia removed this from the 0.5 milestone Mar 19, 2024
@jawoznia jawoznia self-assigned this Aug 13, 2024
@jawoznia
Copy link
Collaborator

jawoznia commented Aug 13, 2024

I would see this feature done in two phases

  • Dispatching with id generation
  • SubMsg creation

Dispatching

Let's start with the reply entry point. I would make it dispatch the message without any parsing. This should be done in some intermediate type to be reused if needed f.e. in overriden entry point.

pub fn reply(
    deps: sylvia::cw_std::DepsMut<sylvia::cw_std::Empty>,
    env: sylvia::cw_std::Env,
    msg: sylvia::cw_std::Reply,
) -> Result<sylvia::cw_std::Response<sylvia::cw_std::Empty>, ContractError> {
    Contract::new()
        .reply((deps, env).into(), msg)
        .map_err(Into::into)
}

The generated dispatcher has to know about the REPLY_IDs respective to the method.
We could either provide it in the #[sv::msg(reply, SOME_REPLY_ID)] or automate it by generating the IDs per method in the sv namespace.

#[sv::msg(reply)]
fn on_instantiated(...) {}

#[sv::msg(reply)]
fn on_exec_failed(...) {}

pub mod sv {
    pub const ON_INSTANTIATE_REPLY_ID: u64 = 1;
    pub const ON_EXEC_FAILED_REPLY_ID: u64 = 2;
}

Those ids would have to be used by the contract creator. We could automate that process with some sort of Executor, but more on that later.

With the generated IDs the dispatcher could look something like this.

pub fn dispatch(ctx: ReplyCtx, reply: Reply) -> Result<Response, ContractError> {
    let contract = CounterContract::new();
    match reply.id {
        ON_INSTANTIATE_REPLY_ID => contract.on_instantiated(ctx, reply),
        ON_EXEC_FAILED_REPLY_ID => contract.on_exec_failed(ctx, reply),
        _ => Err(StdError::generic_err(format!(
            "Invalid reply id\n Expected one of:{}, {}",
            ON_INSTANTIATE_REPLY_ID, ON_EXEC_FAILED_REPLY_ID,
        ))),
    }
}

Marking methods with on_success and so on is also a good idea to ease the default behavior.
We could also deduce it from the parameters defined on the method.

  #[msg(reply)]
  fn reply_success(
    &self,
    ctx: ReplyCtx,
    events: Vec<Event>,
    data: ExpectedData
  ) -> Result<Response, ContractError>;

  #[msg(reply)]
  fn reply_success(
    &self,
    ctx: ReplyCtx,
    error: String,
  ) -> Result<Response, ContractError>;

  #[msg(reply)]
  fn reply_foo(
    &self,
    ctx: ReplyCtx,
    reply: Result<(Vec<Event>, ExpectedData), String>,
  ) -> Result<Response, ContractError>;  

In case of two attributes we treat it as a success, single String param is an error and otherwise we treat it as an ReplyOn::Always case.

I would very like to hear more about the FromBinary trait and how it would differ from the standard one.

SubMsg construction

In the WasmMsg we have two variants that could be constructed via the Remote and ExecuteBuilder:

as those are sent to the external Addr.

The Instantiate and Instantiate2 variants doesn't rely on existing address so separate SubMsgBuilder would have to be introduced. We could reuse it in the ExecuteBuilder.

We can consider providing shortcuts for sending messages like UpdateAdmin through Remote under separate ticket.

Example usage:

let submsg = self.remote // Provides the external contract address
                            .executor()
                            .call_me()  // Constructs WasmMsg
                            .reply_foo() // Creates `SubMsgBuilder` with the appropriate REPLY_ID
                            .build();

@jawoznia jawoznia mentioned this issue Aug 22, 2024
12 tasks
@hashedone
Copy link
Collaborator Author

hashedone commented Aug 23, 2024

We talked it through, I will leave some notes about the outcomes.

Detecting reply variant

Refering to:

Marking methods with on_success and so on is also a good idea to ease the default behavior.
We could also deduce it from the parameters defined on the method.

This might look like API improvement, but we would then struggle with all the edge cases like using type aliases, or some additional variants in the future, this would be not flexible enough.

By default the reply handler would be assumed to be the "reply_always" wariant. Such reply handler should have the signature as follows:

#[msg(reply)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    data: Result<DataT, String>,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;
  • ReplyCtx should be any type implementing From<(DepsMut, Env, u64, Vec<Event>, Vec<MsgResponse>)> as usual. The last u64 is the Reply::gas_used and it should be included, then there are SubMsgResponses::events and SubMsgResponse::msg_responses field - they would be defaulted to empty vectors when handling the failure reply.
  • data is either the deserialized content of the SubMsgResponse::data field, or the error returned. See next section for details about how we want to handle it. The Err variant here should be anything implementing FromStr trait with From::Str error implementing Display (on deserialization error we will fail processing)
  • Any further arguments are used to build the payload type for this call. They would generate the separate structure that would be JSON serialized into the payload field of submessage. Here it would create struct ReplyFooReplyMsg { arg1: String, arg2: u64 }. Attributes forwarding should be working for those as well.

It would be possible to create handler that only handles the success replies:

#[msg(reply, reply_on=success)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    data: DataT,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

Arguments are similar, we expect Ok failure - handler would fail on failed case. Similarly we can have the only failure handler:

#[msg(reply, reply_on=failure)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    error: String,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

Note, that the error here can be anything implementing FromStr<Error: Display>

@hashedone
Copy link
Collaborator Author

hashedone commented Aug 23, 2024

data deserialization

To handle data field properly and roboustly, we want to support deserializing it immediately from the binary as JSON. On the other hand we want to support receiving data that is not JSON encoded.

To support both, we will introcude new trait like:

trait FromData {
  fn from_data(data: Option<Binary>) -> Result<Self, String>;
}

This trait would be use to construct the data field (or its Ok variant when handling reply_always). We would implement it for Binary - using data.ok_or_else(|| "Some error msg".to_owned()), Option<Binary> with Ok(data), and for anything implementing serde::Deserialize by trying to deserialize data.unwrap_or_default() directly to the type. We will also try to add implementation for Option<T> where T implements serde::Deserialize for edge cases where one wants to handle None data separately to empty one.

@hashedone
Copy link
Collaborator Author

Handlers renaming

We want to introduce handlers renaming in the replies. The idea is syntax as follows:

#[msg(reply=reply_bar)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    data: Result<DataT, String>,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

That should generate utility functions in a way, like the function is named reply_bar instead of reply_foo. That would be extensible to generate many handlers from the exact same implementation:

#[msg(reply=reply_bar|reply_baz)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    data: Result<DataT, String>,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

This is supposed to generate two handlers: reply_bar and reply_baz, both with the same implementation. The use case for this using separate success and failure handlers for the exact same message:

#[msg(reply=reply_foo, reply_on=success)]
fn reply_foo_success(
    &self,
    ctx: ReplyCtx,
    data: DataT,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

#[msg(reply=reply_foo, reply_on=failure)]
fn reply_foo_failure(
    &self,
    ctx: ReplyCtx,
    err: String
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

In this situation the reply_foo handler should use the reply_always strategy, and it should dispatch to either handler depending of the reply status.

This also would allow more use-cases, one of them would be sharing one part of a handler, for example:

#[msg(reply, reply_on=success)]
fn reply_foo(
    &self,
    ctx: ReplyCtx,
    data: DataT,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

#[msg(reply, reply_on=success)]
fn reply_bar(
    &self,
    ctx: ReplyCtx,
    data: DataT,
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

#[msg(reply=reply_foo|reply_bar, reply_on=failure)]
fn reply_failure(
    &self,
    ctx: ReplyCtx,
    err: String
    arg1: String,
    arg2: u64,
) -> Result<Response, ContractError>;

Here two handlers are generated: reply_foo and reply_bar, both using reply_always strategy. Both handlers have their unique processing defined for the Ok reply response, but the error variant implementation is shared.

@jawoznia
Copy link
Collaborator

closed with #418

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