From 5b68834e3b97e345f13bcb94acbc8b706909924b Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:13:45 -0700 Subject: [PATCH 01/11] Set correct hash in fetch pack. --- src/cpp/ripple/NetworkOPs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index 70404c310..c218afbc0 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -2029,7 +2029,7 @@ void NetworkOPs::makeFetchPack(Job&, boost::weak_ptr wPeer, reply.set_query(false); if (request->has_seq()) reply.set_seq(request->seq()); - reply.set_ledgerhash(reply.ledgerhash()); + reply.set_ledgerhash(request->ledgerhash()); reply.set_type(ripple::TMGetObjectByHash::otFETCH_PACK); do From d22c11be1c162c700a75a7e71fe69581e3dc1778 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:40:55 -0700 Subject: [PATCH 02/11] Return references. --- src/cpp/ripple/SerializedObject.cpp | 14 ++++++++------ src/cpp/ripple/SerializedObject.h | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/cpp/ripple/SerializedObject.cpp b/src/cpp/ripple/SerializedObject.cpp index 2179d5d64..4b644847c 100644 --- a/src/cpp/ripple/SerializedObject.cpp +++ b/src/cpp/ripple/SerializedObject.cpp @@ -639,38 +639,40 @@ std::vector STObject::getFieldVL(SField::ref field) const return cf->getValue(); } -static const STAmount defaultAmount; const STAmount& STObject::getFieldAmount(SField::ref field) const { + static STAmount empty; const SerializedType* rf = peekAtPField(field); if (!rf) throw std::runtime_error("Field not found"); SerializedTypeID id = rf->getSType(); if (id == STI_NOTPRESENT) - return defaultAmount; // optional field not present + return empty; // optional field not present const STAmount *cf = dynamic_cast(rf); if (!cf) throw std::runtime_error("Wrong field type"); return *cf; } -STPathSet STObject::getFieldPathSet(SField::ref field) const +const STPathSet& STObject::getFieldPathSet(SField::ref field) const { + static STPathSet empty; const SerializedType* rf = peekAtPField(field); if (!rf) throw std::runtime_error("Field not found"); SerializedTypeID id = rf->getSType(); - if (id == STI_NOTPRESENT) return STPathSet(); // optional field not present + if (id == STI_NOTPRESENT) return empty; // optional field not present const STPathSet *cf = dynamic_cast(rf); if (!cf) throw std::runtime_error("Wrong field type"); return *cf; } -STVector256 STObject::getFieldV256(SField::ref field) const +const STVector256& STObject::getFieldV256(SField::ref field) const { + static STVector256 empty; const SerializedType* rf = peekAtPField(field); if (!rf) throw std::runtime_error("Field not found"); SerializedTypeID id = rf->getSType(); - if (id == STI_NOTPRESENT) return STVector256(); // optional field not present + if (id == STI_NOTPRESENT) return empty; // optional field not present const STVector256 *cf = dynamic_cast(rf); if (!cf) throw std::runtime_error("Wrong field type"); return *cf; diff --git a/src/cpp/ripple/SerializedObject.h b/src/cpp/ripple/SerializedObject.h index 97ab764f3..c1a7fa35e 100644 --- a/src/cpp/ripple/SerializedObject.h +++ b/src/cpp/ripple/SerializedObject.h @@ -131,8 +131,8 @@ public: uint160 getFieldAccount160(SField::ref field) const; std::vector getFieldVL(SField::ref field) const; const STAmount& getFieldAmount(SField::ref field) const; - STPathSet getFieldPathSet(SField::ref field) const; - STVector256 getFieldV256(SField::ref field) const; + const STPathSet& getFieldPathSet(SField::ref field) const; + const STVector256& getFieldV256(SField::ref field) const; void setFieldU8(SField::ref field, unsigned char); void setFieldU16(SField::ref field, uint16); From 141c2dce11b986b01525071e2f5bee0c6beecdf3 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:44:37 -0700 Subject: [PATCH 03/11] Avoid needless allocate/copy/free --- src/cpp/ripple/Offer.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cpp/ripple/Offer.h b/src/cpp/ripple/Offer.h index cdb9dd8e8..82bc58646 100644 --- a/src/cpp/ripple/Offer.h +++ b/src/cpp/ripple/Offer.h @@ -15,9 +15,9 @@ public: AccountItem::pointer makeItem(const uint160&, SerializedLedgerEntry::ref ledgerEntry); LedgerEntryType getType(){ return(ltOFFER); } - STAmount getTakerPays(){ return(mTakerPays); } - STAmount getTakerGets(){ return(mTakerGets); } - RippleAddress getAccount(){ return(mAccount); } + const STAmount& getTakerPays(){ return(mTakerPays); } + const STAmount& getTakerGets(){ return(mTakerGets); } + const RippleAddress& getAccount(){ return(mAccount); } int getSeq(){ return(mSeq); } Json::Value getJson(int); From c377562e7f7f6ff7e54f5573f994fc64b1e981b8 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:44:49 -0700 Subject: [PATCH 04/11] Cleanups. --- src/cpp/ripple/RPCHandler.cpp | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/cpp/ripple/RPCHandler.cpp b/src/cpp/ripple/RPCHandler.cpp index de444d4da..93482b0d6 100644 --- a/src/cpp/ripple/RPCHandler.cpp +++ b/src/cpp/ripple/RPCHandler.cpp @@ -1146,9 +1146,6 @@ Json::Value RPCHandler::doAccountOffers(Json::Value jvRequest, int& cost, Scoped AccountState::pointer as = mNetOps->getAccountState(lpLedger, raAccount); - if (lpLedger->isImmutable()) - MasterLockHolder.unlock(); - if (as) { Json::Value& jsonLines = (jvResult["offers"] = Json::arrayValue); @@ -1158,15 +1155,10 @@ Json::Value RPCHandler::doAccountOffers(Json::Value jvRequest, int& cost, Scoped { Offer* offer=(Offer*)item.get(); - STAmount takerPays = offer->getTakerPays(); - STAmount takerGets = offer->getTakerGets(); - //RippleAddress account = offer->getAccount(); - Json::Value& obj = jsonLines.append(Json::objectValue); - //obj["account"] = account.humanAccountID(); - takerPays.setJson(obj["taker_pays"]); - takerGets.setJson(obj["taker_gets"]); + offer->getTakerPays().setJson(obj["taker_pays"]); + offer->getTakerGets().setJson(obj["taker_gets"]); obj["seq"] = offer->getSeq(); } From 249d5575cd836ddaa646e677c92d3fbba4579443 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:46:22 -0700 Subject: [PATCH 05/11] Avoid an allocate/copy/free --- src/cpp/ripple/AccountItems.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/cpp/ripple/AccountItems.cpp b/src/cpp/ripple/AccountItems.cpp index cbad2cb2e..7bb67977d 100644 --- a/src/cpp/ripple/AccountItems.cpp +++ b/src/cpp/ripple/AccountItems.cpp @@ -29,14 +29,12 @@ void AccountItems::fillItems(const uint160& accountID, Ledger::ref ledger) SLE::pointer ownerDir = ledger->getDirNode(currentIndex); if (!ownerDir) return; - STVector256 svOwnerNodes = ownerDir->getFieldV256(sfIndexes); - - BOOST_FOREACH(uint256& uNode, svOwnerNodes.peekValue()) + BOOST_FOREACH(const uint256& uNode, ownerDir->getFieldV256(sfIndexes).peekValue()) { SLE::pointer sleCur = ledger->getSLEi(uNode); AccountItem::pointer item = mOfType->makeItem(accountID, sleCur); - if(item) + if (item) { mItems.push_back(item); } From c0e046a72154e9f53d50a3cde4c299d368886ede Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:46:39 -0700 Subject: [PATCH 06/11] Don't put a leaf node in a fetch pack just because it moved. --- src/cpp/ripple/SHAMap.h | 3 ++- src/cpp/ripple/SHAMapSync.cpp | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/cpp/ripple/SHAMap.h b/src/cpp/ripple/SHAMap.h index b60c8827c..da58b1c2a 100644 --- a/src/cpp/ripple/SHAMap.h +++ b/src/cpp/ripple/SHAMap.h @@ -377,7 +377,8 @@ protected: SHAMapItem::pointer onlyBelow(SHAMapTreeNode*); void eraseChildren(SHAMapTreeNode::pointer); void dropBelow(SHAMapTreeNode*); - bool hasNode(const SHAMapNode& id, const uint256& hash); + bool hasInnerNode(const SHAMapNode& nodeID, const uint256& hash); + bool hasLeafNode(const uint256& tag, const uint256& hash); bool walkBranch(SHAMapTreeNode* node, SHAMapItem::ref otherMapItem, bool isFirstMap, SHAMapDiff& differences, int& maxCount); diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index eb14df863..e52eee5c9 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -410,7 +410,7 @@ bool SHAMap::deepCompare(SHAMap& other) return true; } -bool SHAMap::hasNode(const SHAMapNode& nodeID, const uint256& nodeHash) +bool SHAMap::hasInnerNode(const SHAMapNode& nodeID, const uint256& nodeHash) { SHAMapTreeNode* node = root.get(); while (node->isInner() && (node->getDepth() < nodeID.getDepth())) @@ -423,6 +423,19 @@ bool SHAMap::hasNode(const SHAMapNode& nodeID, const uint256& nodeHash) return node->getNodeHash() == nodeHash; } +bool SHAMap::hasLeafNode(const uint256& tag, const uint256& nodeHash) +{ + SHAMapTreeNode* node = root.get(); + while (node->isInner()) + { + int branch = node->selectBranch(tag); + if (node->isEmptyBranch(branch)) + return false; + node = getNodePointer(node->getChildNodeID(branch), node->getChildHash(branch)); + } + return node->getNodeHash() == nodeHash; +} + std::list SHAMap::getFetchPack(SHAMap* have, bool includeLeaves, int max) { std::list ret; @@ -446,7 +459,7 @@ std::list SHAMap::getFetchPack(SHAMap* have, bool incl if (root->isLeaf()) { if (includeLeaves && !root->getNodeHash().isZero() && - (!have || !have->hasNode(*root, root->getNodeHash()))) + (!have || !have->hasLeafNode(root->getTag(), root->getNodeHash()))) { Serializer s; root->addRaw(s, snfPREFIX); @@ -486,10 +499,10 @@ std::list SHAMap::getFetchPack(SHAMap* have, bool incl SHAMapTreeNode *next = getNodePointer(childID, childHash); if (next->isInner()) { - if (!have || !have->hasNode(*next, childHash)) + if (!have || !have->hasInnerNode(*next, childHash)) stack.push(next); } - else if (includeLeaves && (!have || !have->hasNode(childID, childHash))) + if (includeLeaves && (!have || !have->hasLeafNode(next->getTag(), childHash))) { Serializer s; node->addRaw(s, snfPREFIX); From c2bea03886dc3327bd9d2109a94de6da7fcbf059 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:50:19 -0700 Subject: [PATCH 07/11] Bug in previous commit. --- src/cpp/ripple/SHAMapSync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index e52eee5c9..639ba1a5d 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -502,7 +502,7 @@ std::list SHAMap::getFetchPack(SHAMap* have, bool incl if (!have || !have->hasInnerNode(*next, childHash)) stack.push(next); } - if (includeLeaves && (!have || !have->hasLeafNode(next->getTag(), childHash))) + if (includeLeaves && (!have || !have->hasLeafNode(node->getTag(), childHash))) { Serializer s; node->addRaw(s, snfPREFIX); From ff6e035e6e846984d196199c844fff1c969dba30 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:52:39 -0700 Subject: [PATCH 08/11] Bugfix. --- src/cpp/ripple/SHAMapSync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index 639ba1a5d..33d58e442 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -502,7 +502,7 @@ std::list SHAMap::getFetchPack(SHAMap* have, bool incl if (!have || !have->hasInnerNode(*next, childHash)) stack.push(next); } - if (includeLeaves && (!have || !have->hasLeafNode(node->getTag(), childHash))) + else if (includeLeaves && (!have || !have->hasLeafNode(node->getTag(), childHash))) { Serializer s; node->addRaw(s, snfPREFIX); From 327f20710c946d591c828bbb568f7affcb1724c6 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 01:56:34 -0700 Subject: [PATCH 09/11] Fix. --- src/cpp/ripple/SHAMapSync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index 33d58e442..8d851b25d 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -502,7 +502,7 @@ std::list SHAMap::getFetchPack(SHAMap* have, bool incl if (!have || !have->hasInnerNode(*next, childHash)) stack.push(next); } - else if (includeLeaves && (!have || !have->hasLeafNode(node->getTag(), childHash))) + else if (includeLeaves && (!have || !have->hasLeafNode(next->getTag(), childHash))) { Serializer s; node->addRaw(s, snfPREFIX); From 1139ac5ab4d6c17ef3177dfb4ce6e01760c7dace Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 02:04:04 -0700 Subject: [PATCH 10/11] Make fetch packs less generous. --- src/cpp/ripple/NetworkOPs.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index c218afbc0..6963cc0c6 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -2054,7 +2054,7 @@ void NetworkOPs::makeFetchPack(Job&, boost::weak_ptr wPeer, newObj.set_ledgerseq(lSeq); } - if (wantLedger->getAccountHash().isNonZero() && (pack.size() < 768)) + if (wantLedger->getAccountHash().isNonZero() && (pack.size() < 512)) { pack = wantLedger->peekTransactionMap()->getFetchPack(NULL, true, 256); BOOST_FOREACH(SHAMap::fetchPackEntry_t& node, pack) @@ -2065,11 +2065,11 @@ void NetworkOPs::makeFetchPack(Job&, boost::weak_ptr wPeer, newObj.set_ledgerseq(lSeq); } } - if (reply.objects().size() >= 512) + if (reply.objects().size() >= 256) break; haveLedger = wantLedger; wantLedger = getLedgerByHash(haveLedger->getParentHash()); - } while (wantLedger); + } while (wantLedger && (upTime() <= (uUptime + 1))); cLog(lsINFO) << "Built fetch pack with " << reply.objects().size() << " nodes"; PackedMessage::pointer msg = boost::make_shared(reply, ripple::mtGET_OBJECTS); From 968618a50c992961707b0ee9a5cb1b6da907702e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 17 May 2013 02:04:12 -0700 Subject: [PATCH 11/11] More verbose logging of invalid node requests. --- src/cpp/ripple/Peer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/Peer.cpp b/src/cpp/ripple/Peer.cpp index 75e384c8d..8f9dbd528 100644 --- a/src/cpp/ripple/Peer.cpp +++ b/src/cpp/ripple/Peer.cpp @@ -1608,7 +1608,7 @@ void Peer::recvGetLedger(ripple::TMGetLedger& packet) SHAMapNode mn(packet.nodeids(i).data(), packet.nodeids(i).size()); if(!mn.isValid()) { - cLog(lsWARNING) << "Request for invalid node"; + cLog(lsWARNING) << "Request for invalid node: " << logMe; punishPeer(LT_InvalidRequest); return; }