-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat(cast): get logs #5042
feat(cast): get logs #5042
Conversation
It would be nice to have a |
A few other thoughts:
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supportive, a few nits
cli/src/cmd/cast/logs.rs
Outdated
} | ||
} | ||
|
||
async fn get_block_number<M: Middleware>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would like a one line here what this does: converting the BlockId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to convert_block_number
to prevent overlap with existing functions in the repo. Also added a bit of documentation.
fn build_filter( | ||
from_block: Option<BlockNumber>, | ||
to_block: Option<BlockNumber>, | ||
address: Option<Address>, | ||
sig_or_topic: Option<String>, | ||
topics_or_args: Vec<String>, | ||
) -> Result<Filter, eyre::Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda know what this does, but this is difficult to understand.
we need a few docs here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Split the logic into new functions and added some description, hopefully that makes it easier to follow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome!
this is now much easier to understand
Motivation
It is often very useful to watch for events or get logs using specific topics.
Solution
Added a command that works in two ways.
Example:
Equivalent command using just topics:
Logic is that the signature of the event is TOPIC_0 anyways. So the position argument still is kind of consistent. The arguments for when specifying a signature are the indexed arguments of the signature and are converted to topic hashes.
I am still up skilling with Rust so opening a draft PR to get feedback while I work on adding some tests.
Also realised I still need to handle empty arguments (any) when using a signature.