diff --git a/qa/r4-salvaged-wallet-harness.sh b/qa/r4-salvaged-wallet-harness.sh new file mode 100755 index 000000000..5ca5c35e4 --- /dev/null +++ b/qa/r4-salvaged-wallet-harness.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# r4 now-tier acceptance harness. MUST run inside a network namespace: the v1.0.x binaries +# predate the regtest seed-injection fix and would otherwise dial the live DragonX network. +set -u +OLD=/home/dev/dragonx/release/dragonx-1.0.1-linux-amd64 +NEW=/home/dev/dragonx-dev/src +ROOT=/tmp/claude-1000/-home-dev/45a644d6-ea0b-4b7c-8ec7-6ccb1a64afa7/scratchpad/r4lab +PASS=0; FAIL=0 +ok(){ echo " PASS $1"; PASS=$((PASS+1)); } +no(){ echo " FAIL $1"; FAIL=$((FAIL+1)); } + +# --- hard guardrail: refuse to run unisolated --- +if [ "$(ip route show 2>/dev/null | wc -l)" != "0" ]; then + echo "REFUSING: not in an isolated netns (routes present). Run under: unshare -rn"; exit 90 +fi +ip link set lo up 2>/dev/null +echo "isolation: $(ip route show | wc -l) routes, $(ip -o link | wc -l) interface(s)" + +conf(){ printf 'regtest=1\nrpcuser=t\nrpcpassword=t\nlisten=0\ndnsseed=0\n' > "$1/DRAGONX.conf"; } +start(){ # $1=bindir $2=datadir $3=extra + "$1/dragonxd" -regtest -datadir="$2" -connect=0 -listen=0 -dnsseed=0 $3 -daemon >/dev/null 2>&1 + for i in $(seq 40); do "$1/dragonx-cli" -regtest -datadir="$2" -rpcuser=t -rpcpassword=t getblockcount >/dev/null 2>&1 && return 0; sleep 2; done + return 1; } +cli(){ "$1/dragonx-cli" -regtest -datadir="$2" -rpcuser=t -rpcpassword=t "${@:3}" 2>&1; } +stopn(){ cli "$1" "$2" stop >/dev/null 2>&1; sleep 6; } + +rm -rf "$ROOT"; mkdir -p "$ROOT" + +echo; echo "### build victim wallets with the OLD binary (v1.0.1) ###" +mkdir -p "$ROOT/base/regtest"; conf "$ROOT/base/regtest" +start "$OLD" "$ROOT/base/regtest" "" || { echo "old node failed to start"; exit 91; } +[ "$(cli "$OLD" "$ROOT/base/regtest" getconnectioncount)" = "0" ] && ok "victim-maker has 0 peers (isolated)" || no "victim-maker NOT isolated -- ABORT" +cli "$OLD" "$ROOT/base/regtest" getnewaddress >/dev/null +cli "$OLD" "$ROOT/base/regtest" z_getnewaddress >/dev/null +ZBEFORE=$(cli "$OLD" "$ROOT/base/regtest" z_listaddresses | tr -d ' \n') +stopn "$OLD" "$ROOT/base/regtest" + +for v in C A B; do cp -a "$ROOT/base" "$ROOT/$v"; done + +# A = salvaged by v1.0.1 (drops hdchain). B = A then USED on v1.0.1 (persists a bogus chain). +start "$OLD" "$ROOT/A/regtest" "-salvagewallet" && stopn "$OLD" "$ROOT/A/regtest" +start "$OLD" "$ROOT/B/regtest" "-salvagewallet" && stopn "$OLD" "$ROOT/B/regtest" +start "$OLD" "$ROOT/B/regtest" "" && { cli "$OLD" "$ROOT/B/regtest" z_getnewaddress >/dev/null; stopn "$OLD" "$ROOT/B/regtest"; } + +echo " A hdchain records: $(strings "$ROOT/A/regtest/regtest/wallet.dat" | grep -c hdchain) (expect 0)" +echo " B hdchain records: $(strings "$ROOT/B/regtest/regtest/wallet.dat" | grep -c hdchain) (expect >=1)" + +echo; echo "### open each on the NEW binary ###" +for v in C A B; do + D="$ROOT/$v/regtest"; L="$D/regtest/debug.log" + WDAT="$D/regtest/wallet.dat" + [ -f "$WDAT" ] || { echo " FAIL $v: wallet.dat not found at $WDAT"; exit 92; } + # NOT a byte-identical check: normal startup (keypool top-up, bestblock) rewrites wallet.dat for + # ANY wallet, healthy ones included -- verified with a control. The precise claim is that the + # degraded path never SYNTHESISES an hdchain record, so count that instead. + HD1=$(strings "$WDAT" | grep -c hdchain) + : > "$L" 2>/dev/null + if start "$NEW" "$D" "-exportdir=$D/exp"; then + STARTED=yes; DEG=$(grep -c "DEGRADED" "$L" 2>/dev/null) + MISS=$(grep -c "hdchain record is missing" "$L" 2>/dev/null) + MISM=$(grep -c "does not belong to this wallet" "$L" 2>/dev/null) + if [ "$v" = "A" ]; then + mkdir -p "$D/exp"; EXP=$(cli "$NEW" "$D" z_exportwallet r4dump 2>&1 | head -1) + DUMP=$(find "$D" -name 'r4dump' 2>/dev/null | head -1) + ZNEW=$(cli "$NEW" "$D" z_getnewaddress); TNEW=$(cli "$NEW" "$D" getnewaddress) + fi + stopn "$NEW" "$D" + else STARTED=no; DEG=0; MISS=0; MISM=0; fi + HD2=$(strings "$WDAT" | grep -c hdchain) + + case $v in + C) [ "$STARTED" = yes ] && ok "C healthy wallet opens" || no "C healthy wallet failed to open" + [ "$DEG" = "0" ] && ok "C no false positive (not flagged degraded)" || no "C FALSE POSITIVE: healthy wallet flagged" ;; + A) [ "$STARTED" = yes ] && ok "A salvaged wallet opens (was DB_CORRUPT before)" || no "A salvaged wallet still refuses to open" + [ "$MISS" -ge 1 ] && ok "A flagged: hdchain missing" || no "A not flagged as missing-hdchain" + [ "$HD1" = "0" ] && [ "$HD2" = "0" ] && ok "A no hdchain synthesised (degraded path persists nothing)" || no "A hdchain record appeared ($HD1 -> $HD2)" + echo "$ZNEW" | grep -qi 'error' && ok "A z_getnewaddress refused cleanly (derivation gated)" || no "A z_getnewaddress derived anyway: $ZNEW" + echo "$TNEW" | grep -qiE '^R[a-zA-Z0-9]+$' && ok "A getnewaddress still works (legacy random t-key)" || no "A getnewaddress broke: $TNEW" + if [ -n "${DUMP:-}" ] && [ -f "$DUMP" ]; then + grep -qE '^# HDSeed=[0-9a-f]' "$DUMP" && no "E z_exportwallet emitted an HDSeed line on a degraded wallet" || ok "E z_exportwallet emitted no bogus HDSeed line" + else echo " SKIP E (no dump produced: $EXP)"; fi ;; + B) [ "$STARTED" = yes ] && ok "B poisoned wallet opens" || no "B poisoned wallet failed to open" + [ "$MISM" -ge 1 ] && ok "B CASE-3 DETECTOR FIRED (seedFp mismatch)" || no "B case-3 detector did NOT fire" ;; + esac +done +echo; echo "### $PASS passed, $FAIL failed ###" +exit $FAIL diff --git a/src/init.cpp b/src/init.cpp index f1bb05347..4efe76104 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2260,10 +2260,22 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (nLoadWalletRet != DB_LOAD_OK) { if (nLoadWalletRet == DB_CORRUPT) - strErrors << _("Error loading wallet.dat: Wallet corrupted. If this wallet was last opened " - "by an older version, move wallet.dat aside and restore from your seed " - "phrase with -mnemonic=\"\" -rescan (see debug.log for " - "the specific record at fault).") << "\n"; + { + // Abort HERE, as the DB_NEED_REWRITE branch below already does. Falling through + // runs several hundred more lines of initialisation against a wallet we have just + // declared corrupt -- including SetHDSeedOrigin(), which WRITES to it, and the + // rescan and SetBestChain that follow. + // + // The old text advised restoring with -mnemonic. That is wrong twice over: + // -usemnemonic defaulted to 0 in v1.0.3 so many such wallets never had a phrase, + // and SetHDSeedFromMnemonic refuses a non-empty wallet, so "move wallet.dat aside" + // would discard every non-HD key the salvage preserved. + strErrors << _("Error loading wallet.dat: the wallet database is corrupt. Your keys may " + "still be intact -- do NOT delete or replace wallet.dat. Back it up now, " + "and see debug.log for the specific record at fault.") << "\n"; + LogPrintf("%s", strErrors.str()); + return InitError(strErrors.str()); + } else if (nLoadWalletRet == DB_NONCRITICAL_ERROR) { string msg(_("Warning: error reading wallet.dat! All keys read correctly, but transaction data" diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 069404ca7..7eb29e42c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -746,10 +746,17 @@ UniValue dumpwallet_impl(const UniValue& params, bool fHelp, bool fDumpZKeys) HDSeed hdSeed; // Dump the 64-byte derivation seed (for mnemonic wallets this is the // expanded BIP39 seed), so re-importing the hex reproduces the same keys. - pwalletMain->GetHDSeedForDerivation(hdSeed); - auto rawSeed = hdSeed.RawSeed(); - file << strprintf("# HDSeed=%s fingerprint=%s", HexStr(rawSeed.begin(), rawSeed.end()), hdSeed.Fingerprint().GetHex()); - file << "\n"; + // The return MUST be checked: on failure hdSeed is default-constructed, and emitting it + // anyway writes a blank seed next to a legitimate-looking BLAKE2b-of-empty fingerprint -- + // a backup that looks valid and restores nothing. The per-key dump below is still a + // complete backup without this line. + if (pwalletMain->GetHDSeedForDerivation(hdSeed)) { + auto rawSeed = hdSeed.RawSeed(); + file << strprintf("# HDSeed=%s fingerprint=%s", HexStr(rawSeed.begin(), rawSeed.end()), hdSeed.Fingerprint().GetHex()); + file << "\n"; + } else { + file << "# HDSeed unavailable (wallet locked, no HD seed, or hdchain unproven)\n"; + } } file << "\n"; for (std::vector >::const_iterator it = vKeyBirth.begin(); it != vKeyBirth.end(); it++) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e0cd0d926..ca29538ad 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2813,6 +2813,15 @@ bool CWallet::SetHDSeedFromMnemonic(const std::string& phrase) bool CWallet::GetHDSeedForDerivation(HDSeed& seedOut) const { + // Single choke point for every HD derivation in the wallet. When the hdchain record could not + // be trusted at load time we do not know whether fMnemonicSeed should be true, and guessing + // wrong derives into an entirely different key tree -- so refuse rather than guess. Callers + // surface this as a clean error (z_getnewaddress, sendmany, shieldcoinbase) or skip + // (autoshield). Transparent address generation falls back to the legacy random-key path, + // because hdChain.seedFp stays null and so IsHDTransparentEnabled() is false. + if (fHDChainUnproven) + return false; + HDSeed stored; if (!GetHDSeed(stored)) return false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cf77da83f..95527738b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -961,6 +961,11 @@ protected: /* the hd chain data model (chain counters) */ CHDChain hdChain; + //! Set at load time when hdChain cannot be trusted to describe this wallet's HD seed -- the + //! record was missing, or its seedFp does not match the seed actually loaded. While true the + //! wallet is fully usable for existing keys but refuses to DERIVE new ones, because it cannot + //! tell which key tree it belongs to. Never serialized; recomputed on every load. + bool fHDChainUnproven = false; public: /* @@ -1415,6 +1420,12 @@ public: void SetHDChain(const CHDChain& chain, bool memonly); const CHDChain& GetHDChain() const { return hdChain; } + //! Mark hdChain as untrustworthy for derivation (see the member's declaration). Set only by + //! CWalletDB::LoadWallet; there is deliberately no way to clear it short of reloading, so a + //! degraded wallet cannot be talked back into deriving without a real repair. + void SetHDChainUnproven() { fHDChainUnproven = true; } + bool IsHDChainUnproven() const { return fHDChainUnproven; } + /* Record (in memory and in wallet.dat) how this wallet's HD seed came to exist. Best-effort: a failed write is logged, not fatal — the next start simply re-classifies, and re-classification always errs toward diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 5537972fa..ffde7595b 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -421,6 +421,15 @@ public: // True when that record had to be repaired on read (see the "hdchain" case // in ReadKeyValue); LoadWallet rewrites it in full form afterwards. bool fHDChainRepaired; + // True once an "hdchain" record was ENCOUNTERED, whether or not it parsed. This is what + // separates the two failure shapes: a v1.0.3-or-earlier -salvagewallet drops the record + // entirely (its IsKeyType has no "hdchain" case), so absent == salvaged and recoverable, + // while present-but-unreadable means wider file damage. + bool fHDChainSeen; + // Fingerprint taken from the KEY of the hdseed/chdseed record. Available even for an + // encrypted wallet, where the seed itself cannot be read at load time. + bool fHDSeedSeen; + uint256 hdSeedFpSeen; CWalletScanState() { nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = nSapZAddrs = 0; @@ -429,6 +438,8 @@ public: nFileVersion = 0; fHDChainRead = false; fHDChainRepaired = false; + fHDChainSeen = false; + fHDSeedSeen = false; } }; @@ -836,6 +847,8 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, strErr = "Error reading wallet database: LoadHDSeed failed"; return false; } + wss.fHDSeedSeen = true; + wss.hdSeedFpSeen = seedFp; } else if (strType == "chdseed") { @@ -849,9 +862,15 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return false; } wss.fIsEncrypted = true; + // The fingerprint is the plaintext KEY of the chdseed record, so this works while + // the wallet is locked and the seed itself is unreadable. + wss.fHDSeedSeen = true; + wss.hdSeedFpSeen = seedFp; } else if (strType == "hdchain") { + // Record the ENCOUNTER before any parsing can fail. + wss.fHDChainSeen = true; CHDChain chain; // Keep an untouched copy: a failed >> has already consumed part of ssValue. CDataStream ssRetry(ssValue.begin(), ssValue.end(), ssValue.GetType(), ssValue.GetVersion()); @@ -1046,23 +1065,60 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) } } - // A wallet that holds an HD seed but whose hdchain record is missing or - // unreadable is NOT safe to run. hdChain would fall back to its SetNull - // defaults (walletdb.h:105-113), which (a) clears fMnemonicSeed, switching - // HD derivation from the 64-byte BIP39 seed to the raw 32-byte entropy - // (CWallet::GetHDSeedForDerivation, wallet.cpp:2615-2633) -> an entirely - // different key tree, and (b) resets saplingAccountCounter to 0, so the - // next GenerateNewSaplingZKey walks back over accounts that already exist. - // Both are silent today (a bad hdchain read is only DB_NONCRITICAL_ERROR). - // Fail loud instead of quietly deriving into the wrong tree. - if (pwallet->HaveHDSeed() && !wss.fHDChainRead) + // A wallet that holds an HD seed but no usable hdchain record cannot safely DERIVE: hdChain + // falls back to its SetNull defaults, clearing fMnemonicSeed and so switching derivation from + // the 64-byte BIP39 seed to the raw 32-byte entropy -- an entirely different key tree. + // + // saplingAccountCounter was previously listed here as a second hazard. It is not one: + // GenerateNewSaplingZKey loops `do {...} while (HaveSaplingSpendingKey(...))` and + // DeriveNewChildKey loops `while (HaveKey(...))`, so a counter that starts low walks forward + // past accounts that already exist rather than colliding with them. + // + // Two shapes reach here and only one is real corruption: + // (a) the record is ABSENT -- the signature of a -salvagewallet run by v1.0.3 or earlier, + // whose IsKeyType has no "hdchain" case, so salvage dropped it. The keys are intact. + // Refusing strands the wallet with no way back: the node aborts before the RPC server + // exists, and -mnemonic refuses a non-empty wallet, so there is no user-executable + // recovery path at all. + // (b) the record is PRESENT but unreadable -- wider file damage. Keep refusing. + if (pwallet->HaveHDSeed() && !wss.fHDChainRead && wss.fHDChainSeen) { - LogPrintf("Error loading wallet.dat: HD seed present but the hdchain record is missing or corrupt. " - "Recover by restoring from the seed phrase: move wallet.dat aside and start with " - "-mnemonic=\"\" -rescan\n"); + LogPrintf("Error loading wallet.dat: the hdchain record is present but unreadable. Your keys " + "are intact -- do NOT delete or replace wallet.dat. Back it up and see debug.log.\n"); return DB_CORRUPT; } + { + const char* pszDegraded = NULL; + if (pwallet->HaveHDSeed() && !wss.fHDChainRead) + { + pszDegraded = "the hdchain record is missing (an older -salvagewallet drops it)"; + } + // A chain WAS read, but it does not belong to the seed we loaded. No legitimate writer can + // produce that -- InstallHDSeed always stores seedFp = seed.Fingerprint(). It is the mark + // of a wallet that lost its hdchain to an old salvage and was then USED on that old build, + // which persists a SetNull-derived chain carrying a null seedFp. Such a wallet otherwise + // starts up perfectly clean and derives into the WRONG TREE forever, silently -- strictly + // worse than failing to open, which is why it is worth detecting here. + else if (pwallet->HaveHDSeed() && wss.fHDSeedSeen && + pwallet->GetHDChain().seedFp != wss.hdSeedFpSeen) + { + pszDegraded = "the hdchain record does not belong to this wallet's HD seed"; + } + + if (pszDegraded != NULL) + { + pwallet->SetHDChainUnproven(); + LogPrintf("Wallet opened in DEGRADED mode: %s. Existing keys are intact, spendable and " + "receivable, but no NEW HD-derived key can be generated and new transparent " + "addresses will not be recoverable from a seed phrase. Back up wallet.dat now " + "and do NOT delete or replace it.\n", pszDegraded); + // An old salvage also dropped defaultkey and bestblock, which would make this look like + // a first run and skip the rescan, leaving a permanently zero balance. + SoftSetBoolArg("-rescan", true); + } + } + // Any wallet corruption at all: skip any rewriting or // upgrading, we don't want to make it worse. if (result != DB_LOAD_OK)