- ListTransactions computes a tx's "size" once, lazily, through a const
CTransaction reference instead of a slicing copy per entry.
- listtransactions copies the returned window once from the built array.
- OrderedTxItems can open the wallet DB read-only without a flush on close;
listtransactions uses it, so a read-only RPC no longer forces a BDB
checkpoint (ThreadFlushWalletDB still flushes about 2 s later).
- gettransaction hex-encodes the tx without copying it.
Output is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
z_listreceivedbyaddress copied a whole CWalletTx per returned note to call
GetTxTime(). z_viewtransaction copied the previous tx of each own spend,
CWalletTx::DecryptSaplingNote copied the output and the note data (witness
list included) to read one field, and zsTxSpendsToJSON copied the note
data. All of them now bind const references to the same elements. The
lookups that throw or assert still do, and output is unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
z_listsentbyaddress, getalldata, z_getbalances and z_listreceivedaddress
deep-copied every CWalletTx (witness lists included) into an ordered map,
and the JSON helpers re-derived keys and scanned the whole wallet for every
output: zsTxReceivedToJSON trial-decrypted each output with every spending
key, and each change check called GetNullifiersForAddresses, a full
mapWallet note scan, once per decrypted output.
- ZsTxJSONContext memoises per call the spending keys with their ivk/ovk,
the HD-seed ovk and the wallet's nullifier set.
- Wallet txs are ordered by pointer. stable_sort + unique on nOrderPos keeps
the first entry, exactly like the old map::insert.
- A received output is first decrypted with the ivk the wallet recorded for
it when that ivk belongs to one of the usable keys, then with the
remaining keys in the old order.
- "size" is measured without a slicing copy; the result is reversed in one
pass; getalldata's note loop reads note data by reference and reuses the
memoised keys.
Output is unchanged. New opt-in 6th argument sendsOnly (default false):
true omits each entry's "spends" and "received" sections and the trial
decryption they need. Older daemons reject a 6th argument with the help
text, so a client can fall back.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Second slice of the z_sendmany split. The ~60-line loop that parses and
validates the "outputs" array — rejecting unknown keys, bad addresses, memos
on taddrs, oversize memos and negative amounts, and sorting recipients into
taddr/zaddr lists while accumulating nTotalOut — is lifted verbatim into
ParseSendManyRecipients(outputs, branchId, taddrRecipients, zaddrRecipients,
nTotalOut).
Verbatim extract-method (the loop text was moved unchanged, not retyped); the
helper reads only outputs + branchId and writes the three by-reference outputs
the loop already produced, which the clean compile proves self-contained (a
reference to any other z_sendmany local would fail to link). Built clean;
verifychain=true.
Together with the SelectAnyZaddrSource slice, z_sendmany is now ~135 lines
shorter, with source-address resolution and recipient parsing separated out.
Remaining: note selection, Sietch padding, and tx assembly (later slices,
best validated against a synced node since z_sendmany's linkability guard
blocks the send path on a peerless node).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First slice of the z_sendmany monolith split (Phase-6 refactor). The
`fromaddress == "z"` case ("spend from any zaddr") was ~76 lines inline: it
gathers the wallet's Sapling notes, sums balances per zaddr, and picks a
random zaddr whose confirmed balance covers total outputs + fee. Lift it
verbatim into a helper `SelectAnyZaddrSource(outputs, params)` that returns
the chosen zaddr (or throws the same JSONRPCError), and call it from
z_sendmany.
Behavior-preserving by construction: the block was moved unchanged, the only
edit being `fromaddress = vPotentialAddresses[...]` -> `return ...` with the
call site doing `fromaddress = SelectAnyZaddrSource(outputs, params)`, so
fromaddress receives the identical value (or the identical throw propagates).
The helper takes only outputs/params + globals, which the clean compile
proves (a reference to any z_sendmany local would not link). Built clean;
verifychain=true.
Note: the runtime happy path could not be exercised on an isolated node —
z_sendmany's private-chain "still syncing" linkability guard fires before the
fromaddress resolution when there are no peers — but a verbatim extract-method
does not require it. z_sendmany is now ~76 lines shorter; further extraction
(recipient parsing, fee/tx assembly) remains for follow-up slices.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fifth phase of the code-hygiene remediation, done per-file (18 files) via an
18-agent Workflow with strict guardrails. No consensus or security VALUES
changed (verified: pow/hush_utils/hush_bitcoind/net.h/crypter/stratum have
zero non-comment numeric changes); built clean, self-mined, verifychain=true.
Totals: 57 stale comments corrected, 46 user-facing branding fixes, 6
behavior-preserving named constants, 4 stratum warnings, 77 items deliberately
left (consensus/security values, copyright headers).
- Stale comments -> DragonX: corrected HUSH3/HUSH/Komodo/Equihash/Arrakis
leftovers that misdescribed the active chain (pow AWT/lwma notes, txdb
"Equihash solution" -> RandomX, wallet CLTV/overwinter/Sapling@1 notes,
util datadir provenance, crypter's stale mapSproutSpendingKeys invariant).
Replaced unprofessional/editorializing comments (profane TODOs, "developers
are elite") with neutral technical notes, preserving the real design info.
- User-facing branding: dumpwallet header "created by Hush" -> "DragonX";
RPC help text hushd/hush-cli/hushprivkey/hushaddress/Agama -> dragonx*;
rebranded provably-dead "HUSH3" symbol-fallback literals to DRAGONX.
- Named constants (same values, non-consensus/non-security): addrman
peer-selection factors (kChanceFactorGrowth/kChanceScale/deprioritize),
sietch MIN_ZOUTS, an init notarization-DB cache size.
- Left the security macros (ECC/TFM_TIMING_RESISTANT=420, comment-clarified
only), the consensus subsidy/commission literals + 128/129 TRANSITION
(comment only), and copyright headers untouched.
- Added a prominent WARNING that the stratum server is Equihash-era and NOT
updated for DragonX's RandomX PoW (flagged, not rewritten).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fourth phase of the code-hygiene remediation, addressing the ~1,082 stray
fprintf(stderr)/printf debug calls so consensus and hot paths stop spewing
to stderr/stdout. Done per-file (18 files) with conservative rules; the tree
builds clean, self-mines, and `verifychain` re-validates the whole chain
(pow/txdb/coins/miner paths) with -debug=1 enabling every converted line —
zero tinyformat/format-arg exceptions.
Net across 18 files: 174 commented-out debug lines deleted, 173 unconditional
live prints converted to LogPrint("<cat>",...)/LogPrintf (net/mining/pow/nspv/
zrpc categories, format strings + args preserved exactly), 40 pure-noise or
sensitive prints deleted, and 194 calls DELIBERATELY LEFT (already behind
fDebug/fZdebug guards, or genuine startup/fatal-error output that must reach
the console before logging init).
Notable:
- Deleted sensitive success-path dumps (nSPV SIG_TXHASH + full tx input/
output/change amounts; kvupdate privkey/pubkey hex) that were writing key
and amount material straight to stderr/stdout.
- Removed the raw 32-byte target hex dumps in the zawy adaptive-PoW helpers
and the legacy one-shot `if(height==340000)` HUSH artifact in pow.cpp.
- Converted per-tx relay + ban/banlist (net), per-setgenerate MININGTHREADS
(rpc/mining), signrawtransaction TXPOW, and per-message nSPV traces.
- Left format/arg-mismatched lines untouched (flagged) to avoid introducing
tinyformat runtime throws.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 1 of the code-hygiene remediation. Each is a genuine defect, not
style. The two consensus functions are touched only by provably
behavior-preserving dead-code removal and a comment.
- miner: initialize the anti-spin loop counter (was `int i;` -- the loop
condition read an indeterminate value, UB) and break once the block
time advances past the median, which is what the comment intended.
- consensus/upgrades: drop a stray printf() on the NetworkUpgradeState
path; the following assert already documents the invariant.
- txdb: CBlockTreeDB::Snapshot2's outer catch treated ANY exception as
normal end-of-iteration and built a snapshot from partial data. Fail
instead, matching the inner catch the author marked consensus-relevant
("we need to exit here if so for consensus code!"). iter->Valid()
already handles genuine end-of-iteration.
- wallet/rpcwallet + init: -sietch-min-zouts used a "--" key that the arg
parser (which normalizes --foo to -foo) can never match, so the Sietch
decoy floor was silently stuck at the default. Use the single-dash key
so the knob works, and document it in -help.
- hush_bitcoind + hush_utils: remove four unreachable duplicate `else if`
branches from hush_commission()/hush_block_subsidy() (a second
`height < 23860000` and a second `height < 27220000` in each). Proven
identical across 49,591 heights. NB: the dead values hint at an intended
clean halving schedule that was never wired up; the DEPLOYED schedule
(two double-steps) is preserved exactly. Changing it is a future
consensus decision, not this cleanup.
- hush_bitcoind: replace the "likely a bug" halving TODO with an accurate
note -- INTERVAL is only consumed by a debug fprintf, so the > vs >=
boundary at HALVING1 has no consensus effect.
- git rm two committed macOS build artifacts (cc/customcc.dylib and
libcc.dylib) and add the missing .dylib .gitignore rules.
Built clean on Linux; an isolated node self-mined genesis->5113 exercising
the miner and consensus-subsidy paths, and getsnapshot returned normally.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Six small defects, all found by an audit of the automated-operation path and all
verified present before changing anything.
AsyncRPCQueue::addOperation returned void and silently dropped the operation when
the queue was closed or finishing. Every caller assumed success: the three
schedulers left their running flag set with nothing in flight (which then blocks
every later round), and z_sendmany / z_shieldcoinbase / z_mergetoaddress returned
an opid for work that would never run -- z_shieldcoinbase and z_mergetoaddress
having already locked their selected coins in the constructor. It now returns
bool; the schedulers release the flag and log, and the three RPCs raise an error
instead of handing back an opid. Coin locks are memory-only, so a refusal at
shutdown reclaims them with the process; the lie about success was the defect.
Nothing ever removed finished automated operations from the queue's map.
popOperationForId is reached only from z_getoperationresult, so on a node running
autoshield every 25 blocks the map grew by one entry per round forever. The
schedulers now pop the operation they just cancelled. The worker already handles
a missing id ("cannot find operation in map, may have been removed",
asyncrpcqueue.cpp), and it releases lock_ before calling main(), so popping under
cs_wallet introduces no lock cycle.
The autoshield operation built its transaction against targetHeight_, the
enqueue-time height, while SetExpiryHeight and the network-upgrade straddle guard
both used tipHeight. Since the builder's height selects the consensus branch id,
the guard was checking a height the transaction was not signed against -- it could
not prevent the failure it exists to prevent. Now tipHeight throughout.
cancel() in the sweep, consolidation and autoshield operations set CANCELLED
unconditionally, dropping the base class's guard entirely. The schedulers cancel
the previous operation when they enqueue the next, so a round that had already
SUCCEEDED got its result relabelled as cancelled. Restored a narrower guard: still
cancellable while READY or EXECUTING (the base class refuses the latter, which
would defeat cancellation here), but a terminal state is left alone.
CommitAutomatedTx dumped the whole transaction to stderr on every commit,
duplicating the LogPrintf that CommitTransaction does one call later. ToString()
emits a line per input, so with the 400-input autoshield cap that was tens of KB
of stderr per round. Removed.
Also corrected a comment that credited the immature-coinbase exclusion to
fOnlySpendable (the argument is fOnlyConfirmed; the exclusion is unconditional in
AvailableCoins), and noted that AUTOSHIELD_CTXIN_P2SH_SIZE is a byte size that
merely happens to share the value 400 with the input cap.
Verified on an isolated regtest chain, 8/8: nine consecutive autoshield rounds
succeed after the builder-height change; the operation map stays at 1 entry across
all nine (it grew one per round before); stderr totals 1608 bytes for the whole
run with zero CommitAutomatedTx dumps, while debug.log still records all nine
commits via CommitTransaction; no round is relabelled cancelled; funds shield
correctly and no coin locks leak.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pwalletMain->autoShieldAddress was only ever written by a round running in the
current process, so on any node without an explicit -autoshieldaddress,
z_autoshieldstatus reported an empty destination from startup until the first
round fired. disabled_reason was empty on that path too (rpcwallet.cpp), so the
RPC showed autoshield true, running false, no address and no explanation -- the
exact silent state z_autoshieldstatus was added to eliminate. On a restored
wallet, which pre-derives the whole -mnemonicsaplinggap window and therefore
always has an in-gap account to pick, the answer was known at startup and simply
not computed.
Split the read-only half of resolveDestination into a free function shared with
init: the configured override if set, else the lowest in-gap account
m/32'/coin'/i' the wallet already holds. init calls it once when autoshield is
enabled and no explicit address was given.
It deliberately does not generate a key. Deriving a fresh sapling account as a
side effect of populating a status field would mutate the wallet to make an RPC
prettier, so step 3 of resolveDestination -- the generation path, which must stay
inside the operation where an unlocked wallet is already established -- is left
where it was. A brand-new wallet holds nothing in-gap, so the field stays empty
there and disabled_reason now says why instead of being blank.
Behaviour is otherwise unchanged: same derivation, same lowest-index-wins rule,
same refusal to trust CKeyMetadata, same caching for the life of the process.
Verified on an isolated regtest chain, 12/12:
fresh wallet -> address "", reason "no destination resolved yet; one will
be derived from the HD seed on the first round"
after one round -> address set, reason empty
after RESTART -> address visible with NO block mined since (height 12 both
sides), and z_listaddresses still holds exactly 1 address,
so init derived nothing
-autoshieldaddress-> still overrides the derived destination, and the round
shields into it
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A successful-but-incomplete sweep round deliberately returns with fSweepRunning
still set and nextSweep unadvanced, as a "keep draining next block" baton. Every
early return in RunSaplingSweep, though, leaves that baton set without
re-dispatching -- and RunSaplingConsolidation, which is gated on fSweepRunning,
then returns without advancing nextConsolidation. So the "consolidation is
within 5 blocks" blackout at the top of RunSaplingSweep never lifts: sweep waits
on consolidation, consolidation waits on sweep, and neither runs again.
That much is pre-existing. What is new is that autoshield now shares the gate --
RunAutoShieldCoinbase returns early on fSweepRunning || fConsolidationRunning --
so a wedged sweep silently disables coinbase shielding too, with
z_autoshieldstatus reporting autoshield true, running false, and no reason.
Only honour the baton while a sweep operation is genuinely in flight: if the
operation for saplingSweepOperationId is absent or has reached a terminal state,
drop the stale flag and let the checks below decide afresh. The drain model is
unchanged -- nextSweep is still unadvanced, so the next block re-dispatches.
Also report the deferral in z_autoshieldstatus, so mutual exclusion with sweep
or consolidation reads as a deferral rather than an unexplained idle.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Auto-shielding could silently decline to run with no way to ask why. The
destination it resolves was equally invisible: the only evidence was a LogPrintf
emitted once per round, so an operator wanting to know where their mined coinbase
was going had to grep debug.log.
z_autoshieldstatus reports the enable state, whether a round is in flight, the
next height, interval, fee, minimum utxos, and the resolved destination -- plus
the HD seed provenance in both numeric and readable form, whether the seed is
phrase-recoverable, and a disabled_reason explaining why it is off when it is.
That last field is the point. "autoshield": false on its own does not distinguish
an operator who passed -autoshield=0 from a wallet whose seed provenance is not
known-recoverable, and those need different responses.
Mirrors z_sweepstatus in shape and registration.
Verified on all three branches:
fresh wallet -> autoshield true, origin 1 "created on an empty wallet",
seed_recoverable true, disabled_reason ""
-autoshield=0 -> disabled_reason "disabled by -autoshield=0"
upgraded wallet -> autoshield false, origin 4 "predates provenance recording",
seed_recoverable false, disabled_reason "HD seed origin is
not known-recoverable; back the seed up and pass -autoshield=1"
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An adversarial re-review found the header-accept RandomX check (b9fdc7981 +
7e9b2c661 header-PoW + d2124a303 defer) to be a persistent source of
consensus-liveness bugs: it derives the RandomX key from the ACTIVE chain
(hush_chainactive), the wrong branch for reorg/side-branch/catch-up headers, so
it repeatedly false-rejected validly-mined headers and DoS(100)-hard-banned
honest peers (IBD-tail catch-up and deep-reorg cases); the defer fix and an
extend-tip fix each addressed one case while leaving/creating others (an
extend-tip variant re-opened an unbounded post-IBD side-branch flood). It only
mitigated a low-harm resource DoS -- forged headers bloat mapBlockIndex memory/
disk but are never SELECTED (nMinimumChainWork) and the full RandomX + target
check still runs at block-connect. Revert to fCheckPOW=0 at header-accept
(original behavior). A comment in AcceptBlockHeader records that any re-attempt
must derive the key from the header's OWN ancestry (pindexPrev->GetAncestor),
never the active chain.
Also hardens two issues the same review found:
- #8 IBD header cap now bounds against the VALIDATED chainActive.Height()
(attacker-hard) instead of pindexBestHeader, which a forward-extending flood
advanced in lockstep, defeating the cap.
- opreturn_burn only emits a change output above the dust threshold; a sub-dust
change made the returned tx non-standard/unrelayable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#10 (HIGH) opreturn_burn selected UTXOs for nAmount+txfee but pushed only the
burn vout and returned - so the entire selected-input surplus was silently paid
as miner fee (e.g. a 500-coin UTXO burning 10 lost ~490). Push a change output
for (inputs - nAmount - txfee). Also widen the int32_t txfee (which truncated
large CAmount fees) to CAmount and MoneyRange-validate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Sapling note-selection loop stopped once total_value >= nTotalOut, ignoring
the miner fee, so a wallet with notes covering the amount but not amount+fee
selected too few notes and failed later with a spurious "insufficient funds".
Reserve the fee (default or user-supplied) in the selection target.
Leto eb4fc52273.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
hush_nSPV_fullnode.h: bound the REMOTERPC method strcpy and json memcpy to their
fixed buffers (method[64], json[11000]); add lower-length and memcpy-source bounds
to the UTXOS/TXIDS coinaddr[64] copies and the MEMPOOL handler. These paths
deserialize attacker-controlled request bytes -> stack overflow / OOB read. The
nSPV server is opt-in via -nspv_msg (off by default; DragonX uses lightwalletd).
rpc/blockchain.cpp: getchaintxstats null-checks pwalletMain (crash under -disablewallet).
wallet/rpcwallet.cpp: z_sendmany initializes nFee to the default miners fee (was read
uninitialized when no fee param supplied).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Derive transparent (t-addr) keys from the HD seed and add BIP39 mnemonic seed
phrases that are byte-for-byte compatible with SilentDragonXLite, so the same
24 words recover the same shielded and transparent addresses in either wallet.
HD transparent keys:
- Derive t-keys from the seed at m/44'/coin'/0'/0/i (were random CKeys).
- CHDChain gains a version-gated transparent counter; existing wallets load
unchanged. GenerateNewKey routes through DeriveNewChildKey when enabled
(-hdtransparent, default on).
- Restore from a seed hex via -hdseed with gap-limit pre-derivation; birthday
pinned to genesis so the rescan is not clipped.
BIP39 seed phrases:
- Wire the vendored trezor BIP39 lib (src/crypto/bip39) into the build, fix its
BIP39_WORDS guard, and disable the insecure mnemonic cache.
- Match SDXLite exactly: English wordlist, empty passphrase, PBKDF2 64-byte
seed, coin type 141, ZIP-32 m/32'/141'/i' and BIP44 m/44'/141'/0'/0/i. Store
the 32-byte entropy and expand to the 64-byte seed on demand.
- Restore via -mnemonic, create via -usemnemonic, reveal via z_exportmnemonic.
Verified by gtests including a known-answer BIP39 seed vector and z/t address
derivation checks (src/gtest/test_hdtransparent.cpp, test_mnemonic_compat.cpp).
Docs in doc/hd-transparent-keys.md and doc/seed-phrase.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
DanSandClaude Opus 5.5