chore: Review fixes

sylvia-derive/src/contract/communication/reply.rs

@@ -11,7 +11,36 @@
 use crate::types::msg_variant::{MsgVariant, MsgVariants};
 use crate::utils::emit_turbofish;
 
-const NUMBER_OF_DATA_FIELDS: usize = 1;
+const NO_ALLOWED_DATA_FIELDS: usize = 1;
+const NO_ALLOWED_RAW_PAYLOAD_FIELDS: usize = 1;
+
+/// Make sure that there are no additional parameters between ones marked
+/// with `sv::data` and `sv::payload` and after the one marked with `sv::payload`.
+fn assert_no_redundant_params(payload: &[&MsgField]) {
+    let payload_param = payload.iter().enumerate().find(|(_, field)| {
+        ParsedSylviaAttributes::new(field.attrs().iter())
+            .payload
+            .is_some()
+    });
+
+    if payload.len() == NO_ALLOWED_RAW_PAYLOAD_FIELDS {
+        return;
+    }
+
+    let Some((index, payload_param)) = payload_param else {
+        return;
+    };
+
+    if index == 0 {
+        emit_error!(payload[1].name().span(), "Redundant payload parameter.";
+            note = payload_param.name().span() => "Expected no parameters after the parameter marked with `#[sv::payload(raw)]`."
+        )
+    } else {
+        emit_error!(payload[0].name().span(), "Redundant payload parameter.";
+            note = payload_param.name().span() => "Expected no parameters between the parameter marked with `#[sv::data]` and `#[sv::payload(raw)]`."
+        )
+    }
+}
 
 pub struct Reply<'a> {
     source: &'a ItemImpl,
@@ -208,10 +237,18 @@
         variant.validate_fields_attributes();
         let payload = variant.fields().iter();
         let payload = if data.is_some() || variant.msg_attr().reply_on() != ReplyOn::Success {
-            payload.skip(NUMBER_OF_DATA_FIELDS).collect::<Vec<_>>()
+            payload.skip(NO_ALLOWED_DATA_FIELDS).collect::<Vec<_>>()
         } else {
             payload.collect::<Vec<_>>()
         };
+
+        if payload.is_empty() {
+            emit_error!(variant.name().span(), "Missing payload parameter.";
+                note =  "Expected at least one payload parameter at the end of parameter list."
+            )
+        }
+
+        assert_no_redundant_params(&payload);
         let method_name = variant.function_name();
         let reply_on = variant.msg_attr().reply_on();
 
