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

[Feature] Define ExecutingMessageValidator #98

Open
emhane opened this issue Jan 17, 2025 · 7 comments · May be fixed by #129
Open

[Feature] Define ExecutingMessageValidator #98

emhane opened this issue Jan 17, 2025 · 7 comments · May be fixed by #129
Assignees
Labels
A-protocol Area: protocol crate C-feat Category: new feature or request D-good-first-issue Desc: issues that are easy to pick up for new contributors H-interop Hardfork: interop

Comments

@emhane
Copy link
Member

emhane commented Jan 17, 2025

Component

protocol

Describe the feature you would like

Define new trait that has functionality to
(i) parses receipts to extract ExecutingMessages from logs
(ii) validate a list of receipts, using the result from (i) under the hood to use SupervisorApiClient

pub trait ExecutingMessageValidator {
    type SupervisorClient: SupervisorApiClient;

    ...
}

Additional context

API should be flexible to use on single receipts (list of one) and multiple receipts, and flexible to allow parallelising RPC calls to supervisor and extracting messages form logs.

@emhane emhane added A-protocol Area: protocol crate C-feat Category: new feature or request H-interop Hardfork: interop D-good-first-issue Desc: issues that are easy to pick up for new contributors labels Jan 17, 2025
@emhane
Copy link
Member Author

emhane commented Jan 17, 2025

call to SupervisorApi best be it's own trait method, and can take an additional arg timeout: Option<u64> + the trait can have a constant DEFAULT_TIMEOUT. for docs about when trait method returns false, we need to add case when rpc call to supervisor times out.

@emhane
Copy link
Member Author

emhane commented Jan 18, 2025

how about this one @sergerad ?

@sergerad
Copy link
Contributor

Looks good, will start this one after I push something up for #20

@sergerad sergerad linked a pull request Jan 23, 2025 that will close this issue
@refcell
Copy link
Contributor

refcell commented Jan 23, 2025

FYI @sergerad just merged #128 which contains the supervisor trait in the maili-rpc. I'm not super happy with it living in there so we can move it around if needed :)

@emhane
Copy link
Member Author

emhane commented Jan 23, 2025

@sergerad after looking at home we will use this in op-reth transaction pool, it makes more sense if the trait methods use logs rather than receipts. the reason being that there is no need to construct and entire receipt from execution, when we will only use the logs anyway. that removes the step of extracting logs, the step that remains is extracting executing message events from logs.

@sergerad
Copy link
Contributor

FYI @sergerad just merged #128 which contains the supervisor trait in the maili-rpc. I'm not super happy with it living in there so we can move it around if needed :)

@refcell what is the relationship between pub trait Supervisor from the supervisor module and pub trait SupervisorApi from the api module?

This issue states I should use SupervisorApiClient which is generated from the latter so I'm confused as to the need for the new trait in supervisor module.

TY!

@emhane
Copy link
Member Author

emhane commented Jan 24, 2025

agreed @sergerad let's collapse these two into the one living inside of rpc module...jsonrpsee automatically impls the client version of the trait so the impl in interop crate is redundant. ref https://docs.rs/jsonrpsee/latest/jsonrpsee/async_client/struct.Client.html#method.request

probably makes sense to make the api module in rpc crate into a folder though, and having supervisor api in it's own file since the additional trait methods make it rather much loc

out of scope for this issue though. tldr; use the trait in rpc crate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-protocol Area: protocol crate C-feat Category: new feature or request D-good-first-issue Desc: issues that are easy to pick up for new contributors H-interop Hardfork: interop
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants