From 19b518a0af716317046ef3ff02834e1103cac16e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 12 Oct 2012 17:01:37 -0700 Subject: [PATCH] Quick fix for the bug that was causing ledgers to diverge. LES used an unordered map, causing the traverse of modified ledger nodes to be in random order. This meant different nodes would thread transactions differently, causing ledger divergence. This change switches the LES code to use a standard map. This adds more overhead to LES search functions (because ordered map operations like search and insert are more expensive than unordered map opreations, so it may be worth a separate ordering step just for calcRawMeta instead. --- src/LedgerConsensus.cpp | 2 +- src/LedgerEntrySet.cpp | 24 +++++++++++++----------- src/LedgerEntrySet.h | 14 +++++++------- src/TransactionEngine.cpp | 2 +- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 624e8febf..e8e9e0685 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -18,7 +18,7 @@ #define TRUST_NETWORK -// #define LC_DEBUG +#define LC_DEBUG typedef std::pair u160_prop_pair; typedef std::pair u256_lct_pair; diff --git a/src/LedgerEntrySet.cpp b/src/LedgerEntrySet.cpp index d59b920d0..b1b4ed750 100644 --- a/src/LedgerEntrySet.cpp +++ b/src/LedgerEntrySet.cpp @@ -51,7 +51,7 @@ void LedgerEntrySet::swapWith(LedgerEntrySet& e) // This is basically: copy-on-read. SLE::pointer LedgerEntrySet::getEntry(const uint256& index, LedgerEntryAction& action) { - boost::unordered_map::iterator it = mEntries.find(index); + std::map::iterator it = mEntries.find(index); if (it == mEntries.end()) { action = taaNONE; @@ -98,7 +98,7 @@ SLE::pointer LedgerEntrySet::entryCache(LedgerEntryType letType, const uint256& LedgerEntryAction LedgerEntrySet::hasEntry(const uint256& index) const { - boost::unordered_map::const_iterator it = mEntries.find(index); + std::map::const_iterator it = mEntries.find(index); if (it == mEntries.end()) return taaNONE; return it->second.mAction; @@ -106,7 +106,7 @@ LedgerEntryAction LedgerEntrySet::hasEntry(const uint256& index) const void LedgerEntrySet::entryCache(SLE::ref sle) { - boost::unordered_map::iterator it = mEntries.find(sle->getIndex()); + std::map::iterator it = mEntries.find(sle->getIndex()); if (it == mEntries.end()) { mEntries.insert(std::make_pair(sle->getIndex(), LedgerEntrySetEntry(sle, taaCACHED, mSeq))); @@ -127,7 +127,7 @@ void LedgerEntrySet::entryCache(SLE::ref sle) void LedgerEntrySet::entryCreate(SLE::ref sle) { - boost::unordered_map::iterator it = mEntries.find(sle->getIndex()); + std::map::iterator it = mEntries.find(sle->getIndex()); if (it == mEntries.end()) { mEntries.insert(std::make_pair(sle->getIndex(), LedgerEntrySetEntry(sle, taaCREATE, mSeq))); @@ -157,7 +157,7 @@ void LedgerEntrySet::entryCreate(SLE::ref sle) void LedgerEntrySet::entryModify(SLE::ref sle) { - boost::unordered_map::iterator it = mEntries.find(sle->getIndex()); + std::map::iterator it = mEntries.find(sle->getIndex()); if (it == mEntries.end()) { mEntries.insert(std::make_pair(sle->getIndex(), LedgerEntrySetEntry(sle, taaMODIFY, mSeq))); @@ -192,7 +192,7 @@ void LedgerEntrySet::entryModify(SLE::ref sle) void LedgerEntrySet::entryDelete(SLE::ref sle) { - boost::unordered_map::iterator it = mEntries.find(sle->getIndex()); + std::map::iterator it = mEntries.find(sle->getIndex()); if (it == mEntries.end()) { mEntries.insert(std::make_pair(sle->getIndex(), LedgerEntrySetEntry(sle, taaDELETE, mSeq))); @@ -233,7 +233,7 @@ Json::Value LedgerEntrySet::getJson(int) const Json::Value ret(Json::objectValue); Json::Value nodes(Json::arrayValue); - for (boost::unordered_map::const_iterator it = mEntries.begin(), + for (std::map::const_iterator it = mEntries.begin(), end = mEntries.end(); it != end; ++it) { Json::Value entry(Json::objectValue); @@ -269,7 +269,7 @@ Json::Value LedgerEntrySet::getJson(int) const SLE::pointer LedgerEntrySet::getForMod(const uint256& node, Ledger::ref ledger, boost::unordered_map& newMods) { - boost::unordered_map::iterator it = mEntries.find(node); + std::map::iterator it = mEntries.find(node); if (it != mEntries.end()) { if (it->second.mAction == taaDELETE) @@ -351,7 +351,7 @@ void LedgerEntrySet::calcRawMeta(Serializer& s) // Entries modified only as a result of building the transaction metadata boost::unordered_map newMod; - for (boost::unordered_map::const_iterator it = mEntries.begin(), + for (std::map::const_iterator it = mEntries.begin(), end = mEntries.end(); it != end; ++it) { int nType = TMNEndOfMetadata; @@ -410,8 +410,10 @@ void LedgerEntrySet::calcRawMeta(Serializer& s) if (origNode->getType() == ltRIPPLE_STATE) { - metaNode.addAccount(TMSLowID, NewcoinAddress::createAccountID(origNode->getFieldAmount(sfLowLimit).getIssuer())); - metaNode.addAccount(TMSHighID, NewcoinAddress::createAccountID(origNode->getFieldAmount(sfHighLimit).getIssuer())); + metaNode.addAccount(TMSLowID, + NewcoinAddress::createAccountID(origNode->getFieldAmount(sfLowLimit).getIssuer())); + metaNode.addAccount(TMSHighID, + NewcoinAddress::createAccountID(origNode->getFieldAmount(sfHighLimit).getIssuer())); } } diff --git a/src/LedgerEntrySet.h b/src/LedgerEntrySet.h index be44b0e97..11192c363 100644 --- a/src/LedgerEntrySet.h +++ b/src/LedgerEntrySet.h @@ -32,11 +32,11 @@ class LedgerEntrySet { protected: Ledger::pointer mLedger; - boost::unordered_map mEntries; + std::map mEntries; // cannot be unordered! TransactionMetaSet mSet; int mSeq; - LedgerEntrySet(Ledger::ref ledger, const boost::unordered_map &e, + LedgerEntrySet(Ledger::ref ledger, const std::map &e, const TransactionMetaSet& s, int m) : mLedger(ledger), mEntries(e), mSet(s), mSeq(m) { ; } SLE::pointer getForMod(const uint256& node, Ledger::ref ledger, @@ -123,11 +123,11 @@ public: void calcRawMeta(Serializer&); // iterator functions - bool isEmpty() const { return mEntries.empty(); } - boost::unordered_map::const_iterator begin() const { return mEntries.begin(); } - boost::unordered_map::const_iterator end() const { return mEntries.end(); } - boost::unordered_map::iterator begin() { return mEntries.begin(); } - boost::unordered_map::iterator end() { return mEntries.end(); } + bool isEmpty() const { return mEntries.empty(); } + std::map::const_iterator begin() const { return mEntries.begin(); } + std::map::const_iterator end() const { return mEntries.end(); } + std::map::iterator begin() { return mEntries.begin(); } + std::map::iterator end() { return mEntries.end(); } static bool intersect(const LedgerEntrySet& lesLeft, const LedgerEntrySet& lesRight); }; diff --git a/src/TransactionEngine.cpp b/src/TransactionEngine.cpp index 38dbeb561..36712c6a3 100644 --- a/src/TransactionEngine.cpp +++ b/src/TransactionEngine.cpp @@ -18,7 +18,7 @@ SETUP_LOG(); void TransactionEngine::txnWrite() { // Write back the account states - for (boost::unordered_map::iterator it = mNodes.begin(), end = mNodes.end(); + for (std::map::iterator it = mNodes.begin(), end = mNodes.end(); it != end; ++it) { const SLE::pointer& sleEntry = it->second.mEntry;