-
Notifications
You must be signed in to change notification settings - Fork 987
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
bump substreams client to sf.substreams.rpc.v2.Stream/Blocks #4556
Conversation
This breaks compatibility with previous substreams servers that only support 'sf.substreams.v1.Stream/Blocks'
chain/substreams/src/mapper.rs
Outdated
match message { | ||
Some(SubstreamsMessage::BlockUndoSignal(undo)) => { | ||
let valid_block = match undo.last_valid_block { | ||
None => return Err(SubstreamsError::MissingClockError), |
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 should prolly have a different error message?
chain/substreams/src/mapper.rs
Outdated
panic!("unknown step should not happen in the Firehose response") | ||
} | ||
} | ||
None => { |
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.
Could you add a comment describing when this should happen?
chain/substreams/src/mapper.rs
Outdated
} | ||
} | ||
None => { | ||
//warn!(&logger, "Got None on substream message"); |
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.
Is it worth adding an informative logging message? If not then this can be removed
|
||
tonic_build::configure() | ||
.protoc_arg("--experimental_allow_proto3_optional") | ||
.extern_path(".sf.substreams.v1", "crate::substreams") |
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 should be sf.substreams.rpc.v2 right?
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.
no, the sf.substreams.v1 still exists for the packages, manifests and such. Only, the service layer in it has been deprecated and moved to sf.substreams.rpc.v2 (the v2 to avoid confusion, even if we change namespace)
this extern_path directive informs the rpcv2 tonic handler about where to find the sf.substreams.v1 stuff.
* bump substreams client to sf.substreams.rpc.v2.Stream/Blocks This breaks compatibility with previous substreams servers that only support 'sf.substreams.v1.Stream/Blocks' * adjust substreams mapper code branches for clarity * add comments to clarify intent when ignoring useless substreams messages --------- Co-authored-by: Stéphane Duchesneau <[email protected]> Co-authored-by: Eduard Voiculescu <[email protected]>
Update substreams protocol to sf.substreams.rpc.v2.Stream/Blocks
This breaks compatibility with previous substreams servers that only support 'sf.substreams.v1.Stream/Blocks'