Repository navigation
stratum: Name the client that found a block - #7
jasonsopko wants to merge 2 commits into
Conversation
| // Copy a client-supplied string for logging, replacing anything outside | ||
| // printable ASCII so a miner cannot put line breaks or terminal escapes | ||
| // into the log through its username or user agent. | ||
| static void datum_log_safe_copy(char *dst, size_t dstsz, const char *src) { |
There was a problem hiding this comment.
Moved to datum_utils as strncpy_printable, next to the other two strncpy_* filters, with a test. Dropped the copy of the user agent while I was there: strncpy_uachars already filters it at subscribe.
| DLOG_WARN("************************************************************************************************"); | ||
| DLOG_WARN("******** BLOCK FOUND - %s ********", new_notify_blockhash); | ||
| DLOG_WARN("************************************************************************************************"); | ||
| { |
There was a problem hiding this comment.
This is only one of two possible block-finding paths
There was a problem hiding this comment.
Right, the ABW reveal. Each pending candidate now keeps the client description (one pointer and the height per entry, freed with the candidate), and datum_protocol_abw_reveal logs it before queuing the block. One helper in datum_stratum.c builds the line for both paths.
2b54730 to
389628a
Compare
| DATUM_BLAKE2B_BLOCK_HEADER_SIZE); | ||
| pending->coinbase = coinbase; | ||
| pending->coinbase_size = coinbase_size; | ||
| pending->finder = (finder && finder[0]) ? strdup(finder) : NULL; |
There was a problem hiding this comment.
Rather not be doing memory allocation here
389628a to
583a8fb
Compare
|
Fair point. It's a fixed field on the pending entry now, and the reveal path copies it into a stack buffer. No allocation, no free. |
583a8fb to
e26bec6
Compare
|
Rebased on 69c61a9. The conflicts were all on master's side: the reveal path now submits the block before the ABW audit check, so the finder line sits in front of both and is logged either way; |
| bool subsidy_only; | ||
| bool pool_handled; | ||
| char finder[DATUM_ABW_FINDER_LEN]; | ||
| uint64_t height; |
There was a problem hiding this comment.
Now uint32_t, cast the way the header build in datum_protocol_abw_cache_candidate already does. It sits in the padding after the two bools, so an entry is 248 bytes against 240 on master.
| T_DATUM_ABW_TEMPLATE *block_template; | ||
| bool subsidy_only; | ||
| bool pool_handled; | ||
| char finder[DATUM_ABW_FINDER_LEN]; |
There was a problem hiding this comment.
Suggest adding a new refcounted string type so we don't have to copy the username for every share.
There was a problem hiding this comment.
Added T_DATUM_REFSTR to datum_utils in its own commit. Each client builds its description once in datum_stratum_client_finder, again only if its username changes, and each candidate takes a reference instead of copying the description into a 320-byte field. datum_abw_pending drops from 35.5 MiB to 15.5 MiB (15.0 on master), and the per-share cost from 225 ns to 13 ns in a scratch bench. The line now uses the username the miner sent rather than the modifier's pick, so a rotating modifier doesn't rebuild it.
| datum_stratum_describe_block_finder(finder, sizeof(finder), c, username_s, empty_work); | ||
| DLOG_WARN("Block %s at height %llu found by %s", new_notify_blockhash, (unsigned long long)job->height, finder); |
There was a problem hiding this comment.
Seems like this should be a common helper outside of stratum
There was a problem hiding this comment.
datum_log_block_finder in datum_utils writes the line for both paths now. Building the description stays in datum_stratum.c, since it reads the client's address, session and agent.
With it, a string several owners read and nobody changes is handed on as a reference instead of a copy. The count sits in front of the characters in one allocation and is atomic, so owners on different threads can drop theirs without a shared lock. datum_refstr_unref frees the string with the last reference. All three functions accept NULL.
The BLOCK FOUND banner prints only the hash. Telling a local ASIC apart from rented hashpower meant decoding the session id out of the header extranonce and matching it against thread assignment order. Log one more line with the height, the username sent in mining.submit, the client address, thread and slot, the session id (the bytes written into the header extranonce), the user agent, and whether the share was on empty work. datum_log_block_finder in datum_utils writes it on both paths a block takes: right after the banner in client_mining_submit, and in datum_protocol_abw_reveal once the pool discloses the XOR key. For the ABW path each pending candidate keeps a reference to the client's description and a 32-bit height. A client builds the description again only when its username changes, so a share costs a string compare and a reference instead of a copy. It uses the username the miner sent, not a username modifier's per-share pick, which says who gets credit rather than which machine found the block. The username is filtered through the new strncpy_printable first; the user agent already went through strncpy_uachars at subscribe. No behavior change.
e26bec6 to
695c78a
Compare
2 participants
What
One more WARN line naming the client behind a found block: height, the username sent in
mining.submit, client address, thread and slot, session id, user agent, and whether the share was on empty work.datum_log_block_finderwrites it on both paths a block takes: after the BLOCK FOUND banner inclient_mining_submit, and indatum_protocol_abw_revealwhen the pool discloses the XOR key. The first commit addsT_DATUM_REFSTR, a reference-counted string, for the ABW path.Why
The banner prints only the hash. I had to decode the session id out of the header's extranonce and work through thread assignment order to tell my own ASIC apart from rented hashpower. The session id in the new line is what the gateway writes into the extranonce, so an on-chain block matches a log line directly.
Under ABW the share is not recognizable as a block when it arrives, so each pending candidate keeps a reference to the client's description. A client builds its description again only when its username changes. It uses the username the miner sent, not the one a username modifier picks per share: the modified name says who gets credit, not which machine found the block. The username goes through
strncpy_printablefirst; the user agent already went throughstrncpy_uacharsat subscribe.How I tested
Local builds on 69c61a9 (v0.5.0beta): gcc and clang with
-Wall -Werror,ENABLE_API=OFF,DATUM_API_FOR_UMBREL, gcc ASan+UBSan and clang ASan+UBSan build clean and pass--helpand--test. New tests cover the string type, the client's description and the reference counts through the ABW cache. Breaking any of the reference releases, or the hand-off in the reveal path, fails--testunder ASan. The ABW reveal test now logs the line. TSan is clean with four threads taking and dropping a million references each on one string, and the count ends at one. Per ABW share, in a scratch bench on a Release build: 13 ns with an unchanged username, 232 ns when it changes every share, 225 ns at the previous head. The local path line only fires on a block, so it was not run.Risk and rollback
Log output only, no behavior change. The pending table grows by 8 bytes per entry, 15.0 to 15.5 MiB (35.5 MiB at the previous head), and each client slot by 208 bytes. Worst case is about 22 MiB of descriptions, one for each of the 65536 candidates, and only if the username or connection changes before every share. Candidates on the full coinbase already hold a coinbase copy larger than that (540 to 1557 bytes for CONVOY's 215 blocks from 974108 to 977107), and the previous head spent 20.5 MiB on every pooled gateway. A smaller cap would cut long usernames or user agents short more often, so it stays at 320 bytes. The line puts the finder's username and address in the log at WARN. Revert the two commits.