From 29e683b6853c8a4407148979841aad4e445fad3f Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 16:51:40 -0700 Subject: [PATCH 01/20] fix(sdk): keep the L1 funding gate on the height rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared spend gate (wallet->shielded and SDK L1 sends) opened on scanCaughtUpToTip, which since 2026-09-21 also accepts the iOS aggregate rule: a filter shortfall of up to 0.3% of chain height (~4,600 blocks on testnet). The pipeline-lag veto cannot see that gap while the wallet cursor is unknown (0), so a spend could open with thousands of blocks unscanned. When the cursor is known, the §34 stall parks it with the filter cursor and the veto closed the gate anyway, so requiring the height rule only closes the unsafe case. The gap is measured against the highest tip the snapshot knows (max(bestKnownTipHeight, filterTarget)): dash-spv restores the header target from the last peer report at engine start, which can sit below the stored header tip the filter target already names. The display predicate is unchanged. Raised by CodeRabbit on #1584. Co-Authored-By: Claude Opus 5.5 --- .../sdk/ShieldedBalanceServiceImpl.kt | 25 ++++++++++- .../platform/sdk/SdkL1SendServiceTest.kt | 25 +++++++++++ .../sdk/ShieldedBalanceServiceTest.kt | 42 ++++++++++++------- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt index df33953ffd..1838b0e51b 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt @@ -238,7 +238,11 @@ internal data class WalletFundingGate(val allowed: Boolean, val reason: String) * chain is at a known tip AND the wallet-relevant filter scan has caught * up to it. Deliberately NOT phase==SYNCED (a live shadow may never * report it) and NOT dashj parity (retired above); an all-zero snapshot - * or a filter scan short of the tip still keeps the gate closed. + * or a filter scan short of the tip still keeps the gate closed; + * - filter scan within [ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS] of the + * tip: the HEIGHT rule, even where the display's aggregate rule says + * caught up; + * - block pipeline drained: not [ShadowSyncProgress.blockPipelineLagging]. * * Wallet binding is NOT re-checked here: every caller already preflights * the bound wallet, and an unbound wallet cannot produce a non-IDLE @@ -253,7 +257,24 @@ internal fun evaluateWalletFundingGate( WalletFundingGate(false, "SDK L1 engine reported a sync error") !progress.scanCaughtUpToTip -> WalletFundingGate(false, "SDK L1 filter scan has not caught up to the chain tip yet") - // THIS GATE STARTS A SPEND, so it keeps the veto the DISPLAY predicate + // THIS GATE STARTS A SPEND, so it keeps the HEIGHT rule too. + // `scanCaughtUpToTip` also passes on the iOS aggregate rule, which + // tolerates a filter shortfall of up to 0.3% of chain height (~4,600 + // blocks on testnet) — right for a label, wrong for funding: an unscanned + // block can hold a spend of a UTXO this wallet still reads unspent, or a + // receive the balance is missing. The pipeline-lag veto below cannot cover + // that gap when the wallet cursor is unknown (0), and when the cursor IS + // known the §34 stall parks it with the filter cursor, so that veto + // closes the gate there anyway. The funds-safety argument of + // SCAN_TIP_TOLERANCE_BLOCKS needs the scan itself within tolerance. + // Measured against the HIGHEST tip the snapshot knows, not headerTarget + // alone: dash-spv restores the header target from the last peer report + // at engine start, which can sit below the stored header tip until a peer + // raises it, while the filter target is already that stored tip. + maxOf(progress.bestKnownTipHeight, progress.filterTarget) - progress.filterHeight > + ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS -> + WalletFundingGate(false, "SDK L1 filter scan is still short of the chain tip") + // …and it keeps the veto the DISPLAY predicate // dropped on 2026-09-21. `scanCaughtUpToTip` now also accepts the iOS // aggregate rule and no longer requires the block pipeline drained; that // is right for a label and wrong for funding. Matched blocks still being diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt index 46bbeea64f..a9a79a757e 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt @@ -572,6 +572,31 @@ class SdkL1SendServiceTest { assertEquals(0, source.sendCalls) } + @Test + fun gateClosed_aggregateRuleOnly_withUnknownWalletCursor_isNotBroadcast() = runBlocking { + // The §34 shape: filters 2,167 short of the tip, the aggregate at + // 99.954% (the DISPLAY's iOS rule calls that synced), and the wallet + // cursor unknown, so the pipeline veto has no evidence to act on. The + // unscanned blocks can hold a spend of a UTXO this wallet still reads + // unspent; the send gate must keep the height rule. + val source = readySource() + val aggregateOnly = caughtUpProgress().copy( + headerHeight = 1_558_166, headerTarget = 1_558_166, + filterHeight = 1_555_999, filterTarget = 1_558_166, overallPercent = 0.99954, + walletSyncedHeight = 0 + ) + assertTrue("fixture must read caught up for the display", aggregateOnly.scanCaughtUpToTip) + assertFalse("…with no pipeline evidence", aggregateOnly.blockPipelineLagging) + + val result = service(source, progress = { aggregateOnly }) + .sendToAddress(validAddress, amount, emptyWallet = false) + + assertTrue(result is SdkWriteResult.NotBroadcast) + val reason = (result as SdkWriteResult.NotBroadcast).reason + assertTrue(reason, reason.contains("short of the chain tip")) + assertEquals(0, source.sendCalls) + } + @Test fun gateOpen_noDashjParityRequired() = runBlocking { // Regression guard for the bricked-sends bug: the gate must NOT diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt index d4948a2419..7527f010b6 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt @@ -1569,23 +1569,35 @@ class ShieldedBalanceServiceTest { noAggregate.copy(filterHeight = 1_500_000 - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS - 1) ).allowed ) - // The same one-block-past-tolerance gap WITH an honest aggregate OPENS: - // 3 short of 1.5M is 99.9998%, and since 2026-09-21 the gate applies - // the iOS rule (>= 0.999). This is the change, stated as a test: the - // §34 stall (2,167 short, 99.954%) opens too. - assertTrue( - evaluateWalletFundingGate( - caughtUp.copy(filterHeight = 1_500_000 - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS - 1) - ).allowed + // The same one-block-past-tolerance gap WITH an honest aggregate stays + // CLOSED: 3 short of 1.5M is 99.9998%, which the DISPLAY's iOS rule + // (>= 0.999) calls caught up, but a spend needs the scan itself within + // tolerance. Same for the §34 stall (2,167 short, 99.954%) with the + // wallet cursor unknown — the one case the pipeline veto cannot see. + val pastTolerance = caughtUp.copy(filterHeight = 1_500_000 - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS - 1) + assertTrue("the display calls it caught up", pastTolerance.scanCaughtUpToTip) + val pastToleranceGate = evaluateWalletFundingGate(pastTolerance) + assertFalse(pastToleranceGate.allowed) + assertTrue(pastToleranceGate.reason, pastToleranceGate.reason.contains("short of the chain tip")) + val section34Stall = caughtUp.copy( + headerHeight = 1_558_166, headerTarget = 1_558_166, + filterHeight = 1_555_999, filterTarget = 1_558_166, overallPercent = 0.99954, + walletSyncedHeight = 0 ) - assertTrue( - evaluateWalletFundingGate( - caughtUp.copy( - headerHeight = 1_558_166, headerTarget = 1_558_166, - filterHeight = 1_555_999, filterTarget = 1_558_166, overallPercent = 0.99954 - ) - ).allowed + assertTrue("the display calls it caught up", section34Stall.scanCaughtUpToTip) + assertFalse("an unknown cursor is not lag", section34Stall.blockPipelineLagging) + assertFalse(evaluateWalletFundingGate(section34Stall).allowed) + // A stale header target restored at engine start (below the stored + // header tip the filter target already names): the gap is measured + // against the highest known tip, so 400 unscanned blocks stay closed. + val staleTarget = caughtUp.copy( + headerHeight = 1_500_500, headerTarget = 1_500_000, + filterHeight = 1_500_100, filterTarget = 1_500_500 ) + assertTrue("the display calls it caught up", staleTarget.scanCaughtUpToTip) + val staleGate = evaluateWalletFundingGate(staleTarget) + assertFalse(staleGate.allowed) + assertTrue(staleGate.reason, staleGate.reason.contains("short of the chain tip")) // THE VETO THE GATE KEEPS. A caught-up scan with the block pipeline // provably lagging — this wallet's own spends not yet applied to the From 4f85b93f84b7f15aae721c437826aecb823f95df Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 16:51:40 -0700 Subject: [PATCH 02/20] fix(sync): release the replay wake lock while the replay cannot progress Post-cutover a replay keeps the blockchain service alive and holds a PARTIAL_WAKE_LOCK with no timeout. While offline (NETWORK impediment) or once the filter-stall watchdog has spent its restarts on a wedge, the replay cannot move, yet the CPU stayed awake for as long as it was stuck. The service still stays up (stopping it re-walks from the durable watermark and loops through the replay restart alarm); only the lock is released. FilterStallWatchdogDecider.standingDown marks the wedge, cleared when the cursor moves, events resume, the scan is no longer behind, or the engine stops. TIME_TICK is not delivered while the CPU sleeps, so the impediment update re-acquires the lock when the replay is due again. Acquire and release run under one guard and re-check the inputs, and onDestroy retires the lock so a late network-callback coroutine cannot re-acquire it after the teardown's release. Raised by CodeRabbit on #1584. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 113 +++++++++++++----- .../service/SyncActivityIdleDetector.kt | 16 +++ .../platform/sdk/L1ShadowSyncService.kt | 43 ++++++- .../service/SyncActivityIdleDetectorTest.kt | 10 ++ .../platform/sdk/L1ShadowSyncServiceTest.kt | 46 +++++++ 5 files changed, 195 insertions(+), 33 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index 1932ccdd54..dc2573224e 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -992,6 +992,15 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { */ @Volatile private var replayWakeLockHeld = false + private val replayWakeLockGuard = Any() + /** + * Set first thing in [onDestroy]: from then on the replay lock is only ever + * released. A network-callback coroutine launched just before the callback + * was unregistered can still run, and its re-acquire landing after the + * teardown's release would hold a no-timeout lock until the process died. + */ + @Volatile + private var replayWakeLockRetired = false private var peerConnectivityListener: PeerConnectivityListener? = null private var nm: NotificationManager? = null private val impediments: MutableSet = EnumSet.noneOf( @@ -2462,7 +2471,13 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { committedHeight, durableHeightTracker.observe(durableHeight, System.currentTimeMillis()) ) - holdWakeLockWhileReplaying(replaying || engineStarting || durableLagging) + // A replay that cannot make progress keeps the service up but releases the CPU. + val replayHoldsLock = replayHoldsWakeLock( + replaying, + networkImpeded = impediments.contains(Impediment.NETWORK), + filterScanWedged = dashjHeldByCutover && l1ShadowSyncService.filterScanWedged() + ) + holdWakeLockWhileReplaying(replayHoldsLock || engineStarting || durableLagging) // Those two holds also keep the service in the foreground: the scan reads // synced while they last, and a demoted service would be stopped anyway. if (foregroundDemotionGate.onHold(sdkHoldsService(System.currentTimeMillis()))) { @@ -2513,32 +2528,6 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { onIdleVerdict() } } - - /** - * On the SDK path no peergroup start acquires the partial wake lock, - * so a replay that the idle rule now keeps alive could still be - * dozed. Hold the lock for exactly the span the service is kept - * alive for: acquired while `replaying`, released when the replay - * completes. The dashj path's own acquire/release (checkService / - * stopPeerGroup) is untouched; onDestroy releases either way. - */ - private fun holdWakeLockWhileReplaying(replaying: Boolean) { - if (!dashjHeldByCutover) return - val lock = wakeLock ?: return - try { - if (replaying && !replayWakeLockHeld) { - log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") - lock.acquire() - replayWakeLockHeld = true - } else if (!replaying && replayWakeLockHeld) { - log.info("replay / engine start complete — releasing the wake lock") - replayWakeLockHeld = false - if (lock.isHeld) lock.release() - } - } catch (t: Throwable) { - log.warn("wake lock hold/release for the replay failed", t) - } - } } inner class LocalBinder : Binder() { @@ -3458,6 +3447,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { override fun onDestroy() { log.info(".onDestroy()") + replayWakeLockRetired = true pendingDestroys.schedule(SystemClock.elapsedRealtime()) super.onDestroy() // unregister receivers on the main thread, if they were registered @@ -3622,11 +3612,13 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { // One release per owner: the replay's own acquisition first // (an interrupted replay never reached its completion // release), then whatever the dashj path still holds. - if (replayWakeLockHeld) { - replayWakeLockHeld = false - if (lock.isHeld) { - log.debug("releasing the replay's wake-lock acquisition") - lock.release() + synchronized(replayWakeLockGuard) { + if (replayWakeLockHeld) { + replayWakeLockHeld = false + if (lock.isHeld) { + log.debug("releasing the replay's wake-lock acquisition") + lock.release() + } } } if (lock.isHeld) { @@ -3790,8 +3782,65 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { ) } + /** + * On the SDK path no peergroup start acquires the partial wake lock, + * so a replay that the idle rule now keeps alive could still be + * dozed. Hold the lock for exactly the span the service is kept + * alive for: acquired while `replaying`, released when the replay + * completes — or while it cannot progress ([replayHoldsWakeLock]). + * The dashj path's own acquire/release (checkService / + * stopPeerGroup) is untouched; onDestroy releases either way. + * + * Called from the minute tick (main thread) and, to re-acquire after a + * reconnect, from the network callback (IO), so the held flag and the + * reference-counted lock change together under [replayWakeLockGuard]. + */ + private fun holdWakeLockWhileReplaying(replaying: Boolean) { + if (!dashjHeldByCutover) return + val lock = wakeLock ?: return + synchronized(replayWakeLockGuard) { + try { + if (replaying && !replayWakeLockHeld && !replayWakeLockRetired) { + log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") + lock.acquire() + replayWakeLockHeld = true + } else if (!replaying && replayWakeLockHeld && !replayWakeLockDue()) { + // replayWakeLockDue() re-reads the inputs under the guard: the + // tick decided from a reading the network callback may have + // overtaken (NETWORK cleared, lock re-acquired) since. + log.info("replay / engine start complete or unable to progress — releasing the wake lock") + replayWakeLockHeld = false + if (lock.isHeld) lock.release() + } + } catch (t: Throwable) { + log.warn("wake lock hold/release for the replay failed", t) + } + } + } + + /** + * The tick releases the replay lock while the replay cannot progress, but + * only the tick decides it, and ACTION_TIME_TICK is not delivered while + * the CPU sleeps. So once the impediment that released it clears, take it + * back here, or a screen-off replay that lost the network for a moment + * would stay asleep until something else woke the device. Acquire-only: + * releasing is still the tick's decision. + */ + private fun reacquireReplayWakeLockIfDue() { + if (!dashjHeldByCutover) return + if (replayWakeLockDue()) holdWakeLockWhileReplaying(true) + } + + /** The replay half of the tick's hold decision, read now. */ + private fun replayWakeLockDue(): Boolean = dashjHeldByCutover && replayHoldsWakeLock( + replaying = blockchainState?.replaying == true && !sdkBindBlocked(), + networkImpeded = impediments.contains(Impediment.NETWORK), + filterScanWedged = l1ShadowSyncService.filterScanWedged() + ) + private fun updateBlockchainStateImpediments() { blockchainStateDataProvider.updateImpediments(impediments) + reacquireReplayWakeLockIfDue() } // /** diff --git a/wallet/src/de/schildbach/wallet/service/SyncActivityIdleDetector.kt b/wallet/src/de/schildbach/wallet/service/SyncActivityIdleDetector.kt index 15fe12c270..d42e51620d 100644 --- a/wallet/src/de/schildbach/wallet/service/SyncActivityIdleDetector.kt +++ b/wallet/src/de/schildbach/wallet/service/SyncActivityIdleDetector.kt @@ -136,6 +136,22 @@ fun engineStartHoldsService( capMs: Long = ENGINE_START_KEEPALIVE_MS ): Boolean = startingSinceMs > 0L && nowMs - startingSinceMs < capMs +/** + * Whether an SDK replay should hold the partial wake lock. The idle guard + * keeps the service up for the whole replay, and that is still right when the + * replay cannot move: stopping it re-walks from the durable watermark on the + * next start. The CPU is another matter. The lock has no timeout, so a replay + * that cannot progress held it for as long as it stayed stuck: offline + * ([networkImpeded]: the engine waits for connections, as dashj did after + * `stopPeerGroup` released its own lock), or wedged with the filter-stall + * watchdog's restarts spent ([filterScanWedged]). Pure — host-testable. + */ +fun replayHoldsWakeLock( + replaying: Boolean, + networkImpeded: Boolean, + filterScanWedged: Boolean +): Boolean = replaying && !networkImpeded && !filterScanWedged + /** * How long after the service is created the SDK engine start may still be * pending — not yet begun, so [engineStartHoldsService] cannot see it — and diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index e265a1776a..2bdbc885a5 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -1305,6 +1305,29 @@ internal class FilterStallWatchdogDecider( /** When [Decision.SDK_FINAL_BATCH] was last reported; the restart hold is measured from here. */ private var finalBatchReportedAtMs = 0L + /** + * True while the watchdog has STOOD DOWN on a wedge: every restart it was + * willing to spend is gone and the cursor is still confirmed stalled (the + * [Decision.EXHAUSTED] report, or a later stall past the threshold with + * the budget already spent). Cleared the moment the cursor moves, the scan + * stops being behind, or the wallet-event stream shows the engine alive. + * Read by the blockchain service from another thread: a wedged engine is + * no reason to keep the CPU awake. + */ + @Volatile + var standingDown: Boolean = false + private set + + /** + * The engine stopped. [onCheck] only runs while it is up, so without this + * a wedge of the previous run would still read as current after a restart, + * until the new run's first check. The restart budget is NOT reset: it is + * per process, and a re-wedge sets [standingDown] again on its own. + */ + fun onEngineStopped() { + standingDown = false + } + /** * The threshold the last decision was taken against, and how long the * cursor had actually sat still. Exposed so the log can state what @@ -1376,11 +1399,13 @@ internal class FilterStallWatchdogDecider( // Caught up, or the target is not known yet. Nothing to stall on, // and the next spell of being behind must be timed from scratch. lastHeight = -1L + standingDown = false return Decision.NONE } if (filterHeight != lastHeight) { lastHeight = filterHeight lastAdvanceMs = nowMs + standingDown = false // A moving cursor ends the park. The final-batch report is PER // PARK, not per process: the next park earns its own report and // its own restart hold. (CodeRabbit on #1568: as a process-lifetime @@ -1437,6 +1462,7 @@ internal class FilterStallWatchdogDecider( // cheap guard against the case it names; the BACKOFF above is what // limits the damage when a stall is one the watchdog cannot cure. if (lastWalletEventMs > 0L && nowMs - lastWalletEventMs < threshold) { + standingDown = false return Decision.NONE } // A CONFIRMED stall, so it is now worth asking WHICH stall — because for @@ -1471,9 +1497,15 @@ internal class FilterStallWatchdogDecider( } !exhaustedReported -> { exhaustedReported = true + standingDown = true Decision.EXHAUSTED } - else -> Decision.NONE + else -> { + // The budget is per process: a later wedge gets no restart and + // no second report, but it is still a wedge. + standingDown = true + Decision.NONE + } } } @@ -2510,6 +2542,14 @@ class L1ShadowSyncService internal constructor( */ fun lastEngineActivityMs(): Long = lastWalletEventMs + /** + * Whether the filter-stall watchdog has stood down on a wedged filter + * cursor ([FilterStallWatchdogDecider.standingDown]): the replay reads as + * in progress, but nothing this process will do moves it. The blockchain + * service stops holding the CPU awake for it. + */ + fun filterScanWedged(): Boolean = filterStallDecider.standingDown + /** * When the in-flight engine start began, on [nowMs]'s clock; 0 when no * start is in progress. Set once [startIfEnabled] is committed to bringing @@ -3254,6 +3294,7 @@ class L1ShadowSyncService internal constructor( durableSyncedHeight = 0L } lastWalletEventMs = 0L // a fresh run must not inherit this run's liveness + filterStallDecider.onEngineStopped() // …nor its wedge (the watchdog only runs while up) lastStopAtMs = nowMs() stopCount++ log.info( diff --git a/wallet/test/de/schildbach/wallet/service/SyncActivityIdleDetectorTest.kt b/wallet/test/de/schildbach/wallet/service/SyncActivityIdleDetectorTest.kt index b1a42b4085..18e6affc8e 100644 --- a/wallet/test/de/schildbach/wallet/service/SyncActivityIdleDetectorTest.kt +++ b/wallet/test/de/schildbach/wallet/service/SyncActivityIdleDetectorTest.kt @@ -199,6 +199,16 @@ class SyncActivityIdleDetectorTest { assertFalse("custom cap", engineStartHoldsService(began, began + 5_000, capMs = 5_000)) } + @Test + fun replayWakeLock_heldOnlyWhileTheReplayCanProgress() { + assertTrue(replayHoldsWakeLock(replaying = true, networkImpeded = false, filterScanWedged = false)) + assertFalse("no replay", replayHoldsWakeLock(replaying = false, networkImpeded = false, filterScanWedged = false)) + // Offline: the engine waits for connections, as dashj did with its peergroup stopped. + assertFalse("offline", replayHoldsWakeLock(replaying = true, networkImpeded = true, filterScanWedged = false)) + // The filter-stall watchdog has spent its restarts on a wedge. + assertFalse("wedged", replayHoldsWakeLock(replaying = true, networkImpeded = false, filterScanWedged = true)) + } + @Test fun durableLag_holdsWhileBehindAndAdvancing() { // The field numbers at the 22:23 stop. diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt index 6a839ed57f..44bb796cf8 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt @@ -696,6 +696,52 @@ class L1ShadowSyncServiceTest { assertEquals(FilterStallWatchdogDecider.Decision.EXHAUSTED, check(120 * 60_000L)) } + @Test + fun filterStall_standsDownOnExhaustion_untilTheCursorMoves() { + // The replay wake lock reads this: a wedge every restart failed to cure + // must not keep the CPU awake, and the first sign of life must give the + // lock back to the replay. + val d = stallDecider() + val stuck = 1_500_000L // more than one commit batch behind + val target = 1_556_896L + fun check(atMs: Long, height: Long = stuck, lastEventMs: Long = 0L) = + d.onCheck(atMs, height, target, lastWalletEventMs = lastEventMs, walletSyncedHeight = stuck) + + check(0L) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(10 * 60_000L + 1)) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(30 * 60_000L + 2)) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(60 * 60_000L + 3)) + assertFalse("restarts still in hand: not standing down", d.standingDown) + assertEquals(FilterStallWatchdogDecider.Decision.EXHAUSTED, check(120 * 60_000L)) + assertTrue(d.standingDown) + + // Wallet events arriving again: the engine is alive. + assertEquals(FilterStallWatchdogDecider.Decision.NONE, check(121 * 60_000L, lastEventMs = 121 * 60_000L)) + assertFalse(d.standingDown) + + // A later wedge with the budget spent: no restart, no second report, + // but standing down again. + assertEquals(FilterStallWatchdogDecider.Decision.NONE, check(200 * 60_000L)) + assertTrue(d.standingDown) + + // The cursor moves: cleared. + check(201 * 60_000L, height = stuck + 1) + assertFalse(d.standingDown) + + // Not behind any more: cleared too. + check(300 * 60_000L, height = stuck + 1) + assertTrue(d.standingDown) + d.onCheck(301 * 60_000L, target, target, lastWalletEventMs = 0L) + assertFalse(d.standingDown) + + // An engine stop clears it: the next run's replay must not inherit it. + check(400 * 60_000L) + check(500 * 60_000L) + assertTrue(d.standingDown) + d.onEngineStopped() + assertFalse(d.standingDown) + } + @Test fun filterStall_anUnknownWatermarkIsTreatedAsFarFromTheTip() { From 92feae16f5f6495b0bac533a2f0d913a0d133b77 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 16:51:41 -0700 Subject: [PATCH 03/20] fix(sync): gate the bind-heal L1 start on the issuing service lifetime kickSdkEngines checked serviceLifetime once, after the bind join. A shutdown landing after startIfEnabled() returned (arm() does not suspend, so cancelling the kick cannot stop it) let arm() run after the shutdown's cancel; that waiter survived the teardown and started L1 on a later bind heal with no foreground service. The kick now arms only while its lifetime is current and passes a stillCurrent predicate that the waiter re-reads after the heal. arm and cancel are serialized and the shared job fields are volatile. Raised by CodeRabbit on #1584. Co-Authored-By: Claude Opus 5.5 --- .../service/platform/PlatformSyncService.kt | 14 ++++++++- .../service/platform/sdk/BindHealL1Starter.kt | 29 +++++++++++++++---- .../platform/sdk/BindHealL1StarterTest.kt | 26 +++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt index 87817d0db7..89f92f9a03 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt @@ -547,6 +547,9 @@ class PlatformSynchronizationService @Inject constructor( * [shutdown] also cancels the kick outright. */ private val serviceLifetime = AtomicLong(0) + // Written by the kick (init/resume), read and cancelled by shutdown and + // stopSdkEngines on other threads. + @Volatile private var startupKickJob: Job? = null private fun kickSdkEngines(startL1Engine: Boolean) { @@ -591,7 +594,16 @@ class PlatformSynchronizationService @Inject constructor( // Declined — most often "no wallet bound" after a failed startup // bind pass. The unlock receiver and the retry ladder only bind; // this is what starts L1 when they succeed (review, 2026-09-23). - if (started) bindHealL1Starter.cancel() else bindHealL1Starter.arm() + // The lifetime is re-checked here and again by the waiter after + // the heal: a shutdown can run between the check above and this + // line (startIfEnabled() has returned, and arm() does not + // suspend, so cancelling this kick does not stop it), and a + // waiter armed after the shutdown's cancel would outlive it. + when { + started -> bindHealL1Starter.cancel() + serviceLifetime.get() == lifetime -> + bindHealL1Starter.arm { serviceLifetime.get() == lifetime } + } } // Phase 5d follow-up: the post-cutover UI data source (balance // header / tx list / coins-received detection served from the diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt index d86ec9aa89..d95daf1d01 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt @@ -43,7 +43,12 @@ import org.slf4j.LoggerFactory * L1 once through the same idempotent entry point the startup path uses. * [cancel] is called from the service's shutdown, so a heal that lands after * teardown starts nothing; [serviceTearingDown] covers the window in which - * the shutdown has begun but not yet reached the cancel. + * the shutdown has begun but not yet reached the cancel. The caller's + * `stillCurrent` check covers the other order: an [arm] that lands AFTER the + * shutdown's [cancel] (the kick that armed it was past its last suspension + * point when the shutdown ran), whose waiter would otherwise survive the + * teardown and start L1 once the cleanup flag cleared, with no service. + * [arm] and [cancel] run on different threads, so they are serialized. * * Kept out of `PlatformSynchronizationService` so the ordering can be pinned * by a host test with plain flows. @@ -56,13 +61,19 @@ internal class BindHealL1Starter( ) { private val log = LoggerFactory.getLogger(BindHealL1Starter::class.java) + @Volatile private var job: Job? = null /** Whether a heal is currently being waited for. */ val isArmed: Boolean get() = job?.isActive == true - /** Arm for this service lifetime; a later arm replaces an earlier one. */ - fun arm() { + /** + * Arm for this service lifetime; a later arm replaces an earlier one. + * [stillCurrent] is re-read once the bind heals: false means the service + * lifetime that armed this has ended, and nothing starts. + */ + @Synchronized + fun arm(stillCurrent: () -> Boolean = { true }) { job?.cancel() job = scope.launch { // Nothing may escape this job. It is launched on a long-lived @@ -74,7 +85,7 @@ internal class BindHealL1Starter( // true (the production feed is a StateFlow, but the seam takes any // Flow, and [SdkBindRetryService] already defaults one to empty). try { - awaitHealAndStart() + awaitHealAndStart(stillCurrent) } catch (t: Throwable) { if (t is CancellationException) throw t log.warn("SDK bind-heal wait ended abnormally; nothing was started", t) @@ -82,8 +93,15 @@ internal class BindHealL1Starter( } } - private suspend fun awaitHealAndStart() { + private suspend fun awaitHealAndStart(stillCurrent: () -> Boolean) { bindEstablished.first { it } + if (!stillCurrent()) { + log.info( + "SDK bind healed after the service lifetime that waited for it ended — not starting " + + "L1; the next service start kicks it" + ) + return + } if (serviceTearingDown()) { log.info( "SDK bind healed while the blockchain service is tearing down — not starting L1; " + @@ -105,6 +123,7 @@ internal class BindHealL1Starter( } /** Stop waiting. Called from the service's shutdown and engine stop. */ + @Synchronized fun cancel() { job?.cancel() job = null diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt index 77961a9d51..864092d15c 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt @@ -97,6 +97,32 @@ class BindHealL1StarterTest { assertFalse(starter.isArmed) } + @Test + fun doesNotStartL1_whenTheLifetimeThatArmedItHasEnded() = runBlocking { + // CodeRabbit on #1584: the kick checks its service lifetime once, then + // arms without suspending, so a shutdown can cancel the starter just + // BEFORE the arm lands. That waiter outlives the teardown, and once the + // cleanup flag clears a later bind heal would start L1 with no service. + val bound = MutableStateFlow(false) + var starts = 0 + var lifetime = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + serviceTearingDown = { false }, // teardown already finished + startL1 = { starts++; true } + ) + + val armedIn = lifetime + starter.cancel() // the shutdown's cancel… + lifetime++ // …and its lifetime bump + starter.arm { lifetime == armedIn } // the kick's arm, landing late + bound.value = true + + assertEquals("a heal after the arming lifetime ended starts nothing", 0, starts) + assertFalse(starter.isArmed) + } + @Test fun aLaterArmReplacesAnEarlierOne() = runBlocking { val bound = MutableStateFlow(false) From 96c95a19676ec48ebb9fdadb6fa52ab060e652be Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 16:51:41 -0700 Subject: [PATCH 04/20] test(sync): fix the race-dependent restart and cutover UI tests PlatformSyncEngineRestartTest.resume_isRearmableAcrossRepeatedStopRestartCycles went stale within #1568: a later commit there made stopSdkEngines() cancel a kick still waiting on its bind, so three back-to-back cycles collapse into one or two starts. Each cycle now waits for its own start, and a new test pins the cancel-on-stop behaviour. Six CutoverUiDataServiceTest cases failed depending on timing: since #1570 refreshNativeSplit reads the receive address via withContext(Dispatchers.IO), a real-thread hop inside every runTest refresh, so runCurrent() returned before the refresh published. The read's dispatcher is now injectable (default IO); the test builder passes the runTest dispatcher, and the two parked-read rollback tests keep the real IO hop they gate on. Co-Authored-By: Claude Opus 5.5 --- .../platform/sdk/CutoverUiDataService.kt | 14 ++++++- .../platform/PlatformSyncEngineRestartTest.kt | 41 +++++++++++++++++-- .../platform/sdk/CutoverUiDataServiceTest.kt | 23 +++++++++-- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/CutoverUiDataService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/CutoverUiDataService.kt index 30669f1138..65d11f971e 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/CutoverUiDataService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/CutoverUiDataService.kt @@ -2597,7 +2597,17 @@ class CutoverUiDataService internal constructor( * launch-critical window instead of competing with first frame (the * ~100k-tx crash-loop started life as exactly that competition). */ - private val reconcileInitialDelayMs: Long = RECONCILE_INITIAL_DELAY_MS + private val reconcileInitialDelayMs: Long = RECONCILE_INITIAL_DELAY_MS, + /** + * Where [refreshNativeSplit] runs its blocking receive-address read. The + * refresh lanes publish everything else on [scope]'s dispatcher; a fixed + * [kotlinx.coroutines.Dispatchers.IO] here put one real-thread hop in the + * middle of every refresh, so a virtual-time test could assert before the + * refresh it had just triggered finished (and read the one before it). + * The live read ([sdkReceiveAddressLiveOrNull]) keeps its own IO hop: it + * blocks waiting for the binding and must never run on the caller's thread. + */ + private val receiveRefreshDispatcher: kotlinx.coroutines.CoroutineDispatcher = kotlinx.coroutines.Dispatchers.IO ) { @Inject constructor( @@ -4668,7 +4678,7 @@ class CutoverUiDataService internal constructor( // the read lock — NOT here. Taking it before this dispatch is what // let a queued refresh obtain a NEWER snapshot under an OLDER ticket // and then be rejected by its own fence ([receiveEngineReadLock]). - val (ticket, nextReceive) = kotlinx.coroutines.withContext(kotlinx.coroutines.Dispatchers.IO) { + val (ticket, nextReceive) = kotlinx.coroutines.withContext(receiveRefreshDispatcher) { readReceiveAddressOrdered { source.nextReceiveAddressOrNull(boundWalletIdHex) } } // The CACHE answer, not the revalidation verdict: this read feeds the diff --git a/wallet/test/de/schildbach/wallet/service/platform/PlatformSyncEngineRestartTest.kt b/wallet/test/de/schildbach/wallet/service/platform/PlatformSyncEngineRestartTest.kt index 4c6d2cbc42..1536e77db8 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/PlatformSyncEngineRestartTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/PlatformSyncEngineRestartTest.kt @@ -17,10 +17,14 @@ package de.schildbach.wallet.service.platform import de.schildbach.wallet.service.platform.sdk.L1ShadowSyncService +import de.schildbach.wallet.service.platform.sdk.SdkWalletBinder import de.schildbach.wallet.service.platform.sdk.ShieldedBalanceService +import de.schildbach.wallet.service.platform.sdk.WalletUnlock import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import org.junit.Test @@ -65,7 +69,9 @@ class PlatformSyncEngineRestartTest { /** The engine kick is asynchronous (`syncScope.launch { bindJob.join(); … }`). */ private val kickTimeoutMs = 10_000L - private fun service() = PlatformSynchronizationService( + private fun service( + sdkWalletBinder: SdkWalletBinder = mockk(relaxed = true) + ) = PlatformSynchronizationService( platform = mockk(relaxed = true), platformRepo = mockk(relaxed = true), analytics = mockk(relaxed = true), @@ -90,7 +96,7 @@ class PlatformSyncEngineRestartTest { sdkProfileQueries = mockk(relaxed = true), sdkUsernameQueries = mockk(relaxed = true), sdkIdentityVerifyQueries = mockk(relaxed = true), - sdkWalletBinder = mockk(relaxed = true), + sdkWalletBinder = sdkWalletBinder, nonInteractiveWalletUnlock = mockk(relaxed = true), l1ShadowSyncService = l1ShadowSyncService, shieldedBalanceService = shieldedBalanceService, @@ -184,18 +190,45 @@ class PlatformSyncEngineRestartTest { * teardown) reinstates the latch after the second teardown, which is * precisely how MO-995 presented: the first service restart of the * process looked fine, later ones silently had no SPV. + * + * Each cycle waits for its own kick to land before the next stop. A stop + * cancels a kick still waiting on its bind (see + * [stopSdkEngines_cancelsAKickStillWaitingOnItsBind]), so back-to-back + * cycles legitimately collapse into fewer starts; asserting three starts + * without waiting raced each kick against the next cycle's stop. */ @Test fun resume_isRearmableAcrossRepeatedStopRestartCycles() = runBlocking { val service = service() - repeat(3) { + for (cycle in 1..3) { service.stopSdkEngines() service.resume() + coVerify(timeout = kickTimeoutMs, exactly = cycle) { l1ShadowSyncService.startIfEnabled() } } - coVerify(timeout = kickTimeoutMs, exactly = 3) { l1ShadowSyncService.startIfEnabled() } coVerify(exactly = 3) { l1ShadowSyncService.stop() } Unit } + + /** + * The other half of that contract (review 2026-09-24): a stop that lands + * while a kick is still waiting on its bind cancels the kick, so no engine + * is started behind a service that has just stopped its engines. + */ + @Test + fun stopSdkEngines_cancelsAKickStillWaitingOnItsBind() = runBlocking { + val bind = CompletableDeferred() + val binder = mockk(relaxed = true) + every { binder.bindInBackground(any WalletUnlock?>()) } returns bind + val service = service(sdkWalletBinder = binder) + + service.resume() + service.stopSdkEngines() + bind.complete(Unit) + delay(500) + + coVerify(exactly = 0) { l1ShadowSyncService.startIfEnabled() } + Unit + } } diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/CutoverUiDataServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/CutoverUiDataServiceTest.kt index 3ff26d8878..d8e29f5118 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/CutoverUiDataServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/CutoverUiDataServiceTest.kt @@ -1354,7 +1354,15 @@ class CutoverUiDataServiceTest { /** MO-995: the bind-retry consultation the bound-wallet wait loop drives. */ retryBind: suspend () -> Unit = {}, /** [SdkWalletBinder.walletBindConfirmations]; default never advances. */ - walletBindConfirmations: kotlinx.coroutines.flow.StateFlow = MutableStateFlow(0L) + walletBindConfirmations: kotlinx.coroutines.flow.StateFlow = MutableStateFlow(0L), + /** + * The refresh's receive-address read. Under `runTest` it stays on the + * test scheduler, so `runCurrent()` returns only once the refresh it + * triggered has published; elsewhere the production IO hop. + */ + receiveRefreshDispatcher: kotlinx.coroutines.CoroutineDispatcher = + scope.coroutineContext[kotlin.coroutines.ContinuationInterceptor] as? kotlinx.coroutines.test.TestDispatcher + ?: Dispatchers.IO ) = CutoverUiDataService( source = source, dashPayConfig = dashPayConfig, @@ -1383,7 +1391,8 @@ class CutoverUiDataServiceTest { resolveMetadata = { txids -> metadata.filterKeys { it in txids } }, retryBind = retryBind, walletBindConfirmations = walletBindConfirmations, - nowMs = { now } + nowMs = { now }, + receiveRefreshDispatcher = receiveRefreshDispatcher ) @Test @@ -2771,8 +2780,10 @@ class CutoverUiDataServiceTest { val state = MutableStateFlow("CUT_OVER") val events = kotlinx.coroutines.flow.MutableSharedFlow(extraBufferCapacity = 4) val source = FakeSource(balanceDuffs = MutableStateFlow(123_456L)) + // The parked read below is gated on the IO worker thread it runs on. val service = buildService( - source, configWithMutableState(state), backgroundScope, txEvents = events + source, configWithMutableState(state), backgroundScope, txEvents = events, + receiveRefreshDispatcher = Dispatchers.IO ) service.start() assertTrue( @@ -2867,7 +2878,11 @@ class CutoverUiDataServiceTest { ) val events = kotlinx.coroutines.flow.MutableSharedFlow(extraBufferCapacity = 4) val source = FakeSource(balanceDuffs = MutableStateFlow(123_456L)) - val service = buildService(source, routed.config, backgroundScope, txEvents = events) + // The parked read below is gated on the IO worker thread it runs on. + val service = buildService( + source, routed.config, backgroundScope, txEvents = events, + receiveRefreshDispatcher = Dispatchers.IO + ) service.start() assertTrue( "the overlay must serve the engine address once the pipeline's first read lands", From ba72d273b3f92063068e390ee42bd021f2843e50 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 16:51:41 -0700 Subject: [PATCH 05/20] docs: drop the obsolete correction request from the issues record rust-dashcore#1032 as filed already tells implementers to resolve the key source in wallet_checker.rs from the fully keyed account. Raised by CodeRabbit on #1584. Co-Authored-By: Claude Opus 5.5 --- docs/kotlin-sdk-issues-to-file.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docs/kotlin-sdk-issues-to-file.md b/docs/kotlin-sdk-issues-to-file.md index be89851eee..e03f3909fc 100644 --- a/docs/kotlin-sdk-issues-to-file.md +++ b/docs/kotlin-sdk-issues-to-file.md @@ -172,11 +172,6 @@ Requests: 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` From 6138ffda3aa47ebf0b42fd4595a4a1626ac8d360 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 20 Oct 2026 23:37:23 -0700 Subject: [PATCH 06/20] fix(sync): re-take the replay wake lock when the filter wedge clears Once the filter-stall watchdog had spent its restarts, the minute tick released the replay wake lock. Only the tick or an impediment change re-acquired it, and ACTION_TIME_TICK is not delivered while the CPU sleeps, so a replay that recovered on its own ran unprotected until an unrelated wakeup. The replay hold moves into ReplayWakeLock. It keeps the guard, the teardown retirement, the re-check before release and the owner-only teardown release. FilterStallWatchdogDecider.standingDown is now backed by a StateFlow, and the blockchain service re-checks the lock the moment it clears. The clear is also event-driven: the progress feed (a filter cursor off the stand-down height) and the wallet-event tap end the stand-down at once, instead of waiting for the watchdog's one-minute delay, which also stalls while asleep. The spend gate's pipeline-lag veto now measures against the same highest-known tip as the height rule. headerTarget alone can sit below the stored tip after a restart. Regression test: ReplayWakeLockTest drives the real decider through exhaustion, release and recovery with no tick or network change. Raised by PastaClaw and CodeRabbit on #1632. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 110 +++------- .../wallet/service/ReplayWakeLock.kt | 147 +++++++++++++ .../platform/sdk/L1ShadowSyncService.kt | 52 ++++- .../sdk/ShieldedBalanceServiceImpl.kt | 15 +- .../wallet/service/ReplayWakeLockTest.kt | 194 ++++++++++++++++++ .../sdk/ShieldedBalanceServiceTest.kt | 13 ++ 6 files changed, 444 insertions(+), 87 deletions(-) create mode 100644 wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt create mode 100644 wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index dc2573224e..82688f47a2 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -982,25 +982,27 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { private var wakeLock: PowerManager.WakeLock? = null /** - * Whether the REPLAY's own acquisition of [wakeLock] is outstanding. - * Tracked apart from `isHeld` because the lock is reference-counted and - * shared with the dashj path: with the Tools dashj-sync diagnostic on, a - * peergroup and an SDK replay coexist, so `isHeld` cannot say whose count - * is whose. Each owner releases its own acquisition — the tick receiver - * on replay completion, [onDestroy] for an interrupted replay, where one - * release used to cover two acquisitions (review, 2026-09-22/23). + * The REPLAY's own acquisition of [wakeLock], tracked apart from `isHeld` + * because the lock is reference-counted and shared with the dashj path: + * with the Tools dashj-sync diagnostic on, a peergroup and an SDK replay + * coexist, so `isHeld` cannot say whose count is whose. Each owner releases + * its own acquisition — the tick receiver on replay completion, [onDestroy] + * for an interrupted replay, where one release used to cover two + * acquisitions (review, 2026-09-22/23). See [ReplayWakeLock]. */ - @Volatile - private var replayWakeLockHeld = false - private val replayWakeLockGuard = Any() - /** - * Set first thing in [onDestroy]: from then on the replay lock is only ever - * released. A network-callback coroutine launched just before the callback - * was unregistered can still run, and its re-acquire landing after the - * teardown's release would hold a no-timeout lock until the process died. - */ - @Volatile - private var replayWakeLockRetired = false + private val replayWakeLock = ReplayWakeLock( + lock = { + wakeLock?.let { lock -> + object : ReplayWakeLockHandle { + override val isHeld: Boolean get() = lock.isHeld + override fun acquire() = lock.acquire() + override fun release() = lock.release() + } + } + }, + sdkPath = { dashjHeldByCutover }, + replayDue = { replayWakeLockDue() } + ) private var peerConnectivityListener: PeerConnectivityListener? = null private var nm: NotificationManager? = null private val impediments: MutableSet = EnumSet.noneOf( @@ -2477,7 +2479,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { networkImpeded = impediments.contains(Impediment.NETWORK), filterScanWedged = dashjHeldByCutover && l1ShadowSyncService.filterScanWedged() ) - holdWakeLockWhileReplaying(replayHoldsLock || engineStarting || durableLagging) + replayWakeLock.hold(replayHoldsLock || engineStarting || durableLagging) // Those two holds also keep the service in the foreground: the scan reads // synced while they last, and a demoted service would be stopped anyway. if (foregroundDemotionGate.onHold(sdkHoldsService(System.currentTimeMillis()))) { @@ -2753,6 +2755,11 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { "dashjHeldByCutover={}, dashjSyncDiagnostic={})", dashjEngineMayStart, coordinatorAllowsDashj, dashjHeldByCutover, dashjSyncDiagnostic ) + // Re-take the replay wake lock the moment the filter wedge clears: + // ACTION_TIME_TICK is not delivered while the CPU sleeps (see ReplayWakeLock). + // Unconditional: the cutover gate can be re-resolved later in this + // service's life, and reacquireIfDue re-reads it itself. + replayWakeLock.observeWedge(serviceScope, l1ShadowSyncService.filterScanWedgedState) peerConnectivityListener = PeerConnectivityListener() broadcastPeerState(0) @@ -3447,7 +3454,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { override fun onDestroy() { log.info(".onDestroy()") - replayWakeLockRetired = true + replayWakeLock.retire() pendingDestroys.schedule(SystemClock.elapsedRealtime()) super.onDestroy() // unregister receivers on the main thread, if they were registered @@ -3612,15 +3619,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { // One release per owner: the replay's own acquisition first // (an interrupted replay never reached its completion // release), then whatever the dashj path still holds. - synchronized(replayWakeLockGuard) { - if (replayWakeLockHeld) { - replayWakeLockHeld = false - if (lock.isHeld) { - log.debug("releasing the replay's wake-lock acquisition") - lock.release() - } - } - } + replayWakeLock.releaseForTeardown() if (lock.isHeld) { log.debug("wakelock still held, releasing") lock.release() @@ -3782,55 +3781,6 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { ) } - /** - * On the SDK path no peergroup start acquires the partial wake lock, - * so a replay that the idle rule now keeps alive could still be - * dozed. Hold the lock for exactly the span the service is kept - * alive for: acquired while `replaying`, released when the replay - * completes — or while it cannot progress ([replayHoldsWakeLock]). - * The dashj path's own acquire/release (checkService / - * stopPeerGroup) is untouched; onDestroy releases either way. - * - * Called from the minute tick (main thread) and, to re-acquire after a - * reconnect, from the network callback (IO), so the held flag and the - * reference-counted lock change together under [replayWakeLockGuard]. - */ - private fun holdWakeLockWhileReplaying(replaying: Boolean) { - if (!dashjHeldByCutover) return - val lock = wakeLock ?: return - synchronized(replayWakeLockGuard) { - try { - if (replaying && !replayWakeLockHeld && !replayWakeLockRetired) { - log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") - lock.acquire() - replayWakeLockHeld = true - } else if (!replaying && replayWakeLockHeld && !replayWakeLockDue()) { - // replayWakeLockDue() re-reads the inputs under the guard: the - // tick decided from a reading the network callback may have - // overtaken (NETWORK cleared, lock re-acquired) since. - log.info("replay / engine start complete or unable to progress — releasing the wake lock") - replayWakeLockHeld = false - if (lock.isHeld) lock.release() - } - } catch (t: Throwable) { - log.warn("wake lock hold/release for the replay failed", t) - } - } - } - - /** - * The tick releases the replay lock while the replay cannot progress, but - * only the tick decides it, and ACTION_TIME_TICK is not delivered while - * the CPU sleeps. So once the impediment that released it clears, take it - * back here, or a screen-off replay that lost the network for a moment - * would stay asleep until something else woke the device. Acquire-only: - * releasing is still the tick's decision. - */ - private fun reacquireReplayWakeLockIfDue() { - if (!dashjHeldByCutover) return - if (replayWakeLockDue()) holdWakeLockWhileReplaying(true) - } - /** The replay half of the tick's hold decision, read now. */ private fun replayWakeLockDue(): Boolean = dashjHeldByCutover && replayHoldsWakeLock( replaying = blockchainState?.replaying == true && !sdkBindBlocked(), @@ -3840,7 +3790,9 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { private fun updateBlockchainStateImpediments() { blockchainStateDataProvider.updateImpediments(impediments) - reacquireReplayWakeLockIfDue() + // The network back (or any impediment change): the replay may be due + // the lock again, and the tick will not come while the CPU sleeps. + replayWakeLock.reacquireIfDue() } // /** diff --git a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt new file mode 100644 index 0000000000..4447dc6af0 --- /dev/null +++ b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt @@ -0,0 +1,147 @@ +/* + * Copyright 2026 Dash Core Group. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package de.schildbach.wallet.service + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.launch +import org.slf4j.LoggerFactory + +/** The slice of `PowerManager.WakeLock` the replay hold uses — a seam for host tests. */ +interface ReplayWakeLockHandle { + val isHeld: Boolean + fun acquire() + fun release() +} + +/** + * The SDK replay's own acquisition of the blockchain service's partial wake + * lock (no timeout). + * + * On the SDK path no peergroup start acquires the lock, so a replay that the + * idle rule keeps alive could still be dozed. The lock is held for the span + * the service is kept alive for, minus the stretches the replay cannot + * progress ([replayHoldsWakeLock]: offline, or wedged with the filter-stall + * watchdog's restarts spent). + * + * Who decides: + * - the minute tick acquires and releases ([hold]); + * - the moments a stuck replay can move again re-acquire at once + * ([reacquireIfDue]): the network impediment clearing, and the filter wedge + * clearing ([observeWedge]). ACTION_TIME_TICK is not delivered while the CPU + * sleeps, so waiting for the tick would leave a screen-off replay asleep + * until something unrelated woke the device. + * + * Tracked apart from `isHeld` because the lock is reference-counted and shared + * with the dashj path ([releaseForTeardown] releases only this owner's count). + * The tick runs on the main thread and the re-acquires on IO, so the held flag + * and the lock change together under one monitor. + * + * @param lock the service's wake lock; null before onCreate assigns it. + * @param sdkPath whether the SDK owns L1 (the dashj path manages its own hold). + * @param replayDue the replay half of the hold decision, read now. + */ +internal class ReplayWakeLock( + private val lock: () -> ReplayWakeLockHandle?, + private val sdkPath: () -> Boolean, + private val replayDue: () -> Boolean +) { + private val log = LoggerFactory.getLogger(ReplayWakeLock::class.java) + private val guard = Any() + + @Volatile + var held: Boolean = false + private set + + /** + * Set first thing in onDestroy: from then on the lock is only ever released. + * A network-callback coroutine launched just before the callback was + * unregistered can still run, and its re-acquire landing after the + * teardown's release would hold a no-timeout lock until the process died. + */ + @Volatile + private var retired = false + + /** The tick's decision: [hold] true while the replay (or an engine start / durable save) needs the CPU. */ + fun hold(hold: Boolean) { + if (!sdkPath()) return + val lock = lock() ?: return + synchronized(guard) { + try { + if (hold && !held && !retired) { + log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") + lock.acquire() + held = true + } else if (!hold && held && !replayDue()) { + // replayDue() re-reads the inputs under the guard: the tick + // decided from a reading an event may have overtaken (the + // network back, the wedge cleared, the lock re-acquired) since. + log.info("replay / engine start complete or unable to progress — releasing the wake lock") + held = false + if (lock.isHeld) lock.release() + } + } catch (t: Throwable) { + log.warn("wake lock hold/release for the replay failed", t) + } + } + } + + /** Acquire-only: take the lock back if the replay is due it now. Releasing stays the tick's decision. */ + fun reacquireIfDue() { + if (!sdkPath()) return + if (replayDue()) hold(true) + } + + /** + * Re-check the moment the filter wedge clears ([wedged] turns false after + * the first value): the cursor moved, wallet events resumed, the scan + * caught up, or the engine stopped. + */ + fun observeWedge(scope: CoroutineScope, wedged: Flow): Job = scope.launch { + wedged.distinctUntilChanged() + .drop(1) // the current value is not a transition + .filter { !it } + .collect { + log.info("the filter wedge cleared — re-checking the replay wake lock") + reacquireIfDue() + } + } + + /** onDestroy, first thing: no acquisition after this. */ + fun retire() { + retired = true + } + + /** onDestroy, cleanup: release this owner's count, if any. */ + fun releaseForTeardown() { + val lock = lock() ?: return + synchronized(guard) { + if (held) { + held = false + if (lock.isHeld) { + log.debug("releasing the replay's wake-lock acquisition") + lock.release() + } + } + } + } +} diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index 2bdbc885a5..6a07392f6f 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -1312,11 +1312,48 @@ internal class FilterStallWatchdogDecider( * the budget already spent). Cleared the moment the cursor moves, the scan * stops being behind, or the wallet-event stream shows the engine alive. * Read by the blockchain service from another thread: a wedged engine is - * no reason to keep the CPU awake. + * no reason to keep the CPU awake — and the moment it clears, the replay + * needs the CPU back at once ([standingDownState]). */ + var standingDown: Boolean + get() = _standingDown.value + private set(value) { + _standingDown.value = value + } + private val _standingDown = MutableStateFlow(false) + + /** [standingDown] as a flow, so a clear is an event and not only the next poll's reading. */ + val standingDownState: StateFlow = _standingDown.asStateFlow() + + /** The filter cursor the watchdog stood down at; written before [standingDown] goes true. */ @Volatile - var standingDown: Boolean = false - private set + private var stoodDownAtHeight: Long = -1L + + private fun standDownAt(filterHeight: Long) { + stoodDownAtHeight = filterHeight + standingDown = true + } + + /** + * The event-driven half of the clear. [onCheck] runs on the watchdog's + * one-minute delay, which (like ACTION_TIME_TICK) only advances while the + * CPU is awake, so a recovery noticed there could wait out several brief + * wakes with the replay unprotected. The progress feed and the wallet-event + * tap see the engine move as it happens: a filter cursor off the height the + * watchdog stood down at, or any wallet event (the same liveness rule + * [onCheck] applies), ends the stand-down at once. Only [standingDown] is + * touched, so this is safe from those collectors' threads; [onCheck] + * re-times the next spell itself. A 0 height is a snapshot with no filter + * position (IDLE / CONNECTING), not movement. + */ + fun onFilterProgress(filterHeight: Long) { + if (standingDown && filterHeight > 0L && filterHeight != stoodDownAtHeight) standingDown = false + } + + /** See [onFilterProgress]. */ + fun onWalletEvent() { + if (standingDown) standingDown = false + } /** * The engine stopped. [onCheck] only runs while it is up, so without this @@ -1497,13 +1534,13 @@ internal class FilterStallWatchdogDecider( } !exhaustedReported -> { exhaustedReported = true - standingDown = true + standDownAt(filterHeight) Decision.EXHAUSTED } else -> { // The budget is per process: a later wedge gets no restart and // no second report, but it is still a wedge. - standingDown = true + standDownAt(filterHeight) Decision.NONE } } @@ -2550,6 +2587,9 @@ class L1ShadowSyncService internal constructor( */ fun filterScanWedged(): Boolean = filterStallDecider.standingDown + /** [filterScanWedged] as a flow: the blockchain service re-takes the replay wake lock when it clears. */ + val filterScanWedgedState: StateFlow get() = filterStallDecider.standingDownState + /** * When the in-flight engine start began, on [nowMs]'s clock; 0 when no * start is in progress. Set once [startIfEnabled] is committed to bringing @@ -3469,6 +3509,7 @@ class L1ShadowSyncService internal constructor( sessionFilterStart = sessionFilterStart ) _progress.value = mapped + filterStallDecider.onFilterProgress(mapped.filterHeight) // Verification verdict from the chain state: still // scanning until SYNCED, then "probing" until a parity // probe upgrades to VERIFIED (which progress ticks must @@ -3599,6 +3640,7 @@ class L1ShadowSyncService internal constructor( // any parsing: an event the parsers all drop still proves // the engine is alive and producing. lastWalletEventMs = nowMs() + filterStallDecider.onWalletEvent() // Chainlock feed first: it rides the SAME event stream but // on OTHER variants (ChainLockProcessed / BlockProcessed), // which parseL1TxEvent drops. Monotonic — a replayed or diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt index 1838b0e51b..0edc787d45 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt @@ -271,8 +271,7 @@ internal fun evaluateWalletFundingGate( // alone: dash-spv restores the header target from the last peer report // at engine start, which can sit below the stored header tip until a peer // raises it, while the filter target is already that stored tip. - maxOf(progress.bestKnownTipHeight, progress.filterTarget) - progress.filterHeight > - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS -> + fundingTipHeight(progress) - progress.filterHeight > ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS -> WalletFundingGate(false, "SDK L1 filter scan is still short of the chain tip") // …and it keeps the veto the DISPLAY predicate // dropped on 2026-09-21. `scanCaughtUpToTip` now also accepts the iOS @@ -284,11 +283,21 @@ internal fun evaluateWalletFundingGate( // The wallet cursor within tolerance of the tip is the evidence those // blocks are through; until then the gate stays closed. Unknown cursor // (0) never counts as lagging, so this cannot wedge a wallet at the tip. - progress.blockPipelineLagging -> + // Measured against the same highest-known tip as the height rule above: + // `blockPipelineLagging` reads headerTarget alone, which a restart can + // leave below the stored tip while the filters already reach it. + progress.blockPipelineLagging || + (progress.walletSyncedHeight > 0 && + fundingTipHeight(progress) - progress.walletSyncedHeight > + ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS) -> WalletFundingGate(false, "SDK L1 block pipeline is still processing matched blocks") else -> WalletFundingGate(true, "SDK L1 filter scan caught up to the chain tip") } +/** The highest chain tip a progress snapshot knows: the reference both spend-gate height checks measure against. */ +private fun fundingTipHeight(progress: ShadowSyncProgress): Long = + maxOf(progress.bestKnownTipHeight, progress.filterTarget) + /** * Pure [ShieldedSyncStatus] decision — host-testable. [ready] is the bring-up * latch; [firstPassCompleted] latches once a full sync pass has finished since diff --git a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt new file mode 100644 index 0000000000..9b6a8e49c7 --- /dev/null +++ b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt @@ -0,0 +1,194 @@ +/* + * Copyright 2026 Dash Core Group. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package de.schildbach.wallet.service + +import de.schildbach.wallet.service.platform.sdk.FilterStallWatchdogDecider +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The replay's own hold on the service wake lock ([ReplayWakeLock]), wired to + * the real filter-stall watchdog decider. + * + * Review on #1632 (PastaClaw, CodeRabbit): once the watchdog had spent its + * restarts the minute tick released the lock, and only the tick or a network + * change could take it back. A replay that recovered on its own (the cursor + * moved) ran without the lock, and ACTION_TIME_TICK is not delivered while + * the CPU sleeps, so a screen-off replay stayed asleep until something + * unrelated woke the device. + * + * Unconfined scope so a flow emission runs the observer synchronously. + */ +class ReplayWakeLockTest { + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined) + + @After + fun tearDown() = scope.cancel() + + private class FakeLock : ReplayWakeLockHandle { + var count = 0 + var acquisitions = 0 + override val isHeld: Boolean get() = count > 0 + override fun acquire() { + count++ + acquisitions++ + } + override fun release() { + count-- + } + } + + private val lock = FakeLock() + private val decider = FilterStallWatchdogDecider(stallThresholdMs = 10 * 60_000L, maxRestarts = 3) + private var networkImpeded = false + + // Mirrors BlockchainServiceImpl.replayWakeLockDue() with a replay in progress. + private fun replayDue() = replayHoldsWakeLock( + replaying = true, + networkImpeded = networkImpeded, + filterScanWedged = decider.standingDown + ) + + private val replayWakeLock = ReplayWakeLock( + lock = { lock }, + sdkPath = { true }, + replayDue = ::replayDue + ) + + // The minute tick's call, with no engine start or durable save in flight. + private fun tick() = replayWakeLock.hold(replayDue()) + + private val stuck = 1_500_000L // more than one commit batch behind + private val target = 1_556_896L + + private fun check(atMs: Long, height: Long = stuck) = + decider.onCheck(atMs, height, target, lastWalletEventMs = 0L, walletSyncedHeight = stuck) + + /** The restart ladder of 10 / 20 / 30 minutes, then the stand-down. */ + private fun exhaustTheWatchdog() { + check(0L) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(10 * 60_000L + 1)) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(30 * 60_000L + 2)) + assertEquals(FilterStallWatchdogDecider.Decision.RESTART, check(60 * 60_000L + 3)) + assertEquals(FilterStallWatchdogDecider.Decision.EXHAUSTED, check(120 * 60_000L)) + assertTrue(decider.standingDown) + } + + @Test + fun aRecoveryFromAnExhaustedWedge_retakesTheLock_withoutATickOrANetworkChange() { + replayWakeLock.observeWedge(scope, decider.standingDownState) + tick() + assertTrue("the replay holds the lock", lock.isHeld) + + exhaustTheWatchdog() + tick() + assertFalse("a wedge the watchdog gave up on does not keep the CPU awake", lock.isHeld) + assertFalse(replayWakeLock.held) + + // The scan recovers on its own: the cursor moves. No tick, no network event. + check(121 * 60_000L, height = stuck + 1) + assertFalse(decider.standingDown) + assertTrue("the first sign of life gives the lock back to the replay", lock.isHeld) + assertEquals(2, lock.acquisitions) + assertTrue(replayWakeLock.held) + + // The next tick agrees; it neither double-acquires nor releases. + tick() + assertEquals(1, lock.count) + } + + @Test + fun theEngineMovingRetakesTheLock_beforeTheWatchdogsNextCheck() { + // The watchdog's own check runs on a one-minute delay that only advances + // while awake; the progress feed and the event tap are what see the + // engine move during a brief wake. + replayWakeLock.observeWedge(scope, decider.standingDownState) + exhaustTheWatchdog() + tick() + assertFalse(lock.isHeld) + + decider.onFilterProgress(stuck) + assertFalse("the same cursor is still the wedge", lock.isHeld) + decider.onFilterProgress(0L) + assertFalse("a snapshot with no filter position is not movement", lock.isHeld) + decider.onFilterProgress(stuck + 1) + assertTrue("the cursor moved: the lock is back", lock.isHeld) + + // A later stand-down, ended by a wallet event instead. + check(200 * 60_000L, height = stuck + 1) + check(300 * 60_000L, height = stuck + 1) + assertTrue(decider.standingDown) + tick() + assertFalse(lock.isHeld) + decider.onWalletEvent() + assertTrue(lock.isHeld) + } + + @Test + fun aWedgeClearedWhileOffline_waitsForTheNetwork() { + replayWakeLock.observeWedge(scope, decider.standingDownState) + exhaustTheWatchdog() + networkImpeded = true + tick() + assertFalse(lock.isHeld) + + check(121 * 60_000L, height = stuck + 1) + assertFalse("still offline: nothing to hold the CPU for", lock.isHeld) + + // The network back: the impediment update re-checks. + networkImpeded = false + replayWakeLock.reacquireIfDue() + assertTrue(lock.isHeld) + } + + @Test + fun aWedgeClearingAfterTeardownBegan_acquiresNothing() { + replayWakeLock.observeWedge(scope, decider.standingDownState) + tick() + exhaustTheWatchdog() + tick() + assertFalse(lock.isHeld) + + replayWakeLock.retire() + // The engine stop of the teardown clears the wedge. + decider.onEngineStopped() + assertFalse("no acquisition after onDestroy began", lock.isHeld) + assertEquals(1, lock.acquisitions) + } + + @Test + fun teardownReleasesOnlyTheReplaysOwnCount() { + lock.acquire() // the dashj path's own acquisition + tick() + assertEquals(2, lock.count) + + replayWakeLock.retire() + replayWakeLock.releaseForTeardown() + assertEquals(1, lock.count) + assertFalse(replayWakeLock.held) + replayWakeLock.releaseForTeardown() + assertEquals("idempotent", 1, lock.count) + } +} diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt index 7527f010b6..753424b6b7 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt @@ -1598,6 +1598,19 @@ class ShieldedBalanceServiceTest { val staleGate = evaluateWalletFundingGate(staleTarget) assertFalse(staleGate.allowed) assertTrue(staleGate.reason, staleGate.reason.contains("short of the chain tip")) + // …and the pipeline veto measures against the same tip: filters at the + // stored tip, the wallet cursor level with the stale target (500 back). + val stalePipeline = staleTarget.copy(filterHeight = 1_500_499, walletSyncedHeight = 1_500_000) + assertFalse("headerTarget alone reads no lag", stalePipeline.blockPipelineLagging) + val stalePipelineGate = evaluateWalletFundingGate(stalePipeline) + assertFalse(stalePipelineGate.allowed) + assertTrue(stalePipelineGate.reason, stalePipelineGate.reason.contains("processing matched blocks")) + assertTrue( + "a cursor within tolerance of the highest-known tip opens, stale target or not", + evaluateWalletFundingGate( + stalePipeline.copy(walletSyncedHeight = 1_500_500 - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS) + ).allowed + ) // THE VETO THE GATE KEEPS. A caught-up scan with the block pipeline // provably lagging — this wallet's own spends not yet applied to the From 463740517f5101e8c0cea429e65e2317b591cd3a Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 21 Oct 2026 00:15:20 -0700 Subject: [PATCH 07/20] fix(sync): seed the NETWORK impediment from connectivity at service start The blockchain service's NETWORK impediment was only ever set by its network callback, and a passive registerNetworkCallback reports nothing while no network matches. A service created while already offline therefore read as online, and a replay persisted from the previous session held the no-timeout replay wake lock in airplane mode. onCreate now seeds the impediment from the default network's capabilities (internet + validated, the callback's own rule) before it registers the callback, so every callback update overrides the seed. An unreadable connectivity state answers "usable", the old assumption. Raised by PastaClaw on #1632. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 11 ++ .../wallet/service/NetworkAvailability.kt | 43 +++++++ .../wallet/service/NetworkAvailabilityTest.kt | 112 ++++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt create mode 100644 wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index 82688f47a2..e1ba8ba962 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -2990,6 +2990,17 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) .build() + // Seed the NETWORK impediment from connectivity as it stands + // now: the registration below reports nothing while no network + // matches, so a service created offline read as online — and a + // persisted replay held the no-timeout wake lock in airplane + // mode. Seeded BEFORE registering, so every callback update + // (all of them run after the registration) overrides it. + if (!hasUsableNetworkNow(connectivityManager)) { + log.info("no usable network at service start — NETWORK impediment set until one appears") + impediments.add(Impediment.NETWORK) + updateBlockchainStateImpediments() + } connectivityManager.registerNetworkCallback(networkRequest, networkCallback) networkCallbackRegistered = true log.info("network callback registered with NetworkRequest: {}", networkCallback) diff --git a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt new file mode 100644 index 0000000000..c594b8362d --- /dev/null +++ b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt @@ -0,0 +1,43 @@ +/* + * Copyright 2026 Dash Core Group. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package de.schildbach.wallet.service + +import android.net.ConnectivityManager +import android.net.NetworkCapabilities + +/** + * Whether connectivity AS IT STANDS NOW offers a network the blockchain + * service can sync over: the default network, with internet and validated — + * the same capabilities the service's network callback requires. + * + * The service seeds its NETWORK impediment from this before it registers that + * callback. A passive `registerNetworkCallback` reports nothing while no + * network matches (no `onAvailable`, `onLost` or `onCapabilitiesChanged`; + * `onUnavailable` is for `requestNetwork`), so a service created while already + * offline used to read as online until some network came and went. Never + * throws: an unreadable state answers "usable", the old assumption, so this can + * only ever add an impediment the platform confirmed. + */ +fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try { + val capabilities = connectivityManager.activeNetwork?.let(connectivityManager::getNetworkCapabilities) + capabilities != null && + capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) && + capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) +} catch (t: Throwable) { + true +} diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt new file mode 100644 index 0000000000..dfa857219a --- /dev/null +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -0,0 +1,112 @@ +/* + * Copyright 2026 Dash Core Group. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package de.schildbach.wallet.service + +import android.app.Application +import android.content.Context +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowNetworkCapabilities + +/** + * Review on #1632 (PastaClaw): the blockchain service's NETWORK impediment was + * only ever set by its network callback, and a passive registration reports + * nothing while no network matches. A service created while already offline + * therefore read as online, and a persisted replay held the no-timeout wake + * lock in airplane mode. The service now seeds the impediment from + * [hasUsableNetworkNow] before it registers. + */ +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class, sdk = [29], manifest = Config.NONE) +class NetworkAvailabilityTest { + private val connectivityManager = + RuntimeEnvironment.getApplication().getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager + + private fun defaultNetworkWith(vararg capabilities: Int) { + shadowOf(connectivityManager).setDefaultNetworkActive(true) + val network = connectivityManager.activeNetwork!! + val nc = ShadowNetworkCapabilities.newInstance() + capabilities.forEach { shadowOf(nc).addCapability(it) } + shadowOf(connectivityManager).setNetworkCapabilities(network, nc) + } + + @Test + fun noDefaultNetwork_isNotUsable() { + shadowOf(connectivityManager).setDefaultNetworkActive(false) + assertFalse(hasUsableNetworkNow(connectivityManager)) + } + + @Test + fun aValidatedInternetNetwork_isUsable() { + defaultNetworkWith(NetworkCapabilities.NET_CAPABILITY_INTERNET, NetworkCapabilities.NET_CAPABILITY_VALIDATED) + assertTrue(hasUsableNetworkNow(connectivityManager)) + } + + @Test + fun aNetworkThatIsNotValidated_isNotUsable() { + // Captive portal / no upstream: the same rule the network callback applies. + defaultNetworkWith(NetworkCapabilities.NET_CAPABILITY_INTERNET) + assertFalse(hasUsableNetworkNow(connectivityManager)) + } + + @Test + fun aServiceCreatedOffline_neverTakesTheReplayLock_untilANetworkAppears() { + shadowOf(connectivityManager).setDefaultNetworkActive(false) + + // What onCreate does before registering the network callback. + val networkImpeded = !hasUsableNetworkNow(connectivityManager) + assertTrue("seeded offline", networkImpeded) + + var impeded = networkImpeded + var acquisitions = 0 + var count = 0 + val lock = object : ReplayWakeLockHandle { + override val isHeld: Boolean get() = count > 0 + override fun acquire() { + count++ + acquisitions++ + } + override fun release() { + count-- + } + } + // A replay persisted from the previous session is still in progress. + val replayWakeLock = ReplayWakeLock( + lock = { lock }, + sdkPath = { true }, + replayDue = { replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false) } + ) + replayWakeLock.hold(replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false)) + assertFalse("offline from the start: no wake lock for a replay that cannot move", lock.isHeld) + assertEquals(0, acquisitions) + + // A network appears: the callback clears the impediment and re-checks. + impeded = false + replayWakeLock.reacquireIfDue() + assertTrue(lock.isHeld) + } +} From 505eb301bdeb5ca2f7495efe46ff93cb21c884cf Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 21 Oct 2026 00:28:10 -0700 Subject: [PATCH 08/20] refactor(sync): one definition of the replay wake-lock hold rule The minute tick computed the replay's hold decision inline, while ReplayWakeLock re-read replayWakeLockDue() before releasing. They agreed, but a later edit to one copy alone could leave the tick asking to release while the re-check still read "due", refusing the release on every tick and keeping the no-timeout lock held. The tick now uses replayWakeLockDue() too. Co-Authored-By: Claude Opus 5.5 --- .../schildbach/wallet/service/BlockchainServiceImpl.kt | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index e1ba8ba962..280540e560 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -2474,12 +2474,9 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { durableHeightTracker.observe(durableHeight, System.currentTimeMillis()) ) // A replay that cannot make progress keeps the service up but releases the CPU. - val replayHoldsLock = replayHoldsWakeLock( - replaying, - networkImpeded = impediments.contains(Impediment.NETWORK), - filterScanWedged = dashjHeldByCutover && l1ShadowSyncService.filterScanWedged() - ) - replayWakeLock.hold(replayHoldsLock || engineStarting || durableLagging) + // The same predicate ReplayWakeLock re-reads before releasing — one definition, + // or the two could disagree and refuse the release on every tick. + replayWakeLock.hold(replayWakeLockDue() || engineStarting || durableLagging) // Those two holds also keep the service in the foreground: the scan reads // synced while they last, and a demoted service would be stopped anyway. if (foregroundDemotionGate.onHold(sdkHoldsService(System.currentTimeMillis()))) { From 047acc3812fc414770e2a1cd2b21d30e20422695 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 21 Oct 2026 09:34:36 -0700 Subject: [PATCH 09/20] fix(sync): an ended lifetime's kick cannot replace or cancel the current bind-heal waiter A startup kick passes its lifetime check, then shutdown and the next service start can complete before it reaches arm(). arm() cancelled the waiter the new lifetime had armed and installed one that refuses to start, leaving the running service with no L1 engine and nothing waiting to start it. The kick's cancel() on a successful start had the same hole. arm() now checks stillCurrent first, under the monitor, and ignores an obsolete arm; the kick's cancel takes the same check (shutdown's cancel stays unconditional). The waiter still re-checks after the heal. Raised by PastaClaw on #1632. Co-Authored-By: Claude Opus 5.5 --- .../service/platform/PlatformSyncService.kt | 20 ++--- .../service/platform/sdk/BindHealL1Starter.kt | 25 +++++- .../platform/sdk/BindHealL1StarterTest.kt | 79 +++++++++++++++++++ 3 files changed, 110 insertions(+), 14 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt index 89f92f9a03..de8b7dc169 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt @@ -594,16 +594,16 @@ class PlatformSynchronizationService @Inject constructor( // Declined — most often "no wallet bound" after a failed startup // bind pass. The unlock receiver and the retry ladder only bind; // this is what starts L1 when they succeed (review, 2026-09-23). - // The lifetime is re-checked here and again by the waiter after - // the heal: a shutdown can run between the check above and this - // line (startIfEnabled() has returned, and arm() does not - // suspend, so cancelling this kick does not stop it), and a - // waiter armed after the shutdown's cancel would outlive it. - when { - started -> bindHealL1Starter.cancel() - serviceLifetime.get() == lifetime -> - bindHealL1Starter.arm { serviceLifetime.get() == lifetime } - } + // A shutdown and even the next service start can run between the + // lifetime check above and this line (startIfEnabled() has + // returned, and arm()/cancel() do not suspend, so cancelling this + // kick does not stop it). Both calls therefore re-check the + // lifetime under the starter's monitor, and the waiter re-checks + // it after the heal: a kick of an ended lifetime can neither arm a + // waiter that outlives its service nor replace or cancel the + // waiter the current lifetime armed. + val stillCurrent = { serviceLifetime.get() == lifetime } + if (started) bindHealL1Starter.cancel(stillCurrent) else bindHealL1Starter.arm(stillCurrent) } // Phase 5d follow-up: the post-cutover UI data source (balance // header / tx list / coins-received detection served from the diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt index d95daf1d01..8f0b0dee3f 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt @@ -69,11 +69,22 @@ internal class BindHealL1Starter( /** * Arm for this service lifetime; a later arm replaces an earlier one. - * [stillCurrent] is re-read once the bind heals: false means the service - * lifetime that armed this has ended, and nothing starts. + * + * [stillCurrent] says whether the service lifetime asking is still the + * current one. It is checked FIRST, under the monitor: a startup kick of a + * lifetime that has since ended can reach this after the next lifetime has + * armed its own waiter (the kick passed its lifetime check, then + * shutdown and the next start ran), and replacing that valid waiter with an + * obsolete one would leave the running service with no L1 engine and nothing + * waiting to start it. An obsolete arm therefore touches nothing. It is + * re-read once the bind heals, for a lifetime that ends while waiting. */ @Synchronized fun arm(stillCurrent: () -> Boolean = { true }) { + if (!stillCurrent()) { + log.info("SDK bind-heal arm from an ended service lifetime ignored; the current waiter is kept") + return + } job?.cancel() job = scope.launch { // Nothing may escape this job. It is launched on a long-lived @@ -122,9 +133,15 @@ internal class BindHealL1Starter( ) } - /** Stop waiting. Called from the service's shutdown and engine stop. */ + /** + * Stop waiting. Called unconditionally from the service's shutdown and + * engine stop, and with [stillCurrent] from a startup kick whose own start + * succeeded: like [arm], a kick of an ended lifetime must not cancel the + * waiter the current lifetime armed. + */ @Synchronized - fun cancel() { + fun cancel(stillCurrent: () -> Boolean = { true }) { + if (!stillCurrent()) return job?.cancel() job = null } diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt index 864092d15c..8524c72d25 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt @@ -123,6 +123,85 @@ class BindHealL1StarterTest { assertFalse(starter.isArmed) } + @Test + fun anObsoleteArm_keepsTheCurrentLifetimesWaiter_andTheHealStartsL1() = runBlocking { + // PastaClaw on #1632: an old startup kick passes its lifetime check, is + // descheduled while shutdown and the next service start complete, then + // reaches arm() after the new lifetime armed its own waiter. It used to + // cancel that waiter and install one that refuses to start. + val bound = MutableStateFlow(false) + var starts = 0 + var lifetime = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + serviceTearingDown = { false }, + startL1 = { starts++; true } + ) + + val oldLifetime = lifetime + lifetime++ // shutdown of the old lifetime + val newLifetime = lifetime + starter.arm { lifetime == newLifetime } // the new lifetime's waiter + starter.arm { lifetime == oldLifetime } // the old kick, landing late + + assertTrue("the current waiter is kept", starter.isArmed) + bound.value = true + assertEquals("the heal starts L1 for the running service", 1, starts) + } + + @Test + fun anObsoleteCancel_keepsTheCurrentLifetimesWaiter() = runBlocking { + // The same race on the other branch: an old kick whose own start + // succeeded cancels "its" waiter, which is now the new lifetime's. + val bound = MutableStateFlow(false) + var starts = 0 + var lifetime = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + serviceTearingDown = { false }, + startL1 = { starts++; true } + ) + + val oldLifetime = lifetime + lifetime++ + val newLifetime = lifetime + starter.arm { lifetime == newLifetime } + starter.cancel { lifetime == oldLifetime } + + assertTrue(starter.isArmed) + bound.value = true + assertEquals(1, starts) + + // Shutdown's own cancel stays unconditional. + bound.value = false + starter.arm { lifetime == newLifetime } + starter.cancel() + assertFalse(starter.isArmed) + } + + @Test + fun aLifetimeEndingWhileWaiting_startsNothingOnTheHeal() = runBlocking { + val bound = MutableStateFlow(false) + var starts = 0 + var lifetime = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + serviceTearingDown = { false }, + startL1 = { starts++; true } + ) + + val armedIn = lifetime + starter.arm { lifetime == armedIn } + lifetime++ // that service shut down; its cancel did not reach the waiter + bound.value = true + + assertEquals(0, starts) + assertFalse(starter.isArmed) + } + @Test fun aLaterArmReplacesAnEarlierOne() = runBlocking { val bound = MutableStateFlow(false) From da7e80fe22b15efbd5b131380b8cbec41ec84ec6 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 21 Oct 2026 09:34:36 -0700 Subject: [PATCH 10/20] fix(sync): seed connectivity after registering the network callback The start-up connectivity read ran before registerNetworkCallback. A network lost in between was never reported (nothing matched any more), so the service read as online and a persisted replay could take the no-timeout wake lock offline. The read now runs after registration. ImpedimentGate keeps it from overwriting the callback: every callback update bumps a counter under one lock, and the seed marks the service offline only if no update has landed since the token taken just before registering. The same lock now serializes every change to the impediment set (the network callbacks on IO, the keystore observer on main), and the callbacks run one at a time in delivery order, so an onLost can no longer be processed before the onAvailable it follows. Raised by CodeRabbit on #1632. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 79 +++++++++++-------- .../wallet/service/NetworkAvailability.kt | 65 +++++++++++++-- .../wallet/service/NetworkAvailabilityTest.kt | 43 ++++++++++ 3 files changed, 149 insertions(+), 38 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index 280540e560..01bfeade4b 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -1833,15 +1833,26 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { private var networkCallbackRegistered = false private lateinit var networkCallback: ConnectivityManager.NetworkCallback private val availableNetworks: MutableSet = ConcurrentHashMap.newKeySet() + /** Serializes changes to [impediments] and decides the start-up connectivity seed. */ + private val impedimentGate = ImpedimentGate() + /** + * The network callbacks' work, one at a time in delivery order. Each used to + * launch onto the multi-threaded IO pool, so an `onLost` could be processed + * before the `onAvailable` it follows and leave a lost network counted as + * available — online with no network. + */ + private val networkEventDispatcher = Dispatchers.IO.limitedParallelism(1) private inner class NetworkCallbackImpl : ConnectivityManager.NetworkCallback() { init { // Setup security health monitoring securityFunctions.observeHealth().observe(this@BlockchainServiceImpl) { status -> isKeyStoreHealthy = status.isHealthy - if (isKeyStoreHealthy) { - impediments.remove(Impediment.SECURITY) - } else { - impediments.add(Impediment.SECURITY) + impedimentGate.update { + if (isKeyStoreHealthy) { + impediments.remove(Impediment.SECURITY) + } else { + impediments.add(Impediment.SECURITY) + } } updateBlockchainStateImpediments() check() @@ -1849,7 +1860,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } override fun onAvailable(network: Network) { - serviceScope.launch { + serviceScope.launch(networkEventDispatcher) { val capabilities = connectivityManager.getNetworkCapabilities(network) val hasInternet = capabilities?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true val hasValidated = capabilities?.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) == true @@ -1872,8 +1883,10 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } if (hasInternet && hasValidated) { - availableNetworks.add(network) - impediments.remove(Impediment.NETWORK) + impedimentGate.networkCallbackUpdate { + availableNetworks.add(network) + impediments.remove(Impediment.NETWORK) + } updateBlockchainStateImpediments() check() } @@ -1881,13 +1894,14 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } override fun onLost(network: Network) { - serviceScope.launch { - availableNetworks.remove(network) - log.info("network lost: $network, remaining networks: ${availableNetworks.size}") - + serviceScope.launch(networkEventDispatcher) { // Only add impediment if no suitable networks remain - if (availableNetworks.isEmpty()) { - impediments.add(Impediment.NETWORK) + val noneRemain = impedimentGate.networkCallbackUpdate { + availableNetworks.remove(network) + availableNetworks.isEmpty().also { if (it) impediments.add(Impediment.NETWORK) } + } + log.info("network lost: $network, remaining networks: ${availableNetworks.size}") + if (noneRemain) { updateBlockchainStateImpediments() check() } @@ -1895,7 +1909,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } override fun onCapabilitiesChanged(network: Network, capabilities: NetworkCapabilities) { - serviceScope.launch { + serviceScope.launch(networkEventDispatcher) { val hasInternet = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) val hasValidated = capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) @@ -1907,10 +1921,12 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { log.info(s.toString()) } - if (hasInternet && hasValidated) { - impediments.remove(Impediment.NETWORK) - } else { - impediments.add(Impediment.NETWORK) + impedimentGate.networkCallbackUpdate { + if (hasInternet && hasValidated) { + impediments.remove(Impediment.NETWORK) + } else { + impediments.add(Impediment.NETWORK) + } } updateBlockchainStateImpediments() check() @@ -1919,9 +1935,9 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { override fun onUnavailable() { super.onUnavailable() - serviceScope.launch { + serviceScope.launch(networkEventDispatcher) { log.info("network unavailable - no network satisfies request") - impediments.add(Impediment.NETWORK) + impedimentGate.networkCallbackUpdate { impediments.add(Impediment.NETWORK) } updateBlockchainStateImpediments() check() } @@ -2987,20 +3003,21 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) .addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) .build() - // Seed the NETWORK impediment from connectivity as it stands - // now: the registration below reports nothing while no network - // matches, so a service created offline read as online — and a - // persisted replay held the no-timeout wake lock in airplane - // mode. Seeded BEFORE registering, so every callback update - // (all of them run after the registration) overrides it. - if (!hasUsableNetworkNow(connectivityManager)) { - log.info("no usable network at service start — NETWORK impediment set until one appears") - impediments.add(Impediment.NETWORK) - updateBlockchainStateImpediments() - } + // The registration reports nothing while no network matches, so a + // service created offline read as online — and a persisted replay + // held the no-timeout wake lock in airplane mode. Seed the NETWORK + // impediment from connectivity read AFTER registering (a network + // lost in between is then not missed), unless a callback update + // has already landed: see ImpedimentGate. + val seedToken = impedimentGate.seedToken() connectivityManager.registerNetworkCallback(networkRequest, networkCallback) networkCallbackRegistered = true log.info("network callback registered with NetworkRequest: {}", networkCallback) + val usableNow = hasUsableNetworkNow(connectivityManager) + if (impedimentGate.seedOffline(seedToken, usableNow) { impediments.add(Impediment.NETWORK) }) { + log.info("no usable network at service start — NETWORK impediment set until one appears") + updateBlockchainStateImpediments() + } } monitorOlderTransactions(wallet) wallet.addCoinsReceivedEventListener( diff --git a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt index c594b8362d..55ba8812cf 100644 --- a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt +++ b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt @@ -25,13 +25,14 @@ import android.net.NetworkCapabilities * service can sync over: the default network, with internet and validated — * the same capabilities the service's network callback requires. * - * The service seeds its NETWORK impediment from this before it registers that - * callback. A passive `registerNetworkCallback` reports nothing while no - * network matches (no `onAvailable`, `onLost` or `onCapabilitiesChanged`; - * `onUnavailable` is for `requestNetwork`), so a service created while already - * offline used to read as online until some network came and went. Never - * throws: an unreadable state answers "usable", the old assumption, so this can - * only ever add an impediment the platform confirmed. + * The service seeds its NETWORK impediment from this once that callback is + * registered ([ImpedimentGate.seedOffline]). A passive `registerNetworkCallback` + * reports nothing while no network matches (no `onAvailable`, `onLost` or + * `onCapabilitiesChanged`; `onUnavailable` is for `requestNetwork`), so a + * service created while already offline used to read as online until some + * network came and went. Never throws: an unreadable state answers "usable", + * the old assumption, so this can only ever add an impediment the platform + * confirmed. */ fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try { val capabilities = connectivityManager.activeNetwork?.let(connectivityManager::getNetworkCapabilities) @@ -41,3 +42,53 @@ fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try } catch (t: Throwable) { true } + +/** + * Serializes every change to the blockchain service's impediment set, and + * decides the one-off connectivity seed at service start. + * + * The set is a plain `EnumSet` changed from the network callback's coroutines + * (IO) and the keystore health observer (main); [update] and + * [networkCallbackUpdate] make each change atomic. + * + * The seed: reading connectivity BEFORE registering the callback left a gap — + * a network lost between the read and the registration was never reported + * (nothing matched any more), and the service read as online. So the read + * comes AFTER registration, where it must not overwrite what the callback has + * already said: every callback update bumps a counter, the token is taken + * just before registering, and [seedOffline] marks the service offline only if + * no callback update landed since. Any network that is present after the + * registration is reported by the callback (`onAvailable`, later `onLost`), + * so the seed only ever decides the "nothing matched" case the callback stays + * silent on. + */ +internal class ImpedimentGate { + private val lock = Any() + private var networkCallbackUpdates = 0L + + /** A change that is not a network-callback update (the keystore health observer). */ + fun update(block: () -> T): T = synchronized(lock, block) + + /** A network-callback update; it supersedes the start-up seed. */ + fun networkCallbackUpdate(block: () -> T): T = synchronized(lock) { + networkCallbackUpdates++ + block() + } + + /** Taken just BEFORE `registerNetworkCallback`. */ + fun seedToken(): Long = synchronized(lock) { networkCallbackUpdates } + + /** + * After registration, with [usableNow] read after it too: mark the service + * offline ([markOffline], under the lock) unless a network is usable or a + * callback update has landed since [token]. A stale "not usable" read is + * harmless: a network that appeared before it is reported by the callback, + * and its update either landed first (the seed is skipped) or lands after + * (and clears the impediment). + */ + fun seedOffline(token: Long, usableNow: Boolean, markOffline: () -> Unit): Boolean = synchronized(lock) { + if (usableNow || networkCallbackUpdates != token) return false + markOffline() + true + } +} diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt index dfa857219a..dd00797d66 100644 --- a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -73,6 +73,49 @@ class NetworkAvailabilityTest { assertFalse(hasUsableNetworkNow(connectivityManager)) } + // ── The seed after registration (CodeRabbit on #1632) ────────────────── + + private val impediments = java.util.EnumSet.noneOf( + org.dash.wallet.common.data.entity.BlockchainState.Impediment::class.java + ) + private val network = org.dash.wallet.common.data.entity.BlockchainState.Impediment.NETWORK + private val gate = ImpedimentGate() + + @Test + fun aNetworkLostBeforeRegistration_isSeededOffline() { + // Usable when the old code read it, gone by the time the callback was + // registered: nothing matches, so the callback never says a word. + val token = gate.seedToken() + // registerNetworkCallback(...) — no callback update follows + assertTrue(gate.seedOffline(token, usableNow = false) { impediments.add(network) }) + assertTrue(network in impediments) + } + + @Test + fun aCallbackUpdateBeforeTheSeed_wins_overAStaleRead() { + val token = gate.seedToken() + // A network appears right after registration; its onAvailable lands first. + gate.networkCallbackUpdate { impediments.remove(network) } + // The seed's read was taken just before that network came up. + assertFalse(gate.seedOffline(token, usableNow = false) { impediments.add(network) }) + assertFalse("the callback's online verdict stands", network in impediments) + } + + @Test + fun aCallbackUpdateAfterTheSeed_overridesIt() { + val token = gate.seedToken() + assertTrue(gate.seedOffline(token, usableNow = false) { impediments.add(network) }) + gate.networkCallbackUpdate { impediments.remove(network) } // onAvailable + assertFalse(network in impediments) + } + + @Test + fun aUsableNetwork_isNeverSeededOffline() { + val token = gate.seedToken() + assertFalse(gate.seedOffline(token, usableNow = true) { impediments.add(network) }) + assertFalse(network in impediments) + } + @Test fun aServiceCreatedOffline_neverTakesTheReplayLock_untilANetworkAppears() { shadowOf(connectivityManager).setDefaultNetworkActive(false) From 0a9b03f315ad8928dedeceb65468ac94bdb54a7c Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 21 Oct 2026 10:14:54 -0700 Subject: [PATCH 11/20] fix(sync): re-take the replay wake lock even when the observer misses a wedge flip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReplayWakeLock observed the filter wedge as a StateFlow. A collector slow to run can miss intermediate StateFlow values, so clear → re-wedge → clear arrived as the "not wedged" it had already seen, while the minute tick released the lock during the re-wedge. Nothing re-took it, and a screen-off replay could stay asleep. FilterStallWatchdogDecider now counts its true → false transitions (standDownClears, updated with the flag under one lock), and the service observes that count (L1ShadowSyncService.filterScanWedgeClears). A count the observer has not seen always means a clear happened, so it re-checks; reacquireIfDue reads the current state, so a clear already followed by a new wedge acquires nothing. The baseline is read when observation starts, not on the collector's first run. The teardown retirement guard is unchanged; no polling. Regression: ReplayWakeLockTest runs the clear → re-wedge → clear sequence with the collector held back on a test dispatcher and asserts the lock is re-taken once it runs; plus a clear superseded by a new wedge, and a clear between registration and the collector's first run. Raised by Codex review. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 2 +- .../wallet/service/ReplayWakeLock.kt | 38 +++++++---- .../platform/sdk/L1ShadowSyncService.kt | 29 ++++++-- .../wallet/service/ReplayWakeLockTest.kt | 68 +++++++++++++++++-- 4 files changed, 111 insertions(+), 26 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index 01bfeade4b..06d70b19ed 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -2772,7 +2772,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { // ACTION_TIME_TICK is not delivered while the CPU sleeps (see ReplayWakeLock). // Unconditional: the cutover gate can be re-resolved later in this // service's life, and reacquireIfDue re-reads it itself. - replayWakeLock.observeWedge(serviceScope, l1ShadowSyncService.filterScanWedgedState) + replayWakeLock.observeWedgeClears(serviceScope, l1ShadowSyncService.filterScanWedgeClears) peerConnectivityListener = PeerConnectivityListener() broadcastPeerState(0) diff --git a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt index 4447dc6af0..5d86f60bf3 100644 --- a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt +++ b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt @@ -19,9 +19,7 @@ package de.schildbach.wallet.service import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.launch import org.slf4j.LoggerFactory @@ -47,7 +45,7 @@ interface ReplayWakeLockHandle { * - the minute tick acquires and releases ([hold]); * - the moments a stuck replay can move again re-acquire at once * ([reacquireIfDue]): the network impediment clearing, and the filter wedge - * clearing ([observeWedge]). ACTION_TIME_TICK is not delivered while the CPU + * clearing ([observeWedgeClears]). ACTION_TIME_TICK is not delivered while the CPU * sleeps, so waiting for the tick would leave a screen-off replay asleep * until something unrelated woke the device. * @@ -112,18 +110,28 @@ internal class ReplayWakeLock( } /** - * Re-check the moment the filter wedge clears ([wedged] turns false after - * the first value): the cursor moved, wallet events resumed, the scan - * caught up, or the engine stopped. + * Re-check every time the filter wedge clears: the cursor moved, wallet + * events resumed, the scan caught up, or the engine stopped. + * + * [clears] COUNTS the clears rather than mirroring the flag. A collector + * that is slow to run can miss intermediate values of a StateFlow, and on a + * Boolean that loses events: clear → re-wedge → clear arrives as the + * "not wedged" it already saw, while the minute tick released the lock + * during the re-wedge — and no re-check came. A count it has not seen + * always means a clear happened, so it re-checks. The baseline is read when + * this is CALLED, not when the collector first runs, so a clear in between + * is not taken for the starting value. [reacquireIfDue] reads the current + * state, so a clear that was followed by a new wedge acquires nothing. */ - fun observeWedge(scope: CoroutineScope, wedged: Flow): Job = scope.launch { - wedged.distinctUntilChanged() - .drop(1) // the current value is not a transition - .filter { !it } - .collect { - log.info("the filter wedge cleared — re-checking the replay wake lock") - reacquireIfDue() - } + fun observeWedgeClears(scope: CoroutineScope, clears: StateFlow): Job { + val baseline = clears.value + return scope.launch { + clears.filter { it != baseline } + .collect { + log.info("the filter wedge cleared — re-checking the replay wake lock") + reacquireIfDue() + } + } } /** onDestroy, first thing: no acquisition after this. */ diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index 6a07392f6f..1dfaf032f0 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -1313,17 +1313,31 @@ internal class FilterStallWatchdogDecider( * stops being behind, or the wallet-event stream shows the engine alive. * Read by the blockchain service from another thread: a wedged engine is * no reason to keep the CPU awake — and the moment it clears, the replay - * needs the CPU back at once ([standingDownState]). + * needs the CPU back at once ([standDownClears]). */ var standingDown: Boolean get() = _standingDown.value private set(value) { - _standingDown.value = value + synchronized(transitionLock) { + if (_standingDown.value != value) { + _standingDown.value = value + if (!value) _standDownClears.value = _standDownClears.value + 1 + } + } } + private val transitionLock = Any() private val _standingDown = MutableStateFlow(false) + private val _standDownClears = MutableStateFlow(0L) - /** [standingDown] as a flow, so a clear is an event and not only the next poll's reading. */ - val standingDownState: StateFlow = _standingDown.asStateFlow() + /** + * How many times [standingDown] has gone from true to false — counted, not + * mirrored, so a clear is never lost to conflation. A `StateFlow` + * of the flag collapses clear → re-wedge → clear into the value a delayed + * collector already saw, and the recovery after the second wedge went + * unnoticed. This only ever increases: any change a collector sees means + * at least one clear happened since its last look. + */ + val standDownClears: StateFlow = _standDownClears.asStateFlow() /** The filter cursor the watchdog stood down at; written before [standingDown] goes true. */ @Volatile @@ -2587,8 +2601,11 @@ class L1ShadowSyncService internal constructor( */ fun filterScanWedged(): Boolean = filterStallDecider.standingDown - /** [filterScanWedged] as a flow: the blockchain service re-takes the replay wake lock when it clears. */ - val filterScanWedgedState: StateFlow get() = filterStallDecider.standingDownState + /** + * Counts [filterScanWedged] clearing ([FilterStallWatchdogDecider.standDownClears]): + * the blockchain service re-takes the replay wake lock on every increase. + */ + val filterScanWedgeClears: StateFlow get() = filterStallDecider.standDownClears /** * When the in-flight engine start began, on [nowMs]'s clock; 0 when no diff --git a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt index 9b6a8e49c7..1c8f151752 100644 --- a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt +++ b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt @@ -22,6 +22,8 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancel +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -98,7 +100,7 @@ class ReplayWakeLockTest { @Test fun aRecoveryFromAnExhaustedWedge_retakesTheLock_withoutATickOrANetworkChange() { - replayWakeLock.observeWedge(scope, decider.standingDownState) + replayWakeLock.observeWedgeClears(scope, decider.standDownClears) tick() assertTrue("the replay holds the lock", lock.isHeld) @@ -124,7 +126,7 @@ class ReplayWakeLockTest { // The watchdog's own check runs on a one-minute delay that only advances // while awake; the progress feed and the event tap are what see the // engine move during a brief wake. - replayWakeLock.observeWedge(scope, decider.standingDownState) + replayWakeLock.observeWedgeClears(scope, decider.standDownClears) exhaustTheWatchdog() tick() assertFalse(lock.isHeld) @@ -146,9 +148,67 @@ class ReplayWakeLockTest { assertTrue(lock.isHeld) } + @Test + fun aDelayedObserver_thatMissesClearReWedgeClear_stillRetakesTheLock() = runTest { + // Codex on #1632: observed as a StateFlow, a collector that is + // slow to run sees clear → re-wedge → clear as the "not wedged" it had + // already seen (StateFlow conflates), while the minute tick released the + // lock during the re-wedge. Nothing re-took it. The collector here runs + // only when the test scheduler is advanced, so the whole sequence happens + // while it is held back. + replayWakeLock.observeWedgeClears(backgroundScope, decider.standDownClears) + runCurrent() // the observer is subscribed and idle + tick() + assertTrue("the replay holds the lock", lock.isHeld) + + exhaustTheWatchdog() // wedged + decider.onFilterProgress(stuck + 1) // clear #1 — the observer does not run + // The cursor moved one block and stalls there: re-wedged, budget spent. + check(200 * 60_000L, height = stuck + 1) + check(300 * 60_000L, height = stuck + 1) + assertTrue(decider.standingDown) + tick() + assertFalse("the tick releases the lock during the re-wedge", lock.isHeld) + decider.onFilterProgress(stuck + 2) // clear #2 — still not run + assertFalse(decider.standingDown) + assertFalse("nothing has re-checked yet", lock.isHeld) + + runCurrent() // the delayed observer finally runs + assertTrue("the missed recovery still re-takes the lock", lock.isHeld) + assertEquals(1, lock.count) + } + + @Test + fun aDelayedObserver_seeingAClearFollowedByANewWedge_acquiresNothing() = runTest { + replayWakeLock.observeWedgeClears(backgroundScope, decider.standDownClears) + runCurrent() + exhaustTheWatchdog() + tick() + assertFalse(lock.isHeld) + + decider.onFilterProgress(stuck + 1) // clear — observer held back + check(200 * 60_000L, height = stuck + 1) + check(300 * 60_000L, height = stuck + 1) // re-wedged before it runs + assertTrue(decider.standingDown) + runCurrent() + assertFalse("the clear it sees is already superseded by a wedge", lock.isHeld) + } + + @Test + fun aClearBetweenRegistrationAndTheObserversFirstRun_isNotTakenForTheBaseline() = runTest { + exhaustTheWatchdog() + tick() + assertFalse(lock.isHeld) + + replayWakeLock.observeWedgeClears(backgroundScope, decider.standDownClears) + decider.onFilterProgress(stuck + 1) // clears before the observer's first run + runCurrent() + assertTrue(lock.isHeld) + } + @Test fun aWedgeClearedWhileOffline_waitsForTheNetwork() { - replayWakeLock.observeWedge(scope, decider.standingDownState) + replayWakeLock.observeWedgeClears(scope, decider.standDownClears) exhaustTheWatchdog() networkImpeded = true tick() @@ -165,7 +225,7 @@ class ReplayWakeLockTest { @Test fun aWedgeClearingAfterTeardownBegan_acquiresNothing() { - replayWakeLock.observeWedge(scope, decider.standingDownState) + replayWakeLock.observeWedgeClears(scope, decider.standDownClears) tick() exhaustTheWatchdog() tick() From 3238b3af556c6a7166299ccef110a37c5048b0d1 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 23 Oct 2026 09:22:26 -0700 Subject: [PATCH 12/20] fix(sync): give a restarted engine its own filter-stall window onEngineStopped() cleared the stand-down flag but kept the stopped run's stall timing (lastHeight / lastAdvanceMs). A run restarted at the height the watchdog stood down at was then read as "same cursor, still since long ago" on its first check and stood down again within a minute, so the replay lost its wake lock with none of the stall window spent. Drop the per-run timing on stop (the restart budget stays per process), and synchronize onEngineStopped() with onCheck() so a check still in flight on the cancelled watchdog cannot write the old baseline back. Co-Authored-By: Claude Opus 5.5 --- .../platform/sdk/L1ShadowSyncService.kt | 11 ++++++ .../platform/sdk/L1ShadowSyncServiceTest.kt | 37 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index 1dfaf032f0..fe4389d713 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -1374,8 +1374,18 @@ internal class FilterStallWatchdogDecider( * a wedge of the previous run would still read as current after a restart, * until the new run's first check. The restart budget is NOT reset: it is * per process, and a re-wedge sets [standingDown] again on its own. + * + * The stall TIMING is per run, so it is dropped too. Kept, a run restarted + * at the height the last one stood down at reads as "same cursor, still + * since long ago" on its first check, and stands down again within a + * minute — the replay losing its wake lock with none of the stall window + * spent. Synchronized with [onCheck]: a check still in flight on the + * cancelled watchdog cannot write the old run's baseline back after this. */ + @Synchronized fun onEngineStopped() { + lastHeight = -1L + lastAdvanceMs = 0L standingDown = false } @@ -1438,6 +1448,7 @@ internal class FilterStallWatchdogDecider( * withdrawn, because the value it was read from is the in-memory cursor, * not this one. 0 when unknown. */ + @Synchronized fun onCheck( nowMs: Long, filterHeight: Long, diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt index 44bb796cf8..0363ba1a5b 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt @@ -742,6 +742,43 @@ class L1ShadowSyncServiceTest { assertFalse(d.standingDown) } + @Test + fun filterStall_aRunRestartedAtTheStoodDownHeight_getsAFullStallWindow() { + // PastaClaw on #1632: the stop cleared the flag but kept the old run's + // timing, so a new run resuming at the same height was judged "still + // since long ago" on its first check and stood down at once — the + // replay losing its wake lock about a minute into the restart. + val d = stallDecider() + val stuck = 1_500_000L // more than one commit batch behind + val target = 1_556_896L + fun check(atMs: Long) = + d.onCheck(atMs, stuck, target, lastWalletEventMs = 0L, walletSyncedHeight = stuck) + + check(0L) + check(10 * 60_000L + 1) + check(30 * 60_000L + 2) + check(60 * 60_000L + 3) + assertEquals(FilterStallWatchdogDecider.Decision.EXHAUSTED, check(120 * 60_000L)) + assertTrue(d.standingDown) + + // Stopped, down for hours, restarted with the durable watermark at the + // same height. + d.onEngineStopped() + val restartedAt = 600 * 60_000L + + // The new run's first check, and every check inside the stall window + // (30 min: the budget is spent, so the last rung's wait applies). + assertEquals(FilterStallWatchdogDecider.Decision.NONE, check(restartedAt + 60_000L)) + assertFalse("the new run starts its own stall window", d.standingDown) + assertEquals(FilterStallWatchdogDecider.Decision.NONE, check(restartedAt + 30 * 60_000L)) + assertFalse(d.standingDown) + + // Still at the same height past the window: a real wedge of this run. + // No restart (the budget is per process), but standing down again. + assertEquals(FilterStallWatchdogDecider.Decision.NONE, check(restartedAt + 31 * 60_000L + 1)) + assertTrue(d.standingDown) + } + @Test fun filterStall_anUnknownWatermarkIsTreatedAsFarFromTheTip() { From c17f3882459b0051dc699aa1f809e46bc7771d0e Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 23 Oct 2026 18:53:58 -0700 Subject: [PATCH 13/20] fix(sync): default-network impediment, guarded wake-lock rechecks, bind-heal stop generation Three review blockers on #1632: - NETWORK impediment from the default network. The callback was registered for any network with internet + validated, so a validated secondary network cleared the impediment while the default route was unusable, and one matching network losing validation set it while another was up. It is now a default-network callback (DefaultNetworkTracker): offline unless the default network has internet and is validated. A loss of a former default is ignored in either delivery order. - Replay wake lock rechecks under one monitor. reacquireIfDue() read the decision outside the lock's monitor and could only acquire, so a recovery that read "due" just before a network loss kept the no-timeout lock offline until the next tick. tick() and every event now run recheck(), which reads tickHolds || replayDue() and acquires or releases under the monitor, so the loss's own recheck releases. The tick records the holds only it can see (engine start, durable save). - Bind-heal start carries the engine stop generation. The waiter samples it before its lifetime/teardown checks and passes it to startIfEnabled(expectedExternalStopGeneration), so a shutdown that passes the checks and finishes the engine stop before the start keeps L1 down. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 74 ++++++++++--------- .../wallet/service/NetworkAvailability.kt | 58 +++++++++++++-- .../wallet/service/ReplayWakeLock.kt | 60 +++++++++------ .../service/platform/PlatformSyncService.kt | 3 +- .../service/platform/sdk/BindHealL1Starter.kt | 18 ++++- .../platform/sdk/L1ShadowSyncService.kt | 6 ++ .../wallet/service/NetworkAvailabilityTest.kt | 45 ++++++++++- .../wallet/service/ReplayWakeLockTest.kt | 58 ++++++++++++++- .../platform/sdk/BindHealL1StarterTest.kt | 57 ++++++++++++++ 9 files changed, 306 insertions(+), 73 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index 06d70b19ed..e932c9787f 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -32,7 +32,6 @@ import android.content.pm.ServiceInfo import android.net.ConnectivityManager import android.net.Network import android.net.NetworkCapabilities -import android.net.NetworkRequest import android.net.Uri import android.os.Binder import android.os.Build @@ -1832,7 +1831,8 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } private var networkCallbackRegistered = false private lateinit var networkCallback: ConnectivityManager.NetworkCallback - private val availableNetworks: MutableSet = ConcurrentHashMap.newKeySet() + /** The default network's verdict; touched only inside [ImpedimentGate.networkCallbackUpdate]. */ + private val defaultNetwork = DefaultNetworkTracker() /** Serializes changes to [impediments] and decides the start-up connectivity seed. */ private val impedimentGate = ImpedimentGate() /** @@ -1882,11 +1882,12 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { log.info(s.toString()) } - if (hasInternet && hasValidated) { - impedimentGate.networkCallbackUpdate { - availableNetworks.add(network) - impediments.remove(Impediment.NETWORK) - } + // The NEW default network: its own capabilities decide, usable or not. + val usable = capabilities?.let { hasInternet && hasValidated } + val offline = impedimentGate.networkCallbackUpdate { + defaultNetwork.onAvailable(network, usable).also(::applyNetworkVerdict) + } + if (offline != null) { updateBlockchainStateImpediments() check() } @@ -1895,13 +1896,13 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { override fun onLost(network: Network) { serviceScope.launch(networkEventDispatcher) { - // Only add impediment if no suitable networks remain - val noneRemain = impedimentGate.networkCallbackUpdate { - availableNetworks.remove(network) - availableNetworks.isEmpty().also { if (it) impediments.add(Impediment.NETWORK) } + // Offline only if this is still the default network: a lost former + // default whose successor was already reported says nothing. + val offline = impedimentGate.networkCallbackUpdate { + defaultNetwork.onLost(network).also(::applyNetworkVerdict) } - log.info("network lost: $network, remaining networks: ${availableNetworks.size}") - if (noneRemain) { + log.info("network lost: $network, {}", if (offline == true) "no default network" else "not the default") + if (offline != null) { updateBlockchainStateImpediments() check() } @@ -1922,11 +1923,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } impedimentGate.networkCallbackUpdate { - if (hasInternet && hasValidated) { - impediments.remove(Impediment.NETWORK) - } else { - impediments.add(Impediment.NETWORK) - } + applyNetworkVerdict(defaultNetwork.onCapabilitiesChanged(network, hasInternet && hasValidated)) } updateBlockchainStateImpediments() check() @@ -1943,6 +1940,15 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } } + /** Inside [ImpedimentGate.networkCallbackUpdate]: true offline, false online, null no change. */ + private fun applyNetworkVerdict(offline: Boolean?) { + when (offline) { + true -> impediments.add(Impediment.NETWORK) + false -> impediments.remove(Impediment.NETWORK) + null -> Unit + } + } + fun check() { serviceScope.launch { try { @@ -2490,9 +2496,9 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { durableHeightTracker.observe(durableHeight, System.currentTimeMillis()) ) // A replay that cannot make progress keeps the service up but releases the CPU. - // The same predicate ReplayWakeLock re-reads before releasing — one definition, - // or the two could disagree and refuse the release on every tick. - replayWakeLock.hold(replayWakeLockDue() || engineStarting || durableLagging) + // ReplayWakeLock reads the replay's part itself (replayWakeLockDue), the same + // way the event rechecks do; the tick adds what only it can see. + replayWakeLock.tick(otherHolds = engineStarting || durableLagging) // Those two holds also keep the service in the foreground: the scan reads // synced while they last, and a demoted service would be stopped anyway. if (foregroundDemotionGate.onHold(sdkHoldsService(System.currentTimeMillis()))) { @@ -2771,7 +2777,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { // Re-take the replay wake lock the moment the filter wedge clears: // ACTION_TIME_TICK is not delivered while the CPU sleeps (see ReplayWakeLock). // Unconditional: the cutover gate can be re-resolved later in this - // service's life, and reacquireIfDue re-reads it itself. + // service's life, and recheck re-reads it itself. replayWakeLock.observeWedgeClears(serviceScope, l1ShadowSyncService.filterScanWedgeClears) peerConnectivityListener = PeerConnectivityListener() @@ -2997,22 +3003,19 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { // Initialize network callback (now that securityFunctions is available) networkCallback = NetworkCallbackImpl() - // Register network callback for modern network monitoring - // Use NetworkRequest to reliably detect when no validated network is available (e.g., airplane mode) - val networkRequest = NetworkRequest.Builder() - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) - .build() - // The registration reports nothing while no network matches, so a + // Track the DEFAULT network — the route the service's traffic takes — + // not any network with internet: a validated secondary network said + // "online" while the default route was down (see DefaultNetworkTracker). + // The callback reports nothing while there is no default network, so a // service created offline read as online — and a persisted replay // held the no-timeout wake lock in airplane mode. Seed the NETWORK // impediment from connectivity read AFTER registering (a network // lost in between is then not missed), unless a callback update // has already landed: see ImpedimentGate. val seedToken = impedimentGate.seedToken() - connectivityManager.registerNetworkCallback(networkRequest, networkCallback) + connectivityManager.registerDefaultNetworkCallback(networkCallback) networkCallbackRegistered = true - log.info("network callback registered with NetworkRequest: {}", networkCallback) + log.info("default network callback registered: {}", networkCallback) val usableNow = hasUsableNetworkNow(connectivityManager) if (impedimentGate.seedOffline(seedToken, usableNow) { impediments.add(Impediment.NETWORK) }) { log.info("no usable network at service start — NETWORK impediment set until one appears") @@ -3491,7 +3494,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { if (networkCallbackRegistered) { connectivityManager.unregisterNetworkCallback(networkCallback) networkCallbackRegistered = false - availableNetworks.clear() + impedimentGate.networkCallbackUpdate { defaultNetwork.clear() } } log.info("receivers unregistered, Now starting coroutine to finish the rest of the cleanup") @@ -3815,9 +3818,10 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { private fun updateBlockchainStateImpediments() { blockchainStateDataProvider.updateImpediments(impediments) - // The network back (or any impediment change): the replay may be due - // the lock again, and the tick will not come while the CPU sleeps. - replayWakeLock.reacquireIfDue() + // The network back or gone (any impediment change): the replay may be + // due the lock again, or no longer, and the tick will not come while + // the CPU sleeps. + replayWakeLock.recheck() } // /** diff --git a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt index 55ba8812cf..4f41bc50b0 100644 --- a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt +++ b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt @@ -23,11 +23,12 @@ import android.net.NetworkCapabilities /** * Whether connectivity AS IT STANDS NOW offers a network the blockchain * service can sync over: the default network, with internet and validated — - * the same capabilities the service's network callback requires. + * the same rule the service's default-network callback applies + * ([DefaultNetworkTracker]). * * The service seeds its NETWORK impediment from this once that callback is - * registered ([ImpedimentGate.seedOffline]). A passive `registerNetworkCallback` - * reports nothing while no network matches (no `onAvailable`, `onLost` or + * registered ([ImpedimentGate.seedOffline]). The callback reports nothing + * while there is no default network (no `onAvailable`, `onLost` or * `onCapabilitiesChanged`; `onUnavailable` is for `requestNetwork`), so a * service created while already offline used to read as online until some * network came and went. Never throws: an unreadable state answers "usable", @@ -43,6 +44,49 @@ fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try true } +/** + * The NETWORK impediment from a DEFAULT-network callback + * (`registerDefaultNetworkCallback`): offline unless the default network — the + * route the service's own traffic takes — has internet and is validated. + * + * Review on #1632 (CodeRabbit, PastaClaw): the callback used to be registered + * for ANY network with internet + validated. A validated secondary network + * then cleared the impediment while the default route was unusable, and the + * replay kept its no-timeout wake lock offline. The reverse also happened: + * one matching network losing validation set the impediment while another + * usable one was still up. The default network is one network, so each event + * about it is the whole answer. + * + * Each method returns the verdict: true offline, false online, null when the + * event decides nothing (a loss of a network that is no longer the default — + * its successor's `onAvailable` may already have been delivered — or an + * `onAvailable` whose capabilities could not be read; `onCapabilitiesChanged` + * follows it). Called in delivery order, one at a time. + */ +internal class DefaultNetworkTracker { + private var current: N? = null + + fun onAvailable(network: N, usable: Boolean?): Boolean? { + current = network + return usable?.not() + } + + fun onCapabilitiesChanged(network: N, usable: Boolean): Boolean { + current = network + return !usable + } + + fun onLost(network: N): Boolean? { + if (network != current) return null + current = null + return true + } + + fun clear() { + current = null + } +} + /** * Serializes every change to the blockchain service's impediment set, and * decides the one-off connectivity seed at service start. @@ -57,10 +101,10 @@ fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try * comes AFTER registration, where it must not overwrite what the callback has * already said: every callback update bumps a counter, the token is taken * just before registering, and [seedOffline] marks the service offline only if - * no callback update landed since. Any network that is present after the - * registration is reported by the callback (`onAvailable`, later `onLost`), - * so the seed only ever decides the "nothing matched" case the callback stays - * silent on. + * no callback update landed since. A default network that is present after + * the registration is reported by the callback (`onAvailable`, later + * `onLost`), so the seed only ever decides the "no default network" case the + * callback stays silent on. */ internal class ImpedimentGate { private val lock = Any() diff --git a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt index 5d86f60bf3..48230109bc 100644 --- a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt +++ b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt @@ -41,18 +41,26 @@ interface ReplayWakeLockHandle { * progress ([replayHoldsWakeLock]: offline, or wedged with the filter-stall * watchdog's restarts spent). * - * Who decides: - * - the minute tick acquires and releases ([hold]); - * - the moments a stuck replay can move again re-acquire at once - * ([reacquireIfDue]): the network impediment clearing, and the filter wedge - * clearing ([observeWedgeClears]). ACTION_TIME_TICK is not delivered while the CPU - * sleeps, so waiting for the tick would leave a screen-off replay asleep - * until something unrelated woke the device. + * Who decides: everyone, the same way. The minute tick ([tick]) and every + * event that can change the answer ([recheck]: an impediment change, the + * filter wedge clearing via [observeWedgeClears]) read the decision NOW and + * acquire or release to match, under one monitor. ACTION_TIME_TICK is not + * delivered while the CPU sleeps, so a replay that can move again must not + * wait for the tick to get the lock back. + * + * The read and the change happen under the same monitor, and so does each + * later recheck. A recheck that read "due" just before a concurrent network + * loss can still acquire. But the loss's own recheck then runs after it, reads + * "not due", and releases. (An acquire-only re-check used to leave that lock + * held until the next tick, so the replay held the CPU while offline.) + * + * The tick also knows holds the events cannot see (an engine start, a durable + * save in flight). It records them, and a recheck honours the last recorded + * value. A stale one only keeps the lock, and a held lock keeps the CPU awake, + * so the next tick comes and corrects it. * * Tracked apart from `isHeld` because the lock is reference-counted and shared * with the dashj path ([releaseForTeardown] releases only this owner's count). - * The tick runs on the main thread and the re-acquires on IO, so the held flag - * and the lock change together under one monitor. * * @param lock the service's wake lock; null before onCreate assigns it. * @param sdkPath whether the SDK owns L1 (the dashj path manages its own hold). @@ -79,20 +87,30 @@ internal class ReplayWakeLock( @Volatile private var retired = false - /** The tick's decision: [hold] true while the replay (or an engine start / durable save) needs the CPU. */ - fun hold(hold: Boolean) { + /** The tick's holds the events cannot read; written and read under [guard]. */ + private var tickHolds = false + + /** + * The minute tick. [otherHolds]: an engine start or a durable save needs + * the CPU (the replay's own part is read by [recheck]). + */ + fun tick(otherHolds: Boolean) { + synchronized(guard) { tickHolds = otherHolds } + recheck() + } + + /** Hold the lock iff the replay (or the tick's last other holds) needs the CPU now. */ + fun recheck() { if (!sdkPath()) return val lock = lock() ?: return synchronized(guard) { try { - if (hold && !held && !retired) { + val due = tickHolds || replayDue() + if (due && !held && !retired) { log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") lock.acquire() held = true - } else if (!hold && held && !replayDue()) { - // replayDue() re-reads the inputs under the guard: the tick - // decided from a reading an event may have overtaken (the - // network back, the wedge cleared, the lock re-acquired) since. + } else if (!due && held) { log.info("replay / engine start complete or unable to progress — releasing the wake lock") held = false if (lock.isHeld) lock.release() @@ -103,12 +121,6 @@ internal class ReplayWakeLock( } } - /** Acquire-only: take the lock back if the replay is due it now. Releasing stays the tick's decision. */ - fun reacquireIfDue() { - if (!sdkPath()) return - if (replayDue()) hold(true) - } - /** * Re-check every time the filter wedge clears: the cursor moved, wallet * events resumed, the scan caught up, or the engine stopped. @@ -120,7 +132,7 @@ internal class ReplayWakeLock( * during the re-wedge — and no re-check came. A count it has not seen * always means a clear happened, so it re-checks. The baseline is read when * this is CALLED, not when the collector first runs, so a clear in between - * is not taken for the starting value. [reacquireIfDue] reads the current + * is not taken for the starting value. [recheck] reads the current * state, so a clear that was followed by a new wedge acquires nothing. */ fun observeWedgeClears(scope: CoroutineScope, clears: StateFlow): Job { @@ -129,7 +141,7 @@ internal class ReplayWakeLock( clears.filter { it != baseline } .collect { log.info("the filter wedge cleared — re-checking the replay wake lock") - reacquireIfDue() + recheck() } } } diff --git a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt index de8b7dc169..62f1cbd88d 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt @@ -497,7 +497,8 @@ class PlatformSynchronizationService @Inject constructor( scope = syncScope, bindEstablished = sdkWalletBinder.bindEstablished, serviceTearingDown = { de.schildbach.wallet.service.BlockchainServiceImpl.isCleaningUpNow }, - startL1 = { l1ShadowSyncService.startIfEnabled() } + stopGeneration = { l1ShadowSyncService.stopGeneration() }, + startL1 = { generation -> l1ShadowSyncService.startIfEnabled(expectedExternalStopGeneration = generation) } ) private var lastTopupUpdateTime = 0L private var lastMetadataUpdateTime = 0L diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt index 8f0b0dee3f..b1843b1f6e 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/BindHealL1Starter.kt @@ -19,6 +19,8 @@ package de.schildbach.wallet.service.platform.sdk import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch @@ -50,6 +52,12 @@ import org.slf4j.LoggerFactory * teardown and start L1 once the cleanup flag cleared, with no service. * [arm] and [cancel] run on different threads, so they are serialized. * + * Those checks and the start are not one step: a shutdown can pass between + * them and finish the engine stop before the start runs. So the start carries + * the engine's stop generation ([stopGeneration]), sampled BEFORE the checks, + * and [startL1] refuses under the engine's own lock if a stop has moved it + * since — the guard a filter-stall restart already uses. + * * Kept out of `PlatformSynchronizationService` so the ordering can be pinned * by a host test with plain flows. */ @@ -57,7 +65,8 @@ internal class BindHealL1Starter( private val scope: CoroutineScope, private val bindEstablished: Flow, private val serviceTearingDown: () -> Boolean, - private val startL1: suspend () -> Boolean + private val stopGeneration: () -> Long, + private val startL1: suspend (expectedStopGeneration: Long) -> Boolean ) { private val log = LoggerFactory.getLogger(BindHealL1Starter::class.java) @@ -106,6 +115,11 @@ internal class BindHealL1Starter( private suspend fun awaitHealAndStart(stillCurrent: () -> Boolean) { bindEstablished.first { it } + // Sampled before the checks: a stop that lands after them moves it, and + // the start refuses. A cancel (shutdown, engine stop) that landed before + // the sample is caught here, as nothing below suspends before the start. + val generation = stopGeneration() + currentCoroutineContext().ensureActive() if (!stillCurrent()) { log.info( "SDK bind healed after the service lifetime that waited for it ended — not starting " + @@ -121,7 +135,7 @@ internal class BindHealL1Starter( return } val started = try { - startL1() + startL1(generation) } catch (t: Throwable) { if (t is CancellationException) throw t log.warn("SDK L1 engine start after the in-session bind heal failed", t) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index fe4389d713..d7186942d5 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -2581,6 +2581,12 @@ class L1ShadowSyncService internal constructor( @Volatile private var externalStopGeneration = 0L + /** + * The current [externalStopGeneration], for a start decided outside this + * class (the bind-heal waiter) to pass back to [startIfEnabled]. + */ + fun stopGeneration(): Long = externalStopGeneration + /** * When the engine last delivered ANY wallet event, monotonic-ish wall * clock via [nowMs]; 0 until the first one of this run. diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt index dd00797d66..18aaa8b571 100644 --- a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -143,13 +143,54 @@ class NetworkAvailabilityTest { sdkPath = { true }, replayDue = { replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false) } ) - replayWakeLock.hold(replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false)) + replayWakeLock.tick(otherHolds = false) assertFalse("offline from the start: no wake lock for a replay that cannot move", lock.isHeld) assertEquals(0, acquisitions) // A network appears: the callback clears the impediment and re-checks. impeded = false - replayWakeLock.reacquireIfDue() + replayWakeLock.recheck() assertTrue(lock.isHeld) } + + // ── The default network decides (CodeRabbit, PastaClaw on #1632) ─────── + + private val tracker = DefaultNetworkTracker() + + @Test + fun aValidatedSecondaryNetwork_cannotSayOnline_whileTheDefaultIsUnusable() { + // Registered for the default network, the callback never hears about + // the secondary one. The default's own capabilities are the answer. + assertEquals(true, tracker.onAvailable("wifi", usable = false)) + assertEquals(true, tracker.onCapabilitiesChanged("wifi", usable = false)) + assertEquals(false, tracker.onCapabilitiesChanged("wifi", usable = true)) + } + + @Test + fun theDefaultSwitching_isOnlineThroughout_whicheverOrderTheLossArrives() { + assertEquals(false, tracker.onAvailable("wifi", usable = true)) + // The new default reported before the old one's loss: that loss says nothing. + assertEquals(false, tracker.onAvailable("cell", usable = true)) + assertEquals(null, tracker.onLost("wifi")) + // The other order: briefly offline, then online with the successor. + assertEquals(true, tracker.onLost("cell")) + assertEquals(false, tracker.onAvailable("wifi", usable = true)) + } + + @Test + fun theDefaultLosingValidation_isOffline_evenWithAnotherNetworkUp() { + // The reverse of the secondary-network case: one network losing + // validation is the answer only when it is the default network. + assertEquals(false, tracker.onAvailable("wifi", usable = true)) + assertEquals(true, tracker.onCapabilitiesChanged("wifi", usable = false)) + // The system moves the default to the validated cell network: online again. + assertEquals(false, tracker.onAvailable("cell", usable = true)) + } + + @Test + fun capabilitiesUnreadableOnAvailable_decideNothing_untilTheyArrive() { + assertEquals(null, tracker.onAvailable("wifi", usable = null)) + assertEquals(false, tracker.onCapabilitiesChanged("wifi", usable = true)) + assertEquals(true, tracker.onLost("wifi")) + } } diff --git a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt index 1c8f151752..718eb57d2f 100644 --- a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt +++ b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt @@ -80,7 +80,7 @@ class ReplayWakeLockTest { ) // The minute tick's call, with no engine start or durable save in flight. - private fun tick() = replayWakeLock.hold(replayDue()) + private fun tick() = replayWakeLock.tick(otherHolds = false) private val stuck = 1_500_000L // more than one commit batch behind private val target = 1_556_896L @@ -219,10 +219,64 @@ class ReplayWakeLockTest { // The network back: the impediment update re-checks. networkImpeded = false - replayWakeLock.reacquireIfDue() + replayWakeLock.recheck() assertTrue(lock.isHeld) } + @Test + fun aRecheckThatReadDueBeforeANetworkLoss_isUndoneByTheLossesOwnRecheck() { + // PastaClaw on #1632: the re-acquire read "due" outside the lock's + // monitor and acquired afterwards, and the network loss in between + // could only ever re-acquire. The lock stayed held offline until a tick, + // which does not come while the CPU sleeps. Now the read and the change + // happen under one monitor, and the loss's own recheck, which runs after + // the stale one, releases. + exhaustTheWatchdog() + tick() + assertFalse(lock.isHeld) + + var lossLands = true + val racing = ReplayWakeLock( + lock = { lock }, + sdkPath = { true }, + replayDue = { + replayDue().also { + // The network drops right after the recovery's read. + if (lossLands) { + lossLands = false + networkImpeded = true + } + } + } + ) + check(121 * 60_000L, height = stuck + 1) // the wedge clears + racing.recheck() // the recovery: read "due", then the loss, then acquire + assertTrue("the stale read still acquired", lock.isHeld) + + racing.recheck() // the impediment update of the loss + assertFalse("offline: the replay does not keep the CPU", lock.isHeld) + assertFalse(racing.held) + } + + @Test + fun aNetworkLoss_releasesAtOnce_butNotWhatOnlyTheTickCanSee() { + tick() + assertTrue(lock.isHeld) + + networkImpeded = true + replayWakeLock.recheck() + assertFalse("no tick needed to let the CPU sleep when offline", lock.isHeld) + + // An engine start in flight is a hold the events cannot read: the + // tick records it, and a recheck keeps honouring it. + replayWakeLock.tick(otherHolds = true) + assertTrue(lock.isHeld) + replayWakeLock.recheck() + assertTrue("the engine start still needs the CPU", lock.isHeld) + replayWakeLock.tick(otherHolds = false) + assertFalse(lock.isHeld) + } + @Test fun aWedgeClearingAfterTeardownBegan_acquiresNothing() { replayWakeLock.observeWedgeClears(scope, decider.standDownClears) diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt index 8524c72d25..02c49154c5 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/BindHealL1StarterTest.kt @@ -53,6 +53,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -78,6 +79,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { tearingDown }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -110,6 +112,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, // teardown already finished + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -136,6 +139,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -161,6 +165,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -190,6 +195,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) @@ -202,6 +208,56 @@ class BindHealL1StarterTest { assertFalse(starter.isArmed) } + @Test + fun aShutdownBetweenTheChecksAndTheStart_keepsL1Down() = runBlocking { + // PastaClaw / CodeRabbit on #1632: the lifetime and teardown checks are + // not atomic with the start. A shutdown that passes them and finishes the + // engine stop before the start runs used to be invisible to it: the + // start sampled the stop generation itself, AFTER that stop, and + // started L1 with no service. The fake engine refuses as + // startIfEnabled(expectedExternalStopGeneration) does under its mutex. + val bound = MutableStateFlow(false) + var starts = 0 + var generation = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + // The last check passes, then the shutdown's engine stop completes. + serviceTearingDown = { false.also { generation++ } }, + stopGeneration = { generation }, + startL1 = { expected -> (expected == generation).also { if (it) starts++ } } + ) + + starter.arm() + bound.value = true + + assertEquals("a start decided before the stop does not outlive it", 0, starts) + assertFalse(starter.isArmed) + } + + @Test + fun aStopBeforeTheHeal_doesNotBlockTheStart() = runBlocking { + // The generation is sampled at the heal, not at arm(): a stop/start the + // engine went through while the waiter waited (and which declined, the + // bind still failing) must not leave the heal unable to start it. + val bound = MutableStateFlow(false) + var starts = 0 + var generation = 0L + val starter = BindHealL1Starter( + scope = scope, + bindEstablished = bound, + serviceTearingDown = { false }, + stopGeneration = { generation }, + startL1 = { expected -> (expected == generation).also { if (it) starts++ } } + ) + + starter.arm() + generation++ // a recovery path stopped the engine while the bind was failing + bound.value = true + + assertEquals(1, starts) + } + @Test fun aLaterArmReplacesAnEarlierOne() = runBlocking { val bound = MutableStateFlow(false) @@ -210,6 +266,7 @@ class BindHealL1StarterTest { scope = scope, bindEstablished = bound, serviceTearingDown = { false }, + stopGeneration = { 0L }, startL1 = { starts++; true } ) starter.arm() From 32a837ba6b607de29532487fb4dad5743eeb2593 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Fri, 23 Oct 2026 21:09:23 -0700 Subject: [PATCH 14/20] fix(sdk): close the L1 funding gate while the engine waits for peers WAITING_FOR_CONNECTIONS maps to CONNECTING, and after a route loss or a restart that snapshot can keep the previous session's heights, which pass every height rule while the engine cannot see a new block. The gate admitted it, so a wallet->shielded transfer or an SDK L1 send could open with sync offline. CONNECTING now fails closed like IDLE; the display already reads it as offline. Co-Authored-By: Claude Opus 5.5 --- .../platform/sdk/ShieldedBalanceServiceImpl.kt | 9 ++++++++- .../service/platform/sdk/SdkL1SendServiceTest.kt | 12 ++++++++++++ .../platform/sdk/ShieldedBalanceServiceTest.kt | 5 +++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt index 0edc787d45..2d374aedd0 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt @@ -232,7 +232,8 @@ internal data class WalletFundingGate(val allowed: Boolean, val reason: String) * live 1 Hz feed, which resets to [ShadowSyncProgress.IDLE] the moment the * engine stops — so IDLE IS the "stale/not running" state, no clock * needed): - * - engine running: phase is not [ShadowSyncPhase.IDLE]; + * - engine running and connected: phase is not [ShadowSyncPhase.IDLE] or + * [ShadowSyncPhase.CONNECTING]; * - no sync error: phase is not [ShadowSyncPhase.ERROR]; * - chain view current: [ShadowSyncProgress.scanCaughtUpToTip] — the header * chain is at a known tip AND the wallet-relevant filter scan has caught @@ -253,6 +254,12 @@ internal fun evaluateWalletFundingGate( ): WalletFundingGate = when { progress.phase == ShadowSyncPhase.IDLE -> WalletFundingGate(false, "SDK L1 engine not running") + // Waiting for peers: after a route loss or a restart the snapshot can keep + // the previous session's heights, which would pass every height rule below + // while the engine cannot see a single new block (PastaClaw on #1632). The + // display already reads this phase as offline (SdkBlockchainStateMapper). + progress.phase == ShadowSyncPhase.CONNECTING -> + WalletFundingGate(false, "SDK L1 engine is waiting for peers") progress.phase == ShadowSyncPhase.ERROR -> WalletFundingGate(false, "SDK L1 engine reported a sync error") !progress.scanCaughtUpToTip -> diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt index a9a79a757e..1782fa70c7 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt @@ -521,6 +521,18 @@ class SdkL1SendServiceTest { assertEquals(0, source.sendCalls) } + @Test + fun gateClosed_waitingForPeers_withThePreviousSessionsHeights_isNotBroadcast() = runBlocking { + // PastaClaw on #1632: WAITING_FOR_CONNECTIONS maps to CONNECTING while + // the snapshot keeps heights that read caught up. + val source = readySource() + val connecting = caughtUpProgress().copy(phase = ShadowSyncPhase.CONNECTING) + val result = service(source, progress = { connecting }) + .sendToAddress(validAddress, amount, emptyWallet = false) + assertTrue(result is SdkWriteResult.NotBroadcast) + assertEquals(0, source.sendCalls) + } + @Test fun gateClosed_scanNotCaughtUp_isNotBroadcast() = runBlocking { val source = readySource() diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt index 753424b6b7..4fb8e8ec8d 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt @@ -1534,6 +1534,11 @@ class ShieldedBalanceServiceTest { assertFalse(evaluateWalletFundingGate(ShadowSyncProgress.IDLE).allowed) // Engine errored — closed even with a complete scan snapshot. assertFalse(evaluateWalletFundingGate(caughtUp.copy(phase = ShadowSyncPhase.ERROR)).allowed) + // Waiting for peers (PastaClaw on #1632): the previous session's heights + // still read caught up, but the engine cannot see a new block. + val connecting = evaluateWalletFundingGate(caughtUp.copy(phase = ShadowSyncPhase.CONNECTING)) + assertFalse(connecting.allowed) + assertEquals("SDK L1 engine is waiting for peers", connecting.reason) // Genuinely mid-scan: headers at the tip but the filter scan far // behind (the real "still scanning" state) stays closed. The aggregate From b55f1ca3985acf01611e74cfa73ef066d4cce1af Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 24 Oct 2026 07:53:15 -0700 Subject: [PATCH 15/20] fix(sdk): close the L1 funding gate while the wallet cursor is unknown The pipeline-lag veto only applied when walletSyncedHeight > 0, so a snapshot with filters at the tip and no cursor evidence opened a spend over a UTXO view nothing proved current. An unknown cursor now closes the gate ("SDK L1 wallet scan cursor is not known yet"). It cannot wedge a synced wallet: the cursor is seeded at engine start from the SDK's durable WalletEntity.syncedHeight, and dash-spv emits SyncHeightAdvanced for every wallet each committed filter batch covered, matches or not. On the emulator a fresh testnet wallet reported a non-zero cursor from its first batch, and its restart was seeded from the durable value. Co-Authored-By: Claude Opus 5.5 --- .../sdk/ShieldedBalanceServiceImpl.kt | 36 +++++++++++++------ .../platform/sdk/SdkL1SendServiceTest.kt | 18 +++++++++- .../sdk/ShieldedBalanceServiceTest.kt | 14 ++++++-- 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt index 2d374aedd0..00891bd419 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceImpl.kt @@ -243,7 +243,9 @@ internal data class WalletFundingGate(val allowed: Boolean, val reason: String) * - filter scan within [ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS] of the * tip: the HEIGHT rule, even where the display's aggregate rule says * caught up; - * - block pipeline drained: not [ShadowSyncProgress.blockPipelineLagging]. + * - wallet cursor known: [ShadowSyncProgress.walletSyncedHeight] above 0; + * - block pipeline drained: not [ShadowSyncProgress.blockPipelineLagging], + * and the wallet cursor within tolerance of the tip. * * Wallet binding is NOT re-checked here: every caller already preflights * the bound wallet, and an unbound wallet cannot produce a non-IDLE @@ -269,11 +271,11 @@ internal fun evaluateWalletFundingGate( // tolerates a filter shortfall of up to 0.3% of chain height (~4,600 // blocks on testnet) — right for a label, wrong for funding: an unscanned // block can hold a spend of a UTXO this wallet still reads unspent, or a - // receive the balance is missing. The pipeline-lag veto below cannot cover - // that gap when the wallet cursor is unknown (0), and when the cursor IS - // known the §34 stall parks it with the filter cursor, so that veto - // closes the gate there anyway. The funds-safety argument of - // SCAN_TIP_TOLERANCE_BLOCKS needs the scan itself within tolerance. + // receive the balance is missing. The cursor rules below close that gap + // too (an unknown cursor, or one the §34 stall parks with the filter + // cursor), but this check names the cause and does not depend on the + // cursor feed. The funds-safety argument of SCAN_TIP_TOLERANCE_BLOCKS + // needs the scan itself within tolerance. // Measured against the HIGHEST tip the snapshot knows, not headerTarget // alone: dash-spv restores the header target from the last peer report // at engine start, which can sit below the stored header tip until a peer @@ -288,15 +290,27 @@ internal fun evaluateWalletFundingGate( // wallet already spent still reads unspent, and a transfer built on it is // a double-spend the network rejects (the 48.86 DASH incident's shape). // The wallet cursor within tolerance of the tip is the evidence those - // blocks are through; until then the gate stays closed. Unknown cursor - // (0) never counts as lagging, so this cannot wedge a wallet at the tip. + // blocks are through; until then the gate stays closed. + // + // An UNKNOWN cursor (0) is no evidence either way, so it closes too + // (PastaClaw on #1632: filters at the tip with the cursor unknown opened a + // spend over a UTXO view nothing proved current). It cannot wedge a synced + // wallet: the cursor is seeded at engine start from the SDK's durable + // `WalletEntity.syncedHeight`, and dash-spv emits `SyncHeightAdvanced` for + // every wallet a committed filter batch covered, matches or not, which is + // also what the durable value is written from. A fresh testnet wallet on + // the emulator (2026-10-24) logged a non-zero cursor from its first batch + // on, and its restart was seeded from the durable value. Unknown is left + // for a failed seed read before the next batch commits, or a wallet no + // batch has covered yet. + progress.walletSyncedHeight <= 0L -> + WalletFundingGate(false, "SDK L1 wallet scan cursor is not known yet") // Measured against the same highest-known tip as the height rule above: // `blockPipelineLagging` reads headerTarget alone, which a restart can // leave below the stored tip while the filters already reach it. progress.blockPipelineLagging || - (progress.walletSyncedHeight > 0 && - fundingTipHeight(progress) - progress.walletSyncedHeight > - ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS) -> + fundingTipHeight(progress) - progress.walletSyncedHeight > + ShadowSyncProgress.SCAN_TIP_TOLERANCE_BLOCKS -> WalletFundingGate(false, "SDK L1 block pipeline is still processing matched blocks") else -> WalletFundingGate(true, "SDK L1 filter scan caught up to the chain tip") } diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt index 1782fa70c7..09d1309990 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt @@ -208,7 +208,9 @@ class SdkL1SendServiceTest { headerHeight = 1_500_000, headerTarget = 1_500_000, filterHeight = 1_500_000, - filterTarget = 1_500_000 + filterTarget = 1_500_000, + // A known wallet cursor at the tip: an unknown one closes the gate. + walletSyncedHeight = 1_500_000 ) private var selfSpendMarks = 0 @@ -584,6 +586,20 @@ class SdkL1SendServiceTest { assertEquals(0, source.sendCalls) } + @Test + fun gateClosed_unknownWalletCursor_atTheTip_isNotBroadcast() = runBlocking { + // PastaClaw on #1632: filters at the tip, but no evidence the matched + // blocks were applied — the UTXO view may still miss a spend. + val source = readySource() + val unknownCursor = caughtUpProgress().copy(walletSyncedHeight = 0) + val result = service(source, progress = { unknownCursor }) + .sendToAddress(validAddress, amount, emptyWallet = false) + assertTrue(result is SdkWriteResult.NotBroadcast) + val reason = (result as SdkWriteResult.NotBroadcast).reason + assertTrue(reason, reason.contains("cursor is not known")) + assertEquals(0, source.sendCalls) + } + @Test fun gateClosed_aggregateRuleOnly_withUnknownWalletCursor_isNotBroadcast() = runBlocking { // The §34 shape: filters 2,167 short of the tip, the aggregate at diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt index 4fb8e8ec8d..6e74594026 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/ShieldedBalanceServiceTest.kt @@ -335,7 +335,9 @@ class ShieldedBalanceServiceTest { headerHeight = 1_500_000, headerTarget = 1_500_000, filterHeight = 1_500_000, - filterTarget = 1_500_000 + filterTarget = 1_500_000, + // A known wallet cursor at the tip: an unknown one closes the gate. + walletSyncedHeight = 1_500_000 ) private fun service( @@ -1624,8 +1626,14 @@ class ShieldedBalanceServiceTest { val lagging = evaluateWalletFundingGate(caughtUp.copy(walletSyncedHeight = 1_200_000)) assertFalse(lagging.allowed) assertTrue(lagging.reason, lagging.reason.contains("processing matched blocks")) - // Unknown cursor (0) is not lag: the gate cannot wedge a wallet at the tip. - assertTrue(evaluateWalletFundingGate(caughtUp.copy(walletSyncedHeight = 0)).allowed) + // An unknown cursor (0) is no evidence the matched blocks are through + // (PastaClaw on #1632): closed, with filters at the tip and no lag the + // display could see. + val unknownCursor = caughtUp.copy(walletSyncedHeight = 0) + assertFalse("no lag the display could see", unknownCursor.blockPipelineLagging) + val unknownGate = evaluateWalletFundingGate(unknownCursor) + assertFalse(unknownGate.allowed) + assertEquals("SDK L1 wallet scan cursor is not known yet", unknownGate.reason) } @Test From 5d6926aaad49c538d3cd1dedd6943d88e99a0646 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 24 Oct 2026 09:20:41 -0700 Subject: [PATCH 16/20] fix(sync): ignore capability updates for a network that is not the default DefaultNetworkTracker.onCapabilitiesChanged made whatever network it was told about the current default. A late update for the former default, delivered after its successor's onAvailable, took it back (and could mark the device offline), and that network's later onLost then matched and left the tracker offline with the successor still up. Only onAvailable now changes the default; an update for any other network returns no verdict, and the callback updates the impediment and re-checks only when there is one. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 12 +++++++---- .../wallet/service/NetworkAvailability.kt | 16 ++++++++------ .../wallet/service/NetworkAvailabilityTest.kt | 21 +++++++++++++++++++ 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index e932c9787f..a5296a481f 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -1922,11 +1922,15 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { log.info(s.toString()) } - impedimentGate.networkCallbackUpdate { - applyNetworkVerdict(defaultNetwork.onCapabilitiesChanged(network, hasInternet && hasValidated)) + // An update for a network that is not the tracked default says nothing. + val offline = impedimentGate.networkCallbackUpdate { + defaultNetwork.onCapabilitiesChanged(network, hasInternet && hasValidated) + .also(::applyNetworkVerdict) + } + if (offline != null) { + updateBlockchainStateImpediments() + check() } - updateBlockchainStateImpediments() - check() } } diff --git a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt index 4f41bc50b0..c3a53e4141 100644 --- a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt +++ b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt @@ -58,10 +58,14 @@ fun hasUsableNetworkNow(connectivityManager: ConnectivityManager): Boolean = try * about it is the whole answer. * * Each method returns the verdict: true offline, false online, null when the - * event decides nothing (a loss of a network that is no longer the default — - * its successor's `onAvailable` may already have been delivered — or an - * `onAvailable` whose capabilities could not be read; `onCapabilitiesChanged` - * follows it). Called in delivery order, one at a time. + * event decides nothing: a loss of, or a capability update for, a network + * that is not the tracked default (its successor's `onAvailable` may already + * have been delivered), or an `onAvailable` whose capabilities could not be + * read (`onCapabilitiesChanged` follows it). Only `onAvailable` changes which + * network is the default. A late update for the former default must not take + * it back (PastaClaw on #1632): its later `onLost` would then match and leave + * the tracker offline with the successor still up. Called in delivery order, + * one at a time. */ internal class DefaultNetworkTracker { private var current: N? = null @@ -71,8 +75,8 @@ internal class DefaultNetworkTracker { return usable?.not() } - fun onCapabilitiesChanged(network: N, usable: Boolean): Boolean { - current = network + fun onCapabilitiesChanged(network: N, usable: Boolean): Boolean? { + if (network != current) return null return !usable } diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt index 18aaa8b571..4e120eb82c 100644 --- a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -187,6 +187,27 @@ class NetworkAvailabilityTest { assertEquals(false, tracker.onAvailable("cell", usable = true)) } + @Test + fun aLateCapabilityUpdateFromTheFormerDefault_doesNotTakeItBack() { + // PastaClaw on #1632: an update for A delivered after B's onAvailable + // used to make A current again (and offline), and A's later onLost + // then matched and left the tracker offline with B still up. + assertEquals(false, tracker.onAvailable("wifi", usable = true)) + assertEquals(false, tracker.onAvailable("cell", usable = true)) // the handoff + assertEquals("a stale update for the former default says nothing", null, + tracker.onCapabilitiesChanged("wifi", usable = false)) + assertEquals("…nor does its loss", null, tracker.onLost("wifi")) + // The successor is still the default and still decides. + assertEquals(false, tracker.onCapabilitiesChanged("cell", usable = true)) + assertEquals(true, tracker.onLost("cell")) + } + + @Test + fun aCapabilityUpdateBeforeAnyDefault_decidesNothing() { + assertEquals(null, tracker.onCapabilitiesChanged("wifi", usable = true)) + assertEquals(null, tracker.onLost("wifi")) + } + @Test fun capabilitiesUnreadableOnAvailable_decideNothing_untilTheyArrive() { assertEquals(null, tracker.onAvailable("wifi", usable = null)) From 712e1a1044655ee74f5d403ce46775fefc172fc0 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 24 Oct 2026 10:57:19 -0700 Subject: [PATCH 17/20] fix(sync): serialize the watchdog's event-driven clears with its stall decisions onFilterProgress() and onWalletEvent() read the stand-down and cleared it outside the monitor onCheck() holds. A callback that read an old stand-down could clear a newer one a check had just decided, and the clear re-took the replay wake lock on a wedged engine until the next check. Both are now synchronized on the decider, so the read and the clear judge the state the last decision left. Co-Authored-By: Claude Opus 5.5 --- .../platform/sdk/L1ShadowSyncService.kt | 13 +++-- .../platform/sdk/L1ShadowSyncServiceTest.kt | 50 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt index d7186942d5..ef2cf0848d 100644 --- a/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt +++ b/wallet/src/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncService.kt @@ -1356,15 +1356,22 @@ internal class FilterStallWatchdogDecider( * tap see the engine move as it happens: a filter cursor off the height the * watchdog stood down at, or any wallet event (the same liveness rule * [onCheck] applies), ends the stand-down at once. Only [standingDown] is - * touched, so this is safe from those collectors' threads; [onCheck] - * re-times the next spell itself. A 0 height is a snapshot with no filter - * position (IDLE / CONNECTING), not movement. + * touched; [onCheck] re-times the next spell itself. A 0 height is a + * snapshot with no filter position (IDLE / CONNECTING), not movement. + * + * Synchronized with [onCheck], because the read and the clear must be one + * step (PastaClaw on #1632). Otherwise a callback could read an old + * stand-down, a check could stand down again for a newer wedge, and the + * callback's clear would then end that one: a clear [standDownClears] + * reports, which re-takes the replay wake lock on a wedged engine. */ + @Synchronized fun onFilterProgress(filterHeight: Long) { if (standingDown && filterHeight > 0L && filterHeight != stoodDownAtHeight) standingDown = false } /** See [onFilterProgress]. */ + @Synchronized fun onWalletEvent() { if (standingDown) standingDown = false } diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt index 0363ba1a5b..e40016bad2 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/L1ShadowSyncServiceTest.kt @@ -54,6 +54,9 @@ import org.junit.Assert.assertTrue import org.junit.Test import java.io.File import java.nio.file.Files +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import kotlin.concurrent.thread /** * Host-JVM tests for the Phase 5a L1 shadow-sync parity harness: the @@ -742,6 +745,53 @@ class L1ShadowSyncServiceTest { assertFalse(d.standingDown) } + @Test + fun filterStall_theEventDrivenClears_waitForAStallDecisionInProgress() { + // PastaClaw on #1632: the progress and wallet-event callbacks read the + // stand-down and cleared it outside onCheck's monitor. A callback that + // read an old stand-down, then cleared after a check stood down again + // for a newer wedge, ended that one, and the clear re-took the replay + // wake lock on a wedged engine. Holding the decider's monitor here + // stands in for an onCheck in progress: the callbacks must wait for it, + // and then judge the state it left. + val d = stallDecider() + val stuck = 1_500_000L // more than one commit batch behind + val target = 1_556_896L + fun check(atMs: Long, height: Long) = + d.onCheck(atMs, height, target, lastWalletEventMs = 0L, walletSyncedHeight = stuck) + + check(0L, stuck) + check(10 * 60_000L + 1, stuck) + check(30 * 60_000L + 2, stuck) + check(60 * 60_000L + 3, stuck) + check(120 * 60_000L, stuck) + assertTrue(d.standingDown) + + val progressDone = CountDownLatch(1) + synchronized(d) { + thread { d.onFilterProgress(stuck + 1); progressDone.countDown() } + assertFalse( + "the clear waits for the decision in progress", + progressDone.await(200, TimeUnit.MILLISECONDS) + ) + // That decision: the cursor moved one block, then wedged there. + check(200 * 60_000L, stuck + 1) + check(300 * 60_000L, stuck + 1) + assertTrue(d.standingDown) + } + assertTrue(progressDone.await(5, TimeUnit.SECONDS)) + assertTrue("the stand-down at the new height stands", d.standingDown) + + // The wallet-event clear is serialized the same way. + val eventDone = CountDownLatch(1) + synchronized(d) { + thread { d.onWalletEvent(); eventDone.countDown() } + assertFalse(eventDone.await(200, TimeUnit.MILLISECONDS)) + } + assertTrue(eventDone.await(5, TimeUnit.SECONDS)) + assertFalse("a wallet event after the decision still clears it", d.standingDown) + } + @Test fun filterStall_aRunRestartedAtTheStoodDownHeight_getsAFullStallWindow() { // PastaClaw on #1632: the stop cleared the flag but kept the old run's From 0445d94b8f296063fcd4abec7308eeb936bf8731 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sat, 24 Oct 2026 15:16:38 -0700 Subject: [PATCH 18/20] fix(sync): let only a network-callback verdict supersede the offline seed ImpedimentGate counted every network-callback update, including callbacks the default-network tracker answered with no verdict (an onAvailable whose capabilities could not be read, a loss of or update for a network that is not the default). One of those landing during registration skipped the start-up seed, and a service with no usable network read as online. networkCallbackUpdate now takes the verdict its block applies and counts only a non-null one; onUnavailable returns its offline verdict, and the teardown clear goes through update {}. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 9 ++++--- .../wallet/service/NetworkAvailability.kt | 27 ++++++++++++------- .../wallet/service/NetworkAvailabilityTest.kt | 18 +++++++++++++ 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index a5296a481f..bdb0bdcfac 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -1831,7 +1831,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } private var networkCallbackRegistered = false private lateinit var networkCallback: ConnectivityManager.NetworkCallback - /** The default network's verdict; touched only inside [ImpedimentGate.networkCallbackUpdate]. */ + /** The default network's verdict; touched only under [ImpedimentGate]'s lock. */ private val defaultNetwork = DefaultNetworkTracker() /** Serializes changes to [impediments] and decides the start-up connectivity seed. */ private val impedimentGate = ImpedimentGate() @@ -1938,7 +1938,10 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { super.onUnavailable() serviceScope.launch(networkEventDispatcher) { log.info("network unavailable - no network satisfies request") - impedimentGate.networkCallbackUpdate { impediments.add(Impediment.NETWORK) } + impedimentGate.networkCallbackUpdate { + impediments.add(Impediment.NETWORK) + true // offline: a verdict, which supersedes the start-up seed + } updateBlockchainStateImpediments() check() } @@ -3498,7 +3501,7 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { if (networkCallbackRegistered) { connectivityManager.unregisterNetworkCallback(networkCallback) networkCallbackRegistered = false - impedimentGate.networkCallbackUpdate { defaultNetwork.clear() } + impedimentGate.update { defaultNetwork.clear() } } log.info("receivers unregistered, Now starting coroutine to finish the rest of the cleanup") diff --git a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt index c3a53e4141..2204756fed 100644 --- a/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt +++ b/wallet/src/de/schildbach/wallet/service/NetworkAvailability.kt @@ -103,12 +103,16 @@ internal class DefaultNetworkTracker { * a network lost between the read and the registration was never reported * (nothing matched any more), and the service read as online. So the read * comes AFTER registration, where it must not overwrite what the callback has - * already said: every callback update bumps a counter, the token is taken - * just before registering, and [seedOffline] marks the service offline only if - * no callback update landed since. A default network that is present after - * the registration is reported by the callback (`onAvailable`, later - * `onLost`), so the seed only ever decides the "no default network" case the - * callback stays silent on. + * already said: every callback update that reached a verdict bumps a counter, + * the token is taken just before registering, and [seedOffline] marks the + * service offline only if no such update landed since. A callback that + * decided nothing (a loss of, or an update for, a network that is not the + * default; an `onAvailable` whose capabilities could not be read) does not + * count: it said nothing the seed would overwrite, and counting it skipped + * the seed with no network usable (PastaClaw on #1632). A default network + * that is present after the registration is reported by the callback + * (`onAvailable`, later `onLost`), so the seed only ever decides the "no + * default network" case the callback stays silent on. */ internal class ImpedimentGate { private val lock = Any() @@ -117,10 +121,13 @@ internal class ImpedimentGate { /** A change that is not a network-callback update (the keystore health observer). */ fun update(block: () -> T): T = synchronized(lock, block) - /** A network-callback update; it supersedes the start-up seed. */ - fun networkCallbackUpdate(block: () -> T): T = synchronized(lock) { - networkCallbackUpdates++ - block() + /** + * A network-callback update. [block] applies its verdict and returns it: + * true offline, false online, null when it decided nothing. Only a + * verdict supersedes the start-up seed. + */ + fun networkCallbackUpdate(block: () -> Boolean?): Boolean? = synchronized(lock) { + block().also { if (it != null) networkCallbackUpdates++ } } /** Taken just BEFORE `registerNetworkCallback`. */ diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt index 4e120eb82c..35a9581702 100644 --- a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -101,6 +101,24 @@ class NetworkAvailabilityTest { assertFalse("the callback's online verdict stands", network in impediments) } + @Test + fun aCallbackThatDecidedNothing_doesNotSkipTheSeed() { + // PastaClaw on #1632: every callback used to count, so an onAvailable + // whose capabilities could not be read (or a stale event for a former + // default) landing during registration skipped the seed, and a service + // with no usable network read as online. + val token = gate.seedToken() + assertEquals(null, gate.networkCallbackUpdate { tracker.onAvailable("wifi", usable = null) }) + assertEquals(null, gate.networkCallbackUpdate { tracker.onLost("cell") }) + assertTrue(gate.seedOffline(token, usableNow = false) { impediments.add(network) }) + assertTrue(network in impediments) + + // A verdict still supersedes it. + val later = gate.seedToken() + assertEquals(false, gate.networkCallbackUpdate { tracker.onCapabilitiesChanged("wifi", usable = true) }) + assertFalse(gate.seedOffline(later, usableNow = false) { impediments.add(network) }) + } + @Test fun aCallbackUpdateAfterTheSeed_overridesIt() { val token = gate.seedToken() From a9dd29cae5c0bb165f6e285f46eb8dbb4b1f3dec Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Sun, 25 Oct 2026 21:27:30 -0700 Subject: [PATCH 19/20] fix(sync): read the replay wake lock's other holds live, not from the tick QA on #1632 (D-1632-01): on a network loss during a large replay the replay wake lock was released only at the next minute tick (53 s and 11 s on the emulator). recheck() decided with tickHolds || replayDue(), and tickHolds was the tick's engineStarting || durableLagging, recorded once a minute; the durable save kept it true, so the loss's recheck could not release. ReplayWakeLock now takes an otherHolds supplier and reads it in every recheck. BlockchainServiceImpl computes the same two holds live, the durable lag through DurableHeightTracker.msSinceChange so only the tick's observe() ages it, and the tick records then rechecks. Co-Authored-By: Claude Opus 5.5 --- .../wallet/service/BlockchainServiceImpl.kt | 30 ++++++++++--- .../wallet/service/ReplayWakeLock.kt | 39 +++++++---------- .../wallet/service/NetworkAvailabilityTest.kt | 5 ++- .../wallet/service/ReplayWakeLockTest.kt | 42 ++++++++++++++----- 4 files changed, 76 insertions(+), 40 deletions(-) diff --git a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt index bdb0bdcfac..7e8898cac9 100644 --- a/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt +++ b/wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt @@ -1000,7 +1000,8 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { } }, sdkPath = { dashjHeldByCutover }, - replayDue = { replayWakeLockDue() } + replayDue = { replayWakeLockDue() }, + otherHolds = { replayWakeLockOtherHolds(System.currentTimeMillis()) } ) private var peerConnectivityListener: PeerConnectivityListener? = null private var nm: NotificationManager? = null @@ -2503,9 +2504,10 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { durableHeightTracker.observe(durableHeight, System.currentTimeMillis()) ) // A replay that cannot make progress keeps the service up but releases the CPU. - // ReplayWakeLock reads the replay's part itself (replayWakeLockDue), the same - // way the event rechecks do; the tick adds what only it can see. - replayWakeLock.tick(otherHolds = engineStarting || durableLagging) + // ReplayWakeLock reads both parts live (replayWakeLockDue, replayWakeLockOtherHolds), + // the same way the event rechecks do; the observe() above is what ages the + // durable height for them. + replayWakeLock.recheck() // Those two holds also keep the service in the foreground: the scan reads // synced while they last, and a demoted service would be stopped anyway. if (foregroundDemotionGate.onHold(sdkHoldsService(System.currentTimeMillis()))) { @@ -3816,7 +3818,25 @@ class BlockchainServiceImpl : LifecycleService(), BlockchainService { ) } - /** The replay half of the tick's hold decision, read now. */ + /** + * The rest of the replay wake lock's decision: an SDK engine start in flight, + * or the SDK still saving the scan. The same two holds the tick computes, read + * live so an event recheck sees them as they are now, not as the last tick + * recorded them (D-1632-01). [DurableHeightTracker.msSinceChange] reads the + * tracker without recording, so only the tick ages the durable height. + */ + private fun replayWakeLockOtherHolds(nowMs: Long): Boolean { + if (!dashjHeldByCutover) return false + if (engineStartHoldsService(l1ShadowSyncService.engineStartingSinceMs(), nowMs)) return true + val durableHeight = l1ShadowSyncService.durableSyncedHeight() + return durableLagHoldsService( + durableHeight, + l1ShadowSyncService.engineCommittedHeight(), + durableHeightTracker.msSinceChange(durableHeight, nowMs) + ) + } + + /** The replay half of the hold decision, read now. */ private fun replayWakeLockDue(): Boolean = dashjHeldByCutover && replayHoldsWakeLock( replaying = blockchainState?.replaying == true && !sdkBindBlocked(), networkImpeded = impediments.contains(Impediment.NETWORK), diff --git a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt index 48230109bc..e263fdc1b3 100644 --- a/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt +++ b/wallet/src/de/schildbach/wallet/service/ReplayWakeLock.kt @@ -41,10 +41,10 @@ interface ReplayWakeLockHandle { * progress ([replayHoldsWakeLock]: offline, or wedged with the filter-stall * watchdog's restarts spent). * - * Who decides: everyone, the same way. The minute tick ([tick]) and every - * event that can change the answer ([recheck]: an impediment change, the - * filter wedge clearing via [observeWedgeClears]) read the decision NOW and - * acquire or release to match, under one monitor. ACTION_TIME_TICK is not + * Who decides: everyone, the same way. The minute tick and every event that + * can change the answer (an impediment change, the filter wedge clearing via + * [observeWedgeClears]) call [recheck], which reads the decision NOW and + * acquires or releases to match, under one monitor. ACTION_TIME_TICK is not * delivered while the CPU sleeps, so a replay that can move again must not * wait for the tick to get the lock back. * @@ -54,10 +54,12 @@ interface ReplayWakeLockHandle { * "not due", and releases. (An acquire-only re-check used to leave that lock * held until the next tick, so the replay held the CPU while offline.) * - * The tick also knows holds the events cannot see (an engine start, a durable - * save in flight). It records them, and a recheck honours the last recorded - * value. A stale one only keeps the lock, and a held lock keeps the CPU awake, - * so the next tick comes and corrects it. + * Both halves of the decision are read live: the replay's ([replayDue]) and + * the holds besides it ([otherHolds]: an engine start or a durable save in + * flight). The other holds used to be the tick's value, recorded once a + * minute. During a large replay the durable save keeps that value true, so a + * network loss could not release the lock until the next tick: up to a minute + * held offline, the waste this class exists to prevent (QA on #1632, D-1632-01). * * Tracked apart from `isHeld` because the lock is reference-counted and shared * with the dashj path ([releaseForTeardown] releases only this owner's count). @@ -65,11 +67,14 @@ interface ReplayWakeLockHandle { * @param lock the service's wake lock; null before onCreate assigns it. * @param sdkPath whether the SDK owns L1 (the dashj path manages its own hold). * @param replayDue the replay half of the hold decision, read now. + * @param otherHolds the rest of it (an engine start, a durable save), read now + * from any thread. */ internal class ReplayWakeLock( private val lock: () -> ReplayWakeLockHandle?, private val sdkPath: () -> Boolean, - private val replayDue: () -> Boolean + private val replayDue: () -> Boolean, + private val otherHolds: () -> Boolean ) { private val log = LoggerFactory.getLogger(ReplayWakeLock::class.java) private val guard = Any() @@ -87,25 +92,13 @@ internal class ReplayWakeLock( @Volatile private var retired = false - /** The tick's holds the events cannot read; written and read under [guard]. */ - private var tickHolds = false - - /** - * The minute tick. [otherHolds]: an engine start or a durable save needs - * the CPU (the replay's own part is read by [recheck]). - */ - fun tick(otherHolds: Boolean) { - synchronized(guard) { tickHolds = otherHolds } - recheck() - } - - /** Hold the lock iff the replay (or the tick's last other holds) needs the CPU now. */ + /** Hold the lock iff the replay, an engine start or a durable save needs the CPU now. */ fun recheck() { if (!sdkPath()) return val lock = lock() ?: return synchronized(guard) { try { - val due = tickHolds || replayDue() + val due = otherHolds() || replayDue() if (due && !held && !retired) { log.info("replay or engine start in progress on the SDK path — acquiring the wake lock") lock.acquire() diff --git a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt index 35a9581702..b9a823d469 100644 --- a/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt +++ b/wallet/test/de/schildbach/wallet/service/NetworkAvailabilityTest.kt @@ -159,9 +159,10 @@ class NetworkAvailabilityTest { val replayWakeLock = ReplayWakeLock( lock = { lock }, sdkPath = { true }, - replayDue = { replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false) } + replayDue = { replayHoldsWakeLock(replaying = true, networkImpeded = impeded, filterScanWedged = false) }, + otherHolds = { false } ) - replayWakeLock.tick(otherHolds = false) + replayWakeLock.recheck() // the tick assertFalse("offline from the start: no wake lock for a replay that cannot move", lock.isHeld) assertEquals(0, acquisitions) diff --git a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt index 718eb57d2f..6869796905 100644 --- a/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt +++ b/wallet/test/de/schildbach/wallet/service/ReplayWakeLockTest.kt @@ -73,14 +73,18 @@ class ReplayWakeLockTest { filterScanWedged = decider.standingDown ) + // An engine start or a durable save in flight, as BlockchainServiceImpl reads it live. + private var otherHolds = false + private val replayWakeLock = ReplayWakeLock( lock = { lock }, sdkPath = { true }, - replayDue = ::replayDue + replayDue = ::replayDue, + otherHolds = { otherHolds } ) - // The minute tick's call, with no engine start or durable save in flight. - private fun tick() = replayWakeLock.tick(otherHolds = false) + // The minute tick's call. + private fun tick() = replayWakeLock.recheck() private val stuck = 1_500_000L // more than one commit batch behind private val target = 1_556_896L @@ -247,7 +251,8 @@ class ReplayWakeLockTest { networkImpeded = true } } - } + }, + otherHolds = { otherHolds } ) check(121 * 60_000L, height = stuck + 1) // the wedge clears racing.recheck() // the recovery: read "due", then the loss, then acquire @@ -259,7 +264,7 @@ class ReplayWakeLockTest { } @Test - fun aNetworkLoss_releasesAtOnce_butNotWhatOnlyTheTickCanSee() { + fun aNetworkLoss_releasesAtOnce_unlessAnEngineStartOrSaveStillNeedsTheCpu() { tick() assertTrue(lock.isHeld) @@ -267,16 +272,33 @@ class ReplayWakeLockTest { replayWakeLock.recheck() assertFalse("no tick needed to let the CPU sleep when offline", lock.isHeld) - // An engine start in flight is a hold the events cannot read: the - // tick records it, and a recheck keeps honouring it. - replayWakeLock.tick(otherHolds = true) - assertTrue(lock.isHeld) + // An engine start or a durable save in flight is local work that goes + // on offline: it holds the lock, and any recheck sees it. + otherHolds = true replayWakeLock.recheck() assertTrue("the engine start still needs the CPU", lock.isHeld) - replayWakeLock.tick(otherHolds = false) + otherHolds = false + replayWakeLock.recheck() assertFalse(lock.isHeld) } + @Test + fun aNetworkLoss_afterTheDurableSaveCaughtUp_releasesWithoutATick() { + // QA on #1632, D-1632-01: the tick recorded the durable-save hold as + // true during a large replay, and the event recheck read that recorded + // value. A network loss after the save had caught up then kept the lock + // until the next tick: 53 s and 11 s on the emulator. + otherHolds = true // the durable save is behind + tick() + assertTrue(lock.isHeld) + + otherHolds = false // it caught up — no tick has run since + networkImpeded = true + replayWakeLock.recheck() // the loss's impediment update + assertFalse("released by the network event, not the next tick", lock.isHeld) + assertFalse(replayWakeLock.held) + } + @Test fun aWedgeClearingAfterTeardownBegan_acquiresNothing() { replayWakeLock.observeWedgeClears(scope, decider.standDownClears) From 51972637678df83fb35304d4f9ca1fe04c961415 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Tue, 27 Oct 2026 12:35:13 -0700 Subject: [PATCH 20/20] test(sdk): an unknown wallet cursor is refused before a deferred build reserves #1616's aDeferredBuildWithNoCommittedCursorRecordsNoHeight built a deferred Maya deposit with walletSyncedHeight = 0 and expected a null reservation height. With this branch's funding gate an unknown cursor is refused first ("SDK L1 wallet scan cursor is not known yet"), so that build is no longer reachable. The test now asserts the refusal and that nothing was reserved; reservationHeightLowerBound()'s null stays as the defence behind the gate. Co-Authored-By: Claude Opus 5.5 --- .../platform/sdk/SdkL1SendServiceTest.kt | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt index 151fc6a4b5..3dd5891c57 100644 --- a/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt +++ b/wallet/test/de/schildbach/wallet/service/platform/sdk/SdkL1SendServiceTest.kt @@ -38,6 +38,7 @@ import org.dash.wallet.common.money.Dash import org.dashfoundation.dashsdk.errors.DashSdkError import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Assert.assertTrue @@ -2411,16 +2412,26 @@ class SdkL1SendServiceTest { } @Test - fun aDeferredBuildWithNoCommittedCursorRecordsNoHeight() = runBlocking { + fun aDeferredBuildWithNoCommittedCursor_isRefusedBeforeReserving() = runBlocking { // Review 28775c57d545: header and filter cursors at the tip say nothing about how far the - // engine has processed (that can be 100 blocks behind). Recording one as the reservation - // height would make an older reservation look fresh; unknown must stay unknown. + // engine has processed (that can be 100 blocks behind), so they must never stand in for + // the reservation height. Since #1632 an unknown cursor does not get that far: the funding + // gate refuses it ("wallet scan cursor is not known yet") before anything is reserved, so + // no reservation is ever stamped from an unknown cursor. reservationHeightLowerBound()'s + // null stays as the defence behind the gate. val source = mayaSource(spendable = 1_000_000L, feeDuffs = 500L) val noCommittedCursor = caughtUpProgress().copy(walletSyncedHeight = 0) - val payment = service(source, progress = { noCommittedCursor }) - .buildDeferredMayaDeposit(validAddress, 50_000L, ByteArray(40), stillOwned = { true }) + val refused = try { + service(source, progress = { noCommittedCursor }) + .buildDeferredMayaDeposit(validAddress, 50_000L, ByteArray(40), stillOwned = { true }) + null + } catch (e: IllegalStateException) { + e + } - assertNull(payment.reservedAtHeight) + assertNotNull("the gate refuses an unknown cursor", refused) + assertTrue(refused!!.message, refused.message!!.contains("cursor is not known")) + assertEquals("nothing was reserved", 0, source.mayaBuildCalls) } @Test