fix(sync): bound keep-alive ping and resume a sync stuck on lock/unlock

Three issues surfaced by testing the previous lock/unlock reconnect fix
on Android with a large wallet:

- The keep-alive ping had no timeout, so a "half-open" TCP connection
  (remote end gone with no FIN/RST ever delivered, the common outcome
  of Doze/mobile-radio suspend after a longer lock) left it awaiting a
  response that never arrives — the failure path that tears the dead
  client down was never reached. Bound the ping to 8s and added a
  re-entrancy guard so overlapping 20s ticks can't pile up concurrent
  reconnect attempts while one is stuck.

- CheckConnectionOnResumeAsync bailed out whenever a sync was already
  in progress, so a lock/unlock during an active sync (which has no
  per-request timeout of its own) left it hung indefinitely instead of
  recovering. It now cancels the stuck sync and tears down the dead
  client, marking the interruption as self-inflicted (_resumeRecovering)
  so the UI shows "reconnecting" and restarts immediately instead of a
  transient error message.

- WalletSynchronizer.ExportCaches persisted raw transaction bytes only
  for already-verified transactions, discarding anything downloaded but
  not yet through Merkle-proof verification when a sync was interrupted
  mid-way — forcing a full re-download of a large wallet's transactions
  on every resume instead of resuming straight into proof verification.
  Track confirmed txids at download time (_confirmedTxids), independent
  of verification status, and export/preload against that instead.

