-
Notifications
You must be signed in to change notification settings - Fork 145
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
Add SnifferSV1
to ITF
#1580
base: main
Are you sure you want to change the base?
Add SnifferSV1
to ITF
#1580
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1580 +/- ##
==========================================
+ Coverage 19.31% 19.53% +0.21%
==========================================
Files 131 132 +1
Lines 9298 9328 +30
==========================================
+ Hits 1796 1822 +26
- Misses 7502 7506 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bd91e82
to
86049b4
Compare
..Currently if we exit a test while it is running the miner wont stop due to its background(async) jobs not getting the signal.
86049b4
to
b5b0b08
Compare
This module can be utilized in order to setup a connection either on the upstream or downstream side in a Stratum V1 enviornment.
`SnifferSV1` acts as a middleman between two SV1 roles. It forwards messages from one role to the other and vice versa. It also provides methods to wait for specific messages to be received from the downstream or upstream role.
b5b0b08
to
e4ee253
Compare
/// This struct can be used read and write messages to the other side of the connection. The | ||
/// channel is unidirectional, so you will need two instances of this struct to communicate in both | ||
/// directions. |
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 comment feels a bit confusing
with sender
I can send, with receiver
I can receive... therefore this is bidirectional?
features = ["with_buffer_pool"] |
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.
we should add sv1
feature here so it is added to docs
error!("Failed to send message to receiver_incoming: {:?}", e); | ||
break; // Exit the loop if sending fails | ||
tokio::select!( | ||
_ = tokio::signal::ctrl_c() => { }, |
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 this intentionally empty?
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.
sorry dumb question.. just refreshed my understanding of tokio::select
and now I understand that as soon as ctrl+c happens, all the other branches are cancelled
resolving the similar questions I asked in other parts of the PR
} | ||
|
||
/// Wait for a specific message to be received from the downstream role. | ||
pub async fn wait_for_message_from_downstream(&self, message: &str) { |
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 see this is diverging from the Sv2 Sniffer wait_for_message_type
pattern
here we have two distinct functions, while on Sv2 Sniffer we just have one and take MessageDirection
as input
is there some fundamental limitation on Sv1 Sniffer that forces this specific design?
if not, I think maybe it's better to have both following the same patterns, just to make the APIs a bit more intutive
.await | ||
.map_err(|_| SnifferError::DownstreamClosed)?; | ||
upstream_messages.add_message(msg.clone()).await; | ||
sleep(Duration::from_millis(100)).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.
why sleep here?
|
||
[lib] | ||
path = "lib/mod.rs" | ||
|
||
[features] | ||
default = [] |
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.
default = [] | |
default = ["sv1"] |
having to add --features sv1
to cargo test
feels a bit counter-intuitive... and forgetting to do it basically disables the tests that use Sv1 Sniffer
} | ||
|
||
/// Wait for a specific message to be received from the upstream role. | ||
pub async fn wait_for_message_from_upstream(&self, message: &[&str]) { |
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.
also this takes an array, while wait_for_message_from_downstream
doesn't
which is yet another reason I believe these should be just one function instead of two (to avoid fragmented patterns)
resolves #1564