From 8f9629bddd0e0c28327e81dffa636626fc929f2a Mon Sep 17 00:00:00 2001 From: russeree Date: Thu, 26 Sep 2024 15:25:08 +0100 Subject: [PATCH 1/4] Added chaintype user-facing string display Co-authored-by: D++ <82842780+dplusplus1024@users.noreply.github.com> --- src/kernel/chainparams.h | 4 +++- src/util/chaintype.cpp | 17 +++++++++++++++++ src/util/chaintype.h | 2 ++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index f7ea141c37c..fd4b62d4482 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -105,10 +105,12 @@ public: uint64_t PruneAfterHeight() const { return nPruneAfterHeight; } /** Minimum free space (in GB) needed for data directory */ uint64_t AssumedBlockchainSize() const { return m_assumed_blockchain_size; } - /** Minimum free space (in GB) needed for data directory when pruned; Does not include prune target*/ + /** Minimum free space (in GB) needed for data directory when pruned; Does not include prune target */ uint64_t AssumedChainStateSize() const { return m_assumed_chain_state_size; } /** Whether it is possible to mine blocks on demand (no retargeting) */ bool MineBlocksOnDemand() const { return consensus.fPowNoRetargeting; } + /** Return the chain type as a user-facing string */ + std::string GetChainTypeDisplayString() const { return ChainTypeToDisplayString(m_chain_type); } /** Return the chain type string */ std::string GetChainTypeString() const { return ChainTypeToString(m_chain_type); } /** Return the chain type */ diff --git a/src/util/chaintype.cpp b/src/util/chaintype.cpp index 272466e7afc..9435b6b9c49 100644 --- a/src/util/chaintype.cpp +++ b/src/util/chaintype.cpp @@ -41,3 +41,20 @@ std::optional ChainTypeFromString(std::string_view chain) return std::nullopt; } } + +std::string ChainTypeToDisplayString(ChainType chain) +{ + switch (chain) { + case ChainType::MAIN: + return "Bitcoin"; + case ChainType::TESTNET: + return "testnet"; + case ChainType::TESTNET4: + return "testnet4"; + case ChainType::SIGNET: + return "signet"; + case ChainType::REGTEST: + return "regtest"; + } + assert(false); +} diff --git a/src/util/chaintype.h b/src/util/chaintype.h index 2fe734b64a5..5ba48e17476 100644 --- a/src/util/chaintype.h +++ b/src/util/chaintype.h @@ -16,6 +16,8 @@ enum class ChainType { TESTNET4, }; +std::string ChainTypeToDisplayString(ChainType chain); + std::string ChainTypeToString(ChainType chain); std::optional ChainTypeFromString(std::string_view chain); From 3fda88d78ea5e66881d81262634cedc336fa885a Mon Sep 17 00:00:00 2001 From: Reese Russell Date: Sun, 29 Sep 2024 05:32:37 +0000 Subject: [PATCH 2/4] Include Base58 encoded prefixes in chainparams --- src/kernel/chainparams.cpp | 30 ++++++++++++++++++++++++++++++ src/kernel/chainparams.h | 2 ++ 2 files changed, 32 insertions(+) diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 349e14a5d6e..d60f6de6745 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -160,6 +160,12 @@ public: base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x88, 0xB2, 0x1E}; base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x88, 0xAD, 0xE4}; + base58EncodedPrefixes[PUBKEY_ADDRESS] = {"1"}; + base58EncodedPrefixes[SCRIPT_ADDRESS] = {"3"}; + base58EncodedPrefixes[SECRET_KEY] = {"5","K","L"}; + base58EncodedPrefixes[EXT_PUBLIC_KEY] = {"xpub"}; + base58EncodedPrefixes[EXT_SECRET_KEY] = {"xpriv"}; + bech32_hrp = "bc"; vFixedSeeds = std::vector(std::begin(chainparams_seed_main), std::end(chainparams_seed_main)); @@ -273,6 +279,12 @@ public: base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF}; base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94}; + base58EncodedPrefixes[PUBKEY_ADDRESS] = {"m","n"}; + base58EncodedPrefixes[SCRIPT_ADDRESS] = {"2"}; + base58EncodedPrefixes[SECRET_KEY] = {"9","c"}; + base58EncodedPrefixes[EXT_PUBLIC_KEY] = {"tpub"}; + base58EncodedPrefixes[EXT_SECRET_KEY] = {"tpriv"}; + bech32_hrp = "tb"; vFixedSeeds = std::vector(std::begin(chainparams_seed_test), std::end(chainparams_seed_test)); @@ -377,6 +389,12 @@ public: base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF}; base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94}; + base58EncodedPrefixes[PUBKEY_ADDRESS] = {"m","n"}; + base58EncodedPrefixes[SCRIPT_ADDRESS] = {"2"}; + base58EncodedPrefixes[SECRET_KEY] = {"9","c"}; + base58EncodedPrefixes[EXT_PUBLIC_KEY] = {"tpub"}; + base58EncodedPrefixes[EXT_SECRET_KEY] = {"tpriv"}; + bech32_hrp = "tb"; vFixedSeeds = std::vector(std::begin(chainparams_seed_testnet4), std::end(chainparams_seed_testnet4)); @@ -511,6 +529,12 @@ public: base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF}; base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94}; + base58EncodedPrefixes[PUBKEY_ADDRESS] = {"m","n"}; + base58EncodedPrefixes[SCRIPT_ADDRESS] = {"2"}; + base58EncodedPrefixes[SECRET_KEY] = {"9","c"}; + base58EncodedPrefixes[EXT_PUBLIC_KEY] = {"tpub"}; + base58EncodedPrefixes[EXT_SECRET_KEY] = {"tpriv"}; + bech32_hrp = "tb"; fDefaultConsistencyChecks = false; @@ -648,6 +672,12 @@ public: base58Prefixes[EXT_PUBLIC_KEY] = {0x04, 0x35, 0x87, 0xCF}; base58Prefixes[EXT_SECRET_KEY] = {0x04, 0x35, 0x83, 0x94}; + base58EncodedPrefixes[PUBKEY_ADDRESS] = {"m","n"}; + base58EncodedPrefixes[SCRIPT_ADDRESS] = {"2"}; + base58EncodedPrefixes[SECRET_KEY] = {"9","c"}; + base58EncodedPrefixes[EXT_PUBLIC_KEY] = {"tpub"}; + base58EncodedPrefixes[EXT_SECRET_KEY] = {"tpriv"}; + bech32_hrp = "bcrt"; } }; diff --git a/src/kernel/chainparams.h b/src/kernel/chainparams.h index fd4b62d4482..f1f6959e793 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -118,6 +118,7 @@ public: /** Return the list of hostnames to look up for DNS seeds */ const std::vector& DNSSeeds() const { return vSeeds; } const std::vector& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } + const std::vector& Base58EncodedPrefix(Base58Type type) const { return base58EncodedPrefixes[type]; } const std::string& Bech32HRP() const { return bech32_hrp; } const std::vector& FixedSeeds() const { return vFixedSeeds; } const CCheckpointData& Checkpoints() const { return checkpointData; } @@ -177,6 +178,7 @@ protected: uint64_t m_assumed_chain_state_size; std::vector vSeeds; std::vector base58Prefixes[MAX_BASE58_TYPES]; + std::vector base58EncodedPrefixes[MAX_BASE58_TYPES]; std::string bech32_hrp; ChainType m_chain_type; CBlock genesis; From 6e7cf4c583b5a67c60c7fbc6cbd4f3ce84075bf8 Mon Sep 17 00:00:00 2001 From: Reese Russell Date: Sun, 23 Feb 2025 06:03:00 +0000 Subject: [PATCH 3/4] Base58 decoding logic + bech32 decoding network awareness --- src/key_io.cpp | 51 +++++++++++++++---- .../functional/rpc_invalid_address_message.py | 34 ++++++------- test/functional/rpc_validateaddress.py | 46 +++++------------ test/functional/wallet_basic.py | 2 +- 4 files changed, 73 insertions(+), 60 deletions(-) diff --git a/src/key_io.cpp b/src/key_io.cpp index 6cece47e410..444f8065b72 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -83,14 +83,22 @@ public: CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector* error_locations) { - std::vector data; - uint160 hash; error_str = ""; - // Note this will be false if it is a valid Bech32 address for a different network - bool is_bech32 = (ToLower(str.substr(0, params.Bech32HRP().size())) == params.Bech32HRP()); + static const uint8_t MAX_BASE58_CHARS = 100; + static const uint8_t MAX_BASE58_CHECK_CHARS = 21; - if (!is_bech32 && DecodeBase58Check(str, data, 21)) { + std::vector data, bech32_data; + + auto [bech32_encoding, bech32_hrp, bech32_chars] = bech32::Decode(str); + auto [bech32_error, bech32_error_loc] = bech32::LocateErrors(str); + bool is_bech32 = bech32_encoding != bech32::Encoding::INVALID; + + auto check_base58 = [&]() { return DecodeBase58Check(str, data, MAX_BASE58_CHECK_CHARS); }; + + + if (!is_bech32 && check_base58()) { + uint160 hash; // base58-encoded Bitcoin addresses. // Public-key-hash-addresses have version 0 (or 111 testnet). // The data vector contains RIPEMD160(SHA256(pubkey)), where pubkey is the serialized public key. @@ -114,16 +122,41 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin()))) { error_str = "Invalid length for Base58 address (P2PKH or P2SH)"; } else { - error_str = "Invalid or unsupported Base58-encoded address."; + std::vector encoded_prefixes; + const std::vector& pubkey_prefixes = params.Base58EncodedPrefix(CChainParams::PUBKEY_ADDRESS); + const std::vector& script_prefixes = params.Base58EncodedPrefix(CChainParams::SCRIPT_ADDRESS); + encoded_prefixes.insert(encoded_prefixes.end(), script_prefixes.begin(), script_prefixes.end()); + encoded_prefixes.insert(encoded_prefixes.end(), pubkey_prefixes.begin(), pubkey_prefixes.end()); + + std::string base58_address_prefixes; + for (size_t i = 0; i < encoded_prefixes.size(); ++i) { + if (i > 0) { + base58_address_prefixes += (i == encoded_prefixes.size() - 1) ? ", or " : ", "; + } + base58_address_prefixes += std::string(encoded_prefixes[i]); + } + error_str = strprintf("Invalid Base58 %s address. Expected prefix %s", params.GetChainTypeDisplayString(), base58_address_prefixes); } return CNoDestination(); } else if (!is_bech32) { + bool is_base58 = DecodeBase58(str, data, MAX_BASE58_CHARS); // Try Base58 decoding without the checksum, using a much larger max length - if (!DecodeBase58(str, data, 100)) { - error_str = "Invalid or unsupported Segwit (Bech32) or Base58 encoding."; + if (!is_base58) { + // If bech32 decoding failed due to invalid base32 chars, address format is ambiguous; otherwise, report bech32 error + bool is_validBech32Chars = (bech32_error != "Invalid Base 32 character"); + error_str = is_validBech32Chars ? "Bech32(m) address decoded with error: " + bech32_error : "Address is not valid Base58 or Bech32"; } else { + if (bech32_error == "Invalid character or mixed case") { + error_str = "Invalid checksum or length of Base58 address (P2PKH or P2SH)"; + } + // This covers the case where an address is encoded as valid base58 and invalid bech32(m) due to a non base32 error error_str = "Invalid checksum or length of Base58 address (P2PKH or P2SH)"; } + + if (error_locations) { + *error_locations = std::move(bech32_error_loc); + } + return CNoDestination(); } @@ -136,7 +169,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par } // Bech32 decoding if (dec.hrp != params.Bech32HRP()) { - error_str = strprintf("Invalid or unsupported prefix for Segwit (Bech32) address (expected %s, got %s).", params.Bech32HRP(), dec.hrp); + error_str = strprintf("Invalid or unsupported prefix for Segwit (Bech32) address (expected %s, got %s)", params.Bech32HRP(), dec.hrp); return CNoDestination(); } int version = dec.data[0]; // The first 5 bit symbol is the witness version (0-16) diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index 07795534a0b..f9924862f94 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -65,19 +65,19 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): def test_validateaddress(self): # Invalid Bech32 self.check_invalid(BECH32_INVALID_SIZE, "Invalid Bech32 address program size (41 bytes)") - self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid or unsupported Segwit (Bech32) or Base58 encoding.') + self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid or unsupported prefix for Segwit (Bech32) address (expected bcrt, got bc)') self.check_invalid(BECH32_INVALID_BECH32, 'Version 1+ witness address must use Bech32m checksum') self.check_invalid(BECH32_INVALID_BECH32M, 'Version 0 witness address must use Bech32 checksum') self.check_invalid(BECH32_INVALID_VERSION, 'Invalid Bech32 address witness version') self.check_invalid(BECH32_INVALID_V0_SIZE, "Invalid Bech32 v0 address program size (21 bytes), per BIP141") - self.check_invalid(BECH32_TOO_LONG, 'Bech32 string too long', list(range(90, 108))) - self.check_invalid(BECH32_ONE_ERROR, 'Invalid Bech32 checksum', [9]) - self.check_invalid(BECH32_TWO_ERRORS, 'Invalid Bech32 checksum', [22, 43]) - self.check_invalid(BECH32_ONE_ERROR_CAPITALS, 'Invalid Bech32 checksum', [38]) - self.check_invalid(BECH32_NO_SEPARATOR, 'Missing separator') - self.check_invalid(BECH32_INVALID_CHAR, 'Invalid Base 32 character', [8]) - self.check_invalid(BECH32_MULTISIG_TWO_ERRORS, 'Invalid Bech32 checksum', [19, 30]) - self.check_invalid(BECH32_WRONG_VERSION, 'Invalid Bech32 checksum', [5]) + self.check_invalid(BECH32_TOO_LONG, 'Bech32(m) address decoded with error: Bech32 string too long', list(range(90, 108))) + self.check_invalid(BECH32_ONE_ERROR, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [9]) + self.check_invalid(BECH32_TWO_ERRORS, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [22, 43]) + self.check_invalid(BECH32_ONE_ERROR_CAPITALS, 'Invalid checksum or length of Base58 address (P2PKH or P2SH)', [38]) + self.check_invalid(BECH32_NO_SEPARATOR, 'Bech32(m) address decoded with error: Missing separator') + self.check_invalid(BECH32_INVALID_CHAR, 'Address is not valid Base58 or Bech32', [8]) + self.check_invalid(BECH32_MULTISIG_TWO_ERRORS, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [19, 30]) + self.check_invalid(BECH32_WRONG_VERSION, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [5]) # Valid Bech32 self.check_valid(BECH32_VALID) @@ -86,16 +86,16 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): self.check_valid(BECH32_VALID_MULTISIG) # Invalid Base58 - self.check_invalid(BASE58_INVALID_PREFIX, 'Invalid or unsupported Base58-encoded address.') - self.check_invalid(BASE58_INVALID_CHECKSUM, 'Invalid checksum or length of Base58 address (P2PKH or P2SH)') - self.check_invalid(BASE58_INVALID_LENGTH, 'Invalid checksum or length of Base58 address (P2PKH or P2SH)') + self.check_invalid(BASE58_INVALID_PREFIX, 'Invalid Base58 regtest address. Expected prefix 2, m, or n') + self.check_invalid(BASE58_INVALID_CHECKSUM, 'Invalid checksum or length of Base58 address (P2PKH or P2SH)', [4, 6, 10, 12, 16, 25, 29, 30, 31]) + self.check_invalid(BASE58_INVALID_LENGTH, 'Invalid checksum or length of Base58 address (P2PKH or P2SH)', [3, 8, 9, 11, 15, 16, 18, 19, 21, 22, 23, 27, 39, 40, 41, 44, 46, 47]) # Valid Base58 self.check_valid(BASE58_VALID) # Invalid address format - self.check_invalid(INVALID_ADDRESS, 'Invalid or unsupported Segwit (Bech32) or Base58 encoding.') - self.check_invalid(INVALID_ADDRESS_2, 'Invalid or unsupported Segwit (Bech32) or Base58 encoding.') + self.check_invalid(INVALID_ADDRESS, 'Bech32(m) address decoded with error: Invalid separator position' , [14]) + self.check_invalid(INVALID_ADDRESS_2, 'Bech32(m) address decoded with error: Invalid separator position', [0]) node = self.nodes[0] @@ -108,9 +108,9 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): node = self.nodes[0] assert_raises_rpc_error(-5, "Invalid Bech32 address program size (41 bytes)", node.getaddressinfo, BECH32_INVALID_SIZE) - assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, BECH32_INVALID_PREFIX) - assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", node.getaddressinfo, BASE58_INVALID_PREFIX) - assert_raises_rpc_error(-5, "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", node.getaddressinfo, INVALID_ADDRESS) + assert_raises_rpc_error(-5, "Invalid or unsupported prefix for Segwit (Bech32) address (expected bcrt, got bc)", node.getaddressinfo, BECH32_INVALID_PREFIX) + assert_raises_rpc_error(-5, "Invalid Base58 regtest address. Expected prefix 2, m, or n", node.getaddressinfo, BASE58_INVALID_PREFIX) + assert_raises_rpc_error(-5, "Bech32(m) address decoded with error: Invalid separator position", node.getaddressinfo, INVALID_ADDRESS) assert "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS) def run_test(self): diff --git a/test/functional/rpc_validateaddress.py b/test/functional/rpc_validateaddress.py index bf094a7df89..11b5c0212e8 100755 --- a/test/functional/rpc_validateaddress.py +++ b/test/functional/rpc_validateaddress.py @@ -12,10 +12,10 @@ INVALID_DATA = [ # BIP 173 ( "tc1qw508d6qejxtdg4y5r3zarvary0c5xw7kg3g4ty", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # Invalid hrp + "Invalid or unsupported prefix for Segwit (Bech32) address (expected bc, got tc)", # Invalid hrp [], ), - ("bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t5", "Invalid Bech32 checksum", [41]), + ("bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t5", "Bech32(m) address decoded with error: Invalid Bech32 checksum", [41]), ( "BC13W508D6QEJXTDG4Y5R3ZARVARY0C5XW7KN40WF2", "Version 1+ witness address must use Bech32m checksum", @@ -38,12 +38,12 @@ INVALID_DATA = [ ), ( "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3q0sL5k7", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Mixed case - [], + "Bech32(m) address decoded with error: Invalid character or mixed case", # tb1, Mixed case + [58], ), ( "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3t4", - "Invalid character or mixed case", # bc1, Mixed case, not in BIP 173 test vectors + "Bech32(m) address decoded with error: Invalid character or mixed case", # bc1, Mixed case, not in BIP 173 test vectors [40], ), ( @@ -52,15 +52,15 @@ INVALID_DATA = [ [], ), ( - "tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3pjxtptv", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Non-zero padding in 8-to-5 conversion + "bc1qfysxzmfq2phhyarvv9hxgtjgdajxcw3fpkpph893lw", + "Invalid padding in Bech32 data section", # tb1, Non-zero padding in 8-to-5 conversion [], ), ("bc1gmk9yu", "Empty Bech32 data section", []), # BIP 350 ( "tc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq5zuyut", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # Invalid human-readable part + "Invalid or unsupported prefix for Segwit (Bech32) address (expected bc, got tc)", # Invalid human-readable part [], ), ( @@ -69,29 +69,19 @@ INVALID_DATA = [ [], ), ( - "tb1z0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqglt7rf", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Invalid checksum (Bech32 instead of Bech32m) - [], - ), - ( - "BC1S0XLXVLHEMJA6C4DQV22UAPCTQUPFHLXM9H8Z3K2E72Q4K9HCZ7VQ54WELL", + "BC1PFYSXZMFQ2PHHYARVV9HXGTJGFAZYCWJYPQQQF9ZLQ6", "Version 1+ witness address must use Bech32m checksum", # Invalid checksum (Bech32 instead of Bech32m) [], ), ( - "bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kemeawh", + "bc1qfysxzmfq2phhyarvv9hxgtjgfazycwjypqqqfm706a", "Version 0 witness address must use Bech32 checksum", # Invalid checksum (Bech32m instead of Bech32) [], ), ( - "tb1q0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq24jc47", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Invalid checksum (Bech32m instead of Bech32) - [], - ), - ( - "bc1p38j9r5y49hruaue7wxjce0updqjuyyx0kh56v8s25huc6995vvpql3jow4", - "Invalid Base 32 character", # Invalid character in checksum - [59], + "Address is not valid Base58 or Bech32 ", + "Bech32(m) address decoded with error: Invalid character or mixed case", # Invalid character in checksum + [x for x in range(1, 21)] + [22, 23, 24] + [27, 28, 29, 30] + [32, 33, 34] + [37], ), ( "BC130XLXVLHEMJA6C4DQV22UAPCTQUPFHLXM9H8Z3K2E72Q4K9HCZ7VQ7ZWS8R", @@ -109,21 +99,11 @@ INVALID_DATA = [ "Invalid Bech32 v0 address program size (16 bytes), per BIP141", [], ), - ( - "tb1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq47Zagq", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Mixed case - [], - ), ( "bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v07qwwzcrf", "Invalid padding in Bech32 data section", # zero padding of more than 4 bits [], ), - ( - "tb1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vpggkg4j", - "Invalid or unsupported Segwit (Bech32) or Base58 encoding.", # tb1, Non-zero padding in 8-to-5 conversion - [], - ), ("bc1gmk9yu", "Empty Bech32 data section", []), ] VALID_DATA = [ diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index c968e4333a0..0cfde123477 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -650,7 +650,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999))) # Test getaddressinfo on external address. Note that these addresses are taken from disablewallet.py - assert_raises_rpc_error(-5, "Invalid or unsupported Base58-encoded address.", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy") + assert_raises_rpc_error(-5, "Invalid Base58 regtest address", self.nodes[0].getaddressinfo, "3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy") address_info = self.nodes[0].getaddressinfo("mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ") assert_equal(address_info['address'], "mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ") assert_equal(address_info["scriptPubKey"], "76a9144e3854046c7bd1594ac904e4793b6a45b36dea0988ac") From f974d465d738090d7391e98ce2e5b0cdecba1376 Mon Sep 17 00:00:00 2001 From: Reese Russell Date: Sun, 23 Feb 2025 08:51:49 +0000 Subject: [PATCH 4/4] Bech32 decoding logic --- src/key_io.cpp | 52 +++++++++---------- .../functional/rpc_invalid_address_message.py | 14 ++--- test/functional/rpc_validateaddress.py | 8 +-- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/src/key_io.cpp b/src/key_io.cpp index 444f8065b72..bc65ff9badf 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -95,7 +95,6 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par bool is_bech32 = bech32_encoding != bech32::Encoding::INVALID; auto check_base58 = [&]() { return DecodeBase58Check(str, data, MAX_BASE58_CHECK_CHARS); }; - if (!is_bech32 && check_base58()) { uint160 hash; @@ -124,7 +123,7 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par } else { std::vector encoded_prefixes; const std::vector& pubkey_prefixes = params.Base58EncodedPrefix(CChainParams::PUBKEY_ADDRESS); - const std::vector& script_prefixes = params.Base58EncodedPrefix(CChainParams::SCRIPT_ADDRESS); + const std::vector& script_prefixes = params.Base58EncodedPrefix(CChainParams::SCRIPT_ADDRESS); encoded_prefixes.insert(encoded_prefixes.end(), script_prefixes.begin(), script_prefixes.end()); encoded_prefixes.insert(encoded_prefixes.end(), pubkey_prefixes.begin(), pubkey_prefixes.end()); @@ -160,61 +159,59 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return CNoDestination(); } - data.clear(); - const auto dec = bech32::Decode(str); - if (dec.encoding == bech32::Encoding::BECH32 || dec.encoding == bech32::Encoding::BECH32M) { - if (dec.data.empty()) { + if (bech32_encoding == bech32::Encoding::BECH32 || bech32_encoding == bech32::Encoding::BECH32M) { + if (bech32_chars.empty()) { error_str = "Empty Bech32 data section"; return CNoDestination(); } // Bech32 decoding - if (dec.hrp != params.Bech32HRP()) { - error_str = strprintf("Invalid or unsupported prefix for Segwit (Bech32) address (expected %s, got %s)", params.Bech32HRP(), dec.hrp); + if (bech32_hrp != params.Bech32HRP()) { + error_str = strprintf("Invalid or unsupported prefix for Segwit (Bech32) %s address (expected %s, got %s)", params.GetChainTypeDisplayString(), params.Bech32HRP(), bech32_hrp); return CNoDestination(); } - int version = dec.data[0]; // The first 5 bit symbol is the witness version (0-16) - if (version == 0 && dec.encoding != bech32::Encoding::BECH32) { + int version = bech32_chars[0]; // The first 5 bit symbol is the witness version (0-16) + if (version == 0 && bech32_encoding != bech32::Encoding::BECH32) { error_str = "Version 0 witness address must use Bech32 checksum"; return CNoDestination(); } - if (version != 0 && dec.encoding != bech32::Encoding::BECH32M) { + if (version != 0 && bech32_encoding != bech32::Encoding::BECH32M) { error_str = "Version 1+ witness address must use Bech32m checksum"; return CNoDestination(); } // The rest of the symbols are converted witness program bytes. - data.reserve(((dec.data.size() - 1) * 5) / 8); - if (ConvertBits<5, 8, false>([&](unsigned char c) { data.push_back(c); }, dec.data.begin() + 1, dec.data.end())) { + bech32_data.reserve(((bech32_chars.size() - 1) * 5) / 8); + if (ConvertBits<5, 8, false>([&](unsigned char c) { bech32_data.push_back(c); }, bech32_chars.begin() + 1, bech32_chars.end())) { - std::string_view byte_str{data.size() == 1 ? "byte" : "bytes"}; + std::string_view byte_str{bech32_data.size() == 1 ? "byte" : "bytes"}; if (version == 0) { { WitnessV0KeyHash keyid; - if (data.size() == keyid.size()) { - std::copy(data.begin(), data.end(), keyid.begin()); + if (bech32_data.size() == keyid.size()) { + std::copy(bech32_data.begin(), bech32_data.end(), keyid.begin()); return keyid; } } { WitnessV0ScriptHash scriptid; - if (data.size() == scriptid.size()) { - std::copy(data.begin(), data.end(), scriptid.begin()); + if (bech32_data.size() == scriptid.size()) { + std::copy(bech32_data.begin(), bech32_data.end(), scriptid.begin()); return scriptid; } } - error_str = strprintf("Invalid Bech32 v0 address program size (%d %s), per BIP141", data.size(), byte_str); + error_str = strprintf("Invalid SegWit v0 address program size (%d %s), per BIP141", bech32_data.size(), byte_str); return CNoDestination(); } - if (version == 1 && data.size() == WITNESS_V1_TAPROOT_SIZE) { + if (version == 1 && bech32_data.size() == WITNESS_V1_TAPROOT_SIZE) { static_assert(WITNESS_V1_TAPROOT_SIZE == WitnessV1Taproot::size()); WitnessV1Taproot tap; - std::copy(data.begin(), data.end(), tap.begin()); + std::copy(bech32_data.begin(), bech32_data.end(), tap.begin()); return tap; } - if (CScript::IsPayToAnchor(version, data)) { + if (CScript::IsPayToAnchor(version, bech32_data)) { return PayToAnchor(); } @@ -223,12 +220,12 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par return CNoDestination(); } - if (data.size() < 2 || data.size() > BECH32_WITNESS_PROG_MAX_LEN) { - error_str = strprintf("Invalid Bech32 address program size (%d %s)", data.size(), byte_str); + if (bech32_data.size() < 2 || bech32_data.size() > BECH32_WITNESS_PROG_MAX_LEN) { + error_str = strprintf("Invalid Bech32 address program size (%d %s)", bech32_data.size(), byte_str); return CNoDestination(); } - return WitnessUnknown{version, data}; + return WitnessUnknown{version, bech32_data}; } else { error_str = strprintf("Invalid padding in Bech32 data section"); return CNoDestination(); @@ -236,9 +233,8 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par } // Perform Bech32 error location - auto res = bech32::LocateErrors(str); - error_str = res.first; - if (error_locations) *error_locations = std::move(res.second); + error_str = bech32_error; + if (error_locations) *error_locations = std::move(bech32_error_loc); return CNoDestination(); } } // namespace diff --git a/test/functional/rpc_invalid_address_message.py b/test/functional/rpc_invalid_address_message.py index f9924862f94..8b9f818bda0 100755 --- a/test/functional/rpc_invalid_address_message.py +++ b/test/functional/rpc_invalid_address_message.py @@ -64,12 +64,12 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): def test_validateaddress(self): # Invalid Bech32 - self.check_invalid(BECH32_INVALID_SIZE, "Invalid Bech32 address program size (41 bytes)") - self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid or unsupported prefix for Segwit (Bech32) address (expected bcrt, got bc)') + self.check_invalid(BECH32_INVALID_SIZE, 'Invalid Bech32 address program size (41 bytes)') + self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid or unsupported prefix for Segwit (Bech32) regtest address (expected bcrt, got bc)') self.check_invalid(BECH32_INVALID_BECH32, 'Version 1+ witness address must use Bech32m checksum') self.check_invalid(BECH32_INVALID_BECH32M, 'Version 0 witness address must use Bech32 checksum') self.check_invalid(BECH32_INVALID_VERSION, 'Invalid Bech32 address witness version') - self.check_invalid(BECH32_INVALID_V0_SIZE, "Invalid Bech32 v0 address program size (21 bytes), per BIP141") + self.check_invalid(BECH32_INVALID_V0_SIZE, 'Invalid SegWit v0 address program size (21 bytes), per BIP141') self.check_invalid(BECH32_TOO_LONG, 'Bech32(m) address decoded with error: Bech32 string too long', list(range(90, 108))) self.check_invalid(BECH32_ONE_ERROR, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [9]) self.check_invalid(BECH32_TWO_ERRORS, 'Bech32(m) address decoded with error: Invalid Bech32 checksum', [22, 43]) @@ -107,10 +107,10 @@ class InvalidAddressErrorMessageTest(BitcoinTestFramework): def test_getaddressinfo(self): node = self.nodes[0] - assert_raises_rpc_error(-5, "Invalid Bech32 address program size (41 bytes)", node.getaddressinfo, BECH32_INVALID_SIZE) - assert_raises_rpc_error(-5, "Invalid or unsupported prefix for Segwit (Bech32) address (expected bcrt, got bc)", node.getaddressinfo, BECH32_INVALID_PREFIX) - assert_raises_rpc_error(-5, "Invalid Base58 regtest address. Expected prefix 2, m, or n", node.getaddressinfo, BASE58_INVALID_PREFIX) - assert_raises_rpc_error(-5, "Bech32(m) address decoded with error: Invalid separator position", node.getaddressinfo, INVALID_ADDRESS) + assert_raises_rpc_error(-5, 'Invalid Bech32 address program size (41 bytes)', node.getaddressinfo, BECH32_INVALID_SIZE) + assert_raises_rpc_error(-5, 'Invalid or unsupported prefix for Segwit (Bech32) regtest address (expected bcrt, got bc)', node.getaddressinfo, BECH32_INVALID_PREFIX) + assert_raises_rpc_error(-5, 'Invalid Base58 regtest address. Expected prefix 2, m, or n', node.getaddressinfo, BASE58_INVALID_PREFIX) + assert_raises_rpc_error(-5, 'Bech32(m) address decoded with error: Invalid separator position', node.getaddressinfo, INVALID_ADDRESS) assert "isscript" not in node.getaddressinfo(BECH32_VALID_UNKNOWN_WITNESS) def run_test(self): diff --git a/test/functional/rpc_validateaddress.py b/test/functional/rpc_validateaddress.py index 11b5c0212e8..17d0d1171a1 100755 --- a/test/functional/rpc_validateaddress.py +++ b/test/functional/rpc_validateaddress.py @@ -12,7 +12,7 @@ INVALID_DATA = [ # BIP 173 ( "tc1qw508d6qejxtdg4y5r3zarvary0c5xw7kg3g4ty", - "Invalid or unsupported prefix for Segwit (Bech32) address (expected bc, got tc)", # Invalid hrp + "Invalid or unsupported prefix for Segwit (Bech32) Bitcoin address (expected bc, got tc)", # Invalid hrp [], ), ("bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t5", "Bech32(m) address decoded with error: Invalid Bech32 checksum", [41]), @@ -33,7 +33,7 @@ INVALID_DATA = [ ), ( "BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P", - "Invalid Bech32 v0 address program size (16 bytes), per BIP141", + "Invalid SegWit v0 address program size (16 bytes), per BIP141", [], ), ( @@ -60,7 +60,7 @@ INVALID_DATA = [ # BIP 350 ( "tc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vq5zuyut", - "Invalid or unsupported prefix for Segwit (Bech32) address (expected bc, got tc)", # Invalid human-readable part + "Invalid or unsupported prefix for Segwit (Bech32) Bitcoin address (expected bc, got tc)", # Invalid human-readable part [], ), ( @@ -96,7 +96,7 @@ INVALID_DATA = [ ), ( "BC1QR508D6QEJXTDG4Y5R3ZARVARYV98GJ9P", - "Invalid Bech32 v0 address program size (16 bytes), per BIP141", + "Invalid SegWit v0 address program size (16 bytes), per BIP141", [], ), (