DragonX/ObsidianDragon
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

refactor(audit): batch 6 — de-duplicate the app_network send/balance paths

Browse Source 
Three behavior-preserving consolidations in src/app_network.cpp:

1. Fold the three near-identical pending-send balance-delta blocks
   (upsertPendingSendTransaction debit, removePendingSendTransactions
   restore, applyPendingSendBalanceDeltas debit) into one
   App::applyPendingSendDelta(from, signedAmount, includeAggregates). The
   restore path's unclamped `+amt` is provably identical to the clamped
   signed form (balance/amount are >=0 invariants), so a single clamped
   helper reproduces all three exactly.

2. Collapse createNewZAddress/createNewTAddress into one
   App::createNewAddress(bool shielded, cb) with thin public forwarders,
   preserving the lite early-return, the dual push into the type list AND
   the combined state_.addresses view, and the dirty-flag/refresh bookkeeping.

3. Extract App::submitZSendMany() shared by sendTransaction and
   resendWithFeeGapWorkaround — the only differences (the TraceScope label
   and the retry-only send_feegap_retried_opids_.insert) become parameters.
   The in-flight counter, opid tracking, upsertPendingSendTransaction, and
   pending_send_callbacks_ bookkeeping are byte-equivalent.

Full-node + Lite build clean; ctest 1/1; hygiene clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-07-02 19:36:27 -05:00
parent 9c533fa485
commit 4635a56e89
2 changed files with 97 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
src/app.h
View File

