From e1797817284706ab0d134a094adfac464b182d98 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Sun, 19 Apr 2015 17:08:25 +0200 Subject: [PATCH] More sophisticated (and correct) BlockHashes handling. --- libdevcore/RLP.cpp | 15 +++++++++++++++ libdevcore/RLP.h | 7 ++++++- libethcore/Common.h | 3 +-- libethereum/BlockChain.cpp | 2 +- libethereum/BlockQueue.cpp | 15 +++++++++++++++ libethereum/BlockQueue.h | 11 ++++++++++- libethereum/EthereumPeer.cpp | 11 +++++++++-- 7 files changed, 57 insertions(+), 7 deletions(-) diff --git a/libdevcore/RLP.cpp b/libdevcore/RLP.cpp index 994aac265..25e843c77 100644 --- a/libdevcore/RLP.cpp +++ b/libdevcore/RLP.cpp @@ -111,10 +111,24 @@ unsigned RLP::actualSize() const return 0; } +void RLP::requireGood() const +{ + if (isNull()) + BOOST_THROW_EXCEPTION(BadRLP()); + byte n = m_data[0]; + if (n != c_rlpDataImmLenStart + 1) + return; + if (m_data.size() < 2) + BOOST_THROW_EXCEPTION(BadRLP()); + if (m_data[1] < c_rlpDataImmLenStart) + BOOST_THROW_EXCEPTION(BadRLP()); +} + bool RLP::isInt() const { if (isNull()) return false; + requireGood(); byte n = m_data[0]; if (n < c_rlpDataImmLenStart) return !!n; @@ -141,6 +155,7 @@ unsigned RLP::length() const { if (isNull()) return 0; + requireGood(); unsigned ret = 0; byte n = m_data[0]; if (n < c_rlpDataImmLenStart) diff --git a/libdevcore/RLP.h b/libdevcore/RLP.h index caaf10b6a..c99d1a358 100644 --- a/libdevcore/RLP.h +++ b/libdevcore/RLP.h @@ -253,6 +253,7 @@ public: /// Converts to int of type given; if isString(), decodes as big-endian bytestream. @returns 0 if not an int or string. template _T toInt(int _flags = Strict) const { + requireGood(); if ((!isInt() && !(_flags & AllowNonCanon)) || isList() || isNull()) if (_flags & ThrowOnFail) BOOST_THROW_EXCEPTION(BadCast()); @@ -273,6 +274,7 @@ public: template _N toHash(int _flags = Strict) const { + requireGood(); if (!isData() || (length() > _N::size && (_flags & FailIfTooBig)) || (length() < _N::size && (_flags & FailIfTooSmall))) if (_flags & ThrowOnFail) BOOST_THROW_EXCEPTION(BadCast()); @@ -290,7 +292,7 @@ public: RLPs toList() const; /// @returns the data payload. Valid for all types. - bytesConstRef payload() const { return m_data.cropped(payloadOffset()); } + bytesConstRef payload() const { auto l = length(); if (l > m_data.size()) throw BadRLP(); return m_data.cropped(payloadOffset(), l); } /// @returns the theoretical size of this item. /// @note Under normal circumstances, is equivalent to m_data.size() - use that unless you know it won't work. @@ -300,6 +302,9 @@ private: /// Disable construction from rvalue explicit RLP(bytes const&&) {} + /// Throws if is non-canonical data (i.e. single byte done in two bytes that could be done in one). + void requireGood() const; + /// Single-byte data payload. bool isSingleByte() const { return !isNull() && m_data[0] < c_rlpDataImmLenStart; } diff --git a/libethcore/Common.h b/libethcore/Common.h index 789390033..199057f91 100644 --- a/libethcore/Common.h +++ b/libethcore/Common.h @@ -90,8 +90,7 @@ enum class ImportResult AlreadyInChain, AlreadyKnown, Malformed, - BadChain, - Unknown + BadChain }; struct ImportRequirements diff --git a/libethereum/BlockChain.cpp b/libethereum/BlockChain.cpp index 352739626..2e089dd3b 100644 --- a/libethereum/BlockChain.cpp +++ b/libethereum/BlockChain.cpp @@ -310,7 +310,7 @@ tuple BlockChain::sync(BlockQueue& _bq, OverlayDB const& _st fresh += r.first; dead += r.second; } - catch (UnknownParent) + catch (dev::eth::UnknownParent) { cwarn << "ODD: Import queue contains block with unknown parent." << boost::current_exception_diagnostic_information(); // NOTE: don't reimport since the queue should guarantee everything in the right order. diff --git a/libethereum/BlockQueue.cpp b/libethereum/BlockQueue.cpp index 06fa3dd2b..153d11ac4 100644 --- a/libethereum/BlockQueue.cpp +++ b/libethereum/BlockQueue.cpp @@ -169,6 +169,21 @@ template T advanced(T _t, unsigned _n) return _t; } +QueueStatus BlockQueue::blockStatus(h256 const& _h) const +{ + ReadGuard l(m_lock); + return + m_readySet.count(_h) ? + QueueStatus::Ready : + m_drainingSet.count(_h) ? + QueueStatus::Importing : + m_unknownSet.count(_h) ? + QueueStatus::UnknownParent : + m_knownBad.count(_h) ? + QueueStatus::Bad : + QueueStatus::Unknown; +} + void BlockQueue::drain(std::vector& o_out, unsigned _max) { WriteGuard l(m_lock); diff --git a/libethereum/BlockQueue.h b/libethereum/BlockQueue.h index 47370221a..1932b2f66 100644 --- a/libethereum/BlockQueue.h +++ b/libethereum/BlockQueue.h @@ -46,6 +46,15 @@ struct BlockQueueStatus size_t bad; }; +enum class QueueStatus +{ + Ready, + Importing, + UnknownParent, + Bad, + Unknown +}; + /** * @brief A queue of blocks. Sits between network or other I/O and the BlockChain. * Sorts them ready for blockchain insertion (with the BlockChain::sync() method). @@ -87,7 +96,7 @@ public: BlockQueueStatus status() const { ReadGuard l(m_lock); return BlockQueueStatus{m_ready.size(), m_future.size(), m_unknown.size(), m_knownBad.size()}; } /// Get some infomration on the given block's status regarding us. - ImportResult blockStatus(h256 const& _h) const { ReadGuard l(m_lock); return m_readySet.count(_h) || m_drainingSet.count(_h) ? ImportResult::AlreadyKnown : m_unknownSet.count(_h) ? ImportResult::UnknownParent : m_knownBad.count(_h) ? ImportResult::BadChain : ImportResult::Unknown; } + QueueStatus blockStatus(h256 const& _h) const; template Handler onReady(T const& _t) { return m_onReady.add(_t); } diff --git a/libethereum/EthereumPeer.cpp b/libethereum/EthereumPeer.cpp index 3a9879ac6..4d50a12ae 100644 --- a/libethereum/EthereumPeer.cpp +++ b/libethereum/EthereumPeer.cpp @@ -389,12 +389,19 @@ bool EthereumPeer::interpret(unsigned _id, RLP const& _r) { addRating(1); auto h = _r[i].toHash(); - if (host()->m_bq.blockStatus(h) != ImportResult::Unknown || host()->m_chain.isKnown(h)) + auto status = host()->m_bq.blockStatus(h); + if (status == QueueStatus::Importing || status == QueueStatus::Ready || host()->m_chain.isKnown(h)) { transition(Asking::Blocks); return true; } - else + else if (status == QueueStatus::Bad) + { + cwarn << "BAD hash chain discovered. Ignoring."; + transition(Asking::Nothing); + return true; + } + else if (status == QueueStatus::Unknown) m_syncingNeededBlocks.push_back(h); } // run through - ask for more.