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 IntoEvent macro which automatically implement Into<Event> to a struct #2038

Closed
wants to merge 4 commits into from

Conversation

loloicci
Copy link
Contributor

@loloicci loloicci commented Mar 5, 2024

This PR suggests and implements a derive macro for structs named IntoEvent which is for developers of contracts.
With this, #[derive(IntoEvent)] implements Into<Event> to a decorated struct.
This makes it easier to make a structure representing an event.

An Example usage:

#[derive(Clone, IntoEvent)]
struct TransferEvent {
    from: Addr,
    receiver: Addr,
    #[to_string_fn(coins_to_string)]
    amount: Vec<Coin>,
}

let transfer_event = TransferEvent {
    from: Addr::unchecked("alice"),
    receiver: Addr::unchecked("bob"),
    amount: coins(42, "link"),
};

let response = Response::<Empty>::new().add_event(transfer_event);

See the document of derive_into_event in packages/derive/src/lib.rs to learn how to use it.

See tests in packages/std/src/results/response.rs for more example usages.

move IntoEvent macro to cosmwasm-derive

feat: add attribute "use_to_string" to IntoEvent derive macro
Copy link
Collaborator

@chipshort chipshort left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this up!
I think this is a good approach, but the derive macro is a bit out of scope for cosmwasm-std.
I suggest we merge the Into<Event> change and then the derive macro can be a separate crate that you could even maintain yourself.
Also note that attributes can be added both to events and responses directly, so maybe a more general solution for generating attributes makes more sense? This was brought up in #1928 before.

#[test]
fn using_into_event() {
// IntoEvent can be used only when cosmwasm_std is imported as `cosmwasm_std`
use crate as cosmwasm_std;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Neat!

@loloicci
Copy link
Contributor Author

loloicci commented Mar 8, 2024

@chipshort

I suggest we merge the Into change and then the derive macro can be a separate crate that you could even maintain yourself.

It looks good! Should I reopen a new PR for it? Or will you do it?

@chipshort
Copy link
Collaborator

@loloicci Yes, either a new PR or rebase this one. Also, please add a changelog entry for it under the "Unreleased" section.

@loloicci
Copy link
Contributor Author

@chipshort
OK, I will make a new PR. Thanks.

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