@@ -12,6 +12,7 @@
#include <chrono> #include <chrono>
#include <unordered_map> #include <unordered_map>
#include <unordered_set> #include <unordered_set>
#include <nlohmann/json_fwd.hpp>
#include "data/transaction_history_cache.h" #include "data/transaction_history_cache.h"
#include "data/wallet_state.h" #include "data/wallet_state.h"
#include "rpc/connection.h" #include "rpc/connection.h"
@@ -448,6 +449,11 @@ private:
bool sendStopCommandSafely(rpc::RPCClient& client, const char* context); bool sendStopCommandSafely(rpc::RPCClient& client, const char* context);
void maybeFinishTransactionSendProgress(); void maybeFinishTransactionSendProgress();
// Shared body of createNewZAddress/createNewTAddress (which are thin public forwarders).
// `shielded` selects the z_getnewaddress/getnewaddress RPC, the "shielded"/"transparent" type
// string, and the z_addresses/t_addresses target (the new AddressInfo is pushed into both that
// list and state_.addresses). Lite builds derive locally via the controller and early-return.
void createNewAddress(bool shielded, std::function<void(const std::string&)> callback);
void upsertPendingSendTransaction(const std::string& opid, void upsertPendingSendTransaction(const std::string& opid,
const std::string& from, const std::string& from,
const std::string& to, const std::string& to,
@@ -464,10 +470,28 @@ private:
void resendWithFeeGapWorkaround(const std::string& from, const std::string& to, void resendWithFeeGapWorkaround(const std::string& from, const std::string& to,
double amount, double fee, const std::string& memo, double amount, double fee, const std::string& memo,
std::function<void(bool, const std::string&)> callback); std::function<void(bool, const std::string&)> callback);
// Shared z_sendmany submit path for sendTransaction (single recipient, markFeeGapRetry=false)
// and resendWithFeeGapWorkaround (recipient + self-output, markFeeGapRetry=true). Owns the
// in-flight increment, the worker post + call, and the result closure (dirty flags, opid
// tracking, pending-send bookkeeping, callback delivery). Callers build `recipients` (and pass
// the recipient `to`/`amount`/`memo`/`fee` used to record the optimistic pending-send row).
// When markFeeGapRetry is set, the returned opid is recorded in send_feegap_retried_opids_ so a
// retry of a retry is reported as a real error.
void submitZSendMany(const std::string& from, const std::string& to, double amount, double fee,
const std::string& memo, const nlohmann::json& recipients,
const char* traceLabel, bool markFeeGapRetry,
std::function<void(bool, const std::string&)> callback);
void markPendingSendTransactionSucceeded(const std::string& opid, void markPendingSendTransactionSucceeded(const std::string& opid,
const std::string& txid); const std::string& txid);
void removePendingSendTransactions(const std::vector<std::string>& opids, void removePendingSendTransactions(const std::vector<std::string>& opids,
bool restoreBalances); bool restoreBalances);
// Apply a signed per-address balance delta for a pending send: walk z_addresses then
// t_addresses for `fromAddress` and clamp its balance at >=0. A positive `signedAmount`
// restores a debit; a negative one applies it. When `includeAggregates` is set, adjust the
// private/transparent bucket (chosen by the address's leading 'z') and totalBalance with the
// same clamp. Shared by the three pending-send delta sites so they can't drift.
void applyPendingSendDelta(const std::string& fromAddress, double signedAmount,
bool includeAggregates);
// Deliver a deferred z_sendmany result to its waiting UI callback once the opid // Deliver a deferred z_sendmany result to its waiting UI callback once the opid
// reaches a terminal status. Returns true if a callback was registered (and fired). // reaches a terminal status. Returns true if a callback was registered (and fired).
bool invokeSendResultCallback(const std::string& opid, bool ok, bool invokeSendResultCallback(const std::string& opid, bool ok,

218
src/app_network.cpp
View File

@@ -800,22 +800,7 @@ void App::upsertPendingSendTransaction(const std::string& opid,
state_.transactions.insert(state_.transactions.begin(), std::move(pending)); state_.transactions.insert(state_.transactions.begin(), std::move(pending));
} }
if (newPending) { if (newPending) {
auto applyDelta = [&](std::vector<AddressInfo>& addresses) { applyPendingSendDelta(pendingInfo.from, -pendingInfo.amount, /*includeAggregates*/ true);
for (auto& address : addresses) {
if (address.address == pendingInfo.from) {
address.balance = std::max(0.0, address.balance - pendingInfo.amount);
return true;
}
}
return false;
};
if (!applyDelta(state_.z_addresses)) applyDelta(state_.t_addresses);
if (!pendingInfo.from.empty() && pendingInfo.from[0] == 'z') {
state_.privateBalance = std::max(0.0, state_.privateBalance - pendingInfo.amount);
} else {
state_.transparentBalance = std::max(0.0, state_.transparentBalance - pendingInfo.amount);
}
state_.totalBalance = std::max(0.0, state_.totalBalance - pendingInfo.amount);
} }
state_.last_tx_update = std::time(nullptr); state_.last_tx_update = std::time(nullptr);
} }
@@ -850,22 +835,8 @@ void App::removePendingSendTransactions(const std::vector<std::string>& opids,
for (const auto& opid : opidSet) { for (const auto& opid : opidSet) {
auto pending = pending_send_info_.find(opid); auto pending = pending_send_info_.find(opid);
if (pending == pending_send_info_.end()) continue; if (pending == pending_send_info_.end()) continue;
auto restoreBalance = [&](std::vector<AddressInfo>& addresses) { applyPendingSendDelta(pending->second.from, pending->second.amount,
for (auto& address : addresses) { /*includeAggregates*/ true);
if (address.address == pending->second.from) {
address.balance += pending->second.amount;
return true;
}
}
return false;
};
if (!restoreBalance(state_.z_addresses)) restoreBalance(state_.t_addresses);
if (!pending->second.from.empty() && pending->second.from[0] == 'z') {
state_.privateBalance += pending->second.amount;
} else {
state_.transparentBalance += pending->second.amount;
}
state_.totalBalance += pending->second.amount;
} }
} }
state_.transactions.erase( state_.transactions.erase(
@@ -878,6 +849,32 @@ void App::removePendingSendTransactions(const std::vector<std::string>& opids,
state_.last_tx_update = std::time(nullptr); state_.last_tx_update = std::time(nullptr);
} }
void App::applyPendingSendDelta(const std::string& fromAddress, double signedAmount,
bool includeAggregates)
{
// For a debit (signedAmount < 0) this clamps at >=0 exactly as the debit sites did. For a
// restore (signedAmount > 0) the clamp is a no-op — max(0, bal+amt) == bal+amt when bal,amt>=0 —
// so the single clamped form reproduces the original restore's unclamped bal += amt.
auto applyToAddress = [&](std::vector<AddressInfo>& addresses) {
for (auto& address : addresses) {
if (address.address == fromAddress) {
address.balance = std::max(0.0, address.balance + signedAmount);
return true;
}
}
return false;
};
if (!applyToAddress(state_.z_addresses)) applyToAddress(state_.t_addresses);
if (includeAggregates) {
if (!fromAddress.empty() && fromAddress[0] == 'z') {
state_.privateBalance = std::max(0.0, state_.privateBalance + signedAmount);
} else {
state_.transparentBalance = std::max(0.0, state_.transparentBalance + signedAmount);
}
state_.totalBalance = std::max(0.0, state_.totalBalance + signedAmount);
}
}
void App::trackOperation(const std::string& opid) void App::trackOperation(const std::string& opid)
{ {
if (opid.empty()) return; if (opid.empty()) return;
@@ -903,24 +900,7 @@ void App::applyPendingSendBalanceDeltas(bool includeAggregateBalances)
{ {
for (const auto& [opid, pending] : pending_send_info_) { for (const auto& [opid, pending] : pending_send_info_) {
(void)opid; (void)opid;
auto applyDelta = [&](std::vector<AddressInfo>& addresses) { applyPendingSendDelta(pending.from, -pending.amount, includeAggregateBalances);
for (auto& address : addresses) {
if (address.address == pending.from) {
address.balance = std::max(0.0, address.balance - pending.amount);
return true;
}
}
return false;
};
if (!applyDelta(state_.z_addresses)) applyDelta(state_.t_addresses);
if (includeAggregateBalances) {
if (!pending.from.empty() && pending.from[0] == 'z') {
state_.privateBalance = std::max(0.0, state_.privateBalance - pending.amount);
} else {
state_.transparentBalance = std::max(0.0, state_.transparentBalance - pending.amount);
}
state_.totalBalance = std::max(0.0, state_.totalBalance - pending.amount);
}
} }
} }
@@ -1953,64 +1933,29 @@ void App::applyDefaultBanlist()
void App::createNewZAddress(std::function<void(const std::string&)> callback) void App::createNewZAddress(std::function<void(const std::string&)> callback)
{ {
// Lite build: derive locally via the controller (fast, no network). The backend auto-saves createNewAddress(/*shielded*/ true, std::move(callback));
// new addresses; the next lite refresh lists it with a balance.
if (lite_wallet_) {
const auto result = lite_wallet_->newAddress(/*shielded*/ true);
if (result.ok) {
AddressInfo info;
info.address = result.address;
info.type = "shielded";
info.balance = 0.0;
state_.z_addresses.push_back(info);
state_.addresses.push_back(info);
address_list_dirty_ = true;
}
if (callback) callback(result.ok ? result.address : std::string());
return;
}
if (!state_.connected || !rpc_ || !worker_) return;
worker_->post([this, callback]() -> rpc::RPCWorker::MainCb {
std::string addr;
try {
rpc::RPCClient::TraceScope trace("Receive tab / New shielded address");
json result = rpc_->call("z_getnewaddress");
addr = result.get<std::string>();
} catch (const std::exception& e) {
DEBUG_LOGF("z_getnewaddress error: %s\n", e.what());
}
return [this, callback, addr]() {
if (!addr.empty()) {
// Inject immediately so UI can select the address next frame
AddressInfo info;
info.address = addr;
info.type = "shielded";
info.balance = 0.0;
state_.z_addresses.push_back(info);
state_.addresses.push_back(info); // keep combined view in sync immediately
address_list_dirty_ = true;
// Also trigger full refresh to get proper balances
addresses_dirty_ = true;
refreshAddresses();
}
if (callback) callback(addr);
};
});
} }
void App::createNewTAddress(std::function<void(const std::string&)> callback) void App::createNewTAddress(std::function<void(const std::string&)> callback)
{ {
// Lite build: derive locally via the controller (see createNewZAddress). createNewAddress(/*shielded*/ false, std::move(callback));
}
void App::createNewAddress(bool shielded, std::function<void(const std::string&)> callback)
{
const char* typeStr = shielded ? "shielded" : "transparent";
auto& targetList = shielded ? state_.z_addresses : state_.t_addresses;
// Lite build: derive locally via the controller (fast, no network). The backend auto-saves
// new addresses; the next lite refresh lists it with a balance.
if (lite_wallet_) { if (lite_wallet_) {
const auto result = lite_wallet_->newAddress(/*shielded*/ false); const auto result = lite_wallet_->newAddress(shielded);
if (result.ok) { if (result.ok) {
AddressInfo info; AddressInfo info;
info.address = result.address; info.address = result.address;
info.type = "transparent"; info.type = typeStr;
info.balance = 0.0; info.balance = 0.0;
state_.t_addresses.push_back(info); targetList.push_back(info);
state_.addresses.push_back(info); state_.addresses.push_back(info);
address_list_dirty_ = true; address_list_dirty_ = true;
} }
@@ -2020,23 +1965,26 @@ void App::createNewTAddress(std::function<void(const std::string&)> callback)
if (!state_.connected || !rpc_ || !worker_) return; if (!state_.connected || !rpc_ || !worker_) return;
worker_->post([this, callback]() -> rpc::RPCWorker::MainCb { const char* rpcMethod = shielded ? "z_getnewaddress" : "getnewaddress";
const char* traceLabel = shielded ? "Receive tab / New shielded address"
: "Receive tab / New transparent address";
worker_->post([this, callback, shielded, typeStr, rpcMethod, traceLabel]() -> rpc::RPCWorker::MainCb {
std::string addr; std::string addr;
try { try {
rpc::RPCClient::TraceScope trace("Receive tab / New transparent address"); rpc::RPCClient::TraceScope trace(traceLabel);
json result = rpc_->call("getnewaddress"); json result = rpc_->call(rpcMethod);
addr = result.get<std::string>(); addr = result.get<std::string>();
} catch (const std::exception& e) { } catch (const std::exception& e) {
DEBUG_LOGF("getnewaddress error: %s\n", e.what()); DEBUG_LOGF("%s error: %s\n", rpcMethod, e.what());
} }
return [this, callback, addr]() { return [this, callback, addr, shielded, typeStr]() {
if (!addr.empty()) { if (!addr.empty()) {
// Inject immediately so UI can select the address next frame // Inject immediately so UI can select the address next frame
AddressInfo info; AddressInfo info;
info.address = addr; info.address = addr;
info.type = "transparent"; info.type = typeStr;
info.balance = 0.0; info.balance = 0.0;
state_.t_addresses.push_back(info); (shielded ? state_.z_addresses : state_.t_addresses).push_back(info);
state_.addresses.push_back(info); // keep combined view in sync immediately state_.addresses.push_back(info); // keep combined view in sync immediately
address_list_dirty_ = true; address_list_dirty_ = true;
// Also trigger full refresh to get proper balances // Also trigger full refresh to get proper balances
@@ -2414,26 +2362,36 @@ void App::sendTransaction(const std::string& from, const std::string& to,
return; return;
} }
send_progress_active_ = true;
++send_submissions_in_flight_;
// z_sendmany signature is (fromaddress, amounts, minconf, fee). The fee MUST be a JSON number: // z_sendmany signature is (fromaddress, amounts, minconf, fee). The fee MUST be a JSON number:
// the daemon reads it with get_real(), so a string is rejected ("JSON value is not a number as // the daemon reads it with get_real(), so a string is rejected ("JSON value is not a number as
// expected"). get_real() parses the double directly and accepts any number notation (incl. the // expected"). get_real() parses the double directly and accepts any number notation (incl. the
// "5e-05" form of a small fee), so passing the raw double is correct for every fee value. // "5e-05" form of a small fee), so passing the raw double is correct for every fee value.
// (The recipient "amount" above is intentionally a fixed-decimal STRING — that field is parsed // (The recipient "amount" above is intentionally a fixed-decimal STRING — that field is parsed
// with ParseFixedPoint, which a scientific-notation double would break.) // with ParseFixedPoint, which a scientific-notation double would break.)
worker_->post([this, from, to, amount, fee, memo, recipients, callback]() -> rpc::RPCWorker::MainCb { submitZSendMany(from, to, amount, fee, memo, recipients, "Send tab / Submit transaction",
/*markFeeGapRetry*/ false, std::move(callback));
}
void App::submitZSendMany(const std::string& from, const std::string& to, double amount, double fee,
const std::string& memo, const nlohmann::json& recipients,
const char* traceLabel, bool markFeeGapRetry,
std::function<void(bool, const std::string&)> callback)
{
send_progress_active_ = true;
++send_submissions_in_flight_;
worker_->post([this, from, to, amount, fee, memo, recipients, callback, traceLabel,
markFeeGapRetry]() -> rpc::RPCWorker::MainCb {
bool ok = false; bool ok = false;
std::string result_str; std::string result_str;
try { try {
rpc::RPCClient::TraceScope trace("Send tab / Submit transaction"); rpc::RPCClient::TraceScope trace(traceLabel);
auto result = rpc_->call("z_sendmany", {from, recipients, 1, fee}); auto result = rpc_->call("z_sendmany", {from, recipients, 1, fee});
result_str = result.get<std::string>(); result_str = result.get<std::string>();
ok = true; ok = true;
} catch (const std::exception& e) { } catch (const std::exception& e) {
result_str = e.what(); result_str = e.what();
} }
return [this, callback, ok, result_str, from, to, amount, fee, memo]() { return [this, callback, ok, result_str, from, to, amount, fee, memo, markFeeGapRetry]() {
if (send_submissions_in_flight_ > 0) --send_submissions_in_flight_; if (send_submissions_in_flight_ > 0) --send_submissions_in_flight_;
if (ok) { if (ok) {
// A send changes address balances — refresh on next cycle // A send changes address balances — refresh on next cycle
@@ -2448,6 +2406,8 @@ void App::sendTransaction(const std::string& from, const std::string& to,
// "sent successfully" for an operation that may still fail. // "sent successfully" for an operation that may still fail.
if (!result_str.empty()) { if (!result_str.empty()) {
pending_opids_.push_back(result_str); pending_opids_.push_back(result_str);
if (markFeeGapRetry)
send_feegap_retried_opids_.insert(result_str); // a retry of a retry is a real error
upsertPendingSendTransaction(result_str, from, to, amount, memo, fee); upsertPendingSendTransaction(result_str, from, to, amount, memo, fee);
if (callback) pending_send_callbacks_[result_str] = callback; if (callback) pending_send_callbacks_[result_str] = callback;
} else if (callback) { } else if (callback) {
@@ -2537,40 +2497,8 @@ void App::resendWithFeeGapWorkaround(const std::string& from, const std::string&
selfOut["amount"] = util::formatAmountFixed(fee); selfOut["amount"] = util::formatAmountFixed(fee);
recipients.push_back(selfOut); recipients.push_back(selfOut);
send_progress_active_ = true; submitZSendMany(from, to, amount, fee, memo, recipients, "Send tab / Fee-gap retry",
++send_submissions_in_flight_; /*markFeeGapRetry*/ true, std::move(callback));
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<std::string>();
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);
}
};
});
} }
// -------------------------------------------------------------------------------------------- // --------------------------------------------------------------------------------------------