From ae1e2e071e81d37f621a4d127f36997b56ae4121 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 30 Jun 2012 18:52:52 -0700 Subject: [PATCH 1/5] Fix a crash bug Jed reported. --- src/SHAMapNodes.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/SHAMapNodes.cpp b/src/SHAMapNodes.cpp index dbdc8ee109..af990f6b40 100644 --- a/src/SHAMapNodes.cpp +++ b/src/SHAMapNodes.cpp @@ -255,15 +255,16 @@ SHAMapTreeNode::SHAMapTreeNode(const SHAMapNode& id, const std::vector(Serializer::getSHA512Half(rawNode), s.peekData()); + mItem = boost::make_shared(s.getSHA512Half(), s.peekData()); mType = tnTRANSACTION; } if (prefix == sHP_LeafNode) { uint256 u; s.get256(u, s.getLength() - 32); - s.chop(256 / 8); - if (u.isZero()) throw std::runtime_error("invalid PLN node"); + s.chop(32); + if (u.isZero()) + throw std::runtime_error("invalid PLN node"); mItem = boost::make_shared(u, s.peekData()); mType = tnACCOUNT_STATE; } From 30d76c87a39253128596270ced1ba570d3f7d36e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 30 Jun 2012 18:53:11 -0700 Subject: [PATCH 2/5] Don't crash if we're send a malicious map node from a peer. --- src/SHAMap.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/SHAMap.cpp b/src/SHAMap.cpp index cb48e8df5e..cc27ed70b6 100644 --- a/src/SHAMap.cpp +++ b/src/SHAMap.cpp @@ -630,11 +630,19 @@ SHAMapTreeNode::pointer SHAMap::fetchNodeExternal(const SHAMapNode& id, const ui throw SHAMapMissingNode(id, hash); assert(Serializer::getSHA512Half(obj->getData()) == hash); - SHAMapTreeNode::pointer ret = boost::make_shared(id, obj->getData(), mSeq, STN_ARF_PREFIXED); + try + { + SHAMapTreeNode::pointer ret = boost::make_shared(id, obj->getData(), mSeq, STN_ARF_PREFIXED); #ifdef DEBUG - assert((ret->getNodeHash() == hash) && (id == *ret)); + assert((ret->getNodeHash() == hash) && (id == *ret)); #endif - return ret; + return ret; + } + catch (...) + { + Log(lsWARNING) << "fetchNodeExternal gets an invalid node: " << hash.GetHex(); + throw SHAMapMissingNode(id, hash); + } } void SHAMap::armDirty() From c90f834f7f2eda481352972600728284c7020581 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 30 Jun 2012 18:59:38 -0700 Subject: [PATCH 3/5] Duh! This is the right fix for the bug Jed reported. --- src/SHAMapNodes.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/SHAMapNodes.cpp b/src/SHAMapNodes.cpp index af990f6b40..32d9b65d80 100644 --- a/src/SHAMapNodes.cpp +++ b/src/SHAMapNodes.cpp @@ -247,7 +247,10 @@ SHAMapTreeNode::SHAMapTreeNode(const SHAMapNode& id, const std::vector(s.getSHA512Half(), s.peekData()); mType = tnTRANSACTION; } - if (prefix == sHP_LeafNode) + else if (prefix == sHP_LeafNode) { uint256 u; s.get256(u, s.getLength() - 32); s.chop(32); if (u.isZero()) + { + Log(lsINFO) << "invalid PLN node"; throw std::runtime_error("invalid PLN node"); + } mItem = boost::make_shared(u, s.peekData()); mType = tnACCOUNT_STATE; } - if (prefix == sHP_InnerNode) + else if (prefix == sHP_InnerNode) { if (rawNode.size() != (512 + 4)) throw std::runtime_error("invalid PIN node"); @@ -277,7 +283,10 @@ SHAMapTreeNode::SHAMapTreeNode(const SHAMapNode& id, const std::vector Date: Sat, 30 Jun 2012 19:07:08 -0700 Subject: [PATCH 4/5] Keep and report some extra peer information. --- src/Peer.cpp | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/Peer.cpp b/src/Peer.cpp index 35da711dad..2ce12c4e1b 100644 --- a/src/Peer.cpp +++ b/src/Peer.cpp @@ -870,7 +870,15 @@ void Peer::recvStatus(newcoin::TMStatusChange& packet) Log(lsTRACE) << "Received status change from peer " << getIP(); if (!packet.has_networktime()) packet.set_networktime(theApp->getOPs().getNetworkTimeNC()); - mLastStatus = packet; + + if (!mLastStatus.has_newstatus() || packet.has_newstatus()) + mLastStatus = packet; + else + { // preserve old status + newcoin::NodeStatus status = mLastStatus.newstatus(); + mLastStatus = packet; + packet.set_newstatus(status); + } if (packet.newevent() == newcoin::neLOST_SYNC) { @@ -1165,6 +1173,23 @@ Json::Value Peer::getJson() (mHello.protoversion() != MAKE_VERSION_INT(PROTO_VERSION_MAJOR, PROTO_VERSION_MINOR))) ret["protocol"] = boost::lexical_cast(GET_VERSION_MAJOR(mHello.protoversion())) + "." + boost::lexical_cast(GET_VERSION_MINOR(mHello.protoversion())); + + if (!!mClosedLedgerHash) + ret["ledger"] = mClosedLedgerHash.GetHex(); + + if (mLastStatus.has_newstatus()) + { + switch (mLastStatus.newstatus()) + { + case newcoin::nsCONNECTING: ret["status"] = "connecting"; break; + case newcoin::nsCONNECTED: ret["status"] = "connected"; break; + case newcoin::nsMONITORING: ret["status"] = "monitoring"; break; + case newcoin::nsVALIDATING: ret["status"] = "validating"; break; + case newcoin::nsSHUTTING: ret["status"] = "shutting"; break; + default: Log(lsWARNING) << "Peer has unknown status: " << mLastStatus.newstatus(); + } + } + /* if (!mIpPort.first.empty()) { From c7eb0269c2c0b28a1f465f9d01a66aca409ddd33 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sat, 30 Jun 2012 22:17:53 -0700 Subject: [PATCH 5/5] For now, detect clocks out of sync and disallow connection. Improve speed of ledger synchronization by compressing all three base queries into one. --- src/LedgerAcquire.cpp | 61 +++++++++++++++++++++++++++++++++++-------- src/LedgerAcquire.h | 12 ++++----- src/NetworkOPs.cpp | 5 ++-- src/Peer.cpp | 34 +++++++++++++++++++++++- src/SHAMap.h | 1 + src/SHAMapSync.cpp | 7 +++++ 6 files changed, 100 insertions(+), 20 deletions(-) diff --git a/src/LedgerAcquire.cpp b/src/LedgerAcquire.cpp index 8049ddfbd0..ffd82a7f83 100644 --- a/src/LedgerAcquire.cpp +++ b/src/LedgerAcquire.cpp @@ -138,6 +138,7 @@ void LedgerAcquire::trigger(Peer::pointer peer) newcoin::TMGetLedger tmGL; tmGL.set_ledgerhash(mHash.begin(), mHash.size()); tmGL.set_itype(newcoin::liBASE); + *(tmGL.add_nodeids()) = SHAMapNode().getRawString(); if (peer) { sendRequest(tmGL, peer); @@ -285,7 +286,7 @@ void PeerSet::sendRequest(const newcoin::TMGetLedger& tmGL) } } -bool LedgerAcquire::takeBase(const std::string& data, Peer::pointer peer) +bool LedgerAcquire::takeBase(const std::string& data) { // Return value: true=normal, false=bad data #ifdef LA_DEBUG Log(lsTRACE) << "got base acquiring ledger " << mHash.GetHex(); @@ -311,12 +312,11 @@ bool LedgerAcquire::takeBase(const std::string& data, Peer::pointer peer) if (!mLedger->getTransHash()) mHaveTransactions = true; if (!mLedger->getAccountHash()) mHaveState = true; mLedger->setAcquiring(); - trigger(peer); return true; } bool LedgerAcquire::takeTxNode(const std::list& nodeIDs, - const std::list< std::vector >& data, Peer::pointer peer) + const std::list< std::vector >& data) { if (!mHaveBase) return false; std::list::const_iterator nodeIDit = nodeIDs.begin(); @@ -339,13 +339,12 @@ bool LedgerAcquire::takeTxNode(const std::list& nodeIDs, mHaveTransactions = true; if (mHaveState) mComplete = true; } - trigger(peer); progress(); return true; } bool LedgerAcquire::takeAsNode(const std::list& nodeIDs, - const std::list< std::vector >& data, Peer::pointer peer) + const std::list< std::vector >& data) { #ifdef LA_DEBUG Log(lsTRACE) << "got ASdata acquiring ledger " << mHash.GetHex(); @@ -371,11 +370,22 @@ bool LedgerAcquire::takeAsNode(const std::list& nodeIDs, mHaveState = true; if (mHaveTransactions) mComplete = true; } - trigger(peer); progress(); return true; } +bool LedgerAcquire::takeAsRootNode(const std::vector& data) +{ + if (!mHaveBase) return false; + return mLedger->peekAccountStateMap()->addRootNode(mLedger->getAccountHash(), data, STN_ARF_WIRE); +} + +bool LedgerAcquire::takeTxRootNode(const std::vector& data) +{ + if (!mHaveBase) return false; + return mLedger->peekTransactionMap()->addRootNode(mLedger->getTransHash(), data, STN_ARF_WIRE); +} + LedgerAcquire::pointer LedgerAcquireMaster::findCreate(const uint256& hash) { boost::mutex::scoped_lock sl(mLock); @@ -428,11 +438,34 @@ bool LedgerAcquireMaster::gotLedgerData(newcoin::TMLedgerData& packet, Peer::poi if (packet.type() == newcoin::liBASE) { - if (packet.nodes_size() != 1) return false; + if (packet.nodes_size() < 1) + return false; const newcoin::TMLedgerNode& node = packet.nodes(0); - return ledger->takeBase(node.nodedata(), peer); + if (!ledger->takeBase(node.nodedata())) + return false; + if (packet.nodes_size() == 1) + { + ledger->trigger(peer); + return true; + } + Log(lsDEBUG) << "liBASE includes ASbase"; + if (!ledger->takeAsRootNode(strCopy(packet.nodes(1).nodedata()))) + { + Log(lsWARNING) << "Included ASbase invalid"; + } + if (packet.nodes().size() == 2) + { + ledger->trigger(peer); + return true; + } + Log(lsDEBUG) << "liBASE includes TXbase"; + if (!ledger->takeTxRootNode(strCopy(packet.nodes(2).nodedata()))) + Log(lsWARNING) << "Invcluded TXbase invalid"; + ledger->trigger(peer); + return true; } - else if ((packet.type() == newcoin::liTX_NODE) || (packet.type() == newcoin::liAS_NODE)) + + if ((packet.type() == newcoin::liTX_NODE) || (packet.type() == newcoin::liAS_NODE)) { std::list nodeIDs; std::list< std::vector > nodeData; @@ -446,8 +479,14 @@ bool LedgerAcquireMaster::gotLedgerData(newcoin::TMLedgerData& packet, Peer::poi nodeIDs.push_back(SHAMapNode(node.nodeid().data(), node.nodeid().size())); nodeData.push_back(std::vector(node.nodedata().begin(), node.nodedata().end())); } - if (packet.type() == newcoin::liTX_NODE) return ledger->takeTxNode(nodeIDs, nodeData, peer); - else return ledger->takeAsNode(nodeIDs, nodeData, peer); + bool ret; + if (packet.type() == newcoin::liTX_NODE) + ret = ledger->takeTxNode(nodeIDs, nodeData); + else + ret = ledger->takeAsNode(nodeIDs, nodeData); + if (ret) + ledger->trigger(peer); + return ret; } return false; diff --git a/src/LedgerAcquire.h b/src/LedgerAcquire.h index cc499fdb3a..ba96139372 100644 --- a/src/LedgerAcquire.h +++ b/src/LedgerAcquire.h @@ -73,7 +73,6 @@ protected: void onTimer() { trigger(Peer::pointer()); } void newPeer(Peer::pointer peer) { trigger(peer); } - void trigger(Peer::pointer); boost::weak_ptr pmDowncast(); @@ -87,11 +86,12 @@ public: void addOnComplete(boost::function); - bool takeBase(const std::string& data, Peer::pointer); - bool takeTxNode(const std::list& IDs, const std::list >& data, - Peer::pointer); - bool takeAsNode(const std::list& IDs, const std::list >& data, - Peer::pointer); + bool takeBase(const std::string& data); + bool takeTxNode(const std::list& IDs, const std::list >& data); + bool takeTxRootNode(const std::vector& data); + bool takeAsNode(const std::list& IDs, const std::list >& data); + bool takeAsRootNode(const std::vector& data); + void trigger(Peer::pointer); }; class LedgerAcquireMaster diff --git a/src/NetworkOPs.cpp b/src/NetworkOPs.cpp index 4d3485c479..3800eadd13 100644 --- a/src/NetworkOPs.cpp +++ b/src/NetworkOPs.cpp @@ -388,7 +388,7 @@ bool NetworkOPs::checkLastClosedLedger(const std::vector& peerLis else if ((*it)->isConnected()) { uint256 peerLedger = (*it)->getClosedLedgerHash(); - if (!!peerLedger) + if (peerLedger.isNonZero()) { ValidationCount& vc = ledgers[peerLedger]; if ((vc.nodesUsing == 0) || ((*it)->getNodePublic() > vc.highNode)) @@ -513,7 +513,8 @@ int NetworkOPs::beginConsensus(const uint256& networkClosed, Ledger::pointer clo assert(closingLedger->getParentHash() == mLedgerMaster->getClosedLedger()->getHash()); // Create a consensus object to get consensus on this ledger - if (!!mConsensus) mConsensus->abort(); + if (!!mConsensus) + mConsensus->abort(); prevLedger->setImmutable(); mConsensus = boost::make_shared( networkClosed, diff --git a/src/Peer.cpp b/src/Peer.cpp index 2ce12c4e1b..e15a654dd5 100644 --- a/src/Peer.cpp +++ b/src/Peer.cpp @@ -571,7 +571,14 @@ void Peer::recvHello(newcoin::TMHello& packet) // Cancel verification timeout. (void) mVerifyTimer.cancel(); - if (packet.protoversionmin() < MAKE_VERSION_INT(MIN_PROTO_MAJOR, MIN_PROTO_MINOR)) + uint64 minTime = theApp->getOPs().getNetworkTimeNC() - 4; + uint64 maxTime = minTime + 8; + + if (packet.has_nettime() && ((packet.nettime() < minTime) || (packet.nettime() > maxTime))) + { + Log(lsINFO) << "Recv(Hello): Disconnect: Clocks are too far off"; + } + else if (packet.protoversionmin() < MAKE_VERSION_INT(MIN_PROTO_MAJOR, MIN_PROTO_MINOR)) { Log(lsINFO) << "Recv(Hello): Server requires protocol version " << GET_VERSION_MAJOR(packet.protoversion()) << "." << GET_VERSION_MINOR(packet.protoversion()) @@ -987,6 +994,31 @@ void Peer::recvGetLedger(newcoin::TMGetLedger& packet) Serializer nData(128); ledger->addRaw(nData); reply.add_nodes()->set_nodedata(nData.getDataPtr(), nData.getLength()); + + if (packet.nodeids().size() != 0) + { + Log(lsINFO) << "Ledger root w/map roots request"; + SHAMap::pointer map = ledger->peekAccountStateMap(); + if (map) + { + Serializer rootNode(768); + if (map->getRootNode(rootNode, STN_ARF_WIRE)) + { + reply.add_nodes()->set_nodedata(rootNode.getDataPtr(), rootNode.getLength()); + if (ledger->getTransHash().isNonZero()) + { + map = ledger->peekTransactionMap(); + if (map) + { + rootNode.resize(0); + if (map->getRootNode(rootNode, STN_ARF_WIRE)) + reply.add_nodes()->set_nodedata(rootNode.getDataPtr(), rootNode.getLength()); + } + } + } + } + } + PackedMessage::pointer oPacket = boost::make_shared(reply, newcoin::mtLEDGER_DATA); sendPacket(oPacket); return; diff --git a/src/SHAMap.h b/src/SHAMap.h index f0083dd277..08685a6b39 100644 --- a/src/SHAMap.h +++ b/src/SHAMap.h @@ -323,6 +323,7 @@ public: SHAMapSyncFilter* filter); bool getNodeFat(const SHAMapNode& node, std::vector& nodeIDs, std::list >& rawNode, bool fatLeaves); + bool getRootNode(Serializer& s, int format); bool addRootNode(const uint256& hash, const std::vector& rootNode, int format); bool addRootNode(const std::vector& rootNode, int format); bool addKnownNode(const SHAMapNode& nodeID, const std::vector& rawNode, diff --git a/src/SHAMapSync.cpp b/src/SHAMapSync.cpp index 548008a68b..763dd15147 100644 --- a/src/SHAMapSync.cpp +++ b/src/SHAMapSync.cpp @@ -122,6 +122,13 @@ bool SHAMap::getNodeFat(const SHAMapNode& wanted, std::vector& nodeI return true; } +bool SHAMap::getRootNode(Serializer& s, int format) +{ + boost::recursive_mutex::scoped_lock sl(mLock); + root->addRaw(s, format); + return true; +} + bool SHAMap::addRootNode(const std::vector& rootNode, int format) { boost::recursive_mutex::scoped_lock sl(mLock);