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 users to react to certain events #467

Open
igamigo opened this issue Aug 5, 2024 · 7 comments
Open

Allow users to react to certain events #467

igamigo opened this issue Aug 5, 2024 · 7 comments
Milestone

Comments

@igamigo
Copy link
Collaborator

igamigo commented Aug 5, 2024

As a user, you might be interested in note flows and lifecycles specific to your application only. In this sense, the client does not currently provide many mechanisms to programmatically react to events in a specific manner. In other words, in order to update the local view of the blockchain, the user needs to periodically call the Client::sync_state() API which updates the local entities based on the information that it requests and receives. These information is, in turn, based on the previous state (for example, nullifiers are requested for any note that the store is tracking as Committed) and tags (which can be added and removed by the user).

It would be useful for the user to have a way to react to certain events. In general, any received updates might be of interest to the user depending on the application, but some that come to mind as important are:

  • Notes matched by tags: It might be of interest to the user to update some other part of the application state based on received notes, or decide which received notes are stored, since tags only partially provide this functionality. This is somewhat related to the NoteScreener which already does this filtering, so perhaps it could be solved by adding a way to programmatically add checks here.
  • Notes being consumed (matched nullifiers).
  • Transactions getting committed/canceled: This might not be as important, since reacting to updates is currently virtually similar to just manually searching for transactions after syncing. Perhaps this can be solved with just changing the APIs accordingly (for instance, just returning a list of rejected and accepted transactions after a sync).

The mechanisms through which the user could react to these events (event queues that can be consumed, trait objects, callbacks, channels, etc.) can be discussed separately.

@bobbinth
Copy link
Contributor

bobbinth commented Aug 8, 2024

Notes matched by tags: It might be of interest to the user to update some other part of the application state based on received notes, or decide which received notes are stored, since tags only partially provide this functionality. This is somewhat related to the NoteScreener which already does this filtering, so perhaps it could be solved by adding a way to programmatically add checks here.

I'm wondering if the current NoteScreener can somehow be "morphed" into this customizable functionality. For example, conceptually we could have some on_note_detected() method that could take a note as an input and return NoteConsumability.

Or we could take it even further on_note_detected() look something like this:

pub fn on_note_detected(&self, note: CommittedNote) -> Result<Option<NoteWithProof>, SomeError> {
    ...
}

The default implementation of this method would do all the work of transforming a CommitedNote into some struct (NoteWithProof) which will then be upserted into the store. But the user will be able to override this functionality with a custom implementation.

Not sure if the above makes sense yet - just brainstorming the ideas.

@igamigo igamigo added this to the v0.7.0 milestone Nov 7, 2024
@tomyrd
Copy link
Collaborator

tomyrd commented Nov 27, 2024

Two separate functionalities are mentioned here:

  • Being able to influence the client functionality, in this case when receiving new notes or when analyzing it's consumability.
  • Notify the user when certain note transitions happen.

I think they should be tackled in separate PRs


After working a little bit on the miden order book, I feel like this type of functionality could have been used for some quality of life improvements. Specifically, if we could somehow let the Client know which custom notes are consumable by which tracked accounts. In the order book, for example, we could hide non-consumable orders (there are some orders the user may not have enough funds to fill) by simply calling client.get_consumable_notes when listing.

In terms of specific note events, with the new input note state handling we have some better defined events which we could notify the user when they happen. I'm not sure how difficult would it be to add this, but I think it would be more granular than just detecting new notes.

@bobbinth
Copy link
Contributor

Yes, there should be probably multiple PRs to implement various aspects of what is discussed here. I think first we need to figure out how the user can "plug in" certain functionality into the client (i.e., come up with a general approach) and then we can figure out what specific things we may want to extend.

A much more radical approach could be to break the client into independent components somehow which the user would then be able to compose to in the ways they see fit (in this model CLI would demonstrate how these components can be combined to build a user-facing client).

@igamigo
Copy link
Collaborator Author

igamigo commented Nov 28, 2024

I think a simple but maybe effective approach would be if we could have a component in Client that looks something like this:

pub struct Client<R: FeltRng> {
    // ...
    listeners: Vec<Arc<dyn NoteEventListener>>,
}

and we could provide at least a default event listener that essentially replaces NoteScreener:

struct StandardNoteScreener;

impl NoteEventListener for StandardNoteScreener {
    fn on_input_note_synced(&self, note: &Note, new_status: NoteStatus) -> NoteEventOutcome {
         // Check if note is P2ID, P2IDR or SWAP and if so return the correct outcome
    }

    fn on_input_note_state_changed(&self, note: &Note, old_status: Option<NoteStatus>, new_status: NoteStatus) {
    }

    fn on_input_note_discarded(&self, note: &Note) {
    }

    fn on_nullifier_spent(&self, note: &Note) {
    }

    // ...
}

let mut client = Client::new(/* args */);
client.add_listener(MyListener);

// Or perhaps have a builder API for the client

ClientBuilder::new(store, rpc)
    .with_prover(RemoteTransactionProver::new(endpoint))
    .with_listener(StandardNoteScreener) // Should probably be a default
    .with_listener(CLOBNoteEventListener);

Where NoteEventOutcome would describe if the client should keep the note, discard it, save it as an input note (for output notes), maybe even completely remove them from the store, etc. Obviously function signatures here and events are just a draft.

Or maybe we could add these event listeners to NoteScreener and have that be a more explicit component of Client.
It feels like as the protocol evolves, this would be a good way to progressively build toward a more "reactable" client since it should be (hopefully) fairly backward compatible as long as fundamental lifecycles don't change too much.

One problem that would need solving with this (although it already exists with user-added note tags), is that if users create an incorrect set of event listeners but the client has already synced past the point of "interesting" events then there might be some information loss there, so maybe there should be a special way to sync from-to specific blocks, but this is outside the scope of this issue.

@tomyrd
Copy link
Collaborator

tomyrd commented Dec 3, 2024

I like the idea of the NoteEventListener instead of an exposed event queue. I remember @SantiagoPittella also mentioned, in one of our meetings, having specific callback functions for these events. These could be set up like the listeners:

ClientBuilder::new(store, rpc)
    ...
    .with_new_note_received_callback(custom_clob_callback);

This approach may be simpler, but we wouldn't have the reaction functions centralized in one struct (which is something I personally prefer). Especially if we could have default implementations for most of the methods so that users only implement whichever they prefer.

A much more radical approach could be to break the client into independent components somehow which the user would then be able to compose to in the ways they see fit

I'm not sure if I understand this completely. Do we mean having the user implement specific components? (like what we do with the store and rpc). Maybe by having well defined interfaces for the NoteScreener and creating objects that deal with the internal sync and transaction workflows this could be possible. The user would need to have a good mental model of how the client works and deals with state changes (this is also true for the previous approach, I think improving the public documentation of these workflows will be an important part of the changes).

@bobbinth
Copy link
Contributor

bobbinth commented Dec 3, 2024

I'm not sure if I understand this completely. Do we mean having the user implement specific components? (like what we do with the store and rpc). Maybe by having well defined interfaces for the NoteScreener and creating objects that deal with the internal sync and transaction workflows this could be possible. The user would need to have a good mental model of how the client works and deals with state changes (this is also true for the previous approach, I think improving the public documentation of these workflows will be an important part of the changes).

I don't have any well-formed ideas on this, but the general idea was to provide more "stand-alone" components that the user would be able to combine in various ways that we cannot currently anticipate.

For example, one such component could be StateSync component. We'd instantiate it with some set of parameters (e.g., note tags, nullifiers, etc. we are interested in) as well as some sub-components (e.g., RPC to use to make requests to the network). Then, we'd call sync() on it and it would run the sync and give us all the relevant data, but without saving it to the store. So, basically, StateSync becomes an independent component that can be used even outside of the Client. Then users would be able to compose it with the Store component as they see fit.

Not sure if this specific example makes sense, but that's roughly what I was thinking here.

@tomyrd
Copy link
Collaborator

tomyrd commented Dec 18, 2024

Yeah, I think I understand better now. I do think this is a separate issue that the one about events. I created issue #643 to track this and will be adding some ideas.

Both of this issues are related though. If we have better defined stand-alone components that users can compose/modify and we clearly define their interactions, then users can change them and they would serve as glue between the client processes and the user functionality.

@bobbinth bobbinth modified the milestones: v0.7.0, v0.8.0 Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

3 participants