From 02b4d03fc6828de05ee8356d22c669be7f48c06f Mon Sep 17 00:00:00 2001 From: DanS Date: Wed, 26 Aug 2026 22:10:35 -0500 Subject: [PATCH] wallet: low-severity polish from the dev/v1.2.0 review Follow-up nits surfaced by the multi-agent review of the dragonx..dev delta; none are correctness/consensus bugs, all are defensive/consistency tidy-ups. Builds clean; the diff was reviewed across concurrency, scheduler, and tx-building lenses. - wallet: default CWallet::fAutoShieldEnabled to false. init.cpp always recomputes it (ON only for CREATED/RESTORED seed provenance) before any ChainTip round, so this is behaviour-neutral in the normal path and stops a CWallet that skips that init from auto-enabling for provenance the gate would reject. - wallet: clamp a loaded hdSeedOrigin to UNKNOWN when out of enum range, so a corrupt/hand-edited wallet.dat cannot claim a known-recoverable seed and flip autoshield ON. - wallet: key the sweep and consolidation ops' NU-straddle guard, transaction builder height, and expiry off execution-time tipHeight instead of the stale enqueue-time targetHeight_ -- matching the autoshield op (65130c312) so the builder's consensus-branch selection agrees with the height the tx is signed for. (Sweep previously built at targetHeight_ but expired at the live tip.) - wallet: on the sweep NU-straddle skip, set sweepComplete_ so the round backs nextSweep off one interval instead of re-dispatching a fresh sweep op every block through the activation window. - init: clamp -autoshieldinterval below 5 up to the documented minimum of 5, rather than silently resetting it to the default 25. - chainparams: make the ClearSeeds guard an exact "DRAGONX" match instead of a 7-char prefix, so DRAGONX-prefixed assetchains (e.g. DRAGONX2) no longer inherit DragonX's seeds. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01UtvyqQQSqR64DNEjUEuTmb --- src/chainparams.cpp | 2 +- src/init.cpp | 4 ++-- ...asyncrpcoperation_saplingconsolidation.cpp | 18 +++++++++++---- src/wallet/asyncrpcoperation_sweep.cpp | 22 +++++++++++++------ src/wallet/wallet.h | 9 +++++--- src/wallet/walletdb.cpp | 6 +++++ 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 6396faabb..fc05e59b2 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -552,7 +552,7 @@ void *chainparams_commandline() { ASSETCHAINS_P2PPORT = 18030; } - if (strncmp(SMART_CHAIN_SYMBOL, "DRAGONX", 7) != 0) { + if (strcmp(SMART_CHAIN_SYMBOL, "DRAGONX") != 0) { pCurrentParams->ClearSeeds(); } diff --git a/src/init.cpp b/src/init.cpp index 0766abe03..1bdff45cf 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2521,8 +2521,8 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (pwalletMain->fAutoShieldEnabled) { int autoShieldInterval = GetArg("-autoshieldinterval", 25); if (autoShieldInterval < 5) { - fprintf(stderr,"%s: Invalid autoshield interval of %d < 5, setting to default of 25\n", __func__, autoShieldInterval); - autoShieldInterval = 25; + fprintf(stderr,"%s: autoshield interval %d below the minimum, clamping to 5\n", __func__, autoShieldInterval); + autoShieldInterval = 5; } pwalletMain->autoShieldInterval = autoShieldInterval; pwalletMain->nextAutoShield = pwalletMain->autoShieldInterval + chainActive.Height(); diff --git a/src/wallet/asyncrpcoperation_saplingconsolidation.cpp b/src/wallet/asyncrpcoperation_saplingconsolidation.cpp index 2f360dfed..085768315 100644 --- a/src/wallet/asyncrpcoperation_saplingconsolidation.cpp +++ b/src/wallet/asyncrpcoperation_saplingconsolidation.cpp @@ -107,8 +107,18 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { auto opid=getId(); LogPrintf("%s: Beginning AsyncRPCOperation_saplingconsolidation\n", opid); auto consensusParams = Params().GetConsensus(); - auto nextActivationHeight = NextActivationHeight(targetHeight_, consensusParams); - if (nextActivationHeight && targetHeight_ + CONSOLIDATION_EXPIRY_DELTA >= nextActivationHeight.get()) { + int tipHeight; + { + LOCK(cs_main); + tipHeight = (chainActive.Tip() != NULL) ? chainActive.Tip()->GetHeight() : targetHeight_; + } + + // Build and expire against tipHeight (execution-time), not the stale + // enqueue-time targetHeight_, so the builder's consensus-branch selection and + // the NU-straddle guard agree with the height the tx is signed for. Mirrors + // the autoshield op (commit 65130c312). + auto nextActivationHeight = NextActivationHeight(tipHeight, consensusParams); + if (nextActivationHeight && tipHeight + CONSOLIDATION_EXPIRY_DELTA >= nextActivationHeight.get()) { LogPrintf("%s: Consolidation txs would be created before a NU activation but may expire after. Skipping this round.\n",opid); setConsolidationResult(0, 0, std::vector()); return status; @@ -189,8 +199,8 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() { if (fromNotes.size() < minQuantity) continue; - auto builder = TransactionBuilder(consensusParams, targetHeight_, pwalletMain); - builder.SetExpiryHeight(targetHeight_ + CONSOLIDATION_EXPIRY_DELTA); + auto builder = TransactionBuilder(consensusParams, tipHeight, pwalletMain); + builder.SetExpiryHeight(tipHeight + CONSOLIDATION_EXPIRY_DELTA); auto actualAmountToSend = amountToSend < fConsolidationTxFee ? 0 : amountToSend - fConsolidationTxFee; LogPrintf("%s: %s Beginning to create transaction with Sapling output amount=%s\n", __func__, opid, FormatMoney(actualAmountToSend)); diff --git a/src/wallet/asyncrpcoperation_sweep.cpp b/src/wallet/asyncrpcoperation_sweep.cpp index a67052414..3abd70590 100644 --- a/src/wallet/asyncrpcoperation_sweep.cpp +++ b/src/wallet/asyncrpcoperation_sweep.cpp @@ -126,10 +126,21 @@ bool AsyncRPCOperation_sweep::main_impl() { auto opid=getId(); LogPrintf("%s: Beginning asyncrpcoperation_sweep.\n", getId()); auto consensusParams = Params().GetConsensus(); - auto nextActivationHeight = NextActivationHeight(targetHeight_, consensusParams); - if (nextActivationHeight && targetHeight_ + SWEEP_EXPIRY_DELTA >= nextActivationHeight.get()) { + int tipHeight; + { + LOCK(cs_main); + tipHeight = (chainActive.Tip() != NULL) ? chainActive.Tip()->GetHeight() : targetHeight_; + } + + // Key the NU-straddle guard and the tx builder/expiry off tipHeight (the + // height we actually build and expire against), not the stale enqueue-time + // targetHeight_, so a queue delay cannot slip a straddling expiry past this + // guard. Mirrors the autoshield op (commit 65130c312). + auto nextActivationHeight = NextActivationHeight(tipHeight, consensusParams); + if (nextActivationHeight && tipHeight + SWEEP_EXPIRY_DELTA >= nextActivationHeight.get()) { LogPrintf("%s: Sweep txs would be created before a NU activation but may expire after. Skipping this round.\n", getId()); setSweepResult(0, 0, std::vector()); + sweepComplete_ = true; // nothing to do this round; back nextSweep off one interval instead of re-dispatching every block return true; } @@ -258,11 +269,8 @@ bool AsyncRPCOperation_sweep::main_impl() { fee = 0; } - auto builder = TransactionBuilder(consensusParams, targetHeight_, pwalletMain); - { - LOCK2(cs_main, pwalletMain->cs_wallet); - builder.SetExpiryHeight(chainActive.Tip()->GetHeight()+ SWEEP_EXPIRY_DELTA); - } + auto builder = TransactionBuilder(consensusParams, tipHeight, pwalletMain); + builder.SetExpiryHeight(tipHeight + SWEEP_EXPIRY_DELTA); LogPrintf("%s: Beginning creating transaction with Sapling output amount=%s\n", getId(), FormatMoney(amountToSend - fee)); // Select Sapling notes diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1b77ca291..b54805684 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -810,10 +810,13 @@ public: bool fSweepExternalEnabled = false; bool fSweepRunning = false; - // Automatic coinbase shielding (t->z). Default ON but conditional: it is a - // silent no-op on nodes where it cannot act (no wallet, external + // Automatic coinbase shielding (t->z). The real default is computed in + // init.cpp: ON only for known-recoverable seed provenance (CREATED/RESTORED), + // conditional, and a silent no-op where it cannot act (no wallet, external // -mineraddress, non-mining, or locked wallet). See RunAutoShieldCoinbase. - bool fAutoShieldEnabled = true; + // The member defaults false so a CWallet that skips that init path never + // auto-enables for provenance the gate would otherwise have rejected. + bool fAutoShieldEnabled = false; bool fAutoShieldRunning = false; std::atomic fAbortRescan{false}; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 47bc658a4..cfe632c48 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -905,6 +905,12 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { int64_t nOrigin = 0; ssValue >> nOrigin; + // Clamp an out-of-range value (corrupt or hand-edited wallet.dat) to + // UNKNOWN — the conservative origin that leaves autoshield OFF — rather + // than trusting it to claim a known-recoverable (CREATED/RESTORED) seed. + if (nOrigin < CWallet::HDSEED_ORIGIN_UNRECORDED || nOrigin > CWallet::HDSEED_ORIGIN_UNKNOWN) { + nOrigin = CWallet::HDSEED_ORIGIN_UNKNOWN; + } pwallet->hdSeedOrigin = (int)nOrigin; } else if (strType == "mnementropy")