7131b3673f1e586d5e707007744ed050bb111a46
32171
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  DanSandClaude Opus 5.5
|
7131b3673f |
qa: add fd-guard, the close() guard and regtest harness that found the double close
shim/ is an LD_PRELOAD guard around close/fclose/dup2/dup3: - before each close it resolves /proc/self/fd/N; - it reports, with a backtrace, a non-owner close of wallet.dat, database/log.* or blocks/*.dat, and every close that fails with EBADF (the second half of a double close); - FDGUARD_ABORT=1 aborts on the first hit; - run_tests.sh self-tests it (51 checks). repro/ runs a shimmed dragonxd on isolated regtest. Each run gets its own unprivileged network namespace (loopback only). The loads are: - rpc: a hammer with impatient/RST/half-open clients, work-queue bursts and long-polls; - p2p: TLS peers churned by disconnectnode, ban and kill, plus a raw TCP/TLS misbehaver; - tor: a fake control port; - mine: mining with blocknotify and autoshield. tools/tally.py groups the reports by closer. tools/scan_bdb.py finds BDB pages written into block files. With the wolfSSL CloseSocket macro in effect: - p2pmin (300 s) gives one EBADF double close per disconnect; - the full load (1200 s) gives 930 EBADF closes, 186 non-owner closes (37 of wallet/log/blk files) and a 632 KB db.log with BDB0060 PANIC; - RPC-only and Tor-only loads give none. After the fix, every count is 0 and db.log stays empty. The README has the commands and the numbers. Paths come from the environment (GOLDEN, BIN, RUNS, SHIM, ASMAP and the Sapling params). Run output goes to $RUNS, outside the tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
70b35ac11a |
wallet/db: keep the BDB logs when the shutdown flush fails
CDBEnv::Flush(true) removed database/ (log_archive DB_ARCH_REMOVE, then remove_all) once mapFileUseCount was empty. It did that even when the checkpoint or lsn_reset before it had failed, or the environment had panicked (BDB0060). In those states wallet.dat can be missing committed changes that only the logs hold: - a page write failed (EBADF/ESPIPE through an fd closed behind BDB's back, the wolfSSL CloseSocket bug), so the page stayed dirty in the mpool cache and the checkpoint never completed; - pages written after the last lsn_reset carry real LSNs that point into those logs. Deleting the logs turned a wallet that the next start's DB_RECOVER would repair into one that silently lost those changes. A std::terminate kept the logs; the clean shutdown that ThreadNotifyWallets now does on a wallet DB error deleted them. Remember a failed txn_checkpoint or lsn_reset (in Flush and in CheckpointLSN, which the periodic flush and backupwallet use). Before removing anything, also check the environment for a panic. get_flags reports it, and itself fails with DB_RUNRECOVERY once the environment has panicked. In any of those cases keep database/ and log why. If the kept logs are unusable, the next start already moves database/ aside and retries (CWallet::Verify). WalletDbEnv tests: - a clean shutdown still removes the logs; - a panicked environment (DB_PANIC_ENVIRONMENT) keeps them; - a failed lsn_reset keeps them. With the old db.cpp only the first passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
5b36d7ad24 |
validationinterface: shut down cleanly when a wallet block notification throws
ThreadNotifyWallets' block disconnect and connect loops had no try/catch (only its mempool loop does). A wallet database failure inside SyncWithWallets or ChainTip escaped the thread, and TraceThread logs and rethrows, so the node died in std::terminate with nothing flushed. One example: CWalletDB::ListAccountCreditDebit's "error scanning DB" after BDB lost its file to the wolfSSL CloseSocket bug. That is how the pool's node died on 2026-10-12 and 2026-10-16. Run each block's wallet notifications through NotifyWalletsOrShutdown. On an exception it logs the error, requests a shutdown, shows the usual fatal-error message and stops the notifier, leaving pindexLastTip at the last block the wallet fully processed. Carrying on instead would leave the wallet behind the chain, missing the block it failed on. Thread interruption still propagates. The shutdown is requested before the fatal-error message is shown. FdDefense.WalletNotifyFailureShutsDownCleanly: - a throwing notification requests a shutdown, a clean one doesn't; - a non-std exception is handled the same way; - boost::thread_interrupted still propagates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
9cb31326e3 |
main: open block and undo files read-only when they're opened for reading
OpenDiskFile opened every block or undo file "rb+", even for its fReadOnly callers, so every reader held a writable fd. That's what let the wolfSSL CloseSocket bug corrupt blocks/*.dat. A P2P double close took the wallet's BDB fd away; the number was then handed to a block file opened for a getblock/getdata read; BDB's next wallet page write landed in it, at offset pgno*4096. On production that destroyed blocks 2,728,132-2,728,161 in blk00011.dat and block 3,356,880 in blk00020.dat. Every fReadOnly caller only reads through the handle: - ReadBlockFromDisk, UndoReadFromDisk, both GetTransaction paths and hush_blockload read via CAutoFile >>; - the reindex loop hands it to LoadExternalBlockFile, which reads via CBufferedFile; - the existence check at startup only opens it; - hush_prefetch only freads. Open those "rb". A stray write through a reused fd number then fails (EBADF) instead of silently corrupting a block file. Missing files still fail to open as before; writers keep "rb+"/"wb+". FdDefense.ReadOnlyBlockFileIsNotWritable: pwrite through a read-only block file's fd fails with EBADF, while the writable open still writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
DanSandClaude Opus 5.5
|
d91e069da3 |
net: stop wolfSSL's CloseSocket macro from turning P2P closes into double closes
wolfssl/wolfio.h (included by <wolfssl/ssl.h> in net.h) does
`#ifndef CloseSocket / #define CloseSocket(s) close(s)`. Every TU that
includes net.h after that point compiles CloseSocket(x) to a bare
close(x). That includes all 15 calls in net.cpp, among them the ones
in CloseSocketDisconnect and ~CNode. util/sock.cpp's
CloseSocket(SOCKET&), the only thing that sets the handle to
INVALID_SOCKET, was never called from net.cpp. This has been the case
since OpenSSL was replaced by wolfSSL (Hush
|
||
|
DanSandClaude Opus 5.5
|
69bb035e7e |
wallet/main: break the replay -> witness-rebuild -> kill loop on large wallets
A v1.3.0 node with a ~13.6k-note wallet (-dbcache=4084) looked hung for hours:
every start replayed ~62k blocks from a chainstate 26 days stale, then ran a
47-60 min all-core witness rebuild holding LOCK2(cs_main, cs_wallet) (RPC answered
HTTP 500 "Work queue depth exceeded", peers timed out), the GUI killed it before
anything was flushed, and the next start did it all again. Even after escaping,
each new block cost ~30s under LOCK2, so the node fell behind a 36s chain.
D1 BuildWitnessCache: the post-rebuild latch computed witnesses.front().root()
(~2.4ms) for every pending note on every block, only to feed a log line that
is printed for bulk rebuilds alone. The anchor check now runs on the Phase-2
workers and only for a bulk rebuild; the per-block path just sets the latch.
Phase 2 also runs in parallel whenever the job is big (notes x commitments
>= 2000), not only for a bulk range, so busy blocks no longer extend every
note serially. If the OS refuses a worker thread, the remaining shards run
inline instead of unwinding past joinable threads (std::terminate).
D2 FlushStateToDisk: the coins cache was only flushed on shutdown, gettxoutsetinfo
or after 24h; on this chain it stays a few MiB, so the size triggers never
fire and a node that never reached 24h uptime or a clean stop kept its
chainstate (and block index) weeks behind. Flush at least hourly and every
DATABASE_FLUSH_BLOCKS (5000) connected blocks -- the latter also inside one
long ActivateBestChain -- and right after the startup ActivateBestChain when
it moved the tip.
D3 UpdatedNoteData: a replayed or rescanned tx arrives with FindMySaplingNotes
data (no nullifier). Witnesses and witnessHeight were carried over but the
nullifier was dropped, so VerifyAndSetInitialWitness cleared and reseeded the
witnesses on the next block -- the replay above rewrote 3,750 txs and forced
the full rebuild. Carry the nullifier with the witnesses, unless the tx was
re-mined in a different block (its notes moved; reseed as before). When the
merged data is identical, don't rewrite the tx; just clear the in-memory
witnessRootValidated so the next block re-checks the kept roots once (a
rescan still finds and reseeds a witness that went bad, without rebuilding
the healthy ones). The carry loop no longer deep-copies every note.
SetBestChain: with hourly coin flushes the wallet locator must not run ahead
of the async notifier, or a crash permanently skips the blocks it had not yet
delivered. Stamp the block the wallet's state reflects (pindexWalletTip,
seeded in init before ThreadNotifyWallets starts -- at the startup rescan's
checkpoint if that rescan was interrupted -- and advanced contiguously by
ChainTip), like zcashd's ChainTipAdded. ThreadNotifyWallets no longer hands
the wallet an empty block after a fatal block-read failure.
Tests: new gtest test_wallet_notedata.cpp (5 tests); 23/23 gtests pass, and the
two core D3 tests fail with D3 reverted. Regtest A/B against master with a
1,008-note wallet: notify-thread CPU per block 1,780ms -> 3ms; worst LOCK2 RPC
stall 1.6-2.9s -> 10-15ms on ordinary blocks and 4.6-7.1s -> ~1.6s on a block
carrying a 40-output self-send; a z_importkey rescan went from 35 reseeds plus a
bulk rebuild (10.6s) to 0 reseeds, same balance; after kill -9 at 150 blocks a
build with a 100-block flush threshold restarted at 100, master at 0. Two
adversarial review rounds; every confirmed finding is addressed here.
Not covered: the hourly SetBestChain/flush paths were reviewed, not exercised
(GetTimeMicros ignores mocktime); no run yet against a copy of a real large wallet.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 5
|
da191e447d |
Merge branch 'dev' for the v1.3.0 release
72 commits sincev1.3.0 |
||
|
DanSandClaude Opus 5
|
fc26e2e0a0 |
doc: stamp the man pages with the release version, not a build sha
The man pages were regenerated at |
||
|
DanSandClaude Opus 5
|
1d2d5f32d0 |
version: bump PROTOCOL_VERSION for the v1.3.0 consensus changes
doc/release-process.md requires PROTOCOL_VERSION to increase by 1 for any release carrying a consensus change. This one does: |
||
|
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
|
3aac75e94f |
cli: report the port actually dialled, not a stale global
dragonx-cli resolves -rpcport correctly -- CallRPC does
int port = GetArg("-rpcport", BaseParams().RPCPort());
and dials that port. But both connection-failure messages printed
ASSETCHAINS_RPCPORT instead, a separate global defined at the top of
bitcoin-cli.cpp, initialised to the mainnet default 21769, and never assigned
anywhere in the CLI.
So every failure claimed port 21769 regardless of what was asked for:
$ dragonx-cli -rpcport=21799 getblockcount
error: couldn't connect to server at port 21769
That reads as "your -rpcport was ignored", which is a much more alarming and
much more misleading diagnosis than "nothing is listening yet". It cost an hour
of debugging on a node that was simply still loading a 15 GB txindex, and led to
the wrong conclusion that the CLI could be talking to the production daemon when
it was not.
Both sites now print the local `port`. Verified against the previous binary:
new: -rpcport=21799 -> "couldn't connect to server at port 21799"
-rpcport=59999 -> "couldn't connect to server at port 59999"
old: both -> "couldn't connect to server at port 21769"
and the CLI still reaches a live daemon on a non-default port.
ASSETCHAINS_RPCPORT is left defined; it is referenced by other translation units.
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 5
|
db42091ce3 |
rpc/net: gate and harden stratummine, keep regtest off the live network, init nSPV filter
stratummine (rpc/mining.cpp), a test-only reference miner, was registered
unconditionally behind a bare #ifndef WIN32 with okSafeMode=true, so it shipped
as a live RPC on every non-Windows release build. Four fixes:
- Gated behind an explicit, default-off -stratummine, with regtest exempt so
qa/rpc-tests can still drive it. Deliberately NOT gated on fExperimentalMode:
that defaults to TRUE (init.cpp:1195), so such a gate is a no-op -- an error
made and caught while testing this change.
- mining.notify's three hash fields went straight into uint256(ParseHex(...)).
uint256's vector ctor asserts on a wrong size (uint256.cpp:30) and NDEBUG is
defined nowhere in this build, so that assert is live in release; ParseHex
also truncates silently at the first non-hex character. A short or garbled
field therefore ABORTED THE DAEMON. Added StratumHex256(), which requires 64
hex chars and a 32-byte result, and all three fields are validated before any
is committed so a bad job is rejected rather than half-applied.
- processLine ran inside the window where the RandomX cache, the VM and the
socket are live, all released on the normal path only, and every get_str()
throws on a type mismatch -- so a malformed message leaked 256 MB and the fd.
Body wrapped in try/catch: ignore the line, keep mining.
- Caller-supplied timeout clamped to [1, 3600]; it was unbounded, pinning an
RPC worker and the cache indefinitely. okSafeMode -> false.
Verified against a hostile stratum server on regtest:
- 4 malformed mining.notify payloads (short hex, non-hex, wrong JSON type,
31 bytes) -> all rejected, daemon alive, 0 assertions. The 31-byte case is
the one that previously hit the uint256 assert.
- valid job first (so RandomX actually allocates) then garbage mid-mine ->
RSS delta +2.2 MB, i.e. cache and VM released, not the ~256 MB a leak leaves.
- regtest exemption confirmed: stratummine runs past the gate there.
The non-regtest refusal path is by code reading only -- a testnet node on this
host collides with the production daemon's RPC port, so it was not exercised.
hush_utils.h: stop injecting the mainnet node1-node10.dragonx.is addnode seeds
when -regtest or -testnet is set. regtest reuses mainnet's network magic, so a
supposedly isolated node was handshaking production peers and pulling their
headers into its own index. Gated at the injection site only -- isdragonx itself
must stay true, because it also selects ac_private, ac_algo, blocktime and the
reward/halving schedule (an earlier version of this patch gated isdragonx itself
and silently turned ac_private off on regtest). hush_args() runs between
ParseParameters() and ReadConfigFile() (bitcoind.cpp:115/144/158), so this sees a
command-line -regtest, as qa/rpc-tests uses, but not a config-file regtest=1.
hush_nSPV_fullnode.h: initialize `filter` at both sites. It was assigned only on
the len-11 request form; the other two passed uninitialized stack memory to
NSPV_getaddressutxos/NSPV_getaddresstxids, remotely reachable since
HUSH_NSPV_FULLNODE is on by default.
getblocktemplate_proposals.py still passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 5
|
6d282db216 |
consensus: stop BLOCK_VALID_CONTEXT overwriting the block validity level
BLOCK_VALID_CONTEXT was 6. The validity levels above it are sequential VALUES
packed into a 3-bit field, not independent bits, so BLOCK_VALID_MASK is
1|2|3|4|5 == 7 and the value 6 sat entirely inside it. `pindex->nStatus |=
BLOCK_VALID_CONTEXT` (main.cpp:5616, and :3285) therefore did not set a flag --
it overwrote the validity level.
Consequences, all long-standing:
- A header-only block raised to BLOCK_VALID_TREE(2) became 2|6 == 6, which
reads as >= BLOCK_VALID_CHAIN(4) and >= BLOCK_VALID_SCRIPTS(5). A block
merely written to disk reported full script validity.
- Every later RaiseValidity() silently no-opped, because 6 >= every level.
ConnectBlock's RaiseValidity(BLOCK_VALID_SCRIPTS) was a permanent no-op.
- CheckBlockIndex's "CHAIN valid implies all parents are CHAIN valid" invariant
was violated whenever a stored block sat above a still-header-only ancestor,
i.e. ordinary out-of-order parallel block download. fDefaultConsistencyChecks
is true only for regtest, so the abort was regtest-only -- but the garbled
index is written identically on mainnet, where only the detection is off.
Not a v1.3.0 regression: introduced upstream in Komodo
|
||
|
DanSandClaude Opus 5
|
c5fde12485 |
qa: port 12 wallet/mining tests to python3 and fix three framework blockers
Ports qa/rpc-tests from 6 python3 files to 18. Ran every ported test against
the freshly built v1.3.0 dragonxd. Results, honestly:
PASS (1) getblocktemplate_proposals.py
NOT APPLICABLE (7) wallet.py, walletbackup.py, wallet_protectcoinbase.py,
wallet_listnotes.py, wallet_mergetoaddress.py,
getblocktemplate.py, wallet_shieldcoinbase.py
BLOCKED (4) wallet_sapling, wallet_nullifiers, wallet_persistence,
wallet_treestate
The "not applicable" seven are inherited Zcash/Hush-era tests that exercise
features DragonX deliberately removed. They assume transparent t->t value
transfer, but ASSETCHAINS_PRIVATE=1 (hush_utils.h:1826) makes sendtoaddress
and sendmany consensus-refuse; they assume Sprout joinsplits, which are gone
from the RPC layer entirely; and they hardcode Bitcoin economics (10 coin/block,
100-block maturity) against DragonX's 3 DRGX and COINBASE_MATURITY=1. They are
ported and left in place rather than deleted, but they cannot pass on this chain
without being rewritten around z_shieldcoinbase/autoshield.
Three framework fixes in test_framework/util.py, each of which broke every
multi-node test:
- initialize_chain() passed -connect=0, and init.cpp soft-sets -listen=0 when
-connect is present, so cache node0 never opened its p2p port and nodes 1-3
could never sync to it -- initialize_chain() hung forever in sync_blocks().
Now passes -listen=1 -bind=127.0.0.1 -dnsseed=0 explicitly (an explicit arg
beats SoftSetBoolArg) while keeping the cache nodes off the public network.
- cache cleanup removed files from <datadir> when dragonxd writes them one
level deeper into the net-specific <datadir>/regtest.
- set_node_times() did print("..." + t) with t an int -> TypeError.
- default binary paths corrected to src/dragonxd and src/dragonx-cli.
The four BLOCKED tests are blocked by a daemon assert, not by the port; see the
follow-up commit/report on BLOCK_VALID_CONTEXT.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 5
|
e2e10f6ef8 |
doc: regenerate man pages from the v1.3.0 binaries
Previously the version strings were restamped by hand in
|
||
|
DanSandClaude Opus 5
|
af7d9e2300 |
release: bump to v1.3.0 and document the signing step that was never performed
The tree stamped 1.2.0 in both configure.ac and src/clientversion.h, but
v1.2.0 is an annotated tag already pushed at
|
||
|
DanS
|
4cc7e0491a |
qa: finish the python3 port far enough to build the shared test chain
Follow-up to |
||
|
DanS
|
5e0a706839 |
qa: repair the rpc-test harness so it can start a DragonX node at all
The integration suite has never run against DragonX. Six independent
defects stacked up, each only visible once the previous was fixed:
1. test_framework used python2 implicit relative imports
("from authproxy import ..."), removed in python3, so every test died
at import. Made explicit relative imports.
2. It wrote ZZZ.conf -- a Komodo assetchain convention -- while DragonX
reads DRAGONX.conf. The daemon therefore never saw the generated
config, fell back to mainnet defaults and tried to bind RPC 21769,
which on a seed node is already held by the real node.
3. start_node() was hard-wired for the -ac_name=ZZZ assetchain tests: it
took the RPC port from extra_args[3], passed extra_args[0] as argv[0]
of the CLI, and only wrote a config when extra_args[0] matched. Any
test that passes no extra args crashed on len(None). The generic path
now takes the port from rpc_port(i) -- the same helper
initialize_datadir() already used -- and drives the CLI with -datadir.
4. dragonxd refuses to start without an asmap file, which no test datadir
had. initialize_datadir() now provisions one.
5. -asmap relative paths resolve against the NET-SPECIFIC datadir, so a
copy in <datadir> is never found. Pass an absolute path.
6. Worst: -regtest was only ever set as "regtest=1" in the conf file,
which DragonX ignores. Every "regtest" node therefore ran on MAINNET:
real genesis, real seeds, real peers. An observed run synced 196,180
live blocks and 679MB into /tmp before the test timed out. -regtest is
now passed as a command-line flag, with -connect=0 so an isolated
regtest node stays off the public network.
With these, nodes start, RPC answers, and tests run to a real result.
They do not all pass yet -- getblocktemplate.py reaches an assertion --
but that is now a test outcome rather than a harness failure.
|
||
|
DanSandClaude Opus 4.8
|
60d66022f6 |
regtest: default -checkpoints off so an isolated node leaves IBD
chainparams_commandline() applies the DRAGONX mainnet checkpoint set to every SMART_CHAIN_SYMBOL=="DRAGONX" network, -regtest included, so an isolated regtest node (height ~hundreds) sits far below the top checkpoint (~3.2M). IsInitialBlockDownload() latches true whenever fCheckpointsEnabled && height < GetTotalBlocksEstimate(), so regtest was permanently in IBD -- disabling every ChainTip auto-op that gates on !IBD (autoshield, z_sweep, consolidation) and the below-checkpoint script-check skip, and forcing every regtest test to pass -checkpoints=0 by hand. Default -checkpoints to false on regtest (still overridable with -checkpoints=1). Verified: a fresh regtest node logs "Leaving InitialBlockDownload" after a few blocks with no flag, and -checkpoints=1 keeps it in IBD as before. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGhWvdBSgt6UxxHANr7gfN |
||
|
DanSandClaude Opus 4.8
|
3b2aa866aa |
rpc: honor a command-line rpcpassword across restarts; fix dead -rpcusername key
hush_configfile(), on any restart where the auto-generated DRAGONX.conf
already exists, hard-assigned mapArgs["-rpcpassword"] from the conf and
wrote the username to mapArgs["-rpcusername"] -- a key nothing reads
(InitRPCAuthentication in httprpc.cpp and bitcoin-cli.cpp both read
"-rpcuser"). The hard assignment silently overwrote a -rpcpassword passed
on the command line, so after the first run the effective RPC credentials
became {cmdline-user}:{conf-password}, matching neither the command-line
pair the operator passed nor the full conf pair. Automation or external
clients that connect with the known command-line password broke on every
restart.
Use SoftSetArg for both, so a command-line (or explicitly configured)
value wins and the conf-derived credential is only a fallback. Verified on
regtest: after a restart an external client with the command-line password
gets HTTP 200 and the conf's random password gets 401; the conf-only
operator path (no command-line creds) still authenticates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGhWvdBSgt6UxxHANr7gfN
|
||
|
DanSandClaude Opus 4.8
|
2d6359ea74 |
gtest: add job_id parser regression test for the 63-char abort fix
Covers
|
||
|
DanS
|
fa16e740b6 |
stratum: reject low-diff shares before spending a RandomX hash on them
SubmitBlock validated in the wrong order. After two O(1) length checks it went straight to CheckRandomXSolution() -- a full ~65ms randomx_calculate_hash -- and only afterwards tested the share target. So 32 arbitrary bytes from any peer bought a RandomX hash, and the cost was paid in three bad places at once: on the shared HTTP/RPC libevent thread (stratum uses EventBase(), the same base ThreadHTTP dispatches, and RPC replies are posted back onto it), inside the read loop that holds cs_stratum so BlockWatcher cannot push new work to real miners, and holding the global cs_randomx_validator that block validation also takes. One connection writing submit lines pins that thread indefinitely. Move the share-target test above the RandomX verify. CBlockHeader:: GetHash() is SerializeHash over the header including nSolution, so meeting the target still requires real SHA256d grinding -- an attacker now pays for the hash instead of the node. Semantics are unchanged, including that an empty local_diff parses to zero and still rejects; only the position moved, and the diagnostics are recomputed locally since the old message used variables declared further down. This does not make the submit path cheap, only bounded: at the default share target the grind is small, so a submit rate limit and moving SubmitBlock off the event loop are both still wanted. Also add the authorization check every sibling handler has and mining.submit lacked. Being explicit about what that is worth: mining.authorize validates no credential, so the gate is parity and handshake-ordering, not authentication. The reorder above is the part that actually bounds an unknown peer. |
||
|
DanS
|
5f40c8ede0 |
stratum: stop paying every miner's blocks to whoever asked for work first
CreateNewBlock() builds the stratum template with an OP_FALSE placeholder
in the coinbase, and CustomizeWork() substituted the miner's own payout
address only while that placeholder was still intact. GetWorkUnit() then
wrote the customized coinbase straight back into the shared template:
current_work.GetBlock().vtx[0] = cb;
current_work.GetBlock().hashMerkleRoot = ...BuildMerkleTree();
which consumed the placeholder for everyone. The first client to request
work after a tip change therefore captured the template. Every later
client on that job got a mining.notify whose merkle root already
committed to the first client's coinbase, CustomizeWork() was a no-op for
them on submit, and SubmitBlock() read the shared root back -- so a block
found by miner B was accepted paying miner A. It was silent: the daemon
logged "GOT BLOCK!!! by <B>" while the coinbase paid A.
With untrusted miners that is a reward-theft primitive, and it is cheap:
mining.authorize sets m_send_work, so re-sending it in a loop wins the
race after every tip.
Leave the template pristine and derive each client's header from a local
copy, in GetWorkUnit for the notify and again in SubmitBlock from the
coinbase CustomizeWork() just produced for that client. This is the
refactor the TODO removed here was asking for.
CustomizeWork() now stamps the payout script unconditionally, so a
coinbase that somehow arrives already customized can never be inherited
by another miner, and rejects an invalid address rather than silently
building a coinbase that pays no one.
Single-miner behaviour is unchanged, which is why this survived: it is
only observable with two miners on one template.
|
||
|
DanS
|
b3e81f1eda |
stratum: do not abort the daemon on a malformed 63-character job_id
The "EWBF 31 bytes job_id fix" in stratum_mining_submit completes a 63-character job_id with each hex digit in turn and feeds the result straight to uint256(). ParseHex() stops at the first non-hex character and returns a shorter vector without signalling an error, and base_blob(const std::vector<unsigned char>&) asserts vch.size() == 32. So a single mining.submit whose job_id is any 63-character string containing a non-hex byte -- 63 spaces will do -- aborts the node. asserts are live in release builds here: -DNDEBUG appears only in leveldb's own makefile, never in configure.ac, and the shipped binary still carries the assertion string. The dispatch loop catches UniValue and std::exception; abort() goes through both. Size-check each candidate before constructing, as ParseUInt256() a few hundred lines up already does for the ordinary path. If none of the 16 completions parse, ret stays null, misses work_templates, and the handler returns false exactly as it does for any unknown job. Not gated on IsHex(job_id_str): 63 is odd and IsHex() requires an even length, so that test would disable the EWBF path this code exists for. |
||
|
DanS
|
d05302d450 |
build: stamp container builds with the real version instead of "-unk"
.dockerignore excludes .git, so util/genbuild.sh finds no repository inside the container and emits "// No build information available", which clientversion.cpp renders as the "-unk" suffix. Every binary produced by ./build.sh --linux-compat therefore self-reports "v1.2.0-unk" and cannot be traced to a commit -- including release artifacts, since this build path is part of the v1.2.0 tag. Pre-generating src/obj/build.h does not survive (genbuild rewrites it when the content differs), and simply un-ignoring .git does not help a linked worktree, whose .git is a file pointing outside the build context. So build.sh computes the version on the host, mirroring genbuild.sh rule for rule -- the nearest tag only when HEAD is that tag and the tree is clean, otherwise v<VERSION>-<short sha> with a -dirty suffix -- and passes it through a BUILD_DESC build-arg that Dockerfile.compat exports as DRAGONX_BUILD_DESC. genbuild.sh honours that variable when set and is otherwise untouched; with git metadata present it emits a byte-identical build.h. Every added git call is guarded with || true because build.sh runs under set -eu -o pipefail: a source tarball, a host without git, or a branch whose only reachable tags are lightweight (v1.0.1-v1.0.3 are lightweight; v1.1.0 is the first annotated one) would otherwise abort the build with no diagnostic. Those cases now degrade to the previous "-unk" behaviour with a warning. A direct "docker build -f Dockerfile.compat" passes no BUILD_DESC and still produces -unk; the Dockerfile now says so loudly rather than silently. |
||
|
DanSandClaude Opus 4.8
|
2232868d9f |
stratum: RandomX pool mining support + reference miner (stratummine)
The stratum server was hardcoded for legacy Equihash (1347-byte solution,
sol.begin()+3 offset, CheckEquihashSolution) and was off-by-default with a
"do not use on RandomX" warning. DragonX is RandomX, so external pool mining
was impossible. This wires RandomX end-to-end.
Server (stratum.cpp), branched on ASSETCHAINS_ALGO == ASSETCHAINS_RANDOMX:
* GetWorkUnit sets StratumWork.nHeight and, for RandomX, sends the per-height
RandomX key via a new mining.set_randomx_key message (a miner cannot derive
it without the chain). The legacy mining.notify format is unchanged.
* SubmitBlock/stratum_mining_submit accept a 32-byte solution (== the RandomX
hash, used as nSolution verbatim) and validate it with CheckRandomXSolution
instead of CheckEquihashSolution. Target check (GetHash() < target) and the
nNonce = extranonce1||extranonce2 assembly are shared with the equihash path.
* -stratumtarget=<hex> overrides the pool share target (default diff-1); lets a
solo/low-difficulty test miner accept easy shares.
* GetWorkUnit's IsInitialBlockDownload guard is bypassed under -testnode=1 so an
isolated low-work test chain can serve work.
Reference miner: `stratummine "host" port ("address" timeout)` RPC (rpc/mining.cpp,
POSIX-only). A minimal stratum client that subscribes/authorizes, receives work +
the RandomX key, varies nNonce, hashes with RandomX via GetRandomXInput (byte-
identical to CheckRandomXSolution) and submits a 32-byte solution. Off-the-shelf
Equihash/Monero miners can't speak DragonX's 256-bit-nNonce Zcash header, so this
is the reference implementation. Added to the rpc client arg-conversion table.
Validated: loopback (chain 0->4, accepted every time, verifychain=true) AND a real
2-box LAN run (Linux miner -> Mac stratum server, 3 blocks accepted, verifychain=true).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
DanSandClaude Opus 4.8
|
1745ee4e63 |
net: fix -connect never dialing its targets (empty-addr IsValid gate)
ConnectNode() rejected the connection on !addrConnect.IsValid() (and !IsReachable(addrConnect)) BEFORE the pszDest branch. But -connect (and -addnode host:port / "addnode <host> onetry") reaches ConnectNode with an empty placeholder addrConnect and the real target in pszDest, resolved later by ConnectSocketByName(). An empty CAddress is invalid, so every -connect attempt returned NULL before a socket was ever opened — the peer logs "trying connection <host>" then "ConnectNode FAILED" and never dials. (Introduced upstream with BIP155/addrv2; -connect went unused because normal operation connects via addrman with real, valid addresses.) Guard both early-return checks with `if (!pszDest)` so they apply only when dialing addrConnect directly. Connect-by-name now falls through to ConnectSocketByName() as intended. Direct-address connections (pszDest==NULL, the addrman path) are unchanged. Validated: two nodes on one host, B started with `-connect=<A>` exclusively (no -addnode) now connects to A over TLS, syncs A's chain, and stays isolated (0 other peers) — previously B connected to nothing. Clean build, verifychain 4 0 = true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
DanSandClaude Opus 4.8
|
11766ec61b |
init: fix Windows build regression — fs::path::c_str() into LogPrint
Phase 4 (
|
||
|
DanSandClaude Opus 4.8
|
004808403f |
cleanup: dedup addrman Select_ table walk and pow-limit-by-algo selection
Two behavior-preserving refactors flagged by the Phase-6 hygiene scoping: * addrman: CAddrMan::Select_ contained two ~40-line copies of the same bucket-table random walk, differing only in the table (vvTried/vvNew), its bucket count, and a log label. Extract the shared loop into SelectFromTable_(vvTable, nBucketCount, tableName); Select_ now just dispatches to it. Verbatim move — clean compile proves self-containment. * pow: the "Equihash uses powLimit, everything else uses powAlternate" selection was copy-pasted as an if/else into GetNextWorkRequired, CalculateNextWorkRequired, and lwmaCalculateNextWorkRequired. Extract into PowLimitForAlgo(params). The CheckProofOfWork site (line ~892) is left as-is: it has an extra `height <= 1` genesis special-case and is NOT the same selection. On DragonX (RandomX) this always returns powAlternate, exactly as before. Validated: full build of dragonxd/cli/tx, isolated self-mine to height 336, verifychain 4 0 -> true (exercises PowLimitForAlgo on every block). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
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
|
c92e69a13f |
miner: fix cs_main/mempool.cs lock leak on the isStake error paths
CreateNewBlock takes cs_main and mempool.cs unconditionally via
ENTER_CRITICAL_SECTION, and releases them on every return path. Two of those
LEAVE pairs — the notary-pay failure and the TestBlockValidity failure — were
wrapped in `if (!isStake) { LEAVE; LEAVE; }` but still `return(0)`
afterwards, so when isStake is true the function returned with both locks
still held: a lock leak that deadlocks the next cs_main acquirer. The success
and timelock return paths already release unconditionally, so the guard was
simply wrong.
Make both LEAVE pairs unconditional to match the ENTER. Behavior-identical on
DragonX (a RandomX chain where staking/LWMAPOS is off, so isStake is always
false and the LEAVE already ran), and correct for both isStake values.
Verified: a node self-mined thousands of blocks across two mining threads
(each block ENTER/LEAVEs the locks in CreateNewBlock) with height rising
continuously and verifychain=true — a leak would have deadlocked immediately.
The fuller RAII conversion (replace the manual ENTER/LEAVE with a scoped
LOCK2 for exception safety) remains a worthwhile follow-up, but is a larger
change to cs_main handling best done under its own review.
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
|
267e6f7ad5 |
consensus: drop the dead CBOPRET price-validation from the coinbase check
Step 0 of the scoped HAC/HUSH3-drop work (the zero-risk, self-contained piece). ContextualCheckCoinbaseTransaction's only action was calling hush_opretvalidate() for CBOPRET price-oracle validation, gated on ASSETCHAINS_CBOPRET. On DragonX that global is always 0 (default 0, only ever set by -ac_cbopret, which DragonX never passes), so the branch is dead and the function already returns true for every DragonX coinbase. Remove the dead branch; the function is now unconditionally valid at this stage, which is behavior-identical on DragonX. Verified: a node self-mines and `verifychain` re-validates the whole chain (every coinbase re-checked through this function) = true. hush_opretvalidate (hush_gateway.h) is now unreferenced; its removal is part of the larger hush_gateway cleanup, tracked with the rest of the HAC/HUSH3-drop follow-up. 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
|
f76382f8e7 |
Merge fix/windows-cross-build into the v1.2.0 line
Brings the two mingw cross-compile build-system fixes (link-group via AC_SUBST(LINK_GROUP_*), librustzcash staged-name normalization) onto the v1.2.0 line that now carries Phases 1-2 of the code-hygiene remediation. The two sides are disjoint (build-system files vs. src/wallet/rpc), so this is a clean integration; the merged tree is re-verified to build for Windows (Phase 1/2 source changes had only been built for Linux before). 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
|
ac95106abe |
build(win): make the mingw cross-compile link and find librustzcash
Three things broke the x86_64-w64-mingw32 build; two are real and fixed here (the third was a stale-object contamination from building Linux and Windows in the same tree, resolved by a clean rebuild — not a code fix). 1. Single-pass mingw ld could not resolve the cross-references DragonX added between the internal static archives (libbitcoin_util/common objects pulling in UniValue; util<->common mutual deps). GNU ld on Linux re-scans archives so it never surfaced; ld64 on macOS rejects the grouping flag outright. Bracket each binary's _LDADD in -Wl,--start-group/--end-group, delivered via AC_SUBST(LINK_GROUP_*) so automake does not reject the linker flag inside _LDADD, and left empty on every non-Windows target. 2. The Rust build emits the mingw archive as rustzcash.lib, but the link line asks for -lrustzcash, i.e. librustzcash.a. Normalize the staged filename to librustzcash.a for every host (a no-op on Linux/macOS, where the basename was already librustzcash.a). 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
|
fad05d3ab4 |
doc: regenerate manpages for v1.2.0
They still described v1.0.3-4caf2fc68, so none of the options added since -- -autoshield and its four companions, -mnemonic, -mnemonicsaplinggap -- appeared anywhere in them. Generated from a binary built at a clean tree on an annotated v1.2.0 tag, which is what makes util/genbuild.sh emit BUILD_DESC "v1.2.0" rather than a version with a commit suffix. Note that genbuild.sh uses `git describe --abbrev=0`, which ignores lightweight tags; v1.0.0 through v1.0.3 were lightweight, which is why those releases all reported themselves as v1.0.x-<sha>. Tag releases annotated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeov1.2.0 |
||
|
DanSandClaude Opus 5
|
5d3f7e520c |
packaging: correct the debian changelog and the manpage file list
- dragonx.manpages listed DEBIAN/manpages/*.1, a path nothing creates,
so dh_installman would fail on it. Point it at doc/man/*.1, which is
where util/gen-manpages.sh writes and what doc/man/Makefile.am ships.
- Restore the 1.0.1 and 1.0.2 stanzas, reconstructed from the commits
each tag actually contains. The file jumped 1.0.3 -> 1.0.0.
- The 1.1.0 and 1.0.0 trailers named weekdays that do not match their
dates ("Thu, 21 Aug 2026" is a Friday; "Mon, 03 Mar 2026" is a
Tuesday). Replace both with the real v1.1.0 and v1.0.0 tag dates,
which fixes the weekday and the disagreement with the tag at once.
- Record the 1.2.0 peer-discovery work in its stanza.
- Drop doc/man/hushd.html and doc/man/hush-cli.html: stale Hush-branded
pages for binaries this tree no longer builds, referenced by nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo
|
||
|
DanSandClaude Opus 5
|
092a608fd9 |
chainparams: do not let other smart chains inherit DragonX's seeds
A smart chain builds its params by copying a base network and overriding pieces, but nothing ever touched vSeeds/vFixedSeeds. That is how DRAGONX ran on Hush's seeds -- seed1.hush.is and friends, long since gone from DNS -- for as long as it did, and it means any other assetchain started from this binary now inherits DragonX's. Seeds are per-chain by nature: an address serving one chain is useless to another, and dialling it is at best wasted effort and at worst a peer speaking a different protocol. DRAGONX keeps the seeds configured in CMainParams, which are its own; every other chain starts empty and relies on -addnode/-connect, which an assetchain operator has to configure regardless. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo |
||
|
DanSandClaude Opus 5
|
3dd667b127 |
net: add two seed nodes and reserve names for three more
node6.dragonx.is (13.140.58.251) and node7.dragonx.is (5.104.83.100) are new full nodes in regions the existing five did not cover. Both go into the compiled-in fixed-seed list and into the DRAGONX -addnode set. node8 through node10 are reserved names with no DNS records yet. A hostname that does not resolve is harmless on this path -- ThreadOpenAddedConnections simply fails to open the connection and retries on its normal cycle -- and reserving the names in the binary means a future seed can be brought into the default addnode set by creating a single DNS record, with no release and no waiting for users to upgrade. seed.dragonx.is already gives the DNS-seed path that property; this extends it to the addnode path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo |
||
|
DanSandClaude Opus 5
|
dde6cd810f |
net: seed from a round-robin DNS record instead of hardcoded hosts
seed.dragonx.is is now an A-record set over the five seed nodes (DNS-only, TTL 300,
created in Cloudflare alongside this change). One lookup returns all of them, and
the set can change -- a node added, a node retired -- with a DNS edit rather than a
release.
That is the actual point. Before this the network's entry points were hardcoded
into the binary twice over: here in vSeeds, and again in the -addnode injection in
hush_utils.h. Adding a sixth node meant shipping a new version and waiting for
users to upgrade.
node1 and node5 stay as static fallbacks against the round-robin record being
mistyped or deleted. They resolve to the same hosts, so that is insurance against a
DNS mistake rather than real redundancy.
Verified end to end on a fresh datadir (empty addrman, real node untouched, and
crucially with NO custom -port -- see below):
before: 0 addresses found from DNS seeds, 0 handshakes, 1 block (genesis)
after: 7 addresses found, connection attempts to all five seeds on :21768,
3 version handshakes, 3296 blocks connected and syncing
The earlier run of this test appeared to fail with 0 handshakes. That was the
harness, not the code: -port overrides ASSETCHAINS_P2PPORT, and net.cpp builds
DNS-seeded addresses as CAddress(CService(ip, ASSETCHAINS_P2PPORT)), so a test node
with a custom port dials every seeded peer on its own port and reaches nothing.
That is the mechanism behind the long-standing "never use a custom -port on a test
node" rule; use -listen=0 instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|