diff options
| -rw-r--r-- | src/bench/bench_bitcoin.cpp | 7 | ||||
| -rw-r--r-- | src/cuckoocache.h | 99 | ||||
| -rw-r--r-- | src/net.cpp | 88 | ||||
| -rw-r--r-- | src/net.h | 83 | ||||
| -rw-r--r-- | src/net_processing.cpp | 32 | ||||
| -rw-r--r-- | src/test/cuckoocache_tests.cpp | 10 | ||||
| -rw-r--r-- | src/test/util_tests.cpp | 10 | ||||
| -rw-r--r-- | src/util/time.cpp | 14 | ||||
| -rw-r--r-- | src/util/time.h | 1 | ||||
| -rw-r--r-- | src/wallet/rpcdump.cpp | 17 | ||||
| -rwxr-xr-x | test/functional/p2p_invalid_messages.py | 11 |
11 files changed, 226 insertions, 146 deletions
diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp index 62bd1a52f7..9235d5fe6a 100644 --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -51,6 +51,13 @@ int main(int argc, char** argv) std::string scaling_str = gArgs.GetArg("-scaling", DEFAULT_BENCH_SCALING); bool is_list_only = gArgs.GetBoolArg("-list", false); + if (evaluations == 0) { + return EXIT_SUCCESS; + } else if (evaluations < 0) { + tfm::format(std::cerr, "Error parsing evaluations argument: %d\n", evaluations); + return EXIT_FAILURE; + } + double scaling_factor; if (!ParseDouble(scaling_str, &scaling_factor)) { tfm::format(std::cerr, "Error parsing scaling factor as double: %s\n", scaling_str); diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 4d0b094fa2..674f47b956 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -14,42 +14,40 @@ #include <vector> -/** namespace CuckooCache provides high performance cache primitives +/** High-performance cache primitives. * * Summary: * - * 1) bit_packed_atomic_flags is bit-packed atomic flags for garbage collection + * 1. @ref bit_packed_atomic_flags is bit-packed atomic flags for garbage collection * - * 2) cache is a cache which is performant in memory usage and lookup speed. It - * is lockfree for erase operations. Elements are lazily erased on the next - * insert. + * 2. @ref cache is a cache which is performant in memory usage and lookup speed. It + * is lockfree for erase operations. Elements are lazily erased on the next insert. */ namespace CuckooCache { -/** bit_packed_atomic_flags implements a container for garbage collection flags +/** @ref bit_packed_atomic_flags implements a container for garbage collection flags * that is only thread unsafe on calls to setup. This class bit-packs collection * flags for memory efficiency. * - * All operations are std::memory_order_relaxed so external mechanisms must + * All operations are `std::memory_order_relaxed` so external mechanisms must * ensure that writes and reads are properly synchronized. * - * On setup(n), all bits up to n are marked as collected. + * On setup(n), all bits up to `n` are marked as collected. * * Under the hood, because it is an 8-bit type, it makes sense to use a multiple * of 8 for setup, but it will be safe if that is not the case as well. - * */ class bit_packed_atomic_flags { std::unique_ptr<std::atomic<uint8_t>[]> mem; public: - /** No default constructor as there must be some size */ + /** No default constructor, as there must be some size. */ bit_packed_atomic_flags() = delete; /** * bit_packed_atomic_flags constructor creates memory to sufficiently - * keep track of garbage collection information for size entries. + * keep track of garbage collection information for `size` entries. * * @param size the number of elements to allocate space for * @@ -68,7 +66,7 @@ public: }; /** setup marks all entries and ensures that bit_packed_atomic_flags can store - * at least size entries + * at least `b` entries. * * @param b the number of elements to allocate space for * @post bit_set, bit_unset, and bit_is_set function properly forall x. x < @@ -84,19 +82,18 @@ public: /** bit_set sets an entry as discardable. * - * @param s the index of the entry to bit_set. + * @param s the index of the entry to bit_set * @post immediately subsequent call (assuming proper external memory * ordering) to bit_is_set(s) == true. - * */ inline void bit_set(uint32_t s) { mem[s >> 3].fetch_or(1 << (s & 7), std::memory_order_relaxed); } - /** bit_unset marks an entry as something that should not be overwritten + /** bit_unset marks an entry as something that should not be overwritten. * - * @param s the index of the entry to bit_unset. + * @param s the index of the entry to bit_unset * @post immediately subsequent call (assuming proper external memory * ordering) to bit_is_set(s) == false. */ @@ -105,10 +102,10 @@ public: mem[s >> 3].fetch_and(~(1 << (s & 7)), std::memory_order_relaxed); } - /** bit_is_set queries the table for discardability at s + /** bit_is_set queries the table for discardability at `s`. * - * @param s the index of the entry to read. - * @returns if the bit at index s was set. + * @param s the index of the entry to read + * @returns true if the bit at index `s` was set, false otherwise * */ inline bool bit_is_set(uint32_t s) const { @@ -116,15 +113,15 @@ public: } }; -/** cache implements a cache with properties similar to a cuckoo-set +/** @ref cache implements a cache with properties similar to a cuckoo-set. * - * The cache is able to hold up to (~(uint32_t)0) - 1 elements. + * The cache is able to hold up to `(~(uint32_t)0) - 1` elements. * * Read Operations: - * - contains(*, false) + * - contains() for `erase=false` * * Read+Erase Operations: - * - contains(*, true) + * - contains() for `erase=true` * * Erase Operations: * - allow_erase() @@ -141,10 +138,10 @@ public: * * User Must Guarantee: * - * 1) Write Requires synchronized access (e.g., a lock) - * 2) Read Requires no concurrent Write, synchronized with the last insert. - * 3) Erase requires no concurrent Write, synchronized with last insert. - * 4) An Erase caller must release all memory before allowing a new Writer. + * 1. Write requires synchronized access (e.g. a lock) + * 2. Read requires no concurrent Write, synchronized with last insert. + * 3. Erase requires no concurrent Write, synchronized with last insert. + * 4. An Erase caller must release all memory before allowing a new Writer. * * * Note on function names: @@ -177,7 +174,7 @@ private: mutable std::vector<bool> epoch_flags; /** epoch_heuristic_counter is used to determine when an epoch might be aged - * & an expensive scan should be done. epoch_heuristic_counter is + * & an expensive scan should be done. epoch_heuristic_counter is * decremented on insert and reset to the new number of inserts which would * cause the epoch to reach epoch_size when it reaches zero. */ @@ -194,24 +191,25 @@ private: uint32_t epoch_size; /** depth_limit determines how many elements insert should try to replace. - * Should be set to log2(n)*/ + * Should be set to log2(n). + */ uint8_t depth_limit; /** hash_function is a const instance of the hash function. It cannot be * static or initialized at call time as it may have internal state (such as * a nonce). - * */ + */ const Hash hash_function; /** compute_hashes is convenience for not having to write out this * expression everywhere we use the hash values of an Element. * * We need to map the 32-bit input hash onto a hash bucket in a range [0, size) in a - * manner which preserves as much of the hash's uniformity as possible. Ideally + * manner which preserves as much of the hash's uniformity as possible. Ideally * this would be done by bitmasking but the size is usually not a power of two. * * The naive approach would be to use a mod -- which isn't perfectly uniform but so - * long as the hash is much larger than size it is not that bad. Unfortunately, + * long as the hash is much larger than size it is not that bad. Unfortunately, * mod/division is fairly slow on ordinary microprocessors (e.g. 90-ish cycles on * haswell, ARM doesn't even have an instruction for it.); when the divisor is a * constant the compiler will do clever tricks to turn it into a multiply+add+shift, @@ -223,10 +221,10 @@ private: * somewhat complicated and the result is still slower than other options: * * Instead we treat the 32-bit random number as a Q32 fixed-point number in the range - * [0,1) and simply multiply it by the size. Then we just shift the result down by - * 32-bits to get our bucket number. The result has non-uniformity the same as a + * [0, 1) and simply multiply it by the size. Then we just shift the result down by + * 32-bits to get our bucket number. The result has non-uniformity the same as a * mod, but it is much faster to compute. More about this technique can be found at - * http://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/ + * http://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/ . * * The resulting non-uniformity is also more equally distributed which would be * advantageous for something like linear probing, though it shouldn't matter @@ -237,8 +235,8 @@ private: * 32*32->64 multiply, which means the operation is reasonably fast even on a * typical 32-bit processor. * - * @param e the element whose hashes will be returned - * @returns std::array<uint32_t, 8> of deterministic hashes derived from e + * @param e The element whose hashes will be returned + * @returns Deterministic hashes derived from `e` uniformly mapped onto the range [0, size) */ inline std::array<uint32_t, 8> compute_hashes(const Element& e) const { @@ -252,14 +250,14 @@ private: (uint32_t)(((uint64_t)hash_function.template operator()<7>(e) * (uint64_t)size) >> 32)}}; } - /* end - * @returns a constexpr index that can never be inserted to */ + /** invalid returns a special index that can never be inserted to + * @returns the special constexpr index that can never be inserted to */ constexpr uint32_t invalid() const { return ~(uint32_t)0; } - /** allow_erase marks the element at index n as discardable. Threadsafe + /** allow_erase marks the element at index `n` as discardable. Threadsafe * without any concurrent insert. * @param n the index to allow erasure of */ @@ -268,7 +266,7 @@ private: collection_flags.bit_set(n); } - /** please_keep marks the element at index n as an entry that should be kept. + /** please_keep marks the element at index `n` as an entry that should be kept. * Threadsafe without any concurrent insert. * @param n the index to prioritize keeping */ @@ -336,7 +334,7 @@ public: * * @param new_size the desired number of elements to store * @returns the maximum number of elements storable - **/ + */ uint32_t setup(uint32_t new_size) { // depth_limit must be at least one otherwise errors can occur. @@ -360,7 +358,7 @@ public: * negligible compared to the size of the elements. * * @param bytes the approximate number of bytes to use for this data - * structure. + * structure * @returns the maximum number of elements storable (see setup() * documentation for more detail) */ @@ -376,10 +374,12 @@ public: * It drops the last tried element if it runs out of depth before * encountering an open slot. * - * Thus + * Thus: * + * ``` * insert(x); * return contains(x, false); + * ``` * * is not guaranteed to return true. * @@ -387,7 +387,6 @@ public: * @post one of the following: All previously inserted elements and e are * now in the table, one previously inserted element is evicted from the * table, the entry attempted to be inserted is evicted. - * */ inline void insert(Element e) { @@ -416,9 +415,9 @@ public: /** Swap with the element at the location that was * not the last one looked at. Example: * - * 1) On first iteration, last_loc == invalid(), find returns last, so + * 1. On first iteration, last_loc == invalid(), find returns last, so * last_loc defaults to locs[0]. - * 2) On further iterations, where last_loc == locs[k], last_loc will + * 2. On further iterations, where last_loc == locs[k], last_loc will * go to locs[k+1 % 8], i.e., next of the 8 indices wrapping around * to 0 if needed. * @@ -439,17 +438,19 @@ public: } } - /* contains iterates through the hash locations for a given element + /** contains iterates through the hash locations for a given element * and checks to see if it is present. * * contains does not check garbage collected state (in other words, * garbage is only collected when the space is needed), so: * + * ``` * insert(x); * if (contains(x, true)) * return contains(x, false); * else * return true; + * ``` * * executed on a single thread will always return true! * @@ -458,7 +459,7 @@ public: * contains returns a bool set true if the element was found. * * @param e the element to check - * @param erase + * @param erase whether to attempt setting the garbage collect flag * * @post if erase is true and the element is found, then the garbage collect * flag is set diff --git a/src/net.cpp b/src/net.cpp index ad91e3f8f6..674f2ecf24 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -567,42 +567,28 @@ 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; + i->second += msg.m_raw_message_size; + + // push the message to the process queue, + vRecvMsg.push_back(std::move(msg)); - msg.nTime = nTimeMicros; complete = true; } } @@ -636,8 +622,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; @@ -658,9 +643,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; @@ -668,7 +654,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); @@ -685,14 +671,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.begin()); 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(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE), + HexStr(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) const EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_vSend) { auto it = pnode->vSendMsg.begin(); @@ -1344,9 +1360,9 @@ void CConnman::SocketHandler() 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); @@ -2676,6 +2692,8 @@ 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)); } CNode::~CNode() @@ -609,56 +609,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<TransportDeserializer> m_deserializer; + // socket std::atomic<ServiceFlags> nServices{NODE_NONE}; SOCKET hSocket GUARDED_BY(cs_hSocket); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 4a3076e3c7..fd31c962c2 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3272,41 +3272,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 - 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; - const 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(hash.begin(), hash.begin()+CMessageHeader::CHECKSUM_SIZE), - HexStr(hdr.pchChecksum, hdr.pchChecksum+CMessageHeader::CHECKSUM_SIZE)); + LogPrint(BCLog::NET, "%s(%s, %u bytes): CHECKSUM ERROR peer=%d\n", __func__, + SanitizeString(strCommand), nMessageSize, pfrom->GetId()); return fMoreWork; } @@ -3314,7 +3310,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter bool fRet = false; try { - fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.nTime, chainparams, connman, interruptMsgProc); + fRet = ProcessMessage(pfrom, strCommand, vRecv, msg.m_time, chainparams, connman, interruptMsgProc); if (interruptMsgProc) return false; if (!pfrom->vRecvGetData.empty()) diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp index d38ede691a..a3017da3e7 100644 --- a/src/test/cuckoocache_tests.cpp +++ b/src/test/cuckoocache_tests.cpp @@ -10,11 +10,11 @@ /** Test Suite for CuckooCache * - * 1) All tests should have a deterministic result (using insecure rand + * 1. All tests should have a deterministic result (using insecure rand * with deterministic seeds) - * 2) Some test methods are templated to allow for easier testing + * 2. Some test methods are templated to allow for easier testing * against new versions / comparing - * 3) Results should be treated as a regression test, i.e., did the behavior + * 3. Results should be treated as a regression test, i.e., did the behavior * change significantly from what was expected. This can be OK, depending on * the nature of the change, but requires updating the tests to reflect the new * expected behavior. For example improving the hit rate may cause some tests @@ -82,9 +82,9 @@ static double test_cache(size_t megabytes, double load) * * Examples: * - * 1) at load 0.5, we expect a perfect hit rate, so we multiply by + * 1. at load 0.5, we expect a perfect hit rate, so we multiply by * 1.0 - * 2) at load 2.0, we expect to see half the entries, so a perfect hit rate + * 2. at load 2.0, we expect to see half the entries, so a perfect hit rate * would be 0.5. Therefore, if we see a hit rate of 0.4, 0.4*2.0 = 0.8 is the * normalized hit rate. * diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 02303d0f65..569ce53092 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -145,9 +145,17 @@ BOOST_AUTO_TEST_CASE(util_Join) BOOST_CHECK_EQUAL(Join<std::string>({"foo", "bar"}, ", ", op_upper), "FOO, BAR"); } -BOOST_AUTO_TEST_CASE(util_FormatISO8601DateTime) +BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) { BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z"); + BOOST_CHECK_EQUAL(FormatISO8601DateTime(0), "1970-01-01T00:00:00Z"); + + BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("2011-09-30T23:36:17Z"), 1317425777); + + auto time = GetSystemTimeInSeconds(); + BOOST_CHECK_EQUAL(ParseISO8601DateTime(FormatISO8601DateTime(time)), time); } BOOST_AUTO_TEST_CASE(util_FormatISO8601Date) diff --git a/src/util/time.cpp b/src/util/time.cpp index 2b202ae95f..2afff2626b 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -111,3 +111,17 @@ std::string FormatISO8601Date(int64_t nTime) { #endif return strprintf("%04i-%02i-%02i", ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday); } + +int64_t ParseISO8601DateTime(const std::string& str) +{ + static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); + static const std::locale loc(std::locale::classic(), + new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ")); + std::istringstream iss(str); + iss.imbue(loc); + boost::posix_time::ptime ptime(boost::date_time::not_a_date_time); + iss >> ptime; + if (ptime.is_not_a_date_time() || epoch > ptime) + return 0; + return (ptime - epoch).total_seconds(); +}
\ No newline at end of file diff --git a/src/util/time.h b/src/util/time.h index c0470a2136..af4390aa1c 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -48,5 +48,6 @@ T GetTime(); */ std::string FormatISO8601DateTime(int64_t nTime); std::string FormatISO8601Date(int64_t nTime); +int64_t ParseISO8601DateTime(const std::string& str); #endif // BITCOIN_UTIL_TIME_H diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 1cd4cb93b4..9af567fe0d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -23,23 +23,10 @@ #include <tuple> #include <boost/algorithm/string.hpp> -#include <boost/date_time/posix_time/posix_time.hpp> #include <univalue.h> -int64_t static DecodeDumpTime(const std::string &str) { - static const boost::posix_time::ptime epoch = boost::posix_time::from_time_t(0); - static const std::locale loc(std::locale::classic(), - new boost::posix_time::time_input_facet("%Y-%m-%dT%H:%M:%SZ")); - std::istringstream iss(str); - iss.imbue(loc); - boost::posix_time::ptime ptime(boost::date_time::not_a_date_time); - iss >> ptime; - if (ptime.is_not_a_date_time()) - return 0; - return (ptime - epoch).total_seconds(); -} std::string static EncodeDumpString(const std::string &str) { std::stringstream ret; @@ -598,7 +585,7 @@ UniValue importwallet(const JSONRPCRequest& request) continue; CKey key = DecodeSecret(vstr[0]); if (key.IsValid()) { - int64_t nTime = DecodeDumpTime(vstr[1]); + int64_t nTime = ParseISO8601DateTime(vstr[1]); std::string strLabel; bool fLabel = true; for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { @@ -617,7 +604,7 @@ UniValue importwallet(const JSONRPCRequest& request) } else if(IsHex(vstr[0])) { std::vector<unsigned char> vData(ParseHex(vstr[0])); CScript script = CScript(vData.begin(), vData.end()); - int64_t birth_time = DecodeDumpTime(vstr[1]); + int64_t birth_time = ParseISO8601DateTime(vstr[1]); scripts.push_back(std::pair<CScript, int64_t>(script, birth_time)); } } diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index 58c72f89d8..f0ceb8e6a3 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -101,11 +101,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()) @@ -168,7 +167,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 |