Skip to content

stratum: Blake payout path + rebase of open hardening onto #10 - #15

Open
innerhat-dev wants to merge 9 commits into
CONVOYMining:masterfrom
Blockvase:blake-payout-hardening
Open

innerhat-dev wants to merge 9 commits into
CONVOYMining:masterfrom
Blockvase:blake-payout-hardening

Conversation

@innerhat-dev

Copy link
Copy Markdown

What

On BLAKE2b the miner never sees the coinbase. Stock CONVOY DATUM still picked a SHA-sized class (usually 2 / 755 B, class 0 on some Antminers). A find then truncated the Prime split and paid the remainder to the pool address.

After the coinbaser is ready, every miner now gets class 4 (COINBASE_TYPE_YUGE). While pooled and the coinbaser is late, the job is subsidy-only (DATUM_COINBASE_ID_EMPTY, N job prefix, no merkle) instead of class 0 paired with a full template. Solo stays class 0 until ready.

if (new_block) return DATUM_COINBASE_ID_EMPTY;
if (!ready) return datum_protocol_is_active() ? DATUM_COINBASE_ID_EMPTY : 0;
return COINBASE_TYPE_YUGE;

Job id, commitment (txn_count 1 vs full), submit, assembleBlockAndSubmit(..., empty_work, ...), and the DATUM 0x02 blob all use that same index / subsidy_only flag. YUGE also enforces the template sigop budget and BLAKE2b header weight so a large split cannot blow the block.

#10 already subtracted a per-output sigop cost from the GBT leftover, but the parser still used the OCEAN first-byte guess (0x76 / P2PKH -> 4, else 0). A bare CHECKSIG / CHECKMULTISIG script is consensus-valid in a coinbase (submitblock is not mempool policy) and would have been charged 0. This stack sets available_coinbase_outputs[].sigops and the pool-output charge from datum_script_legacy_sigop_cost: Bitcoin GetLegacySigOpCount on the scriptPubKey, times 4. Pushes are skipped. A truncated push stops the walk. P2PKH stays 4, witness / P2SH stay 0, bare P2PK is 4, bare multisig is 80.

The coinbaser fetch takes the mutex before send, clears a stale reply, and waits for this job's coinbase_value. Without that, localhost races burn the 5 s timeout and publish empty/pool-only work. If the 0x11 reply is 32 bytes longer than the blob, those bytes are the request prevhash and the wait requires both. Stock OCEAN/CONVOY replies stay value + blob; the extra bytes are ignored by old parsers because they are outside blob_len. Requiring the trailer would break those servers.

Credit

Payout path (do not drop):

PR Author What landed
#10 @iohzrd YUGE for every miner once full_coinbase_ready; sigops budget + BLAKE2b weight (still the first-byte P2PKH guess). CONVOY-shaped port of innerhat-dev#17 (@jasonsopko).
#13 @AwokenLazarus Pooled + coinbaser late -> DATUM_COINBASE_ID_EMPTY, not class 0 + full template.
#9 @jasonsopko Lock before coinbaser send; wait for this job's value.

