From d603a54618c6f56ee0790a1b3e63da1aab206fc2 Mon Sep 17 00:00:00 2001 From: DanS Date: Sat, 8 Aug 2026 22:22:29 -0500 Subject: [PATCH] fix(import): recognize real DragonX key formats in the import gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client-side pre-check rejected legitimate keys before the daemon ever saw them, surfacing "Unrecognized key format" / a cryptic daemon "Invalid" error. Two concrete defects plus the brittle heuristic behind them: - Viewing keys: isViewingKey looked for Zcash's "zxview" extended-FVK prefix, but DragonX's z_exportviewingkey emits a Sapling *incoming* viewing key (HRP "zivks"), which z_importviewingkey is the only form the daemon decodes. Every real DragonX viewing key was refused. (F1) - Uncompressed transparent WIF: the length+first-char heuristic accepted {5,K,L,U} only, but a version-188 uncompressed key starts with '7'. (F2) Replace the heuristic with structural validation using the existing checksum validators (F3): add util::decodeBase58Check (checksum-stripped payload) and util::bech32Hrp (HRP of a valid Bech32 string). Transparent keys are now accepted by decoding Base58Check and checking the payload is a 33/34-byte secret key with a DragonX SECRET_KEY version byte (188 main/ regtest, 128 testnet) — covering compressed and uncompressed, rejecting addresses/typos by real checksum. Viewing keys are matched by the real incoming-VK HRPs (zivks / zivktestsapling / zivkregtestsapling). The Sweep gate and the dialog's live type indicator run off the same predicates, so they are fixed too (F4). Messaging now names the likely cause and appends a wrong-coin/network hint to the daemon's raw "Invalid" error (F5). Adds testPrivateKeyImportRecognition plus decodeBase58Check/bech32Hrp coverage; suite green (1/1). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/app_network.cpp | 12 +++- src/services/wallet_security_controller.cpp | 32 +++++++++-- src/services/wallet_security_controller.h | 2 +- src/util/address_validation.cpp | 25 ++++++++- src/util/address_validation.h | 11 ++++ tests/test_phase4.cpp | 61 +++++++++++++++++++++ 6 files changed, 133 insertions(+), 10 deletions(-) diff --git a/src/app_network.cpp b/src/app_network.cpp index d423a8b..cf84462 100644 --- a/src/app_network.cpp +++ b/src/app_network.cpp @@ -3834,7 +3834,9 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight, // Reject anything that isn't a recognized Z/T private key or shielded viewing key before handing // it to the daemon (the dialog's indicator and this guard share isRecognizedImportKey). if (!services::WalletSecurityController::isRecognizedImportKey(key)) { - if (callback) callback(false, "Unrecognized key format.", ""); + if (callback) callback(false, + "Not a recognized DragonX private key or viewing key. Check for missing or " + "mistyped characters, and that this is a DragonX key (not another coin).", ""); return; } @@ -3874,6 +3876,14 @@ void App::importPrivateKey(const std::string& rawKey, int startHeight, } // Scrub the worker's copy of the key now that the request has been sent (all paths). if (!key.empty()) sodium_memzero(&key[0], key.size()); + // A checksum-valid key the daemon still rejects is almost always the right *format* but the + // wrong network/coin (Komodo-family chains share version bytes) or a corrupted paste — say so, + // since the bare "Invalid …" text reads like a wallet bug (F5). + if (!err.empty() && err.find("Invalid") != std::string::npos && + err.find("DragonX") == std::string::npos) { + err += " — check the key is for DragonX (not another coin or network) and has no missing " + "or altered characters."; + } return [this, err, addr, callback]() { if (!err.empty()) { if (callback) callback(false, err, ""); diff --git a/src/services/wallet_security_controller.cpp b/src/services/wallet_security_controller.cpp index cf07b1f..29e30ad 100644 --- a/src/services/wallet_security_controller.cpp +++ b/src/services/wallet_security_controller.cpp @@ -1,9 +1,12 @@ #include "wallet_security_controller.h" #include "../util/secure_vault.h" +#include "../util/address_validation.h" #include +#include #include #include +#include namespace dragonx { namespace services { @@ -108,18 +111,35 @@ WalletSecurityController::KeyKind WalletSecurityController::classifyPrivateKey(c bool WalletSecurityController::isViewingKey(const std::string& key) { - // Sapling extended full viewing key (mainnet HRP "zxviews"; "zxview" also matches the prefix the - // lite backend recognizes). Watch-only: reveals the address's funds but cannot spend them. - return key.rfind("zxview", 0) == 0; + // DragonX's z_exportviewingkey returns a Sapling *incoming* viewing key (mainnet HRP "zivks"); + // z_importviewingkey only decodes that form. Recognize it structurally — a valid Bech32 checksum + // plus a known HRP — instead of a bare prefix, and cover testnet/regtest too. (The old check + // looked for Zcash's "zxview" extended-FVK HRP, which DragonX never emits, so every real viewing + // key was rejected client-side.) Watch-only: reveals the address's funds but cannot spend them. + const std::string hrp = util::bech32Hrp(key); + return hrp == "zivks" // mainnet + || hrp == "zivktestsapling" // testnet + || hrp == "zivkregtestsapling"; // regtest } bool WalletSecurityController::isRecognizedPrivateKey(const std::string& key) { + // Sapling z spending key (HRP "secret-extended-key-{main,test,regtest}"). These run ~300 chars, + // past the Bech32 length cap, so match by HRP prefix and let the daemon vet the payload. if (key.rfind("secret-extended-key-", 0) == 0) return true; // Sapling z spending key if (key.size() >= 2 && key[0] == 'S' && key[1] == 'K') return true; // Sprout z spending key - // Transparent WIF: base58, ~51-52 chars, common version prefixes. - if (key.size() >= 51 && key.size() <= 52 && - (key[0] == '5' || key[0] == 'K' || key[0] == 'L' || key[0] == 'U')) return true; + // Transparent WIF: decode Base58Check and confirm it is actually a secret key — version byte plus + // a 32-byte key, optionally a compression flag (payload 33 or 34 bytes). This accepts BOTH the + // compressed ("U…") and uncompressed ("7…") mainnet forms and the testnet form, and rejects + // addresses / typos via the real checksum — the old length+first-char heuristic dropped the + // uncompressed mainnet key (which starts with '7', not one of 5/K/L/U). + std::vector payload; + if (util::decodeBase58Check(key, payload) && + (payload.size() == 33 || payload.size() == 34) && + (payload[0] == 188 /* DragonX main/regtest SECRET_KEY */ || + payload[0] == 128 /* DragonX testnet SECRET_KEY */)) { + return true; + } return false; } diff --git a/src/services/wallet_security_controller.h b/src/services/wallet_security_controller.h index 239c3ba..da7cf14 100644 --- a/src/services/wallet_security_controller.h +++ b/src/services/wallet_security_controller.h @@ -74,7 +74,7 @@ public: std::size_t minLength = 4); static KeyKind classifyAddress(const std::string& address); static KeyKind classifyPrivateKey(const std::string& key); - // True if `key` is a shielded viewing key (extended full viewing key, "zxview…" — watch-only). + // True if `key` is a shielded viewing key (Sapling incoming viewing key, "zivks…" — watch-only). static bool isViewingKey(const std::string& key); // True if `key` looks like a recognized Z (Sapling/Sprout spending) or T (WIF) private key. static bool isRecognizedPrivateKey(const std::string& key); diff --git a/src/util/address_validation.cpp b/src/util/address_validation.cpp index cba64f1..eb87af8 100644 --- a/src/util/address_validation.cpp +++ b/src/util/address_validation.cpp @@ -76,7 +76,7 @@ std::vector bech32HrpExpand(const std::string& hrp) } // namespace -bool isValidBase58Check(const std::string& s) +bool decodeBase58Check(const std::string& s, std::vector& payloadOut) { if (s.size() < 5 || s.size() > 256) return false; std::vector data; @@ -88,7 +88,15 @@ bool isValidBase58Check(const std::string& s) unsigned char h2[crypto_hash_sha256_BYTES]; crypto_hash_sha256(h1, data.data(), payloadLen); crypto_hash_sha256(h2, h1, sizeof(h1)); - return std::memcmp(h2, data.data() + payloadLen, 4) == 0; + if (std::memcmp(h2, data.data() + payloadLen, 4) != 0) return false; + payloadOut.assign(data.begin(), data.begin() + payloadLen); + return true; +} + +bool isValidBase58Check(const std::string& s) +{ + std::vector payload; + return decodeBase58Check(s, payload); } bool isValidBech32(const std::string& s) @@ -127,5 +135,18 @@ bool isValidBech32(const std::string& s) return bech32Polymod(combined) == 1; // original Bech32 constant (Sapling, not Bech32m) } +std::string bech32Hrp(const std::string& s) +{ + if (!isValidBech32(s)) return {}; + // isValidBech32 already rejected mixed case and guaranteed a non-empty HRP before the + // final '1' separator, so lower-casing and splitting there recovers the HRP verbatim. + std::string lower(s); + std::transform(lower.begin(), lower.end(), lower.begin(), + [](unsigned char c) { return static_cast(std::tolower(c)); }); + const std::size_t sep = lower.rfind('1'); + if (sep == std::string::npos) return {}; + return lower.substr(0, sep); +} + } // namespace util } // namespace dragonx diff --git a/src/util/address_validation.h b/src/util/address_validation.h index bc17a33..885f8e1 100644 --- a/src/util/address_validation.h +++ b/src/util/address_validation.h @@ -11,7 +11,9 @@ #pragma once +#include #include +#include namespace dragonx { namespace util { @@ -20,9 +22,18 @@ namespace util { // (transparent R-addresses). Version-byte agnostic by design. bool isValidBase58Check(const std::string& s); +// Decodes `s` as Base58Check; on success returns true and fills `payloadOut` with the +// decoded bytes EXCLUDING the trailing 4-byte checksum (i.e. version byte + data). Lets +// callers inspect the version byte / payload length (e.g. to tell a WIF from an address). +bool decodeBase58Check(const std::string& s, std::vector& payloadOut); + // True if `s` is a valid Bech32 string (Sapling zs-addresses). The HRP is taken // from the string itself and folded into the checksum, so no HRP is hardcoded. bool isValidBech32(const std::string& s); +// Returns the (lower-cased) human-readable prefix of a valid Bech32 string, or "" if +// `s` is not valid Bech32. The HRP identifies the key/address type (e.g. "zivks"). +std::string bech32Hrp(const std::string& s); + } // namespace util } // namespace dragonx diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index 0b7516d..cecc138 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -5952,6 +5952,66 @@ void testAddressChecksumValidation() EXPECT_FALSE(isValidBech32("abc1rzg")); // too short / bad checksum EXPECT_FALSE(isValidBech32("Abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw")); // mixed case EXPECT_FALSE(isValidBech32("nosalt")); // no separator + + // decodeBase58Check exposes the checksum-stripped payload so callers can inspect version/length. + using dragonx::util::decodeBase58Check; + std::vector payload; + EXPECT_TRUE(decodeBase58Check("1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa", payload)); + EXPECT_EQ(payload.size(), (size_t)21); // version(1) + 20-byte hash160, checksum stripped + EXPECT_EQ((int)payload[0], 0); // mainnet P2PKH version byte + EXPECT_FALSE(decodeBase58Check("1A1zP1eP5QGefi2DMPTfTL5SLmv7Divfna", payload)); // bad checksum + + // bech32Hrp returns the (lower-cased) HRP of a valid string, or "" when invalid. + using dragonx::util::bech32Hrp; + EXPECT_EQ(bech32Hrp("abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw"), std::string("abcdef")); + EXPECT_EQ(bech32Hrp("A12UEL5L"), std::string("a")); // lower-cased + EXPECT_EQ(bech32Hrp("A12UEL5M"), std::string("")); // invalid → empty +} + +// Import-key recognition: the client gate must accept every real DragonX key form and reject +// non-keys, so it never blocks a valid import with "Unrecognized key format" (audit F1/F2/F3). +void testPrivateKeyImportRecognition() +{ + using dragonx::services::WalletSecurityController; + using KeyKind = WalletSecurityController::KeyKind; + + // --- Transparent WIF (DragonX SECRET_KEY version 188; testnet 128) --- + const std::string wifCompressed = "Up3W7uVYkLxCfH91APxjSpkkGBWJyBrm3tt1bCz64V5fpZK9ef3C"; // v188 compressed 'U' + const std::string wifUncompressed = "7JTPumX2kofLQdHKANy8MMLRkmXCmuJcosiv9f4RFqW9oCJXBHD"; // v188 uncompressed '7' + const std::string wifTestnet = "KwFfpDsaF7yxCELuyrH9gP5XL7TAt5b9HPWC1xCQbmrxvhJgMQHb"; // v128 compressed + + EXPECT_TRUE(WalletSecurityController::isRecognizedPrivateKey(wifCompressed)); + EXPECT_TRUE(WalletSecurityController::isRecognizedPrivateKey(wifUncompressed)); // F2 regression: '7' was rejected + EXPECT_TRUE(WalletSecurityController::isRecognizedPrivateKey(wifTestnet)); + EXPECT_TRUE(WalletSecurityController::isRecognizedImportKey(wifUncompressed)); + EXPECT_EQ(WalletSecurityController::classifyPrivateKey(wifUncompressed), KeyKind::Transparent); + EXPECT_FALSE(WalletSecurityController::isViewingKey(wifCompressed)); + + // A corrupted WIF (flipped last char) fails the checksum → caught locally, not at the daemon. + std::string wifBad = wifCompressed; + wifBad.back() = (wifBad.back() == 'C' ? 'D' : 'C'); + EXPECT_FALSE(WalletSecurityController::isRecognizedPrivateKey(wifBad)); + + // A transparent R-address is Base58Check-valid but NOT a key (21-byte payload, not 33/34). + EXPECT_FALSE(WalletSecurityController::isRecognizedPrivateKey("R9NXAVJezHiBnT3ijTpg3JUZre7PxhJWti")); + + // --- Sapling incoming viewing key (mainnet HRP "zivks") — the F1 regression --- + const std::string ivk = "zivks1qypqxpq9qcrsszg2pvxq6rs0zqg3yyc5z5tpwxqergd3c8g7rusq45amw7"; + EXPECT_TRUE(WalletSecurityController::isViewingKey(ivk)); + EXPECT_TRUE(WalletSecurityController::isRecognizedImportKey(ivk)); + EXPECT_FALSE(WalletSecurityController::isRecognizedPrivateKey(ivk)); // a viewing key can't spend + // The stale Zcash "zxview…" prefix is NOT a DragonX viewing key (and not valid Bech32 here). + EXPECT_FALSE(WalletSecurityController::isViewingKey("zxviews1abcdef")); + + // --- Sapling z spending key (recognized by HRP prefix; daemon vets the long payload) --- + const std::string zspend = "secret-extended-key-main1qxxxxxxxxxxxxxxxxxxxx"; + EXPECT_TRUE(WalletSecurityController::isRecognizedPrivateKey(zspend)); + EXPECT_TRUE(WalletSecurityController::isRecognizedImportKey(zspend)); + EXPECT_EQ(WalletSecurityController::classifyPrivateKey(zspend), KeyKind::Shielded); + + // --- Garbage / empty --- + EXPECT_FALSE(WalletSecurityController::isRecognizedImportKey("")); + EXPECT_FALSE(WalletSecurityController::isRecognizedImportKey("hello world")); } // Live probe of a real lite server (env-gated). Validates CONNECT_ONLY latency + IP capture. @@ -7195,6 +7255,7 @@ int main() testHushChatShuffledReceive(); testWalletFileProbe(); testAddressChecksumValidation(); + testPrivateKeyImportRecognition(); testLiteServerProbeLive(); testXmrigLiveInstall(); testGeneratedResourceBehavior();