-
Notifications
You must be signed in to change notification settings - Fork 16
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
feat: Better reply support (#418) #452
base: main
Are you sure you want to change the base?
Conversation
New optional parameters: - handlers - (handler1, handler2, ..) - reply_on - success|failure|always
Pass handlers bracketed instead of parenthesized
Remove redundancy between `MsgAttr` and `MsgType`
Since this change would be breaking it's hidden behind `sv_replies` feature flag.
Remove `sv_replies` feature as it does not make the change non breaking.
Expect `(raw)` parameter in the `sv::payload` attribute.
Remove unnecessary `#[allow(deprecated)]` statements.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #452 +/- ##
==========================================
+ Coverage 70.08% 72.37% +2.29%
==========================================
Files 53 63 +10
Lines 3085 3823 +738
==========================================
+ Hits 2162 2767 +605
- Misses 923 1056 +133 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replies are part of a mechanism that is not trivial for non-sylvia developers. Handling them in sylvia is a huge milestone for this crate and this should make it more useful for sylvia devs. That's a great work done and pretty complex issue solved here! Since the final PR combines all of the partial work checked already in other PRs, I'm not that worried about the details here. I left some clean-up related comments that we should consider before merging it to the main branch. Thank you for this implementation, well done!
.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() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that sylvia prints reply data. What would happen if the msg content size is for e.g. 1MB?
Just one more question. What would happen if we implement such a reply: #[sv::msg(reply, reply_on=success)]
fn some_reply(
&self,
_ctx: ReplyCtx,
#[sv::data] _data: ComplexData,
#[sv::payload(raw)] _payload: Binary,
some_custom_arg: String // We provide sv::payload(raw) and a custom arg
) -> Result<Response, ContractError> {
Ok(Response::new())
} |
Good question. I haven't thought about that. I will check |
This was potentially a breaking change
Uncomment auto deserialization of payload in tests. Add test case for deserialization of custom data type.
No description provided.