Reject s9_..._... deposit-format wrapper in Thunder address validation

Empirical finding on the live forknet deployment: the LayerTwo-Labs
enforcer accepts the wrapper string in CreateDepositTransaction's
address field, and Thunder's OP_RETURN parser logs 'Ignoring invalid
deposit address' for deposits made to it. Thunder's own transfer RPC
also refuses to send to the wrapper form. Net effect: a miner who
authorizes with the wrapper accrues PPS balance in pps_credits that
can never be paid out — the reserve is intact but the address is
unspendable.

Change: thunder_address_decode strictly rejects any address containing
'_'. Only bare base58 (raw base58 of the 20-byte hash) is accepted.
Miners still see an actionable error at mining.authorize time with a
message pointing at 'thunder-cli get-new-address' as the correct form.

- src/thunder.c: reject on '_' with a friendly message before b58_decode
- src/thunder.h: docstring rewritten to describe the reject; explains
  why the wrapper is invalid at the byte level despite looking like a
  Thunder address
- tests/test_thunder.c: test_deposit_format renamed to
  test_rejects_wrapper_form and inverted — the wrapper must now be
  rejected, and the error message must contain a wrapper-related
  keyword so the miner knows what to fix
- OPERATOR_GUIDE.md: deposit runbook rewritten to use POOL_ADDR
  directly (bare base58) instead of the wrapper from
  format-deposit-address; adds a note explaining why
- scripts/regtest/thunder-init.sh: same fix; the printed grpcurl
  example now passes the bare address to CreateDepositTransaction
- PPS_THUNDER.md: user-facing description now says 'bare Thunder
  addresses' — wrapper explicitly noted as rejected

Verified: 7/7 test suites green including the inverted wrapper test.

Author: rsantacroce

OPERATOR_GUIDE.md

@@ -124,27 +124,40 @@ reconciliation.
 ```sh
 ssh -i <ssh-key> root@<pool-host>
 
-# 1. Get a Thunder-owned deposit address (formatted for the mainchain wallet)
+# 1. Get a Thunder-owned deposit address (BARE base58 — 20-byte hash).
+#    Do NOT use `format-deposit-address`; that produces the display-only
+#    `s<n>_<base58>_<hex6>` wrapper that Thunder's own OP_RETURN parser
+#    rejects. Empirically verified — deposits to the wrapper form are
+#    logged as "Ignoring invalid deposit address" and end up unpayable.
 TCLI=/home/forknet/forknet-software/thunder-rust/target/debug/thunder_app_cli
 POOL_ADDR=$(sudo -u forknet $TCLI get-new-address)
-DEP=$(sudo -u forknet $TCLI format-deposit-address "$POOL_ADDR")
-echo "deposit target: $DEP"
+echo "deposit target: $POOL_ADDR"
 
 # 2. Trigger the deposit via the enforcer's wallet. AMOUNT + FEE are yours to pick.
 GRPCURL=/home/forknet/forknet-software/grpcurl
 AMOUNT_SATS=1000000     # 0.01 BTC = 1_000_000 sats
 FEE_SATS=1000
 $GRPCURL -plaintext \
-  -d "{\"sidechain_id\":9,\"address\":\"$DEP\",\"value_sats\":$AMOUNT_SATS,\"fee_sats\":$FEE_SATS}" \
+  -d "{\"sidechain_id\":9,\"address\":\"$POOL_ADDR\",\"value_sats\":$AMOUNT_SATS,\"fee_sats\":$FEE_SATS}" \
   127.0.0.1:50051 cusf.mainchain.v1.WalletService/CreateDepositTransaction
 
-# 3. Wait for a mainchain block. If you want to force one on the test:
+# 3. Wait for a natural mainchain block from your miner (or on regtest,
+#    force one with GenerateBlocks).
 $GRPCURL -plaintext -d '{"blocks":1}' \
   127.0.0.1:50051 cusf.mainchain.v1.WalletService/GenerateBlocks
 
 # 4. Confirm the Ctip moved
 $GRPCURL -plaintext -d '{"sidechain_number":9}' \
   127.0.0.1:50051 cusf.mainchain.v1.ValidatorService/GetCtip
+
+# 5. Poke Thunder to include the new deposit in a sidechain block.
+#    `mine` may time out on the client side but the block gets produced.
+curl -sS --max-time 15 -H 'content-type: application/json' \
+  --data '{"jsonrpc":"2.0","id":1,"method":"mine","params":[]}' \
+  http://127.0.0.1:6009/
+
+# 6. Confirm Thunder's wallet balance grew
+sudo -u forknet $TCLI balance
 ```
 
 **About the "enforcer's wallet"**: the enforcer runs its own on-box BTC

PPS_THUNDER.md

@@ -46,8 +46,10 @@ b1f3537 Add Redis broadcast for pool events alongside SQLite
 ### 2. PPS / Thunder pool mode
 - New config key `pool_mode = solo | pps` (default `solo`, behaviour
   unchanged). In `pps`:
-  - Stratum usernames must be **Thunder addresses** (base58 of the
-    20-byte hash, or the `s9_<base58>_<hex6>` deposit-format wrapper).
+  - Stratum usernames must be **bare Thunder addresses** — base58 of
+    the 20-byte hash. The `s9_<base58>_<hex6>` deposit-format wrapper
+    is rejected (Thunder itself doesn't recognize it at the byte level,
+    so a miner authorized with it would accrue unpayable PPS balance).
     Validated in [src/thunder.c](src/thunder.c).
   - Every block's coinbase is a **BIP300 drivechain deposit** into the
     configured `pool_thunder_reserve_address` instead of paying the

scripts/regtest/thunder-init.sh

@@ -22,9 +22,7 @@ fi
 # succeeds silently when yes; errors "no seed" to stderr when no.
 if ADDR="$($TCLI get-new-address 2>/dev/null)" && [[ -n "$ADDR" ]]; then
     echo "==> Thunder wallet already initialised"
-    DEP="$($TCLI format-deposit-address "$ADDR" 2>/dev/null)"
-    echo "  new address:    $ADDR"
-    echo "  deposit format: $DEP"
+    echo "  new address: $ADDR"
     exit 0
 fi
 
@@ -36,15 +34,17 @@ echo "==> setting seed"
 $TCLI set-seed-from-mnemonic "$MNEMONIC" >/dev/null
 
 ADDR="$($TCLI get-new-address)"
-DEP="$($TCLI format-deposit-address "$ADDR")"
 
 echo ""
 echo "Thunder wallet ready."
-echo "  new address:    $ADDR"
-echo "  deposit format: $DEP"
+echo "  new address: $ADDR"
 echo ""
+# NOTE: pass the BARE base58 address to CreateDepositTransaction. The
+# display-only 's<n>_<base58>_<hex6>' wrapper from format-deposit-address
+# is NOT recognized by Thunder's OP_RETURN parser — deposits to it are
+# logged as 'Ignoring invalid deposit address' and end up unpayable.
 echo "To send a deposit into this wallet from the mainchain:"
-echo "  grpcurl -plaintext -d '{\"sidechain_id\":9, \"address\":\"$DEP\","
+echo "  grpcurl -plaintext -d '{\"sidechain_id\":9, \"address\":\"$ADDR\","
 echo "    \"value_sats\":100000000, \"fee_sats\":1000}' \\"
 echo "    127.0.0.1:50051 cusf.mainchain.v1.WalletService/CreateDepositTransaction"
 echo "  grpcurl -plaintext -d '{\"blocks\":1}' \\"

src/thunder.c

@@ -60,33 +60,35 @@ int thunder_address_decode(const char *addr,
     }
     size_t alen = strlen(addr);
 
-    /* Deposit-format: 's' <digits> '_' <base58> '_' <hex6>.
-     * Strip the prefix and checksum suffix; keep the base58 middle.
+    /* Accept ONLY the bare base58 form (raw base58 of 20 hash bytes).
      *
-     * We don't verify the checksum here — that's the wallet's job and
-     * its bytes (3-byte sha256 prefix of the rest) are constructed
-     * client-side. We only need the raw 20 bytes for accounting. */
-    const char *b58_start = addr;
-    size_t b58_len = alen;
-    if (addr[0] == 's' || addr[0] == 'S') {
-        const char *first_us = strchr(addr, '_');
-        if (first_us) {
-            const char *last_us = strrchr(addr, '_');
-            if (last_us && last_us != first_us) {
-                b58_start = first_us + 1;
-                b58_len   = (size_t)(last_us - b58_start);
-            }
-        }
+     * The deposit-format wrapper `s<sc>_<base58>_<hex6>` looks like a
+     * Thunder address but is a display-only string that neither Thunder's
+     * wallet nor its OP_RETURN parser recognizes at the byte level — a
+     * pool that stores wrapper strings as workers.payout_address ends up
+     * with unpayable balances (thunder.transfer rejects the wrapper).
+     * To catch this at authorize time rather than payout time we treat
+     * any character outside the base58 alphabet as a hard error.
+     *
+     * '_' isn't in base58, so the wrapper's underscores make b58_decode
+     * fail below — but we bail early with a friendlier message so a
+     * miner using the wrong form gets an actionable stratum reject. */
+    if (strchr(addr, '_') != NULL) {
+        set_err(errbuf, errlen,
+                "thunder address contains '_' — looks like the "
+                "'s<n>_<base58>_<hex6>' deposit-format wrapper; use "
+                "the bare base58 (thunder-cli get-new-address) instead");
+        return -1;
     }
-    if (b58_len < 20 || b58_len > 40) {
+    if (alen < 20 || alen > 40) {
         set_err(errbuf, errlen,
-                "thunder address base58 length %zu out of range", b58_len);
+                "thunder address base58 length %zu out of range", alen);
         return -1;
     }
 
     uint8_t buf[64];
     size_t  dec_len = 0;
-    if (b58_decode(b58_start, b58_len, buf, sizeof buf, &dec_len) < 0) {
+    if (b58_decode(addr, alen, buf, sizeof buf, &dec_len) < 0) {
         set_err(errbuf, errlen, "thunder address base58 decode failed");
         return -1;
     }

src/thunder.h

@@ -8,21 +8,27 @@
  *
  * Per thunder-rust (`lib/types/address.rs`), a Thunder address is a
  * 20-byte hash, displayed as plain base58 (no version byte, no
- * checksum) — i.e. shorter than a P2PKH base58check address. There is
- * also a deposit-format string `s9_<base58>_<3-byte sha256 prefix hex>`
- * used by the mainchain wallet UI; we accept either form and surface
- * the raw 20 bytes.
+ * checksum) — i.e. shorter than a P2PKH base58check address.
  *
- * The mainchain coinbase carries the *pool's* Thunder reserve address
- * (every deposit accrues to the pool), not the miner's. The miner's
- * Thunder address from the stratum username is recorded for accounting
- * so the PPS payout worker (separate service, off-chain Thunder txs)
- * can credit the right account.
+ * There is also a display-only "deposit-format" wrapper
+ * `s<sc>_<base58>_<3-byte sha256 prefix hex>` produced by
+ * `thunder-cli format-deposit-address`. That form is for humans to
+ * paste into a mainchain wallet's deposit UI; it is NOT a Thunder
+ * address at the byte level and Thunder rejects it in every RPC that
+ * takes an address. Live empirical evidence: transfer() to a wrapper
+ * form fails, and the enforcer's OP_RETURN parser logs
+ * "Ignoring invalid deposit address" for it. We therefore reject
+ * the wrapper here so miners get a clear stratum error at authorize
+ * time instead of unpayable PPS balances at payout time.
+ *
+ * The miner's bare Thunder address from the stratum username is
+ * recorded for accounting; the PPS payout worker (separate service,
+ * off-chain Thunder txs) uses it as the recipient of thunder.transfer.
  */
 
-/* Decode a Thunder address string. Accepts:
- *   - bare base58:     `<28-34 chars>`            -> 20 raw bytes
- *   - deposit-format:  `s<num>_<base58>_<hex6>`   -> 20 raw bytes
+/* Decode a Thunder address string. Accepts ONLY the bare base58
+ * form (no wrapper, no underscores). The mainchain-wallet deposit
+ * wrapper is rejected with a message pointing at the correct form.
  *
  * `out[20]` receives the address bytes. Returns 0 ok, negative on error.
  * errbuf may be NULL. */

tests/test_thunder.c

@@ -71,8 +71,11 @@ static void test_roundtrip_random_pattern(void) {
     CHECK(memcmp(got, in, 20) == 0);
 }
 
-static void test_deposit_format(void) {
-    /* s9_<base58>_<checksum hex> — the wrapper format. */
+/* The `s<n>_<base58>_<hex6>` wrapper is a display-only string produced
+ * by `thunder-cli format-deposit-address`. Thunder's own transfer RPC
+ * refuses it, so we must reject it at authorize time; otherwise a
+ * miner authorizes fine but their PPS balance is unpayable forever. */
+static void test_rejects_wrapper_form(void) {
     uint8_t in[20];
     for (int i = 0; i < 20; i++) in[i] = (uint8_t)(i * 11 + 1);
     char b58[64];
@@ -82,11 +85,13 @@ static void test_deposit_format(void) {
     snprintf(wrapped, sizeof wrapped, "s9_%s_a1b2c3", b58);
 
     uint8_t got[20];
-    char err[128];
+    char err[192];
     int rc = thunder_address_decode(wrapped, got, err, sizeof err);
-    if (rc != 0) fprintf(stderr, "deposit decode err: %s\n", err);
-    CHECK(rc == 0);
-    CHECK(memcmp(got, in, 20) == 0);
+    CHECK(rc < 0);
+    /* Error message must mention the wrapper so miners know what to do. */
+    CHECK(strstr(err, "wrapper") != NULL ||
+          strstr(err, "deposit-format") != NULL ||
+          strstr(err, "_") != NULL);
 }
 
 static void test_rejects_garbage(void) {
@@ -102,7 +107,7 @@ static void test_rejects_garbage(void) {
 int main(void) {
     test_roundtrip_bare();
     test_roundtrip_random_pattern();
-    test_deposit_format();
+    test_rejects_wrapper_form();
     test_rejects_garbage();
     if (failures) {
         fprintf(stderr, "%d failure(s)\n", failures);