Merge pull request #4323 from kittywhiskers/net

merge bitcoin 15246, 15330, 15697, 16445, 16202, 16562, 18260, 22331, 23271, 16493, 17931: Refactor network message deserialization

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

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) and poly1305 key-derivation cipher instance
-    ChaCha20 m_chacha_header;                                    // AAD cipher instance (encrypted length)
+    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
 

src/net.cpp

@@ -628,9 +628,9 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &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.m_ping_usec = nPingUsecTime;
-    stats.dMinPing  = (((double)nMinPingUsecTime) / 1e6);
+    stats.m_min_ping_usec  = nMinPingUsecTime;
-    stats.dPingWait = (((double)nPingUsecWait) / 1e6);
+    stats.m_ping_wait_usec = nPingUsecWait;
 
     // Leave string empty if addrLocal invalid (not filled in yet)
     CService addrLocalUnlocked = GetAddrLocal();
@@ -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;
+        int handled = m_deserializer->Read(pch, nBytes);
-        if (!msg.in_data) {
+        if (handled < 0) return false;
-            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;
-        }
 
         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;
+            i->second += msg.m_raw_message_size;
-            statsClient.count("bandwidth.message." + std::string(msg.hdr.pchCommand) + ".bytesReceived", msg.hdr.nMessageSize + CMessageHeader::HEADER_SIZE, 1.0f);
+            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 V1TransportDeserializer::readHeader(const char *pch, unsigned int nBytes)
-int CNetMessage::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
+    // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH
-    if (hdr.nMessageSize > MAX_SIZE)
+    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,57 @@ 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<uint8_t>(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE)),
+                 HexStr(Span<uint8_t>(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;
+}
+
+void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& 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();
@@ -1828,9 +1856,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())
+                // vRecvMsg contains only completed CNetMessage
-                    break;
+                // the single possible partially deserialized message are held by TransportDeserializer
-                nSizeAdded += it->vRecv.size() + CMessageHeader::HEADER_SIZE;
+                nSizeAdded += it->m_raw_message_size;
             }
             {
                 LOCK(pnode->cs_vProcessMsg);
@@ -3732,6 +3760,9 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     } else {
         LogPrint(BCLog::NET, "Added connection peer=%d\n", id);
     }
+
+    m_deserializer = MakeUnique<V1TransportDeserializer>(V1TransportDeserializer(Params().MessageStart(), SER_NETWORK, INIT_PROTO_VERSION));
+    m_serializer = MakeUnique<V1TransportSerializer>(V1TransportSerializer());
 }
 
 CNode::~CNode()
@@ -3747,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());
+
+    // make sure we use the appropriate network transport format
+    std::vector<unsigned char> 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<unsigned char> 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);

src/net.h

@@ -763,9 +763,9 @@ public:
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
     NetPermissionFlags m_permissionFlags;
     bool m_legacyWhitelisted;
-    double dPingTime;
+    int64_t m_ping_usec;
-    double dPingWait;
+    int64_t m_ping_wait_usec;
-    double dMinPing;
+    int64_t m_min_ping_usec;
     // Our address, as reported by the peer
     std::string addrLocal;
     // Address of this peer
@@ -782,56 +782,120 @@ 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);
     }
-
+    void SetVersion(int nVersionIn) override
-    const uint256& GetMessageHash() const;
-
-    void SetVersion(int nVersionIn)
     {
         hdrbuf.SetVersion(nVersionIn);
         vRecv.SetVersion(nVersionIn);
     }
-
+    int Read(const char *pch, unsigned int nBytes) override {
-    int readHeader(const char *pch, unsigned int nBytes);
+        int ret = in_data ? readData(pch, nBytes) : readHeader(pch, nBytes);
-    int readData(const char *pch, unsigned int nBytes);
+        if (ret < 0) Reset();
+        return ret;
+    }
+    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<unsigned char>& header) = 0;
+    virtual ~TransportSerializer() {}
+};
+
+class V1TransportSerializer  : public TransportSerializer {
+public:
+    void prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) override;
+};
 
 /** Information about a peer */
 class CNode
 {
     friend class CConnman;
 public:
+    std::unique_ptr<TransportDeserializer> m_deserializer;
+    std::unique_ptr<TransportSerializer> m_serializer;
+
     // socket
     std::atomic<ServiceFlags> nServices{NODE_NONE};
     SOCKET hSocket GUARDED_BY(cs_hSocket);

src/net_processing.cpp

@@ -4026,41 +4026,37 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& 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
+    // Check network magic
-    if (memcmp(msg.hdr.pchMessageStart, chainparams.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) {
+    if (!msg.m_valid_netmagic) {
-        LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.hdr.GetCommand()), pfrom->GetId());
+        LogPrint(BCLog::NET, "PROCESSMESSAGE: INVALID MESSAGESTART %s peer=%d\n", SanitizeString(msg.m_command), pfrom->GetId());
         pfrom->fDisconnect = true;
         return false;
     }
 
-    // Read header
+    // Check header
-    CMessageHeader& hdr = msg.hdr;
+    if (!msg.m_valid_header)
-    if (!hdr.IsValid(chainparams.MessageStart()))
     {
-        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;
+    CDataStream& vRecv = msg.m_recv;
-    uint256 hash = msg.GetMessageHash();
+    if (!msg.m_valid_checksum)
-    if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
     {
-        LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
+        LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR peer=%d\n", __func__,
-           SanitizeString(strCommand), nMessageSize,
+           SanitizeString(strCommand), nMessageSize, pfrom->GetId());
-           HexStr(Span<uint8_t>(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE)),
-           HexStr(hdr.pchChecksum));
         return fMoreWork;
     }
 
@@ -4068,7 +4064,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& 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())

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<int64_t>::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<int64_t>::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)

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*/
+    /* Format a CNodeStats.m_ping_usec into a user-readable string or display N/A, if 0*/
-    QString formatPingTime(double dPingTime);
+    QString formatPingTime(int64_t ping_usec);
 
     /* Format a CNodeCombinedStats.nTimeOffset into a user-readable string. */
     QString formatTimeOffset(int64_t nTimeOffset);

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:

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->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_usec));
-    ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.dPingWait));
+    ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_wait_usec));
-    ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.dMinPing));
+    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));

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)
+        if (stats.m_ping_usec > 0) {
-            obj.pushKV("pingtime", stats.dPingTime);
+            obj.pushKV("pingtime", ((double)stats.m_ping_usec) / 1e6);
-        if (stats.dMinPing < static_cast<double>(std::numeric_limits<int64_t>::max())/1e6)
+        }
-            obj.pushKV("minping", stats.dMinPing);
+        if (stats.m_min_ping_usec < std::numeric_limits<int64_t>::max()) {
-        if (stats.dPingWait > 0.0)
+            obj.pushKV("minping", ((double)stats.m_min_ping_usec) / 1e6);
-            obj.pushKV("pingwait", stats.dPingWait);
+        }
+        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

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);
@@ -716,8 +716,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector)
 
     TestChaCha20Poly1305AEAD(true, 0,
         /* m  */ "0000000000000000000000000000000000000000000000000000000000000000",
-        /* k1 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000",
+        /* k1 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000",
-        /* k2 (AAD) */ "0000000000000000000000000000000000000000000000000000000000000000",
+        /* k2 (payload) */ "0000000000000000000000000000000000000000000000000000000000000000",
         /* AAD keystream */ "76b8e0ada0f13d90405d6ae55386bd28bdd219b8a08ded1aa836efcc8b770dc7da41597c5157488d7724e03fb8d84a376a43b8f41518a11cc387b669b2ee6586",
         /* encrypted message & MAC */ "76b8e09f07e7be5551387a98ba977c732d080dcb0f29a048e3656912c6533e32d2fc11829c1b6c1df1f551cd6131ff08",
         /* encrypted message & MAC at sequence 999 */ "b0a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3aaa7aa16ec62c5e24f040c08bb20c3598");
@@ -730,8 +730,8 @@ BOOST_AUTO_TEST_CASE(chacha20_poly1305_aead_testvector)
         "b1a03d5bd2855d60699e7d3a3133fa47be740fe4e4c1f967555e2d9271f31c3a8bd94d54b5ecabbc41ffbb0c90924080");
     TestChaCha20Poly1305AEAD(true, 255,
         "ff0000f195e66982105ffb640bb7757f579da31602fc93ec01ac56f85ac3c134a4547b733b46413042c9440049176905d3be59ea1c53f15916155c2be8241a38008b9a26bc35941e2444177c8ade6689de95264986d95889fb60e84629c9bd9a5acb1cc118be563eb9b3a4a472f82e09a7e778492b562ef7130e88dfe031c79db9d4f7c7a899151b9a475032b63fc385245fe054e3dd5a97a5f576fe064025d3ce042c566ab2c507b138db853e3d6959660996546cc9c4a6eafdc777c040d70eaf46f76dad3979e5c5360c3317166a1c894c94a371876a94df7628fe4eaaf2ccb27d5aaae0ad7ad0f9d4b6ad3b54098746d4524d38407a6deb3ab78fab78c9",
-        "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
         "ff0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
+        "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
         "c640c1711e3ee904ac35c57ab9791c8a1c408603a90b77a83b54f6c844cb4b06d94e7fc6c800e165acd66147e80ec45a567f6ce66d05ec0cae679dceeb890017",
         "3940c1e92da4582ff6f92a776aeb14d014d384eeb30f660dacf70a14a23fd31e91212701334e2ce1acf5199dc84f4d61ddbe6571bca5af874b4c9226c26e650995d157644e1848b96ed6c2102d5489a050e71d29a5a66ece11de5fb5c9558d54da28fe45b0bc4db4e5b88030bfc4a352b4b7068eccf656bae7ad6a35615315fc7c49d4200388d5eca67c2e822e069336c69b40db67e0f3c81209c50f3216a4b89fb3ae1b984b7851a2ec6f68ab12b101ab120e1ea7313bb93b5a0f71185c7fea017ddb92769861c29dba4fbc432280d5dff21b36d1c4c790128b22699950bb18bf74c448cdfe547d8ed4f657d8005fdc0cd7a050c2d46050a44c4376355858981fbe8b184288276e7a93eabc899c4a",
         "f039c6689eaeef0456685200feaab9d54bbd9acde4410a3b6f4321296f4a8ca2604b49727d8892c57e005d799b2a38e85e809f20146e08eec75169691c8d4f54a0d51a1e1c7b381e0474eb02f994be9415ef3ffcbd2343f0601e1f3b172a1d494f838824e4df570f8e3b0c04e27966e36c82abd352d07054ef7bd36b84c63f9369afe7ed79b94f953873006b920c3fa251a771de1b63da927058ade119aa898b8c97e42a606b2f6df1e2d957c22f7593c1e2002f4252f4c9ae4bf773499e5cfcfe14dfc1ede26508953f88553bf4a76a802f6a0068d59295b01503fd9a600067624203e880fdf53933b96e1f4d9eb3f4e363dd8165a278ff667a41ee42b9892b077cefff92b93441f7be74cf10e6cd");

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 = 180
-        self.rpc_timeout = 90
 
         # 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):

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"],
+        self.add_nodes(3, extra_args=self.extra_args)
-                                      ["-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"],
-                                      ["-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"]])
         # 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,

test/functional/p2p_invalid_messages.py

@@ -1,12 +1,14 @@
 #!/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."""
+import asyncio
 import struct
+import sys
 
 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
 
 
@@ -15,7 +17,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 +27,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 +42,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 +62,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 +89,14 @@ class InvalidMessagesTest(BitcoinTestFramework):
         #
         # Send an oversized message, ensure we're disconnected.
         #
-        msg_over_size = msg_unrecognized("b" * (valid_data_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=0, 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)
@@ -102,6 +105,8 @@ class InvalidMessagesTest(BitcoinTestFramework):
             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.
@@ -109,7 +114,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 +141,66 @@ 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())
+
+        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(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)
+            self.nodes[0].disconnect_p2ps()
+
+    def test_checksum(self):
+        conn = self.nodes[0].add_p2p_connection(P2PDataStore())
+        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
+                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("<I", 0x02000000 + 1) + msg[cut_len + 4:]
+            self.nodes[0].p2p.send_raw_message(msg)
+            conn.wait_for_disconnect(timeout=1)
+            self.nodes[0].disconnect_p2ps()
+
+    def test_command(self):
+        conn = self.nodes[0].add_p2p_connection(P2PDataStore())
+        with self.nodes[0].assert_debug_log(['PROCESSMESSAGE: ERRORS IN HEADER']):
+            msg = msg_unrecognized(str_data="d")
+            msg.command = b'\xff' * 12
+            msg = conn.build_message(msg)
+            # Modify command
+            msg = msg[:7] + b'\x00' + msg[7 + 1:]
+            self.nodes[0].p2p.send_raw_message(msg)
+            conn.sync_with_ping(timeout=1)
+            self.nodes[0].disconnect_p2ps()
 
     def _tweak_msg_data_size(self, message, wrong_size):
         """
@@ -170,6 +223,5 @@ class InvalidMessagesTest(BitcoinTestFramework):
         return raw_msg_with_wrong_size
 
 
-
 if __name__ == '__main__':
     InvalidMessagesTest().main()

test/functional/test_framework/mininode.py

@@ -152,10 +152,10 @@ class P2PConnection(asyncio.Protocol):
         # The initial message to send after the connection was made:
         self.on_connection_send_msg = None
         self.recvbuf = b""
-        self.network = net
+        self.magic_bytes = MAGIC_BYTES[net]
         self.uacomment = uacomment
 
-        if self.network == "devnet":
+        if net == "devnet":
             devnet_name = "devnet1"  # see initialize_datadir()
             if self.uacomment is None:
                 self.strSubVer = MY_SUBVERSION % ("(devnet.devnet-%s)" % devnet_name).encode()
@@ -217,7 +217,7 @@ class P2PConnection(asyncio.Protocol):
             while True:
                 if len(self.recvbuf) < 4:
                     return
-                if self.recvbuf[:4] != MAGIC_BYTES[self.network]:
+                if self.recvbuf[:4] != self.magic_bytes:
                     raise ValueError("magic bytes mismatch: {} != {}".format(repr(self.magic_bytes), repr(self.recvbuf)))
                 if len(self.recvbuf) < 4 + 12 + 4 + 4:
                     return
@@ -279,7 +279,7 @@ class P2PConnection(asyncio.Protocol):
         """Build a serialized P2P message"""
         command = message.command
         data = message.serialize()
-        tmsg = MAGIC_BYTES[self.network]
+        tmsg = self.magic_bytes
         tmsg += command
         tmsg += b"\x00" * (12 - len(command))
         tmsg += struct.pack("<I", len(data))