Conversation
|
@ryanofsky is there a way to make methods like |
|
Rebased after #57. |
88588c3 to
5f77626
Compare
src/sv2/template_provider.cpp
Outdated
| }; | ||
|
|
||
| while (!m_flag_interrupt_sv2) { | ||
| uint256 prev_hash; |
There was a problem hiding this comment.
Hey @Sjors, I think there's an issue with the prev_hash variable in ThreadSv2ClientHandler.
Right now, prev_hash is declared at the top of the while loop but only gets assigned when block_template is null (inside that first if block). This works fine on the first iteration, but on subsequent iterations block_template won't be null anymore (since it gets updated at line 341), so the assignment never happens. This means the comparison if (new_prev_hash != prev_hash) ends up reading uninitialized memory.
I think the fix is pretty straightforward - just move prev_hash outside the loop and update it after each waitNext() call:
uint256 prev_hash; // Move this before the while loop
while (!m_flag_interrupt_sv2) {
if (!block_template) {
// ... create template ...
prev_hash = block_template->getBlockHeader().hashPrevBlock;
m_block_template_cache.insert({template_id, std::make_pair(prev_hash, block_template)});
}
std::shared_ptr<BlockTemplate> tmpl = block_template->waitNext(options);
if (tmpl) {
block_template = tmpl;
uint256 new_prev_hash{block_template->getBlockHeader().hashPrevBlock};
if (new_prev_hash != prev_hash) {
future_template = true;
// ...
}
prev_hash = new_prev_hash; // Add this line to update for next iteration
m_block_template_cache.insert({m_template_id, std::make_pair(new_prev_hash, block_template)});
}
}This also has the nice side effect of removing that getBlockHeader() call that was happening every iteration in the original code (the old old_prev_hash line), so it's actually more optimized.
There was a problem hiding this comment.
uint256 is a class, so it's default initialized to 0.
But you're right that it should be set outside the while loop.
Also cache the most recent template prev_hash between while iterations. This prevents repeating calls to getBlockHeader()
5f77626 to
debc0c1
Compare
xyephy
left a comment
There was a problem hiding this comment.
looks good, it can be merged now.
This was broken by stratum-mining#58 resulting in spurious "Tip changed" messages. Worse however is that that this caused m_last_block_time to update too often. When -sv2interval is set to 10 seconds or less, this results in PruneBlockTemplateCache never running.
|
Unfortunately this was incorrect, see #66. |
| { | ||
| LOCK(m_tp_mutex); | ||
| if (new_prev_hash != old_prev_hash) { | ||
| if (new_prev_hash != prev_hash) { |
There was a problem hiding this comment.
Previously old_prev_hash probably should have been m_best_prev_hash.
let me read through it and have a better understanding of it, sorry about my previous mistake. |
|
@xyephy I didn't see it either, it's not the best code. |
This prevents repeating calls to
getBlockHeader()