btc: F10/(b) complete — delete residual flat-count version-punish (#365)

Removes the non-canonical 95%-flat-count version-obsolescence punish that
the gap-audit found half-applied on the BTC twin (share_check.hpp already
carried the F10/(b) marker; the punish path itself was still live).

- delete ShareTracker::should_punish_version (share_tracker.hpp) and its
  live head-decoration call (formerly :1198, reason=1); head-scoring now
  punishes only naughty heads, matching canonical Phase-4 tip selection
- delete dead AutoRatchet::validate_version_switch (auto_ratchet.hpp;
  zero callers in src/impl/btc/) — the version gate is the check() 60%
  PPLNS-weighted switch rule in share_check step 2
- mirrors the landed LTC (share_tracker.hpp:2670) / DGB (:2651) removal,
  with the same F10/(b) removal marker
- new f10b_version_punish_removal_test.cpp (3 KATs) locks the accept-gate
  vs head-selection invariant on the version axis; full btc_share_test 9/9

Scope: src/impl/btc/ only. Consensus-bearing (tip selection) — hold for
operator merge.

Co-authored-by: frstrtr <frstrtr@users.noreply.github.com>

Author: frstrtr

src/impl/btc/auto_ratchet.hpp

@@ -273,60 +273,11 @@ class AutoRatchet
             return {current_version, target_version_};
     }
 
-    /// Validate a version switch between consecutive shares.
-    /// Returns empty string if valid, error message if invalid.
-    /// Implements the 60% switch rule from Python check() method.
-    static std::string validate_version_switch(
-        int64_t share_version, int64_t prev_version,
-        ShareTracker& tracker, const uint256& prev_hash)
-    {
-        // Same version — always ok
-        if (share_version == prev_version)
-            return {};
-
-        int32_t height = tracker.chain.get_height(prev_hash);
-        uint32_t chain_length = PoolConfig::chain_length();
-
-        if (height < static_cast<int32_t>(chain_length))
-        {
-            // Not enough history for version switch
-            if (share_version > prev_version)
-                return "version switch without enough history";
-            return {}; // downgrade ok without history
-        }
-
-        // Upgrade: requires 60% in sampling window [CHAIN_LENGTH*9/10, CHAIN_LENGTH]
-        if (share_version == prev_version + 1)
-        {
-            uint32_t window_start = (chain_length * 9) / 10;
-            uint32_t window_size = chain_length / 10;
-            auto ancestor = tracker.chain.get_nth_parent_key(prev_hash, window_start);
-            auto counts = tracker.get_desired_version_counts(ancestor, window_size);
-
-            int64_t new_ver_count = 0;
-            int64_t total_count = 0;
-            for (auto& [ver, cnt] : counts)
-            {
-                total_count += cnt;
-                if (ver >= share_version)
-                    new_ver_count += cnt;
-            }
-
-            if (total_count > 0 && new_ver_count * 100 < total_count * SWITCH_THRESHOLD)
-                return "version switch without enough hash power upgraded ("
-                       + std::to_string(new_ver_count * 100 / total_count)
-                       + "% < " + std::to_string(SWITCH_THRESHOLD) + "%)";
-            return {};
-        }
-
-        // Downgrade by 1 (AutoRatchet deactivation): allowed
-        if (share_version == prev_version - 1)
-            return {};
-
-        // Multi-version jump: not allowed
-        return "invalid version jump from " + std::to_string(prev_version)
-             + " to " + std::to_string(share_version);
-    }
+    // F10 (per ltc): validate_version_switch is intentionally absent — the
+    // single source of truth for the version-switch gate is share_check step 2,
+    // which calls ShareTracker::get_desired_version_weights and matches p2pool
+    // check() (data.py:1396-1414) exactly. The VOTING tail guard above stays
+    // inline and work-weighted (mint<->accept coupling).
 
 private:
     std::string state_file_;

src/impl/btc/share_tracker.hpp

@@ -1190,16 +1190,16 @@ class ShareTracker
                     if (!head_idx) continue;
                     int64_t ts = head_idx->time_seen;
 
-                    // Punish heads: version obsolescence OR naughty (invalid block)
+                    // Punish heads: naughty (invalid block) only.
+                    // F10/(b): the non-canonical 95%-version-obsolescence punish
+                    // is dropped — canonical Phase-4 head-scoring punishes only
+                    // naughty heads. The version gate is the check() 60% weighted
+                    // switch rule (share_check), not head-scoring.
                     int32_t reason = 0;
                     {
-                        auto share_version = chain.get_share(hh).version();
-                        auto lookbehind = static_cast<int32_t>(PoolConfig::chain_length());
-                        if (should_punish_version(hh, share_version, lookbehind))
-                            reason = 1;
                         auto* idx = chain.get_index(hh);
                         if (idx && idx->naughty > 0)
-                            reason = std::max(reason, idx->naughty);
+                            reason = idx->naughty;
                     }
 
                     // p2pool: sort key = (work - min(punish,1)*ata(target), -reason, -time_seen)
@@ -2683,30 +2683,12 @@ class ShareTracker
         return result;
     }
 
-    // Returns true if shares at `share_version` should be punished because
-    // a newer version has reached the 95% activation threshold.
-    // Python ref: share.check() version_after_check logic
-    bool should_punish_version(const uint256& share_hash, int64_t share_version, int32_t lookbehind)
-    {
-        if (!chain.contains(share_hash))
-            return false;
-        auto counts = get_desired_version_counts(share_hash, lookbehind);
-        auto height = chain.get_height(share_hash);
-        auto actual = std::min(lookbehind, height);
-        if (actual <= 0)
-            return false;
-
-        // Check if any version higher than share_version has >= 95% support
-        for (auto& [ver, count] : counts)
-        {
-            if (static_cast<int64_t>(ver) > share_version)
-            {
-                if (count * 100 >= actual * 95) // 95% threshold
-                    return true;
-            }
-        }
-        return false;
-    }
+    // F10/(b): should_punish_version (the 95%-obsolescence punish) was removed.
+    // It was non-canonical — canonical p2pool check() has no 95% obsolescence
+    // rule; the AutoRatchet (work-weighted, per #290) plus the 60% weighted
+    // switch rule (share_check step 2) are the only version gates. Keeping it
+    // would punish/score-down shares canonical accepts, breaking the #81 zero-
+    // divergence gate. Head-scoring (Phase 4) now punishes only naughty heads.
 
 private:
     std::vector<stale_callback_t> m_stale_callbacks;

src/impl/btc/test/CMakeLists.txt

@@ -1,7 +1,7 @@
 if (BUILD_TESTING AND GTest_FOUND)
     # btc twin of ltc share_test — uniquely named to avoid the CMP0002 target
     # collision with src/impl/ltc/test (both subdirs build in the same tree).
-    add_executable(btc_share_test share_test.cpp f11_donation_invariance_test.cpp)
+    add_executable(btc_share_test share_test.cpp f11_donation_invariance_test.cpp f10b_version_punish_removal_test.cpp)
     target_link_libraries(btc_share_test PRIVATE
         GTest::gtest_main GTest::gtest
         core btc

src/impl/btc/test/f10b_version_punish_removal_test.cpp

@@ -0,0 +1,111 @@
+// F10/(b) version-punish-removal KAT — guards the #288 tip-selection axis.
+//
+// This branch deletes should_punish_version() (the non-canonical 95%-flat-count
+// version-obsolescence punish, formerly src/impl/btc/share_tracker.hpp) and its
+// live head-decoration call (formerly share_tracker.hpp:1198, reason=1). It
+// mirrors the already-landed LTC (share_tracker.hpp:2670) / DGB (:2654) deletion.
+//
+// Backfills the coverage gap that let the #288-class divergence ship: there was
+// no test asserting that BTC tip-selection agrees with the canonical accept gate
+// on the version axis, so the extra punish down-ranked tips the gate accepts and
+// nobody noticed. This BTC twin locks the invariant.
+//
+// The two predicates this test pins (both quoted verbatim from the code as it
+// stood / stands):
+//
+//   ACCEPT GATE (canonical, SURVIVES — src/impl/btc/share_check.hpp:1793,
+//   matching p2pool data.py check() lines 1396-1414):
+//       reject boundary iff  new_ver_weight*100 < total_weight*60
+//   i.e. a one-version upgrade boundary is VALID when the new version holds
+//   >= 60% of the PPLNS-WEIGHTED desired-version support in the sampling window.
+//
+//   PUNISH (non-canonical, REMOVED — former should_punish_version):
+//       punish (reason=1) iff  newer_version_count*100 >= actual*95
+//   a flat (un-weighted) head-count rule with NO basis in p2pool check().
+//
+// #288 axis: there exists a desired-version distribution where the canonical
+// accept gate deems an upgrade boundary VALID while the removed flat-count
+// punish would have down-ranked that same tip. After this deletion the only
+// head-scoring punish contributor is `naughty` (invalid block), so a non-naughty
+// tip the accept gate accepts is never down-ranked. This test proves the
+// divergence existed and that the surviving gate is now the sole version decider.
+
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <map>
+
+namespace {
+
+// Canonical accept gate, byte-for-byte the predicate at share_check.hpp:1793.
+// weights: desired-version -> PPLNS weight (work, target_to_average_attempts).
+// Returns true if a boundary share at `share_ver` (== parent_ver + 1) is VALID.
+bool accept_gate_valid(const std::map<int64_t, uint64_t>& weights, int64_t share_ver) {
+    uint64_t new_ver_weight = 0, total_weight = 0;
+    for (auto& [ver, w] : weights) {
+        total_weight += w;
+        if (ver == share_ver) new_ver_weight += w;
+    }
+    // Canonical: counts.get(self.VERSION,0) < sum(counts)*60//100  ->  reject
+    return !(new_ver_weight * 100 < total_weight * 60);
+}
+
+// The REMOVED flat-count punish, reproduced exactly as should_punish_version
+// computed it. counts: version -> flat head count. `actual` = window size.
+// Returns true if a share at `share_ver` WOULD have been punished (reason=1).
+bool removed_flat_punish(const std::map<int64_t, uint64_t>& counts,
+                         int64_t share_ver, uint64_t actual) {
+    if (actual == 0) return false;
+    for (auto& [ver, count] : counts)
+        if (ver > share_ver && count * 100 >= actual * 95) // 95% threshold
+            return true;
+    return false;
+}
+
+// Head-scoring reason after the deletion: the only contributor is `naughty`.
+// Mirrors share_tracker.hpp:1196-1198 post-change (reason = idx->naughty).
+int32_t head_reason_after_deletion(int32_t naughty) { return naughty; }
+
+} // namespace
+
+// Core #288 case: a single newer version dominates by flat head count (>=95%)
+// but the upgraded version's WEIGHTED support clears 60%. The accept gate
+// accepts the boundary; the removed punish would have down-ranked it.
+TEST(BTC_F10b_VersionPunishRemoval, AcceptGateAndRemovedPunishDiverge) {
+    // 10 heads desire v37, the upgrade target; the boundary share is v37.
+    // By flat count v37 holds 95% (would have punished a v36 share as obsolete),
+    // but in weighted terms v37 also clears 60% so the boundary is canonical.
+    const int64_t share_ver = 37; // parent_ver(36) + 1
+
+    // Weighted view used by the surviving accept gate: v37 holds 7/10 weight.
+    std::map<int64_t, uint64_t> weights{{36, 300}, {37, 700}};
+    EXPECT_TRUE(accept_gate_valid(weights, share_ver))
+        << "canonical 60% weighted gate must accept the v37 boundary";
+
+    // The removed flat-count punish keyed off a NEWER version (v38) reaching
+    // 95% would have flagged the (older) v37 tip as obsolete and down-ranked it.
+    std::map<int64_t, uint64_t> flat_counts{{37, 0}, {38, 96}};
+    EXPECT_TRUE(removed_flat_punish(flat_counts, share_ver, /*actual=*/100))
+        << "the removed punish WOULD have down-ranked the v37 tip (the #288 miss)";
+
+    // Post-deletion, head-scoring no longer consults version at all: a
+    // non-naughty tip gets reason 0 and is NOT down-ranked.
+    EXPECT_EQ(0, head_reason_after_deletion(/*naughty=*/0))
+        << "after F10/(b) a non-naughty tip is never down-ranked on the version axis";
+}
+
+// Boundary just under canonical: 60% weighted gate REJECTS, so this is purely a
+// gate decision — nothing about head-scoring re-introduces a competing punish.
+TEST(BTC_F10b_VersionPunishRemoval, AcceptGateRejectsBelowSixtyWeighted) {
+    std::map<int64_t, uint64_t> weights{{36, 410}, {37, 590}}; // 59% < 60%
+    EXPECT_FALSE(accept_gate_valid(weights, /*share_ver=*/37))
+        << "below 60% weighted support the canonical gate alone rejects the boundary";
+}
+
+// The surviving head-scoring punish is naughty-only and order-independent of
+// the version distribution: naughty>0 still down-ranks (invalid block), and that
+// is the ONLY remaining reason a head is down-ranked.
+TEST(BTC_F10b_VersionPunishRemoval, NaughtyIsTheSoleSurvivingPunish) {
+    EXPECT_EQ(0, head_reason_after_deletion(0));
+    EXPECT_EQ(3, head_reason_after_deletion(3)); // naughty propagation preserved
+}