fix(payout): classify Monero daemon-connection failures as pre-broadcast so transient wallet-rpc wedges self-heal#4342
Draft
davidleomay wants to merge 1 commit into
Conversation
…ast so transient wallet-rpc wedges self-heal Two Monero payout incidents (2026-07-18 order 112451, 2026-07-20 order 112677) both escalated to PayoutUncertain in a state where the wallet had provably not signed or broadcast anything: monero-wallet-rpc reported code -38 "no connection to daemon" and code -4 "failed to get output distribution" during the `transfer` call while its persistent daemon socket was wedged. Since the fail-closed broadcast boundary landed in #4238, every send-RPC failure without an allowlisted pre-broadcast marker escalates in ~30s — and neither of these markers was on the list, even though both classes are constructed inside wallet2 before ring signatures. Both codes now stay plain and route into the existing pre-broadcast retry loop: - WALLET_RPC_ERROR_CODE_NO_DAEMON_CONNECTION (-38): mapped in wallet_rpc_server::handle_rpc_exception before signing, cited from src/wallet/wallet_rpc_server_error_codes.h. - (-4, "failed to get output distribution"): fall-through to GENERIC_TRANSFER_ERROR because handle_rpc_exception has no dedicated catch for error::get_output_distribution; the exact what() message is hard-coded in src/wallet/wallet_errors.h and thrown only during decoy selection (src/wallet/wallet2.cpp). The classifier gains an optional (code, message) allowlist parameter with exact-string matching only — a substring/regex match would erode the guarantee that only the specific pre-signing wallet exception surfaces, and a rewording upstream should fail closed on the next release rather than silently expand the allowlist. Bitcoin/Firo/Zano keep the unchanged 2-arg call via a default empty allowlist. With the default maxPreBroadcastRetries=3, a transient wedge is now absorbed within ~90 seconds instead of escalating, matching the recovery pattern observed on both incidents.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two Monero payout incidents both escalated to
PayoutUncertainin a state where the wallet had provably not signed or broadcast anything — monero-wallet-rpc's persistent daemon socket was wedged and returned two different pre-signing errors that the classifier introduced in #4238 wasn't allowlisting:code -4, "failed to get output distribution"code -38, "no connection to daemon"Both classes are constructed inside
wallet2::transfer_selected_rctbefore ring signatures; both are self-recoverable transient wedges (the wallet reconnected on its own within minutes). With the fail-closed broadcast boundary, however, every send-RPC failure without an allowlisted pre-broadcast marker escalates toPayoutUncertainin ~30s — so both incidents needed manual reset even though the API's pre-broadcast retry loop would have absorbed them.This PR routes both classes into the existing
maxPreBroadcastRetriesloop:-38joinsMONERO_PRE_BROADCAST_RPC_CODESdirectly. Cited fromsrc/wallet/wallet_rpc_server_error_codes.h; mapped inwallet_rpc_server::handle_rpc_exceptionbefore signing (source:src/wallet/wallet_rpc_server.cpp).(-4, "failed to get output distribution")uses a new narrow(code, message)allowlist because-4isGENERIC_TRANSFER_ERROR(Monero's catch-all — cannot allowlist by code alone). Thewhat()message is hard-coded instruct get_output_distribution(src/wallet/wallet_errors.h) and only thrown during decoy selection (src/wallet/wallet2.cpp).Message matching is exact (not substring/regex): a rewording upstream should fail closed on the next release rather than silently expand the allowlist. Bitcoin/Firo/Zano keep the unchanged 2-arg call via a default empty allowlist parameter.
With
maxPreBroadcastRetries = 3(default), a transient wedge is now absorbed within ~90 seconds, matching the observed self-recovery timing on both incidents.Test plan
-38allowlist, exact-message match, wrong-code and substring-match negatives)-38and(code=-4, message="failed to get output distribution")allowlist entries)jest tx-broadcast.error|monero-client|payout-bitcoin-based→ 85/85 passtsc --noEmitclean (Bitcoin/Firo/Zano callers keep the 2-arg call via default empty allowlist)eslintclean on all 4 changed filesRelated