From 7744f7c11111956860ddc331d952c93134dbd338 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 16 Sep 2012 20:58:38 -0700 Subject: [PATCH 1/3] Cleanups and improved comments. Fix a race condition if a peer ledger comes in before we take our initial position. --- src/LedgerConsensus.cpp | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index aff9f7bdaf..48b26bdc12 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -359,6 +359,13 @@ void LedgerConsensus::takeInitialPosition(Ledger& initialLedger) uint256 txSet = initialSet->getHash(); Log(lsINFO) << "initial position " << txSet; + if (mValidating) + mOurPosition = boost::make_shared + (mValSeed, initialLedger.getParentHash(), txSet, mCloseTime); + else + mOurPosition = boost::make_shared(initialLedger.getParentHash(), txSet, mCloseTime); + mapComplete(txSet, initialSet, false); + // if any peers have taken a contrary position, process disputes boost::unordered_set found; BOOST_FOREACH(u160_prop_pair& it, mPeerPositions) @@ -372,12 +379,6 @@ void LedgerConsensus::takeInitialPosition(Ledger& initialLedger) } } - if (mValidating) - mOurPosition = boost::make_shared - (mValSeed, initialLedger.getParentHash(), txSet, mCloseTime); - else - mOurPosition = boost::make_shared(initialLedger.getParentHash(), txSet, mCloseTime); - mapComplete(txSet, initialSet, false); if (mProposing) propose(); } @@ -389,16 +390,17 @@ void LedgerConsensus::createDisputes(SHAMap::ref m1, SHAMap::ref m2) for (SHAMap::SHAMapDiff::iterator pos = differences.begin(), end = differences.end(); pos != end; ++pos) { // create disputed transactions (from the ledger that has them) if (pos->second.first) - { + { // transaction is in first map assert(!pos->second.second); addDisputedTransaction(pos->first, pos->second.first->peekData()); } else if (pos->second.second) - { + { // transaction is in second map assert(!pos->second.first); addDisputedTransaction(pos->first, pos->second.second->peekData()); } - else assert(false); + else // No other disagreement over a transaction should be possible + assert(false); } } @@ -417,7 +419,10 @@ void LedgerConsensus::mapComplete(const uint256& hash, SHAMap::ref map, bool acq assert(hash == map->getHash()); if (mAcquired.find(hash) != mAcquired.end()) + { + mAcquiring.erase(hash); return; // we already have this map + } if (mOurPosition && (!mOurPosition->isBowOut()) && (hash != mOurPosition->getCurrentHash())) { // this could create disputed transactions @@ -799,19 +804,20 @@ void LedgerConsensus::addDisputedTransaction(const uint256& txID, const std::vec { Log(lsTRACE) << "Transaction " << txID << " is disputed"; boost::unordered_map::iterator it = mDisputes.find(txID); - if (it != mDisputes.end()) return; + if (it != mDisputes.end()) + return; - bool ourPosition = false; + bool ourVote = false; if (mOurPosition) { boost::unordered_map::iterator mit = mAcquired.find(mOurPosition->getCurrentHash()); if (mit != mAcquired.end()) - ourPosition = mit->second->hasItem(txID); + ourVote = mit->second->hasItem(txID); else assert(false); // We don't have our own position? } - LCTransaction::pointer txn = boost::make_shared(txID, tx, ourPosition); + LCTransaction::pointer txn = boost::make_shared(txID, tx, ourVote); mDisputes[txID] = txn; BOOST_FOREACH(u160_prop_pair& pit, mPeerPositions) From 89518e23cc8cfd00c8fbfd140f5f4cec7d420148 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Mon, 17 Sep 2012 00:38:47 -0700 Subject: [PATCH 2/3] Fix two more race conditions involving us taking our position late. Remove an incorrect comment. --- src/LedgerConsensus.cpp | 9 +++++++-- src/LedgerConsensus.h | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 48b26bdc12..0745329de7 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -358,13 +358,18 @@ void LedgerConsensus::takeInitialPosition(Ledger& initialLedger) SHAMap::pointer initialSet = initialLedger.peekTransactionMap()->snapShot(false); uint256 txSet = initialSet->getHash(); Log(lsINFO) << "initial position " << txSet; + mapComplete(txSet, initialSet, false); if (mValidating) mOurPosition = boost::make_shared (mValSeed, initialLedger.getParentHash(), txSet, mCloseTime); else mOurPosition = boost::make_shared(initialLedger.getParentHash(), txSet, mCloseTime); - mapComplete(txSet, initialSet, false); + + BOOST_FOREACH(u256_lct_pair& it, mDisputes) + { + it.second->setOurVote(initialLedger.hasTransaction(it.first)); + } // if any peers have taken a contrary position, process disputes boost::unordered_set found; @@ -372,7 +377,7 @@ void LedgerConsensus::takeInitialPosition(Ledger& initialLedger) { uint256 set = it.second->getCurrentHash(); if (found.insert(set).second) - { // OPTIMIZEME: Don't process the same set more than once + { boost::unordered_map::iterator iit = mAcquired.find(set); if (iit != mAcquired.end()) createDisputes(initialSet, iit->second); diff --git a/src/LedgerConsensus.h b/src/LedgerConsensus.h index 3833c300b3..44b4db409f 100644 --- a/src/LedgerConsensus.h +++ b/src/LedgerConsensus.h @@ -62,6 +62,7 @@ public: const uint256& getTransactionID() const { return mTransactionID; } bool getOurVote() const { return mOurVote; } Serializer& peekTransaction() { return transaction; } + void setOurVote(bool o) { mOurVote = o; } void setVote(const uint160& peer, bool votesYes); void unVote(const uint160& peer); From 92fbff0efc05fcf0fd466be005c188a3a22ec307 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Mon, 17 Sep 2012 00:54:48 -0700 Subject: [PATCH 3/3] If a new transaction is discovered in the consensus process and we have not relayed it recently, do so. This is not the perfect solution, it would be better to relay it when we accept the new ledger, relaying only if it fits in the consensus ledger. --- src/LedgerConsensus.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 0745329de7..2b2af08a78 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -832,6 +832,16 @@ void LedgerConsensus::addDisputedTransaction(const uint256& txID, const std::vec if (cit != mAcquired.end() && cit->second) txn->setVote(pit.first, cit->second->hasItem(txID)); } + + if (!ourVote && theApp->isNew(txID)) + { + newcoin::TMTransaction msg; + msg.set_rawtransaction(&(tx.front()), tx.size()); + msg.set_status(newcoin::tsNEW); + msg.set_receivetimestamp(theApp->getOPs().getNetworkTimeNC()); + PackedMessage::pointer packet = boost::make_shared(msg, newcoin::mtTRANSACTION); + theApp->getConnectionPool().relayMessage(NULL, packet); + } } bool LedgerConsensus::peerPosition(const LedgerProposal::pointer& newPosition)