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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
- 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
