-
Notifications
You must be signed in to change notification settings - Fork 470
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
Fix subscribe_logs to pass topics correctly #118
Conversation
Still seeing some strange behavior from geth in testing with this, am looking into it more. Any insight into how to correctly |
src/api/eth_subscribe.rs
Outdated
#[derive(Serialize)] | ||
struct SubscribeLogParameters { | ||
address: AddressVariants, | ||
topics: Option<Vec<H256>>, |
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.
This most likely should be Vec<Option<H256>>
(the Vec can have 0-4 elements). Passing None
tells the query to match all values on that particular position, omitting some positions should assume None
(alternatively we can always require 4-element tuple of Option<H256>
).
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'm doing some tests on it now, will have an updated PR in a couple mins. Seems like there may be larger geth compatibility issues with log filters
self | ||
} | ||
|
||
/// Topics | ||
pub fn topics(mut self, topic1: Option<Vec<H256>>, topic2: Option<Vec<H256>>, topic3: Option<Vec<H256>>, topic4: Option<Vec<H256>>) -> Self { | ||
self.filter.topics = Some(vec![topic1, topic2, topic3, topic4]); | ||
let mut topics = vec![topic1, topic2, topic3, topic4] |
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.
Somewhat of a compromise to reduce breakage in the interface here, trims off the trailing None
values from the topics as a shorter topic array will match regardless.
Think ideally this would be e.g. pub fn topics(mut self, topcs: Vec<Option<Vec<H256>>>) -> Self
with perhaps build()
returning Result<Filter, ...>
checking that topics.len() <= 4
, 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.
Makes sense as well. I hope that Rust will allow us to omit None
soon in the future by introducing optional parameters, though :)
… filtering examples
Did some experimentation with geth, added two new examples showing listening for event logs via Determined the following in serialized filters were causing issues with geth:
Restored Question about change to FilterBuilder api added as comment in review. |
self | ||
} | ||
|
||
/// Topics | ||
pub fn topics(mut self, topic1: Option<Vec<H256>>, topic2: Option<Vec<H256>>, topic3: Option<Vec<H256>>, topic4: Option<Vec<H256>>) -> Self { | ||
self.filter.topics = Some(vec![topic1, topic2, topic3, topic4]); | ||
let mut topics = vec![topic1, topic2, topic3, topic4] |
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.
Makes sense as well. I hope that Rust will allow us to omit None
soon in the future by introducing optional parameters, though :)
IMHO sounds like something that could be fixed in geth. It's a bit inconsistent that
Weird as well, makes client libraries way more complicated without any gains. Ideally all these should be in the "spec" :( |
Addresses #117 , reference https://github.com/ethereum/go-ethereum/wiki/RPC-PUB-SUB#logs for parameter types