Hardening (ported onto #10):

PR Author What landed
#1 @f4u57ox hex_to_bin_exact on submit; cache blake2b_prevblock_hidden per job.
#2 + #11 @jasonsopko Bound 0x11 / job-validation replies (0xF4, datum_protocol_stxlist_reply_fits).
#3 @jasonsopko Knots duplicate on submitblock is success.
#4 @jasonsopko Log the node's RPC error body (no FAILONERROR). Keep a parsed body that already passed the result/error gate even if HTTP is >= 400.
#6 @jasonsopko Logger queue + file before the writer thread.
#7 @jasonsopko Name the client that found a block.
#8 @luke-jr Parser leak + payout-value overflow only.
#12 @jasonsopko roundDownToPowerOfTwo_64(0) - __builtin_clzll(0) is UB.
#14 @jasonsopko Explicit datum_header_pk / datum_header_upk (replaces #5).

Not an open CONVOY PR (this stack):

What landed
Sigop cost datum_script_legacy_sigop_cost replaces #10's first-byte 0x76 guess. Same GBT leftover math, correct cost for bare CHECKSIG / CHECKMULTISIG.
Coinbaser prevhash Optional 32-byte trailer on 0x11. #9 still waits for value; a late reply with the same sats and a different parent is ignored when the trailer is present.

Not in this PR

  • Misc 20260904 fixes #8's YUGE / fingerprint commit. Wrong shape: it sets the default class and blanks UA fingerprinting, including for SHA. stratum: Serve every miner the largest coinbase class on BLAKE2b work #10 already does the Blake fix and waits for the coinbaser.
  • Empty work on a new tip still pays the pool if it actually finds. Kept on purpose: EMPTY switches the parent immediately so miners catch the new tip instead of hashing a stale parent (orphan) or idling until the coinbaser. A find in that window is subsidy-only (no merkle, no fee txs, no miner split). Holding the notify until this-prevhash coinbaser, then type 4, would close the unfair-valid-block case and give up that catch-up. Carrying the last payout plan is a separate design question (see innerhat-dev#16). Using full_coinbase_ready on new_block is the same trap unless that flag is proven to be this prevhash. Unconditional EMPTY on a new tip is the rule this PR keeps.
  • Closed #5 (superseded by protocol: Make T_DATUM_PROTOCOL_HEADER serialisation explicit #14).
  • Listen-port / site config. Defaults unchanged.

Known leftovers (intentional)

  • #13 keys off datum_protocol_is_active() (session state 3, plus ABW if the pool requires it). Handshake states 1-2 are still class 0: full template, local mining.pool_address. That is dual-mode solo. Widening EMPTY to handshake or "thread running" would give subsidy-only work (no merkle) while the pool is down or still connecting. With pooled_mining_only the ASIC is not accepted in that window, so there is no work and no find. Leaving the dual-mode policy alone; not a #13 change in this PR.
  • Stock DATUM servers still reply value + blob only. Against those, #9's value wait is unchanged. The same-sats / different-parent race is closed only when the server sends the trailer.
  • A coinbaser that overflows the subsidy cap, or an output that misses the size / sigop leftover, is skipped and those sats go to the pool (break / skip, not hard-fail). Hard-fail would send the whole remainder to the pool; keep-prefix still pays the outputs that fit. Prime already refuses total > value and only emits standard address scripts (live splits are P2WPKH, 0 sigops, two outputs in 16 KB YUGE). Assembler defense, not a Prime payout case. Leaving keep-prefix.

Test

  • ./build/datum_gateway --test exits 0. Fixture ERROR/WARN lines (bad config version, unsigned migration, example-pool hosts, oversize stxlist) are expected.
  • datum_blake2b_coinbase_selection_tests covers solo class 0, handshake class 0, pooled-and-late EMPTY, ready YUGE, and new_block EMPTY while ready.
  • datum_script_legacy_sigop_cost_tests covers P2PKH (4), P2WPKH / P2SH / P2TR (0), bare P2PK (4), bare multisig (80), CHECKSIG inside a push (0), a script that starts OP_DUP but has two CHECKSIGs (8), and a truncated push (0).
  • datum_coinbaser_bare_p2pk_sigops_tests: parse of a bare P2PK sets sigops == 4; a 0 leftover skips it; a leftover of 4 packs it. The old first-byte guess would have packed the 0-budget case.
  • datum_protocol_coinbaser_prevhash_tests: stock 0x11 (no trailer) matches any parent; a 32-byte trailer matches only that prevhash.
  • Live on a BLAKE2b DATUM pool: handshake generation v3, 2-way coinbaser in a few ms, type-4 work after ready, shares accepted (reason=0). Live payouts are P2WPKH, so the new counter matches the old guess on that path.
  • A network block is not required to validate the payout path. An accepted share is the same job, class, job-id nibble, Prime 0x02 blob, and reconstruct-on-submit code. A find only adds Knots submitblock of that exact coinbase + template.

… miner on BLAKE2b work the largest coinbase class.
…and count the coinbase's static bytes as 124 instead of 119.

@jasonsopko jasonsopko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on 6341bf9. The ports are faithful and everything I could exercise passes. One thing needs a change before this meets the CONVOY server: the way the new prevhash trailer is detected.

The ports. For each of the twelve PRs folded in here I took every line the original PR added and looked for it in this tree. All of them are present, or replaced by something equivalent: #3 differs only in comments, #14 only in whitespace, #9's wait loop is now the new reply-matching helper, and #11's inner length check is covered by the up-front 3 + 2 * req_count > len check because the loop starts at offset 3. #8 contributes only its two bug fixes, as the description says. --test exits 0.

Sigop counting. I compared the new script walk with Bitcoin Core's legacy sigop rules on 600,000 random scripts, weighted toward the cases that matter (every push form, truncated pushes, the four CHECKSIG opcodes), plus the standard output templates. They agreed every time. Knots reports and enforces a sigop limit of 80,000 under the reduced-data rules, so the budget this code works from is the consensus one.

Coinbase classes. The 396-configuration parity check and the weight check from the #10 review both still pass on this head. On a regtest node with the fork at height 1 and the gateway solo, a client announcing itself as an Antminer S19 gets, after a new block: the empty job, class 0, then class 4 within the same second. A build from before this PR gives that client class 2, the 755-byte class that truncates the payout split. On both builds the first job after gateway start stays class 0 until the next block; that is not new.

Fuzzing. My libFuzzer harnesses on this head: 5.3 million runs against the command-5 path, which includes the 0x11 reply and the job-validation replies, and 20.5 million Stratum lines. Nothing crashed.

The trailer. The parser decides that a reply carries a prevhash by length alone: if there are 32 or more bytes after the blob, the first 32 are taken as the prevhash. Fed by hand:

stock value+blob                 has_prevhash=0 matches_parent=1
stock + 31 pad bytes             has_prevhash=0 matches_parent=1
stock + 32 pad bytes (random)    has_prevhash=1 matches_parent=0 matches_other=0
stock + 100 pad bytes            has_prevhash=1 matches_parent=0 matches_other=0
trailer == parent                has_prevhash=1 matches_parent=1 matches_other=0

Does the CONVOY server pad its 0x11 reply? The gateway pads every frame it sends, including this request, and the old handler already accepted replies longer than the blob. If the server pads by 32 bytes or more, the gateway reads the padding as the wrong prevhash, waits out the 5 seconds on every coinbaser fetch, and every gateway running this code hands the pool subsidy-only work. Prime's echo (c_datum_prime 72e3c5b) is exact and unpadded, so the live test in the description would not show it.

A 4-byte magic in front of the 32 bytes closes it: take the trailer only when the length allows 36 bytes and the magic matches, have Prime write the magic, and a padding server collides once in 2^32 instead of every time. With that, or with someone who has seen the CONVOY server's reply bytes confirming it never pads, I would ACK the whole thing.

nit: datum_stratum.c:977 warns under -Wformat-truncation on gcc 13. That line came from my #7; harmless, finder could be 512.

nit: Luke asked on #3 for the refactor to stay in its own commit. One commit for eleven PRs loses that, and the authorship. Co-authored-by trailers would carry the second.

innerhat-dev and others added 2 commits September 8, 2026 14:27
…er work subsidy-only.

Blake jobs use YUGE after the coinbaser and EMPTY while pooled and late, plus the remaining open-PR bounds/RPC/logger fixes.

Co-authored-by: iohzrd <3422336+iohzrd@users.noreply.github.com>
Co-authored-by: Jason Sopko <22288016+jasonsopko@users.noreply.github.com>
Co-authored-by: AwokenLazarus <262377448+AwokenLazarus@users.noreply.github.com>
Co-authored-by: f4u57ox <72417751+f4u57ox@users.noreply.github.com>
Co-authored-by: Luke Dashjr <1095675+luke-jr@users.noreply.github.com>
…vhash.

The first-byte P2PKH guess undercounted bare CHECKSIG. The optional trailer
closes the same-sats / different-parent wait without breaking stock servers.
@innerhat-dev
innerhat-dev force-pushed the blake-payout-hardening branch from 6341bf9 to 39400ea Compare September 8, 2026 19:27
@innerhat-dev

Copy link
Copy Markdown
Author

Happy to pull out #3 if that makes sense, was going for completeness and appreciate the feedback. I currently have this running against a c_datum_prime pool server.

For the trailer, if the CONVOY server can pad by 32+ I’ll add the 4-byte magic and have c_datum_prime write it. Not sure if I’m missing something there, happy to do whichever you want for the ACK.

@jasonsopko

Copy link
Copy Markdown

Add the magic. Nobody outside CONVOY has seen the server's 0x11 reply bytes, and I have not run pooled against it, so whether it pads by 32 or more is unknown here. What the code says pulls both ways. Every message this gateway sends ends in a random run of padding, 1 to 80 bytes on the coinbaser request itself. On the other side, every server message Luke added this year (bulk ack, the five ABW messages, the parent fetch, the migration request) is checked for an exact length ending in 0xFE, so the server sends those unpadded or ABW would not work. The 0x11 reply is 2024 code and the old handler took anything longer than the blob, so it could go either way.

The magic makes the question moot: a padding server collides once in 2^32 instead of every time, a non-padding server sees no difference, and Prime writing four more bytes costs nothing. One detail worth using: the gateway's own padding is a single byte value repeated (memset with one rand()), so if the server pads the same way, a magic whose four bytes are not all equal can never match it.

Parser rule: take the trailer only when len >= 12 + x + 36 and the four bytes at data[12 + x] equal the magic. Anything else, whatever the tail length, is has_prevhash = false. Once that is pushed I will rerun the reply harness (stock, 31/32/100 pad, magic + parent, magic + other, wrong magic + 32 bytes), the solo regtest run, and ACK.

Leave #3 in. The trailers on 4593826 cover what I was after on authorship. Whether it lands as one commit, or #3 first and a rebase, is Luke's call when he gets to this.

A padded CONVOY 0x11 reply then collides once in 2^32 instead of every time.
@innerhat-dev

Copy link
Copy Markdown
Author

Added the 4-byte magic and have c_datum_prime writing it. Trailer only counts if the tail is at least 36 bytes and those four bytes at the blob edge match, anything else is no prevhash as you described. Leaving #3 in. Thank you for the follow up.

@f4u57x

f4u57x commented Sep 9, 2026

Copy link
Copy Markdown
Reviewed `6cc67363`. The latest trailer-magic change addresses the padding concern. I found three additional issues and prepared fixes in [Blockvase/datum_gateway#1](https://github.com/Blockvase/datum_gateway/pull/1), each in a separate commit.

### 1. Subsidy-only work conflicts with quick difficulty changes

While the pool connection is active and the coinbaser is pending, coinbase selection returns `DATUM_COINBASE_ID_EMPTY` (`ff`). If quickdiff triggers in that window, `send_mining_notify()` advertises `Q...ff`. However, the submission parser only recognizes subsidy-only work through the `N` prefix, so it rejects the advertised job as `unknown-work` before checking PoW.

I reproduced this through the actual notify → submit path. The fix recognizes the empty coinbase index alongside the quickdiff flag, preserving both subsidy-only reconstruction and quickdiff target accounting. The regression test covers `N...ff`, `Q...ff`, and ready `Q...04` jobs. Its deliberately invalid proof reaches PoW validation instead of failing job lookup.

### 2. The strict hex decoder reads past some truncated inputs

`hex_to_bin_exact()` reads both characters before checking whether the first is invalid. With an empty or short even-length string, the first character can be the terminator and the second read goes past the allocation. ASan reproduces this.

Current production Stratum callers check exact lengths first, so this is not a demonstrated remotely reachable submission crash. Nevertheless, the helper should safely reject truncated strings itself. The fix checks the first character before reading the second, with exact-sized allocation tests for every truncated length. This helper came from my #1; I have corrected it there too.

### 3. Logger thread creation failure silently disables logging

Initialization sets `datum_logger_initialized = true` before an unchecked `pthread_create()`. If thread creation fails, initialization reports success and subsequent messages queue without a consumer.

The fix checks thread creation, publishes successful initialization only afterward, and cleans up allocated buffers and the log file on failure. Console fallback remains available. A failure-injection test exercises `EAGAIN`, verifies cleanup and fallback output, and checks that successful initialization has usable queues immediately.

### Validation

Normal and ASan/UBSan builds and full `--test` suites pass on macOS with `ENABLE_API=OFF`.

These are local regression tests; I have not run the fixes against live miners or the CONVOY pool server.

@jasonsopko jasonsopko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK 6cc6736 for the trailer, with one thing to take before merge: the Q...ff fix in Blockvase#1.

The magic. The parser does what was asked: a trailer counts only when the tail is at least 36 bytes and the four bytes at the blob edge are CBPH, and everything else is no prevhash. My reply harness on this head, same rows as before plus the new ones:

stock, +31 pad, +32 pad, +100 pad          has_prevhash=0  matches_parent=1
bare 32-byte parent (the old trailer)       has_prevhash=0  matches_parent=1
100 x 0x43 pad (a repeated 'C' run)         has_prevhash=0  matches_parent=1
wrong magic + parent, magic + 31 bytes      has_prevhash=0  matches_parent=1
magic + parent, also with 8 pad bytes after has_prevhash=1  matches_parent=1  matches_other=0
magic + other                               has_prevhash=1  matches_parent=0

A stale reply for the other parent no longer satisfies the wait, and a value mismatch with a matching trailer does not either. Prime 5463332 writes the same four bytes ahead of the hash. --test exits 0, and the solo regtest run is unchanged from 6341bf9: an S19 client gets N ff, 0, 4 after a new block.

f4u57ox's three. I checked the Q...ff one against this head rather than take it on trust. It is real and it is new here: datum_stratum_coinbase_index now returns the empty class while the coinbaser is late on a pooled connection, so a quick-difficulty notify in that window goes out as Q<job>ff, and client_mining_submit only turns on empty_work for the N prefix. The share is refused as unknown-work before proof-of-work is looked at. On master the empty class is only picked for the new-block notify, which is never a quickdiff one, so this came in with the late-coinbaser change. His regression test fails on this head at datum_stratum_tests.c:370 and :371 without his stratum commit and passes with it; the fix is the five lines you would expect. The hex read past a short string is one byte and every current caller checks the length first, so it is hardening, and the logger startup one is a return value nobody checked. His branch builds clean here apart from an unchecked fread in the new logger test, and its --test exits 0.

nit: datum_stratum.c:977 still warns under -Wformat-truncation; that line is mine from #7.

@innerhat-dev

Copy link
Copy Markdown
Author

Took all three. Had not hit the Q...ff case here because the coinbaser is usually back before vardiff can fire. Thank you @f4u57ox @jasonsopko

@jasonsopko jasonsopko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK ed89090. This head is 6cc6736 plus the three Blockvase#1 commits, patch-identical (git range-diff shows all three as =), so it is the tree I checked last night with the condition now met.

Reran on ed89090: Release build clean apart from the unchecked fread warning at datum_logger.c:548, --test exits 0, and an ASan/UBSan build of the same head passes --test with no reports. The 0x11 reply harness output is byte for byte what I posted for 6cc6736, and the solo regtest run still gives an S19 client N ff, 0, 4 after a new block.

@luke-jr

luke-jr commented Sep 26, 2026

Copy link
Copy Markdown

Not in this PR: Misc 20260904 fixes #8's YUGE / fingerprint commit. Wrong shape: it sets the default class and blanks UA fingerprinting, including for SHA. stratum: Serve every miner the largest coinbase class on BLAKE2b work #10 already does the Blake fix and waits for the coinbaser.

SHA is dead. Neither Bitcoin nor DATUM support it anymore.

Fingerprinting is not dead: there is empirical evidence miners in the wild do time rolling incorrectly, and we should eventually support fingerprinting them to disable time rolling.

Dynamically sizing the generation transaction is also not dead: Currently, we require the node to leave a "reserve weight" available for us, but ideally we would allow the node to make the block larger and adapt our generation as needed. Given the current long maturity of generated coins, however, we should probably also let the pool require full generation (and force the node to make smaller blocks as needed).

Let's not delete code we're going to want in the future. #8 is merged now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants