From e82f8db2de4a3fa90c4710dfc40adc6a9114f342 Mon Sep 17 00:00:00 2001 From: subtly Date: Sat, 20 Jun 2015 07:24:03 -0400 Subject: [PATCH 01/11] Prep for #2179. Abstract parsing of frame header from Session into struct. Catch unhandled exceptions thrown by ASIO. --- libp2p/Common.h | 11 +- libp2p/Host.cpp | 14 +- libp2p/RLPXFrameCoder.cpp | 12 ++ libp2p/RLPXFrameCoder.h | 14 ++ libp2p/Session.cpp | 337 +++++++++++++++++++------------------- libp2p/Session.h | 10 +- test/libp2p/net.cpp | 10 +- 7 files changed, 225 insertions(+), 183 deletions(-) diff --git a/libp2p/Common.h b/libp2p/Common.h index 4a1b64b70..445ba0cca 100644 --- a/libp2p/Common.h +++ b/libp2p/Common.h @@ -149,14 +149,15 @@ using CapDescs = std::vector; */ struct PeerSessionInfo { - NodeId id; - std::string clientVersion; - std::string host; - unsigned short port; + NodeId const id; + std::string const clientVersion; + std::string const host; + unsigned short const port; std::chrono::steady_clock::duration lastPing; - std::set caps; + std::set const caps; unsigned socketId; std::map notes; + unsigned const protocolVersion; }; using PeerSessionInfos = std::vector; diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index feb116c4a..c9c2743ee 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -254,7 +254,7 @@ void Host::startPeerSession(Public const& _id, RLP const& _rlp, RLPXFrameCoder* clog(NetMessageSummary) << "Hello: " << clientVersion << "V[" << protocolVersion << "]" << _id << showbase << capslog.str() << dec << listenPort; // create session so disconnects are managed - auto ps = make_shared(this, _io, _s, p, PeerSessionInfo({_id, clientVersion, p->endpoint.address.to_string(), listenPort, chrono::steady_clock::duration(), _rlp[2].toSet(), 0, map()})); + auto ps = make_shared(this, _io, _s, p, PeerSessionInfo({_id, clientVersion, p->endpoint.address.to_string(), listenPort, chrono::steady_clock::duration(), _rlp[2].toSet(), 0, map(), protocolVersion})); if (protocolVersion < dev::p2p::c_protocolVersion - 1) { ps->disconnect(IncompatibleProtocol); @@ -724,8 +724,16 @@ void Host::startedWorking() void Host::doWork() { - if (m_run) - m_ioService.run(); + try + { + if (m_run) + m_ioService.run(); + } + catch (std::exception const& _e) + { + clog(NetP2PWarn) << "Exception in Network Thread:" << _e.what(); + clog(NetP2PWarn) << "Network Restart is Recommended."; + } } void Host::keepAlivePeers() diff --git a/libp2p/RLPXFrameCoder.cpp b/libp2p/RLPXFrameCoder.cpp index c4bb46814..193c45511 100644 --- a/libp2p/RLPXFrameCoder.cpp +++ b/libp2p/RLPXFrameCoder.cpp @@ -29,6 +29,18 @@ using namespace dev; using namespace dev::p2p; using namespace CryptoPP; +RLPXFrameInfo::RLPXFrameInfo(bytesConstRef _header) +{ + length = (_header[0] * 256 + _header[1]) * 256 + _header[2]; + padding = ((16 - (length % 16)) % 16); + RLP header(_header.cropped(3), RLP::ThrowOnFail | RLP::FailIfTooSmall); + auto itemCount = header.itemCount(); + protocolId = header[0].toInt(); + hasSequence = itemCount > 1; + sequenceId = hasSequence ? header[1].toInt() : 0; + totalLength = itemCount == 3 ? header[2].toInt() : 0; +} + RLPXFrameCoder::RLPXFrameCoder(RLPXHandshake const& _init) { // we need: diff --git a/libp2p/RLPXFrameCoder.h b/libp2p/RLPXFrameCoder.h index 7c5eedbff..3964326ff 100644 --- a/libp2p/RLPXFrameCoder.h +++ b/libp2p/RLPXFrameCoder.h @@ -32,7 +32,21 @@ namespace dev { namespace p2p { + +struct RLPXFrameInfo +{ + RLPXFrameInfo() = default; + /// Constructor. frame-size || protocol-type, [sequence-id[, total-packet-size]] + RLPXFrameInfo(bytesConstRef _frameHeader); + uint32_t length = 0; ///< Max: 2**24 + uint8_t padding = 0; + uint16_t protocolId = 0; + bool hasSequence = false; + uint16_t sequenceId = 0; + uint32_t totalLength = 0; +}; + class RLPXHandshake; /** diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index c3f0f2e35..95fd4a3d6 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -27,7 +27,6 @@ #include #include #include -#include "RLPxHandshake.h" #include "Host.h" #include "Capability.h" using namespace std; @@ -156,111 +155,25 @@ void Session::serviceNodesRequest() addNote("peers", "done"); } -bool Session::interpret(PacketType _t, RLP const& _r) +bool Session::frameReceived(uint16_t _capId, PacketType _t, RLP const& _r) { m_lastReceived = chrono::steady_clock::now(); - clog(NetRight) << _t << _r; try // Generic try-catch block designed to capture RLP format errors - TODO: give decent diagnostics, make a bit more specific over what is caught. { - switch (_t) - { - case DisconnectPacket: - { - string reason = "Unspecified"; - auto r = (DisconnectReason)_r[0].toInt(); - if (!_r[0].isInt()) - drop(BadProtocol); - else - { - reason = reasonOf(r); - clog(NetMessageSummary) << "Disconnect (reason: " << reason << ")"; - drop(DisconnectRequested); - } - break; - } - case PingPacket: - { - clog(NetTriviaSummary) << "Ping"; - RLPStream s; - sealAndSend(prep(s, PongPacket)); - break; - } - case PongPacket: - m_info.lastPing = std::chrono::steady_clock::now() - m_ping; - clog(NetTriviaSummary) << "Latency: " << chrono::duration_cast(m_info.lastPing).count() << " ms"; - break; - case GetPeersPacket: - // Disabled for interop testing. - // GetPeers/PeersPacket will be modified to only exchange new nodes which it's peers are interested in. - break; - - clog(NetTriviaSummary) << "GetPeers"; - m_theyRequestedNodes = true; - serviceNodesRequest(); - break; - case PeersPacket: - // Disabled for interop testing. - // GetPeers/PeersPacket will be modified to only exchange new nodes which it's peers are interested in. - break; - - clog(NetTriviaSummary) << "Peers (" << dec << (_r.itemCount() - 1) << " entries)"; - m_weRequestedNodes = false; - for (unsigned i = 0; i < _r.itemCount(); ++i) - { - bi::address peerAddress; - if (_r[i][0].size() == 16) - peerAddress = bi::address_v6(_r[i][0].toHash>().asArray()); - else if (_r[i][0].size() == 4) - peerAddress = bi::address_v4(_r[i][0].toHash>().asArray()); - else - { - cwarn << "Received bad peer packet:" << _r; - disconnect(BadProtocol); - return true; - } - auto ep = bi::tcp::endpoint(peerAddress, _r[i][1].toInt()); - NodeId id = _r[i][2].toHash(); - - clog(NetAllDetail) << "Checking: " << ep << "(" << id << ")"; - - if (!isPublicAddress(peerAddress)) - goto CONTINUE; // Private address. Ignore. - - if (!id) - goto LAMEPEER; // Null identity. Ignore. - - if (m_server->id() == id) - goto LAMEPEER; // Just our info - we already have that. - - if (id == this->id()) - goto LAMEPEER; // Just their info - we already have that. - - if (!ep.port()) - goto LAMEPEER; // Zero port? Don't think so. - - if (ep.port() >= /*49152*/32768) - goto LAMEPEER; // Private port according to IANA. - - // OK passed all our checks. Assume it's good. - addRating(1000); - m_server->addNode(id, NodeIPEndpoint(ep.address(), ep.port(), ep.port())); - clog(NetTriviaDetail) << "New peer: " << ep << "(" << id << ")"; - CONTINUE:; - LAMEPEER:; - } - break; - default: + // v4 frame headers are useless, offset packet type used + // v5 protocol type is in header, packet type not offset + if (_capId == 0 && _t < UserPacket) + return interpret(_t, _r); + if (m_info.protocolVersion >= 5) + for (auto const& i: m_capabilities) + if (_capId == (uint16_t)i.first.second) + return i.second->m_enabled ? i.second->interpret(_t, _r) : true; + if (m_info.protocolVersion <= 4) for (auto const& i: m_capabilities) if (_t >= (int)i.second->m_idOffset && _t - i.second->m_idOffset < i.second->hostCapability()->messageCount()) - { - if (i.second->m_enabled) - return i.second->interpret(_t - i.second->m_idOffset, _r); - else - return true; - } - return false; - } + return i.second->m_enabled ? i.second->interpret(_t - i.second->m_idOffset, _r) : true; + return false; } catch (std::exception const& _e) { @@ -271,6 +184,101 @@ bool Session::interpret(PacketType _t, RLP const& _r) return true; } +bool Session::interpret(PacketType _t, RLP const& _r) +{ + switch (_t) + { + case DisconnectPacket: + { + string reason = "Unspecified"; + auto r = (DisconnectReason)_r[0].toInt(); + if (!_r[0].isInt()) + drop(BadProtocol); + else + { + reason = reasonOf(r); + clog(NetMessageSummary) << "Disconnect (reason: " << reason << ")"; + drop(DisconnectRequested); + } + break; + } + case PingPacket: + { + clog(NetTriviaSummary) << "Ping"; + RLPStream s; + sealAndSend(prep(s, PongPacket)); + break; + } + case PongPacket: + m_info.lastPing = std::chrono::steady_clock::now() - m_ping; + clog(NetTriviaSummary) << "Latency: " << chrono::duration_cast(m_info.lastPing).count() << " ms"; + break; + case GetPeersPacket: + // Disabled for interop testing. + // GetPeers/PeersPacket will be modified to only exchange new nodes which it's peers are interested in. + break; + + clog(NetTriviaSummary) << "GetPeers"; + m_theyRequestedNodes = true; + serviceNodesRequest(); + break; + case PeersPacket: + // Disabled for interop testing. + // GetPeers/PeersPacket will be modified to only exchange new nodes which it's peers are interested in. + break; + + clog(NetTriviaSummary) << "Peers (" << dec << (_r.itemCount() - 1) << " entries)"; + m_weRequestedNodes = false; + for (unsigned i = 0; i < _r.itemCount(); ++i) + { + bi::address peerAddress; + if (_r[i][0].size() == 16) + peerAddress = bi::address_v6(_r[i][0].toHash>().asArray()); + else if (_r[i][0].size() == 4) + peerAddress = bi::address_v4(_r[i][0].toHash>().asArray()); + else + { + cwarn << "Received bad peer packet:" << _r; + disconnect(BadProtocol); + return true; + } + auto ep = bi::tcp::endpoint(peerAddress, _r[i][1].toInt()); + NodeId id = _r[i][2].toHash(); + + clog(NetAllDetail) << "Checking: " << ep << "(" << id << ")"; + + if (!isPublicAddress(peerAddress)) + goto CONTINUE; // Private address. Ignore. + + if (!id) + goto LAMEPEER; // Null identity. Ignore. + + if (m_server->id() == id) + goto LAMEPEER; // Just our info - we already have that. + + if (id == this->id()) + goto LAMEPEER; // Just their info - we already have that. + + if (!ep.port()) + goto LAMEPEER; // Zero port? Don't think so. + + if (ep.port() >= /*49152*/32768) + goto LAMEPEER; // Private port according to IANA. + + // OK passed all our checks. Assume it's good. + addRating(1000); + m_server->addNode(id, NodeIPEndpoint(ep.address(), ep.port(), ep.port())); + clog(NetTriviaDetail) << "New peer: " << ep << "(" << id << ")"; + CONTINUE:; + LAMEPEER:; + } + break; + default: + return false; + } + return true; +} + void Session::ping() { RLPStream s; @@ -292,12 +300,9 @@ void Session::sealAndSend(RLPStream& _s) bool Session::checkPacket(bytesConstRef _msg) { - if (_msg.size() < 2) + if (_msg[0] > 0x7f || _msg.size() < 2) return false; - if (_msg[0] > 0x7f) - return false; - RLP r(_msg.cropped(1)); - if (r.actualSize() + 1 != _msg.size()) + if (RLP(_msg.cropped(1)).actualSize() + 1 != _msg.size()) return false; return true; } @@ -413,82 +418,78 @@ void Session::doRead() { ThreadContext tc(info().id.abridged()); ThreadContext tc2(info().clientVersion); - if (ec && ec.category() != boost::asio::error::get_misc_category() && ec.value() != boost::asio::error::eof) + if (!checkRead(h256::size, ec, length)) + return; + else if (!m_io->authAndDecryptHeader(bytesRef(m_data.data(), length))) { - clog(NetWarn) << "Error reading: " << ec.message(); - drop(TCPError); + clog(NetWarn) << "header decrypt failed"; + drop(BadProtocol); // todo: better error + return; + } + + RLPXFrameInfo header; + try + { + header = RLPXFrameInfo(bytesConstRef(m_data.data(), length)); } - else if (ec && length == 0) + catch (std::exception const& _e) + { + clog(NetWarn) << "Exception decoding frame header RLP:" << bytesConstRef(m_data.data(), h128::size).cropped(3); + drop(BadProtocol); return; - else + } + + /// read padded frame and mac + auto tlen = header.length + header.padding + h128::size; + ba::async_read(m_socket->ref(), boost::asio::buffer(m_data, tlen), [this, self, header, tlen](boost::system::error_code ec, std::size_t length) { - /// authenticate and decrypt header - bytesRef header(m_data.data(), h256::size); - if (!m_io->authAndDecryptHeader(header)) + ThreadContext tc(info().id.abridged()); + ThreadContext tc2(info().clientVersion); + if (!checkRead(tlen, ec, length)) + return; + else if (!m_io->authAndDecryptFrame(bytesRef(m_data.data(), tlen))) { - clog(NetWarn) << "header decrypt failed"; + clog(NetWarn) << "frame decrypt failed"; drop(BadProtocol); // todo: better error return; } - /// check frame size - uint32_t frameSize = (m_data[0] * 256 + m_data[1]) * 256 + m_data[2]; - if (frameSize >= (uint32_t)1 << 24) + bytesConstRef frame(m_data.data(), header.length); + if (!checkPacket(frame)) { - clog(NetWarn) << "frame size too large"; - drop(BadProtocol); + cerr << "Received " << frame.size() << ": " << toHex(frame) << endl; + clog(NetWarn) << "INVALID MESSAGE RECEIVED"; + disconnect(BadProtocol); return; } - - /// rlp of header has protocol-type, sequence-id[, total-packet-size] - bytes headerRLP(13); - bytesConstRef(m_data.data(), h128::size).cropped(3).copyTo(&headerRLP); - - /// read padded frame and mac - auto tlen = frameSize + ((16 - (frameSize % 16)) % 16) + h128::size; - ba::async_read(m_socket->ref(), boost::asio::buffer(m_data, tlen), [this, self, headerRLP, frameSize, tlen](boost::system::error_code ec, std::size_t length) + else { - ThreadContext tc(info().id.abridged()); - ThreadContext tc2(info().clientVersion); - if (ec && ec.category() != boost::asio::error::get_misc_category() && ec.value() != boost::asio::error::eof) - { - clog(NetWarn) << "Error reading: " << ec.message(); - drop(TCPError); - } - else if (ec && length < tlen) - { - clog(NetWarn) << "Error reading - Abrupt peer disconnect: " << ec.message(); - repMan().noteRude(*this); - drop(TCPError); - return; - } - else - { - if (!m_io->authAndDecryptFrame(bytesRef(m_data.data(), tlen))) - { - clog(NetWarn) << "frame decrypt failed"; - drop(BadProtocol); // todo: better error - return; - } - - bytesConstRef frame(m_data.data(), frameSize); - if (!checkPacket(frame)) - { - cerr << "Received " << frame.size() << ": " << toHex(frame) << endl; - clog(NetWarn) << "INVALID MESSAGE RECEIVED"; - disconnect(BadProtocol); - return; - } - else - { - auto packetType = (PacketType)RLP(frame.cropped(0, 1)).toInt(); - RLP r(frame.cropped(1)); - if (!interpret(packetType, r)) - clog(NetWarn) << "Couldn't interpret packet." << RLP(r); - } - doRead(); - } - }); - } + auto packetType = (PacketType)RLP(frame.cropped(0, 1)).toInt(); + RLP r(frame.cropped(1)); + if (!frameReceived(header.protocolId, packetType, r)) + clog(NetWarn) << "Couldn't interpret packet." << RLP(r); + } + doRead(); + }); }); } + +bool Session::checkRead(std::size_t _expected, boost::system::error_code _ec, std::size_t _length) +{ + if (_ec && _ec.category() != boost::asio::error::get_misc_category() && _ec.value() != boost::asio::error::eof) + { + clog(NetConnect) << "Error reading: " << _ec.message(); + drop(TCPError); + return false; + } + else if (_ec && _length < _expected) + { + clog(NetWarn) << "Error reading - Abrupt peer disconnect: " << _ec.message(); + repMan().noteRude(*this); + drop(TCPError); + return false; + } + // If this fails then there's an unhandled asio error + assert(_expected == _length); + return true; +} diff --git a/libp2p/Session.h b/libp2p/Session.h index 6b45fe381..ec079cd1d 100644 --- a/libp2p/Session.h +++ b/libp2p/Session.h @@ -96,13 +96,19 @@ private: /// Perform a read on the socket. void doRead(); + + /// Check error code after reading and drop peer if error code. + bool checkRead(std::size_t _expected, boost::system::error_code _ec, std::size_t _length); /// Perform a single round of the write operation. This could end up calling itself asynchronously. void write(); - /// Interpret an incoming message. - bool interpret(PacketType _t, RLP const& _r); + /// Deliver RLPX packet to Session or Capability for interpretation. + bool frameReceived(uint16_t _capId, PacketType _t, RLP const& _r); + /// Interpret an incoming Session packet. + bool interpret(PacketType _t, RLP const& _r); + /// @returns true iff the _msg forms a valid message for sending or receiving on the network. static bool checkPacket(bytesConstRef _msg); diff --git a/test/libp2p/net.cpp b/test/libp2p/net.cpp index 8c652b479..1e3e2e15c 100644 --- a/test/libp2p/net.cpp +++ b/test/libp2p/net.cpp @@ -314,23 +314,23 @@ BOOST_AUTO_TEST_CASE(kademlia) node.nodeTable->discover(); // ideally, joining with empty node table logs warning we can check for node.setup(); node.populate(); - clog << "NodeTable:\n" << *node.nodeTable.get() << endl; +// clog << "NodeTable:\n" << *node.nodeTable.get() << endl; node.populateAll(); - clog << "NodeTable:\n" << *node.nodeTable.get() << endl; +// clog << "NodeTable:\n" << *node.nodeTable.get() << endl; auto nodes = node.nodeTable->nodes(); nodes.sort(); node.nodeTable->reset(); - clog << "NodeTable:\n" << *node.nodeTable.get() << endl; +// clog << "NodeTable:\n" << *node.nodeTable.get() << endl; node.populate(1); - clog << "NodeTable:\n" << *node.nodeTable.get() << endl; +// clog << "NodeTable:\n" << *node.nodeTable.get() << endl; node.nodeTable->discover(); this_thread::sleep_for(chrono::milliseconds(2000)); - clog << "NodeTable:\n" << *node.nodeTable.get() << endl; +// clog << "NodeTable:\n" << *node.nodeTable.get() << endl; BOOST_REQUIRE_EQUAL(node.nodeTable->count(), 8); From 78467d3ae0af55b3c8c96a9e3a779ae21bc2d0c3 Mon Sep 17 00:00:00 2001 From: subtly Date: Sat, 20 Jun 2015 07:31:20 -0400 Subject: [PATCH 02/11] small rename --- libp2p/Session.cpp | 4 ++-- libp2p/Session.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index 95fd4a3d6..f24377fb2 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -155,7 +155,7 @@ void Session::serviceNodesRequest() addNote("peers", "done"); } -bool Session::frameReceived(uint16_t _capId, PacketType _t, RLP const& _r) +bool Session::readPacket(uint16_t _capId, PacketType _t, RLP const& _r) { m_lastReceived = chrono::steady_clock::now(); clog(NetRight) << _t << _r; @@ -466,7 +466,7 @@ void Session::doRead() { auto packetType = (PacketType)RLP(frame.cropped(0, 1)).toInt(); RLP r(frame.cropped(1)); - if (!frameReceived(header.protocolId, packetType, r)) + if (!readPacket(header.protocolId, packetType, r)) clog(NetWarn) << "Couldn't interpret packet." << RLP(r); } doRead(); diff --git a/libp2p/Session.h b/libp2p/Session.h index ec079cd1d..ba46c5a16 100644 --- a/libp2p/Session.h +++ b/libp2p/Session.h @@ -104,7 +104,7 @@ private: void write(); /// Deliver RLPX packet to Session or Capability for interpretation. - bool frameReceived(uint16_t _capId, PacketType _t, RLP const& _r); + bool readPacket(uint16_t _capId, PacketType _t, RLP const& _r); /// Interpret an incoming Session packet. bool interpret(PacketType _t, RLP const& _r); From 813c365fc421e65301296a3054f0b07d1dc26674 Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 30 Jun 2015 03:58:41 -0400 Subject: [PATCH 03/11] Only modify logger to \r \r for interactive mode and do so with thread safely. --- eth/main.cpp | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/eth/main.cpp b/eth/main.cpp index 64679762d..ba55e5664 100644 --- a/eth/main.cpp +++ b/eth/main.cpp @@ -703,12 +703,24 @@ int main(int argc, char** argv) string logbuf; std::string additional; - g_logPost = [&](std::string const& a, char const*){ - if (g_silence) - logbuf += a + "\n"; - else - cout << "\r \r" << a << endl << additional << flush; - }; + if (interactive) + g_logPost = [&](std::string const& a, char const*){ + static SpinLock s_lock; + SpinGuard l(s_lock); + + if (g_silence) + logbuf += a + "\n"; + else + cout << "\r \r" << a << endl << additional << flush; + + // helpful to use OutputDebugString on windows + #ifdef _WIN32 + { + OutputDebugStringA(_s.data()); + OutputDebugStringA("\n"); + } + #endif + }; auto getPassword = [&](string const& prompt){ auto s = g_silence; From b39d687be274a74d42e16f316b3a9ed970dd7190 Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 30 Jun 2015 05:12:39 -0400 Subject: [PATCH 04/11] fix copy/paste fail --- eth/main.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/eth/main.cpp b/eth/main.cpp index ba55e5664..4361598d4 100644 --- a/eth/main.cpp +++ b/eth/main.cpp @@ -712,11 +712,11 @@ int main(int argc, char** argv) logbuf += a + "\n"; else cout << "\r \r" << a << endl << additional << flush; - + // helpful to use OutputDebugString on windows #ifdef _WIN32 { - OutputDebugStringA(_s.data()); + OutputDebugStringA(a.data()); OutputDebugStringA("\n"); } #endif From 43f0112456b7f5d56f746f00f1eb3c1cea0616ea Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Mon, 29 Jun 2015 12:38:06 +0200 Subject: [PATCH 05/11] Mutex & reference counter --- libwhisper/WhisperHost.cpp | 5 ++++- libwhisper/WhisperPeer.cpp | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/libwhisper/WhisperHost.cpp b/libwhisper/WhisperHost.cpp index 1a47a56d1..2d037133c 100644 --- a/libwhisper/WhisperHost.cpp +++ b/libwhisper/WhisperHost.cpp @@ -94,8 +94,11 @@ unsigned WhisperHost::installWatch(shh::Topics const& _t) DEV_GUARDED(m_filterLock) { - if (!m_filters.count(h)) + auto it = m_filters.find(h); + if (m_filters.end() == it) m_filters.insert(make_pair(h, f)); + else + it->second.refCount++; ret = m_watches.size() ? m_watches.rbegin()->first + 1 : 0; m_watches[ret] = ClientWatch(h); diff --git a/libwhisper/WhisperPeer.cpp b/libwhisper/WhisperPeer.cpp index 665364f49..1f48468df 100644 --- a/libwhisper/WhisperPeer.cpp +++ b/libwhisper/WhisperPeer.cpp @@ -58,7 +58,10 @@ bool WhisperPeer::interpret(unsigned _id, RLP const& _r) disable("Invalid protocol version."); for (auto const& m: host()->all()) + { + Guard l(x_unseen); m_unseen.insert(make_pair(0, m.first)); + } if (session()->id() < host()->host()->id()) sendMessages(); From c50437a5f95be117e23ff5b6f5d341628e74d680 Mon Sep 17 00:00:00 2001 From: Vlad Gluhovsky Date: Mon, 29 Jun 2015 13:01:05 +0200 Subject: [PATCH 06/11] uninstallWatch() fixed + test --- libwhisper/WhisperHost.cpp | 7 +++---- test/libwhisper/whisperTopic.cpp | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/libwhisper/WhisperHost.cpp b/libwhisper/WhisperHost.cpp index 2d037133c..150d5cd63 100644 --- a/libwhisper/WhisperHost.cpp +++ b/libwhisper/WhisperHost.cpp @@ -100,11 +100,10 @@ unsigned WhisperHost::installWatch(shh::Topics const& _t) else it->second.refCount++; + m_bloom.addRaw(f.filter.exportBloom()); ret = m_watches.size() ? m_watches.rbegin()->first + 1 : 0; m_watches[ret] = ClientWatch(h); cwatshh << "+++" << ret << h; - - m_bloom.addRaw(f.filter.exportBloom()); } noteAdvertiseTopicsOfInterest(); @@ -141,16 +140,16 @@ void WhisperHost::uninstallWatch(unsigned _i) auto it = m_watches.find(_i); if (it == m_watches.end()) return; + auto id = it->second.id; m_watches.erase(it); auto fit = m_filters.find(id); if (fit != m_filters.end()) { + m_bloom.removeRaw(fit->second.filter.exportBloom()); if (!--fit->second.refCount) m_filters.erase(fit); - - m_bloom.removeRaw(fit->second.filter.exportBloom()); } } diff --git a/test/libwhisper/whisperTopic.cpp b/test/libwhisper/whisperTopic.cpp index a152f756e..47d17503c 100644 --- a/test/libwhisper/whisperTopic.cpp +++ b/test/libwhisper/whisperTopic.cpp @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(topicAdvertising) Host host2("second", NetworkPreferences("127.0.0.1", 30305, false)); host2.setIdealPeerCount(1); auto whost2 = host2.registerCapability(new WhisperHost()); - whost2->installWatch(BuildTopicMask("test2")); + unsigned w2 = whost2->installWatch(BuildTopicMask("test2")); host2.start(); while (!host2.haveNetwork()) @@ -354,7 +354,7 @@ BOOST_AUTO_TEST_CASE(topicAdvertising) BOOST_REQUIRE(bf1); BOOST_REQUIRE(!whost1->bloom()); - whost1->installWatch(BuildTopicMask("test1")); + unsigned w1 = whost1->installWatch(BuildTopicMask("test1")); for (int i = 0; i < 600; ++i) { @@ -372,6 +372,16 @@ BOOST_AUTO_TEST_CASE(topicAdvertising) bf1 = whost1->bloom(); BOOST_REQUIRE_EQUAL(bf1, bf2); BOOST_REQUIRE(bf1); + + unsigned random = 0xC0FFEE; + whost1->uninstallWatch(w1); + whost1->uninstallWatch(random); + whost1->uninstallWatch(w1); + whost1->uninstallWatch(random); + whost2->uninstallWatch(random); + whost2->uninstallWatch(w2); + whost2->uninstallWatch(random); + whost2->uninstallWatch(w2); } BOOST_AUTO_TEST_SUITE_END() From 9c483859d1bf346a0af2ca026e3c767565d1594a Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 23 Jun 2015 16:56:59 +0200 Subject: [PATCH 07/11] Fixed checking of abstract functions. Fixes #2264 --- libsolidity/AST.cpp | 36 +++++++++++++------ .../SolidityNameAndTypeResolution.cpp | 17 +++++++++ 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index cb935183f..fac4360f1 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -175,24 +175,40 @@ void ContractDefinition::checkDuplicateFunctions() const void ContractDefinition::checkAbstractFunctions() { - map functions; + // Mapping from name to function definition (exactly one per argument type equality class) and + // flag to indicate whether it is fully implemented. + using FunTypeAndFlag = std::pair; + map> functions; // Search from base to derived for (ContractDefinition const* contract: boost::adaptors::reverse(getLinearizedBaseContracts())) for (ASTPointer const& function: contract->getDefinedFunctions()) { - string const& name = function->getName(); - if (!function->isFullyImplemented() && functions.count(name) && functions[name]) - BOOST_THROW_EXCEPTION(function->createTypeError("Redeclaring an already implemented function as abstract")); - functions[name] = function->isFullyImplemented(); + auto& overloads = functions[function->getName()]; + FunctionTypePointer funType = make_shared(*function); + auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag) + { + return funType->hasEqualArgumentTypes(*_funAndFlag.first); + }); + if (it == overloads.end()) + overloads.push_back(make_pair(funType, function->isFullyImplemented())); + else if (it->second) + { + if (!function->isFullyImplemented()) + BOOST_THROW_EXCEPTION(function->createTypeError("Redeclaring an already implemented function as abstract")); + } + else if (function->isFullyImplemented()) + it->second = true; } + // Set to not fully implemented if at least one flag is false. for (auto const& it: functions) - if (!it.second) - { - setFullyImplemented(false); - break; - } + for (auto const& funAndFlag: it.second) + if (!funAndFlag.second) + { + setFullyImplemented(false); + return; + } } void ContractDefinition::checkAbstractConstructors() diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index df976eaea..4914ef975 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -403,6 +403,23 @@ BOOST_AUTO_TEST_CASE(abstract_contract) BOOST_CHECK(derived->getDefinedFunctions()[0]->isFullyImplemented()); } +BOOST_AUTO_TEST_CASE(abstract_contract_with_overload) +{ + ASTPointer sourceUnit; + char const* text = R"( + contract base { function foo(bool); } + contract derived is base { function foo(uint) {} } + )"; + ETH_TEST_REQUIRE_NO_THROW(sourceUnit = parseTextAndResolveNames(text), "Parsing and name Resolving failed"); + std::vector> nodes = sourceUnit->getNodes(); + ContractDefinition* base = dynamic_cast(nodes[0].get()); + ContractDefinition* derived = dynamic_cast(nodes[1].get()); + BOOST_REQUIRE(base); + BOOST_CHECK(!base->isFullyImplemented()); + BOOST_REQUIRE(derived); + BOOST_CHECK(!derived->isFullyImplemented()); +} + BOOST_AUTO_TEST_CASE(create_abstract_contract) { ASTPointer sourceUnit; From 49a861726163cd14837ecb0c44894c17265b6d27 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 30 Jun 2015 14:08:26 +0200 Subject: [PATCH 08/11] fixed block memory management --- libethereum/BlockChain.cpp | 2 +- libethereum/BlockQueue.cpp | 6 +++--- libethereum/State.cpp | 2 +- libethereum/VerifiedBlock.h | 24 ++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/libethereum/BlockChain.cpp b/libethereum/BlockChain.cpp index 55c213a64..d58a80ec2 100644 --- a/libethereum/BlockChain.cpp +++ b/libethereum/BlockChain.cpp @@ -541,7 +541,7 @@ ImportRoute BlockChain::import(VerifiedBlockRef const& _block, OverlayDB const& #endif } #if ETH_CATCH - catch (BadRoot& ex) + catch (BadRoot&) { cwarn << "BadRoot error. Retrying import later."; BOOST_THROW_EXCEPTION(FutureTime()); diff --git a/libethereum/BlockQueue.cpp b/libethereum/BlockQueue.cpp index 9e647c9c1..074278d9a 100644 --- a/libethereum/BlockQueue.cpp +++ b/libethereum/BlockQueue.cpp @@ -102,7 +102,7 @@ void BlockQueue::verifierBody() BlockInfo bi; bi.mixHash = work.hash; bi.parentHash = work.parentHash; - m_verifying.push_back(VerifiedBlock { VerifiedBlockRef { bytesConstRef(), move(bi), Transactions() }, bytes() }); + m_verifying.emplace_back(move(bi)); } VerifiedBlock res; @@ -148,7 +148,7 @@ void BlockQueue::verifierBody() m_knownBad.insert(res.verified.info.hash()); } else - m_verified.push_back(move(res)); + m_verified.emplace_back(move(res)); while (m_verifying.size() && !m_verifying.front().blockData.empty()) { if (m_knownBad.count(m_verifying.front().verified.info.parentHash)) @@ -157,7 +157,7 @@ void BlockQueue::verifierBody() m_knownBad.insert(res.verified.info.hash()); } else - m_verified.push_back(move(m_verifying.front())); + m_verified.emplace_back(move(m_verifying.front())); m_verifying.pop_front(); } ready = true; diff --git a/libethereum/State.cpp b/libethereum/State.cpp index ead72f496..badc05887 100644 --- a/libethereum/State.cpp +++ b/libethereum/State.cpp @@ -117,7 +117,7 @@ State::State(OverlayDB const& _db, BaseState _bs, Address _coinbaseAddress): PopulationStatistics State::populateFromChain(BlockChain const& _bc, h256 const& _h, ImportRequirements::value _ir) { - PopulationStatistics ret; + PopulationStatistics ret { 0.0, 0.0 }; if (!_bc.isKnown(_h)) { diff --git a/libethereum/VerifiedBlock.h b/libethereum/VerifiedBlock.h index ddd808901..6cafe4b2f 100644 --- a/libethereum/VerifiedBlock.h +++ b/libethereum/VerifiedBlock.h @@ -43,8 +43,32 @@ struct VerifiedBlockRef /// @brief Verified block info, combines block data and verified info/transactions struct VerifiedBlock { + VerifiedBlock() {}; + + VerifiedBlock(BlockInfo&& _bi) + { + verified.info = _bi; + } + + VerifiedBlock(VerifiedBlock&& _other): + verified(std::move(_other.verified)), + blockData(std::move(_other.blockData)) + { + } + + VerifiedBlock& operator=(VerifiedBlock&& _other) + { + verified = (std::move(_other.verified)); + blockData = (std::move(_other.blockData)); + return *this; + } + VerifiedBlockRef verified; ///< Verified block structures bytes blockData; ///< Block data + +private: + VerifiedBlock(VerifiedBlock const&) = delete; + VerifiedBlock operator=(VerifiedBlock const&) = delete; }; using VerifiedBlocks = std::vector; From e22e3d2d85ac10d2abf1609e304ba9b5996d63c5 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 30 Jun 2015 15:45:28 +0200 Subject: [PATCH 09/11] fixing linux build --- libdevcore/Common.h | 1 - libwhisper/Common.h | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libdevcore/Common.h b/libdevcore/Common.h index 4ed4e4365..f1d35bbc7 100644 --- a/libdevcore/Common.h +++ b/libdevcore/Common.h @@ -45,7 +45,6 @@ #pragma warning(push) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-parameter" -#pragma GCC diagnostic ignored "-fpermissive" #include #if (BOOST_VERSION == 105800) #include "boost_multiprecision_number_compare_bug_workaround.hpp" diff --git a/libwhisper/Common.h b/libwhisper/Common.h index deb13d111..c05ac34ca 100644 --- a/libwhisper/Common.h +++ b/libwhisper/Common.h @@ -58,8 +58,8 @@ enum WhisperPacket PacketCount }; -enum { TopicBloomFilterSize = 64 }; -enum { WhisperProtocolVersion = 3 }; +const int TopicBloomFilterSize = 64; +const int WhisperProtocolVersion = 3; using AbridgedTopic = FixedHash<4>; using Topic = h256; From 77bfce921b0698ce5e349eb4c9c0f3e3d335ee89 Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 30 Jun 2015 17:07:54 +0200 Subject: [PATCH 10/11] static --- libwhisper/Common.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libwhisper/Common.h b/libwhisper/Common.h index c05ac34ca..7a0707801 100644 --- a/libwhisper/Common.h +++ b/libwhisper/Common.h @@ -58,8 +58,8 @@ enum WhisperPacket PacketCount }; -const int TopicBloomFilterSize = 64; -const int WhisperProtocolVersion = 3; +static const int TopicBloomFilterSize = 64; +static const int WhisperProtocolVersion = 3; using AbridgedTopic = FixedHash<4>; using Topic = h256; From 4941aa82bfe34c643af49112ab834ed566eeb17e Mon Sep 17 00:00:00 2001 From: arkpar Date: Tue, 30 Jun 2015 17:24:32 +0200 Subject: [PATCH 11/11] style --- libwhisper/BloomFilter.h | 2 +- libwhisper/Common.cpp | 6 +++--- libwhisper/Common.h | 8 +++++--- libwhisper/WhisperHost.h | 4 ++-- libwhisper/WhisperPeer.cpp | 4 ++-- libwhisper/WhisperPeer.h | 10 +++++----- test/libwhisper/bloomFilter.cpp | 4 ++-- test/libwhisper/whisperTopic.cpp | 4 ++-- 8 files changed, 22 insertions(+), 20 deletions(-) diff --git a/libwhisper/BloomFilter.h b/libwhisper/BloomFilter.h index 157f4b011..a624be157 100644 --- a/libwhisper/BloomFilter.h +++ b/libwhisper/BloomFilter.h @@ -91,7 +91,7 @@ bool TopicBloomFilterBase::isBitSet(FixedHash const& _h, unsigned _index) return (_h[iByte] & c_powerOfTwoBitMmask[iBit]) != 0; } -using TopicBloomFilter = TopicBloomFilterBase; +using TopicBloomFilter = TopicBloomFilterBase; } } diff --git a/libwhisper/Common.cpp b/libwhisper/Common.cpp index 79c87b96f..748180647 100644 --- a/libwhisper/Common.cpp +++ b/libwhisper/Common.cpp @@ -95,12 +95,12 @@ TopicFilter::TopicFilter(RLP const& _r) } } -FixedHash TopicFilter::exportBloom() const +TopicBloomFilterHash TopicFilter::exportBloom() const { - FixedHash ret; + TopicBloomFilterHash ret; for (TopicMask const& t: m_topicMasks) for (auto const& i: t) - ret |= i.first.template bloomPart(); + ret |= i.first.template bloomPart(); return ret; } diff --git a/libwhisper/Common.h b/libwhisper/Common.h index 7a0707801..d5d926291 100644 --- a/libwhisper/Common.h +++ b/libwhisper/Common.h @@ -58,8 +58,8 @@ enum WhisperPacket PacketCount }; -static const int TopicBloomFilterSize = 64; -static const int WhisperProtocolVersion = 3; +static const int c_topicBloomFilterSize = 64; +static const int c_whisperProtocolVersion = 3; using AbridgedTopic = FixedHash<4>; using Topic = h256; @@ -67,6 +67,8 @@ using Topic = h256; using AbridgedTopics = std::vector; using Topics = h256s; +using TopicBloomFilterHash = FixedHash; + AbridgedTopic abridge(Topic const& _topic); AbridgedTopics abridge(Topics const& _topics); @@ -107,7 +109,7 @@ public: void streamRLP(RLPStream& _s) const { _s << m_topicMasks; } h256 sha3() const; bool matches(Envelope const& _m) const; - FixedHash exportBloom() const; + TopicBloomFilterHash exportBloom() const; private: TopicMasks m_topicMasks; diff --git a/libwhisper/WhisperHost.h b/libwhisper/WhisperHost.h index 1a43eda3a..a6de09c38 100644 --- a/libwhisper/WhisperHost.h +++ b/libwhisper/WhisperHost.h @@ -50,11 +50,11 @@ class WhisperHost: public HostCapability, public Interface, public public: WhisperHost(); virtual ~WhisperHost(); - unsigned protocolVersion() const { return WhisperProtocolVersion; } + unsigned protocolVersion() const { return c_whisperProtocolVersion; } /// remove old messages void cleanup(); std::map all() const { dev::ReadGuard l(x_messages); return m_messages; } - FixedHash bloom() const { dev::Guard l(m_filterLock); return m_bloom; } + TopicBloomFilterHash bloom() const { dev::Guard l(m_filterLock); return m_bloom; } virtual void inject(Envelope const& _e, WhisperPeer* _from = nullptr) override; virtual Topics const& fullTopics(unsigned _id) const override { try { return m_filters.at(m_watches.at(_id).id).full; } catch (...) { return EmptyTopics; } } diff --git a/libwhisper/WhisperPeer.cpp b/libwhisper/WhisperPeer.cpp index 665364f49..4475769e3 100644 --- a/libwhisper/WhisperPeer.cpp +++ b/libwhisper/WhisperPeer.cpp @@ -74,7 +74,7 @@ bool WhisperPeer::interpret(unsigned _id, RLP const& _r) } case TopicFilterPacket: { - setBloom((FixedHash)_r[0]); + setBloom((TopicBloomFilterHash)_r[0]); break; } default: @@ -115,7 +115,7 @@ void WhisperPeer::noteNewMessage(h256 _h, Envelope const& _m) m_unseen.insert(make_pair(rating(_m), _h)); } -void WhisperPeer::sendTopicsOfInterest(FixedHash const& _bloom) +void WhisperPeer::sendTopicsOfInterest(TopicBloomFilterHash const& _bloom) { DEV_GUARDED(x_advertiseTopicsOfInterest) m_advertiseTopicsOfInterest = false; diff --git a/libwhisper/WhisperPeer.h b/libwhisper/WhisperPeer.h index 1be2df97e..48f984013 100644 --- a/libwhisper/WhisperPeer.h +++ b/libwhisper/WhisperPeer.h @@ -53,10 +53,10 @@ public: virtual ~WhisperPeer(); WhisperHost* host() const; static std::string name() { return "shh"; } - static u256 version() { return WhisperProtocolVersion; } + static u256 version() { return c_whisperProtocolVersion; } static unsigned messageCount() { return PacketCount; } - FixedHash bloom() const { dev::Guard g(x_bloom); return m_bloom; } - void sendTopicsOfInterest(FixedHash const& _bloom); ///< sends our bloom filter to remote peer + TopicBloomFilterHash bloom() const { dev::Guard g(x_bloom); return m_bloom; } + void sendTopicsOfInterest(TopicBloomFilterHash const& _bloom); ///< sends our bloom filter to remote peer void noteAdvertiseTopicsOfInterest() { dev::Guard g(x_advertiseTopicsOfInterest); m_advertiseTopicsOfInterest = true; } private: @@ -64,14 +64,14 @@ private: void sendMessages(); unsigned rating(Envelope const&) const { return 0; } // TODO void noteNewMessage(h256 _h, Envelope const& _m); - void setBloom(FixedHash const& _b) { dev::Guard g(x_bloom); m_bloom = _b; } + void setBloom(TopicBloomFilterHash const& _b) { dev::Guard g(x_bloom); m_bloom = _b; } mutable dev::Mutex x_unseen; std::multimap m_unseen; ///< Rated according to what they want. std::chrono::system_clock::time_point m_timer = std::chrono::system_clock::now(); mutable dev::Mutex x_bloom; - FixedHash m_bloom; ///< Peer's topics of interest + TopicBloomFilterHash m_bloom; ///< Peer's topics of interest mutable dev::Mutex x_advertiseTopicsOfInterest; bool m_advertiseTopicsOfInterest; diff --git a/test/libwhisper/bloomFilter.cpp b/test/libwhisper/bloomFilter.cpp index 3e71ca305..814990d52 100644 --- a/test/libwhisper/bloomFilter.cpp +++ b/test/libwhisper/bloomFilter.cpp @@ -28,7 +28,7 @@ using namespace dev; using namespace dev::shh; using TopicBloomFilterShort = TopicBloomFilterBase<4>; -using TopicBloomFilterTest = TopicBloomFilterBase; +using TopicBloomFilterTest = TopicBloomFilterBase; void testAddNonExisting(TopicBloomFilterShort& _f, AbridgedTopic const& _h) { @@ -244,4 +244,4 @@ BOOST_AUTO_TEST_CASE(bloomFilterRaw) BOOST_REQUIRE(!f.contains(b00110111)); } -BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/libwhisper/whisperTopic.cpp b/test/libwhisper/whisperTopic.cpp index a152f756e..e309239c9 100644 --- a/test/libwhisper/whisperTopic.cpp +++ b/test/libwhisper/whisperTopic.cpp @@ -348,8 +348,8 @@ BOOST_AUTO_TEST_CASE(topicAdvertising) } BOOST_REQUIRE(sessions.size()); - FixedHash bf1 = sessions.back().first->cap()->bloom(); - FixedHash bf2 = whost2->bloom(); + TopicBloomFilterHash bf1 = sessions.back().first->cap()->bloom(); + TopicBloomFilterHash bf2 = whost2->bloom(); BOOST_REQUIRE_EQUAL(bf1, bf2); BOOST_REQUIRE(bf1); BOOST_REQUIRE(!whost1->bloom());