fix(upgrade): v12 cutover and sync lifecycle, plus a measured account of what still loses money

DASHJ-KILL-LIST.md

@@ -54,6 +54,125 @@ Most-used bitcoinj types repo-wide: `Coin` (105 files), `Transaction` (86), `Sha
   with the engine); dashj's `BlockchainState` (sync %, replaying, impediments) must be fully
   derived from `SpvSyncProgressData` (partially wired for the home header already).
 
+#### 1a. BLOCKER on the balance replacement: partial rescan state is published as fact
+
+`CutoverUiDataService` is the SDK-side replacement for the home-screen balance, so dashj cannot be
+deleted until this is right — after step B there is no second engine to sanity-check it against.
+
+Measured on a restored wallet, 2026-09-17 (see §26.4 of the upgrade memory and sync plan). Opening
+the Contacts screen triggered a DashPay-provisioning rewind of ~214,000 blocks, and during the
+re-scan the service published, in sequence:
+
+| Published balance | In DASH |
+|---:|---:|
+| 28,214,058,609 | 282.1 |
+| 24,233,701,645 | 242.3 |
+| 21,351,026,841 | 213.5 |
+| 16,902,773,043 | **169.0 — correct, matches pre-wipe exactly** |
+
+For ~30 seconds the wallet displayed up to **113 DASH more than the user actually has**, stepping
+down as the rescan re-derived the UTXO set. The end state is correct; the intermediate states are
+not, and they are indistinguishable from real money on screen.
+
+The signal to fix it with is already present: **every wrong value was published with
+`l1Synced=false`.** Options are to suppress publication while that flag is false, hold the last
+known-good figure, or mark the value as provisional in the UI. Today it emits partial rescan state
+as though it were settled.
+
+##### 1a.1 UPDATE 2026-09-19 — the source is the SDK's native ledger, and it does NOT self-correct
+
+Reproduced on a Samsung SM-S901U on the job-flower wallet, and this time the end state was WRONG
+too, which the 2026-09-17 reading was not.
+
+```
+18:47:57   107.08 DASH        <- correct
+18:52:54   107.08             <- app start
+19:00:24   107.08             <- app start
+19:11:02   107.08             <- app start
+19:18:43   107.08             <- app start
+19:29:34   110.07             <- replay to tip begins
+19:29:56  1649.00
+19:30:09  2942.25
+19:31:07  3024.56             <- settles here, 28x too high, and STAYS past SYNCED
+```
+
+**The SDK's own database is correct throughout.** Pulled mid-defect and queried directly:
+
+```
+UNSPENT   811 txos   107.08173522 DASH   <- the true balance, exactly
+spent    9236 txos   193556.21892690 DASH
+```
+
+Both spend markers agree — `isSpent` and `spendingTxid` classify identically, zero disagreement.
+So this is not missed spend detection, not double-inserted outputs, and not a persistence fault.
+
+**It is not our code either.** `CutoverUiDataService.currentBalanceSplitDuffs` does no arithmetic:
+
+```kotlin
+// Native ledger read — the SAME accessor L1ShadowSyncService.sdkBalanceDuffs
+// uses. A self-send debits this immediately; the Room `txos` table lags.
+val balance = wallet.balance()
+```
+
+It publishes whatever `wallet.balance()` returns. The wrong number comes out of the SDK's
+**in-memory native ledger**, which applies block events on top of an already-correct rehydrated
+balance and therefore double-counts anything re-walked. This wallet re-walked 20,000 blocks after a
+watchdog restart plus several full re-scans, which is why it reached 28x.
+
+That also explains D-041's reported "only a force-stop fixes it": a restart rehydrates the ledger
+from the `txos` table, which is why **every app start above reads exactly 107.08**.
+
+**Consequences for this item.** The `l1Synced=false` suppression proposed above is necessary but NO
+LONGER SUFFICIENT — the 2026-09-19 value was still wrong after SYNCED, so a wrong figure can now be
+published with `l1Synced=true`. Two further mitigations are available on the app side while the
+upstream fix is pending:
+
+- reconcile against the `txos` table once `synced` is true and publish that instead; the table was
+  correct at every point measured, and 1a's own requirement is that this value be right before
+  dashj can be deleted;
+- or treat a post-SYNCED disagreement between `wallet.balance()` and the table as a defect signal
+  and log it, so the field reports say which one was wrong.
+
+**The real repair is upstream**, in `dash-spv` / `platform-wallet`'s `wallet.balance()`. Like the
+filter-batch defect in §34 of the plan it sits below the FFI, so iOS is equally exposed. A cheap
+reproduction for whoever takes it: force a re-walk (the filter-stall watchdog's restart does it),
+watch the published balance climb, and confirm the `txos` table stays correct throughout — that
+distinguishes it from a persistence bug in one pass.
+
+Why this is a kill-list item and not just a bug: the rewind that exposes it is itself triggered by
+ordinary DashPay provisioning (§17.6, §26.3), so it is not a rare path, and the pre-cutover
+comparison against dashj's own balance (`L1Parity` probing) is the only thing that would currently
+catch a regression here. That probe is suspended post-cutover and disappears entirely at step B.
+
+#### 1b. DEAD PATH still shouting: the dashj broadcast handler warns that it did not send
+
+`BlockchainServiceImpl.kt:2700`, inside the `ACTION_BROADCAST_TRANSACTION` handler:
+
+```kotlin
+if (peerGroup != null) {
+    peerGroup!!.broadcastTransaction(tx, minimum, true)
+} else {
+    log.warn("peergroup not available, not broadcasting transaction {}", tx.txId)
+}
+```
+
+Post-cutover `peerGroup` is always null, so **every send from a cut-over wallet logs that it is not
+broadcasting** — while `SdkL1SendService` broadcasts it perfectly well through the SDK. The message
+states the opposite of what happened.
+
+The codebase already knows this: `SweepTxBroadcaster`'s KDoc says "Post-cutover that peergroup
+broadcast no-ops ('peergroup not available, not broadcasting')" and routes around it. The warning
+was simply never adjusted.
+
+Measured 2026-09-17 (§27.5 of the upgrade memory and sync plan): a contact payment logged this
+warning on the sender and was nonetheless confirmed on chain in block 1555561. It cost real time in
+that test — the run was nearly written off as a failed send — and it will mislead anyone reading a
+field log, which is exactly when this line gets read.
+
+The whole handler is dead weight that dies with the engine at step B. Until then, either demote the
+message and say the SDK owns broadcast now, or drop it when the cutover is committed. Cheap, and it
+removes a false signal from every send log we collect between now and step B.
+
 ### 2. Key derivation / signing / seed handling
 - **What it does**: `DeterministicSeed` (10 files), `KeyChainGroup`, `DeterministicKeyChain`,
   `ECKey` (15), `KeyCrypterScrypt`/`KeyCrypterException` (17), `MnemonicCode`, `BIP38PrivateKey`,
@@ -110,6 +229,42 @@ Most-used bitcoinj types repo-wide: `Coin` (105 files), `Transaction` (86), `Sha
 - **Blast radius**: `wallet/.../ui/dashpay/**`, `wallet/.../service/platform/**` — ~50 files,
   plus the per-flavor `dash-sdk-{java,kotlin,android}` Gradle lines.
 
