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

Sylvia: Interoperability Cw <-> Sv #97

Merged
merged 3 commits into from
Jul 25, 2024
Merged

Conversation

jawoznia
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Jul 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
cosmwasm-docs ✅ Ready (Inspect) Visit Preview 12 resolved Jul 25, 2024 11:07am

Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments on vercel

Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better structure than before, but still some comments about wording. Also it is like less information is there. I miss examples - in particular executor and queried usage.

Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment on vercel. When documenting you need to think on all the usecases, not only the basic ones- in fact our software is more often used for chain-dedicated contracts.

@jawoznia
Copy link
Contributor Author

jawoznia commented Jul 23, 2024

Better structure than before, but still some comments about wording. Also it is like less information is there. I miss examples - in particular executor and queried usage.

Those are explained in their own page. This page is linked in the docs.

One comment on vercel. When documenting you need to think on all the usecases, not only the basic ones- in fact our software is more often used for chain-dedicated contracts.

You are right. I missed customs when describing usage of MultiTest

@hashedone
Copy link
Collaborator

hashedone commented Jul 23, 2024

Those are explained in their own page. This page is linked in the docs.

Ahh, the way you linked them made me think those are links to docs.rs; I didn't even click them, tbh. This is a good separation, but I'll add the additional sentence "Read more about how to use them in dedicated Communication section" (and make "Communication" a separate link to the chapter on its own, leaving both entity links pointing to sections directly).

Copy link
Collaborator

@hashedone hashedone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll accept it (there are 3 more small comments in vercel, no more review required when you consider this). However we miss an important part here - submessages workflow.

You briefly mentioned sending messages, but what about submessages? Sure, you send them the same way just use the add_submessage and add some stuff, but there is more housekeeping about them - in particular reply handling. This is described very briefly in sv::override_entry_point but I don't believe it is solid enough. First of all - there is no link to that in this place, it is too much to "figure out". Second of all - this is a very common case and requires attention. We need to add separate section here about this, that would link to override_entry_point, and it would contain well described minimal example of sending submessage to the remote contract (do not put the remote contract code in the example - it is irrelevant boilerplate, use something well-know like cw20-base or cw4-group), and then handling response (including a simple entry point overwrite). Make sure to explain, that without that, the contract would not well-behave in multitest.

I'll create issue for that so it's done in separate PR.

@jawoznia jawoznia force-pushed the jawoznia/sylvia/interoperability branch from 1f8ff93 to d47fb22 Compare July 25, 2024 11:06
@jawoznia
Copy link
Contributor Author

I'll accept it (there are 3 more small comments in vercel, no more review required when you consider this). However we miss an important part here - submessages workflow.

You briefly mentioned sending messages, but what about submessages? Sure, you send them the same way just use the add_submessage and add some stuff, but there is more housekeeping about them - in particular reply handling. This is described very briefly in sv::override_entry_point but I don't believe it is solid enough. First of all - there is no link to that in this place, it is too much to "figure out". Second of all - this is a very common case and requires attention. We need to add separate section here about this, that would link to override_entry_point, and it would contain well described minimal example of sending submessage to the remote contract (do not put the remote contract code in the example - it is irrelevant boilerplate, use something well-know like cw20-base or cw4-group), and then handling response (including a simple entry point overwrite). Make sure to explain, that without that, the contract would not well-behave in multitest.

I'll create issue for that so it's done in separate PR.

Good catch!

@jawoznia jawoznia merged commit d2d111b into main Jul 25, 2024
5 checks passed
@jawoznia jawoznia deleted the jawoznia/sylvia/interoperability branch July 25, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants