From a4fe70171b6fa570eda71d86b59d0fb24c2f0614 Mon Sep 17 00:00:00 2001
From: Samuel Dobson <dobsonsa68@gmail.com>
Date: Sat, 4 Dec 2021 12:49:27 +1300
Subject: [PATCH] Make Bech32 LocateErrors return error list rather than using
 out-arg

---
 src/bech32.cpp            | 27 ++++++++++++++++-----------
 src/bech32.h              |  2 +-
 src/key_io.cpp            | 10 +++-------
 src/test/bech32_tests.cpp |  6 ++----
 4 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/src/bech32.cpp b/src/bech32.cpp
index 119be74b57..3cda1dfff5 100644
--- a/src/bech32.cpp
+++ b/src/bech32.cpp
@@ -396,23 +396,28 @@ DecodeResult Decode(const std::string& str) {
 }
 
 /** Find index of an incorrect character in a Bech32 string. */
-std::string LocateErrors(const std::string& str, std::vector<int>& error_locations) {
+std::pair<std::string, std::vector<int>> LocateErrors(const std::string& str) {
+    std::vector<int> error_locations{};
+
     if (str.size() > 90) {
         error_locations.resize(str.size() - 90);
         std::iota(error_locations.begin(), error_locations.end(), 90);
-        return "Bech32 string too long";
+        return std::make_pair("Bech32 string too long", std::move(error_locations));
     }
+
     if (!CheckCharacters(str, error_locations)){
-        return "Invalid character or mixed case";
+        return std::make_pair("Invalid character or mixed case", std::move(error_locations));
     }
+
     size_t pos = str.rfind('1');
     if (pos == str.npos) {
-        return "Missing separator";
+        return std::make_pair("Missing separator", std::vector<int>{});
     }
     if (pos == 0 || pos + 7 > str.size()) {
         error_locations.push_back(pos);
-        return "Invalid separator position";
+        return std::make_pair("Invalid separator position", std::move(error_locations));
     }
+
     std::string hrp;
     for (size_t i = 0; i < pos; ++i) {
         hrp += LowerCase(str[i]);
@@ -425,7 +430,7 @@ std::string LocateErrors(const std::string& str, std::vector<int>& error_locatio
         int8_t rev = CHARSET_REV[c];
         if (rev == -1) {
             error_locations.push_back(i);
-            return "Invalid Base 32 character";
+            return std::make_pair("Invalid Base 32 character", std::move(error_locations));
         }
         values[i - pos - 1] = rev;
     }
@@ -545,8 +550,7 @@ std::string LocateErrors(const std::string& str, std::vector<int>& error_locatio
             }
         } else {
             // No errors
-            error_locations.clear();
-            return "";
+            return std::make_pair("", std::vector<int>{});
         }
 
         if (error_locations.empty() || (!possible_errors.empty() && possible_errors.size() < error_locations.size())) {
@@ -554,10 +558,11 @@ std::string LocateErrors(const std::string& str, std::vector<int>& error_locatio
             if (!error_locations.empty()) error_encoding = encoding;
         }
     }
-    return error_encoding == Encoding::BECH32M ? "Invalid Bech32m checksum"
-            : error_encoding == Encoding::BECH32 ? "Invalid Bech32 checksum"
-            : "Invalid checksum";
+    std::string error_message = error_encoding == Encoding::BECH32M ? "Invalid Bech32m checksum"
+                              : error_encoding == Encoding::BECH32 ? "Invalid Bech32 checksum"
+                              : "Invalid checksum";
 
+    return std::make_pair(error_message, std::move(error_locations));
 }
 
 } // namespace bech32
diff --git a/src/bech32.h b/src/bech32.h
index 7e92d5d23a..5e89e6efda 100644
--- a/src/bech32.h
+++ b/src/bech32.h
@@ -46,7 +46,7 @@ struct DecodeResult
 DecodeResult Decode(const std::string& str);
 
 /** Return the positions of errors in a Bech32 string. */
-std::string LocateErrors(const std::string& str, std::vector<int>& error_locations);
+std::pair<std::string, std::vector<int>> LocateErrors(const std::string& str);
 
 } // namespace bech32
 
diff --git a/src/key_io.cpp b/src/key_io.cpp
index c89493e29d..3559f3a03d 100644
--- a/src/key_io.cpp
+++ b/src/key_io.cpp
@@ -188,13 +188,9 @@ CTxDestination DecodeDestination(const std::string& str, const CChainParams& par
     }
 
     // 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);
-    }
-
+    auto res = bech32::LocateErrors(str);
+    error_str = res.first;
+    if (error_locations) *error_locations = std::move(res.second);
     return CNoDestination();
 }
 } // namespace
diff --git a/src/test/bech32_tests.cpp b/src/test/bech32_tests.cpp
index 5d851a9735..51a1d1199e 100644
--- a/src/test/bech32_tests.cpp
+++ b/src/test/bech32_tests.cpp
@@ -97,8 +97,7 @@ BOOST_AUTO_TEST_CASE(bech32_testvectors_invalid)
         const auto& err = ERRORS[i];
         const auto dec = bech32::Decode(str);
         BOOST_CHECK(dec.encoding == bech32::Encoding::INVALID);
-        std::vector<int> error_locations;
-        std::string error = bech32::LocateErrors(str, error_locations);
+        auto [error, error_locations] = bech32::LocateErrors(str);
         BOOST_CHECK_EQUAL(err.first, error);
         BOOST_CHECK(err.second == error_locations);
         i++;
@@ -150,8 +149,7 @@ BOOST_AUTO_TEST_CASE(bech32m_testvectors_invalid)
         const auto& err = ERRORS[i];
         const auto dec = bech32::Decode(str);
         BOOST_CHECK(dec.encoding == bech32::Encoding::INVALID);
-        std::vector<int> error_locations;
-        std::string error = bech32::LocateErrors(str, error_locations);
+        auto [error, error_locations] = bech32::LocateErrors(str);
         BOOST_CHECK_EQUAL(err.first, error);
         BOOST_CHECK(err.second == error_locations);
         i++;