+#### 5a. App-side DIP-15 friend-chain provisioning — now redundant (added 2026-09-16)
+
+- **What it is**: `SdkWalletBinder.provisionContactAccountsIfEnabled` /
+  `provisionContactAccountsInBackground`, plus `DashPayBackfillGate`. It derives the
+  `m/9'/coin'/15'` per-contact keychains and registers them with the SDK so contact/username
+  payments land in the filter set.
+- **Why it exists**: its own comment says it — "the app keeps DashPay contacts on dashj and
+  never drives the SDK's contact-sync path, so the bound SDK L1 wallet derives NONE of the
+  `m/9'/coin'/15'` friend chains and misses contact/username payments". It was the patch for
+  the SDK not being told the contacts existed. The exposure was real: 0.0836 DASH of
+  genuinely-ours coins missed at scan time on the topple wallet.
+- **Why it is now redundant**: `1266edc1c` started calling the SDK's own ordered bring-up,
+  `PlatformWalletManager.startWalletSubsystems`, which does identity → contacts →
+  contact-account drain and reports `dashPaySyncRan` / `contactAccountsDrained` /
+  `contactAccountsPending`. Measured on emulator-5556, 2026-09-16: the bring-up reported
+  `status=READY dashPaySyncRan=true drained=14 pending=0`, and every app-side sweep afterwards
+  reported `pendingBuilds=0, drainScheduled=false`. The app sweep runs once a minute and finds
+  nothing. `DashPayBackfillGate` is already `ALWAYS_RUN`, i.e. a no-op shell: `evaluate` always
+  returns `shouldRun = true, reason = "backfill gate disabled"`, and every bookkeeping method
+  returns a constant.
+- **The one job left**: `startWalletSubsystems` runs only at engine start
+  (`L1ShadowSyncService.startIfEnabled`, guarded by `if (!source.isSpvRunning())`), so a contact
+  established while the engine is already running is not picked up by it. `updateContactRequests`
+  calls the app sweep with `force = addedContact` for exactly that case. **Deleting the sweep
+  requires the SDK to re-run its contact sync on a contact change, or the app to restart the
+  engine on one.** Confirm with the SDK team before removing.
+- **Kill**: delete `provisionContactAccountsIfEnabled`,
+  `provisionContactAccountsInBackground`, `DashPayBackfillGate` and its `ALWAYS_RUN` shell, the
+  `watchArmedBackfillRewind` poller, and the two call sites in `PlatformSyncService`
+  (`kickSdkEngines`, `updateContactRequests`). The rewind that catches payments missed before a
+  chain was derived is already the SDK's: it marks newly registered contacts at
+  `synced_height = 0` and its filter manager rescans via the account-generation guard
+  (`dash-spv/src/sync/filters/manager.rs`). That is also the single mechanism the whole
+  contact-payment recovery now depends on — see section 16 of
+  `docs/upgrade-memory-and-sync-plan.md`, which tests it.
+
 ### 6. Checkpoints / birth height / chain bootstrap
 - **What it does**: `CheckpointManager` (3 files) + `wallet/assets/checkpoints{,-testnet}.txt`
   fast-forward dashj's chain; `Constants.java` (11 bitcoinj imports) holds `NetworkParameters`.

build.gradle

@@ -3,7 +3,7 @@ buildscript {
         kotlin_version = '2.1.0'
         coroutinesVersion = '1.6.4'
         ok_http_version = '4.12.0'
-        dashjVersion = '22.0.5-SNAPSHOT'
+        dashjVersion = '22.0.5'
         // Dash Platform Kotlin SDK (org.dashj:dash-sdk-android). Single source of
         // truth for both the dependency coordinate (wallet/build.gradle) and the
         // DASH_SDK_VERSION BuildConfig field shown on the About screen once the
@@ -14,13 +14,13 @@ buildscript {
         // #4439 (TXO reconcile) + the restore branch (#4474/#4475) + the ordered
         // bring-up binding (#4658) + #4643 keystore fixes, on the fork engine pin
         // af88edf6 (v4.2-dev pin 93260bf + rust-dashcore #979/#1001/#989).
-        dashSdkVersion = '0.1.0-v42int19-SNAPSHOT'
+        dashSdkVersion = '0.1.0-v42int21-SNAPSHOT'
         // 4.0.1 fixes the CBOR crash that aborted every identity read:
         // IdentityPublicKey.toObject() put the raw ContractBounds object
         // (SingleContractDocumentType) into the map handed to Cbor, which has
         // no converter for it -> IllegalArgumentException("No converter for ...").
         // 4.0.1 calls this.toObject() first. Cbor.kt itself is unchanged.
-        dppVersion = "4.0.1-SNAPSHOT"
+        dppVersion = "4.0.1"
         hiltVersion = '2.53'
         hiltCompilerVersion = '1.2.0'
         hiltWorkVersion = '1.2.0'

common/src/main/java/org/dash/wallet/common/data/BaseConfig.kt

@@ -133,6 +133,18 @@ abstract class BaseConfig(
         }
     }
 
+    /**
+     * Apply several key writes in ONE DataStore transaction.
+     *
+     * [set] is a transaction PER KEY, so a group of keys that is only
+     * meaningful together can be torn apart by a process death between two
+     * consecutive calls, leaving a half-written record that still reads as
+     * valid. Write such a group through this instead.
+     */
+    open suspend fun editPreferences(transform: (MutablePreferences) -> Unit) {
+        context.dataStore.edit { preferences -> transform(preferences) }
+    }
+
     /** Remove [key] entirely, so a later [get] returns null (not a stale value). */
     open suspend fun <T> remove(key: Preferences.Key<T>) {
         context.dataStore.edit { preferences ->

docs/kotlin-sdk-issues-to-file.md

@@ -123,3 +123,76 @@ balance so integrators can validate the amount and show the correct "available"
 balance overstates what's shieldable. Also: the error is a WalletOperation with the reason in the
 message string; a typed InsufficientFunds error (with available/required) would let integrators
 classify + display it without message-matching (relates to #14).
+
+## 16. DIP-15 contact address pools never grow — the 21st payment from any contact is invisible
+
+> **FILED** as dashpay/rust-dashcore#1032 (2026-09-16). The defect is in the `key-wallet` crate
+> in rust-dashcore, not in platform, so it went there rather than on dashpay/platform. Note the
+> root cause is narrower than first written: `AccountTypeToCheck` is a fieldless enum, so
+> `extended_public_key_for_account_type` structurally cannot identify which contact account is
+> meant. The fix belongs in `wallet_checker.rs`, which already holds the fully-keyed
+> `AccountType`.
+
+Live (testnet, two emulators, `rust-dashcore` rev `af88edf`): a DashPay contact chain watches
+exactly 20 addresses, indices 0-19, and **never extends**. The 21st payment a contact sends
+lands on index 20, which is never derived, so it is never matched by the filter scan and never
+enters the balance. It is unrecoverable by rescanning, because the address does not exist
+client-side. Money-visibility bug, not latency.
+
+Three things compose:
+
+- `key-wallet/src/wallet/helper.rs:612` — `extended_public_key_for_account_type` returns `None`
+  for `DashpayReceivingFunds` and `DashpayExternalAccount`, commented "Currently not retrieved
+  via this helper". So `key_source_for_account_type` yields `KeySource::NoKeySource`.
+- `key-wallet/src/transaction_checking/wallet_checker.rs:357` — `if matches!(key_source,
+  KeySource::NoKeySource) { continue; }` bails **before** `maintain_gap_limit`. Note
+  `mark_address_used` runs *above* the guard, so used flags advance normally while the window
+  does not. That divergence is what makes the bug hard to see.
+- `key-wallet/src/managed_account/managed_account_type.rs:726` and `:745` — both DashPay pools
+  are built via `single_pool(..., 20, ...)` with a **hardcoded literal**, not a named constant.
+  `maintain_gap_limit` with `highest_used == None` targets `gap_limit - 1` = 19, which is
+  exactly the observed pool.
+
+Measured on a wallet with 14 contact accounts: every one sits at top index 19 regardless of
+use. Driving six payments onto one chain advanced `isUsed` to index 5 while the top index
+stayed 19. A sliding pool (`highest_used + gap_limit`, as `maintain_gap_limit` documents)
+would have reached 0-25.
+
+Requests:
+
+- Make the checker reach `maintain_gap_limit` for the two DashPay pools. The material is
+  already persisted — every contact account row carries `accountExtendedPubKeyBytes`
+  (104 bytes) — and friend-chain receiving addresses derive from that xpub alone, so no seed
+  and no device unlock is needed. This is unfinished wiring, not a cryptographic limit.
+
+  Resolve it in `wallet_checker.rs`, **not** by teaching
+  `extended_public_key_for_account_type` to return the contact xpub. That helper takes
+  `AccountTypeToCheck`, which is a FIELDLESS enum: given only `DashpayReceivingFunds` it
+  cannot say *which* contact account is being asked about, so there is no correct value for it
+  to return. `wallet_checker.rs` already holds the fully-keyed `AccountType`, so either it
+  does the lookup itself, or it passes that keyed value down — but the fieldless helper cannot
+  be the place the decision is made.
+
+  (An earlier revision of this section asked for exactly that helper change. The note at the
+  top of this issue corrected the root cause; this bullet had not been brought in line, so the
+  issue as filed still points implementers at an API change that cannot fix the defect.
+  rust-dashcore#1032 needs a comment saying so.)
+- Replace the hardcoded `20` with a named constant. Note `DEFAULT_CONTACT_GAP_LIMIT = 10`
+  exists in `rs-platform-wallet/src/wallet/identity/crypto/dip14.rs:254` (quoting DIP-15's
+  recommendation) but has **no consumer** — neither it nor `derive_contact_payment_address`
+  is called outside its own module. Decide which value is intended and wire one of them.
+- Consider whether `mark_address_used` should stay above the `NoKeySource` guard. Advancing
+  used-state for a pool that can never be maintained produces a database that looks healthy
+  while the watch set is stale.
+
+Also affects `dashpayExternalAccount`, so a wallet stops tracking its own outgoing contact
+payments past index 19.
+
+**Reproduced end to end (testnet, 2026-09-16).** Twenty sequential contact payments filled
+indices 0-19 and were all received. The 21st, `b837bcb2f4f50c29502cd32549b3c369f28157b4ead872b7f41351cebb121ab0`
+(29,000 duffs to `yR3kpYnbcPipq4982WcEK2mCBrQXh7P39K`, block 1555216, 2 confirmations), was
+never seen. With the receiving wallet's service running and synced through block 1555217,
+its database holds zero `transactions` rows, zero `txos` rows, and — decisively — zero
+`core_addresses` rows for the destination address. The address was never derived, so the
+BIP158 scan could not match it. Coins are on-chain and spendable by the seed, but invisible
+to the wallet permanently and unrecoverable by rescan.