QA: stratum_dupes: Test the parts of #18 that its tests do not reach

[alphaminetech]

This builds on #18 ("Bugfix: stratum: Keep the dupe index valid across a table cleanup") and does not replace it. The fix is #18's. Its commit 6ccfbe5 is included unchanged, through a merge of that pull request's head onto current master (08123fc), because the tests here need it. Nothing in this pull request changes src/datum_stratum_dupes.c; it adds four test commits to src/datum_stratum_dupes_tests.c (+268 lines). If #18 is merged first, those four commits are all that is left here.

Why

#18 was re-tested on current master by taking each part of it out again and running ./datum_gateway --test:

Part of #18 taken out	with #18's three tests	with the four tests added here
the cleanup at the top of datum_stratum_check_for_dupe, moved back into the insert (the bug restored)	281 checks fail	286 fail, then glibc aborts in malloc; with ASan: heap-use-after-free
the memset of the index in the full_wipe branch	all pass	1 fails, in datum_dupe_table_full_wipe_tests
the floor of 16 on the table size	all pass	1 fails, in datum_dupe_table_min_size_tests
the refusal of an insert into a full table	all pass	5 fail, in datum_dupe_table_full_insert_tests
if (!p) return false; after a refused insert	all pass	all pass

#18's tests prove its main change. The three smaller parts could be taken out again without a test noticing, and the first three commits here close that. The last row stays open on purpose: that line only runs after a refusal, and datum_stratum_check_for_dupe cannot get to a refusal as long as the cleanup at its top makes room, so no test reaches it without breaking something else first.

Two more things came out of the same runs:

• On master without the fix, datum_dupe_table_fill_tests passes in a normal build (10 of 10 runs, none of its checks fail). It catches the bug only when built with AddressSanitizer. The prune and cycle tests fail in any build (235 and 46 checks, the same in every run).
• None of Bugfix: stratum: Keep the dupe index valid across a table cleanup #18's tests looks at what datum_stratum_check_for_dupe returns after a prune. The fourth commit checks that, against a list of what was submitted.

The bug, and when a gateway gets to it

#18 describes it; briefly, from master's code. datum_stratum_add_new_dupe runs the cleanup after it has written the entry that fills the table, and then returns a pointer the cleanup has just invalidated: the prune sorts the array, the expand reallocates it. Two of the four insert sites in datum_stratum_check_for_dupe use that pointer: line 301, when the share is the first in its bucket, and lines 316 to 318, when it becomes the new first entry of a bucket. The other two discard it, and for those the index that the cleanup rebuilt is correct.

Each stratum thread has one table, created when the thread starts and not reset afterwards. It holds max_clients_per_thread * vardiff_target_shares_min * (share_stale_seconds / 60) * 16 entries, 32768 with the defaults, and every share that gets as far as the duplicate check and is not a duplicate adds one. At the vardiff target of 8 shares a minute the table is full after about 68 hours on a thread with a single client and after about half an hour on a thread with 128, and then again each time it has refilled. With 65536 buckets for 32768 entries most buckets are empty or hold one entry, so for evenly spread nonces the share that fills the table lands on one of the two faulty sites roughly four times in five (by a rough count 61 % first in its bucket, 18 % new first entry).

What happens then depends on which way the cleanup went:

