Track Bitcoin target in proxy channel from SetNewPrevHash nbits#20
Open
rx18-eng wants to merge 1 commit into
Open
Track Bitcoin target in proxy channel from SetNewPrevHash nbits#20rx18-eng wants to merge 1 commit into
rx18-eng wants to merge 1 commit into
Conversation
Signed-off-by: rx18-eng <remopanda78@gmail.com>
Author
|
@Fi3 @Priceless-P would appreciate your thoughts on this ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Body
fixes the no-tp side of dmnd-pool/dmnd-client#111
so right now if youre running translator-style proxy with no TP connected, the non-jd branches in
channel_factoryjust hardcodebitcoin_target = [0; 32]. which meansOnNewShare::ShareMeetBitcoinTargetliterally cant fire for us coz no hash is ever <= 0. so a share that actually finds a block looks like a normal share, hits the 70/min rate limiter on the dmnd-client side, and if were unlucky it just gets dropped. real block lost.went with what @Fi3 suggested in the thread - just read nbits from the SetNewPrevHash the pool already sends us. we trust the pool for jobs+prev_hash anyway so its not a new trust assumption, no new deps, just a tiny
nbits_to_targethelper that wrapsbitcoin::Target::from_compactand gives back amining_sv2::Target.actual diff is small btw -
bitswas already being extracted right under the[0;32]line and passed tocheck_target. i just moved thebitcoin_targetline down a few lines so it can usebits, and dropped the.into()at the call site since the helper returnsTargetdirectly.fail-open semantics preserved (the constraint @Fi3 flagged - wrong target must never block a share):
so worst case wrong target = wrong classification, never a dropped share.
Tests i added (3 unit tests on the helper):
nbits_to_target_zero_yields_zero_targetso malformed pool msgs cant produce false-positive blocksnbits_to_target_matches_existing_test_helpercross-checks against thenbit_to_targetfixture thats already in this same test modulenbits_to_target_regtest_max_is_greater_than_mainnetquick sanity check on Target ordering since check_target doeshash <= bitcoin_targetthe actual integration (the
ShareMeetBitcoinTargetarm becoming reachable end to end) ill cover in the dmnd-client bridge PR - will link it back here once this merges. existing JD-path testtest_complete_mining_roundstill passes so didn't break that side.cc @jbesraa @Priceless-P @Fi3