Merge bitcoin/bitcoin#16807: Let validateaddress locate error in Bech32 address

88cc4810926e4f5af6757ee1b0eed61abda3d746 Modify copyright header on Bech32 code (Samuel Dobson)
5599813b80e53a1539c66625b4320ab1b4fb4848 Add lots of comments to Bech32 (Samuel Dobson)
2eb5792ec7bbeaf7138420b6c85c5cd0a0404946 Add release notes for validateaddress Bech32 error detection (MeshCollider)
42d6a029e57a32f2d1d829ff7718b6d40d58b9d1 Refactor and add more tests for validateaddress (Samuel Dobson)
c4979f77c1264f0099d1dfa278b1d9c18340b5f9 Add boost tests for bech32 error detection (MeshCollider)
02a7bdee429ae307a5e57832727fed789e2e04fb Add error_locations to validateaddress RPC (Samuel Dobson)
b62b67e06cc406fdad68da4c091168fb5f11c1d4 Add Bech32 error location function (Samuel Dobson)
0b06e720c0182dee8b560d2e8d3891b036f63ea7 More detailed error checking for base58 addresses (Samuel Dobson)

Pull request description:

  Addresses (partially) #16779 - no GUI change in this PR

  Adds a LocateError function the bech32 library, which is then called by `validateaddress` RPC, (and then eventually from a GUI tool too, future work). I think modifying validateaddress is nicer than adding a separate RPC for this.
  Includes tests.

  Based on https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js
  Credit to sipa for that code

ACKs for top commit:
  laanwj:
    Code review and manually tested ACK 88cc4810926e4f5af6757ee1b0eed61abda3d746
  ryanofsky:
    Code review ACK 88cc4810926e4f5af6757ee1b0eed61abda3d746 with caveat that I only checked the new `LocateErrors` code to try to verify it didn't have unsafe or unexpected operations or loop forever or crash. Did not try to verify behavior corresponds to the spec. In the worst case bugs here should just affect error messages not actual decoding of addresses so this seemed ok.
  w0xlt:
    tACK 88cc481

Tree-SHA512: 9c7fe9745bc7527f80a30bd4c1e3034e16b96a02cc7f6c268f91bfad08a6965a8064fe44230aa3f87e4fa3c938f662ff4446bc682c83cb48c1a3f95cf4186688

1 files changed, 32 insertions, 11 deletions

diff --git a/src/key_io.cpp b/src/key_io.cpp
index 615f4c9312..6908c5ea52 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -76,12 +76,16 @@ public:
     std::string operator()(const CNoDestination& no) const { return {}; }
 };
 
-CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str)
+CTxDestination DecodeDestination(const std::string& str, const CChainParams& params, std::string& error_str, std::vector<int>* error_locations)
 {
     std::vector<unsigned char> data;
     uint160 hash;
     error_str = "";
-    if (DecodeBase58Check(str, data, 21)) {
+
+    // 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());
+
+    if (!is_bech32 && DecodeBase58Check(str, data, 21)) {
         // 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.
@@ -98,15 +102,27 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
             return ScriptHash(hash);
         }
 
-        // Set potential error message.
-        // This message may be changed if the address can also be interpreted as a Bech32 address.
-        error_str = "Invalid prefix for Base58-encoded address";
+        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 {
+            error_str = "Invalid length for Base58 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";
+        } else {
+            error_str = "Invalid checksum or length of Base58 address";
+        }
+        return CNoDestination();
     }
+
     data.clear();
     const auto dec = bech32::Decode(str);
     if ((dec.encoding == bech32::Encoding::BECH32 || dec.encoding == bech32::Encoding::BECH32M) && dec.data.size() > 0) {
         // Bech32 decoding
-        error_str = "";
         if (dec.hrp != params.Bech32HRP()) {
             error_str = "Invalid prefix for Bech32 address";
             return CNoDestination();
@@ -168,8 +184,13 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
         }
     }
 
-    // Set error message if address can't be interpreted as Base58 or Bech32.
-    if (error_str.empty()) error_str = "Invalid address format";
+    // Perform Bech32 error location
+    if (!error_locations) {
+        std::vector<int> dummy_errors;
+        error_str = bech32::LocateErrors(str, dummy_errors);
+    } else {
+        error_str = bech32::LocateErrors(str, *error_locations);
+    }
 
     return CNoDestination();
 }
@@ -258,9 +279,9 @@ std::string EncodeDestination(const CTxDestination& dest)
     return std::visit(DestinationEncoder(Params()), dest);
 }
 
-CTxDestination DecodeDestination(const std::string& str, std::string& error_msg)
+CTxDestination DecodeDestination(const std::string& str, std::string& error_msg, std::vector<int>* error_locations)
 {
-    return DecodeDestination(str, Params(), error_msg);
+    return DecodeDestination(str, Params(), error_msg, error_locations);
 }
 
 CTxDestination DecodeDestination(const std::string& str)
@@ -272,7 +293,7 @@ CTxDestination DecodeDestination(const std::string& str)
 bool IsValidDestinationString(const std::string& str, const CChainParams& params)
 {
     std::string error_msg;
-    return IsValidDestination(DecodeDestination(str, params, error_msg));
+    return IsValidDestination(DecodeDestination(str, params, error_msg, nullptr));
 }
 
 bool IsValidDestinationString(const std::string& str)