From de70e684722f3906f8d90d5b456e4394e35a9819 Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 17 Jun 2026 14:27:52 -0500 Subject: [PATCH] fix(fullnode): work around daemon note-selection fee-gap on shielded sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dragonxd's z_sendmany picks notes to cover the recipient total (nTotalOut) but not the miner fee, then rejects the build unless the selected notes cover amount+fee (rpcwallet.cpp:5312 vs asyncrpcoperation_sendmany.cpp:278). So a shielded send whose largest notes sum exactly to the amount fails with "Insufficient shielded funds, have H, need H+fee" despite ample balance — e.g. sending exactly 2.0 from an address whose biggest note is 2.0. Since the failure is async (reported via the opid poll), detect it there: when a shielded send fails with that message and the selected total H >= the requested amount (selection covered the amount but stopped one note short of the fee — vs a genuine shortfall where H < amount), re-issue the send once with a tiny self-output (= fee) back to the from-address. That lifts the daemon's selection target past the boundary so it grabs another note and can cover the fee; the recipient still receives the exact amount. Retries are tracked so a second failure surfaces normally (no loop). Co-Authored-By: Claude Opus 4.8 --- src/app.cpp | 5 ++ src/app.h | 17 ++++++- src/app_network.cpp | 120 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 138 insertions(+), 4 deletions(-) diff --git a/src/app.cpp b/src/app.cpp index 9a93cdf..53b7d49 100644 --- a/src/app.cpp +++ b/src/app.cpp @@ -865,6 +865,11 @@ void App::update() // its own error toast); otherwise surface a generic notification (this is // how shield/merge/auto-shield failures become visible). for (const auto& [opid, rawMsg] : parsed.failureByOpid) { + // Auto-work-around the daemon's note-selection fee-gap: re-issues the send with a + // self-output so it covers the fee. If a retry was issued, defer the outcome to it. + if (maybeRetrySendForFeeGap(opid, rawMsg)) { + continue; + } std::string msg = sendErrorNeedsRescan(rawMsg) ? rawMsg + "\n\n" + TR("send_err_needs_rescan") : rawMsg; diff --git a/src/app.h b/src/app.h index b88bdbd..402a01a 100644 --- a/src/app.h +++ b/src/app.h @@ -411,7 +411,18 @@ private: const std::string& from, const std::string& to, double amount, - const std::string& memo); + const std::string& memo, + double fee = 0.0); + // Work around a dragonxd note-selection bug: its z_sendmany picks notes to cover the recipient + // total but not the miner fee, so a shielded send whose largest notes sum exactly to the amount + // fails with "Insufficient shielded funds, have H, need H+fee" despite ample balance. When a + // failed opid matches that (H >= the requested amount), re-issue the send once with a tiny + // self-output that lifts the daemon's selection target past the boundary so it grabs another + // note; the recipient still receives the exact amount. Returns true if a retry was issued. + bool maybeRetrySendForFeeGap(const std::string& opid, const std::string& rawMsg); + void resendWithFeeGapWorkaround(const std::string& from, const std::string& to, + double amount, double fee, const std::string& memo, + std::function callback); void markPendingSendTransactionSucceeded(const std::string& opid, const std::string& txid); void removePendingSendTransactions(const std::vector& opids, @@ -618,9 +629,13 @@ private: std::string to; std::string memo; double amount = 0.0; + double fee = 0.0; std::int64_t timestamp = 0; }; std::unordered_map pending_send_info_; + // Opids issued as a fee-gap auto-retry (see maybeRetrySendForFeeGap). Tracked so a retry that + // fails again is reported to the user instead of looping. + std::unordered_set send_feegap_retried_opids_; // z_sendmany UI callbacks held until the opid reaches a terminal status, so the // user isn't told "sent successfully" before the tx is actually built/broadcast. std::unordered_map> diff --git a/src/app_network.cpp b/src/app_network.cpp index 19194a0..f41bccf 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -767,7 +767,8 @@ void App::upsertPendingSendTransaction(const std::string& opid, const std::string& from, const std::string& to, double amount, - const std::string& memo) + const std::string& memo, + double fee) { if (opid.empty()) return; @@ -778,6 +779,7 @@ void App::upsertPendingSendTransaction(const std::string& opid, pendingInfo.to = to; pendingInfo.amount = std::abs(amount); pendingInfo.memo = memo; + pendingInfo.fee = fee; TransactionInfo pending; pending.txid = opid; @@ -2357,7 +2359,7 @@ void App::sendTransaction(const std::string& from, const std::string& to, } catch (const std::exception& e) { result_str = e.what(); } - return [this, callback, ok, result_str, from, to, amount, memo]() { + return [this, callback, ok, result_str, from, to, amount, fee, memo]() { if (send_submissions_in_flight_ > 0) --send_submissions_in_flight_; if (ok) { // A send changes address balances — refresh on next cycle @@ -2372,7 +2374,7 @@ void App::sendTransaction(const std::string& from, const std::string& to, // "sent successfully" for an operation that may still fail. if (!result_str.empty()) { pending_opids_.push_back(result_str); - upsertPendingSendTransaction(result_str, from, to, amount, memo); + upsertPendingSendTransaction(result_str, from, to, amount, memo, fee); if (callback) pending_send_callbacks_[result_str] = callback; } else if (callback) { callback(true, result_str); // no opid to track — report as-is @@ -2385,4 +2387,116 @@ void App::sendTransaction(const std::string& from, const std::string& to, }); } +// Parse the daemon's "Insufficient shielded funds, have , need " message. std::stod stops at +// the trailing comma/text, so it extracts each amount cleanly. Returns false if it isn't that error. +static bool parseInsufficientShielded(const std::string& msg, double& have, double& need) +{ + if (msg.find("Insufficient shielded funds") == std::string::npos) return false; + auto hp = msg.find("have "); + auto np = msg.find("need "); + if (hp == std::string::npos || np == std::string::npos) return false; + try { + have = std::stod(msg.substr(hp + 5)); + need = std::stod(msg.substr(np + 5)); + } catch (...) { + return false; + } + return true; +} + +bool App::maybeRetrySendForFeeGap(const std::string& opid, const std::string& rawMsg) +{ + double have = 0.0, need = 0.0; + if (!parseInsufficientShielded(rawMsg, have, need)) return false; + + auto it = pending_send_info_.find(opid); + if (it == pending_send_info_.end()) return false; + const PendingSendInfo info = it->second; // copy before any cleanup + + // Only shielded sends hit the bug; a transparent "from" uses a different (correct) selector. + if (info.from.empty() || info.from[0] != 'z') return false; + // Discriminator: the daemon stopped once it covered the amount but short of the fee, so the + // selected total (have) is >= the amount. A genuine shortfall reports have < amount (it grabbed + // every note and still couldn't reach the amount) — don't "retry" those. + if (have + 1e-9 < info.amount) return false; + // Never retry a retry (the self-output already widened the target; a second failure is real). + if (send_feegap_retried_opids_.count(opid)) return false; + + const double fee = info.fee > 0.0 ? info.fee : 0.0001; + + // Hand the waiting UI callback to the retry so the user sees the final outcome, not the + // intermediate "insufficient" we're working around. + std::function cb; + auto cbIt = pending_send_callbacks_.find(opid); + if (cbIt != pending_send_callbacks_.end()) { + cb = cbIt->second; + pending_send_callbacks_.erase(cbIt); + } + + DEBUG_LOGF("[send] fee-gap workaround: retrying %s with self-output (have=%.8f need=%.8f amount=%.8f fee=%.8f)\n", + opid.c_str(), have, need, info.amount, fee); + resendWithFeeGapWorkaround(info.from, info.to, info.amount, fee, info.memo, std::move(cb)); + return true; +} + +void App::resendWithFeeGapWorkaround(const std::string& from, const std::string& to, + double amount, double fee, const std::string& memo, + std::function callback) +{ + if (!state_.connected || !rpc_ || !worker_) { + if (callback) callback(false, "Not connected"); + return; + } + + // recipients = the real recipient + a tiny self-output (= fee) back to `from`. The extra output + // raises the daemon's note-selection target (nTotalOut) above the single largest note, so its + // greedy picker grabs another note and can therefore also cover the miner fee. The recipient + // still receives EXACTLY `amount`; the self-output and any change return to `from`. + nlohmann::json recipients = nlohmann::json::array(); + nlohmann::json primary; + primary["address"] = to; + primary["amount"] = util::formatAmountFixed(amount); + if (!memo.empty()) primary["memo"] = memo; + recipients.push_back(primary); + nlohmann::json selfOut; + selfOut["address"] = from; + selfOut["amount"] = util::formatAmountFixed(fee); + recipients.push_back(selfOut); + + send_progress_active_ = true; + ++send_submissions_in_flight_; + worker_->post([this, from, to, amount, fee, memo, recipients, callback]() -> rpc::RPCWorker::MainCb { + bool ok = false; + std::string result_str; + try { + rpc::RPCClient::TraceScope trace("Send tab / Fee-gap retry"); + auto result = rpc_->call("z_sendmany", {from, recipients, 1, fee}); + result_str = result.get(); + ok = true; + } catch (const std::exception& e) { + result_str = e.what(); + } + return [this, callback, ok, result_str, from, to, amount, fee, memo]() { + if (send_submissions_in_flight_ > 0) --send_submissions_in_flight_; + if (ok) { + addresses_dirty_ = true; + transactions_dirty_ = true; + last_tx_block_height_ = -1; + network_refresh_.markWalletMutationRefresh(); + if (!result_str.empty()) { + pending_opids_.push_back(result_str); + send_feegap_retried_opids_.insert(result_str); // a retry of a retry is a real error + upsertPendingSendTransaction(result_str, from, to, amount, memo, fee); + if (callback) pending_send_callbacks_[result_str] = callback; + } else if (callback) { + callback(true, result_str); + } + } else { + send_progress_active_ = false; + if (callback) callback(false, result_str); + } + }; + }); +} + } // namespace dragonx