Also unified the three sync progress messages onto one template
reporting against the sync's total transaction/proof counts rather than
this session's download count, so a resumed sync immediately shows
"transactions n/n" instead of a misleading "0/0" before jumping into
proof verification.
This commit is contained in:
2026-07-19 17:11:39 +02:00
parent 94a474fe41
commit 4cd5fab736
3 changed files with 125 additions and 35 deletions
+19 -3
View File
@@ -314,11 +314,13 @@ public partial class MainWindowViewModel
} }
catch (OperationCanceledException) catch (OperationCanceledException)
{ {
// Intentional cancellation due to server change request — not an error. // Intentional cancellation: a server change request, or CheckConnectionOnResumeAsync
// recovering a sync stuck on a socket that died while the app was suspended
// (_resumeRecovering) — neither is an error the user needs to see as one.
cancelled = true; cancelled = true;
IsConnected = false; IsConnected = false;
ConnectionStatus = Loc.Tr("conn.none"); ConnectionStatus = _resumeRecovering ? Loc.Tr("conn.reconnecting") : Loc.Tr("conn.none");
ConnectionStatusShort = Loc.Tr("conn.none"); ConnectionStatusShort = _resumeRecovering ? Loc.Tr("conn.reconnecting") : Loc.Tr("conn.none");
StatusMessage = ""; StatusMessage = "";
} }
catch (CertificatePinMismatchException ex) catch (CertificatePinMismatchException ex)
@@ -331,9 +333,22 @@ public partial class MainWindowViewModel
catch (Exception ex) catch (Exception ex)
{ {
IsConnected = _client?.IsConnected == true; IsConnected = _client?.IsConnected == true;
if (_resumeRecovering && !IsConnected)
{
// We tore the connection down ourselves (see CheckConnectionOnResumeAsync);
// whether that surfaces here as a cancellation or as a transport exception
// is a race, not a real failure — show "reconnecting" and restart right away
// instead of a scary error message and a wait for the next keep-alive tick.
cancelled = true;
ConnectionStatus = Loc.Tr("conn.reconnecting");
ConnectionStatusShort = Loc.Tr("conn.reconnecting");
}
else
{
ConnectionStatus = IsConnected ? ConnectionStatus : Loc.Tr("conn.none"); ConnectionStatus = IsConnected ? ConnectionStatus : Loc.Tr("conn.none");
ConnectionStatusShort = IsConnected ? Loc.Tr("conn.connectedto") : Loc.Tr("conn.none"); ConnectionStatusShort = IsConnected ? Loc.Tr("conn.connectedto") : Loc.Tr("conn.none");
StatusMessage = $"{Loc.Tr("msg.error")}: {DescribeError(ex)}"; StatusMessage = $"{Loc.Tr("msg.error")}: {DescribeError(ex)}";
}
if (_account is not null) if (_account is not null)
{ {
_syncFailed = true; _syncFailed = true;
@@ -345,6 +360,7 @@ public partial class MainWindowViewModel
finally finally
{ {
IsSyncing = false; IsSyncing = false;
_resumeRecovering = false;
} }
// If cancelled due to a server change, restart immediately with the new server. // If cancelled due to a server change, restart immediately with the new server.
+55 -7
View File
@@ -77,6 +77,7 @@ public partial class MainWindowViewModel : ViewModelBase
// ---- keep-alive ---- // ---- keep-alive ----
private bool _autoReconnect; private bool _autoReconnect;
private bool _syncFailed; private bool _syncFailed;
private bool _resumeRecovering;
private readonly DispatcherTimer _keepAliveTimer; private readonly DispatcherTimer _keepAliveTimer;
// ---- server UI sync ---- // ---- server UI sync ----
@@ -161,10 +162,18 @@ public partial class MainWindowViewModel : ViewModelBase
_ = CheckForUpdatesAsync(); _ = CheckForUpdatesAsync();
} }
private bool _keepAliveRunning;
private async System.Threading.Tasks.Task KeepAliveTickAsync() private async System.Threading.Tasks.Task KeepAliveTickAsync()
{ {
if (IsSyncing) // Re-entrancy guard: without it, a ping stuck on a half-open socket (see
// below) would let every subsequent 20s tick pile up another concurrent
// ping/reconnect attempt on top of it.
if (IsSyncing || _keepAliveRunning)
return; return;
_keepAliveRunning = true;
try
{
if (_client is { IsConnected: true } client) if (_client is { IsConnected: true } client)
{ {
// If the wallet is open and the last sync failed, retry automatically. // If the wallet is open and the last sync failed, retry automatically.
@@ -175,15 +184,21 @@ public partial class MainWindowViewModel : ViewModelBase
} }
try try
{ {
await client.PingAsync(); // A "half-open" TCP connection (remote end gone with no FIN/RST
// ever delivered — the common outcome of Android Doze/mobile-radio
// suspend killing the route silently) never fails the write, so
// PingAsync would otherwise await a response that never arrives.
// Bound it explicitly instead of relying on it to throw.
using var timeoutCts = new System.Threading.CancellationTokenSource(System.TimeSpan.FromSeconds(8));
await client.PingAsync(timeoutCts.Token);
} }
catch catch
{ {
// TcpClient.Connected only reflects the last known socket state, so a // TcpClient.Connected only reflects the last known socket state, so a
// connection killed silently while the app was suspended (e.g. Android // connection killed silently while the app was suspended still reports
// Doze after screen lock) still reports IsConnected == true. A failed // IsConnected == true. A failed/timed-out ping is the only reliable
// ping is the only reliable signal here: tear the dead client down so // signal here: tear the dead client down so the next tick reconnects
// the next tick reconnects instead of retrying forever on a dead socket. // instead of retrying forever on a dead socket.
await DisconnectAsync(); await DisconnectAsync();
if (_autoReconnect) if (_autoReconnect)
{ {
@@ -198,6 +213,11 @@ public partial class MainWindowViewModel : ViewModelBase
await ConnectAndSync(); await ConnectAndSync();
} }
} }
finally
{
_keepAliveRunning = false;
}
}
/// <summary>Forces an immediate connection health check, bypassing the 20s timer. /// <summary>Forces an immediate connection health check, bypassing the 20s timer.
/// Called when the app resumes from background/lock screen, since the socket may /// Called when the app resumes from background/lock screen, since the socket may
@@ -206,7 +226,35 @@ public partial class MainWindowViewModel : ViewModelBase
/// during Doze).</summary> /// during Doze).</summary>
public async System.Threading.Tasks.Task CheckConnectionOnResumeAsync() public async System.Threading.Tasks.Task CheckConnectionOnResumeAsync()
{ {
if (IsSyncing) return; if (IsSyncing)
{
// A sync in progress when the phone locked may be stuck awaiting a
// response on a socket that died silently while suspended (same
// half-open-TCP scenario as the keep-alive ping, but sync requests have
// no timeout of their own). Cancel it and tear the client down instead
// of leaving it hung forever. _resumeRecovering tells ConnectAndSync's
// error handling that this interruption is self-inflicted and expected,
// so it shows "reconnecting" and restarts immediately instead of
// flashing a scary error message and waiting for the next keep-alive tick.
_resumeRecovering = true;
ConnectionStatus = Loc.Tr("conn.reconnecting");
ConnectionStatusShort = Loc.Tr("conn.reconnecting");
_syncCts.Cancel();
// Deliberately not DisconnectAsync() here: it also nulls _synchronizer,
// which still holds whatever this sync already downloaded/verified (e.g.
// thousands of Merkle proofs on a large wallet). Nulling it before the
// cancelled ConnectAndSync unwinds would discard that progress, forcing a
// full restart instead of a resume. Only tear down the dead socket here;
// ConnectAndSync's own cancellation handling persists the partial cache
// (PersistPartialTxCache) before it recreates the synchronizer — same
// sequencing already used for the "server changed mid-sync" case.
if (_client is { } deadClient)
{
_client = null;
try { await deadClient.DisposeAsync(); } catch { }
}
return;
}
await KeepAliveTickAsync(); await KeepAliveTickAsync();
} }
+30 -4
View File
@@ -75,6 +75,14 @@ public sealed class WalletSynchronizer(IWalletAccount account, ElectrumClient cl
private readonly ConcurrentDictionary<string, Transaction> _txCache = new(); private readonly ConcurrentDictionary<string, Transaction> _txCache = new();
// txids known confirmed (height > 0) as of download time, independent of whether their
// Merkle proof has been verified yet — lets ExportCaches persist raw tx bytes for a
// confirmed transaction interrupted before verification, instead of forcing a
// re-download on the next sync just because _verifiedAtHeight hasn't caught up. Every
// entry here has txHeights[txid] > 0 at the time it was recorded, i.e. server-confirmed;
// unconfirmed (mempool/RBF-able) transactions are deliberately never added.
private readonly ConcurrentDictionary<string, byte> _confirmedTxids = new();
// Concurrent: written incrementally by individual merkle-verification tasks as they // Concurrent: written incrementally by individual merkle-verification tasks as they
// complete (§7.4 progressive verification), not just once after they all finish. // complete (§7.4 progressive verification), not just once after they all finish.
private readonly ConcurrentDictionary<string, int> _verifiedAtHeight = new(); private readonly ConcurrentDictionary<string, int> _verifiedAtHeight = new();
@@ -116,7 +124,12 @@ public sealed class WalletSynchronizer(IWalletAccount account, ElectrumClient cl
Dictionary<int, int>? anchoredUpTo = null) Dictionary<int, int>? anchoredUpTo = null)
{ {
foreach (var (txid, hex) in rawTxHex) foreach (var (txid, hex) in rawTxHex)
{
_txCache.TryAdd(txid, Transaction.Parse(hex, network)); _txCache.TryAdd(txid, Transaction.Parse(hex, network));
// ExportCaches only ever wrote confirmed transactions here (see its own
// filter), so every preloaded entry is safe to mark confirmed too.
_confirmedTxids.TryAdd(txid, 0);
}
foreach (var (txid, height) in verifiedAt) foreach (var (txid, height) in verifiedAt)
if (!_verifiedAtHeight.ContainsKey(txid)) if (!_verifiedAtHeight.ContainsKey(txid))
_verifiedAtHeight[txid] = height; _verifiedAtHeight[txid] = height;
@@ -147,7 +160,7 @@ public sealed class WalletSynchronizer(IWalletAccount account, ElectrumClient cl
Dictionary<int, int> AnchoredUpTo) Dictionary<int, int> AnchoredUpTo)
ExportCaches(Network network) ExportCaches(Network network)
{ {
var rawHex = _verifiedAtHeight.Keys var rawHex = _confirmedTxids.Keys
.Where(_txCache.ContainsKey) .Where(_txCache.ContainsKey)
.ToDictionary(txid => txid, txid => _txCache[txid].ToHex()); .ToDictionary(txid => txid, txid => _txCache[txid].ToHex());
@@ -232,17 +245,30 @@ public sealed class WalletSynchronizer(IWalletAccount account, ElectrumClient cl
&& (!_verifiedAtHeight.TryGetValue(kv.Key, out var h) || h != kv.Value)) && (!_verifiedAtHeight.TryGetValue(kv.Key, out var h) || h != kv.Value))
.ToList(); .ToList();
// Total/already-cached counts (not just this session's downloads): on a sync resumed
// after an interruption, `missing` is often empty because everything was already
// fetched last time (see _confirmedTxids/PreloadCaches) — reporting against the total
// shows "n/n transactions" immediately instead of a misleading "0/0" before jumping
// straight to proof verification.
var totalTx = txHeights.Count;
var alreadyCached = totalTx - missing.Count;
string DownloadVerifyStatus(int downloaded, int verified) =>
$"transactions {downloaded}/{totalTx}, proofs {verified}/{toVerify.Count}…";
if (missing.Count > 0 || toVerify.Count > 0) if (missing.Count > 0 || toVerify.Count > 0)
Progress?.Invoke($"downloading {missing.Count} txs, verifying {toVerify.Count} proofs…"); Progress?.Invoke(DownloadVerifyStatus(alreadyCached, 0));
var dlDone = 0; var dlDone = 0;
await Task.WhenAll(missing.Select(txid => RetryOnBusyAsync(async () => await Task.WhenAll(missing.Select(txid => RetryOnBusyAsync(async () =>
{ {
var raw = await client.GetTransactionAsync(txid, ct); var raw = await client.GetTransactionAsync(txid, ct);
_txCache[txid] = Transaction.Parse(raw, network); _txCache[txid] = Transaction.Parse(raw, network);
if (txHeights[txid] > 0)
_confirmedTxids.TryAdd(txid, 0);
var n = Interlocked.Increment(ref dlDone); var n = Interlocked.Increment(ref dlDone);
if (n % 50 == 0 || n == missing.Count) if (n % 50 == 0 || n == missing.Count)
Progress?.Invoke($"tx {n}/{missing.Count}, proofs 0/{toVerify.Count}…"); Progress?.Invoke(DownloadVerifyStatus(alreadyCached + n, 0));
}, ct))); }, ct)));
SyncResult BuildSnapshot() => SyncResult BuildSnapshot() =>
@@ -273,7 +299,7 @@ public sealed class WalletSynchronizer(IWalletAccount account, ElectrumClient cl
_verifiedAtHeight[txid] = height; _verifiedAtHeight[txid] = height;
var n = Interlocked.Increment(ref merkDone); var n = Interlocked.Increment(ref merkDone);
if (n % 50 == 0 || n == toVerify.Count) if (n % 50 == 0 || n == toVerify.Count)
Progress?.Invoke($"tx {missing.Count}/{missing.Count}, proofs {n}/{toVerify.Count}…"); Progress?.Invoke(DownloadVerifyStatus(totalTx, n));
if (n % PartialResultBatchSize == 0) if (n % PartialResultBatchSize == 0)
PartialResult?.Invoke(BuildSnapshot()); PartialResult?.Invoke(BuildSnapshot());
}, ct)).ToList(); }, ct)).ToList();