Skip to content

Commit 4e43243

Browse files
reubenyapclaude
reubenyap
and
claude
committed
fix: recover stalled wallet syncs via idle watchdog and progress reorder
Two targeted changes to _refresh() in the base Wallet<T> class, plus a supporting heartbeat hook on ElectrumXClient. Affects all wallet types routing through the shared refresh flow (BTC, LTC, Firo, Monero, ETH, Cardano, Solana, etc. — but NOT MimbleWimbleCoin, ETH tokens, or SOL tokens, which override refresh()). 1. Idle watchdog on the refresh body. A Timer.periodic checks every 30s whether any refresh activity has been observed within the last 5 minutes. If not, the refresh is assumed wedged, the watchdog throws TimeoutException, and the existing catch/finally releases refreshMutex. Previously the mutex stayed locked forever when any sub-operation hung, making every subsequent periodic sync bail out at the isLocked check until the app was force-closed. "Activity" is fed from two sources: - _fireRefreshPercentChange (coarse checkpoints + Spark per-sector progress during the anon-set download) - successful electrum RPCs, via a new onRequestComplete hook on ElectrumXClient The electrum hook is important because updateTransactions() on a wallet with large history does hundreds of sequential getTransaction RPCs with no _fireRefreshPercentChange calls between 0.65 and 0.70. Without the hook, a slow server could legitimately trigger a false timeout on an initial restore. With the hook, the watchdog stays fed as long as electrum is answering. Note: the watchdog does not cancel in-flight work; it only unblocks the outer future so the mutex can be released. The orphaned work eventually resolves or errors on its own. 2. Fire progress updates *after* the awaited work completes rather than before. The 0.65 and 0.70 calls previously fired immediately after kicking off updateUTXOs/updateTransactions, making the bar appear stuck at those values while the real work was still running. The ~65% stall in Firo syncs was the most visible manifestation of this; other wallets had the same cosmetic issue. The refresh body is also extracted to a private _doRefreshWork(viewOnly) helper to keep _refresh() focused on mutex/event/watchdog/error handling. No behavioural change from the extraction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 243015b commit 4e43243

2 files changed

Lines changed: 154 additions & 84 deletions

File tree

