From c3e81a5fa65c8e7c449a6ae7e9ce423b36a06c77 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 9 Aug 2026 00:12:17 -0500 Subject: [PATCH] fix(send): accept P2SH/multisig recipients in the send + URI address gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same defect class as the import-key fix: a hardcoded prefix/length pre-filter layered over the checksum validators rejected valid addresses before the daemon saw them. The send-screen recipient gate required a[0]=='R', and the payment-URI parser accepted only 'R'/'t' with rigid length bands — so every valid P2SH / multisig address (DragonX SCRIPT_ADDRESS=85 → 'b…') was silently refused, leaving the Send button disabled with no usable recipient. Centralize recipient recognition in util/address_validation: - isTransparentAddress: Base58Check with a 21-byte version+hash160 payload — covers P2PKH ('R…', v60) AND P2SH ('b…', v85) on every network, rejects WIF keys / typos by real checksum. - isShieldedAddress: Bech32 + a Sapling payment-address HRP (zs / ztestsapling / zregtestsapling), distinguishing a payment address from a viewing key. - isValidRecipientAddress: either of the above. send_tab's two validity helpers (the single choke point for all 5 call sites) and the payment-URI format check now route through these. The URI parser now checksum-validates the recipient (fail-fast on transcription errors) rather than being prefix/length-only. Tests use real checksummed vectors (P2PKH/P2SH/shielded, WIF- and typo-rejection); testPaymentUri updated off its old fake fixed-char addresses. Suite green (1/1). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/ui/windows/send_tab.cpp | 13 ++++++------- src/util/address_validation.cpp | 21 ++++++++++++++++++++ src/util/address_validation.h | 13 +++++++++++++ src/util/payment_uri.cpp | 20 ++++++------------- tests/test_phase4.cpp | 34 +++++++++++++++++++++++++++++++-- 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/src/ui/windows/send_tab.cpp b/src/ui/windows/send_tab.cpp index 11cc73f..c0cb5ab 100644 --- a/src/ui/windows/send_tab.cpp +++ b/src/ui/windows/send_tab.cpp @@ -148,14 +148,14 @@ static double GetAvailableBalance(App* app) { return 0.0; } -// Recipient validity = prefix/length pre-filter AND a real encoding-checksum check, so a -// transcription error that still matches the prefix/length is no longer labelled "Valid". -// The checksum verifiers are version-agnostic, so they never reject a genuine address. +// Recipient validity via the shared, structure-based recognizers: a real encoding-checksum check +// plus the actual DragonX address types — so a transcription error is never "Valid", and a valid +// P2SH/multisig ('b…') recipient is no longer dropped by a hardcoded 'R'-only prefix filter. static bool IsValidShieldedAddr(const char* a) { - return a[0] == 'z' && a[1] == 's' && strlen(a) > 60 && dragonx::util::isValidBech32(a); + return a && dragonx::util::isShieldedAddress(a); } static bool IsValidTransparentAddr(const char* a) { - return a[0] == 'R' && strlen(a) >= 34 && dragonx::util::isValidBase58Check(a); + return a && dragonx::util::isTransparentAddress(a); } static std::string timeAgo(int64_t timestamp) { @@ -1318,8 +1318,7 @@ void RenderSendTab(App* app) trimmed.erase(trimmed.begin()); while (!trimmed.empty() && (trimmed.back() == ' ' || trimmed.back() == '\n' || trimmed.back() == '\r' || trimmed.back() == '\t')) trimmed.pop_back(); - bool looksValid = (trimmed.size() > 30 && - ((trimmed[0] == 'z' && trimmed[1] == 's') || trimmed[0] == 'R')); + bool looksValid = dragonx::util::isValidRecipientAddress(trimmed); if (looksValid && s_to_address[0] == '\0') { s_preview_text = trimmed; s_paste_previewing = true; diff --git a/src/util/address_validation.cpp b/src/util/address_validation.cpp index eb87af8..6f3d6b6 100644 --- a/src/util/address_validation.cpp +++ b/src/util/address_validation.cpp @@ -148,5 +148,26 @@ std::string bech32Hrp(const std::string& s) return lower.substr(0, sep); } +bool isTransparentAddress(const std::string& s) +{ + std::vector payload; + // version byte (1) + hash160 (20) = 21 bytes, checksum stripped. Covers P2PKH ('R', v60) and + // P2SH/multisig ('b', v85); the daemon vets the exact version byte for the active network. + return decodeBase58Check(s, payload) && payload.size() == 21; +} + +bool isShieldedAddress(const std::string& s) +{ + const std::string hrp = bech32Hrp(s); + return hrp == "zs" // mainnet Sapling payment address + || hrp == "ztestsapling" // testnet + || hrp == "zregtestsapling"; // regtest +} + +bool isValidRecipientAddress(const std::string& s) +{ + return isTransparentAddress(s) || isShieldedAddress(s); +} + } // namespace util } // namespace dragonx diff --git a/src/util/address_validation.h b/src/util/address_validation.h index 885f8e1..ee3b365 100644 --- a/src/util/address_validation.h +++ b/src/util/address_validation.h @@ -35,5 +35,18 @@ bool isValidBech32(const std::string& s); // `s` is not valid Bech32. The HRP identifies the key/address type (e.g. "zivks"). std::string bech32Hrp(const std::string& s); +// True if `s` is a transparent (Base58Check) address — P2PKH *or* P2SH/multisig. Accepts any +// address whose payload is a 21-byte version+hash160, so it covers both the 'R…' (v60) and 'b…' +// (v85 script) forms on every DragonX network and rejects WIF keys / typos by checksum. Version-byte +// agnostic by design — a bare prefix check ('R' only) silently drops valid P2SH recipients. +bool isTransparentAddress(const std::string& s); + +// True if `s` is a shielded Sapling payment address (HRP "zs" / "ztestsapling" / "zregtestsapling"), +// with a valid Bech32 checksum. Distinguishes a payment address from a viewing key (e.g. "zivks…"). +bool isShieldedAddress(const std::string& s); + +// True if `s` is any address a payment can be sent to (transparent or shielded). +bool isValidRecipientAddress(const std::string& s); + } // namespace util } // namespace dragonx diff --git a/src/util/payment_uri.cpp b/src/util/payment_uri.cpp index f7ef8bb..bb835d3 100644 --- a/src/util/payment_uri.cpp +++ b/src/util/payment_uri.cpp @@ -3,6 +3,7 @@ // Released under the GPLv3 #include "payment_uri.h" +#include "address_validation.h" #include #include @@ -161,20 +162,11 @@ PaymentURI parsePaymentURI(const std::string& uri) return result; } - // Basic address format check. NOTE: this is format-only by design — the send flow - // checksum-validates the recipient (isValidBase58Check / shielded check) before broadcasting, - // so an invalid-checksum address parsed here can never actually be sent to. - bool validFormat = false; - - // z-address: starts with 'zs' and is 78+ chars - if (result.address[0] == 'z' && result.address.size() >= 78) { - validFormat = true; - } - // t-address: starts with 'R' (DragonX) or 't' (HUSH) and is ~34 chars - else if ((result.address[0] == 'R' || result.address[0] == 't') && - result.address.size() >= 26 && result.address.size() <= 36) { - validFormat = true; - } + // Address format check via the shared, structure-based recognizers (checksum + real DragonX + // address types). This accepts shielded ("zs…"), P2PKH ("R…") and P2SH/multisig ("b…") forms — + // the old prefix/length heuristic rejected P2SH and hardcoded a 't' prefix DragonX never emits. + const bool validFormat = isShieldedAddress(result.address) || + isTransparentAddress(result.address); if (!validFormat) { result.error = "Invalid address format"; diff --git a/tests/test_phase4.cpp b/tests/test_phase4.cpp index cecc138..810398b 100644 --- a/tests/test_phase4.cpp +++ b/tests/test_phase4.cpp @@ -726,7 +726,9 @@ void testConnectionConfig() void testPaymentUri() { - std::string taddr = "R" + std::string(33, 'a'); + // Real checksummed addresses — the parser now checksum-validates the recipient (not a bare + // prefix/length filter), so it accepts P2PKH / P2SH / shielded and rejects transcription errors. + std::string taddr = "R9NXAVJezHiBnT3ijTpg3JUZre7PxhJWti"; // P2PKH (v60) auto parsed = dragonx::util::parsePaymentURI( "drgx:" + taddr + "?amount=1.25000000&label=Main+Wallet&memo=hello%20there&message=thanks"); @@ -737,11 +739,19 @@ void testPaymentUri() EXPECT_EQ(parsed.memo, std::string("hello there")); EXPECT_EQ(parsed.message, std::string("thanks")); - std::string zaddr = "zs" + std::string(76, 'b'); + std::string zaddr = "zs1qqqsyqcyq5rqwzqfpg9scrgwpugpzysnzs23v9ccrydpk8qarc0jqgfzyvjz2f389q5j5ctfvp5"; auto zparsed = dragonx::util::parsePaymentURI("hush://" + zaddr + "?amt=0.5"); EXPECT_TRUE(zparsed.valid); EXPECT_NEAR(zparsed.amount, 0.5, 0.00000001); + // Regression: a P2SH/multisig recipient ("b…", v85) must parse — the old 'R'/'t'-only filter dropped it. + auto p2sh = dragonx::util::parsePaymentURI("drgx:bCpbnCkrjoJ6EHXtLx9eASHEbFYyikt35C?amount=1"); + EXPECT_TRUE(p2sh.valid); + + // A transcription error (flipped checksum char) is now rejected at parse time. + auto typo = dragonx::util::parsePaymentURI("drgx:R9NXAVJezHiBnT3ijTpg3JUZre7PxhJWtX?amount=1"); + EXPECT_FALSE(typo.valid); + auto invalid = dragonx::util::parsePaymentURI("drgx:" + taddr + "?amount=-1"); EXPECT_FALSE(invalid.valid); EXPECT_EQ(invalid.error, std::string("Invalid negative amount")); @@ -5966,6 +5976,26 @@ void testAddressChecksumValidation() EXPECT_EQ(bech32Hrp("abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw"), std::string("abcdef")); EXPECT_EQ(bech32Hrp("A12UEL5L"), std::string("a")); // lower-cased EXPECT_EQ(bech32Hrp("A12UEL5M"), std::string("")); // invalid → empty + + // Address type recognizers: accept every real DragonX recipient form, reject non-addresses. + using dragonx::util::isTransparentAddress; + using dragonx::util::isShieldedAddress; + using dragonx::util::isValidRecipientAddress; + const std::string p2pkh = "R9NXAVJezHiBnT3ijTpg3JUZre7PxhJWti"; // v60 + const std::string p2sh = "bCpbnCkrjoJ6EHXtLx9eASHEbFYyikt35C"; // v85 multisig — the regression + const std::string zaddr = "zs1qqqsyqcyq5rqwzqfpg9scrgwpugpzysnzs23v9ccrydpk8qarc0jqgfzyvjz2f389q5j5ctfvp5"; + EXPECT_TRUE(isTransparentAddress(p2pkh)); + EXPECT_TRUE(isTransparentAddress(p2sh)); // was silently dropped by the old 'R'-only filter + EXPECT_FALSE(isShieldedAddress(p2pkh)); + EXPECT_TRUE(isShieldedAddress(zaddr)); + EXPECT_FALSE(isTransparentAddress(zaddr)); + EXPECT_TRUE(isValidRecipientAddress(p2pkh)); + EXPECT_TRUE(isValidRecipientAddress(p2sh)); + EXPECT_TRUE(isValidRecipientAddress(zaddr)); + // A WIF spending key is NOT a recipient (33/34-byte payload, not 21); nor is a typo'd address. + EXPECT_FALSE(isTransparentAddress("Up3W7uVYkLxCfH91APxjSpkkGBWJyBrm3tt1bCz64V5fpZK9ef3C")); + EXPECT_FALSE(isValidRecipientAddress("R9NXAVJezHiBnT3ijTpg3JUZre7PxhJWtX")); // flipped checksum char + EXPECT_FALSE(isValidRecipientAddress("")); } // Import-key recognition: the client gate must accept every real DragonX key form and reject