Add ExecutingMessageValidator trait #129
Conversation
sergerad
force-pushed
the
sergerad/executing-message-validator
branch
from
42518b6 to
f79be89
Compare
crates/rpc/src/traits.rs
Outdated
| // TODO: should we `impl From<Log> for ExecutingMessage`? | ||
| // There is `impl From<Log> for MessagePayload` but I'm unsure | ||
| // about the relationship between `MessagePayload` and `ExecutingMessage` | ||
| ExecutingMessage::abi_decode(&log.data.data, true) |
There was a problem hiding this comment.
@emhane Do you have any advice around the log decoding? The comment above this line summarizes my question(s).
If the approach here is in fact correct (ExecutingMessage::abi_decode(&log.data.data, true)), then should I add some logic that checks topics before attempting to decode? So as to filter out irrelevant logs rather than error out. E.G. similar idea to pub const CONFIG_UPDATE_TOPIC: B256.
TY!
There was a problem hiding this comment.
you can find the logic to extract the executing messages here
https://github.com/op-rs/kona/blob/a59f643d0627320efff49f40f4803741ae9194f1/crates/interop/src/message.rs#L102-L114
not sure what the magic number 2 should be called as a constant instead @clabby ?
you're totally right that we don't want to return error, instead we just want to filter that log out if it doesn't contain an executing message
crates/rpc/src/traits.rs
Outdated
| fn parse_messages( | ||
| logs: &[Log], | ||
| ) -> Result<Vec<ExecutingMessage>, ExecutingMessageValidatorError> { |
There was a problem hiding this comment.
| fn parse_messages( | |
| logs: &[Log], | |
| ) -> Result<Vec<ExecutingMessage>, ExecutingMessageValidatorError> { | |
| fn filter_executing_msgs( | |
| logs: &[Log], | |
| ) -> Result<impl IntoIterator<Item = ExecutingMessage>, ExecutingMessageValidatorError> { |
let the caller collect better..maybe for some reason the caller wants to bail early, if we don't know, better leave it open by returning iterator and letting caller collect into vec
crates/rpc/src/traits.rs
Outdated
| // TODO: should we `impl From<Log> for ExecutingMessage`? | ||
| // There is `impl From<Log> for MessagePayload` but I'm unsure | ||
| // about the relationship between `MessagePayload` and `ExecutingMessage` | ||
| ExecutingMessage::abi_decode(&log.data.data, true) |
There was a problem hiding this comment.
you can find the logic to extract the executing messages here
https://github.com/op-rs/kona/blob/a59f643d0627320efff49f40f4803741ae9194f1/crates/interop/src/message.rs#L102-L114
not sure what the magic number 2 should be called as a constant instead @clabby ?
you're totally right that we don't want to return error, instead we just want to filter that log out if it doesn't contain an executing message
sergerad
force-pushed
the
sergerad/executing-message-validator
branch
from
f79be89 to
deb0b94
Compare
| use alloy_sol_types::SolEvent; | ||
| use maili_interop::ExecutingMessage as InteropMessage; | ||
| use maili_interop::CROSS_L2_INBOX_ADDRESS; | ||
| use maili_protocol::ExecutingMessage as ProtocolMessage; |
There was a problem hiding this comment.
@emhane I have the two ExecutingMessage types here just for the sake of compilation atm.
The snippet you shared for parsing logs for ExecutingMessage relies on the interop crate. But the SupervisorApiClient uses the struct from protocol crate.
WDYT is the correct path here? E.G. I could use the protocol crate version and try to use ExecutingMessage::abi_decode instead. Or I could update the SupervisorApiClient to use the interop crate ExecutingMessage.
There are also some misc TODOs if you have time to weigh in on would be appreciated.
TY!
Closes #98
WIP