fix(import): recognize real DragonX key formats in the import gate
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
// 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).
|
// it to the daemon (the dialog's indicator and this guard share isRecognizedImportKey).
|
||||||
if (!services::WalletSecurityController::isRecognizedImportKey(key)) {
|
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;
|
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).
|
// 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());
|
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]() {
|
return [this, err, addr, callback]() {
|
||||||
if (!err.empty()) {
|
if (!err.empty()) {
|
||||||
if (callback) callback(false, err, "");
|
if (callback) callback(false, err, "");
|
||||||
|
|||||||
@@ -1,9 +1,12 @@
|
|||||||
#include "wallet_security_controller.h"
|
#include "wallet_security_controller.h"
|
||||||
#include "../util/secure_vault.h"
|
#include "../util/secure_vault.h"
|
||||||
|
#include "../util/address_validation.h"
|
||||||
|
|
||||||
#include <cctype>
|
#include <cctype>
|
||||||
|
#include <cstdint>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
#include <utility>
|
#include <utility>
|
||||||
|
#include <vector>
|
||||||
|
|
||||||
namespace dragonx {
|
namespace dragonx {
|
||||||
namespace services {
|
namespace services {
|
||||||
@@ -108,18 +111,35 @@ WalletSecurityController::KeyKind WalletSecurityController::classifyPrivateKey(c
|
|||||||
|
|
||||||
bool WalletSecurityController::isViewingKey(const std::string& key)
|
bool WalletSecurityController::isViewingKey(const std::string& key)
|
||||||
{
|
{
|
||||||
// Sapling extended full viewing key (mainnet HRP "zxviews"; "zxview" also matches the prefix the
|
// DragonX's z_exportviewingkey returns a Sapling *incoming* viewing key (mainnet HRP "zivks");
|
||||||
// lite backend recognizes). Watch-only: reveals the address's funds but cannot spend them.
|
// z_importviewingkey only decodes that form. Recognize it structurally — a valid Bech32 checksum
|
||||||
return key.rfind("zxview", 0) == 0;
|
// 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)
|
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.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
|
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.
|
// Transparent WIF: decode Base58Check and confirm it is actually a secret key — version byte plus
|
||||||
if (key.size() >= 51 && key.size() <= 52 &&
|
// a 32-byte key, optionally a compression flag (payload 33 or 34 bytes). This accepts BOTH the
|
||||||
(key[0] == '5' || key[0] == 'K' || key[0] == 'L' || key[0] == 'U')) return true;
|
// 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<std::uint8_t> 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;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -74,7 +74,7 @@ public:
|
|||||||
std::size_t minLength = 4);
|
std::size_t minLength = 4);
|
||||||
static KeyKind classifyAddress(const std::string& address);
|
static KeyKind classifyAddress(const std::string& address);
|
||||||
static KeyKind classifyPrivateKey(const std::string& key);
|
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);
|
static bool isViewingKey(const std::string& key);
|
||||||
// True if `key` looks like a recognized Z (Sapling/Sprout spending) or T (WIF) private key.
|
// True if `key` looks like a recognized Z (Sapling/Sprout spending) or T (WIF) private key.
|
||||||
static bool isRecognizedPrivateKey(const std::string& key);
|
static bool isRecognizedPrivateKey(const std::string& key);
|
||||||
|
|||||||
@@ -76,7 +76,7 @@ std::vector<int> bech32HrpExpand(const std::string& hrp)
|
|||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
||||||
bool isValidBase58Check(const std::string& s)
|
bool decodeBase58Check(const std::string& s, std::vector<std::uint8_t>& payloadOut)
|
||||||
{
|
{
|
||||||
if (s.size() < 5 || s.size() > 256) return false;
|
if (s.size() < 5 || s.size() > 256) return false;
|
||||||
std::vector<std::uint8_t> data;
|
std::vector<std::uint8_t> data;
|
||||||
@@ -88,7 +88,15 @@ bool isValidBase58Check(const std::string& s)
|
|||||||
unsigned char h2[crypto_hash_sha256_BYTES];
|
unsigned char h2[crypto_hash_sha256_BYTES];
|
||||||
crypto_hash_sha256(h1, data.data(), payloadLen);
|
crypto_hash_sha256(h1, data.data(), payloadLen);
|
||||||
crypto_hash_sha256(h2, h1, sizeof(h1));
|
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<std::uint8_t> payload;
|
||||||
|
return decodeBase58Check(s, payload);
|
||||||
}
|
}
|
||||||
|
|
||||||
bool isValidBech32(const std::string& s)
|
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)
|
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<char>(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 util
|
||||||
} // namespace dragonx
|
} // namespace dragonx
|
||||||
|
|||||||
@@ -11,7 +11,9 @@
|
|||||||
|
|
||||||
#pragma once
|
#pragma once
|
||||||
|
|
||||||
|
#include <cstdint>
|
||||||
#include <string>
|
#include <string>
|
||||||
|
#include <vector>
|
||||||
|
|
||||||
namespace dragonx {
|
namespace dragonx {
|
||||||
namespace util {
|
namespace util {
|
||||||
@@ -20,9 +22,18 @@ namespace util {
|
|||||||
// (transparent R-addresses). Version-byte agnostic by design.
|
// (transparent R-addresses). Version-byte agnostic by design.
|
||||||
bool isValidBase58Check(const std::string& s);
|
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<std::uint8_t>& payloadOut);
|
||||||
|
|
||||||
// True if `s` is a valid Bech32 string (Sapling zs-addresses). The HRP is taken
|
// 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.
|
// from the string itself and folded into the checksum, so no HRP is hardcoded.
|
||||||
bool isValidBech32(const std::string& s);
|
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 util
|
||||||
} // namespace dragonx
|
} // namespace dragonx
|
||||||
|
|||||||
@@ -5952,6 +5952,66 @@ void testAddressChecksumValidation()
|
|||||||
EXPECT_FALSE(isValidBech32("abc1rzg")); // too short / bad checksum
|
EXPECT_FALSE(isValidBech32("abc1rzg")); // too short / bad checksum
|
||||||
EXPECT_FALSE(isValidBech32("Abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw")); // mixed case
|
EXPECT_FALSE(isValidBech32("Abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw")); // mixed case
|
||||||
EXPECT_FALSE(isValidBech32("nosalt")); // no separator
|
EXPECT_FALSE(isValidBech32("nosalt")); // no separator
|
||||||
|
|
||||||
|
// decodeBase58Check exposes the checksum-stripped payload so callers can inspect version/length.
|
||||||
|
using dragonx::util::decodeBase58Check;
|
||||||
|
std::vector<std::uint8_t> 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.
|
// Live probe of a real lite server (env-gated). Validates CONNECT_ONLY latency + IP capture.
|
||||||
@@ -7195,6 +7255,7 @@ int main()
|
|||||||
testHushChatShuffledReceive();
|
testHushChatShuffledReceive();
|
||||||
testWalletFileProbe();
|
testWalletFileProbe();
|
||||||
testAddressChecksumValidation();
|
testAddressChecksumValidation();
|
||||||
|
testPrivateKeyImportRecognition();
|
||||||
testLiteServerProbeLive();
|
testLiteServerProbeLive();
|
||||||
testXmrigLiveInstall();
|
testXmrigLiveInstall();
|
||||||
testGeneratedResourceBehavior();
|
testGeneratedResourceBehavior();
|
||||||
|
|||||||
Reference in New Issue
Block a user