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

Shared external event definitions #809

Closed
Robbepop opened this issue Jun 8, 2021 · 3 comments · Fixed by #1827
Closed

Shared external event definitions #809

Robbepop opened this issue Jun 8, 2021 · 3 comments · Fixed by #1827
Assignees
Labels
A-ink_lang [ink_lang] Work item B-design Designing a new component, interface or functionality. B-enhancement New feature or request

Comments

@Robbepop
Copy link
Collaborator

Robbepop commented Jun 8, 2021

Due to ink!'s design it is not possible to share event definitions between multiple contracts since events can only be defined in the ink! module scope directly.
In order to more extensively use the upcoming trait features we want to make it possible for users to define events to be used in multiple contracts.

For this we will need a new attribute macro #[ink::event_definition] or just a derive macro #[derive(ink::Event)] that will setup everything required for ink! events. There will be some initial limitations. E.g. events can only be enum or struct with certain properties.
However, this will allow to define events in some library and use the same event definition in many ink! smart contract or trait definitions.

@yjhmelody
Copy link
Contributor

yjhmelody commented Jun 9, 2021

Hi, the topic design of the current event seems to be related to the contract, and may also need to be considered.
For example, erc20:

#[cfg(not(feature = "ink-as-dependency"))]
    const _: () = {
        impl ::ink_env::Topics for Transfer {
            type RemainingTopics = [::ink_env::topics::state::HasRemainingTopics; 4usize];
            fn topics<E, B>(
                &self,
                builder: ::ink_env::topics::TopicsBuilder<
                    ::ink_env::topics::state::Uninit,
                    E,
                    B,
                >,
            ) -> <B as ::ink_env::topics::TopicsBuilderBackend<E>>::Output
            where
                E: ::ink_env::Environment,
                B: ::ink_env::topics::TopicsBuilderBackend<E>,
            {
                builder
                    .build::<Self>()
                    .push_topic::<::ink_env::topics::PrefixedValue<[u8; 15usize]>>(
                        &::ink_env::topics::PrefixedValue {
                            value: b"Erc20::Transfer",
                            prefix: b"",
                        },
                    )
                    .push_topic::<::ink_env::topics::PrefixedValue<Option<AccountId>>>(
                        &::ink_env::topics::PrefixedValue {
                            value: &self.from,
                            prefix: b"Erc20::Transfer::from",
                        },
                    )
                    .push_topic::<::ink_env::topics::PrefixedValue<Option<AccountId>>>(
                        &::ink_env::topics::PrefixedValue {
                            value: &self.to,
                            prefix: b"Erc20::Transfer::to",
                        },
                    )
                    .push_topic::<::ink_env::topics::PrefixedValue<Balance>>(
                        &::ink_env::topics::PrefixedValue {
                            value: &self.value,
                            prefix: b"Erc20::Transfer::value",
                        },
                    )
                    .finish()
            }
        }
    };

These prefixs are contract related data and not appear in metadata.

@xgreenx
Copy link
Collaborator

xgreenx commented Nov 13, 2021

@Robbepop What do you think about the idea to change the way have we generated the metadata? I created an example of how we can add events to the metadata if they were defined in a separate crate.

@xgreenx
Copy link
Collaborator

xgreenx commented Nov 19, 2021

To achieve user-friendly events they should be independent. At the moment the user can define events only in the body of the contract(under #[ink(contract)] macro). Those events are available only in the scope of that contract. So it is not possible to import them from another crate.

We have to reason for that limitation:

  1. The identifier of the event is generated based on the name of the contract.
  2. We can't collect metadata from other crates. So we can't generate metadata for those events because we don't know about them.

Solution:

  1. Can be resolved if we change the model of how identifiers are generated. For example, we can calculate it based on the name of the event(maybe the count of fields also should affect that too). The topic's identifier can be a combination of the name of the event + the index of the field. The user is able to add a prefix to the event. For example #[ink(event(prefix = "Erc20"))].

  2. This repository shows an example(for more details check the readme and comments in the code) of how we can collect all events from other crates used in the contract. With this approach, we will not miss events in metadata, but we will collect used and not used events, but I think it is not a problem for the ABI to contains also useless events. The idea is: attribute macro #[ink(event)] or #[ink::event_definition] will add on_startup macro for the event. This macro will add an event to a static metadata variable on the start-up of the metadata binary. Metadata binary will process all events from a static variable and will generate ABI based on events from that variable and messages from the contract body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ink_lang [ink_lang] Work item B-design Designing a new component, interface or functionality. B-enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants