-
Notifications
You must be signed in to change notification settings - Fork 35
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
Comments
I'm wondering if the current Or we could take it even further 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 Not sure if the above makes sense yet - just brainstorming the ideas. |
Two separate functionalities are mentioned here:
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 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. |
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). |
I think a simple but maybe effective approach would be if we could have a component in pub struct Client<R: FeltRng> {
// ...
listeners: Vec<Arc<dyn NoteEventListener>>,
} and we could provide at least a default event listener that essentially replaces 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 Or maybe we could add these event listeners to 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. |
I like the idea of the 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.
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 |
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 Not sure if this specific example makes sense, but that's roughly what I was thinking here. |
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. |
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 asCommitted
) 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:
NoteScreener
which already does this filtering, so perhaps it could be solved by adding a way to programmatically add checks here.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.
The text was updated successfully, but these errors were encountered: