From e96ed3b6acbf61a2c83bab46cc7a1a8a7d84b30c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 3 Nov 2021 10:44:59 +0530 Subject: [PATCH 01/11] merge bitcoin#15246: Add tests for invalid message headers Co-authored-by: UdjinM6 --- test/functional/p2p_invalid_messages.py | 81 ++++++++++++++++------ test/functional/test_framework/mininode.py | 8 +-- 2 files changed, 63 insertions(+), 26 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index dbf9c057a5..3fdec2a488 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -15,7 +15,7 @@ class msg_unrecognized: command = b'badmsg' - def __init__(self, str_data): + def __init__(self, *, str_data): self.str_data = str_data.encode() if not isinstance(str_data, bytes) else str_data def serialize(self): @@ -25,19 +25,14 @@ class msg_unrecognized: return "{}(data={})".format(self.command, self.str_data) -class msg_nametoolong(msg_unrecognized): - - command = b'thisnameiswayyyyyyyyytoolong' - - class InvalidMessagesTest(BitcoinTestFramework): - def set_test_params(self): self.num_nodes = 1 self.setup_clean_chain = True def run_test(self): """ + . Test msg header 0. Send a bunch of large (3MB) messages of an unrecognized type. Check to see that it isn't an effective DoS against the node. @@ -45,10 +40,12 @@ class InvalidMessagesTest(BitcoinTestFramework): 2. Send a few messages with an incorrect data size in the header, ensure the messages are ignored. - - 3. Send an unrecognized message with a command name longer than 12 characters. - """ + self.test_magic_bytes() + self.test_checksum() + self.test_size() + self.test_command() + node = self.nodes[0] self.node = node node.add_p2p_connection(P2PDataStore()) @@ -63,7 +60,7 @@ class InvalidMessagesTest(BitcoinTestFramework): # Send as large a message as is valid, ensure we aren't disconnected but # also can't exhaust resources. # - msg_at_size = msg_unrecognized("b" * valid_data_limit) + msg_at_size = msg_unrecognized(str_data="b" * valid_data_limit) assert len(msg_at_size.serialize()) == msg_limit with node.assert_memory_usage_stable(increase_allowed=0.5): @@ -90,10 +87,10 @@ class InvalidMessagesTest(BitcoinTestFramework): # # Send an oversized message, ensure we're disconnected. # - msg_over_size = msg_unrecognized("b" * (valid_data_limit + 1)) + msg_over_size = msg_unrecognized(str_data="b" * (valid_data_limit + 1)) assert len(msg_over_size.serialize()) == (msg_limit + 1) - with node.assert_debug_log(["Oversized message from peer=0, disconnecting"]): + with node.assert_debug_log(["Oversized message from peer=4, disconnecting"]): # An unknown message type (or *any* message type) over # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. node.p2p.send_message(msg_over_size) @@ -109,7 +106,7 @@ class InvalidMessagesTest(BitcoinTestFramework): # Send messages with an incorrect data size in the header. # actual_size = 100 - msg = msg_unrecognized("b" * actual_size) + msg = msg_unrecognized(str_data="b" * actual_size) # TODO: handle larger-than cases. I haven't been able to pin down what behavior to expect. for wrong_size in (2, 77, 78, 79): @@ -136,18 +133,58 @@ class InvalidMessagesTest(BitcoinTestFramework): node.disconnect_p2ps() node.add_p2p_connection(P2PDataStore()) - # - # 3. - # - # Send a message with a too-long command name. - # - node.p2p.send_message(msg_nametoolong("foobar")) - node.p2p.wait_for_disconnect(timeout=4) - # Node is still up. conn = node.add_p2p_connection(P2PDataStore()) conn.sync_with_ping() + def test_magic_bytes(self): + conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + conn.magic_bytes = b'\x00\x11\x22\x32' + with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART ping']): + conn.send_message(messages.msg_ping(nonce=0xff)) + conn.wait_for_disconnect(timeout=1) + self.nodes[0].disconnect_p2ps() + + def test_checksum(self): + conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + with self.nodes[0].assert_debug_log(['ProcessMessages(badmsg, 2 bytes): CHECKSUM ERROR expected 78df0a04 was ffffffff']): + msg = conn.build_message(msg_unrecognized(str_data="d")) + cut_len = ( + 4 + # magic + 12 + # command + 4 #len + ) + # modify checksum + msg = msg[:cut_len] + b'\xff' * 4 + msg[cut_len + 4:] + self.nodes[0].p2p.send_raw_message(msg) + conn.sync_with_ping(timeout=1) + self.nodes[0].disconnect_p2ps() + + def test_size(self): + conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + with self.nodes[0].assert_debug_log(['']): + msg = conn.build_message(msg_unrecognized(str_data="d")) + cut_len = ( + 4 + # magic + 12 # command + ) + # modify len to MAX_SIZE + 1 + msg = msg[:cut_len] + struct.pack(" Date: Sat, 2 Feb 2019 17:49:28 -0500 Subject: [PATCH 02/11] merge bitcoin#15330: Fix race in p2p_invalid_messages --- test/functional/p2p_invalid_messages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 3fdec2a488..b0e8afc3b9 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2015-2018 The Bitcoin Core developers +# Copyright (c) 2015-2019 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test node responses to invalid network messages.""" @@ -139,6 +139,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_magic_bytes(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) + conn._on_data = lambda: None # Need to ignore all incoming messages from now, since they come with "invalid" magic bytes conn.magic_bytes = b'\x00\x11\x22\x32' with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART ping']): conn.send_message(messages.msg_ping(nonce=0xff)) @@ -207,6 +208,5 @@ class InvalidMessagesTest(BitcoinTestFramework): return raw_msg_with_wrong_size - if __name__ == '__main__': InvalidMessagesTest().main() From a5445a4584df25095136fa252d1e0cf805ac10f9 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 5 Aug 2021 20:20:33 +0530 Subject: [PATCH 03/11] merge bitcoin#15697: Make swap_magic_bytes in p2p_invalid_messages atomic --- test/functional/p2p_invalid_messages.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index b0e8afc3b9..74d28bece4 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -3,10 +3,11 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test node responses to invalid network messages.""" +import asyncio import struct from test_framework import messages -from test_framework.mininode import P2PDataStore +from test_framework.mininode import P2PDataStore, NetworkThread from test_framework.test_framework import BitcoinTestFramework @@ -139,8 +140,15 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_magic_bytes(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - conn._on_data = lambda: None # Need to ignore all incoming messages from now, since they come with "invalid" magic bytes - conn.magic_bytes = b'\x00\x11\x22\x32' + + def swap_magic_bytes(): + conn._on_data = lambda: None # Need to ignore all incoming messages from now, since they come with "invalid" magic bytes + conn.magic_bytes = b'\x00\x11\x22\x32' + + # Call .result() to block until the atomic swap is complete, otherwise + # we might run into races later on + asyncio.run_coroutine_threadsafe(asyncio.coroutine(swap_magic_bytes)(), NetworkThread.network_event_loop).result() + with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART ping']): conn.send_message(messages.msg_ping(nonce=0xff)) conn.wait_for_disconnect(timeout=1) From 907101098d6e09537bdb451405b935ce01946b55 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Tue, 23 Jul 2019 15:10:36 -0400 Subject: [PATCH 04/11] merge bitcoin#16445: Skip flaky p2p_invalid_messages test on macOS --- test/functional/p2p_invalid_messages.py | 28 ++++++++++++++++--------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 74d28bece4..8378cab912 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -5,6 +5,7 @@ """Test node responses to invalid network messages.""" import asyncio import struct +import sys from test_framework import messages from test_framework.mininode import P2PDataStore, NetworkThread @@ -88,18 +89,25 @@ class InvalidMessagesTest(BitcoinTestFramework): # # Send an oversized message, ensure we're disconnected. # - msg_over_size = msg_unrecognized(str_data="b" * (valid_data_limit + 1)) - assert len(msg_over_size.serialize()) == (msg_limit + 1) + # Under macOS this test is skipped due to an unexpected error code + # returned from the closing socket which python/asyncio does not + # yet know how to handle. + # + if sys.platform != 'darwin': + msg_over_size = msg_unrecognized(str_data="b" * (valid_data_limit + 1)) + assert len(msg_over_size.serialize()) == (msg_limit + 1) - with node.assert_debug_log(["Oversized message from peer=4, disconnecting"]): - # An unknown message type (or *any* message type) over - # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. - node.p2p.send_message(msg_over_size) - node.p2p.wait_for_disconnect(timeout=4) + with node.assert_debug_log(["Oversized message from peer=4, disconnecting"]): + # An unknown message type (or *any* message type) over + # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. + node.p2p.send_message(msg_over_size) + node.p2p.wait_for_disconnect(timeout=4) - node.disconnect_p2ps() - conn = node.add_p2p_connection(P2PDataStore()) - conn.wait_for_verack() + node.disconnect_p2ps() + conn = node.add_p2p_connection(P2PDataStore()) + conn.wait_for_verack() + else: + self.log.info("Skipping test p2p_invalid_messages/1 (oversized message) under macOS") # # 2. From c0d4df0a0662715dd0e8f593765687b79f389b46 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 13 Jun 2019 10:39:44 +0200 Subject: [PATCH 05/11] merge bitcoin#16202: Refactor network message deserialization Co-authored-by: UdjinM6 --- src/net.cpp | 91 +++++++++++++++---------- src/net.h | 83 +++++++++++++++++----- src/net_processing.cpp | 32 ++++----- test/functional/p2p_invalid_messages.py | 11 ++- 4 files changed, 139 insertions(+), 78 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index d30c9f5fd8..04bc6b71fd 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -653,44 +653,29 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete nLastRecv = nTimeMicros / 1000000; nRecvBytes += nBytes; while (nBytes > 0) { - - // get current incomplete message, or create a new one - if (vRecvMsg.empty() || - vRecvMsg.back().complete()) - vRecvMsg.push_back(CNetMessage(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION)); - - CNetMessage& msg = vRecvMsg.back(); - // absorb network data - int handled; - if (!msg.in_data) { - handled = msg.readHeader(pch, nBytes); - } else { - handled = msg.readData(pch, nBytes); - } - - if (handled < 0) - return false; - - if (msg.in_data && msg.hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) { - LogPrint(BCLog::NET, "Oversized message from peer=%i, disconnecting\n", GetId()); - return false; - } + int handled = m_deserializer->Read(pch, nBytes); + if (handled < 0) return false; pch += handled; nBytes -= handled; - if (msg.complete()) { + if (m_deserializer->Complete()) { + // decompose a transport agnostic CNetMessage from the deserializer + CNetMessage msg = m_deserializer->GetMessage(Params().MessageStart(), nTimeMicros); + //store received bytes per message command //to prevent a memory DOS, only allow valid commands - mapMsgCmdSize::iterator i = mapRecvBytesPerMsgCmd.find(msg.hdr.pchCommand); + mapMsgCmdSize::iterator i = mapRecvBytesPerMsgCmd.find(msg.m_command); if (i == mapRecvBytesPerMsgCmd.end()) i = mapRecvBytesPerMsgCmd.find(NET_MESSAGE_COMMAND_OTHER); assert(i != mapRecvBytesPerMsgCmd.end()); - i->second += msg.hdr.nMessageSize + CMessageHeader::HEADER_SIZE; - statsClient.count("bandwidth.message." + std::string(msg.hdr.pchCommand) + ".bytesReceived", msg.hdr.nMessageSize + CMessageHeader::HEADER_SIZE, 1.0f); + i->second += msg.m_raw_message_size; + statsClient.count("bandwidth.message." + std::string(msg.m_command) + ".bytesReceived", msg.m_raw_message_size, 1.0f); + + // push the message to the process queue, + vRecvMsg.push_back(std::move(msg)); - msg.nTime = nTimeMicros; complete = true; } } @@ -724,8 +709,7 @@ int CNode::GetSendVersion() const return nSendVersion; } - -int CNetMessage::readHeader(const char *pch, unsigned int nBytes) +int V1TransportDeserializer::readHeader(const char *pch, unsigned int nBytes) { // copy data to temporary parsing buffer unsigned int nRemaining = 24 - nHdrPos; @@ -746,9 +730,10 @@ int CNetMessage::readHeader(const char *pch, unsigned int nBytes) return -1; } - // reject messages larger than MAX_SIZE - if (hdr.nMessageSize > MAX_SIZE) + // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH + if (hdr.nMessageSize > MAX_SIZE || hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) { return -1; + } // switch state to reading message data in_data = true; @@ -756,7 +741,7 @@ int CNetMessage::readHeader(const char *pch, unsigned int nBytes) return nCopy; } -int CNetMessage::readData(const char *pch, unsigned int nBytes) +int V1TransportDeserializer::readData(const char *pch, unsigned int nBytes) { unsigned int nRemaining = hdr.nMessageSize - nDataPos; unsigned int nCopy = std::min(nRemaining, nBytes); @@ -773,14 +758,44 @@ int CNetMessage::readData(const char *pch, unsigned int nBytes) return nCopy; } -const uint256& CNetMessage::GetMessageHash() const +const uint256& V1TransportDeserializer::GetMessageHash() const { - assert(complete()); + assert(Complete()); if (data_hash.IsNull()) hasher.Finalize(data_hash); return data_hash; } +CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) { + // decompose a single CNetMessage from the TransportDeserializer + CNetMessage msg(std::move(vRecv)); + + // store state about valid header, netmagic and checksum + msg.m_valid_header = hdr.IsValid(message_start); + msg.m_valid_netmagic = (memcmp(hdr.pchMessageStart, message_start, CMessageHeader::MESSAGE_START_SIZE) == 0); + uint256 hash = GetMessageHash(); + + // store command string, payload size + msg.m_command = hdr.GetCommand(); + msg.m_message_size = hdr.nMessageSize; + msg.m_raw_message_size = hdr.nMessageSize + CMessageHeader::HEADER_SIZE; + + msg.m_valid_checksum = (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) == 0); + if (!msg.m_valid_checksum) { + LogPrint(BCLog::NET, "CHECKSUM ERROR (%s, %u bytes), expected %s was %s\n", + SanitizeString(msg.m_command), msg.m_message_size, + HexStr(Span(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE)), + HexStr(Span(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE))); + } + + // store receive time + msg.m_time = time; + + // reset the network deserializer (prepare for the next message) + Reset(); + return msg; +} + size_t CConnman::SocketSendData(CNode *pnode) EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_vSend) { auto it = pnode->vSendMsg.begin(); @@ -1828,9 +1843,9 @@ size_t CConnman::SocketRecvData(CNode *pnode) size_t nSizeAdded = 0; auto it(pnode->vRecvMsg.begin()); for (; it != pnode->vRecvMsg.end(); ++it) { - if (!it->complete()) - break; - nSizeAdded += it->vRecv.size() + CMessageHeader::HEADER_SIZE; + // vRecvMsg contains only completed CNetMessage + // the single possible partially deserialized message are held by TransportDeserializer + nSizeAdded += it->m_raw_message_size; } { LOCK(pnode->cs_vProcessMsg); @@ -3732,6 +3747,8 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn } else { LogPrint(BCLog::NET, "Added connection peer=%d\n", id); } + + m_deserializer = MakeUnique(V1TransportDeserializer(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION)); } CNode::~CNode() diff --git a/src/net.h b/src/net.h index 95492602ba..dc338b676a 100644 --- a/src/net.h +++ b/src/net.h @@ -782,56 +782,105 @@ public: - +/** Transport protocol agnostic message container. + * Ideally it should only contain receive time, payload, + * command and size. + */ class CNetMessage { +public: + CDataStream m_recv; // received message data + int64_t m_time = 0; // time (in microseconds) of message receipt. + bool m_valid_netmagic = false; + bool m_valid_header = false; + bool m_valid_checksum = false; + uint32_t m_message_size = 0; // size of the payload + uint32_t m_raw_message_size = 0; // used wire size of the message (including header/checksum) + std::string m_command; + + CNetMessage(CDataStream&& recv_in) : m_recv(std::move(recv_in)) {} + + void SetVersion(int nVersionIn) + { + m_recv.SetVersion(nVersionIn); + } +}; + +/** The TransportDeserializer takes care of holding and deserializing the + * network receive buffer. It can deserialize the network buffer into a + * transport protocol agnostic CNetMessage (command & payload) + */ +class TransportDeserializer { +public: + // returns true if the current deserialization is complete + virtual bool Complete() const = 0; + // set the serialization context version + virtual void SetVersion(int version) = 0; + // read and deserialize data + virtual int Read(const char *data, unsigned int bytes) = 0; + // decomposes a message from the context + virtual CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) = 0; + virtual ~TransportDeserializer() {} +}; + +class V1TransportDeserializer final : public TransportDeserializer +{ private: mutable CHash256 hasher; mutable uint256 data_hash; -public: bool in_data; // parsing header (false) or data (true) - CDataStream hdrbuf; // partially received header CMessageHeader hdr; // complete header - unsigned int nHdrPos; - CDataStream vRecv; // received message data + unsigned int nHdrPos; unsigned int nDataPos; - int64_t nTime; // time (in microseconds) of message receipt. + const uint256& GetMessageHash() const; + int readHeader(const char *pch, unsigned int nBytes); + int readData(const char *pch, unsigned int nBytes); - CNetMessage(const CMessageHeader::MessageStartChars& pchMessageStartIn, int nTypeIn, int nVersionIn) : hdrbuf(nTypeIn, nVersionIn), hdr(pchMessageStartIn), vRecv(nTypeIn, nVersionIn) { + void Reset() { + vRecv.clear(); + hdrbuf.clear(); hdrbuf.resize(24); in_data = false; nHdrPos = 0; nDataPos = 0; - nTime = 0; + data_hash.SetNull(); + hasher.Reset(); } - bool complete() const +public: + + V1TransportDeserializer(const CMessageHeader::MessageStartChars& pchMessageStartIn, int nTypeIn, int nVersionIn) : hdrbuf(nTypeIn, nVersionIn), hdr(pchMessageStartIn), vRecv(nTypeIn, nVersionIn) { + Reset(); + } + + bool Complete() const override { if (!in_data) return false; return (hdr.nMessageSize == nDataPos); } - - const uint256& GetMessageHash() const; - - void SetVersion(int nVersionIn) + void SetVersion(int nVersionIn) override { hdrbuf.SetVersion(nVersionIn); vRecv.SetVersion(nVersionIn); } - - int readHeader(const char *pch, unsigned int nBytes); - int readData(const char *pch, unsigned int nBytes); + int Read(const char *pch, unsigned int nBytes) override { + int ret = in_data ? readData(pch, nBytes) : readHeader(pch, nBytes); + if (ret < 0) Reset(); + return ret; + } + CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) override; }; - /** Information about a peer */ class CNode { friend class CConnman; public: + std::unique_ptr m_deserializer; + // socket std::atomic nServices{NODE_NONE}; SOCKET hSocket GUARDED_BY(cs_hSocket); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 284a66a315..92e68b39c4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -4026,41 +4026,37 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter return false; // Just take one message msgs.splice(msgs.begin(), pfrom->vProcessMsg, pfrom->vProcessMsg.begin()); - pfrom->nProcessQueueSize -= msgs.front().vRecv.size() + CMessageHeader::HEADER_SIZE; + pfrom->nProcessQueueSize -= msgs.front().m_raw_message_size; pfrom->fPauseRecv = pfrom->nProcessQueueSize > connman->GetReceiveFloodSize(); fMoreWork = !pfrom->vProcessMsg.empty(); } CNetMessage& msg(msgs.front()); msg.SetVersion(pfrom->GetRecvVersion()); - // Scan for message start - if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) { - LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId()); + // Check network magic + if (!msg.m_valid_netmagic) { + LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId()); pfrom->fDisconnect = true; return false; } - // Read header - CMessageHeader& hdr = msg.hdr; - if (!hdr.IsValid(chainparams.MessageStart())) + // Check header + if (!msg.m_valid_header) { - LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(hdr.GetCommand()), pfrom->GetId()); + LogPrint(BCLog::NET, "PROCESSMESSAGE: ERRORS IN HEADER %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId()); return fMoreWork; } - std::string strCommand = hdr.GetCommand(); + const std::string& strCommand = msg.m_command; // Message size - unsigned int nMessageSize = hdr.nMessageSize; + unsigned int nMessageSize = msg.m_message_size; // Checksum - CDataStream& vRecv = msg.vRecv; - uint256 hash = msg.GetMessageHash(); - if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0) + CDataStream& vRecv = msg.m_recv; + if (!msg.m_valid_checksum) { - LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__, - SanitizeString(strCommand), nMessageSize, - HexStr(Span(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)), - HexStr(hdr.pchChecksum)); + LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR peer=%d\n", __func__, + SanitizeString(strCommand), nMessageSize, pfrom->GetId()); return fMoreWork; } @@ -4068,7 +4064,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic& inter bool fRet = false; try { - fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc, m_enable_bip61); + fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.m_time, chainparams, connman, interruptMsgProc, m_enable_bip61); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 8378cab912..19f9258dbd 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -97,11 +97,10 @@ class InvalidMessagesTest(BitcoinTestFramework): msg_over_size = msg_unrecognized(str_data="b" * (valid_data_limit + 1)) assert len(msg_over_size.serialize()) == (msg_limit + 1) - with node.assert_debug_log(["Oversized message from peer=4, disconnecting"]): - # An unknown message type (or *any* message type) over - # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. - node.p2p.send_message(msg_over_size) - node.p2p.wait_for_disconnect(timeout=4) + # An unknown message type (or *any* message type) over + # MAX_PROTOCOL_MESSAGE_LENGTH should result in a disconnect. + node.p2p.send_message(msg_over_size) + node.p2p.wait_for_disconnect(timeout=4) node.disconnect_p2ps() conn = node.add_p2p_connection(P2PDataStore()) @@ -164,7 +163,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_checksum(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['ProcessMessages(badmsg, 2 bytes): CHECKSUM ERROR expected 78df0a04 was ffffffff']): + with self.nodes[0].assert_debug_log(['CHECKSUM ERROR (badmsg, 2 bytes), expected 78df0a04 was ffffffff']): msg = conn.build_message(msg_unrecognized(str_data="d")) cut_len = ( 4 + # magic From 31cdcb64b83e378ae23d412d67e94cce0b71e25b Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 7 Aug 2019 15:56:24 +0200 Subject: [PATCH 06/11] merge bitcoin#16562: Refactor message transport packaging --- src/net.cpp | 31 +++++++++++++++++++++---------- src/net.h | 15 +++++++++++++++ 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 04bc6b71fd..1181dfc5e0 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -796,6 +796,19 @@ CNetMessage V1TransportDeserializer::GetMessage(const CMessageHeader::MessageSta return msg; } +void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vector& header) { + // create dbl-sha256 checksum + uint256 hash = Hash(msg.data.begin(), msg.data.end()); + + // create header + CMessageHeader hdr(Params().MessageStart(), msg.command.c_str(), msg.data.size()); + memcpy(hdr.pchChecksum, hash.begin(), CMessageHeader::CHECKSUM_SIZE); + + // serialize header + header.reserve(CMessageHeader::HEADER_SIZE); + CVectorWriter{SER_NETWORK, INIT_PROTO_VERSION, header, 0, hdr}; +} + size_t CConnman::SocketSendData(CNode *pnode) EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_vSend) { auto it = pnode->vSendMsg.begin(); @@ -3749,6 +3762,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn } m_deserializer = MakeUnique(V1TransportDeserializer(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION)); + m_serializer = MakeUnique(V1TransportSerializer()); } CNode::~CNode() @@ -3764,19 +3778,16 @@ bool CConnman::NodeFullyConnected(const CNode* pnode) void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) { size_t nMessageSize = msg.data.size(); - size_t nTotalSize = nMessageSize + CMessageHeader::HEADER_SIZE; - LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", SanitizeString(msg.command.c_str()), nMessageSize, pnode->GetId()); + LogPrint(BCLog::NET, "sending %s (%d bytes) peer=%d\n", SanitizeString(msg.command.c_str()), nMessageSize, pnode->GetId()); + + // make sure we use the appropriate network transport format + std::vector serializedHeader; + pnode->m_serializer->prepareForTransport(msg, serializedHeader); + + size_t nTotalSize = nMessageSize + serializedHeader.size(); statsClient.count("bandwidth.message." + SanitizeString(msg.command.c_str()) + ".bytesSent", nTotalSize, 1.0f); statsClient.inc("message.sent." + SanitizeString(msg.command.c_str()), 1.0f); - std::vector serializedHeader; - serializedHeader.reserve(CMessageHeader::HEADER_SIZE); - uint256 hash = Hash(msg.data.data(), msg.data.data() + nMessageSize); - CMessageHeader hdr(Params().MessageStart(), msg.command.c_str(), nMessageSize); - memcpy(hdr.pchChecksum, hash.begin(), CMessageHeader::CHECKSUM_SIZE); - - CVectorWriter{SER_NETWORK, INIT_PROTO_VERSION, serializedHeader, 0, hdr}; - size_t nBytesSent = 0; { LOCK(pnode->cs_vSend); diff --git a/src/net.h b/src/net.h index dc338b676a..417ea31ff9 100644 --- a/src/net.h +++ b/src/net.h @@ -874,12 +874,27 @@ public: CNetMessage GetMessage(const CMessageHeader::MessageStartChars& message_start, int64_t time) override; }; +/** The TransportSerializer prepares messages for the network transport + */ +class TransportSerializer { +public: + // prepare message for transport (header construction, error-correction computation, payload encryption, etc.) + virtual void prepareForTransport(CSerializedNetMsg& msg, std::vector& header) = 0; + virtual ~TransportSerializer() {} +}; + +class V1TransportSerializer : public TransportSerializer { +public: + void prepareForTransport(CSerializedNetMsg& msg, std::vector& header) override; +}; + /** Information about a peer */ class CNode { friend class CConnman; public: std::unique_ptr m_deserializer; + std::unique_ptr m_serializer; // socket std::atomic nServices{NODE_NONE}; From d949c90bc098101edc3fe09b0afd5ad6009acf41 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Tue, 3 Mar 2020 08:35:01 -0500 Subject: [PATCH 07/11] merge bitcoin#18260: Fix implicit value conversion in formatPingTime --- src/net.cpp | 6 +++--- src/net.h | 6 +++--- src/qt/guiutil.cpp | 4 ++-- src/qt/guiutil.h | 4 ++-- src/qt/peertablemodel.cpp | 4 ++-- src/qt/rpcconsole.cpp | 6 +++--- src/rpc/net.cpp | 15 +++++++++------ 7 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 1181dfc5e0..907db7dbe4 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -628,9 +628,9 @@ void CNode::copyStats(CNodeStats &stats, const std::vector &m_asmap) } // Raw ping time is in microseconds, but show it to user as whole seconds (Dash users should be well used to small numbers with many decimal places by now :) - stats.dPingTime = (((double)nPingUsecTime) / 1e6); - stats.dMinPing = (((double)nMinPingUsecTime) / 1e6); - stats.dPingWait = (((double)nPingUsecWait) / 1e6); + stats.m_ping_usec = nPingUsecTime; + stats.m_min_ping_usec = nMinPingUsecTime; + stats.m_ping_wait_usec = nPingUsecWait; // Leave string empty if addrLocal invalid (not filled in yet) CService addrLocalUnlocked = GetAddrLocal(); diff --git a/src/net.h b/src/net.h index 417ea31ff9..6b00e12aa3 100644 --- a/src/net.h +++ b/src/net.h @@ -763,9 +763,9 @@ public: mapMsgCmdSize mapRecvBytesPerMsgCmd; NetPermissionFlags m_permissionFlags; bool m_legacyWhitelisted; - double dPingTime; - double dPingWait; - double dMinPing; + int64_t m_ping_usec; + int64_t m_ping_wait_usec; + int64_t m_min_ping_usec; // Our address, as reported by the peer std::string addrLocal; // Address of this peer diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index e1bfd0287b..7192ffacd8 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -1757,9 +1757,9 @@ QString formatServicesStr(quint64 mask) return QObject::tr("None"); } -QString formatPingTime(double dPingTime) +QString formatPingTime(int64_t ping_usec) { - return (dPingTime == std::numeric_limits::max()/1e6 || dPingTime == 0) ? QObject::tr("N/A") : QString(QObject::tr("%1 ms")).arg(QString::number((int)(dPingTime * 1000), 10)); + return (ping_usec == std::numeric_limits::max() || ping_usec == 0) ? QObject::tr("N/A") : QString(QObject::tr("%1 ms")).arg(QString::number((int)(ping_usec / 1000), 10)); } QString formatTimeOffset(int64_t nTimeOffset) diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 516e4d4e7b..4c81c54635 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -407,8 +407,8 @@ namespace GUIUtil /* Format CNodeStats.nServices bitmask into a user-readable string */ QString formatServicesStr(quint64 mask); - /* Format a CNodeCombinedStats.dPingTime into a user-readable string or display N/A, if 0*/ - QString formatPingTime(double dPingTime); + /* Format a CNodeStats.m_ping_usec into a user-readable string or display N/A, if 0*/ + QString formatPingTime(int64_t ping_usec); /* Format a CNodeCombinedStats.nTimeOffset into a user-readable string. */ QString formatTimeOffset(int64_t nTimeOffset); diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp index 4f77b42f09..13c8a9e7bc 100644 --- a/src/qt/peertablemodel.cpp +++ b/src/qt/peertablemodel.cpp @@ -35,7 +35,7 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombine case PeerTableModel::Subversion: return pLeft->cleanSubVer.compare(pRight->cleanSubVer) < 0; case PeerTableModel::Ping: - return pLeft->dMinPing < pRight->dMinPing; + return pLeft->m_min_ping_usec < pRight->m_min_ping_usec; case PeerTableModel::Sent: return pLeft->nSendBytes < pRight->nSendBytes; case PeerTableModel::Received: @@ -170,7 +170,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const case Subversion: return QString::fromStdString(rec->nodeStats.cleanSubVer); case Ping: - return GUIUtil::formatPingTime(rec->nodeStats.dMinPing); + return GUIUtil::formatPingTime(rec->nodeStats.m_min_ping_usec); case Sent: return GUIUtil::formatBytes(rec->nodeStats.nSendBytes); case Received: diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index a2ef1c43ff..f814d9c4b1 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1253,9 +1253,9 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes)); ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes)); ui->peerConnTime->setText(GUIUtil::formatDurationStr(GetSystemTimeInSeconds() - stats->nodeStats.nTimeConnected)); - ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingTime)); - ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingWait)); - ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.dMinPing)); + ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_usec)); + ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_wait_usec)); + ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_usec)); ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset)); ui->peerVersion->setText(QString("%1").arg(QString::number(stats->nodeStats.nVersion))); ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer)); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 9460af4894..b378e29ae6 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -178,12 +178,15 @@ static UniValue getpeerinfo(const JSONRPCRequest& request) obj.pushKV("bytesrecv", stats.nRecvBytes); obj.pushKV("conntime", stats.nTimeConnected); obj.pushKV("timeoffset", stats.nTimeOffset); - if (stats.dPingTime > 0.0) - obj.pushKV("pingtime", stats.dPingTime); - if (stats.dMinPing < static_cast(std::numeric_limits::max())/1e6) - obj.pushKV("minping", stats.dMinPing); - if (stats.dPingWait > 0.0) - obj.pushKV("pingwait", stats.dPingWait); + if (stats.m_ping_usec > 0) { + obj.pushKV("pingtime", ((double)stats.m_ping_usec) / 1e6); + } + if (stats.m_min_ping_usec < std::numeric_limits::max()) { + obj.pushKV("minping", ((double)stats.m_min_ping_usec) / 1e6); + } + if (stats.m_ping_wait_usec > 0) { + obj.pushKV("pingwait", ((double)stats.m_ping_wait_usec) / 1e6); + } obj.pushKV("version", stats.nVersion); // Use the sanitized form of subver here, to avoid tricksy remote peers from // corrupting or modifying the JSON output by putting special characters in From df06dfa435a96c3d28bed61bf81dc1e2ee13d375 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Fri, 18 Jun 2021 13:25:17 -0700 Subject: [PATCH 08/11] merge bitcoin#22331: Fix K1/K2 use in ChaCha20-Poly1305 AEAD --- src/crypto/chacha_poly_aead.cpp | 5 +++-- src/test/crypto_tests.cpp | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/crypto/chacha_poly_aead.cpp b/src/crypto/chacha_poly_aead.cpp index 6a3d43deb1..259736cf1e 100644 --- a/src/crypto/chacha_poly_aead.cpp +++ b/src/crypto/chacha_poly_aead.cpp @@ -32,8 +32,9 @@ ChaCha20Poly1305AEAD::ChaCha20Poly1305AEAD(const unsigned char* K_1, size_t K_1_ { assert(K_1_len == CHACHA20_POLY1305_AEAD_KEY_LEN); assert(K_2_len == CHACHA20_POLY1305_AEAD_KEY_LEN); - m_chacha_main.SetKey(K_1, CHACHA20_POLY1305_AEAD_KEY_LEN); - m_chacha_header.SetKey(K_2, CHACHA20_POLY1305_AEAD_KEY_LEN); + + m_chacha_header.SetKey(K_1, CHACHA20_POLY1305_AEAD_KEY_LEN); + m_chacha_main.SetKey(K_2, CHACHA20_POLY1305_AEAD_KEY_LEN); // set the cached sequence number to uint64 max which hints for an unset cache. // we can't hit uint64 max since the rekey rule (which resets the sequence number) is 1GB diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index 911c417692..ae996ffcb0 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -639,7 +639,7 @@ static void TestChaCha20Poly1305AEAD(bool must_succeed, unsigned int expected_aa ChaCha20Poly1305AEAD aead(aead_K_1.data(), aead_K_1.size(), aead_K_2.data(), aead_K_2.size()); // create a chacha20 instance to compare against - ChaCha20 cmp_ctx(aead_K_2.data(), 32); + ChaCha20 cmp_ctx(aead_K_1.data(), 32); // encipher bool res = aead.Crypt(seqnr_payload, seqnr_aad, aad_pos, ciphertext_buf.data(), ciphertext_buf.size(), plaintext_buf.data(), plaintext_buf.size(), true); @@ -730,8 +730,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector) "b1a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3a8bd94d54b5ecabbc41ffbb0c90924080"); TestChaCha20Poly1305AEAD(true, 255, "ff0000f195e66982105ffb640bb7757f579da31602fc93ec01ac56f85ac3c134a4547b733b46413042c9440049176905d3be59ea1c53f15916155c2be8241a38008b9a26bc35941e2444177c8ade6689de95264986d95889fb60e84629c9bd9a5acb1cc118be563eb9b3a4a472f82e09a7e778492b562ef7130e88dfe031c79db9d4f7c7a899151b9a475032b63fc385245fe054e3dd5a97a5f576fe064025d3ce042c566ab2c507b138db853e3d6959660996546cc9c4a6eafdc777c040d70eaf46f76dad3979e5c5360c3317166a1c894c94a371876a94df7628fe4eaaf2ccb27d5aaae0ad7ad0f9d4b6ad3b54098746d4524d38407a6deb3ab78fab78c9", - "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "ff0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", + "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "c640c1711e3ee904ac35c57ab9791c8a1c408603a90b77a83b54f6c844cb4b06d94e7fc6c800e165acd66147e80ec45a567f6ce66d05ec0cae679dceeb890017", "3940c1e92da4582ff6f92a776aeb14d014d384eeb30f660dacf70a14a23fd31e91212701334e2ce1acf5199dc84f4d61ddbe6571bca5af874b4c9226c26e650995d157644e1848b96ed6c2102d5489a050e71d29a5a66ece11de5fb5c9558d54da28fe45b0bc4db4e5b88030bfc4a352b4b7068eccf656bae7ad6a35615315fc7c49d4200388d5eca67c2e822e069336c69b40db67e0f3c81209c50f3216a4b89fb3ae1b984b7851a2ec6f68ab12b101ab120e1ea7313bb93b5a0f71185c7fea017ddb92769861c29dba4fbc432280d5dff21b36d1c4c790128b22699950bb18bf74c448cdfe547d8ed4f657d8005fdc0cd7a050c2d46050a44c4376355858981fbe8b184288276e7a93eabc899c4a", "f039c6689eaeef0456685200feaab9d54bbd9acde4410a3b6f4321296f4a8ca2604b49727d8892c57e005d799b2a38e85e809f20146e08eec75169691c8d4f54a0d51a1e1c7b381e0474eb02f994be9415ef3ffcbd2343f0601e1f3b172a1d494f838824e4df570f8e3b0c04e27966e36c82abd352d07054ef7bd36b84c63f9369afe7ed79b94f953873006b920c3fa251a771de1b63da927058ade119aa898b8c97e42a606b2f6df1e2d957c22f7593c1e2002f4252f4c9ae4bf773499e5cfcfe14dfc1ede26508953f88553bf4a76a802f6a0068d59295b01503fd9a600067624203e880fdf53933b96e1f4d9eb3f4e363dd8165a278ff667a41ee42b9892b077cefff92b93441f7be74cf10e6cd"); From bb30e9f0072032c6585a8976e9ff23e74281e4d1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Fri, 18 Jun 2021 13:25:17 -0700 Subject: [PATCH 09/11] merge bitcoin#23271: Fix K1/K2 use in the comments in ChaCha20-Poly1305 AEAD --- src/crypto/chacha_poly_aead.h | 4 ++-- src/test/crypto_tests.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/crypto/chacha_poly_aead.h b/src/crypto/chacha_poly_aead.h index b3ba781cdd..3fd759a117 100644 --- a/src/crypto/chacha_poly_aead.h +++ b/src/crypto/chacha_poly_aead.h @@ -117,8 +117,8 @@ static constexpr int AAD_PACKAGES_PER_ROUND = 21; /* 64 / 3 round down*/ class ChaCha20Poly1305AEAD { private: - ChaCha20 m_chacha_main; // payload and poly1305 key-derivation cipher instance - ChaCha20 m_chacha_header; // AAD cipher instance (encrypted length) + ChaCha20 m_chacha_header; // AAD cipher instance (encrypted length) and poly1305 key-derivation cipher instance + ChaCha20 m_chacha_main; // payload unsigned char m_aad_keystream_buffer[CHACHA20_ROUND_OUTPUT]; // aad keystream cache uint64_t m_cached_aad_seqnr; // aad keystream cache hint diff --git a/src/test/crypto_tests.cpp b/src/test/crypto_tests.cpp index ae996ffcb0..1731677535 100644 --- a/src/test/crypto_tests.cpp +++ b/src/test/crypto_tests.cpp @@ -716,8 +716,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector) TestChaCha20Poly1305AEAD(true, 0, /* m */ "0000000000000000000000000000000000000000000000000000000000000000", - /* k1 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000", - /* k2 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000", + /* k1 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000", + /* k2 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000", /* AAD keystream */ "76b8e0ada0f13d90405d6ae55386bd28bdd219b8a08ded1aa836efcc8b770dc7da41597c5157488d7724e03fb8d84a376a43b8f41518a11cc387b669b2ee6586", /* encrypted message & MAC */ "76b8e09f07e7be5551387a98ba977c732d080dcb0f29a048e3656912c6533e32d2fc11829c1b6c1df1f551cd6131ff08", /* encrypted message & MAC at sequence 999 */ "b0a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3aaa7aa16ec62c5e24f040c08bb20c3598"); From 4caa6d394caeb08fca6e3ece22829b965d1dc0cd Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Thu, 20 Dec 2018 06:44:46 +1300 Subject: [PATCH 10/11] merge bitcoin#16493: Fix test failures --- test/functional/feature_dbcrash.py | 21 ++++++++++++++++----- test/functional/feature_fee_estimation.py | 11 ++++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py index a544801238..37c120b21d 100755 --- a/test/functional/feature_dbcrash.py +++ b/test/functional/feature_dbcrash.py @@ -30,17 +30,27 @@ import http.client import random import time -from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut, ToHex +from test_framework.messages import ( + COIN, + COutPoint, + CTransaction, + CTxIn, + CTxOut, + ToHex, +) from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, create_confirmed_utxos, hex_str_to_bytes +from test_framework.util import ( + assert_equal, + create_confirmed_utxos, + hex_str_to_bytes, +) class ChainstateWriteCrashTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 self.setup_clean_chain = False - # Need a bit of extra time for the nodes to start up for this test - self.rpc_timeout = 90 + self.rpc_timeout = 180 # Set -maxmempool=0 to turn off mempool memory sharing with dbcache # Set -rpcservertimeout=900 to reduce socket disconnects in this @@ -54,7 +64,8 @@ class ChainstateWriteCrashTest(BitcoinTestFramework): self.node2_args = ["-dbcrashratio=24", "-dbcache=16"] + self.base_args # Node3 is a normal node with default args, will mine full blocks - self.node3_args = [] + # and non-standard txs (e.g. txs with "dust" outputs) + self.node3_args = ["-acceptnonstdtxn"] self.extra_args = [self.node0_args, self.node1_args, self.node2_args, self.node3_args] def skip_test_if_missing_module(self): diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 26153d5f52..8b36d3f423 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -120,9 +120,16 @@ def check_estimates(node, fees_seen): else: assert_greater_than_or_equal(i + 1, e["blocks"]) + class EstimateFeeTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 + # mine non-standard txs (e.g. txs with "dust" outputs) + self.extra_args = [ + ["-maxorphantxsize=1000", "-whitelist=127.0.0.1"], + ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], + ["-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"] + ] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -133,9 +140,7 @@ class EstimateFeeTest(BitcoinTestFramework): But first we need to use one node to create a lot of outputs which we will use to generate our transactions. """ - self.add_nodes(3, extra_args=[["-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"]]) + self.add_nodes(3, extra_args=self.extra_args) # Use node0 to mine blocks for input splitting # Node1 mines small blocks but that are bigger than the expected transaction rate. # NOTE: the CreateNewBlock code starts counting block size at 1,000 bytes, From b0ee2a40a4dcac8276f1ba25e76e1a8871703ec1 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Wed, 15 Jan 2020 14:16:27 +0200 Subject: [PATCH 11/11] merge bitcoin#17931: Fix p2p_invalid_messages failing in Python 3.8 because of warning --- test/functional/p2p_invalid_messages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 19f9258dbd..b6eb7c8fe3 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -148,13 +148,13 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_magic_bytes(self): conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - def swap_magic_bytes(): + async def swap_magic_bytes(): conn._on_data = lambda: None # Need to ignore all incoming messages from now, since they come with "invalid" magic bytes conn.magic_bytes = b'\x00\x11\x22\x32' # Call .result() to block until the atomic swap is complete, otherwise # we might run into races later on - asyncio.run_coroutine_threadsafe(asyncio.coroutine(swap_magic_bytes)(), NetworkThread.network_event_loop).result() + asyncio.run_coroutine_threadsafe(swap_magic_bytes(), NetworkThread.network_event_loop).result() with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: INVALID MESSAGESTART ping']): conn.send_message(messages.msg_ping(nonce=0xff))