@@ -494,14 +531,31 @@
     /// Validates attributes and returns `Some(MsgField)` if a field marked with `sv::data` attribute
     /// is present and the `reply_on` attribute is set to `ReplyOn::Success`.
     fn as_data_field(&'a self) -> Option<&'a MsgField<'a>> {
-        let data_attrs = self.fields().first().map(|field| {
+        let data_param = self.fields().iter().enumerate().find(|(_, field)| {
             ParsedSylviaAttributes::new(field.attrs().iter())
                 .data
                 .is_some()
         });
-        match data_attrs {
-            Some(attrs) if attrs && self.msg_attr().reply_on() == ReplyOn::Success => {
-                self.fields().first()
+        match data_param {
+            Some((index, field))
+                if self.msg_attr().reply_on() == ReplyOn::Success && index == 0 =>
+            {
+                Some(field)
+            }
+            Some((index, field))
+                if self.msg_attr().reply_on() == ReplyOn::Success && index != 0 =>
+            {
+                emit_error!(field.name().span(), "Wrong usage of `#[sv::data]` attribute.";
+                    note = "The `#[sv::data]` attribute can only be used on the first parameter after the `ReplyCtx`."
+                );
+                None
+            }
+            Some((_, field)) if self.msg_attr().reply_on() != ReplyOn::Success => {
+                emit_error!(field.name().span(), "Wrong usage of `#[sv::data]` attribute.";
+                    note = "The `#[sv::data]` attribute can only be used in `success` scenario.";
+                    note = format!("Found usage in `{}` scenario.", self.msg_attr().reply_on())
+                );
+                None
             }
             _ => None,
         }
@@ -523,8 +577,18 @@
         let sylvia = crate_module();
         let data = ParsedSylviaAttributes::new(self.attrs().iter()).data;
         let missing_data_err = "Missing reply data field.";
+        let transaction_id = quote! {
+            env
+                .transaction
+                .as_ref()
+                .map(|tx| format!("{}", &tx.index))
+                .unwrap_or_else(|| "Missing".to_string())
+        };
         let invalid_reply_data_err = quote! {
-            format! {"Invalid reply data: {}\nSerde error while deserializing {}", data, err}
+            format! {"Invalid reply data at block height: {}, transaction id: {}.\nSerde error while deserializing {}",
+                env.block.height,
+                #transaction_id,
+                err}
         };
         let execute_data_deserialization = quote! {
             let deserialized_data =

sylvia/tests/reply_data.rs

@@ -336,9 +336,11 @@ fn data_opt() {
         .call(&owner)
         .unwrap_err();
     assert_eq!(
-            err,
-            StdError::generic_err("Invalid reply data: SW52YWxpZERhdGE=\nSerde error while deserializing Error parsing into type alloc::string::String: Invalid type").into()
-        );
+        err,
+        StdError::generic_err(
+            "Invalid reply data at block height: 12345, transaction id: 0.\nSerde error while deserializing Error parsing into type alloc::string::String: Invalid type"
+        ).into()
+    );
 
     contract
         .send_message_expecting_data(data.clone(), DATA_OPT_REPLY_ID)
@@ -377,7 +379,12 @@ fn data() {
         .send_message_expecting_data(invalid_data, DATA_REPLY_ID)
         .call(&owner)
         .unwrap_err();
-    assert_eq!(err, StdError::generic_err("Invalid reply data: SW52YWxpZERhdGE=\nSerde error while deserializing Error parsing into type alloc::string::String: Invalid type").into());
+    assert_eq!(
+        err,
+        StdError::generic_err(
+            "Invalid reply data at block height: 12345, transaction id: 0.\nSerde error while deserializing Error parsing into type alloc::string::String: Invalid type"
+        ).into()
+    );
 
     contract
         .send_message_expecting_data(data, DATA_REPLY_ID)

sylvia/tests/ui/attributes/data/invalid_usage.rs

@@ -0,0 +1,63 @@
+#![allow(unused_imports, unused_variables)]
+use sylvia::ctx::{InstantiateCtx, ReplyCtx};
+use sylvia::cw_std::{Addr, Binary, Response, StdResult};
+
+pub mod attributes_swapped {
+    use super::*;
+
+    pub struct Contract {}
+
+    #[sylvia::contract]
+    #[sv::features(replies)]
+    impl Contract {
+        pub const fn new() -> Self {
+            Self {}
+        }
+
+        #[sv::msg(instantiate)]
+        fn instantiate(&self, _ctx: InstantiateCtx) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+
+        #[sv::msg(reply, reply_on=success)]
+        fn reply(
+            &self,
+            _ctx: ReplyCtx,
+            #[sv::payload(raw)] param: Binary,
+            #[sv::data(opt, raw)] _data: Option<Binary>,
+        ) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+    }
+}
+
+pub mod error_handler {
+    use super::*;
+
+    pub struct Contract {}
+
+    #[sylvia::contract]
+    #[sv::features(replies)]
+    impl Contract {
+        pub const fn new() -> Self {
+            Self {}
+        }
+
+        #[sv::msg(instantiate)]
+        fn instantiate(&self, _ctx: InstantiateCtx) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+
+        #[sv::msg(reply, reply_on=error)]
+        fn reply(
+            &self,
+            _ctx: ReplyCtx,
+            #[sv::data(opt, raw)] _data: Option<Binary>,
+            #[sv::payload(raw)] param: Binary,
+        ) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+    }
+}
+
+fn main() {}

sylvia/tests/ui/attributes/data/invalid_usage.stderr

@@ -0,0 +1,27 @@
+error: Wrong usage of `#[sv::data]` attribute.
+
+         = note: The `#[sv::data]` attribute can only be used on the first parameter after the `ReplyCtx`.
+
+  --> tests/ui/attributes/data/invalid_usage.rs:27:35
+   |
+27 |             #[sv::data(opt, raw)] _data: Option<Binary>,
+   |                                   ^^^^^
+
+error: Redundant payload parameter.
+
+         = note: Expected no parameters after the parameter marked with `#[sv::payload(raw)]`.
+
+  --> tests/ui/attributes/data/invalid_usage.rs:27:35
+   |
+27 |             #[sv::data(opt, raw)] _data: Option<Binary>,
+   |                                   ^^^^^
+
+error: Wrong usage of `#[sv::data]` attribute.
+
+         = note: The `#[sv::data]` attribute can only be used in `success` scenario.
+         = note: Found usage in `error` scenario.
+
+  --> tests/ui/attributes/data/invalid_usage.rs:55:35
+   |
+55 |             #[sv::data(opt, raw)] _data: Option<Binary>,
+   |                                   ^^^^^

sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs

@@ -19,26 +19,41 @@ impl Contract {
     }
 
     #[sv::msg(reply, handlers=[handler1])]
-    fn reply_always(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult<Response> {
+    fn reply_always(
+        &self,
+        _ctx: ReplyCtx,
+        _result: SubMsgResult,
+        #[sv::payload(raw)] payload: Binary,
+    ) -> StdResult<Response> {
         Ok(Response::new())
     }
 
     #[sv::msg(reply, handlers=[handler1], reply_on=success)]
     fn duplicated_success_for_reply_always(
         &self,
         _ctx: ReplyCtx,
-        _reply: Reply,
+        #[sv::payload(raw)] reply: Binary,
     ) -> StdResult<Response> {
         Ok(Response::new())
     }
 
     #[sv::msg(reply, handlers=[handler2], reply_on=error)]
-    fn some_reply(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult<Response> {
+    fn some_reply(
+        &self,
+        _ctx: ReplyCtx,
+        error: String,
+        #[sv::payload(raw)] payload: Binary,
+    ) -> StdResult<Response> {
         Ok(Response::new())
     }
 
     #[sv::msg(reply, reply_on=error)]
-    fn handler2(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult<Response> {
+    fn handler2(
+        &self,
+        _ctx: ReplyCtx,
+        error: String,
+        #[sv::payload(raw)] payload: Binary,
+    ) -> StdResult<Response> {
         Ok(Response::new())
     }
 }

sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr

@@ -2,16 +2,16 @@ error: Duplicated reply handler.
 
          = note: Previous definition of handler=`HANDLER_1_REPLY_ID` for reply_on=`always` defined on `fn reply_always()`
 
-  --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:26:32
+  --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:31:32
    |
-26 |     #[sv::msg(reply, handlers=[handler1], reply_on=success)]
+31 |     #[sv::msg(reply, handlers=[handler1], reply_on=success)]
    |                                ^^^^^^^^
 
 error: Duplicated reply handler.
 
          = note: Previous definition of handler=`HANDLER_2_REPLY_ID` for reply_on=`error` defined on `fn some_reply()`
 
-  --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:41:8
+  --> tests/ui/attributes/msg/overlapping_reply_handlers.rs:51:8
    |
-41 |     fn handler2(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult<Response> {
+51 |     fn handler2(
    |        ^^^^^^^^

sylvia/tests/ui/attributes/payload/invalid_usage.rs

@@ -0,0 +1,29 @@
+#![allow(unused_imports, unused_variables)]
+use sylvia::ctx::{InstantiateCtx, ReplyCtx};
+use sylvia::cw_std::{Addr, Binary, Response, StdResult};
+
+pub mod used_on_context {
+    use super::*;
+
+    pub struct Contract {}
+
+    #[sylvia::contract]
+    #[sv::features(replies)]
+    impl Contract {
+        pub const fn new() -> Self {
+            Self {}
+        }
+
+        #[sv::msg(instantiate)]
+        fn instantiate(&self, _ctx: InstantiateCtx) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+
+        #[sv::msg(reply, reply_on=success)]
+        fn reply(&self, #[sv::payload(raw)] _ctx: ReplyCtx) -> StdResult<Response> {
+            Ok(Response::new())
+        }
+    }
+}
+
+fn main() {}

sylvia/tests/ui/attributes/payload/invalid_usage.stderr

@@ -0,0 +1,8 @@
+error: Missing payload parameter.
+
+         = note: Expected at least one payload parameter at the end of parameter list.
+
+  --> tests/ui/attributes/payload/invalid_usage.rs:23:12
+   |
+23 |         fn reply(&self, #[sv::payload(raw)] _ctx: ReplyCtx) -> StdResult<Response> {
+   |            ^^^^^