‎lib/electrumx_rpc/electrumx_client.dart‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,12 @@ class ElectrumXClient {
117117
final Mutex _torConnectingLock = Mutex();
118118
bool _requireMutex = false;
119119

120+
/// Optional hook fired after each successful RPC completes. Used by
121+
/// higher layers (e.g. the wallet refresh idle watchdog) to detect
122+
/// liveness during long sequential RPC loops without having to
123+
/// instrument every call site.
124+
void Function()? onRequestComplete;
125+
120126
ElectrumXClient({
121127
required String host,
122128
required int port,
@@ -368,6 +374,7 @@ class ElectrumXClient {
368374
}
369375

370376
currentFailoverIndex = -1;
377+
onRequestComplete?.call();
371378

372379
// If the command is a ping, a good return should always be null.
373380
if (command.contains("ping")) {
@@ -474,6 +481,7 @@ class ElectrumXClient {
474481
// }
475482

476483
currentFailoverIndex = -1;
484+
onRequestComplete?.call();
477485
return response;
478486
} on WifiOnlyException {
479487
rethrow;

‎lib/wallets/wallet/wallet.dart‎

Lines changed: 146 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,10 @@ abstract class Wallet<T extends CryptoCurrency> {
108108
Timer? _periodicRefreshTimer;
109109
Timer? _networkAliveTimer;
110110

111+
/// Timestamp of the last _fireRefreshPercentChange during an active
112+
/// refresh. Consumed by the idle watchdog in _refresh() to detect hangs.
113+
DateTime? _lastRefreshProgress;
114+
111115
bool _shouldAutoSync = false;
112116

113117
bool _isConnected = false;
@@ -603,6 +607,7 @@ abstract class Wallet<T extends CryptoCurrency> {
603607
}
604608

605609
void _fireRefreshPercentChange(double percent) {
610+
_lastRefreshProgress = DateTime.now();
606611
if (this is ElectrumXInterface) {
607612
(this as ElectrumXInterface?)?.refreshingPercent = percent;
608613
}
@@ -641,95 +646,60 @@ abstract class Wallet<T extends CryptoCurrency> {
641646
);
642647
}
643648

644-
// add some small buffer before making calls.
645-
// this can probably be removed in the future but was added as a
646-
// debugging feature
647-
await Future<void>.delayed(const Duration(milliseconds: 300));
648-
649-
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
650-
final Set<String> codesToCheck = {};
651-
if (this is PaynymInterface && !viewOnly) {
652-
// isSegwit does not matter here at all
653-
final myCode = await (this as PaynymInterface).getPaymentCode(
654-
isSegwit: false,
655-
);
656-
657-
final nym = await PaynymIsApi().nym(myCode.toString());
658-
if (nym.value != null) {
659-
for (final follower in nym.value!.followers) {
660-
codesToCheck.add(follower.code);
661-
}
662-
for (final following in nym.value!.following) {
663-
codesToCheck.add(following.code);
664-
}
665-
}
649+
// Idle watchdog: trips when no refresh activity has been observed
650+
// for idleThreshold, signalling that the refresh is wedged.
651+
// Slow-but-active syncs keep the watchdog fed and aren't killed:
652+
// - _fireRefreshPercentChange ticks (e.g. Spark per-sector progress)
653+
// - successful electrum RPCs (via ElectrumXClient.onRequestComplete)
654+
// Per-call hang detection is still the responsibility of the
655+
// underlying adapters (e.g. electrum's connectionTimeout). This only
656+
// catches what slips through those layers and would otherwise hold
657+
// refreshMutex locked until the app is force-closed.
658+
const idleThreshold = Duration(minutes: 5);
659+
_lastRefreshProgress = DateTime.now();
660+
661+
// Feed the watchdog from successful electrum RPCs, so long sequential
662+
// fetches (e.g. updateTransactions on a wallet with a large history)
663+
// are classified as active rather than idle.
664+
if (this is ElectrumXInterface) {
665+
(this as ElectrumXInterface).electrumXClient.onRequestComplete = () {
666+
_lastRefreshProgress = DateTime.now();
667+
};
666668
}
667669

668-
_fireRefreshPercentChange(0);
669-
await updateChainHeight();
670-
671-
if (this is BitcoinFrostWallet) {
672-
await (this as BitcoinFrostWallet).lookAhead();
673-
}
674-
675-
_fireRefreshPercentChange(0.1);
676-
677-
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
678-
if (this is MultiAddressInterface) {
679-
if (info.otherData[WalletInfoKeys.reuseAddress] != true) {
680-
await (this as MultiAddressInterface)
681-
.checkReceivingAddressForTransactions();
670+
final watchdogCompleter = Completer<void>();
671+
final watchdog = Timer.periodic(const Duration(seconds: 30), (timer) {
672+
if (watchdogCompleter.isCompleted) {
673+
timer.cancel();
674+
return;
682675
}
683-
}
684-
685-
_fireRefreshPercentChange(0.2);
686-
687-
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
688-
if (this is MultiAddressInterface) {
689-
if (info.otherData[WalletInfoKeys.reuseAddress] != true) {
690-
await (this as MultiAddressInterface)
691-
.checkChangeAddressForTransactions();
676+
final last = _lastRefreshProgress;
677+
if (last == null) return;
678+
if (DateTime.now().difference(last) >= idleThreshold) {
679+
timer.cancel();
680+
watchdogCompleter.completeError(
681+
TimeoutException(
682+
'Wallet refresh for $walletId idle for '
683+
'${idleThreshold.inMinutes} min',
684+
idleThreshold,
685+
),
686+
);
692687
}
688+
});
689+
690+
try {
691+
await Future.any([
692+
_doRefreshWork(viewOnly),
693+
watchdogCompleter.future,
694+
]);
695+
} finally {
696+
watchdog.cancel();
697+
if (this is ElectrumXInterface) {
698+
(this as ElectrumXInterface).electrumXClient.onRequestComplete =
699+
null;
700+
}
701+
_lastRefreshProgress = null;
693702
}
694-
_fireRefreshPercentChange(0.3);
695-
if (this is SparkInterface && !viewOnly) {
696-
// this should be called before updateTransactions()
697-
await (this as SparkInterface).refreshSparkData((0.3, 0.6));
698-
}
699-
700-
if (this is NamecoinWallet) {
701-
await updateUTXOs();
702-
_fireRefreshPercentChange(0.6);
703-
await (this as NamecoinWallet).checkAutoRegisterNameNewOutputs();
704-
_fireRefreshPercentChange(0.70);
705-
await updateTransactions();
706-
} else {
707-
final fetchFuture = updateTransactions();
708-
_fireRefreshPercentChange(0.6);
709-
final utxosRefreshFuture = updateUTXOs();
710-
_fireRefreshPercentChange(0.65);
711-
await utxosRefreshFuture;
712-
_fireRefreshPercentChange(0.70);
713-
await fetchFuture;
714-
}
715-
716-
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
717-
if (!viewOnly && this is PaynymInterface && codesToCheck.isNotEmpty) {
718-
await (this as PaynymInterface).checkForNotificationTransactionsTo(
719-
codesToCheck,
720-
);
721-
// check utxos again for notification outputs
722-
await updateUTXOs();
723-
}
724-
_fireRefreshPercentChange(0.80);
725-
726-
// await getAllTxsToWatch();
727-
728-
_fireRefreshPercentChange(0.90);
729-
730-
await updateBalance();
731-
732-
_fireRefreshPercentChange(1.0);
733703

734704
completer.complete();
735705
} catch (error, strace) {
@@ -750,6 +720,98 @@ abstract class Wallet<T extends CryptoCurrency> {
750720
}
751721
}
752722

723+
Future<void> _doRefreshWork(bool viewOnly) async {
724+
// add some small buffer before making calls.
725+
// this can probably be removed in the future but was added as a
726+
// debugging feature
727+
await Future<void>.delayed(const Duration(milliseconds: 300));
728+
729+
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
730+
final Set<String> codesToCheck = {};
731+
if (this is PaynymInterface && !viewOnly) {
732+
// isSegwit does not matter here at all
733+
final myCode = await (this as PaynymInterface).getPaymentCode(
734+
isSegwit: false,
735+
);
736+
737+
final nym = await PaynymIsApi().nym(myCode.toString());
738+
if (nym.value != null) {
739+
for (final follower in nym.value!.followers) {
740+
codesToCheck.add(follower.code);
741+
}
742+
for (final following in nym.value!.following) {
743+
codesToCheck.add(following.code);
744+
}
745+
}
746+
}
747+
748+
_fireRefreshPercentChange(0);
749+
await updateChainHeight();
750+
751+
if (this is BitcoinFrostWallet) {
752+
await (this as BitcoinFrostWallet).lookAhead();
753+
}
754+
755+
_fireRefreshPercentChange(0.1);
756+
757+
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
758+
if (this is MultiAddressInterface) {
759+
if (info.otherData[WalletInfoKeys.reuseAddress] != true) {
760+
await (this as MultiAddressInterface)
761+
.checkReceivingAddressForTransactions();
762+
}
763+
}
764+
765+
_fireRefreshPercentChange(0.2);
766+
767+
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
768+
if (this is MultiAddressInterface) {
769+
if (info.otherData[WalletInfoKeys.reuseAddress] != true) {
770+
await (this as MultiAddressInterface)
771+
.checkChangeAddressForTransactions();
772+
}
773+
}
774+
_fireRefreshPercentChange(0.3);
775+
if (this is SparkInterface && !viewOnly) {
776+
// this should be called before updateTransactions()
777+
await (this as SparkInterface).refreshSparkData((0.3, 0.6));
778+
}
779+
780+
if (this is NamecoinWallet) {
781+
await updateUTXOs();
782+
_fireRefreshPercentChange(0.6);
783+
await (this as NamecoinWallet).checkAutoRegisterNameNewOutputs();
784+
_fireRefreshPercentChange(0.70);
785+
await updateTransactions();
786+
} else {
787+
final fetchFuture = updateTransactions();
788+
_fireRefreshPercentChange(0.6);
789+
final utxosRefreshFuture = updateUTXOs();
790+
await utxosRefreshFuture;
791+
_fireRefreshPercentChange(0.65);
792+
await fetchFuture;
793+
_fireRefreshPercentChange(0.70);
794+
}
795+
796+
// TODO: [prio=low] handle this differently. Extra modification of this file for coin specific functionality should be avoided.
797+
if (!viewOnly && this is PaynymInterface && codesToCheck.isNotEmpty) {
798+
await (this as PaynymInterface).checkForNotificationTransactionsTo(
799+
codesToCheck,
800+
);
801+
// check utxos again for notification outputs
802+
await updateUTXOs();
803+
}
804+
_fireRefreshPercentChange(0.80);
805+
806+
// await getAllTxsToWatch();
807+
808+
_fireRefreshPercentChange(0.90);
809+
810+
await updateBalance();
811+
812+
_fireRefreshPercentChange(1.0);
813+
}
814+
753815
Future<void> exit() async {
754816
Logging.instance.i("exit called on $walletId");
755817
_periodicRefreshTimer?.cancel();

0 commit comments

Comments
 (0)