From 9e806887a8f9ef63431b28d7dfd0470aa663dd02 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 3 Dec 2020 14:59:27 -0800 Subject: Don't send 'sendaddrv2' to pre-70016 software Github-Pull: #20564 Rebased-From: c5a89196602e43ebb1cdc9cd4f08d153419c13e1 --- src/net_processing.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index c649cf7757..44f6f5d6b6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2367,7 +2367,13 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); // Signal ADDRv2 support (BIP155). - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2)); + if (greatest_common_version >= 70016) { + // BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some + // implementations reject messages they don't know. As a courtesy, don't send + // it to nodes with a version before 70016, as no software is known to support + // BIP155 that doesn't announce at least that protocol version number. + m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2)); + } pfrom.nServices = nServices; pfrom.SetAddrLocal(addrMe); -- cgit v1.2.3 From bead93547067e4b62b44fba335f1d4697119c2d7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 7 Dec 2020 09:12:37 -0800 Subject: Send and require SENDADDRV2 before VERACK See the corresponding BIP change: https://github.com/bitcoin/bips/pull/1043 Github-Pull: #20564 Rebased-From: 1583498fb6781c01ca2f33c09319ed793964c574 --- src/net_processing.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 44f6f5d6b6..98e3d90c2d 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2364,8 +2364,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY)); } - m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); - // Signal ADDRv2 support (BIP155). if (greatest_common_version >= 70016) { // BIP155 defines addrv2 and sendaddrv2 for all protocol versions, but some @@ -2375,6 +2373,8 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2)); } + m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK)); + pfrom.nServices = nServices; pfrom.SetAddrLocal(addrMe); { @@ -2546,6 +2546,17 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat return; } + if (msg_type == NetMsgType::SENDADDRV2) { + if (pfrom.fSuccessfullyConnected) { + // Disconnect peers that send SENDADDRV2 message after VERACK; this + // must be negotiated between VERSION and VERACK. + pfrom.fDisconnect = true; + return; + } + pfrom.m_wants_addrv2 = true; + return; + } + if (!pfrom.fSuccessfullyConnected) { LogPrint(BCLog::NET, "Unsupported message \"%s\" prior to verack from peer=%d\n", SanitizeString(msg_type), pfrom.GetId()); return; @@ -2613,11 +2624,6 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat return; } - if (msg_type == NetMsgType::SENDADDRV2) { - pfrom.m_wants_addrv2 = true; - return; - } - if (msg_type == NetMsgType::SENDHEADERS) { LOCK(cs_main); State(pfrom.GetId())->fPreferHeaders = true; -- cgit v1.2.3 From 06c84232b310e6196c814894537ad935d773fe98 Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 4 Dec 2020 11:20:39 +0100 Subject: wallet, bugfix: allow send to take string fee rate values Github-Pull: #20573 Rebased-From: ce207d6b93d35bc02fcd2dd28f1fd95869261d43 --- src/wallet/rpcwallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9ba10e6c39..e8c3ae888d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4083,7 +4083,7 @@ static RPCHelpMan send() UniValueType(), // outputs (ARR or OBJ, checked later) UniValue::VNUM, // conf_target UniValue::VSTR, // estimate_mode - UniValue::VNUM, // fee_rate + UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode() UniValue::VOBJ, // options }, true ); -- cgit v1.2.3 From 1caa32e3f2a74cd5700a4afe8ecf650f9020fb5c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Mon, 7 Dec 2020 14:37:09 -0800 Subject: Improve heuristic hex transaction decoding Whenever both encodings are permitted, try both, and if only one succeeds, return that one. Otherwise prefer the one for which the heuristic sanity check passes. If that is the case for neither or for both, return the extended-permitting deserialization. Github-Pull: #20595 Rebased-From: 39c42c442044aef611d03ee7053d2dd6df63deb7 --- src/core_read.cpp | 59 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/core_read.cpp b/src/core_read.cpp index 121e62457c..fc02b1b647 100644 --- a/src/core_read.cpp +++ b/src/core_read.cpp @@ -119,31 +119,72 @@ static bool CheckTxScriptsSanity(const CMutableTransaction& tx) static bool DecodeTx(CMutableTransaction& tx, const std::vector& tx_data, bool try_no_witness, bool try_witness) { + // General strategy: + // - Decode both with extended serialization (which interprets the 0x0001 tag as a marker for + // the presense of witnesses) and with legacy serialization (which interprets the tag as a + // 0-input 1-output incomplete transaction). + // - Restricted by try_no_witness (which disables legacy if false) and try_witness (which + // disables extended if false). + // - Ignore serializations that do not fully consume the hex string. + // - If neither succeeds, fail. + // - If only one succeeds, return that one. + // - If both decode attempts succeed: + // - If only one passes the CheckTxScriptsSanity check, return that one. + // - If neither or both pass CheckTxScriptsSanity, return the extended one. + + CMutableTransaction tx_extended, tx_legacy; + bool ok_extended = false, ok_legacy = false; + + // Try decoding with extended serialization support, and remember if the result successfully + // consumes the entire input. if (try_witness) { CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION); try { - ssData >> tx; - // If transaction looks sane, we don't try other mode even if requested - if (ssData.empty() && (!try_no_witness || CheckTxScriptsSanity(tx))) { - return true; - } + ssData >> tx_extended; + if (ssData.empty()) ok_extended = true; } catch (const std::exception&) { // Fall through. } } + // Optimization: if extended decoding succeeded and the result passes CheckTxScriptsSanity, + // don't bother decoding the other way. + if (ok_extended && CheckTxScriptsSanity(tx_extended)) { + tx = std::move(tx_extended); + return true; + } + + // Try decoding with legacy serialization, and remember if the result successfully consumes the entire input. if (try_no_witness) { CDataStream ssData(tx_data, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS); try { - ssData >> tx; - if (ssData.empty()) { - return true; - } + ssData >> tx_legacy; + if (ssData.empty()) ok_legacy = true; } catch (const std::exception&) { // Fall through. } } + // If legacy decoding succeeded and passes CheckTxScriptsSanity, that's our answer, as we know + // at this point that extended decoding either failed or doesn't pass the sanity check. + if (ok_legacy && CheckTxScriptsSanity(tx_legacy)) { + tx = std::move(tx_legacy); + return true; + } + + // If extended decoding succeeded, and neither decoding passes sanity, return the extended one. + if (ok_extended) { + tx = std::move(tx_extended); + return true; + } + + // If legacy decoding succeeded and extended didn't, return the legacy one. + if (ok_legacy) { + tx = std::move(tx_legacy); + return true; + } + + // If none succeeded, we failed. return false; } -- cgit v1.2.3