backport: Merge bitcoin#228164, (partial) 28291, 28164, 27888, 28168, (partial) gui#771, gui#742

src/net.cpp

@@ -4459,6 +4459,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();
@@ -4470,7 +4475,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);
     }
 }
 

src/net.h

@@ -1460,6 +1460,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);

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) {

src/qt/bitcoin.cpp

@@ -550,6 +550,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

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

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) {

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);
 };

src/qt/sendcoinsdialog.cpp

@@ -362,7 +362,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

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);

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

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;

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 {

src/test/rpc_tests.cpp

@@ -285,43 +285,14 @@ 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
     BOOST_CHECK_THROW(AmountFromValue(ValueFromString("1e11")), UniValue); //overflow error signless
     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")));

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.  */

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)}
 {
 }
 

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)

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;