hygiene: Phase 6 (partial) — fix 7 verified bugs + quick-wins + dedup
From the Phase-6 structural scoping: land the verified behavioral bugs and
the safe quick-wins/dedups now; the large refactors (monolith splits, ~180
header globals) and consensus-adjacent items stay deferred. Built clean;
self-mined; verifychain=true.
BUGS (verified by reading the code):
- net.cpp CNode::Ban: a braceless `if (subNet.Match(...))` left
`pnode->fDisconnect = true;` OUTSIDE the guard, so banning any one subnet
marked EVERY connected peer for disconnect (dropped the whole peer set).
Wrapped the two statements in braces. (LIVE, high severity.)
- rpcdump.cpp importwallet: the `!fGood -> throw "Error adding some keys"`
check was trapped inside the `if (fRescan)` block, so importwallet with
rescan=false silently reported success when key import failed. Hoisted the
check before the rescan branch and cleaned the garbled braces/indentation.
- wallet.cpp CommitTransaction: ignored AddToWallet()'s return, so a failed
disk-persist of a just-signed spend was swallowed while the tx broadcast.
Now logs a hard error on failure.
- hush_nSPV_fullnode.h: the UTXOS branch declared `uint8_t filter` while the
twin TXIDS branch uses `uint32_t filter`; dragon_rwnum switches on
sizeof(filter), so the utxos path parsed only 1 of 4 wire filter bytes.
Widened to uint32_t.
- asyncrpcoperation_sweep.cpp: LogPrintf("%s ... %s", one-arg) read a missing
vararg; added the __func__ argument.
- rpcdump.cpp importprivkey: inner `auto secret_key` shadowed the outer
uint8_t and changed the type into DecodeCustomSecret; dropped the shadow.
- rpcdump.cpp getrescaninfo: char[8] + sprintf("%.4f") overflows when the
ratio >= 10.0 (transient reorg); widened to char[16] + snprintf.
QUICK WINS: removed the duplicate DRAGON_MAXSCRIPTSIZE #define; pinned the
dead HUSH3-branch NOTARISATION_SCAN_LIMIT_BLOCKS to 1440; fixed init typos
(fRequestShutdown, RPC warmup).
DEDUP: extracted the 19-line try/catch error-mapping block — copy-pasted
identically into all six async operations — into
AsyncRPCOperation::set_error_from_current_exception(), so the mapping is
edited in one place. Behavior-identical (verified all six blocks were byte-
identical first).
Deferred (endorsed by the scoping, better as their own PRs): addrman Select_
dedup, the pow.cpp powLimit helper (consensus file), the miner CreateNewBlock
lock-asymmetry, the wallet monolith splits, and the ~180-global / consensus-
retarget / Komodo-heritage work.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -18,6 +18,7 @@
|
||||
******************************************************************************/
|
||||
|
||||
#include "asyncrpcoperation.h"
|
||||
#include <stdexcept>
|
||||
|
||||
#include <boost/uuid/uuid.hpp>
|
||||
#include <boost/uuid/uuid_generators.hpp>
|
||||
@@ -58,6 +59,31 @@ AsyncRPCOperation::AsyncRPCOperation(const AsyncRPCOperation& o) :
|
||||
{
|
||||
}
|
||||
|
||||
// Shared error mapping for every async op's main(): rethrow the in-flight
|
||||
// exception and translate it to this operation's error code/message. Keeping
|
||||
// it here means the mapping is edited in one place, not copy-pasted into six.
|
||||
void AsyncRPCOperation::set_error_from_current_exception()
|
||||
{
|
||||
try {
|
||||
throw;
|
||||
} catch (const UniValue& objError) {
|
||||
set_error_code(find_value(objError, "code").get_int());
|
||||
set_error_message(find_value(objError, "message").get_str());
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
}
|
||||
}
|
||||
|
||||
AsyncRPCOperation& AsyncRPCOperation::operator=( const AsyncRPCOperation& other ) {
|
||||
this->id_ = other.id_;
|
||||
this->creation_time_ = other.creation_time_;
|
||||
|
||||
@@ -148,6 +148,11 @@ protected:
|
||||
this->error_message_ = errorMessage;
|
||||
}
|
||||
|
||||
// Map the in-flight (rethrown) exception to error_code_/error_message_. Called from
|
||||
// every async op's main() catch(...) so the UniValue/runtime/logic/exception mapping
|
||||
// lives in one place instead of being copy-pasted into all six operations.
|
||||
void set_error_from_current_exception();
|
||||
|
||||
void set_result(UniValue v) {
|
||||
std::lock_guard<std::mutex> guard(lock_);
|
||||
this->result_ = v;
|
||||
|
||||
@@ -41,7 +41,9 @@
|
||||
// XXX: There are potential crashes wherever we access chainActive without a lock,
|
||||
// because it might be disconnecting blocks at the same time.
|
||||
// TODO: this assumes a blocktime of 75 seconds for HUSH and 60 seconds for other chains
|
||||
int NOTARISATION_SCAN_LIMIT_BLOCKS = strncmp(SMART_CHAIN_SYMBOL, "HUSH3",5) == 0 ? 1152 : 1440;
|
||||
// DragonX: the HUSH3 (1152) branch is dead — SMART_CHAIN_SYMBOL is always "DRAGONX", and at
|
||||
// static-init time it is empty, so this already always resolved to 1440. Pinned to 1440.
|
||||
int NOTARISATION_SCAN_LIMIT_BLOCKS = 1440;
|
||||
CBlockIndex *hush_getblockindex(uint256 hash);
|
||||
|
||||
/* On HUSH */
|
||||
|
||||
@@ -597,7 +597,7 @@ void hush_netevent(std::vector<uint8_t> payload);
|
||||
int32_t getacseason(uint32_t timestamp);
|
||||
int32_t gethushseason(int32_t height);
|
||||
|
||||
#define DRAGON_MAXSCRIPTSIZE 10001
|
||||
// DRAGON_MAXSCRIPTSIZE is defined once near the top of this header; the duplicate here was removed.
|
||||
#define HUSH_KVDURATION 1440
|
||||
#define HUSH_KVBINARY 2
|
||||
#define PRICES_SMOOTHWIDTH 1
|
||||
|
||||
@@ -660,7 +660,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
|
||||
struct NSPV_utxosresp U;
|
||||
if ( len >= 2 && len < 64+5 && request[1] < 64 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
|
||||
{
|
||||
int32_t skipcount = 0; char coinaddr[64]; uint8_t filter; uint8_t isCC = 0;
|
||||
int32_t skipcount = 0; char coinaddr[64]; uint32_t filter; uint8_t isCC = 0;
|
||||
memcpy(coinaddr,&request[2],request[1]); // request[1] < 64 bounds the copy + the terminator write
|
||||
coinaddr[request[1]] = 0;
|
||||
if ( request[1] == len-3 )
|
||||
|
||||
@@ -153,7 +153,7 @@ std::atomic<bool> fRequestShutdown(false);
|
||||
void StartShutdown()
|
||||
{
|
||||
if(fDebug) {
|
||||
fprintf(stderr,"%s: fRequestShudown=true\n", __FUNCTION__);
|
||||
fprintf(stderr,"%s: fRequestShutdown=true\n", __FUNCTION__);
|
||||
}
|
||||
fRequestShutdown = true;
|
||||
}
|
||||
@@ -2816,7 +2816,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
|
||||
|
||||
SetRPCWarmupFinished();
|
||||
if(fDebug)
|
||||
fprintf(stderr,"RPC warmump finished\n");
|
||||
fprintf(stderr,"RPC warmup finished\n");
|
||||
uiInterface.InitMessage(_("Full Node Done Loading! :)"));
|
||||
|
||||
#ifdef ENABLE_WALLET
|
||||
|
||||
@@ -691,9 +691,10 @@ void CNode::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t banti
|
||||
{
|
||||
LOCK(cs_vNodes);
|
||||
for (CNode* pnode : vNodes) {
|
||||
if (subNet.Match(static_cast<CNetAddr>(pnode->addr)))
|
||||
if (subNet.Match(static_cast<CNetAddr>(pnode->addr))) {
|
||||
LogPrint("net", "%s: disconnecting from banned node %s\n", __func__, pnode->addr.ToString().c_str() );
|
||||
pnode->fDisconnect = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -63,23 +63,8 @@ void AsyncRPCOperation_autoshieldcoinbase::main() {
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
stop_execution_clock();
|
||||
|
||||
@@ -133,23 +133,8 @@ void AsyncRPCOperation_mergetoaddress::main()
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
#ifdef ENABLE_MINING
|
||||
|
||||
@@ -52,23 +52,8 @@ void AsyncRPCOperation_saplingconsolidation::main() {
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
stop_execution_clock();
|
||||
|
||||
@@ -155,23 +155,8 @@ void AsyncRPCOperation_sendmany::main() {
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
unlock_notes();
|
||||
|
||||
@@ -116,23 +116,8 @@ void AsyncRPCOperation_shieldcoinbase::main() {
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
#ifdef ENABLE_MINING
|
||||
|
||||
@@ -45,23 +45,8 @@ void AsyncRPCOperation_sweep::main() {
|
||||
|
||||
try {
|
||||
success = main_impl();
|
||||
} catch (const UniValue& objError) {
|
||||
int code = find_value(objError, "code").get_int();
|
||||
std::string message = find_value(objError, "message").get_str();
|
||||
set_error_code(code);
|
||||
set_error_message(message);
|
||||
} catch (const runtime_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("runtime error: " + string(e.what()));
|
||||
} catch (const logic_error& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("logic error: " + string(e.what()));
|
||||
} catch (const exception& e) {
|
||||
set_error_code(-1);
|
||||
set_error_message("general exception: " + string(e.what()));
|
||||
} catch (...) {
|
||||
set_error_code(-2);
|
||||
set_error_message("unknown error");
|
||||
set_error_from_current_exception();
|
||||
}
|
||||
|
||||
stop_execution_clock();
|
||||
@@ -111,7 +96,7 @@ bool IsExcludedAddress(libzcash::SaplingPaymentAddress zaddr) {
|
||||
}
|
||||
} else {
|
||||
// This is an invalid sapling zaddr
|
||||
LogPrintf("%s: Invalid zsweepexclude zaddr %s, ignoring\n", sweepExcludeAddress);
|
||||
LogPrintf("%s: Invalid zsweepexclude zaddr %s, ignoring\n", __func__, sweepExcludeAddress);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
@@ -196,9 +196,9 @@ UniValue getrescaninfo(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
auto startHeight = pwalletMain->rescanStartHeight;
|
||||
auto currentHeight = chainActive.Height();
|
||||
// if current height is 0, progress=1 since there is nothing to rescan
|
||||
char progress[8];
|
||||
char progress[16];
|
||||
if (currentHeight != 0) {
|
||||
sprintf(progress, "%.4f", (double) rescanHeight / (double) currentHeight );
|
||||
snprintf(progress, sizeof(progress), "%.4f", (double) rescanHeight / (double) currentHeight );
|
||||
ret.push_back(Pair("rescan_progress", progress));
|
||||
}
|
||||
ret.push_back(Pair("rescan_start_height", startHeight));
|
||||
@@ -305,7 +305,7 @@ UniValue importprivkey(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
|
||||
if (params.size() > 4)
|
||||
{
|
||||
auto secret_key = AmountFromValue(params[4])/100000000;
|
||||
secret_key = AmountFromValue(params[4])/100000000;
|
||||
key = DecodeCustomSecret(strSecret, secret_key);
|
||||
} else {
|
||||
key = DecodeSecret(strSecret);
|
||||
@@ -585,25 +585,27 @@ UniValue importwallet_impl(const UniValue& params, bool fHelp, bool fImportZKeys
|
||||
}
|
||||
}
|
||||
|
||||
if (fRescan) {
|
||||
CBlockIndex *pindex = chainActive.LastTip();
|
||||
while (pindex && pindex->pprev && pindex->GetBlockTime() > nTimeBegin - 7200)
|
||||
pindex = pindex->pprev;
|
||||
// A failed key/address add must surface on BOTH paths; previously this
|
||||
// check lived inside the fRescan block, so importwallet with rescan=false
|
||||
// silently reported success even when some keys failed to import.
|
||||
if (!fGood)
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys to wallet");
|
||||
|
||||
LogPrintf("Rescanning last %i blocks\n", chainActive.Height() - pindex->GetHeight() + 1);
|
||||
if (!pwalletMain->nTimeFirstKey || nTimeBegin < pwalletMain->nTimeFirstKey)
|
||||
pwalletMain->nTimeFirstKey = nTimeBegin;
|
||||
pwalletMain->ScanForWalletTransactions(pindex);
|
||||
pwalletMain->MarkDirty();
|
||||
if (fRescan) {
|
||||
CBlockIndex *pindex = chainActive.LastTip();
|
||||
while (pindex && pindex->pprev && pindex->GetBlockTime() > nTimeBegin - 7200)
|
||||
pindex = pindex->pprev;
|
||||
|
||||
if (!fGood)
|
||||
throw JSONRPCError(RPC_WALLET_ERROR, "Error adding some keys to wallet");
|
||||
LogPrintf("Rescanning last %i blocks\n", chainActive.Height() - pindex->GetHeight() + 1);
|
||||
if (!pwalletMain->nTimeFirstKey || nTimeBegin < pwalletMain->nTimeFirstKey)
|
||||
pwalletMain->nTimeFirstKey = nTimeBegin;
|
||||
pwalletMain->ScanForWalletTransactions(pindex);
|
||||
pwalletMain->MarkDirty();
|
||||
} else {
|
||||
LogPrintf("Importwallet without rescan successful\n");
|
||||
}
|
||||
|
||||
return NullUniValue; }
|
||||
|
||||
else{
|
||||
LogPrintf("Importwallet without Rescan successfull\n");
|
||||
return NullUniValue;}
|
||||
return NullUniValue;
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -4703,7 +4703,8 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey)
|
||||
|
||||
// Add tx to wallet, because if it has change it's also ours,
|
||||
// otherwise just for transaction history.
|
||||
AddToWallet(wtxNew, false, pwalletdb);
|
||||
if (!AddToWallet(wtxNew, false, pwalletdb))
|
||||
LogPrintf("CommitTransaction(): Error: failed to persist wallet tx %s to disk; wallet may be out of sync with the ledger\n", wtxNew.GetHash().ToString());
|
||||
|
||||
// Notify that old coins are spent
|
||||
set<CWalletTx*> setCoins;
|
||||
|
||||
Reference in New Issue
Block a user