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

Allow matching for multiple arbitrary event types with one filter #4356

Open
DCNick3 opened this issue Mar 11, 2024 · 2 comments
Open

Allow matching for multiple arbitrary event types with one filter #4356

DCNick3 opened this issue Mar 11, 2024 · 2 comments
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@DCNick3
Copy link
Contributor

DCNick3 commented Mar 11, 2024

After the merge of #4240, it would be possible to filter for multiple event types with a single filter, but only if they are coming from the same category (for example, both are AccountEvents).

A logical extension of that would be to allow to filter for multiple events from different categories. This can be accomplished by making the DataEventFilter along with EventFilterBox and TriggeringEventFilterBox into structures instead of enums and making them test all the filters sequentially (how about the structure size? can it become too large?)

Originally posted by @mversic in #4240 (comment)

@DCNick3 DCNick3 added Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST api-changes Changes in the API for client libraries labels Mar 11, 2024
@DCNick3
Copy link
Contributor Author

DCNick3 commented Mar 15, 2024

We can't currently combine arbitrary filters:

  1. Combining filters of the same type

Can't do this because id matchers have only 1 id. We cant combine AccountEventFilter::new().for_account(acc_id1) with AccountEventFilter::new().for_account(acc_id2). This prevents us from using a very natural | operator, because the semantics would be really confusing (we would have to evict some of the id matchers).

Potential solutions:

1.1. Do not provide an | operator, but explicit function that replaces the account event filter: DataEventFilterBox::empty().with_account(filter1).with_account(filter2) (only filter2 active). Maybe even make a typestate-based API to ensure account filer.

The filter implementation will have one field for each event type storing the corresponding filter (as they are implemented today)

1.2. Give up on the idea of bounded-size filters and allow full-fledged logical expressions (probably a bad idea and should be handled by the trigger itself)

  1. Combining domain events

What should be semantics of combining a DomainEvent and an AccountEvent? What if their id matchers are different?

A naive approach to just directly match them without any connection to each other will allow to match stuff from two different domains, but is this a meaningful semantics, especially since we can't do different account ids with AccountEvents. This, again nudges to the idea of more arbitrary event filters...

@DCNick3
Copy link
Contributor Author

DCNick3 commented Mar 28, 2024

It was somewhat handled with the merge #4366 (listen_events now accepts an array of event filters). For smart contracts - just install multiple triggers (I think the wasm is deduplicated in this case). Any other places where one needs to listen to multiple events?

@DCNick3 DCNick3 removed their assignment Mar 29, 2024
@mversic mversic added this to Iroha Oct 10, 2024
@mversic mversic moved this to Roadmap in Iroha Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-changes Changes in the API for client libraries Enhancement New feature or request iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

1 participant