Address review comments for Bech32 error validation

Author: meshcollider

src/bech32.cpp

@@ -391,7 +391,7 @@ void push_range(int from, int to, std::vector<int>& vec)
     }
 }
 
-/** Return index of first invalid character in a Bech32 string. */
+/** Return indices of invalid characters in a Bech32 string. */
 bool CheckCharacters(const std::string& str, std::vector<int>& errors) {
     bool lower = false, upper = false;
     for (size_t i = 0; i < str.size(); ++i) {

src/key_io.cpp

@@ -102,17 +102,20 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
             return ScriptHash(hash);
         }
 
-        if (!std::equal(script_prefix.begin(), script_prefix.end(), data.begin()) &&
-            !std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin())) {
-            error_str = "Invalid prefix for Base58-encoded address";
-        } else {
+        // If the prefix of data matches either the script or pubkey prefix, the length must have been wrong
+        if ((data.size() >= script_prefix.size() &&
+                std::equal(script_prefix.begin(), script_prefix.end(), data.begin())) ||
+            (data.size() >= pubkey_prefix.size() &&
+                std::equal(pubkey_prefix.begin(), pubkey_prefix.end(), data.begin()))) {
             error_str = "Invalid length for Base58 address";
+        } else {
+            error_str = "Invalid prefix for Base58-encoded address";
         }
         return CNoDestination();
     } else if (!is_bech32) {
         // Try Base58 decoding without the checksum, using a much larger max length
         if (!DecodeBase58(str, data, 100)) {
-            error_str = "Invalid HRP or Base58 character in address";
+            error_str = "Not a valid Bech32 or Base58 encoding";
         } else {
             error_str = "Invalid checksum or length of Base58 address";
         }

test/functional/rpc_invalid_address_message.py

@@ -61,7 +61,7 @@ def check_invalid(self, addr, error_str, error_locations=None):
     def test_validateaddress(self):
         # Invalid Bech32
         self.check_invalid(BECH32_INVALID_SIZE, 'Invalid Bech32 address data size')
-        self.check_invalid(BECH32_INVALID_PREFIX, 'Invalid HRP or Base58 character in address')
+        self.check_invalid(BECH32_INVALID_PREFIX, 'Not a valid Bech32 or Base58 encoding')
         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')
@@ -89,19 +89,19 @@ def test_validateaddress(self):
         self.check_valid(BASE58_VALID)
 
         # Invalid address format
-        self.check_invalid(INVALID_ADDRESS, 'Invalid HRP or Base58 character in address')
-        self.check_invalid(INVALID_ADDRESS_2, 'Invalid HRP or Base58 character in address')
+        self.check_invalid(INVALID_ADDRESS, 'Not a valid Bech32 or Base58 encoding')
+        self.check_invalid(INVALID_ADDRESS_2, 'Not a valid Bech32 or Base58 encoding')
 
     def test_getaddressinfo(self):
         node = self.nodes[0]
 
         assert_raises_rpc_error(-5, "Invalid Bech32 address data size", node.getaddressinfo, BECH32_INVALID_SIZE)
 
-        assert_raises_rpc_error(-5, "Invalid HRP or Base58 character in address", node.getaddressinfo, BECH32_INVALID_PREFIX)
+        assert_raises_rpc_error(-5, "Not a valid Bech32 or Base58 encoding", node.getaddressinfo, BECH32_INVALID_PREFIX)
 
         assert_raises_rpc_error(-5, "Invalid prefix for Base58-encoded address", node.getaddressinfo, BASE58_INVALID_PREFIX)
 
-        assert_raises_rpc_error(-5, "Invalid HRP or Base58 character in address", node.getaddressinfo, INVALID_ADDRESS)
+        assert_raises_rpc_error(-5, "Not a valid Bech32 or Base58 encoding", node.getaddressinfo, INVALID_ADDRESS)
 
     def run_test(self):
         self.test_validateaddress()