-
Notifications
You must be signed in to change notification settings - Fork 2
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: implement an initial mempool.space websocket client, and library for block-events #3
Conversation
836c8e6
to
3191aa3
Compare
23d823e
to
4db855e
Compare
8c0dd96
to
7bf75eb
Compare
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
ce and basic apis Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
Signed-off-by: Leonardo Lima <oleonardolima@users.noreply.github.com>
7bf75eb
to
10e1e11
Compare
e96ea41
to
2147092
Compare
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've suggested some changes. Most of which I've suggested to you on our call :)
src/lib.rs
Outdated
network: &Network, | ||
data: &MempoolSpaceWebSocketRequestData, | ||
) -> anyhow::Result<impl Stream<Item = BlockEvent>> { | ||
env_logger::init(); |
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.
in general, you shouldn't env_logger::init
inside a library. This should be done in the fn main
.
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.
Thanks! I've moved it to the main function
src/mempool_space/mod.rs
Outdated
) -> anyhow::Result<impl Stream<Item = BlockEvent>> { | ||
let url: Url = url::Url::parse(&build_websocket_address(network)).unwrap(); | ||
websocket::connect_and_publish_message(url, message).await | ||
} |
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 think this function can be removed. Let the caller pass in the mempool.space .../api
to use to find the websocket. For example, What if I want to use my own self-hosted mempool.space?
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.
Yeah, agreed. I've removed this fn as updated the other fn to subscribe_to_blocks
src/mempool_space/mod.rs
Outdated
message | ||
} | ||
|
||
fn build_websocket_address(network: &Network) -> String { |
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 could be renamed and made public so you could get the string and pass it in as I suggested above.
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.
Done, this is being used by caller now :)
src/mempool_space/websocket.rs
Outdated
let (mut websocket_stream, websocket_response) = | ||
connect_async_tls_with_config(&url, None, None) | ||
.await | ||
.unwrap_or_else(|_| panic!("failed to connect with url: {}", &url)); |
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 can just be await?
I think . It's a bad idea to panic on an IO failure!
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.
Yeah, updated
src/mempool_space/mod.rs
Outdated
websocket::connect_and_publish_message(url, message).await | ||
} | ||
|
||
pub fn build_websocket_request_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.
This should be a private function for now.
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.
@LLFourn Good, I've updated it to be private and moved this one to websocket.rs as it's only used by websocket, wdyt ?
src/mempool_space/websocket.rs
Outdated
use tokio_tungstenite::tungstenite::protocol::Message; | ||
use url::Url; | ||
|
||
pub async fn connect_and_publish_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.
I think this function should just be subscribe_to_blocks
and take the mempool.space ../api
endpoint. Later it will take an optional height and blockhash checkpoint.
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.
@LLFourn Great, I've renamed it and updated it to expect only the URL for now 😁
…rary for block-events from #3
This PR and its commits have been squashed and merged into #10, to make it easier for review and code checking at this stage. |
…point and reorg handling) (#10) * feature: implement an initial mempool.space websocket client, and library for block-events from #3 * feature: add unit and integration tests to block events from #8 * feat+test: add initial http client and tests * wip(feat): add initial approach for subscribing to blocks from a starting height * wip(feat): fix fn and use a tuple as return type for prev and new block streams * wip(refactor): initial updates for fns refactor and architecture change * wip(feat+refactor): add fn to process candidate BlockHeaders, handle reorg and yield BlockEvent * wip(refactor+docs): extract fns to cache struct, and add documentation for fns, enums and structs * wip(fixes+tests): fixes, improve error handling and add new integration tests * wip(fix+test): fix common ancestor and fork branch bug, add new reorg tests * fix: disconnected and connected block events emitting order for reorg events * chore: simplify cli usage and output * chore: update docs and readme examples, use u32 instead of u64 * fix: remaining cargo.toml conflicts from other branches * feat: add full block stream api, better error handling - use TryStream instead of stream, returning result instead - use `?` operator instead of unwraping and panicking - add new mempool.space endpoints for http client - add features for wss:// and https:// usage - add features for api versioning based on mempool.space backend * refactor: do not use features expect two base_url instead, and improve error handling - refactor and update http client, url usage and methods - refactor and add creation methods for cache struct, use specific method to build initial one - add `get_block_header` method in http client, and refactor lib to use `bitcoin::BlockHeader` instead of custom mempool.space `BlockExtended` struct - refactor lib and websocket to improve error handling, update some `unwrap()` usage and match errors for messages in `WebSocketStream` - remove duplicated/unused deps, and use only necessary features * chore: add and update CHANGELOG.md file * fix(test): docs and integration tests - bump mempool/backend docker container version to v2.4.1 - update api+websocket client to handle genesis blocks without prev_blockhash - update and fix integration tests to new fns and structure
Description
This PR focus on solving the issues:
Refactoring the previous implementation for the competency test to become an initial library that produces a stream of
BlockEvent
, focusing only on new block data and deprecating the previoustrack-address
and other features implemented.This PR although moving some files in library structure, it's still maintained and makes it possible to use it through a CLI interface where the block events are printed to stdout.
As a result, now we have the module for mempool.space wrt to the WebSocket client, message building, and parsing (both for request and response).
Notes to the reviewers
There is no need to go through each commit, the PR could be reviewed as a whole to connect the parts and get an overview related to the logic and implementation.
There is no unit or integration test at the moment, as I'm still researching the best way to build some sort of running process with electrsd and bitcoind that replicates the mempool.space behaviour, and it's being addressed by the #6 . If you have any suggestion feel free to reach me on BDK discord 😁
It has been tested locally with nigiri for running the bitcoind and also running mempool.space through docker, connected to the local regtest RPC.
If you would like to test it, please take a look at the README file for the commands and guidelines.
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committingNew Features:
CHANGELOG.md