From 751544c9654c7f4b1f5e60ab18e1482761378da0 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Wed, 25 Oct 2023 13:15:58 +0100
Subject: [PATCH 1/7] (Partial) Merge bitcoin-core/gui#771: Avoid error-prone
 leading whitespace in translatable strings

856325fac17465d102da621f1282b6d8ed02f679 lint: Add `lint-qt-translation.py` (Hennadii Stepanov)
294a018bf5106b03af39a2a8cfa4d5f2ebf6912b qt: Avoid error prone leading spaces in translatable strings (Hennadii Stepanov)
d8298e7f069f961fc077ceacff2c332d58734688 qt, refactor: Drop superfluous type conversions (Hennadii Stepanov)

Pull request description:

  While working on the GUI translation via Transifex web interface, I found it error-prone to have leading whitespace in translatable strings. This is because it is very easy to unintentionally drop them in translations unnoticed.

  Fixed all current cases. Added a linter to prevent similar cases in the future.

ACKs for top commit:
  furszy:
    utACK 856325f

Tree-SHA512: b1ca5effb2db6649e1e99382de79acf3a9f81cc9dad434db5623338489e597897e8addd60c1ab3dcc7506ae62753a7a4ad5a41d7a865f8fcdf94348b54baa7e7
---
 src/qt/psbtoperationsdialog.cpp  | 13 +++++++------
 src/qt/psbtoperationsdialog.h    |  3 ++-
 src/qt/sendcoinsdialog.cpp       |  2 +-
 test/lint/lint-qt-translation.py | 23 +++++++++++++++++++++++
 4 files changed, 33 insertions(+), 8 deletions(-)
 create mode 100755 test/lint/lint-qt-translation.py

diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp
index 5e817cdc4f6f..2a98bfa9abb1 100644
--- a/src/qt/psbtoperationsdialog.cpp
+++ b/src/qt/psbtoperationsdialog.cpp
@@ -168,19 +168,20 @@ void PSBTOperationsDialog::saveTransaction() {
 }
 
 void PSBTOperationsDialog::updateTransactionDisplay() {
-    m_ui->transactionDescription->setText(QString::fromStdString(renderTransaction(m_transaction_data)));
+    m_ui->transactionDescription->setText(renderTransaction(m_transaction_data));
     showTransactionStatus(m_transaction_data);
 }
 
-std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransaction &psbtx)
+QString PSBTOperationsDialog::renderTransaction(const PartiallySignedTransaction &psbtx)
 {
-    QString tx_description = "";
+    QString tx_description;
+    QLatin1String bullet_point(" * ");
     CAmount totalAmount = 0;
     for (const CTxOut& out : psbtx.tx->vout) {
         CTxDestination address;
         ExtractDestination(out.scriptPubKey, address);
         totalAmount += out.nValue;
-        tx_description.append(tr(" * Sends %1 to %2")
+        tx_description.append(bullet_point).append(tr("Sends %1 to %2")
             .arg(BitcoinUnits::formatWithUnit(BitcoinUnit::DASH, out.nValue))
             .arg(QString::fromStdString(EncodeDestination(address))));
         // Check if the address is one of ours
@@ -189,7 +190,7 @@ std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransac
     }
 
     PSBTAnalysis analysis = AnalyzePSBT(psbtx);
-    tx_description.append(" * ");
+    tx_description.append(bullet_point);
     if (!*analysis.fee) {
         // This happens if the transaction is missing input UTXO information.
         tx_description.append(tr("Unable to calculate transaction fee or total transaction amount."));
@@ -218,7 +219,7 @@ std::string PSBTOperationsDialog::renderTransaction(const PartiallySignedTransac
         tx_description.append(tr("Transaction has %1 unsigned inputs.").arg(QString::number(num_unsigned)));
     }
 
-    return tx_description.toStdString();
+    return tx_description;
 }
 
 void PSBTOperationsDialog::showStatus(const QString &msg, StatusLevel level) {
diff --git a/src/qt/psbtoperationsdialog.h b/src/qt/psbtoperationsdialog.h
index f37bdbe39a3c..23f7dbf22771 100644
--- a/src/qt/psbtoperationsdialog.h
+++ b/src/qt/psbtoperationsdialog.h
@@ -6,6 +6,7 @@
 #define BITCOIN_QT_PSBTOPERATIONSDIALOG_H
 
 #include <QDialog>
+#include <QString>
 
 #include <psbt.h>
 #include <qt/clientmodel.h>
@@ -46,7 +47,7 @@ public Q_SLOTS:
 
     size_t couldSignInputs(const PartiallySignedTransaction &psbtx);
     void updateTransactionDisplay();
-    std::string renderTransaction(const PartiallySignedTransaction &psbtx);
+    QString renderTransaction(const PartiallySignedTransaction &psbtx);
     void showStatus(const QString &msg, StatusLevel level);
     void showTransactionStatus(const PartiallySignedTransaction &psbtx);
 };
diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp
index b5e6e81ffdde..4ecf635bd209 100644
--- a/src/qt/sendcoinsdialog.cpp
+++ b/src/qt/sendcoinsdialog.cpp
@@ -363,7 +363,7 @@ bool SendCoinsDialog::send(const QList<SendCoinsRecipient>& recipients, QString&
         // generate amount string with wallet name in case of multiwallet
         QString amount = BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), rcp.amount);
         if (model->isMultiwallet()) {
-            amount.append(tr(" from wallet '%1'").arg(model->getWalletName()));
+            amount = tr("%1 from wallet '%2'").arg(amount, model->getWalletName());
         }
 
         // generate address string
diff --git a/test/lint/lint-qt-translation.py b/test/lint/lint-qt-translation.py
new file mode 100755
index 000000000000..47bf96b654f4
--- /dev/null
+++ b/test/lint/lint-qt-translation.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+#
+# Copyright (c) 2023-present The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://opensource.org/license/mit/.
+#
+# Check for leading whitespaces in the translatable strings.
+
+import subprocess
+import sys
+
+
+def main():
+    tr_strings = subprocess.run(['git', 'grep', '-e', 'tr("[[:space:]]', '--', 'src/qt'], stdout=subprocess.PIPE, text=True).stdout
+
+    if tr_strings.strip():
+        print("Avoid leading whitespaces in:")
+        print(tr_strings)
+        sys.exit(1)
+
+
+if __name__ == "__main__":
+    main()

From 8c8ee056d64f7309d6793b37b61829cb2b7b3bd0 Mon Sep 17 00:00:00 2001
From: Andrew Chow <github@achow101.com>
Date: Thu, 20 Apr 2023 17:12:48 -0400
Subject: [PATCH 2/7] Merge bitcoin/bitcoin#27412: logging, net: add ASN from
 peers on logs

0076bed45eb2b42111fa3f4c95181393c685a42e logging: log ASN when using `-asmap` (brunoerg)
9836c76ae048698e4f7dab21e3be37313e8392ae net: add `GetMappedAS` in `CConnman` (brunoerg)

Pull request description:

  When using `-asmap`, you can check the ASN assigned to the peers only with the RPC command `getpeerinfo` (check `mapped_as` field), however, it's not possible to check it in logs (e.g. see in logs the ASN of the peers when a new outbound peer has been connected). This PR includes the peers' ASN in debug output when using `-asmap`.

  Obs: Open this primarily to chase some Concept ACK, I've been using this on my node to facilitate to track the peers' ASN especially when reading the logs.

ACKs for top commit:
  Sjors:
    tACK 0076bed45eb2b42111fa3f4c95181393c685a42e
  jamesob:
    ACK 0076bed45eb2b42111fa3f4c95181393c685a42e ([`jamesob/ackr/27412.1.brunoerg.logging_net_add_asn_from`](https://github.com/jamesob/bitcoin/tree/ackr/27412.1.brunoerg.logging_net_add_asn_from))
  achow101:
    ACK 0076bed45eb2b42111fa3f4c95181393c685a42e

Tree-SHA512: c19cd11e8ab49962021f390459aadf6d33d221ae9a2c3df331a25d6865a8df470e2c8828f6e5219b8a887d6ab5b3450d34be9e26c00cca4d223b4ca64d51111b
---
 src/net.cpp            |  7 ++++++-
 src/net.h              |  1 +
 src/net_processing.cpp | 11 +++++++----
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index f62ebb16820d..4467507f68e0 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -4499,6 +4499,11 @@ size_t CConnman::GetMaxOutboundOnionNodeCount()
     return m_max_outbound_onion;
 }
 
+uint32_t CConnman::GetMappedAS(const CNetAddr& addr) const
+{
+    return m_netgroupman.GetMappedAS(addr);
+}
+
 void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
 {
     vstats.clear();
@@ -4510,7 +4515,7 @@ void CConnman::GetNodeStats(std::vector<CNodeStats>& vstats) const
         }
         vstats.emplace_back();
         pnode->CopyStats(vstats.back());
-        vstats.back().m_mapped_as = m_netgroupman.GetMappedAS(pnode->addr);
+        vstats.back().m_mapped_as = GetMappedAS(pnode->addr);
     }
 }
 
diff --git a/src/net.h b/src/net.h
index 1da1093f187b..6a742647ac0b 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1472,6 +1472,7 @@ friend class CNode;
     size_t GetMaxOutboundNodeCount();
     size_t GetMaxOutboundOnionNodeCount();
     void GetNodeStats(std::vector<CNodeStats>& vstats) const EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex);
+    uint32_t GetMappedAS(const CNetAddr& addr) const;
     bool DisconnectNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex);
     bool DisconnectNode(const CSubNet& subnet) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex);
     bool DisconnectNode(const CNetAddr& addr) EXCLUSIVE_LOCKS_REQUIRED(!m_nodes_mutex);
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 851848a60d5d..541778a06a4c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3942,10 +3942,11 @@ void PeerManagerImpl::ProcessMessage(
         if (fLogIPs)
             remoteAddr = ", peeraddr=" + pfrom.addr.ToStringAddrPort();
 
-        LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, txrelay=%d, peer=%d%s\n",
+        const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
+        LogPrint(BCLog::NET, "receive version message: %s: version %d, blocks=%d, us=%s, txrelay=%d, peer=%d%s%s\n",
                   cleanSubVer, pfrom.nVersion,
                   peer->m_starting_height, addrMe.ToStringAddrPort(), fRelay, pfrom.GetId(),
-                  remoteAddr);
+                  remoteAddr, (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
 
         int64_t nTimeOffset = nTime - GetTime();
         pfrom.nTimeOffset = nTimeOffset;
@@ -3981,11 +3982,13 @@ void PeerManagerImpl::ProcessMessage(
         // Log successful connections unconditionally for outbound, but not for inbound as those
         // can be triggered by an attacker at high rate.
         if (!pfrom.IsInboundConn() || LogAcceptCategory(BCLog::NET, BCLog::Level::Debug)) {
-            LogPrintf("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s\n",
+            const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
+            LogPrintf("New %s %s peer connected: version: %d, blocks=%d, peer=%d%s%s\n",
                       pfrom.ConnectionTypeAsString(),
                       TransportTypeAsString(pfrom.m_transport->GetInfo().transport_type),
                       pfrom.nVersion.load(), peer->m_starting_height,
-                      pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""));
+                      pfrom.GetId(), (fLogIPs ? strprintf(", peeraddr=%s", pfrom.addr.ToStringAddrPort()) : ""),
+                      (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : ""));
         }
 
         if (is_masternode && !pfrom.m_masternode_probe_connection) {

From 42a5a9ed803e33430360ba6d6b8b00b6eb4d2710 Mon Sep 17 00:00:00 2001
From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
Date: Wed, 25 Oct 2023 13:05:04 +0100
Subject: [PATCH 3/7] Merge bitcoin-core/gui#742: Exit and show error if
 unrecognized command line args are present

51e4dc49f5335b5bae6c14606d1cc653a08a96b1 gui: Show error if unrecognized command line args are present (John Moffett)

Pull request description:

  Fixes https://github.com/bitcoin-core/gui/issues/741

  Starting bitcoin-qt with non-hyphen ("-") arguments causes it to silently ignore any later valid options. For instance, invoking `bitcoin-qt -server=1 foo -regtest` on a fresh install will run `mainnet` instead of `regtest`.

  This change makes the client exit with an error message if any such "loose" arguments are encountered. This mirrors how `bitcoind` handles it:

  https://github.com/bitcoin/bitcoin/blob/c6287faae4c0e705a9258a340dfcf548906f12af/src/bitcoind.cpp#L127-L132

  However, BIP-21 `bitcoin:` payment URIs are still allowed, but only if they're not followed by any additional options.

ACKs for top commit:
  maflcko:
    lgtm ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1
  hernanmarino:
    tested ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1
  pablomartin4btc:
    tACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1
  hebasto:
    ACK 51e4dc49f5335b5bae6c14606d1cc653a08a96b1, I have reviewed the code and it looks OK.

Tree-SHA512: 3997a7a9a747314f13e118aee63e8679e00ed832d9c6f115559a4c39c9c4091572207c60e362cb4c19fc8da980d4b0b040050aa70c5ef84a855cb7e3568bbf13
---
 src/qt/bitcoin.cpp     | 27 +++++++++++++++++++++++++++
 src/qt/paymentserver.h |  2 ++
 2 files changed, 29 insertions(+)

diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index 6f468642f74f..362e8002d4f7 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -551,6 +551,33 @@ int GuiMain(int argc, char* argv[])
         return EXIT_FAILURE;
     }
 
+    // Error out when loose non-argument tokens are encountered on command line
+    // However, allow BIP-21 URIs only if no options follow
+    bool payment_server_token_seen = false;
+    for (int i = 1; i < argc; i++) {
+        QString arg(argv[i]);
+        bool invalid_token = !arg.startsWith("-");
+#ifdef ENABLE_WALLET
+        if (arg.startsWith(BITCOIN_IPC_PREFIX, Qt::CaseInsensitive)) {
+            invalid_token &= false;
+            payment_server_token_seen = true;
+        }
+#endif
+        if (payment_server_token_seen && arg.startsWith("-")) {
+            InitError(Untranslated(strprintf("Options ('%s') cannot follow a BIP-21 payment URI", argv[i])));
+            QMessageBox::critical(nullptr, PACKAGE_NAME,
+                                  // message cannot be translated because translations have not been initialized
+                                  QString::fromStdString("Options ('%1') cannot follow a BIP-21 payment URI").arg(QString::fromStdString(argv[i])));
+            return EXIT_FAILURE;
+        }
+        if (invalid_token) {
+            InitError(Untranslated(strprintf("Command line contains unexpected token '%s', see dash-qt -h for a list of options.", argv[i])));
+            QMessageBox::critical(nullptr, PACKAGE_NAME,
+                                  // message cannot be translated because translations have not been initialized
+                                  QString::fromStdString("Command line contains unexpected token '%1', see dash-qt -h for a list of options.").arg(QString::fromStdString(argv[i])));
+            return EXIT_FAILURE;
+        }
+    }
     /// 3. Application identification
     // must be set before OptionsModel is initialized or translations are loaded,
     // as it is used to locate QSettings
diff --git a/src/qt/paymentserver.h b/src/qt/paymentserver.h
index 545007ae7b97..4b62f180643a 100644
--- a/src/qt/paymentserver.h
+++ b/src/qt/paymentserver.h
@@ -54,6 +54,8 @@ class QLocalServer;
 class QUrl;
 QT_END_NAMESPACE
 
+extern const QString BITCOIN_IPC_PREFIX;
+
 class PaymentServer : public QObject
 {
     Q_OBJECT

From 5697ea2ff1d95231499fa125b295e2c383316880 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Fri, 2 Jun 2023 16:03:50 +0100
Subject: [PATCH 4/7] Merge bitcoin/bitcoin#27256: refactor: rpc: Remove
 unnecessary uses of ParseNonRFCJSONValue() and rename it

cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687 refactor: rpc: hide and rename ParseNonRFCJSONValue() (stickies-v)
6c8bde6d54d03224709dce54b8ba32b8c3e37ac7 test: move coverage on ParseNonRFCJSONValue() to UniValue::read() (stickies-v)

Pull request description:

  Follow-up to https://github.com/bitcoin/bitcoin/pull/26612#issuecomment-1453623741. As per https://github.com/bitcoin/bitcoin/pull/26506#pullrequestreview-1211984059, `ParseNonRFCJSONValue()` is no longer necessary and we can use `UniValue::read()` directly:

  > IIRC before that PR UniValue::read could only parse JSON object and array values, but now it will parse string/number/boolean/null values as well. laanwj pointed this out in https://github.com/bitcoin/bitcoin/issues/9028#issuecomment-257885368

  The implementation of `ParseNonRFCJSONValue()` was already [simplified in #26612](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-84c7a7f36362b9724c31e5dec9879b2f81eae0d0addbc9c0933c3558c577de65R259-R263)  and [test coverage updated](https://github.com/bitcoin/bitcoin/pull/26612/files#diff-fc0f86b6c3bb23b0e983bcf79d7546d1c9eaa15d6e4d8a7b03b5b85955f585abR292-R312) to ensure behaviour didn't change.

  To avoid code duplication, we keep the function to throw on invalid input data but rename it to `Parse()` and remove it from the header.

  The existing test coverage we had on `ParseNonRFCJSONValue()` is moved over to `UniValue::read()`.

ACKs for top commit:
  ryanofsky:
    Code review ACK cfbc8a623b5133f1d0b0c0c9be73b2b107e0d687. Only change since last review is adding a new test

Tree-SHA512: 89be959d2353af7ace0c1348ba1600b9ac1d3c7b3cf7f0b59f6e005b7fb9d695ce3e8720e1be3cf77fe7e318a4017c880df108928e7179ec50447583d13bc781
---
 src/rpc/client.cpp               | 22 ++++++++++------------
 src/rpc/client.h                 |  5 -----
 src/test/fuzz/parse_univalue.cpp |  9 +++------
 src/test/fuzz/string.cpp         |  4 ----
 src/test/rpc_tests.cpp           | 31 +------------------------------
 src/univalue/test/object.cpp     | 27 +++++++++++++++++++++++++++
 6 files changed, 41 insertions(+), 57 deletions(-)

diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 636ca29d6163..8e6dd1cd1e26 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -277,6 +277,14 @@ static const CRPCConvertParam vRPCConvertParams[] =
 };
 // clang-format on
 
+/** Parse string to UniValue or throw runtime_error if string contains invalid JSON */
+static UniValue Parse(std::string_view raw)
+{
+    UniValue parsed;
+    if (!parsed.read(raw)) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw));
+    return parsed;
+}
+
 class CRPCConvertTable
 {
 private:
@@ -304,7 +312,7 @@ class CRPCConvertTable
     UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, int param_idx)
     {
         if (const auto it = members.find({method, param_idx}); it != members.end() && (!it->second || (it->second && LikelyJSONType(arg_value)))) {
-            return ParseNonRFCJSONValue(MaybeUnquoteString(arg_value));
+            return Parse(MaybeUnquoteString(arg_value));
         }
         return arg_value;
     }
@@ -313,7 +321,7 @@ class CRPCConvertTable
     UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, const std::string& param_name)
     {
         if (const auto it = membersByName.find({method, param_name}); it != membersByName.end() && (!it->second || (it->second && LikelyJSONType(arg_value)))) {
-            return ParseNonRFCJSONValue(MaybeUnquoteString(arg_value));
+            return Parse(MaybeUnquoteString(arg_value));
         }
         return arg_value;
     }
@@ -337,16 +345,6 @@ CRPCConvertTable::CRPCConvertTable()
 
 static CRPCConvertTable rpcCvtTable;
 
-/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null)
- * as well as objects and arrays.
- */
-UniValue ParseNonRFCJSONValue(std::string_view raw)
-{
-    UniValue parsed;
-    if (!parsed.read(raw)) throw std::runtime_error(tfm::format("Error parsing JSON: %s", raw));
-    return parsed;
-}
-
 UniValue RPCConvertValues(std::string strMethod, const std::vector<std::string> &strParams)
 {
     UniValue params(UniValue::VARR);
diff --git a/src/rpc/client.h b/src/rpc/client.h
index 60e118de084c..c825021b301f 100644
--- a/src/rpc/client.h
+++ b/src/rpc/client.h
@@ -17,9 +17,4 @@ UniValue RPCConvertValues(std::string strMethod, const std::vector<std::string>&
 /** Convert named arguments to command-specific RPC representation */
 UniValue RPCConvertNamedValues(std::string strMethod, const std::vector<std::string>& strParams);
 
-/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null)
- * as well as objects and arrays.
- */
-UniValue ParseNonRFCJSONValue(std::string_view raw);
-
 #endif // BITCOIN_RPC_CLIENT_H
diff --git a/src/test/fuzz/parse_univalue.cpp b/src/test/fuzz/parse_univalue.cpp
index 0d889c60a8cc..993a4800c640 100644
--- a/src/test/fuzz/parse_univalue.cpp
+++ b/src/test/fuzz/parse_univalue.cpp
@@ -23,12 +23,9 @@ FUZZ_TARGET(parse_univalue, .init = initialize_parse_univalue)
     const std::string random_string(buffer.begin(), buffer.end());
     bool valid = true;
     const UniValue univalue = [&] {
-        try {
-            return ParseNonRFCJSONValue(random_string);
-        } catch (const std::runtime_error&) {
-            valid = false;
-            return UniValue{};
-        }
+        UniValue uv;
+        if (!uv.read(random_string)) valid = false;
+        return valid ? uv : UniValue{};
     }();
     if (!valid) {
         return;
diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp
index 80d6625e6700..96be403a42dc 100644
--- a/src/test/fuzz/string.cpp
+++ b/src/test/fuzz/string.cpp
@@ -157,10 +157,6 @@ FUZZ_TARGET(string)
     const util::Settings settings;
     (void)OnlyHasDefaultSectionSetting(settings, random_string_1, random_string_2);
     (void)ParseNetwork(random_string_1);
-    try {
-        (void)ParseNonRFCJSONValue(random_string_1);
-    } catch (const std::runtime_error&) {
-    }
     (void)RemovePrefix(random_string_1, random_string_2);
     (void)ResolveErrMsg(random_string_1, random_string_2);
     try {
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index 6ebc56e250e1..cff19c4fd8d6 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -285,6 +285,7 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values)
     BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.00000001000000")), 1LL); //should pass, cut trailing 0
     BOOST_CHECK_THROW(AmountFromValue(ValueFromString("19e-9")), UniValue); //should fail
     BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("0.19e-6")), 19); //should pass, leading 0 is present
+    BOOST_CHECK_EXCEPTION(AmountFromValue(".19e-6"), UniValue, HasJSON(R"({"code":-3,"message":"Invalid amount"})")); //should fail, no leading 0
 
     BOOST_CHECK_THROW(AmountFromValue(ValueFromString("92233720368.54775808")), UniValue); //overflow error
     BOOST_CHECK_THROW(AmountFromValue(ValueFromString("1e+11")), UniValue); //overflow error
@@ -292,36 +293,6 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values)
     BOOST_CHECK_THROW(AmountFromValue(ValueFromString("93e+9")), UniValue); //overflow error
 }
 
-BOOST_AUTO_TEST_CASE(json_parse_errors)
-{
-    // Valid
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0").get_real(), 1.0);
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("true").get_bool(), true);
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("[false]")[0].get_bool(), false);
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"a\": true}")["a"].get_bool(), true);
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("{\"1\": \"true\"}")["1"].get_str(), "true");
-    // Valid, with leading or trailing whitespace
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue(" 1.0").get_real(), 1.0);
-    BOOST_CHECK_EQUAL(ParseNonRFCJSONValue("1.0 ").get_real(), 1.0);
-
-    BOOST_CHECK_THROW(AmountFromValue(ParseNonRFCJSONValue(".19e-6")), std::runtime_error); //should fail, missing leading 0, therefore invalid JSON
-    BOOST_CHECK_EQUAL(AmountFromValue(ParseNonRFCJSONValue("0.00000000000000000000000000000000000001e+30 ")), 1);
-    // Invalid, initial garbage
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("[1.0"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("a1.0"), std::runtime_error);
-    // Invalid, trailing garbage
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0sds"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("1.0]"), std::runtime_error);
-    // Invalid, keys have to be names
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("{1: \"true\"}"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("{true: 1}"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("{[1]: 1}"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("{{\"a\": \"a\"}: 1}"), std::runtime_error);
-    // BTC addresses should fail parsing
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"), std::runtime_error);
-    BOOST_CHECK_THROW(ParseNonRFCJSONValue("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"), std::runtime_error);
-}
-
 BOOST_AUTO_TEST_CASE(rpc_ban)
 {
     BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned")));
diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp
index b19977c1ddc0..b2a86a3faf76 100644
--- a/src/univalue/test/object.cpp
+++ b/src/univalue/test/object.cpp
@@ -412,6 +412,33 @@ void univalue_readwrite()
 
     BOOST_CHECK_EQUAL(strJson1, v.write());
 
+    // Valid
+    BOOST_CHECK(v.read("1.0") && (v.get_real() == 1.0));
+    BOOST_CHECK(v.read("true") && v.get_bool());
+    BOOST_CHECK(v.read("[false]") && !v[0].get_bool());
+    BOOST_CHECK(v.read("{\"a\": true}") && v["a"].get_bool());
+    BOOST_CHECK(v.read("{\"1\": \"true\"}") && (v["1"].get_str() == "true"));
+    // Valid, with leading or trailing whitespace
+    BOOST_CHECK(v.read(" 1.0") && (v.get_real() == 1.0));
+    BOOST_CHECK(v.read("1.0 ") && (v.get_real() == 1.0));
+    BOOST_CHECK(v.read("0.00000000000000000000000000000000000001e+30 ") && v.get_real() == 1e-8);
+
+    BOOST_CHECK(!v.read(".19e-6")); //should fail, missing leading 0, therefore invalid JSON
+    // Invalid, initial garbage
+    BOOST_CHECK(!v.read("[1.0"));
+    BOOST_CHECK(!v.read("a1.0"));
+    // Invalid, trailing garbage
+    BOOST_CHECK(!v.read("1.0sds"));
+    BOOST_CHECK(!v.read("1.0]"));
+    // Invalid, keys have to be names
+    BOOST_CHECK(!v.read("{1: \"true\"}"));
+    BOOST_CHECK(!v.read("{true: 1}"));
+    BOOST_CHECK(!v.read("{[1]: 1}"));
+    BOOST_CHECK(!v.read("{{\"a\": \"a\"}: 1}"));
+    // BTC addresses should fail parsing
+    BOOST_CHECK(!v.read("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"));
+    BOOST_CHECK(!v.read("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"));
+
     /* Check for (correctly reporting) a parsing error if the initial
        JSON construct is followed by more stuff.  Note that whitespace
        is, of course, exempt.  */

From a79c1e8887e878764f67fb7b73e8c9e2b7f39800 Mon Sep 17 00:00:00 2001
From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
Date: Tue, 6 Dec 2022 10:30:54 +0100
Subject: [PATCH 5/7] Merge bitcoin/bitcoin#26611: wallet: Change coin
 selection fee assert to error

3eb041f014870954db564369a4be4bd0dea48fbe wallet: Change coin selection fee assert to error (Andrew Chow)
c6e7f224c119f47af250a9e0c5b185cb98b30c4c util: Add StrFormatInternalBug and STR_INTERNAL_BUG (MarcoFalke)

Pull request description:

  Returning an error instead of asserting for the low fee check will be better as it does not crash the node and instructs users to report the bug.

ACKs for top commit:
  S3RK:
    ACK 3eb041f014870954db564369a4be4bd0dea48fbe
  aureleoules:
    ACK 3eb041f014870954db564369a4be4bd0dea48fbe
  furszy:
    ACK 3eb041f0

Tree-SHA512: 118c13d7cdfce492080edd4cb12e6d960695377b978c7573f9c58b6d918664afd0e8e591eed0605d08ac756fa8eceed456349de5f3a025174069abf369bb5a5f
---
 src/util/check.cpp   | 13 ++++++++-----
 src/util/check.h     |  4 ++++
 src/wallet/spend.cpp |  6 ++++++
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/util/check.cpp b/src/util/check.cpp
index 047cd4b1b3f7..5149a2e18a10 100644
--- a/src/util/check.cpp
+++ b/src/util/check.cpp
@@ -14,13 +14,16 @@
 #include <cstdio>
 #include <cstdlib>
 
-
-NonFatalCheckError::NonFatalCheckError(const char* msg, const char* file, int line, const char* func)
-    : std::runtime_error{
-          strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\n"
+std::string StrFormatInternalBug(const char* msg, const char* file, int line, const char* func)
+{
+    return strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\n"
                     "%s %s\n"
                     "Please report this issue here: %s\n",
-                    msg, file, line, func, PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT)}
+                    msg, file, line, func, PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
+}
+
+NonFatalCheckError::NonFatalCheckError(const char* msg, const char* file, int line, const char* func)
+    : std::runtime_error{StrFormatInternalBug(msg, file, line, func)}
 {
 }
 
diff --git a/src/util/check.h b/src/util/check.h
index b6c03bed2ac8..b79194450256 100644
--- a/src/util/check.h
+++ b/src/util/check.h
@@ -10,12 +10,16 @@
 #include <stdexcept>
 #include <utility>
 
+std::string StrFormatInternalBug(const char* msg, const char* file, int line, const char* func);
+
 class NonFatalCheckError : public std::runtime_error
 {
 public:
     NonFatalCheckError(const char* msg, const char* file, int line, const char* func);
 };
 
+#define STR_INTERNAL_BUG(msg) StrFormatInternalBug((msg), __FILE__, __LINE__, __func__)
+
 /** Helper for CHECK_NONFATAL() */
 template <typename T>
 T&& inline_check_non_fatal(LIFETIMEBOUND T&& val, const char* file, int line, const char* func, const char* assertion)
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 823c7a37bfd6..bbba86cdac1d 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -1002,6 +1002,12 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
 
     nFeeRet = result->GetSelectedValue() - recipients_sum - change_amount;
 
+    // The only time that fee_needed should be less than the amount available for fees is when
+    // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug.
+    if (!coin_selection_params.m_subtract_fee_outputs && fee_needed > nFeeRet) {
+        return util::Error{Untranslated(STR_INTERNAL_BUG("Fee needed > fee paid"))};
+    }
+
     // Update nFeeRet in case fee_needed changed due to dropping the change output
     if (fee_needed <= change_and_fee - change_amount) {
         nFeeRet = change_and_fee - change_amount;

From 38016eaa3566ca399d654e4209a4e52a08890f80 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Tue, 5 Sep 2023 11:40:21 +0300
Subject: [PATCH 6/7] (Partial)Merge bitcoin/bitcoin#28291: rpc: removed
 StrFormatInternalBug quote delimitation

6e8f6468cbf1320b70cf01333002a31b44cb7c33 removed StrFormatInternalBug quote delimitation (Reese Russell)

Pull request description:

  This PR rectifies an unnecessary set of quotes delimiting the contents of  ```StrFormatInternalBug```. This is a follow up to MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493.  The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format.

  ```STR_INTERNAL_BUG``` was applied to https://github.com/bitcoin/bitcoin/pull/28123 in ```std::string RPCArg::ToString(const bool oneline)``` in ```rpc/util.cpp```

  The results can be seen below.

  Previously
  ![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-9fc1-04255eeb4641)

  This PR
  ![image](https://github.com/bitcoin/bitcoin/assets/3104223/5c6a3110-f1f3-4b3c-8e8a-9c8f1c3176e7)

  Additional context can be found here.
  https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716

  Thank you.

ACKs for top commit:
  MarcoFalke:
    review ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33
  stickies-v:
    ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33

Tree-SHA512: 35317e31a527630495b566407e37db9941dab7f81cfaeb1ea3309683c48e4273284645ad615f73e646a137b4f2ae35933603e9182a7dbdd22cac98d038c491dc
---
 src/util/check.cpp          | 2 +-
 test/functional/rpc_misc.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/util/check.cpp b/src/util/check.cpp
index 5149a2e18a10..4356d400b519 100644
--- a/src/util/check.cpp
+++ b/src/util/check.cpp
@@ -16,7 +16,7 @@
 
 std::string StrFormatInternalBug(const char* msg, const char* file, int line, const char* func)
 {
-    return strprintf("Internal bug detected: \"%s\"\n%s:%d (%s)\n"
+    return strprintf("Internal bug detected: %s\n%s:%d (%s)\n"
                     "%s %s\n"
                     "Please report this issue here: %s\n",
                     msg, file, line, func, PACKAGE_NAME, FormatFullVersion(), PACKAGE_BUGREPORT);
diff --git a/test/functional/rpc_misc.py b/test/functional/rpc_misc.py
index e3f706edcf89..a5c7ff7aafb8 100755
--- a/test/functional/rpc_misc.py
+++ b/test/functional/rpc_misc.py
@@ -27,7 +27,7 @@ def run_test(self):
         self.log.info("test CHECK_NONFATAL")
         assert_raises_rpc_error(
             -1,
-            'Internal bug detected: "request.params[9].get_str() != "trigger_internal_bug""',
+            'Internal bug detected: request.params[9].get_str() != "trigger_internal_bug"',
             lambda: node.echo(arg9='trigger_internal_bug'),
         )
 

From fc5eb9d90e748907af4bfb2ea77f916228656e0b Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Thu, 27 Jul 2023 12:24:57 +0100
Subject: [PATCH 7/7] Merge bitcoin/bitcoin#28164: test: remove unused code in
 `wallet_fundrawtransaction`

108c6255bc670bbf2732f2b79f6c32c26e181208 test: remove unused `totalOut` code (brunoerg)
0fc3deee9a34d2f8e8014da776e6cefc2bd6f664 test: remove unecessary `decoderawtransaction` calls (brunoerg)

Pull request description:

  This PR removes in `wallet_fundrawtransaction`:
  - unecessary variables/calls to `decoderawtransaction`
  - unused `totalOut` variable and its related code (`totalOut` is used in some functions to test change, in other ones its value is not used)

ACKs for top commit:
  kevkevinpal:
    utACK [108c625](https://github.com/bitcoin/bitcoin/pull/28164/commits/108c6255bc670bbf2732f2b79f6c32c26e181208)
  MarcoFalke:
    lgtm ACK 108c6255bc670bbf2732f2b79f6c32c26e181208

Tree-SHA512: c352524f3633146117534c79bd1a24523a7068f13a17d0b8a425cc3c85d62cb769a79ea60db8b075b137da2a0cc43142c43a23ca5af89246ff86cd824e37cf17
---
 test/functional/rpc_fundrawtransaction.py | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py
index 1326ba893be2..377fec97915e 100755
--- a/test/functional/rpc_fundrawtransaction.py
+++ b/test/functional/rpc_fundrawtransaction.py
@@ -169,7 +169,6 @@ def test_simple(self):
         inputs  = [ ]
         outputs = { self.nodes[0].getnewaddress() : 10 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
-        dec_tx  = self.nodes[2].decoderawtransaction(rawtx)
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
         assert len(dec_tx['vin']) > 0  #test that we have enough inputs
@@ -179,8 +178,6 @@ def test_simple_two_coins(self):
         inputs  = [ ]
         outputs = { self.nodes[0].getnewaddress() : 22 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
-        dec_tx  = self.nodes[2].decoderawtransaction(rawtx)
-
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
         assert len(dec_tx['vin']) > 0  #test if we have enough inputs
@@ -192,13 +189,10 @@ def test_simple_two_outputs(self):
         inputs  = [ ]
         outputs = { self.nodes[0].getnewaddress() : 26, self.nodes[1].getnewaddress() : 25 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
-        dec_tx  = self.nodes[2].decoderawtransaction(rawtx)
 
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
-        totalOut = 0
         for out in dec_tx['vout']:
-            totalOut += out['value']
             address = out['scriptPubKey']['address']
             if address in outputs.keys():
                 assert_equal(satoshi_round(outputs[address]), out['value'])
@@ -311,10 +305,8 @@ def test_coin_selection(self):
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx)
 
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
-        totalOut = 0
         matchingOuts = 0
         for i, out in enumerate(dec_tx['vout']):
-            totalOut += out['value']
             if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1
             else:
@@ -342,10 +334,8 @@ def test_two_vin(self):
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True})
 
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
-        totalOut = 0
         matchingOuts = 0
         for out in dec_tx['vout']:
-            totalOut += out['value']
             if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1
 
@@ -376,10 +366,8 @@ def test_two_vin_two_vout(self):
         rawtxfund = self.nodes[2].fundrawtransaction(rawtx, {"add_inputs": True})
 
         dec_tx  = self.nodes[2].decoderawtransaction(rawtxfund['hex'])
-        totalOut = 0
         matchingOuts = 0
         for out in dec_tx['vout']:
-            totalOut += out['value']
             if out['scriptPubKey']['address'] in outputs:
                 matchingOuts+=1