• Prune, when at least 5 % of the entries are on jobs older than share_stale_seconds. This is the usual case: a table that took longer than that to fill is mostly entries on old jobs. The array is sorted in place and not moved. The bucket of the share that triggered the cleanup ends up pointing at the last slot of the array, which is now empty and past current_items, instead of at the share's entry. That share is no longer found, so submitting it again is not reported as a duplicate; later shares of that bucket are chained behind the empty slot; and the slot is handed out to a new entry when the table is full again. All of this stays inside the table's own memory. A chain that ends up linked to itself is possible from there on paper (a later share of the same bucket has to be given a slot that the bucket's chain already ends in). No run here produced one.
• Expand, when fewer than 5 % are, which means the table filled within about share_stale_seconds: with the defaults more than about 31000 accepted shares on one thread in two minutes, some 15 times what the table is dimensioned for. That takes a small configured table or clients far above the vardiff target. realloc may move the array, and then the same two sites go through a pointer into freed memory: line 318 (p->next = i) writes 8 bytes through it in the same call, and line 309 reads through the bucket on the next share in that bucket.

Commits

1. Merge of Bugfix: stratum: Keep the dupe index valid across a table cleanup #18's head (6ccfbe5, unchanged) onto master.
2. QA: stratum_dupes: Check that a full wipe leaves no bucket behind. Fills a 16 entry table with 12 shares in 6 buckets, calls datum_stratum_dupes_cleanup(dupes, true), and checks that no bucket is left, that the same shares are new again and that they are then found again. Nothing calls the full wipe today, so no other test reaches that branch.
3. QA: stratum_dupes: Check the size floor of the dupe table. Sizes the table from 0 and from -1 clients per thread and checks that it has at least 16 entries, takes 64 shares on a live job (it has to grow for that) and finds all 64 again.
4. QA: stratum_dupes: Check that a full table refuses the insert. Calls datum_stratum_add_new_dupe directly on a full table, once as the first entry of a bucket and once linked in after an entry, and checks that it returns NULL, counts nothing, links nothing and leaves the two slots behind the table alone. The function is declared in the test file, not added to the header.
5. QA: stratum_dupes: Check duplicate detection across cleanups. 20000 rounds on a table that starts with 16 entries, four jobs of which two have aged out, all shares in eight buckets, every nonce used by two shares that differ in ntime and extranonce. A share that was submitted before on a current job must be reported as a duplicate, a share that was never submitted must not be. With the fix the run goes through 28 expands and 51 prunes; that it sees both kinds is asserted.

Commits 2, 3 and 5 go through the three functions the header exports, so they can be carried over to another fix of this bug with little change (commit 3 expects the floor of 16; commits 2 and 5 also call datum_dupe_index_is_sound from #18's tests). Commit 4 is tied to #18's refusal.

Testing

One x86_64 host, Linux 6.8, Ubuntu 24.04 container without network, gcc 13.3.0, cmake 3.28.3. "Plain" is -Wall -Werror; "sanitizers" is the CI's -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all with -g, run with the CI's ASAN_OPTIONS and UBSAN_OPTIONS and with ASLR switched off for the process. The number of checks is counted by a counter on datum_test that exists only in the lab copy.

Tree	Build	./datum_gateway --test
master 08123fc	plain	exit 0; 14460 checks, 0 failed
master 08123fc	sanitizers	exit 0, no report
master + only #18's test file	plain	281 of 84868 checks fail: 235 bucket_points_at_a_live_entry (prune test), 46 the_new_share_became_an_entry (cycle test)
master + only #18's test file	sanitizers	heap-use-after-free, READ of size 8 at datum_stratum_dupes.c:309 in the fill test; freed by realloc at line 138, reached from 231, 274, 301
master + only this branch's test file	plain	288 checks fail (the 281, plus 1, 1 and 5 in the tests of commits 2 to 4), then malloc(): mismatching next->prev_size (unsorted) and abort in the test of commit 5
master + only this branch's test file, test of commit 5 alone	sanitizers	heap-use-after-free, WRITE of size 8 at datum_stratum_dupes.c:318; freed by realloc at line 138, reached from 241, 274, 316
master + #18	plain	exit 0; 86052 checks, 0 failed
master + #18	sanitizers	exit 0, no report
this branch	plain	exit 0; 99729 checks, 0 failed
this branch	sanitizers	exit 0, no report

Each of the four new tests was also run alone on master without the fix, 10 times plain and 5 times with the sanitizers, with the same result every time: 1, 1 and 5 checks fail for commits 2 to 4; the test of commit 5 aborts in glibc's malloc (plain) or stops at the use-after-free write above (sanitizers).

Further mutants of the fixed source, for completeness. With the cleanup taken out altogether, 149 checks fail (fill 49, cycle 1, min size 96, reference 3), plain and with the sanitizers alike, and the sanitizers report nothing: the refusal keeps every insert inside the array. Running the cleanup only when current_items > max_items gives the same 149 (plain build). With the cleanup and the refusal both taken out, ASan reports a heap-buffer-overflow at the write of the new entry in datum_stratum_add_new_dupe.

Not in the branch, lab only: the test of commit 5 at the default table size (32768 entries), over all 65536 buckets, for 1.5 million rounds. With the fix it passes plain and with the sanitizers (13 expands, 28 prunes). Without the fix the sanitizer build stops at the same use-after-free write, and the plain build runs to the end and fails missed_duplicates == 0.

Not done: no clang build, no musl, no macOS (the CI covers those), and no run of a gateway with miners attached; the section above on when a gateway gets to the bug is from reading the code.

What this does not change

• No file other than src/datum_stratum_dupes_tests.c beyond what Bugfix: stratum: Keep the dupe index valid across a table cleanup #18 itself changes, and no behaviour of the gateway.
• --test evaluates 13677 more checks and prints one more ERROR line: the refusal's own message, from the test of commit 4.

Notes on #18 from this review

• It merges onto current master without conflicts, and the two files it touches are unchanged on master since its base.
• The comment in datum_stratum_dupes_init says sixteen is the point below which the rounding stops the table growing. (n * 125) / 100 is larger than n for every n from 4 up, so that point is 4. The floor of 16 is fine as it is.
• The floor and the refusal look like safety nets, not fixes for something a gateway does today. With the checks in datum_conf.c, a table size of zero or less needs stratum.max_clients_per_thread at zero or below, and datum_sockets.c then assigns no client to a thread; the other way is an overflow of the int product from a very large stratum.vardiff_target_shares_min, which has no upper bound. This is from reading, not from a run.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

Static review identified no blocking issues in the inherited fix or added tests; tests were not run in this environment.

0 open findings

What changed in this PR

Extends regression coverage for stratum duplicate detection, building on the unchanged fix from #18.

Changes:

• Includes #18’s cleanup ordering and table safeguards.
• Adds tests for full wipes, minimum capacity, refused inserts, and duplicate detection across repeated cleanups.

File	Description
src/​datum_stratum_dupes.c	Includes #18’s fix to preserve valid indexes across cleanup.
src/​datum_stratum_dupes_tests.c	Adds regression coverage for cleanup behavior and safeguards.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.