fix(send): treat a lost send reply as "outcome unknown", not a plain failure, to prevent a double-send
Resilience audit, highest-value finding: a z_sendmany (full node) or lite broadcast whose REPLY is
lost — a transport timeout / connection cut AFTER the request already left us — was reported as an
ordinary "Transaction failed". The user then retries and can broadcast the same payment twice, because
the daemon may already hold the operation (z_sendmany returns an opid and runs async).
Classify the ambiguous case and surface it distinctly, without touching the send itself:
- New pure helpers in rpc/rpc_failure.h: sendReplyMayBeLost() (an RpcTransportError that was requestSent
with no HTTP status and a non-OK curl code — reply-timeout / got-nothing / our-own-abort; definitive
rejections and any HTTP status are excluded), liteSendErrorMayBeInFlight() (a transport-style gRPC text
heuristic, vetoing connect-phase and server-rejection text), and a readable-English marker tag.
- app_network.cpp: the z_sendmany catch records the flag; the failure branch (non-background only) tags
the callback text and forces a wallet re-read so a committed send shows in History. onDisconnected()
tags already-pending send callbacks the same way (they exist only after an opid was returned).
- Lite UserSend (app.cpp): a transport-style broadcast error is tagged identically.
- Send tab: a tagged outcome shows a non-success warning pointing at History and clears the compose form
so any resend must be retyped deliberately; it is NOT presented as a retryable failure.
The actual rpc_->call("z_sendmany", {from, recipients, 1, fee}), the lite send/broadcast call, and all
amounts/fees/recipients/memo bytes are byte-identical; only result-handling and presentation change.
Pure classifier/marker regression tests added (testSendMaybeInFlight).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+5
-1
@@ -1050,7 +1050,11 @@ void App::update()
|
||||
case LiteOpKind::None:
|
||||
default:
|
||||
if (lite_send_callback_) {
|
||||
lite_send_callback_(broadcast.ok, broadcast.ok ? broadcast.txid : broadcast.error);
|
||||
// A transport-style failure may mean the server accepted the tx but the reply was
|
||||
// lost: tag it so the Send tab says "check History" instead of inviting a resend.
|
||||
const std::string liteErr = (!broadcast.ok && rpc::liteSendErrorMayBeInFlight(broadcast.error))
|
||||
? rpc::markSendMaybeInFlight(broadcast.error) : broadcast.error;
|
||||
lite_send_callback_(broadcast.ok, broadcast.ok ? broadcast.txid : liteErr);
|
||||
lite_send_callback_ = nullptr;
|
||||
}
|
||||
break;
|
||||
|
||||
+17
-3
@@ -891,8 +891,10 @@ void App::onDisconnected(const std::string& reason)
|
||||
pending_opids_.clear();
|
||||
pending_send_info_.clear();
|
||||
// Resolve any deferred send callbacks so their UI doesn't spin forever on disconnect.
|
||||
// Each callback here exists only because z_sendmany already returned an opid, so the op may still
|
||||
// commit (or have committed) while we lost the connection: report "outcome unknown", not a failure.
|
||||
for (auto& entry : pending_send_callbacks_) {
|
||||
if (entry.second) entry.second(false, reason);
|
||||
if (entry.second) entry.second(false, rpc::markSendMaybeInFlight(reason));
|
||||
}
|
||||
pending_send_callbacks_.clear();
|
||||
consecutive_core_failures_ = 0;
|
||||
@@ -7209,6 +7211,7 @@ void App::submitZSendMany(const std::string& from, const std::string& to, double
|
||||
worker_->post([this, from, to, amount, fee, memo, recipients, callback, traceLabel,
|
||||
markFeeGapRetry, background]() -> rpc::RPCWorker::MainCb {
|
||||
bool ok = false;
|
||||
bool maybeInFlight = false; // reply lost after the request left: the node may already hold the op
|
||||
std::string result_str;
|
||||
try {
|
||||
rpc::RPCClient::TraceScope trace(traceLabel);
|
||||
@@ -7217,12 +7220,13 @@ void App::submitZSendMany(const std::string& from, const std::string& to, double
|
||||
ok = true;
|
||||
} catch (const std::exception& e) {
|
||||
result_str = e.what();
|
||||
maybeInFlight = rpc::sendReplyMayBeLost(e);
|
||||
} catch (...) {
|
||||
// Non-std throw must not escape into the worker — the main-thread callback
|
||||
// below would never run, leaving a stuck "Sending…" spinner.
|
||||
result_str = "Send failed (unknown error)";
|
||||
}
|
||||
return [this, callback, ok, result_str, from, to, amount, fee, memo, markFeeGapRetry, background]() {
|
||||
return [this, callback, ok, maybeInFlight, result_str, from, to, amount, fee, memo, markFeeGapRetry, background]() {
|
||||
if (send_submissions_in_flight_ > 0) --send_submissions_in_flight_;
|
||||
if (ok) {
|
||||
// z_sendmany only returned an opid: the transaction is built/signed/
|
||||
@@ -7245,7 +7249,17 @@ void App::submitZSendMany(const std::string& from, const std::string& to, double
|
||||
}
|
||||
} else {
|
||||
if (!background) send_progress_active_ = false;
|
||||
if (callback) callback(false, result_str);
|
||||
if (maybeInFlight && !background) {
|
||||
// Ambiguous: z_sendmany's reply never arrived, so the op may be running. Surface the
|
||||
// distinct "check History before resending" outcome and re-read the wallet so a
|
||||
// committed send shows up promptly. Definitive rejections stay plain failures below.
|
||||
DEBUG_LOGF("[send] z_sendmany reply lost (%s): outcome unknown, not a definitive failure\n",
|
||||
result_str.c_str());
|
||||
markWalletMutated(/*forceBalance=*/true);
|
||||
if (callback) callback(false, rpc::markSendMaybeInFlight(result_str));
|
||||
} else if (callback) {
|
||||
callback(false, result_str);
|
||||
}
|
||||
}
|
||||
};
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
|
||||
#include <curl/curl.h>
|
||||
|
||||
#include <cctype>
|
||||
#include <stdexcept>
|
||||
#include <string>
|
||||
|
||||
@@ -152,6 +153,54 @@ inline RpcFailureKind rpcFailureKindOf(const std::exception& e)
|
||||
return RpcFailureKind::Other;
|
||||
}
|
||||
|
||||
// Did a failed z_sendmany (or any submit call) possibly reach the node and take effect? True only for a
|
||||
// transport failure AFTER the request left us with no HTTP status back (reply timeout, connection cut
|
||||
// mid-reply, our own abort): the daemon may already hold the operation, so the caller must not invite an
|
||||
// immediate resend. Definitive answers/rejections (AppError, QueueFull, Auth, Warmup, Deferred,
|
||||
// NotListening, ConnectTimeout, any HTTP status) mean nothing was queued and stay plain failures.
|
||||
inline bool sendReplyMayBeLost(const std::exception& e)
|
||||
{
|
||||
const auto* t = dynamic_cast<const RpcTransportError*>(&e);
|
||||
return t && t->requestSent && t->httpStatus == 0 && t->curlCode != CURLE_OK;
|
||||
}
|
||||
|
||||
// Lite broadcast errors are plain backend text (tonic/gRPC). Ambiguous = a transport-style failure while
|
||||
// talking to the lightwalletd server (the server may have accepted the tx before the reply was lost).
|
||||
// Connect-phase failures and an explicit server rejection ("error_code") are definitive. Heuristic by
|
||||
// design: a false positive only shows a more cautious message, never changes the send.
|
||||
inline bool liteSendErrorMayBeInFlight(const std::string& err)
|
||||
{
|
||||
std::string lc = err;
|
||||
for (char& c : lc) c = static_cast<char>(std::tolower(static_cast<unsigned char>(c)));
|
||||
auto has = [&](const char* n) { return lc.find(n) != std::string::npos; };
|
||||
if (has("error_code") || has("error trying to connect") || has("connection refused") ||
|
||||
has("dns error") || has("insufficient") || has("locked"))
|
||||
return false;
|
||||
return has("timed out") || has("timeout") || has("deadline") || has("transport error") ||
|
||||
has("connection reset") || has("broken pipe") || has("h2 protocol error") ||
|
||||
has("unexpected eof") || has("status: unknown") || has("status: cancelled");
|
||||
}
|
||||
|
||||
// Tag carried in the failure text of a send callback (bool, string) so the Send tab can show the distinct
|
||||
// "may have been broadcast - check History before resending" outcome without changing the callback type.
|
||||
// The prefix is readable English so callers that just print the text still say something accurate.
|
||||
inline const char* sendMaybeInFlightPrefix()
|
||||
{
|
||||
return "Send status unknown - it may already have been broadcast: ";
|
||||
}
|
||||
inline bool isSendMaybeInFlight(const std::string& s)
|
||||
{
|
||||
return s.rfind(sendMaybeInFlightPrefix(), 0) == 0;
|
||||
}
|
||||
inline std::string markSendMaybeInFlight(const std::string& detail)
|
||||
{
|
||||
return isSendMaybeInFlight(detail) ? detail : std::string(sendMaybeInFlightPrefix()) + detail;
|
||||
}
|
||||
inline std::string stripSendMaybeInFlight(const std::string& s)
|
||||
{
|
||||
return isSendMaybeInFlight(s) ? s.substr(std::string(sendMaybeInFlightPrefix()).size()) : s;
|
||||
}
|
||||
|
||||
// What a failed walletpassphrase call means. Only WrongPassphrase (-14) may count toward the lockout;
|
||||
// a busy/warming-up node, a wallet that is not encrypted, or anything unknown is never a bad guess.
|
||||
enum class UnlockFailure { WrongPassphrase, NodeBusy, NotEncrypted, Other };
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
#include "../../util/i18n.h"
|
||||
#include "../../util/text_format.h"
|
||||
#include "../../util/amount_format.h" // toSats / canAfford — integer-satoshi affordability check
|
||||
#include "../../rpc/rpc_failure.h" // isSendMaybeInFlight — lost-reply (outcome unknown) send tag
|
||||
#include "../notifications.h"
|
||||
#include "../layout.h"
|
||||
#include "contact_picker.h"
|
||||
@@ -955,6 +956,18 @@ void RenderSendConfirmPopup(App* app) {
|
||||
s_amount = 0.0;
|
||||
s_memo[0] = '\0';
|
||||
s_send_max = false;
|
||||
} else if (rpc::isSendMaybeInFlight(result)) {
|
||||
// Outcome unknown: the reply was lost after the request left. Do NOT present a
|
||||
// plain failure (it invites a retry/double-send). Warn, point at History, and
|
||||
// clear the form so a resend has to be retyped deliberately.
|
||||
s_tx_status = std::string(TR("send_maybe_sent")) + "\n\n" + rpc::stripSendMaybeInFlight(result);
|
||||
s_result_txid.clear();
|
||||
s_status_success = false;
|
||||
Notifications::instance().warning(TR("send_maybe_sent"), 15.0f);
|
||||
s_to_address[0] = '\0';
|
||||
s_amount = 0.0;
|
||||
s_memo[0] = '\0';
|
||||
s_send_max = false;
|
||||
} else {
|
||||
// Show a friendly cause in the toast; keep the raw backend text below it in
|
||||
// the overlay (copyable) so the detail isn't lost for diagnostics.
|
||||
|
||||
@@ -2698,6 +2698,7 @@ void I18n::loadBuiltinEnglish()
|
||||
strings_["send_tooltip_view_only"] = "View-only address — no spending key, cannot send";
|
||||
strings_["send_total"] = "Total";
|
||||
strings_["send_tx_failed"] = "Transaction failed";
|
||||
strings_["send_maybe_sent"] = "This transaction may already have been broadcast (the reply was lost). Do NOT send again yet - check History first.";
|
||||
strings_["send_tx_sent"] = "Transaction sent!";
|
||||
strings_["send_cannot_send_now"] = "Cannot send now — check connection, sync, and available balance.";
|
||||
strings_["send_tx_success"] = "Transaction sent successfully!";
|
||||
|
||||
@@ -819,6 +819,41 @@ void testClassifyRpcFailure()
|
||||
EXPECT_FALSE(dragonx::rpc::isRpcTimeout(K::QueueFull));
|
||||
}
|
||||
|
||||
// Lost-reply sends: only a transport failure AFTER the request left (no HTTP status) is ambiguous.
|
||||
void testSendMaybeInFlight()
|
||||
{
|
||||
using namespace dragonx::rpc;
|
||||
using K = RpcFailureKind;
|
||||
// Ambiguous: reply timeout, connection cut mid-reply, our own abort - request sent, no HTTP status.
|
||||
EXPECT_TRUE(sendReplyMayBeLost(RpcTransportError(K::ReplyTimeout, "t", CURLE_OPERATION_TIMEDOUT, 0, true)));
|
||||
EXPECT_TRUE(sendReplyMayBeLost(RpcTransportError(K::Other, "t", CURLE_GOT_NOTHING, 0, true)));
|
||||
EXPECT_TRUE(sendReplyMayBeLost(RpcTransportError(K::Aborted, "t", CURLE_ABORTED_BY_CALLBACK, 0, true)));
|
||||
// Definitive: never reached the daemon, or the daemon/HTTP layer answered.
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::ConnectTimeout, "t", CURLE_OPERATION_TIMEDOUT, 0, false)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::NotListening, "t", CURLE_COULDNT_CONNECT, 0, false)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::QueueFull, "t", CURLE_OK, 500, true)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::ShuttingDown, "t", CURLE_OK, 503, true)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::Auth, "t", CURLE_OK, 401, true)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcTransportError(K::Deferred, "t", CURLE_OK, 0, false)));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcError(-6, "Insufficient funds")));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(RpcError(-5, "Invalid address")));
|
||||
EXPECT_FALSE(sendReplyMayBeLost(std::runtime_error("plain")));
|
||||
// Lite text heuristic.
|
||||
EXPECT_TRUE(liteSendErrorMayBeInFlight("status: Unknown, message: \"transport error\""));
|
||||
EXPECT_TRUE(liteSendErrorMayBeInFlight("operation timed out"));
|
||||
EXPECT_FALSE(liteSendErrorMayBeInFlight("error trying to connect: tcp connect error"));
|
||||
EXPECT_FALSE(liteSendErrorMayBeInFlight("Error: SendResponse { error_code: 1, error_message: \"bad-txns\" }"));
|
||||
EXPECT_FALSE(liteSendErrorMayBeInFlight("Insufficient verified funds"));
|
||||
EXPECT_FALSE(liteSendErrorMayBeInFlight("Wallet is locked"));
|
||||
// Marker round trip + idempotence.
|
||||
const std::string marked = markSendMaybeInFlight("RPC request failed: Timeout");
|
||||
EXPECT_TRUE(isSendMaybeInFlight(marked));
|
||||
EXPECT_FALSE(isSendMaybeInFlight("RPC request failed: Timeout"));
|
||||
EXPECT_EQ(markSendMaybeInFlight(marked), marked);
|
||||
EXPECT_EQ(stripSendMaybeInFlight(marked), std::string("RPC request failed: Timeout"));
|
||||
EXPECT_EQ(stripSendMaybeInFlight("plain"), std::string("plain"));
|
||||
}
|
||||
|
||||
void testSecureWipeAndRemove()
|
||||
{
|
||||
namespace fs = std::filesystem;
|
||||
@@ -19135,6 +19170,7 @@ int main()
|
||||
{
|
||||
testConnectionConfig();
|
||||
testClassifyRpcFailure();
|
||||
testSendMaybeInFlight();
|
||||
testClassifyUnlockFailure();
|
||||
testHttpSinkAccept();
|
||||
testReadFileExact();
|
||||
|
||||
Reference in New Issue
Block a user