diff --git a/src/impl/dash/local_mint_ledger.hpp b/src/impl/dash/local_mint_ledger.hpp index b4527b3ee..cc7e9f814 100644 --- a/src/impl/dash/local_mint_ledger.hpp +++ b/src/impl/dash/local_mint_ledger.hpp @@ -50,6 +50,28 @@ enum class MintVerdict { /// height and get_nth_parent_via_skip() is the O(log n) Bitcoin-Core skip-list /// walk — the same two primitives the producer walks already use. The caller /// holds the tracker read guard. +/// +/// ── LOCK-SAFETY NOTE (do not delete — #828 GP-fault crash class) ──────────── +/// Both primitives are NON-CONST and mutate lazily-built caches, yet the only +/// caller (main_dash.cpp's best-share-changed binding) holds a SHARED tracker +/// lock, so two of them can run concurrently. That is safe TODAY, and ONLY +/// because each cache carries its own internal LEAF mutex: +/// +/// * get_acc_height() -> TrackerView::get_delta() populates m_deltas / +/// m_reverse_deltas / m_delta_refs / m_reverse_delta_refs, all serialized +/// by TrackerView::m_cache_mutex (src/sharechain/tracker_view.hpp:118-124); +/// * get_nth_parent_via_skip() -> DistanceSkipList builds m_skips lazily, +/// serialized by DistanceSkipList::m_skips_mutex (src/sharechain/ +/// skip_list.hpp:76-80), documented there as a leaf — taken around each +/// individual map operation, with m_previous_fn invoked outside it, so it +/// never nests with the tracker lock. +/// +/// Strip either leaf mutex — e.g. a "faster" unsynchronized height/skip cache +/// refactor — and this call becomes one thread rehashing an unordered_map +/// while another iterates it: the exact freed-memory dereference of the #828 +/// GP-fault class. If you touch those caches, either keep the leaf mutexes or +/// promote this call site to the EXCLUSIVE tracker lock. Do NOT reason "the +/// tracker lock protects it" — the tracker lock is SHARED here, by design. template inline MintVerdict classify_local_mint(ChainT& chain, const uint256& best_share, diff --git a/src/impl/dash/node.cpp b/src/impl/dash/node.cpp index 453903e1f..4388eddf5 100644 --- a/src/impl/dash/node.cpp +++ b/src/impl/dash/node.cpp @@ -3,6 +3,7 @@ #include "share.hpp" #include "mint_runloop.hpp" // dash::mint::elect_best_share (election policy SSOT) #include "known_txs_retention.hpp" // dash::retain_template_txs / all_txs_backable (F1/F3) +#include "think_gate.hpp" // dash::think:: think-slot protocol + clean-cycle tip decision (#854) #include @@ -488,16 +489,37 @@ void NodeImpl::apply_min_protocol_ratchet() void NodeImpl::run_think() { + // THREAD-CONFINEMENT (enforced, not assumed — register_template_txs + // precedent): the think-slot flags (m_think_running / m_rethink_pending) + // are IO-thread-only. Every caller is on the single ioc.run() thread: the + // mint hook (add_local_share), the reception path (add_verified_shares, + // which posts itself back to m_context after the verify pool), the 2 s + // advert drain, and both cycles' IO-phase epilogues. The compute thread + // NEVER touches them — it only runs the tracker under the exclusive lock. + // A compute-thread caller would break the "release then re-post" handshake + // below (it would re-enter the slot it still owns), so trip loudly in + // debug builds rather than let it be discovered in production. + assert(!is_compute_thread() && + "run_think must not be called from the compute thread — the think " + "slot (m_think_running/m_rethink_pending) is IO-thread-confined"); + // Serialize: only one think() in flight (compute pool has 1 thread). - if (m_think_running.exchange(true)) { + // #854: a skipped request is no longer DROPPED — acquire_think_slot() + // records it in m_rethink_pending and the cycle that owns the slot + // re-posts run_think() when it releases. Without that, a local mint or a + // peer best-advert landing during a think/clean IO-phase (tracker lock + // already released, flag still true) silently loses its re-election, so + // the tip change never reaches the rigs until the 25 s keepalive. + if (!dash::think::acquire_think_slot(m_think_running, m_rethink_pending)) { static int skip_log = 0; if (skip_log++ % 20 == 0) - LOG_INFO << "[ASYNC-THINK] skipped — compute thread busy"; + LOG_INFO << "[ASYNC-THINK] skipped — compute thread busy (re-think queued)"; return; } if (!m_context) { - // Rig-free (KAT/standalone) node: no IO context to hop back to. - m_think_running.store(false); + // Rig-free (KAT/standalone) node: no IO context to hop back to, so + // there is nowhere to post a queued re-think either — drop it. + (void)dash::think::release_think_slot(m_think_running, m_rethink_pending); return; } @@ -611,16 +633,24 @@ void NodeImpl::run_think() drain_pending_adds(); - m_think_running.store(false); + // #854: release_think_slot() reports whether a run_think() request + // was dropped while this cycle held the slot — e.g. the mint that + // drain_pending_adds()/add_local_share() just landed. Serve it, or + // that share's election (and the miner notify it drives) is lost + // until the next timer tick. + const bool rethink_owed = + dash::think::release_think_slot(m_think_running, m_rethink_pending); - if (needs_continue) + if (needs_continue || rethink_owed) boost::asio::post(*m_context, [this]() { run_think(); }); } catch (const std::exception& e) { LOG_ERROR << "run_think() IO phase failed: " << e.what(); - m_think_running.store(false); + if (dash::think::release_think_slot(m_think_running, m_rethink_pending)) + boost::asio::post(*m_context, [this]() { run_think(); }); } catch (...) { LOG_ERROR << "run_think() IO phase failed: unknown error"; - m_think_running.store(false); + if (dash::think::release_think_slot(m_think_running, m_rethink_pending)) + boost::asio::post(*m_context, [this]() { run_think(); }); } }); }); @@ -812,11 +842,12 @@ void NodeImpl::drain_peer_best_adverts() to_download.emplace_back(weak_peer, best); } } - // TODO(dash-async, follow-up): when drain_peer_best_adverts runs FROM the - // think() IO-phase, m_think_running is still true, so this run_think() is a - // no-op (skipped). A m_rethink_pending flag checked at the end of the think - // cycle would guarantee the re-election happens. Benign today: the 2s - // advert timer calls drain again shortly, and by then think() has finished. + // #854 (was a TODO here): when drain_peer_best_adverts runs FROM the + // think() IO-phase, m_think_running is still true, so this run_think() + // cannot start a cycle. It is no longer a no-op — acquire_think_slot() + // records the request in m_rethink_pending and the owning cycle re-posts + // run_think() when it releases, so the re-election is guaranteed rather + // than left to the next 2 s advert-timer tick. if (rethink) run_think(); for (auto& [weak_peer, best] : to_download) @@ -913,26 +944,25 @@ void NodeImpl::clean_tracker() if (m_clean_running.exchange(true)) return; - // Skip if think() is already in flight — the periodic timer will retry. - // TODO(dash-async, follow-up): this load()-then-store() is check-then-act; - // a run_think() could win the m_think_running flag between the load and the - // store below. Benign today because both only ever run on the IO thread - // (the timer callbacks are serialized on the single io_context), so there - // is no true concurrency — but a compare_exchange on m_think_running would - // make the guard symmetric with the exchange() above and future-proof. - if (m_think_running.load()) { - m_clean_running.store(false); - return; - } if (!m_context) { m_clean_running.store(false); return; // rig-free (KAT/standalone) node } + // Take the think slot, which also blocks run_think() re-entry for the + // duration of the clean. #854: acquire_clean_slot() is a compare_exchange, + // closing the load()-then-store() check-then-act window this prologue used + // to carry as a TODO — the guard is now symmetric with the m_clean_running + // exchange() above. A failure records NO pending re-think: the think cycle + // that won the slot performs the same election clean's Step 1 would, and + // the periodic timer retries the prune shortly. + if (!dash::think::acquire_clean_slot(m_think_running)) { + m_clean_running.store(false); + return; + } + // Post the entire body to the compute thread: chain modifications happen // under the exclusive lock, never concurrent with think() or IO reads. - m_think_running.store(true); // block think() re-entry during clean - boost::asio::post(m_think_pool, [this]() { m_compute_thread_id.store(std::this_thread::get_id(), std::memory_order_relaxed); @@ -941,6 +971,14 @@ void NodeImpl::clean_tracker() try { std::unique_lock lock(m_tracker_mutex); // exclusive + // #854: the tip AS THE CLEAN CYCLE FOUND IT. clean_best_changed is + // decided against this, not against the value Step 1 has already + // written — otherwise an election absorbed by Step 1 (the case a mint + // landing during a think IO-phase produces) makes Step 4's comparison + // trivially false and no work is pushed to the rigs. Read under the + // exclusive lock, on the compute thread, before either think() runs. + const uint256 best_at_entry = m_best_share_hash; + auto block_rel_height = m_block_rel_height_fn ? m_block_rel_height_fn : std::function([](uint256) -> int32_t { return 0; }); @@ -1072,10 +1110,16 @@ void NodeImpl::clean_tracker() /*bits=*/0, bootstrap); m_last_top5_heads = std::move(result.top5_heads); if (!result.best.IsNull()) { - clean_best_changed = (m_best_share_hash != result.best); m_best_share_hash = result.best; apply_min_protocol_ratchet(); // v36 accept-floor ratchet (dgb node.cpp:2077 parallel) } + // #854: decide across BOTH thinks — entry tip vs exit tip. Placed + // outside the IsNull() guard on purpose: if Step 1 elected a new + // tip and Step 4's think returns nothing (e.g. everything it would + // have scored was just pruned), m_best_share_hash still carries + // Step 1's election and that IS a tip change the rigs must see. + clean_best_changed = + dash::think::clean_cycle_best_changed(best_at_entry, m_best_share_hash); publish_snapshot(); flush_verified_to_leveldb(); } @@ -1100,6 +1144,10 @@ void NodeImpl::clean_tracker() try { if (clean_best_changed) { LOG_INFO << "[CLEAN] IO-phase: work refresh (best changed)"; + // Same single fan-out every other tip-change leg uses: the + // binding in main_dash.cpp calls dash::stratum:: + // fire_share_tip_refresh (bump -> notify_all -> dashboard). + // No second notify call-site with its own logic. if (m_on_best_share_changed) m_on_best_share_changed(); broadcast_share(m_best_share_hash); // re-announce new head @@ -1110,8 +1158,14 @@ void NodeImpl::clean_tracker() } catch (...) { LOG_ERROR << "[CLEAN] IO phase failed: unknown error"; } - m_think_running.store(false); + // #854: the clean cycle holds the think slot too, so run_think() + // requests that arrived during it (a mint, a peer best-advert) were + // queued rather than dropped — serve them now. + const bool rethink_owed = + dash::think::release_think_slot(m_think_running, m_rethink_pending); m_clean_running.store(false); + if (rethink_owed) + boost::asio::post(*m_context, [this]() { run_think(); }); }); }); } @@ -1379,6 +1433,11 @@ uint256 NodeImpl::add_local_share(ShareType share) } // Lock released — relay + rescore (both take their own locks / post). + // #854: this run_think() is the ONLY thing that promotes the share we just + // minted to the tip and thereby pushes fresh work to the rigs. It used to + // be dropped outright whenever it landed during a think/clean IO-phase + // (IO thread, tracker lock already released, m_think_running still true) — + // the residual missed-notify path. acquire_think_slot() now queues it. broadcast_share(hash); run_think(); return hash; diff --git a/src/impl/dash/node.hpp b/src/impl/dash/node.hpp index 88956df03..3ba2ef88e 100644 --- a/src/impl/dash/node.hpp +++ b/src/impl/dash/node.hpp @@ -146,6 +146,18 @@ class NodeImpl : public pool::BaseNode m_think_running{false}; std::atomic m_clean_running{false}; + // #854: a run_think() request that arrives while a cycle is already in + // flight used to be DROPPED. It is reachable on the hot path — a local + // mint (add_local_share) or a peer best-advert drain landing during a + // think/clean IO-phase, when the tracker lock is already released but + // m_think_running is still true — and the dropped election is the tip + // change that never reaches the rigs. Set by dash::think:: + // acquire_think_slot() when the slot is busy, consumed by + // dash::think::release_think_slot() at the end of the running cycle, + // which then re-posts run_think(). Both flags are IO-thread-only (single + // ioc.run() thread in main_dash); the compute thread never touches them. + // Protocol + rationale: src/impl/dash/think_gate.hpp. + std::atomic m_rethink_pending{false}; mutable std::shared_mutex m_tracker_mutex; // ── Lock-free stats snapshot ───────────────────────────────────────── diff --git a/src/impl/dash/think_gate.hpp b/src/impl/dash/think_gate.hpp new file mode 100644 index 000000000..c3927e7a2 --- /dev/null +++ b/src/impl/dash/think_gate.hpp @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later +#pragma once + +// ============================================================================ +// think_gate.hpp — the DASH async-think serialization protocol, in ONE place. +// +// The DASH node runs at most one tracker.think() at a time: the compute pool +// has a single thread and think() holds m_tracker_mutex exclusively. That is +// enforced by a `running` flag which run_think() exchanges on entry and the +// IO-phase clears on exit. +// +// The bug that flag alone creates (#854): a run_think() request that arrives +// while a cycle is in flight is DROPPED, not queued. It is reachable on the +// hot path — add_local_share() (node.cpp, mint) and drain_peer_best_adverts() +// both call run_think() from the IO thread, and a think cycle's IO-phase runs +// on that same thread with `running` still true (the tracker lock is already +// released, so the mint that triggered the request has really landed in the +// chain). The election that would have promoted the freshly minted share to +// the tip never runs, so the sharechain best-share-changed callback never +// fires, so no mining.notify is pushed and rigs keep hashing the previous +// prev_share_hash until the 25 s keepalive — which is itself non-clean +// (clean_jobs is keyed on the COIN prevhash, core/stratum_server.cpp), i.e. a +// bounded sibling burst. That is the residual of the #853 storm. +// +// The fix is a second flag: a request that loses the race is REMEMBERED, and +// the owner of the cycle services it when it releases. These two free +// functions are the whole protocol, so node.cpp and the KATs +// (test_dash_stratum_notify_roundtrip) exercise literally the same code. +// +// ── THREADING ─────────────────────────────────────────────────────────────── +// Both flags are touched ONLY on the IO thread in main_dash (one ioc.run() +// thread): run_think()'s prologue, clean_tracker()'s prologue (the periodic +// timer callback), and both cycles' IO-phase epilogues, which are +// boost::asio::post(*m_context, ...) handlers. The compute thread NEVER reads +// or writes either flag. They are std::atomic anyway so the release/acquire +// pairing across the compute-thread hop is well defined, and so the pattern +// stays correct if the DASH lane ever grows a second IO thread. +// +// Release ordering is deliberate: `running` is cleared BEFORE the pending flag +// is consumed. A request that interleaves at that point sees running == false, +// wins the slot outright and runs the think itself — so the re-think is served +// exactly once and never lost. The reverse order could drop it. +// +// Consensus-neutral: this file decides only WHEN an election re-runs and when +// miners are told about it. It touches no share bytes, no timestamp, no +// gentx/ref_hash, no PPLNS weight, and no part of the won-block submit path. +// ============================================================================ + +#include + +#include + +namespace dash::think { + +/// Try to take the single think slot for a run_think() cycle. +/// +/// Returns true if the caller now owns the cycle and must eventually call +/// release_think_slot(). Returns false if a cycle is already in flight — in +/// which case the request is recorded, and the owner of the running cycle will +/// re-post run_think() when it releases. +inline bool acquire_think_slot(std::atomic& running, + std::atomic& rethink_pending) +{ + if (running.exchange(true)) { + rethink_pending.store(true); // #854: remember the dropped request + return false; + } + return true; +} + +/// Try to take the single think slot for the CLEAN maintenance cycle. +/// +/// Unlike acquire_think_slot() a failure here records NOTHING: clean_tracker() +/// is periodic maintenance driven by its own timer, and the think cycle that +/// beat it to the slot already performs the election clean's Step 1 would have +/// performed. Queuing a re-think for a skipped CLEAN would just add a +/// redundant think. +/// +/// compare_exchange (not load-then-store) closes the check-then-act window the +/// pre-#854 clean_tracker() prologue carried as a TODO: the guard is now +/// symmetric with the m_clean_running exchange() above it. +inline bool acquire_clean_slot(std::atomic& running) +{ + bool expected = false; + return running.compare_exchange_strong(expected, true); +} + +/// Release the think slot at the end of a cycle's IO-phase. +/// +/// Returns true if at least one run_think() request was dropped while the +/// cycle held the slot, i.e. the caller MUST re-post run_think(). Multiple +/// dropped requests collapse into one re-think — think() is a full re-election +/// over the current chain, so one pass subsumes any number of requests. +inline bool release_think_slot(std::atomic& running, + std::atomic& rethink_pending) +{ + running.store(false); // clear FIRST — see the ordering note + return rethink_pending.exchange(false); +} + +/// Did the CLEAN cycle move the sharechain tip? +/// +/// #854: clean_tracker() runs think() TWICE — Step 1 (score the chain as +/// found) and Step 4 (re-score after stale-head eating / tail dropping) — and +/// BOTH assign m_best_share_hash. The pre-#854 code computed the "did the tip +/// move" flag inside Step 4 only, against the value Step 1 had ALREADY +/// updated: +/// +/// Step 1: m_best_share_hash = result.best; // A -> B +/// Step 4: changed = (m_best_share_hash != result.best); // B != B -> false +/// +/// so an election absorbed by Step 1 — exactly the case a mint landing during +/// a think IO-phase produces — evaluated false and pushed no work to the rigs. +/// +/// Comparing the tip at cycle ENTRY against the tip at cycle EXIT detects the +/// change wherever inside the cycle it materialised. `best_at_exit` is +/// m_best_share_hash after both thinks; it is null only on a node that has +/// never elected anything, which is not a tip change. +inline bool clean_cycle_best_changed(const uint256& best_at_entry, + const uint256& best_at_exit) +{ + return !best_at_exit.IsNull() && best_at_exit != best_at_entry; +} + +} // namespace dash::think diff --git a/test/test_dash_stratum_notify_roundtrip.cpp b/test/test_dash_stratum_notify_roundtrip.cpp index cdd444752..a3959f194 100644 --- a/test/test_dash_stratum_notify_roundtrip.cpp +++ b/test/test_dash_stratum_notify_roundtrip.cpp @@ -39,6 +39,7 @@ #include +#include #include #include #include @@ -49,6 +50,7 @@ #include // merkle_branches_raw, sha256d, EXTRANONCE2_SIZE #include // fire_share_tip_refresh (tip-change fan-out) #include // LocalMintLedger / classify_local_mint +#include // acquire/release_think_slot, clean_cycle_best_changed (#854) #include // HexStr #include @@ -465,3 +467,255 @@ TEST(DashLocalMintLedger, PendingSetIsBounded) { EXPECT_EQ(s.dropped, 10u); EXPECT_DOUBLE_EQ(s.orphan_rate, 0.0); // nothing settled -> no rate invented } + +// ═════════════════════════════════════════════════════════════════════════════ +// #854 — the RESIDUAL missed-notify paths #853 did not cover. +// +// #853 fixed the callback (it now pushes mining.notify instead of only +// invalidating the cached payload). What it could not fix from that seam is +// WHEN the callback runs at all. Two pre-existing holes in src/impl/dash/ +// node.cpp kept a real sharechain tip change from ever reaching it: +// +// (A) CLEAN Step 1 absorbs the election. clean_tracker() runs think() twice +// and BOTH passes assign m_best_share_hash; the pre-#854 code decided +// "did the tip move" inside Step 4 only, against the value Step 1 had +// ALREADY written — so an election that materialised in Step 1 compared +// equal, clean_best_changed stayed false and nothing was pushed. +// +// (B) The skipped-think race. run_think() bailed out when m_think_running +// was set and DROPPED the request. A local mint (add_local_share) or a +// peer best-advert lands on the IO thread during a think/clean IO-phase +// — tracker lock already released, flag still true — so the re-election +// that would promote the just-minted share to the tip never ran. +// +// Both then fall through to the 25 s keepalive, which is NON-clean +// (clean_jobs is keyed on the COIN prevhash, core/stratum_server.cpp:1711), so +// the ASIC drains its queued stale work first: a rare bounded sibling burst. +// +// These KATs run the REAL landed helpers (src/impl/dash/think_gate.hpp) and +// the REAL fan-out (fire_share_tip_refresh), and each one carries the +// pre-#854 formula verbatim alongside so the divergence is pinned, not +// asserted. Every pre-#854 expectation below FAILS against master. +// +// Consensus-neutral by construction: nothing here (or in the code it pins) +// reads or writes a share, a timestamp, a gentx, a ref_hash, a PPLNS weight, +// or the won-block path. It decides only when an election re-runs and when +// miners are told. +// ═════════════════════════════════════════════════════════════════════════════ + +namespace { + +// Mirrors NodeImpl::clean_tracker()'s compute phase: `best` stands in for +// m_best_share_hash, and the two arguments are what each think() elected +// (null = think returned no best, e.g. everything scorable was just pruned). +struct CleanCycleSim { + uint256 best; + + // The LANDED (#854) decision: entry tip vs exit tip, across both thinks. + bool run(const uint256& step1_best, const uint256& step4_best) { + const uint256 best_at_entry = best; + if (!step1_best.IsNull()) best = step1_best; // Step 1 + if (!step4_best.IsNull()) best = step4_best; // Step 4 + return dash::think::clean_cycle_best_changed(best_at_entry, best); + } + + // The pre-#854 decision, verbatim from master: computed inside Step 4's + // IsNull() guard, against the value Step 1 already overwrote. + bool run_pre854(const uint256& step1_best, const uint256& step4_best) { + bool clean_best_changed = false; + if (!step1_best.IsNull()) best = step1_best; + if (!step4_best.IsNull()) { + clean_best_changed = (best != step4_best); + best = step4_best; + } + return clean_best_changed; + } +}; + +// What the rigs actually see: the clean IO-phase fires the shared fan-out. +int notifies_for(bool clean_best_changed) { + SpyLog log; + SpyWorkSource ws{&log}; + SpyStratumServer ss{&log}; + SpyWebServer web{&log}; + if (clean_best_changed) + dash::stratum::fire_share_tip_refresh(&ws, &ss, &web); + return ss.notifies; +} + +} // namespace + +// (15) THE #854 REGRESSION PIN: a tip change absorbed by CLEAN Step 1 must +// still push work. Step 4 re-elects the SAME share Step 1 just installed, +// which is exactly what happens when the election has already converged +// by the time the prune finishes. +TEST(DashCleanCycleTipRefresh, TipChangeAbsorbedByStep1PushesNotify) { + const uint256 A = fill_hash(0xA1); // tip as the clean cycle found it + const uint256 B = fill_hash(0xB2); // the new tip (e.g. our fresh mint) + + CleanCycleSim sim{A}; + EXPECT_TRUE(sim.run(/*step1=*/B, /*step4=*/B)) + << "tip moved A->B inside the clean cycle but the cycle reported no change"; + EXPECT_EQ(sim.best, B); + EXPECT_EQ(notifies_for(true), 1); + + // Master's formula on the same inputs: B != B -> false -> zero notifies. + CleanCycleSim pre{A}; + EXPECT_FALSE(pre.run_pre854(B, B)); + EXPECT_EQ(notifies_for(false), 0); +} + +// (16) Step 1 elects, Step 4's think returns nothing (its scorable heads were +// pruned). Step 1's election still stands in m_best_share_hash and IS the +// tip the rigs must be moved to — master skipped the flag entirely here, +// because it lived inside Step 4's IsNull() guard. +TEST(DashCleanCycleTipRefresh, Step1ElectionSurvivesEmptyStep4) { + const uint256 A = fill_hash(0xA1), B = fill_hash(0xB2); + + CleanCycleSim sim{A}; + EXPECT_TRUE(sim.run(/*step1=*/B, /*step4=*/uint256())); + EXPECT_EQ(sim.best, B); + + CleanCycleSim pre{A}; + EXPECT_FALSE(pre.run_pre854(B, uint256())); +} + +// (17) No regression on the path master DID handle: the tip moves only in +// Step 4 (the prune changed the scoring). Both formulas agree. +TEST(DashCleanCycleTipRefresh, Step4OnlyTipChangeStillPushesNotify) { + const uint256 A = fill_hash(0xA1), C = fill_hash(0xC3); + + CleanCycleSim sim{A}; + EXPECT_TRUE(sim.run(/*step1=*/A, /*step4=*/C)); + CleanCycleSim pre{A}; + EXPECT_TRUE(pre.run_pre854(A, C)); + EXPECT_EQ(notifies_for(true), 1); +} + +// (18) A quiet clean cycle must NOT push: no tip change, no notify. Clean runs +// on a periodic timer, so a false positive here would spam every rig with +// clean_jobs=true and throw away queued work for nothing. +TEST(DashCleanCycleTipRefresh, UnchangedTipDoesNotPush) { + const uint256 A = fill_hash(0xA1); + + CleanCycleSim sim{A}; + EXPECT_FALSE(sim.run(A, A)); + EXPECT_FALSE(sim.run(uint256(), uint256())); // neither think elected + EXPECT_EQ(sim.best, A); + EXPECT_EQ(notifies_for(false), 0); + + // And a node that has never elected anything is not a tip change. + CleanCycleSim fresh{uint256()}; + EXPECT_FALSE(fresh.run(uint256(), uint256())); +} + +// (19) THE SKIP-RACE PIN: a run_think() request that loses the slot is QUEUED, +// and the owning cycle serves it on release. Pre-#854 it was dropped. +TEST(DashThinkGate, SkippedRunThinkIsQueuedAndServed) { + std::atomic running{false}, pending{false}; + + // Cycle #1 takes the slot. + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + + // IO-phase of cycle #1: a local mint lands and calls run_think(). + EXPECT_FALSE(dash::think::acquire_think_slot(running, pending)) + << "a second cycle must not start while one is in flight"; + EXPECT_TRUE(pending.load()) << "the dropped request was not remembered"; + + // Cycle #1 releases: it is told a re-think is owed, and the flag is + // consumed so the re-think happens exactly once. + EXPECT_TRUE(dash::think::release_think_slot(running, pending)); + EXPECT_FALSE(pending.load()); + EXPECT_FALSE(running.load()); + + // The owed re-think runs and finds nothing further queued. + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + EXPECT_FALSE(dash::think::release_think_slot(running, pending)); +} + +// (20) N dropped requests collapse into ONE re-think — think() is a full +// re-election over the current chain, so one pass subsumes them all. This +// is what keeps the fix from turning a burst into a think storm. +TEST(DashThinkGate, ManySkipsCollapseToOneRethink) { + std::atomic running{false}, pending{false}; + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + for (int i = 0; i < 16; ++i) + EXPECT_FALSE(dash::think::acquire_think_slot(running, pending)); + EXPECT_TRUE(dash::think::release_think_slot(running, pending)); + EXPECT_FALSE(dash::think::release_think_slot(running, pending)); +} + +// (21) An idle cycle owes nothing: no spurious re-post, no think loop. +TEST(DashThinkGate, QuietCycleOwesNoRethink) { + std::atomic running{false}, pending{false}; + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + EXPECT_FALSE(dash::think::release_think_slot(running, pending)); + EXPECT_FALSE(running.load()); +} + +// (22) The CLEAN slot is a compare_exchange (closing the pre-#854 +// load()-then-store() check-then-act window) and a lost CLEAN queues +// NOTHING — the think that beat it already performs that election, and +// clean's own periodic timer retries the prune. +TEST(DashThinkGate, CleanSlotIsAtomicAndQueuesNothing) { + std::atomic running{false}, pending{false}; + + ASSERT_TRUE(dash::think::acquire_clean_slot(running)); + EXPECT_TRUE(running.load()); + + // A think starting now must lose, and IS queued (it is a real election). + EXPECT_FALSE(dash::think::acquire_think_slot(running, pending)); + EXPECT_TRUE(pending.load()); + + // A second clean must lose, and must NOT leave the flag set on its own. + std::atomic running2{true}, pending2{false}; + EXPECT_FALSE(dash::think::acquire_clean_slot(running2)); + EXPECT_FALSE(pending2.load()); + + // Clean's IO-phase release serves the think that was queued during it. + EXPECT_TRUE(dash::think::release_think_slot(running, pending)); +} + +// (23) End to end, the production sequence: a share is minted while a think +// cycle is in its IO-phase; the queued re-think elects it as the new tip; +// the tip change fires the SAME fan-out every other leg uses, so the rigs +// are pushed a clean job. Pre-#854 the request was dropped and the rigs +// saw nothing until the (non-clean) 25 s keepalive. +TEST(DashThinkGate, MintDuringThinkIoPhaseStillReachesTheMiners) { + const uint256 OLD_TIP = fill_hash(0x51); + const uint256 MINTED = fill_hash(0x52); + + SpyLog log; + SpyWorkSource ws{&log}; + SpyStratumServer ss{&log}; + SpyWebServer web{&log}; + + std::atomic running{false}, pending{false}; + uint256 best = OLD_TIP; + + // Cycle #1 runs and elects nothing new. + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + // ... IO-phase (lock released, flag still set): add_local_share() lands + // MINTED in the chain and calls run_think(). + const bool started_now = dash::think::acquire_think_slot(running, pending); + EXPECT_FALSE(started_now); + // Cycle #1 releases and is told to serve the queued re-think. + ASSERT_TRUE(dash::think::release_think_slot(running, pending)); + + // The owed re-think: the election now sees MINTED as the best head. + ASSERT_TRUE(dash::think::acquire_think_slot(running, pending)); + const uint256 best_at_entry = best; + best = MINTED; + const bool best_changed = (best != best_at_entry); + EXPECT_FALSE(dash::think::release_think_slot(running, pending)); + + ASSERT_TRUE(best_changed); + if (best_changed) + dash::stratum::fire_share_tip_refresh(&ws, &ss, &web); + + EXPECT_EQ(ss.notifies, 1) << "the freshly minted tip never reached the rigs"; + ASSERT_EQ(log.calls.size(), 3u); + EXPECT_EQ(log.calls[0], "bump"); + EXPECT_EQ(log.calls[1], "notify"); + EXPECT_EQ(log.calls[2], "web"); +}