diff --git a/sylvia-derive/src/contract/communication/reply.rs b/sylvia-derive/src/contract/communication/reply.rs index 464f0bc0..95234e39 100644 --- a/sylvia-derive/src/contract/communication/reply.rs +++ b/sylvia-derive/src/contract/communication/reply.rs @@ -11,7 +11,36 @@ use crate::types::msg_field::MsgField; 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 @@ impl<'a> ReplyData<'a> { 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::>() + payload.skip(NO_ALLOWED_DATA_FIELDS).collect::>() } else { payload.collect::>() }; + + 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 @@ impl<'a> ReplyVariant<'a> for MsgVariant<'a> { /// 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 @@ impl DataField for MsgField<'_> { 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 = diff --git a/sylvia/tests/reply_data.rs b/sylvia/tests/reply_data.rs index aec01218..8d282f44 100644 --- a/sylvia/tests/reply_data.rs +++ b/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) diff --git a/sylvia/tests/ui/attributes/data/invalid_usage.rs b/sylvia/tests/ui/attributes/data/invalid_usage.rs new file mode 100644 index 00000000..92ca86b8 --- /dev/null +++ b/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 { + 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, + ) -> StdResult { + 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 { + Ok(Response::new()) + } + + #[sv::msg(reply, reply_on=error)] + fn reply( + &self, + _ctx: ReplyCtx, + #[sv::data(opt, raw)] _data: Option, + #[sv::payload(raw)] param: Binary, + ) -> StdResult { + Ok(Response::new()) + } + } +} + +fn main() {} diff --git a/sylvia/tests/ui/attributes/data/invalid_usage.stderr b/sylvia/tests/ui/attributes/data/invalid_usage.stderr new file mode 100644 index 00000000..fd6884a4 --- /dev/null +++ b/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, + | ^^^^^ + +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, + | ^^^^^ + +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, + | ^^^^^ diff --git a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs index 40f2da47..f19b7104 100644 --- a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs +++ b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.rs @@ -19,7 +19,12 @@ impl Contract { } #[sv::msg(reply, handlers=[handler1])] - fn reply_always(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult { + fn reply_always( + &self, + _ctx: ReplyCtx, + _result: SubMsgResult, + #[sv::payload(raw)] payload: Binary, + ) -> StdResult { Ok(Response::new()) } @@ -27,18 +32,28 @@ impl Contract { fn duplicated_success_for_reply_always( &self, _ctx: ReplyCtx, - _reply: Reply, + #[sv::payload(raw)] reply: Binary, ) -> StdResult { Ok(Response::new()) } #[sv::msg(reply, handlers=[handler2], reply_on=error)] - fn some_reply(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult { + fn some_reply( + &self, + _ctx: ReplyCtx, + error: String, + #[sv::payload(raw)] payload: Binary, + ) -> StdResult { Ok(Response::new()) } #[sv::msg(reply, reply_on=error)] - fn handler2(&self, _ctx: ReplyCtx, _reply: Reply) -> StdResult { + fn handler2( + &self, + _ctx: ReplyCtx, + error: String, + #[sv::payload(raw)] payload: Binary, + ) -> StdResult { Ok(Response::new()) } } diff --git a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr b/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr index edb8c673..a452d773 100644 --- a/sylvia/tests/ui/attributes/msg/overlapping_reply_handlers.stderr +++ b/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 { +51 | fn handler2( | ^^^^^^^^ diff --git a/sylvia/tests/ui/attributes/payload/invalid_usage.rs b/sylvia/tests/ui/attributes/payload/invalid_usage.rs new file mode 100644 index 00000000..70297070 --- /dev/null +++ b/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 { + Ok(Response::new()) + } + + #[sv::msg(reply, reply_on=success)] + fn reply(&self, #[sv::payload(raw)] _ctx: ReplyCtx) -> StdResult { + Ok(Response::new()) + } + } +} + +fn main() {} diff --git a/sylvia/tests/ui/attributes/payload/invalid_usage.stderr b/sylvia/tests/ui/attributes/payload/invalid_usage.stderr new file mode 100644 index 00000000..6896e3a4 --- /dev/null +++ b/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 { + | ^^^^^ diff --git a/sylvia/tests/ui/method_signature/reply.rs b/sylvia/tests/ui/method_signature/reply.rs index 100b8f5b..04dc6ddd 100644 --- a/sylvia/tests/ui/method_signature/reply.rs +++ b/sylvia/tests/ui/method_signature/reply.rs @@ -1,6 +1,6 @@ -#![allow(unused_imports)] +#![allow(unused_imports, unused_variables)] use sylvia::ctx::{InstantiateCtx, ReplyCtx}; -use sylvia::cw_std::{Binary, Response, StdResult}; +use sylvia::cw_std::{Addr, Binary, Response, StdResult}; pub mod mismatched_params { use super::*; @@ -30,12 +30,7 @@ pub mod mismatched_params { } #[sv::msg(reply, handlers=[on_instantiated], reply_on=error)] - fn second_reply( - &self, - _ctx: ReplyCtx, - #[sv::data(opt, raw)] _data: Option, - param: u32, - ) -> StdResult { + fn second_reply(&self, _ctx: ReplyCtx, error: String, param: u32) -> StdResult { Ok(Response::new()) } } @@ -72,7 +67,7 @@ pub mod mismatched_param_arity { fn second_reply( &self, _ctx: ReplyCtx, - #[sv::data(opt, raw)] _data: Option, + error: String, param: String, param: u32, ) -> StdResult { @@ -81,4 +76,60 @@ pub mod mismatched_param_arity { } } +pub mod redundant_params { + 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 { + Ok(Response::new()) + } + + #[sv::msg(reply, reply_on=success)] + fn first_reply( + &self, + _ctx: ReplyCtx, + redundant_before1: u32, + redundant_before2: String, + #[sv::data(opt, raw)] _data: Option, + #[sv::payload(raw)] param: String, + ) -> StdResult { + Ok(Response::new()) + } + + #[sv::msg(reply, reply_on=success)] + fn second_reply( + &self, + _ctx: ReplyCtx, + #[sv::data(opt, raw)] _data: Option, + redundant_between1: u32, + redudnant_between2: String, + #[sv::payload(raw)] param: String, + redundant_after1: Binary, + redundant_after2: Addr, + ) -> StdResult { + Ok(Response::new()) + } + + #[sv::msg(reply, reply_on=success)] + fn third_reply( + &self, + _ctx: ReplyCtx, + #[sv::data(opt, raw)] _data: Option, + #[sv::payload(raw)] param: String, + redundant: Binary, + ) -> StdResult { + Ok(Response::new()) + } + } +} + fn main() {} diff --git a/sylvia/tests/ui/method_signature/reply.stderr b/sylvia/tests/ui/method_signature/reply.stderr index d75a4008..2aad7223 100644 --- a/sylvia/tests/ui/method_signature/reply.stderr +++ b/sylvia/tests/ui/method_signature/reply.stderr @@ -13,7 +13,43 @@ error: Mismatched quantity of method parameters. = note: Both `on_instantiated` handlers should have the same number of parameters. = note: Previous definition of on_instantiated handler. - --> tests/ui/method_signature/reply.rs:62:12 + --> tests/ui/method_signature/reply.rs:57:12 | -62 | fn first_reply( +57 | fn first_reply( | ^^^^^^^^^^^ + +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/method_signature/reply.rs:102:35 + | +102 | #[sv::data(opt, raw)] _data: Option, + | ^^^^^ + +error: Redundant payload parameter. + + = note: Expected no parameters between the parameter marked with `#[sv::data]` and `#[sv::payload(raw)]`. + + --> tests/ui/method_signature/reply.rs:100:13 + | +100 | redundant_before1: u32, + | ^^^^^^^^^^^^^^^^^ + +error: Redundant payload parameter. + + = note: Expected no parameters between the parameter marked with `#[sv::data]` and `#[sv::payload(raw)]`. + + --> tests/ui/method_signature/reply.rs:113:13 + | +113 | redundant_between1: u32, + | ^^^^^^^^^^^^^^^^^^ + +error: Redundant payload parameter. + + = note: Expected no parameters after the parameter marked with `#[sv::payload(raw)]`. + + --> tests/ui/method_signature/reply.rs:128:13 + | +128 | redundant: Binary, + | ^^^^^^^^^