fd67a1a15af87c68b78dd98ed0644c9ce40d6582
2549
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  DanSandClaude Opus 5.5
|
fd67a1a15a |
wallet: don't abort building witnesses for viewing-key-only notes
A note found with only an incoming viewing key (z_importviewingkey) never gets a nullifier: UpdateSaplingNullifierNoteMapWithTx needs the full viewing key. VerifyAndSetInitialWitness built a witness for such a note anyway and then dereferenced *item.second.nullifier, aborting the daemon on the boost::optional assertion. Importing a viewing key that had received funds therefore crashed the node during the import's rescan, and again at every start and on every block, since witnesses are rebuilt then too. Reproduced on regtest (z_importviewingkey -> ScanForWalletTransactions -> BuildWitnessCache -> VerifyAndSetInitialWitness). Skip notes that have no nullifier and no full viewing key: they cannot be spent and need no witness. Only call SaplingWitnessMinimumHeight when the nullifier is set, and log the corrupt-data case where a note with a full viewing key gets no witness, which used to abort as well. A note whose spending key is imported later is picked up on the next rebuild. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
7f64df008e |
rpc: answer z_getoperationstatus without cs_main and cs_wallet
z_getoperationstatus took LOCK2(cs_main, cs_wallet) for a pure read of the async operation queue, so a GUI polling it every few seconds queued behind every wallet scan and block connect. It also sorted UniValues with a by-value comparator. - z_getoperationstatus_IMPL takes no wallet locks. z_getoperationresult takes LOCK2 itself, so the removing path stays serialised with the wallet's swap of automated operations (cancel then remove, under cs_wallet). - Sort (creation_time, index) pairs comparing the time only. libstdc++'s sort depends only on the comparator results, so the order, ties included, is the same as before. - AsyncRPCQueue::popOperationForId finds and erases under one lock. - z_listoperationids takes no wallet locks; the status-name map is const and read with at(). Output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
c32c85b7f0 |
wallet: count the entries listtransactions skips instead of building them
listtransactions formatted every entry before "from" and then threw it away. Paging through the history that way is quadratic: ObsidianDragon pages up to 5000 rows at a time, so a 30k-row mining wallet built about 105k entries per full refresh (30k with this change). Skipped entries are now counted after the same account and watch-only checks as before and never formatted. Output is unchanged for every valid input. When count + from overflows (the build uses -fwrapv), the result is [] instead of an out-of-bounds read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
1541d045ee |
wallet: cut per-entry copies in listtransactions and gettransaction
- 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> |
||
|
DanSandClaude Opus 5.5
|
23f3e26d2f |
wallet: stop zsTxSpendsToJSON inserting into the nullifier and note maps
zsTxSpendsToJSON looked spends up with operator[]: every foreign nullifier in a listed tx inserted a null outpoint into mapSaplingNullifiersToNotes, and the note-data lookup could insert an empty SaplingNoteData. A read-only RPC was mutating wallet state. z_viewtransaction then found those null outpoints and failed with RPC -1 "map::at" for every tx with a foreign Sapling spend (most incoming z->z payments) once z_listsentbyaddress or getalldata had run, until restart. ObsidianDragon treats that error as transient and retried those txs on every refresh. Use find() with the same test as IsSaplingNullifierFromMe(). This call produces the same rows in every case; z_viewtransaction now answers as it does on a freshly started daemon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
08afe5bb3b |
wallet: read wallet txs and note data by reference in more RPCs
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> |
||
|
DanSandClaude Opus 5.5
|
375b995b38 |
wallet: stop z_listsentbyaddress re-deriving keys and copying the wallet
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> |
||
|
DanSandClaude Opus 5.5
|
106fc1ceac |
wallet: filter notes before decrypting them in GetFilteredNotes
GetFilteredNotes copied every CWalletTx and every SaplingNoteData (witness list included) by value, and trial-decrypted every note before it applied the address, spent and lock filters. It sits under z_listunspent, z_gettotalbalance, z_getbalance, z_listreceivedbyaddress, z_mergetoaddress and sweep/consolidation, all with cs_main and cs_wallet held, so on a wallet with thousands of notes it is most of the time those RPCs block the block-connect thread. - Read wallet txs and note data by const reference, and skip txs with no Sapling notes before the finality, maturity and depth checks. - Check IsSaplingSpent before decrypting, so spent notes are not decrypted when ignoreSpent is set. - Address pre-filter: when 1-16 filter addresses all have a recorded ivk that is canonical (below 2^251) and actually generates that address, a note whose canonical ivk is none of them cannot pay a filter address and is skipped without decrypting. z_importviewingkey stores (ivk, address) pairs unchecked, hence the verification; an aliased ivk + r_J would pass it, hence the canonical check. Any other case takes the old path. - SaplingNotePlaintext::decrypt_with_pk_d returns the pk_d that decryption already computes, so the note's address and SaplingNote are built without two more scalar multiplications. decrypt() is now a wrapper around it. RPC output is unchanged. The one behaviour difference: a spent note that cannot be decrypted no longer trips assert(maybe_pt) when ignoreSpent is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
c1040028e4 |
wallet: checkpoint rescan progress when the scan is interrupted
An aborted or shutdown-interrupted rescan discarded all of its progress: nothing
advanced the on-disk best-block locator, so the next run started over. On a large
shielded wallet that is a multi-minute witness rebuild repeated from scratch.
Writes the locator ONCE, at the interrupt, rather than periodically through the
scan. That delivers the whole stated benefit -- "resume where it stopped" -- at
the cost of exactly one SetBestChain per interrupted scan. A periodic checkpoint
would additionally survive SIGKILL and power loss, a much weaker requirement, and
it is the part that carries all the cost: SetBestChainINTERNAL is O(whole wallet)
regardless of progress, so on the reported 5.3k-tx / 7.5k-note wallet a checkpoint
every 2500 blocks would plausibly cost more than the rebuild it was meant to save.
If someone later demonstrates a need for crash resilience mid-scan, add it then,
with a measured interval and fFlush=false -- not before.
Two guards, both necessary:
- CONTIGUITY. The RPC entry points (rescan / importprivkey / z_importkey /
z_importviewingkey) take a caller-supplied start height validated only against
chainActive.Height(), never against the wallet's own persisted locator. A scan
beginning above that locator must not checkpoint at all, or it would record
the skipped range as scanned and hide any funds in it. Logged once when it
applies, so an operator can see why an interrupt did not persist.
- MONOTONICITY. A checkpoint may only advance the locator, never move it back.
The locator points at the last FULLY PROCESSED block -- the parent of the block we
were about to scan -- so resume restarts one block early. CChain::GetLocator pushes
its argument first and FindForkInGlobalIndex returns that same block, so resume
begins AT it; the one-block overlap is deliberate and idempotent, since AddToWallet
only takes its merge path when the transaction is already present.
No witness work is done at the interrupt, which answers the two "//TODO: should we
update witness caches?" comments this replaces: witnesses are re-derived from each
note's own witnessHeight independently of the locator, and witnessRootValidated is
never serialized, so every note is revalidated against hashFinalSaplingRoot on the
next start. A mid-scan checkpoint does capture notes at mixed witnessHeights -- the
in-loop BuildWitnessCache(pindex, true) returns before the extension phase and only
seeds new notes at their own transaction's height -- but that state is recoverable:
the post-loop BuildWitnessCache(tip, false) levels every note, and it now occurs at
most once per scan instead of hundreds of times.
Uses SetBestChainNoFlush so the checkpoint does not trigger a full BDB
txn_checkpoint over the whole cache, and reports failure rather than silently
leaving the operator to discover the replay.
Depends on the fLastRescanCompleted gate in the previous commit: without it
init.cpp would overwrite this checkpoint with a tip locator immediately after the
scan returns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 5
|
a6b6f80db0 |
wallet: stop an interrupted rescan from hiding funds, and un-latch fAbortRescan
Two independent fund-visibility bugs on the rescan path, both pre-existing.
1. AN INTERRUPTED RESCAN RECORDED ITSELF AS COMPLETE.
ScanForWalletTransactions returns a bare `int ret` (a found-tx count) on both its
abort and shutdown bail-outs, so the caller could not tell "finished" from
"stopped at block H". init.cpp then ran, unconditionally:
pwalletMain->ScanForWalletTransactions(pindexRescan, true);
pwalletMain->SetBestChain(chainActive.GetLocator()); // TIP locator
An interrupted scan therefore stamped the wallet as scanned all the way to the
chain tip. On the next start, the `chainActive.Tip() != pindexRescan` guard just
above sees no work to do and skips the rescan entirely, so every transaction in
the never-scanned range stays out of mapWallet -- permanently invisible to
getbalance and unspendable. That is exactly the case `-rescan` exists for: a key
import, right after ClearNoteWitnessCache has run.
Adds CWallet::fLastRescanCompleted, set false at scan entry and true only on the
normal exit, and gates the init.cpp locator write on it. Deliberately NOT
overloading the int return, which callers already use as a tx count.
2. fAbortRescan WAS NEVER RESET.
wallet.h declares it, AbortRescan() sets it true, and nothing in src/ ever sets
it false. The abortrescan RPC is live. After one call, for the remaining lifetime
of the process:
- every ScanForWalletTransactions returns immediately, so re-running an import
silently no-ops while the RPC still reports success;
- BuildWitnessCache bails on the same flag on EVERY call, including the routine
per-block extension from ChainTip.
Witness heights then diverge across notes while the chain advances, and
GetSaplingNoteWitnesses elects the majority root as the anchor and returns
boost::none for every note that disagrees. Those notes still show in the balance
but cannot be spent. Cleared at scan entry and consumed in the abort branch.
Also in BuildWitnessCache's abort branch: stop clearing fRescanning. A witness
rebuild is not a rescan and must not touch a flag ScanForWalletTransactions owns.
3. Both bail-out log lines had two format specifiers and one argument:
LogPrintf("%s: Rescan aborted at block %d\n", pwalletMain->rescanHeight);
tinyformat's "too many conversion specifiers" guard is disabled in this tree
(tinyformat.h: `if(*fmt != '\0' && 0 ) // disabled due to complaints`), so this
does not throw -- verified by compiling the exact call against this tree's
tinyformat.h. It emits "3841207: Rescan aborted at block " with the height in the
__func__ slot, the real height dropped, and the trailing newline swallowed so the
next log line concatenates onto it. On the one path an operator has to diagnose
an interrupted rescan, the only record was corrupt. Fixed at both sites, and the
state updates moved above the log call.
4. Makes SetBestChainINTERNAL report whether the atomic write committed, so a
checkpointing caller can tell (six failure paths returned void). Adds
SetBestChainNoFlush, which opens the wallet DB with fFlushOnClose=false: the
default ctor makes ~CDB run a full BDB txn_checkpoint over the whole cache, which
is fine hourly but not from inside a scan -- the same flush wallet.cpp already
documents avoiding elsewhere "for performance reasons". Nothing calls it yet.
5. SetBestChainINTERNAL took each CWalletTx by value, deep-copying every note's
witness deque (WITNESS_CACHE_SIZE entries) per transaction per call, purely to
serialize it. Now by const reference.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 5
|
5634aed750 |
wallet: open salvaged wallets in a degraded mode, and detect a mismatched hdchain
A wallet.dat repaired by v1.0.3-or-earlier -salvagewallet could not be opened by
this build at all. Those builds' IsKeyType has no "hdchain" case, so salvage
dropped the record; LoadWallet then returned DB_CORRUPT and told the user to
restore from a seed phrase. There is no such recovery path: the node aborts
before the RPC server exists, -usemnemonic defaulted to 0 in v1.0.3 so many of
these wallets never had a phrase, and SetHDSeedFromMnemonic refuses a non-empty
wallet, so "move wallet.dat aside" discards every non-HD key salvage preserved.
The guard itself was right to refuse to DERIVE -- a missing hdchain falls back to
SetNull defaults, clearing fMnemonicSeed and switching derivation from the
64-byte BIP39 seed to the raw 32-byte entropy, an entirely different key tree.
It was wrong to refuse to OPEN. Split the two:
- "hdchain" record ABSENT (the salvage signature, tracked by a new
wss.fHDChainSeen set before any parse attempt) -> open DEGRADED.
- "hdchain" record PRESENT but unreadable -> still DB_CORRUPT; that signals
wider file damage.
Degraded means the wallet spends, receives and rescans normally but refuses to
derive any new HD key. Gated at the single choke point GetHDSeedForDerivation
rather than at the two generators, so z_getnewaddress/sendmany/shieldcoinbase
raise a clean error and autoshield skips. hdChain.seedFp is deliberately left
null, which keeps IsHDTransparentEnabled false so getnewaddress falls back to the
legacy random-key path -- gating it instead would break TopUpKeyPool, change
addresses and block templates. -rescan is soft-set because an old salvage also
dropped defaultkey and bestblock, which would otherwise look like a first run and
skip the rescan, leaving a permanently zero balance.
NEW DETECTOR, covering strictly more damage than the guard beside it. A wallet
salvaged by v1.0.3 and then USED on v1.0.3 persists a SetNull-derived hdchain
carrying seedFp=null. On this build fHDChainRead is then true, the guard never
fires, the node starts perfectly clean -- and derives into the wrong key tree
forever, silently. That state is unforgeable by any legitimate writer, since
InstallHDSeed always stores seedFp = seed.Fingerprint(), so compare the loaded
chain's seedFp against the fingerprint taken from the hdseed/chdseed record KEY
(which works while an encrypted wallet is locked) and degrade on mismatch.
Also:
- init.cpp: abort in the DB_CORRUPT branch itself, as DB_NEED_REWRITE already
does. Falling through ran several hundred more lines against a wallet just
declared corrupt, including SetHDSeedOrigin(), which WRITES to it. Error text
no longer advises -mnemonic (wrong twice over, see above).
- rpcdump.cpp: z_exportwallet discarded GetHDSeedForDerivation's return and
emitted the line regardless, so a failure wrote a blank seed next to a
legitimate-looking BLAKE2b-of-empty fingerprint -- a backup that looks valid
and restores nothing.
- Corrected the guard's comment: the saplingAccountCounter half was overstated.
Both generators loop while(Have...Key(...)), so a low counter walks forward
past existing accounts rather than colliding with them.
qa/r4-salvaged-wallet-harness.sh builds the victim wallets with the on-disk
v1.0.1 release binary and asserts the behaviour end to end. It REFUSES to run
outside a network namespace and re-checks getconnectioncount==0, because those
old binaries predate the regtest seed-injection fix and would otherwise dial the
live network. 11/11 pass:
C healthy wallet opens and is NOT flagged (no false positives)
A salvaged wallet opens, flagged, persists no hdchain, z_getnewaddress refused,
getnewaddress still works, z_exportwallet emits no bogus HDSeed line
B poisoned wallet opens and the seedFp detector fires
Note the harness asserts "no hdchain synthesised", NOT "wallet.dat unchanged":
a control run showed normal startup rewrites wallet.dat for healthy wallets too.
This is the access/detection half. Proving fMnemonicSeed by re-deriving against
held keys, and repairing the record, is deliberately left out: it requires a
write, and the affected population is unmeasured. dev's own salvage already
preserves hdchain, so the class is closed going forward.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 4.8
|
e55c67b0eb |
wallet: extract ParseSendManyRecipients from z_sendmany
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> |
||
|
DanSandClaude Opus 4.8
|
4a2e531cb6 |
wallet: extract SelectAnyZaddrSource from z_sendmany
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>
|
||
|
DanSandClaude Opus 4.8
|
81f803948c |
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>
|
||
|
DanSandClaude Opus 4.8
|
817c6b2d0e |
hygiene: Phase 5 — correct stale comments + name safe magic numbers
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> |
||
|
DanSandClaude Opus 4.8
|
cf15b0a399 |
hygiene: Phase 4 — route unconditional debug output through LogPrint
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>
|
||
|
DanSandClaude Opus 4.8
|
0de30bbbd1 |
hygiene: Phase 3 follow-up — sweep commented-out debug cruft
Remove ~116 commented-out debug/dead-code lines the audit flagged as noise (findings F6/F7/F15/F22): the Komodo "%s tikN" step-tracer comments and other commented-out fprintf/printf/LogPrintf/std::cerr calls threaded through AppInit2 (init.cpp), CreateNewBlock and the miner loops (miner.cpp), plus a few commented-out dead-code fragments (sendmany SetLockTime/nLockTime, the CCtx CC_vinselect random-pick block and AddNormalinputsLocal remote-mypk redirect, miner adaptive-PoW assignments). Comments only — no compiled behavior changes; the tree builds clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
b7060c7de0 |
hygiene: Phase 3 — dead-code excision (~7,100 lines)
Third phase of the code-hygiene remediation: remove provably-dead code.
Each in-file deletion was adversarially verified (a per-finding investigator
plus a skeptic that greps the tree to refute deletability); the sanity grep
confirms zero dangling references, and the tree builds and self-mines clean.
Removed wholesale:
- src/cc/dapps/ (9 files, ~6,939 lines): a git-tracked but never-built Komodo
DEX / z-migration tool (hushdex.c, zmigrate.c, cJSON.c, ...). Nothing in
Makefile.am/configure.ac references it.
In-file dead code:
- pow.cpp: the `#ifdef original_algo` oldRT_CST_RST function (the macro is
never defined in source or build flags) and two `if ( 0 )` debug blocks.
- walletdb.cpp: the "orphaned staking transaction" cleanup path (deadTxns) —
DragonX is RandomX PoW with no staking, so the guard is always false and the
block never runs; also drop the now-unused static/extern decls.
- cc/eval.h: the ProcessCC / Eval::ImportCoin / ImportPayout / DisputePayout
declarations that have no definition anywhere in the tree.
- rpc/crosschain.cpp: the crosschainproof stub RPC (returned {} unconditionally)
and its registrations in rpc/server.cpp, rpc/server.h, rpc/client.cpp.
- coins.cpp: the commented-out `//TODO: delete` Sprout PushAnchor template.
- wallet.cpp: the permanently-zero KMD `interest2` term in CreateTransaction.
- rpc/net.cpp: hush_longestchain's always-zero `n` var (num > (n>>1) => num>0)
and its `if ( 0 )` debug branch.
- hush_nSPV.h / hush_nSPV_fullnode.h: three `if ( 0 && ... )` dead debug branches.
- net.cpp, crypter.h, saplingconsolidation.cpp (dup set_error_code),
shieldcoinbase.cpp (`donation < 0` on a uint8_t), cclib.cpp (unused
FAUCET2SIZE): single-line dead-code fixes.
Deliberately NOT touched (verification refuted the audit's "dead on DragonX"
premise): the ~2,250-line HUSH3 checkpoint block, the miner notary/timelock
paths, and hush_gateway.h — all reachable at runtime via -ac_name / -ac_*
args (the inherited Komodo assetchain model), and hush_gateway's
hush_opretvalidate is called from ConnectBlock (consensus). Dropping those
requires a deliberate decision to remove HAC/HUSH3 mode, tracked separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 4.8
|
aff6101987 |
hygiene: Phase 2 — DragonX-authored cleanup (13 findings)
Second phase of the code-hygiene remediation, covering the items the
DragonX team introduced or the rebrand missed. No consensus behavior
changes; validated on a local self-mining node.
Log cruft:
- chainparams: route the startup ">>>>>>>>>>" banner and the port line
through LogPrintf/LogPrint("net") instead of fprintf(stderr).
- pow: drop the fprintf(stderr) hash-mismatch dump that duplicated the
LogPrintf copy verbatim.
- miner: gate the RandomXDatasetManager per-alloc/per-VM address dumps and
MemDiag /proc reads behind LogPrint("randomx"); keep a one-line
"allocated shared dataset (N GB)" summary at default verbosity.
Named constants (single source of truth in wallet.h):
- DEFAULT_AUTOSHIELD_FEE/INTERVAL, MIN_AUTOSHIELD_INTERVAL,
DEFAULT_AUTOSHIELD_MIN_UTXOS, AUTOSHIELD_MIN/MAX_FEE for the autoshield
option parsing and help text (were bare 10000/25/5 literals repeated
across init.cpp and wallet.h).
- AUTO_OP_TARGET_HEIGHT_OFFSET replaces the three copy-pasted
`blockHeight + 5` async-op scheduling offsets.
- AUTO_OP_EXPIRY_DELTA replaces the three per-file *_EXPIRY_DELTA=15
constants (sweep/consolidation/autoshield) with one shared value.
- Move DEFAULT_AUTOSHIELD_FEE out of the op header into wallet.h so it no
longer collides when both headers are included.
Error surfacing:
- init: report clamped/out-of-range -autoshieldinterval/-autoshieldfee via
InitWarning() (surfaces to GUI/log) instead of fprintf(stderr).
Rebrand / dead foreign-chain code (approved removals):
- server: drop the HUSH3 special-cases in stop() and HelpExampleCli; the
cli example now shows "dragonx-cli" instead of "hush-cli".
- getinfo (misc): remove the stale dPoW notarization block (notarized,
prevMoMheight, notarizedhash, notarizedtxid, notarizedtxid_height,
HUSHnotarized_height, notarized_confirms) — DragonX is a private chain
from genesis with no active dPoW. Also fixes the hardcoded "HUSH3" that
made getinfo query a foreign chain's notarization.
- delete the dead Komodo notary RPCs getera/getdragonjson/
getnotarysendmany/geterablockheights (getera returned 0;
getnotarysendmany was marked "this is broke") and their registrations.
- remove the dead ASSETCHAINS_EQUIHASH reporting branches in getinfo and
getmininginfo — DragonX is RandomX-only.
chainparams: document why the upstream Equihash params and the literal
Bitcoin genesis are retained under RandomX (do not "fix" them).
Deferred: the hdSeedOrigin int->string switch in z_autoshieldstatus
(no shared helper exists to reuse; low value).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 4.8
|
798eccc624 |
hygiene: fix seven latent defects from the v1.2.0 code audit
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>
|
||
|
DanSandClaude Opus 4.8
|
02b4d03fc6 |
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 (
|
||
|
DanSandClaude Opus 5
|
65130c3120 |
async/wallet: close the loose ends around automated operations
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>
|
||
|
DanSandClaude Opus 5
|
a0ccb4be1d |
wallet: repair a truncated hdchain record instead of refusing the wallet
A build predating VERSION_HD_TRANSPARENT rewrites the hdchain record with only its four base fields while leaving nVersion at whatever it read. Our version-gated reads then run off the end of the stream, ReadKeyValue turns the throw into fHDChainRead=false, and LoadWallet escalates that to DB_CORRUPT. Verified against the real v1.0.2-2b011d6ee release binary: a dev wallet, opened once by v1.0.2 and given a single new sapling address, came back to Error reading wallet database: hdchain record is corrupt Error loading wallet.dat: HD seed present but the hdchain record is missing or corrupt Error loading wallet.dat: Wallet corrupted Nothing is actually lost there -- the same test showed the seed phrase restoring the full balance, the autoshield destination, and even the address v1.0.2 had generated -- but the user is shown "Wallet corrupted" with no hint of that. Recover instead, for a v1 or v2 record. Everything derivation depends on is either in the four-field prefix or in the separate hdseed record, and the trailing counters are self-healing: DeriveNewChildKey and GenerateNewSaplingZKey both skip indices whose key the wallet already holds, so restarting a counter at 0 re-walks past existing keys rather than reissuing them. Read the prefix from an untouched copy of the stream, default the missing tail, log it, and rewrite the record in full form so the next load is clean. A record claiming nVersion >= VERSION_HD_MNEMONIC still fails loud: that flag selects the derivation input, so guessing it wrong yields a different key tree in silence. No wallet this code has written can be in that state -- every InstallHDSeed call site passes fMnemonic=false -- so the branch is defensive only. Also say what to do about it. Both the log line and the init error now name the remedy (move wallet.dat aside, restart with -mnemonic and -rescan) rather than stopping at "Wallet corrupted". Verified on regtest against the real v1.0.2 binary, 14/14: the round-trip that previously ended in "Wallet corrupted" now loads, logs the repair and the rewrite, keeps the balance, the autoshield destination and v1.0.2's own address, still issues distinct fresh t-addresses after the counter reset, needs no repair on the second load, and remains fully recoverable from the seed phrase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
358011bd54 |
wallet: resolve the autoshield destination at startup
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>
|
||
|
DanSandClaude Opus 5
|
7a62fc4877 |
wallet: break the sweep/consolidation/autoshield deadlock
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> |
||
|
DanSandClaude Opus 5
|
9a8f17b2c8 |
wallet: bound autoshield rounds and lock their inputs
Two defects in a single autoshield round, both of the quiet kind.
The size estimate reserved a flat 2000 bytes for "header + sietch outputs", but
every autoshield tx carries three Sapling OutputDescriptions -- the change note
plus the two Sietch dummies -- and the real fixed cost is ~2937 bytes. Measured
across seven live mainnet coinbase shields: 113.5 bytes per input and 2936.6
+/- 0.8 bytes fixed, of which 3 * 948 = 2844 is the output descriptions. The
estimate was therefore short by ~937 bytes before a single input was counted.
Inputs are charged AUTOSHIELD_CTXIN_DUST_SIZE = 148, which is conservative for
the default P2PK coinbase but exact for P2PKH, so with a P2PKH coinbase a
backlog of 1332..1337 utxos passed the estimate and built a tx over
MAX_TX_SIZE_AFTER_SAPLING. CommitTransaction calls AddToWallet before
AcceptToMemoryPool, so a rejected oversize tx leaves its inputs reading as spent.
z_shieldcoinbase caps a manual shield at SHIELD_COINBASE_DEFAULT_LIMIT = 50
utxos; autoshield dropped that cap and relied on the byte estimate alone.
Restore one -- AUTOSHIELD_MAX_INPUTS = 400 -- so the byte arithmetic is no longer
the only thing between a large backlog and an oversize transaction. The
remainder is shielded on the next round.
Second, the proof build deliberately runs without cs_wallet so wallet RPCs are
not stalled, which leaves a multi-second window in which a concurrent
z_shieldcoinbase or z_sendmany can re-select the same coinbase outputs.
AvailableCoins already honours IsLockedCoin and z_shieldcoinbase already
brackets its selection with LockCoin/UnlockCoin; autoshield made zero LockCoin
calls. Take the locks under cs_wallet at selection time and release them via
RAII, since several early returns sit between selection and commit and a leaked
lock would exclude those coins from every future round.
Verified on an isolated regtest chain with a 540-utxo backlog:
round 1 logged "reached per-round input cap (400)" and committed exactly 400
inputs in a 48351-byte tx (estimate 62300, limit 200000)
round 2 took the remaining 151; backlog drained 540 -> 0
listlockunspent showed 400 coins locked mid-round and 0 afterwards
48351 bytes for 400 inputs implies 2937 bytes of fixed overhead, agreeing
with the mainnet measurement to 14 bytes
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 5
|
1ec6590fb7 |
wallet: add z_autoshieldstatus
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>
|
||
|
DanSandClaude Opus 5
|
92e6c7008d |
wallet: define CHDChain's static constants out of line
hush-gtest failed to link with "undefined reference to CHDChain::VERSION_HD_MNEMONIC". The version constants are static const int with in-class initialisers and no definition anywhere, so any ODR use needs one -- and gtest's EXPECT_*/ASSERT_* macros take their arguments by const reference, which is exactly that. test_mnemonic_compat.cpp:161 passes VERSION_HD_MNEMONIC to EXPECT_LT. dragonxd links either way, because nothing in the daemon binds these to a reference; only the test target exposed it, and the test target was never built on the branch that introduced the test. Define all four rather than only the one that failed: VERSION_HD_BASE, VERSION_HD_TRANSPARENT and CURRENT_VERSION carry the identical latent fault, and the next EXPECT_EQ against any of them would hit the same wall. Fixing the test instead would have hidden the problem rather than removed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
9734402d7b |
wallet: create new wallets from a BIP39 seed phrase by default
New wallets now get an exportable 24-word phrase instead of a random seed that
no phrase can ever reproduce. Only wallets with no seed yet are affected;
GenerateNewSeed is reachable from one place, under !HaveHDSeed(), and all three
key stores refuse to replace an existing seed.
This is safe to default on now that the storage form is backwards compatible: the
expanded 64-byte BIP39 seed is what gets stored, so a binary predating any of
this reads it and derives the same keys.
Verified on an isolated chain before flipping:
- a new-format wallet reopened with the tagged v1.1.0 binary, which has no
knowledge of the entropy record, listed identical addresses;
- restoring only the 24 words into a fresh datadir recovered every address.
Note this changes what a new wallet is, not what an existing one is: the same
entropy yields a different key tree depending on which side of this commit
created the wallet. Nothing migrates, and nothing needs to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 5
|
20b2cbe830 |
wallet: store the expanded BIP39 seed for mnemonic wallets
Mnemonic wallets stored the 32-byte BIP39 entropy as the HD seed and relied on CHDChain.fMnemonicSeed to tell the deriver to expand it first. That flag is version-gated in the CHDChain serialisation, so a binary that predates it reads the record, never consumes the trailing byte, and derives from the raw entropy -- a different key tree, silently, with no error. Store the expanded 64-byte BIP39 seed instead, with fMnemonic = false, and keep the entropy in its own display-only record. Derivation then reads the stored bytes directly on every binary, old or new, so key trees are identical and no CHDChain version bump or minversion fence is needed. The wallet format stays readable by earlier releases rather than becoming one-way. Addresses are unchanged: the previous format expanded the entropy on every read and fed the same 64 bytes to Master(). This is also the form the tree already round-trips through -- z_exportwallet dumps the expanded seed, and restoring that hex via -hdseed installs it with fMnemonic = false. Write order is load-bearing: seed first, entropy second. A crash between them leaves a wallet with a seed and no phrase, which is merely inconvenient. The reverse would leave an entropy record with no seed, and the next start would mint a different seed while the wallet still held a phrase for the old one. -usemnemonic still defaults to false; only explicit opt-in and -mnemonic restores take this path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
2d7dd90c55 |
wallet: plumb the mnemonic entropy through CWallet
Wires the key store secret to the database records: load and store paths on CWallet, the two ReadKeyValue arms, and the export path. GetMnemonicPhrase now prefers the entropy record and verifies it before printing: a phrase is only returned if expanding it reproduces the seed derivation actually uses. It falls back to the existing fMnemonicSeed path, so wallets that store the entropy AS the seed keep working unchanged. IsMnemonicSeed() now means "a phrase is available" rather than "the seed is the entropy", which is what every caller actually wants. Still a no-op on every existing wallet: nothing creates an entropy record yet, so GetMnemonicEntropy returns false and the old code path is taken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
3caec548ae |
wallet: persist the mnemonic entropy in wallet.dat
Adds the record pair for the entropy, mirroring "hdseed"/"chdseed": a plaintext form, an encrypted form that erases its plaintext counterpart the way WriteCryptedKey does, and an erase. Both new types are registered in IsKeyType so -salvagewallet preserves them. Without that, salvage would silently drop the phrase while keeping the wallet otherwise intact. The records are defined but nothing writes them yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
2a7fdcc1db |
wallet: add an optional mnemonic-entropy secret to the key stores
Additive plumbing for storing a BIP39 entropy alongside the HD seed, mirroring how the seed itself is handled through both key store layers: a plaintext member on CBasicKeyStore, an encrypted pair on CCryptoKeyStore, encryption during the unencrypted-to-encrypted conversion in EncryptKeys with the plaintext cleared, and decryption on unlock. RawHDSeed and CKeyingMaterial are the same secure_allocator vector type, so the entropy passes through EncryptSecret/DecryptSecret with no adaptation. Nothing calls this yet; there is no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
143c33de48 |
wallet: erase the plaintext HD seed record when the wallet is encrypted
CWalletDB::WriteCryptedHDSeed wrote the "chdseed" record and left "hdseed" in place, unlike WriteCryptedKey which erases "key"/"wkey" after writing "ckey". No erase of "hdseed" existed anywhere in src/wallet/. CDB::Rewrite does not save us: EncryptWallet calls it with pszSkip defaulted, so it copies every surviving record verbatim into the new file. The result is that a wallet created unencrypted and later encrypted keeps its raw HD seed in cleartext on disk permanently, and reloads it into memory on every start. Add CWalletDB::EraseHDSeed and call it from CWallet::SetCryptedHDSeed after the encrypted record is written, through the same CWalletDB so it shares EncryptWallet's transaction. Erase returns true on DB_NOTFOUND, so a wallet that was never written in plaintext is unaffected. The erase is deliberately best-effort and only logs on failure. A hard failure here propagates into CCryptoKeyStore::EncryptKeys, which EncryptWallet turns into assert(false) with half the keys encrypted in memory; a warning is strictly better than that. Note this path is only reachable with -developerencryptwallet, which is experimental and off by default on this chain, so this is a latent fix rather than a live one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
a494eabdce |
wallet: harden HD seed/chain persistence and record seed provenance
Autoshield sends mined coinbase to a seed-derived z-address, so the records that decide how the seed derives keys become fund-safety critical. Three of them were not treated that way. InstallHDSeed wrote the seed record before the chain record, non-transactionally. A crash between the two left a wallet holding a seed with no hdchain: on the next load hdChain silently reverts to defaults, clearing fMnemonicSeed -- which switches the derivation input from the expanded BIP39 seed to the raw entropy -- and resetting saplingAccountCounter. Write the chain first; the opposite torn state is harmless and self-heals, because HaveHDSeed() is then false and init installs again. The hdchain record was read as a bare deserialise inside a catch-all with strErr never set, and it is not a key type, so a corrupt record was downgraded to a non-critical error and the node booted into the wrong key tree. Report it, track whether it was read, and refuse to load a wallet that holds a seed but no readable hdchain. Also preserve hdchain through a keys-only salvage, which would otherwise drop it and produce exactly the state we now refuse. GenerateNewSeed silently fell back to a random seed when BIP39 generation failed, producing a wallet that looks mnemonic-capable but whose words can never be exported and which no seed phrase can restore. A user who asked for -usemnemonic now gets that or a hard failure. Finally, record how the seed came to exist -- created on an empty wallet, restored from -mnemonic/-hdseed, retrofitted onto a pre-existing seedless wallet, or predating this record. A retrofitted seed is in no backup the user already holds, so a feature that moves funds into addresses only that seed can re-derive must not enable itself there by default. Nothing consumes this yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5
|
dad162a89a |
wallet: pick the autoshield destination by seed re-derivation, in-gap only
resolveDestination() took the first spendable address in std::set order, which orders on the raw Sapling diversifier (zcash/Address.hpp:95-98) -- uncorrelated with anything the operator can see, and unstable across restarts as addresses are added. Worse, GetSaplingPaymentAddresses() also returns z_importkey/z_importwallet addresses, and CKeyMetadata is NOT evidence of provenance: both hdKeypath and seedFp are copied verbatim out of the import source (rpcdump.cpp:511-516 -> wallet.cpp:5440-5441) with no verification. A crafted import can therefore claim this wallet's seedFp and keypath m/32'/coin'/0' and capture every shielded mining reward into a key no seed restore can reproduce. Filtering on metadata would not have caught that. Derive instead. A bare -mnemonic/-hdseed restore pre-derives exactly -mnemonicsaplinggap sapling accounts from index 0 with saplingAccountCounter reset (init.cpp:2349-2355), so the only self-recoverable destinations are the default addresses of m/32'/<coin>'/i' for i below the gap. Walk that window from the seed and take the lowest index the wallet holds a spending key for. Deriving is the only authoritative test and cannot be spoofed. When nothing in the window is held yet, derive the next account -- but only if it will land inside the window. GenerateNewSaplingZKey does not derive at saplingAccountCounter: its do/while skips indices already held (wallet.cpp:150-157), so a bare counter-below-gap test is unsound. Predict the lowest free index at or above the counter and post-verify the returned address. If no free account remains below the gap, refuse the round and leave the coinbase transparent -- transparent funds are still recoverable through the 1000-key transparent gap, an unfindable note is not. Refusing is safe: main_impl turns a false return into a clean skip, and main() always advances nextAutoShield and clears fAutoShieldRunning, so a refused round cannot latch the feature off. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
4d72e5fc30 |
wallet: fix sweep-scheduler wedge and unsynchronized driver mutation
The zaddr-sweep op cleared fSweepRunning/nextSweep only on the sweepComplete success path (inside main_impl), so a cancelled or throwing sweep left fSweepRunning stuck true. Since fSweepRunning now also gates consolidation and the default-on autoshield, a persistently failing sweep (e.g. a corrupt-witness note) would wedge all three background ops for the session. Move the scheduler bookkeeping into main() so it runs on every terminal state (success/failure/exception/cancel), guarded by op id. Preserve the intended "keep draining every block until swept" model: on a successful-but-incomplete round the flag stays set and nextSweep is not advanced; on completion OR on failure/exception the flag is released and nextSweep backs off one interval, so a failing sweep no longer retries every block or wedges the other ops. Also fix RunSaplingSweep to take cs_wallet itself (was AssertLockHeld, a no-op in release builds) since ChainTip does not hold it there and the driver mutates scheduler state + enqueues -- matching RunSaplingConsolidation and RunAutoShieldCoinbase. sweepComplete is recorded via a new member; saplingSweepOperationId made public. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
ca730a5d98 |
wallet: fix consolidation-scheduler wedge (ran every block; dead mutual-exclusion)
The Sapling auto-consolidation scheduler never advanced nextConsolidation after init and never set fConsolidationRunning, so once the tip passed the init threshold `-consolidation` dispatched a consolidation op every block instead of once per -consolidationinterval, and every guard that reads fConsolidationRunning (in RunSaplingSweep, and in the new autoshield driver) was dead. Mirror the intended scheduler model: - RunSaplingConsolidation sets fConsolidationRunning=true before dispatch and self-guards with `if (fConsolidationRunning) return;`. - The consolidation op advances nextConsolidation = consolidationInterval + tipHeight and clears fConsolidationRunning on every terminal state (success/failure/exception/cancel), guarded by op id so only the current op mutates scheduler state. - saplingConsolidationOperationId moved to public so the op can read it. Restores the documented once-per-interval cadence and makes the sweep/consolidation/autoshield mutual-exclusion guards effective. Note: the sweep op has the same latent wedge (fSweepRunning/nextSweep are cleared only on the sweepComplete success path, so a cancelled or throwing sweep leaves fSweepRunning stuck true) - left for a follow-up; this commit is the template to port. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
2ebbbc777c |
wallet: add default-on auto-shield-coinbase; drain miner coinbase into a z-addr
On this ac_private=1 chain miners accumulate one transparent coinbase UTXO per block (the only transparent output the chain permits). It had to be shielded manually via z_shieldcoinbase, and left unshielded it is the sole persistent metadata leak on the chain and the source of miner UTXO-fragmentation send failures. Add AsyncRPCOperation_autoshieldcoinbase: a periodic, default-on wallet op driven from CWallet::ChainTip alongside sweep/consolidation, draining matured coinbase into a wallet-owned Sapling z-address in size-bounded batches. - Enqueue-only driver (RunAutoShieldCoinbase): takes only cs_wallet in the notify context; all gathering runs on the async worker under LOCK2(cs_main, cs_wallet). - Dedicated op (not a reuse of z_shieldcoinbase) so it never toggles mining. - Default-ON but conditional: silent no-op on -disablewallet, external -mineraddress, non-mining, or locked wallets (explicit IsLocked() guard). - Destination is reuse-then-create; -autoshieldaddress override is spend-key-validated at init so funds cannot be stranded. - Sweep-model bookkeeping: advances nextAutoShield and clears the running flag on every terminal state; an op-id guard stops a stale op clobbering scheduler state; a self-guard stops cancel/re-enqueue churn. - Sietch-padded output shape matches manual z_shieldcoinbase txns. Config: -autoshield (default true), -autoshieldinterval, -autoshieldaddress, -autoshieldfee (range-validated), -autoshieldminutxos. Incorporates fixes from an 8-angle code review: CAmount fee type with init-time range validation, op-id-guarded flag bookkeeping, driver self-guard, and a tipHeight-consistent NU-activation guard. Note: the fConsolidationRunning / nextConsolidation consolidation-scheduler wedge is a pre-existing bug, left for a separate change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
e3247d946e |
cleanup: rebrand residual user-facing "HUSH" strings to DragonX
Sweeps the leftover coin-name strings in RPC help text, RPC output, and log
messages that the currency-unit change didn't cover:
- RPC help: "mining reward amount in HUSH" -> DRAGONX (mining.cpp x2);
"at least minbal HUSH" -> DRAGONX (rpcwallet.cpp); "the HUSH address" /
"(string) HUSH address" -> DragonX (rawtransaction.cpp)
- RPC output: the SMART_CHAIN_SYMBOL[0]==0 ? "HUSH" : SYMBOL coin-name fallback
(crosschain/misc/mining/blockchain) -> "DRAGONX"; the notarizations JSON key
make_pair("HUSH", ...) -> "DRAGONX"
- Logs: "HUSH blocktime changing", "stopping HUSH HTTP/REST/RPC",
"HUSH raw magic=" -> DragonX
Left untouched (verified): the 82 "HUSH3"/ishush3 chain-symbol consensus checks;
hush_globals.h CURRENCIES[] price-oracle basket (internal lookup, dead feature
on DragonX); hush.h notarization debug printf; a commented-out cout in main.cpp.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 4.8
|
bf3c33c53a |
revert(net): remove header-accept RandomX check; keep nMinimumChainWork + #8
An adversarial re-review found the header-accept RandomX check ( |
||
|
DanSandClaude Opus 4.8
|
b5050d06c0 |
fix(wallet): opreturn_burn return change + widen txfee to CAmount
#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> |
||
|
DanSandClaude Opus 4.8
|
14e3fb6708 |
fix(wallet): reserve miner fee during z_sendmany note selection
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
|
||
|
DanSandClaude Opus 4.8
|
4351d5b733 |
fix(nspv/wallet): bound nSPV request buffers + fix uninitialized fee / null-deref
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> |
||
|
DanSandClaude Opus 4.8
|
762e25294f |
fix: guard BuildWitnessCache against an off-active-chain pindex (heap overflow)
BuildWitnessCache sizes its blockCms buffer from pindex->GetHeight() but the Phase-1 loop walks the active chain (chainActive.Next), terminating only on pbi==pindex. If a reorg moved pindex off the active chain while the notify thread lagged (cs_main is released between per-block ChainTip calls) and the new active tip is taller, the loop never reaches pindex and, once past pindex's height, writes blockCms[h-startHeight] out of bounds -- a heap overflow. Rebuilding witnesses for an abandoned block is meaningless anyway, so bail early when pindex is not on the active chain; cs_main is held for the whole function, so the check cannot race the loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
4caf2fc68f |
Add BIP39 seed phrases (SilentDragonXLite-compatible) and HD transparent keys
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 4.8
|
82d77344d2 |
Fix Sapling witness desync and parallelize witness cache rebuild
Wallets upgraded across the 1.0.1->1.0.2 network transition could end up with note witnesses stuck at a stale height, causing z_sendmany / z_mergetoaddress to fail to build a valid spend. Root cause was a trio of issues that let a desynced witnessHeight perpetuate instead of self-healing: - DecrementNoteWitnesses left witnessRootValidated and the witness deque in an asymmetric state on the size<=1 path. - VerifyAndSetInitialWitness blindly trusted witnessHeight instead of validating the cached root against the chain, so a bad height survived. - UpdatedNoteData copied witnessHeight even when no witnesses were present. - witnessRootValidated was uninitialized and never serialized, so a garbage true value could short-circuit the self-heal. Fixes: - Default witnessRootValidated to false (in-memory only; never serialized). - VerifyAndSetInitialWitness now validates the cached witness root against the block's hashFinalSaplingRoot and reseeds on mismatch. - Symmetric reset of witness state in DecrementNoteWitnesses. - Guard the witnessHeight copy in UpdatedNoteData behind a non-empty witnesses check. - Defensive majority-root guard in GetSaplingNoteWitnesses. Also rewrites BuildWitnessCache to rebuild the witness cache in parallel (per-block commitment extraction + worker pool), cutting a full repair from ~28 min to ~2 min. Tunable via -witnessbuildthreads and -witnessfastrebuild; output verified byte-identical to the serial path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
M
|
d77088c1f2 |
Fix macOS Sequoia build with GCC 15 and update README
- Update compiler references from gcc-8 to gcc-15 across build system (build-mac.sh, darwin.mk, Makefile_custom) - Use system Rust (rustup) instead of bundled Rust 1.32.0 for librustzcash to fix rlib linker incompatibility on macOS Sequoia - Replace deprecated std::random_shuffle with std::shuffle (net.cpp, transaction_builder.cpp, wallet.cpp) - Fix -std=gnu17 -> -std=gnu++17 for C++ targets (libzcash, libhush) - Fix nodiscard warning in glibcxx_sanity.cpp - Replace deprecated OSMemoryBarrier with std::atomic_thread_fence in LevelDB - Add -Wno-error=deprecated-declarations to CXXFLAGS for third-party headers - Fix REMAINING_ARGS unbound variable in build.sh - Add --disable-tests handling to build-mac.sh - Update README with correct macOS build dependencies and instructions |
||
|
Duke
|
f8136f5839 | DragonX has left the nest | ||
|
duke
|
f26f27656a | Set a mainnet donation zaddr for z_shieldcoinbase |