-
Notifications
You must be signed in to change notification settings - Fork 136
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 tproxy
initializer for Integration Tests
#1254
Add tproxy
initializer for Integration Tests
#1254
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1254 +/- ##
=======================================
Coverage 19.30% 19.30%
=======================================
Files 164 164
Lines 10849 10849
=======================================
Hits 2094 2094
Misses 8755 8755
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Bencher Report
Click to view all benchmark results
|
Bencher Report
Click to view all benchmark results
|
Bencher Report
🚨 1 Alert
Click to view all benchmark results
|
@@ -282,3 +286,86 @@ pub async fn start_template_provider(tp_port: u16) -> TemplateProvider { | |||
template_provider.generate_blocks(16); | |||
template_provider | |||
} | |||
|
|||
pub async fn start_sv2_translator(upstream: SocketAddr) -> SocketAddr { |
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.
let's try to keep starter APIs consistent, as highlighted in #1234
I'm not opinionated towards any specific approach
I just feel it's important to be careful in fragmenting patterns because that will eventually result in a poor UX for SRI contributors writing tests while working on this like e.g.: #1229 (which will likely happen sooner, rather than later)
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 the approach here is mostly what we want. No need for the end user to call get_available_address
but the role itself does that and returns it
listening_address | ||
} | ||
|
||
fn measure_hashrate(duration_secs: u64) -> f64 { |
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.
interesting approach of dynamically measuring local CPU hashrate
this strategy will save us A LOT of headache and it's quite a game changer in comparison to the static nature of MG tests
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.
Credit to @GitGab19
.expect("failed"); | ||
let listening_address = get_available_address(); | ||
let listening_port = listening_address.port(); | ||
let hashrate = measure_hashrate(3) as f32 / 100.0; |
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 divide by 100?
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 is done just part of the testing, I think this will change also until we figure out the best numbers here and in measure_hashrate
let mut hashes: u64 = 0; | ||
let duration = std::time::Duration::from_secs(duration_secs); | ||
|
||
let hash = |share: &mut [u8; 80]| -> Target { |
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 return Target
, it seems to be used nowhere?
let mut nonce = u64::from_le_bytes(nonce); | ||
nonce += 1; | ||
share[0..8].copy_from_slice(&nonce.to_le_bytes()); | ||
let hash = Sha256::digest(&share).to_vec(); |
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.
Bitcoin protocol uses sha256d, which means we actually calculate sha256 2x
every ASIC hashrate will tell you how many sha256d hashes were calculated in a second, not sha256
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.
here's an easy way to calculate sha256d
hashes with stratum_common
:
use stratum_common::bitcoin::hashes::{sha256d, Hash, HashEngine};
let input = 0;
let mut engine = sha256d::Hash::engine();
engine.input(&input);
let hashed = sha256d::Hash::from_engine(engine).into_inner();
let mut share = { | ||
let mut rng = thread_rng(); | ||
let mut arr = [0u8; 80]; | ||
rng.fill(&mut arr[..]); | ||
arr | ||
}; | ||
let start_time = std::time::Instant::now(); | ||
let mut hashes: u64 = 0; | ||
let duration = std::time::Duration::from_secs(duration_secs); |
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 fine with this approach but it seems a bit convoluted and confusing to read/understand
I'm not convinced we really need to replicate the bitcoin protocol so closely if all we want is to know the capacity of the system to calculate sha256d (attention to the d
, it's 2x sha256)
why not simply hash some simple input?
if you REALLY want to closely replicate what happens on bitcoin mining, I would suggest leveraging rust-bitcoin
you can initialize a Header
struct and then call header.block_hash()
to "mine" it.
that's how both mining-device
and sv1-mining-device
do it
@plebhash the |
I must point out that my comments about in it's current form, this I don't think we should introduce something with broken functionality to fix later, especially given that it's pretty straightforward to fix it now. please see #1254 (comment) |
after discussing with @GitGab19 (he did not write this code) we tracked down the root cause of the confusion around stratum/roles/translator/src/lib/downstream_sv1/diff_management.rs Lines 374 to 400 in 8c98647
we should not blindly copy-paste this code because it will result in a broken ITF I'm taking note of this on #1229 so it can be fixed as we modularize vardiff (which is what these unit tests are for) |
tproxy
initializertproxy
initializer for Integration Tests
3425383
to
ada6c75
Compare
ada6c75
to
1fea709
Compare
resolves #1250