From 4930ebb945a204eb81778f1883496b5be731062b Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Mon, 3 Sep 2012 20:13:57 -0700 Subject: [PATCH] Simplify the way we handle validations. Include a signing time instead of a closing time. Keep only the validation with the most recent signing time. Sign using network time. This eliminates the ValidationPair nightmare and makes the logic must easier to understand, increasing confidence that it does what it's supposed to do. --- src/DBInit.cpp | 2 +- src/LedgerConsensus.cpp | 2 +- src/NetworkOPs.cpp | 11 +++- src/NetworkOPs.h | 2 + src/SerializedObject.h | 1 + src/SerializedValidation.cpp | 10 ++-- src/SerializedValidation.h | 4 +- src/ValidationCollection.cpp | 98 ++++++++++++------------------------ src/ValidationCollection.h | 16 ++---- 9 files changed, 57 insertions(+), 89 deletions(-) diff --git a/src/DBInit.cpp b/src/DBInit.cpp index 245b2c5f78..e968ddf3c0 100644 --- a/src/DBInit.cpp +++ b/src/DBInit.cpp @@ -56,7 +56,7 @@ const char *LedgerDBInit[] = { LedgerHash CHARACTER(64), \ NodePubKey CHARACTER(56), \ Flags BIGINT UNSIGNED, \ - CloseTime BIGINT UNSIGNED, \ + SignTime BIGINT UNSIGNED, \ Signature BLOB \ );", "CREATE INDEX ValidationByHash ON \ diff --git a/src/LedgerConsensus.cpp b/src/LedgerConsensus.cpp index 1a5b975ad3..b60c42fb64 100644 --- a/src/LedgerConsensus.cpp +++ b/src/LedgerConsensus.cpp @@ -1005,7 +1005,7 @@ void LedgerConsensus::accept(const SHAMap::pointer& set) if (mValidating) { SerializedValidation::pointer v = boost::make_shared - (newLCLHash, newLCL->getCloseTimeNC(), mValSeed, mProposing); + (newLCLHash, theApp->getOPs().getValidationTimeNC(), mValSeed, mProposing); v->setTrusted(); Log(lsINFO) << "CNF Val " << newLCLHash; theApp->getValidations().addValidation(v); diff --git a/src/NetworkOPs.cpp b/src/NetworkOPs.cpp index aefa4065af..8ec3e71289 100644 --- a/src/NetworkOPs.cpp +++ b/src/NetworkOPs.cpp @@ -25,7 +25,7 @@ NetworkOPs::NetworkOPs(boost::asio::io_service& io_service, LedgerMaster* pLedgerMaster) : mMode(omDISCONNECTED),mNetTimer(io_service), mLedgerMaster(pLedgerMaster), mCloseTimeOffset(0), - mLastCloseProposers(0), mLastCloseConvergeTime(LEDGER_IDLE_INTERVAL) + mLastCloseProposers(0), mLastCloseConvergeTime(LEDGER_IDLE_INTERVAL), mLastValidationTime(0) { } @@ -46,6 +46,15 @@ uint32 NetworkOPs::getCloseTimeNC() return iToSeconds(getNetworkTimePT() + boost::posix_time::seconds(mCloseTimeOffset)); } +uint32 NetworkOPs::getValidationTimeNC() +{ + uint32 vt = getNetworkTimeNC(); + if (vt >= mLastValidationTime) + vt = mLastValidationTime + 1; + mLastValidationTime = vt; + return vt; +} + void NetworkOPs::closeTimeOffset(int offset) { mCloseTimeOffset += offset / 4; diff --git a/src/NetworkOPs.h b/src/NetworkOPs.h index c59ce687a8..25ac145f40 100644 --- a/src/NetworkOPs.h +++ b/src/NetworkOPs.h @@ -64,6 +64,7 @@ protected: int mLastCloseProposers, mLastCloseConvergeTime; uint256 mLastCloseHash; uint32 mLastCloseTime; + uint32 mLastValidationTime; // XXX Split into more locks. boost::interprocess::interprocess_upgradable_mutex mMonitorLock; @@ -89,6 +90,7 @@ public: // network information uint32 getNetworkTimeNC(); uint32 getCloseTimeNC(); + uint32 getValidationTimeNC(); void closeTimeOffset(int); boost::posix_time::ptime getNetworkTimePT(); uint32 getCurrentLedgerID(); diff --git a/src/SerializedObject.h b/src/SerializedObject.h index fde0d7ec3f..d53dd5ed12 100644 --- a/src/SerializedObject.h +++ b/src/SerializedObject.h @@ -92,6 +92,7 @@ enum SOE_Field sfSequence, sfSignature, sfSigningKey, + sfSigningTime, sfSourceTag, sfTakerGets, sfTakerPays, diff --git a/src/SerializedValidation.cpp b/src/SerializedValidation.cpp index a87aa141e5..9ae32c31bb 100644 --- a/src/SerializedValidation.cpp +++ b/src/SerializedValidation.cpp @@ -6,7 +6,7 @@ SOElement SerializedValidation::sValidationFormat[] = { { sfFlags, "Flags", STI_UINT32, SOE_FLAGS, 0 }, { sfLedgerHash, "LedgerHash", STI_HASH256, SOE_REQUIRED, 0 }, - { sfCloseTime, "CloseTime", STI_UINT32, SOE_REQUIRED, 0 }, + { sfSigningTime, "SignTime", STI_UINT32, SOE_REQUIRED, 0 }, { sfSigningKey, "SigningKey", STI_VL, SOE_REQUIRED, 0 }, { sfInvalid, NULL, STI_DONE, SOE_NEVER, -1 }, }; @@ -19,12 +19,12 @@ SerializedValidation::SerializedValidation(SerializerIterator& sit, bool checkSi if (checkSignature && !isValid()) throw std::runtime_error("Invalid validation"); } -SerializedValidation::SerializedValidation(const uint256& ledgerHash, uint32 closeTime, +SerializedValidation::SerializedValidation(const uint256& ledgerHash, uint32 signTime, const NewcoinAddress& naSeed, bool isFull) : STObject(sValidationFormat), mSignature("Signature"), mTrusted(false) { setValueFieldH256(sfLedgerHash, ledgerHash); - setValueFieldU32(sfCloseTime, closeTime); + setValueFieldU32(sfSigningTime, signTime); if (naSeed.isValid()) setValueFieldVL(sfSigningKey, NewcoinAddress::createNodePublic(naSeed).getNodePublic()); if (!isFull) setFlag(sFullFlag); @@ -50,9 +50,9 @@ uint256 SerializedValidation::getLedgerHash() const return getValueFieldH256(sfLedgerHash); } -uint32 SerializedValidation::getCloseTime() const +uint32 SerializedValidation::getSignTime() const { - return getValueFieldU32(sfCloseTime); + return getValueFieldU32(sfSigningTime); } uint32 SerializedValidation::getFlags() const diff --git a/src/SerializedValidation.h b/src/SerializedValidation.h index 5886b6dbba..81e4c1d6e1 100644 --- a/src/SerializedValidation.h +++ b/src/SerializedValidation.h @@ -23,10 +23,10 @@ public: SerializedValidation(SerializerIterator& sit, bool checkSignature = true); SerializedValidation(const Serializer& s, bool checkSignature = true); - SerializedValidation(const uint256& ledgerHash, uint32 closeTime, const NewcoinAddress& naSeed, bool isFull); + SerializedValidation(const uint256& ledgerHash, uint32 signTime, const NewcoinAddress& naSeed, bool isFull); uint256 getLedgerHash() const; - uint32 getCloseTime() const; + uint32 getSignTime() const; uint32 getFlags() const; NewcoinAddress getSignerPublic() const; bool isValid() const; diff --git a/src/ValidationCollection.cpp b/src/ValidationCollection.cpp index cb173f63ef..b6d109777c 100644 --- a/src/ValidationCollection.cpp +++ b/src/ValidationCollection.cpp @@ -17,7 +17,7 @@ bool ValidationCollection::addValidation(const SerializedValidation::pointer& va { val->setTrusted(); uint32 now = theApp->getOPs().getCloseTimeNC(); - uint32 valClose = val->getCloseTime(); + uint32 valClose = val->getSignTime(); if ((now > (valClose - LEDGER_EARLY_INTERVAL)) && (now < (valClose + LEDGER_VAL_INTERVAL))) isCurrent = true; else @@ -34,22 +34,16 @@ bool ValidationCollection::addValidation(const SerializedValidation::pointer& va return false; if (isCurrent) { - boost::unordered_map::iterator it = mCurrentValidations.find(node); - if ((it == mCurrentValidations.end()) || (!it->second.newest) || - (val->getCloseTime() > it->second.newest->getCloseTime())) + boost::unordered_map::iterator it = mCurrentValidations.find(node); + if (it == mCurrentValidations.end()) + mCurrentValidations.insert(std::make_pair(node, val)); + else if (!it->second) + it->second = val; + else if (val->getSignTime() > it->second->getSignTime()) { - if (it != mCurrentValidations.end()) - { - if (it->second.oldest) - { - mStaleValidations.push_back(it->second.oldest); - condWrite(); - } - it->second.oldest = it->second.newest; - it->second.newest = val; - } - else - mCurrentValidations.insert(std::make_pair(node, ValidationPair(val))); + mStaleValidations.push_back(it->second); + it->second = val; + condWrite(); } } } @@ -76,7 +70,7 @@ void ValidationCollection::getValidationCount(const uint256& ledger, bool curren trusted = untrusted = 0; boost::mutex::scoped_lock sl(mValidationLock); boost::unordered_map::iterator it = mValidations.find(ledger); - uint32 now = theApp->getOPs().getCloseTimeNC(); + uint32 now = theApp->getOPs().getNetworkTimeNC(); if (it != mValidations.end()) { for (ValidationSet::iterator vit = it->second.begin(), end = it->second.end(); vit != end; ++vit) @@ -84,7 +78,7 @@ void ValidationCollection::getValidationCount(const uint256& ledger, bool curren bool isTrusted = vit->second->isTrusted(); if (isTrusted && currentOnly) { - uint32 closeTime = vit->second->getCloseTime(); + uint32 closeTime = vit->second->getSignTime(); if ((now < (closeTime - LEDGER_EARLY_INTERVAL)) || (now > (closeTime + LEDGER_VAL_INTERVAL))) isTrusted = false; else @@ -125,10 +119,10 @@ int ValidationCollection::getCurrentValidationCount(uint32 afterTime) { int count = 0; boost::mutex::scoped_lock sl(mValidationLock); - for (boost::unordered_map::iterator it = mCurrentValidations.begin(), + for (boost::unordered_map::iterator it = mCurrentValidations.begin(), end = mCurrentValidations.end(); it != end; ++it) { - if (it->second.newest->isTrusted() && (it->second.newest->getCloseTime() > afterTime)) + if (it->second->isTrusted() && (it->second->getSignTime() > afterTime)) ++count; } return count; @@ -136,54 +130,26 @@ int ValidationCollection::getCurrentValidationCount(uint32 afterTime) boost::unordered_map ValidationCollection::getCurrentValidations() { - uint32 now = theApp->getOPs().getCloseTimeNC(); + uint32 cutoff = theApp->getOPs().getNetworkTimeNC() - LEDGER_VAL_INTERVAL; boost::unordered_map ret; { boost::mutex::scoped_lock sl(mValidationLock); - boost::unordered_map::iterator it = mCurrentValidations.begin(); + boost::unordered_map::iterator it = mCurrentValidations.begin(); while (it != mCurrentValidations.end()) { - ValidationPair& pair = it->second; - - if (pair.oldest && (now > (pair.oldest->getCloseTime() + LEDGER_VAL_INTERVAL))) - { -#ifdef VC_DEBUG - Log(lsINFO) << "VC: " << it->first << " removeOldestStale"; -#endif - mStaleValidations.push_back(pair.oldest); - pair.oldest = SerializedValidation::pointer(); - condWrite(); - } - if (pair.newest && (now > (pair.newest->getCloseTime() + LEDGER_VAL_INTERVAL))) - { -#ifdef VC_DEBUG - Log(lsINFO) << "VC: " << it->first << " removeNewestStale"; -#endif - mStaleValidations.push_back(pair.newest); - pair.newest = SerializedValidation::pointer(); - condWrite(); - } - if (!pair.newest && !pair.oldest) + if (!it->second) // contains no record it = mCurrentValidations.erase(it); + else if (it->second->getSignTime() < cutoff) + { // contains a stale record + mStaleValidations.push_back(it->second); + it->second = SerializedValidation::pointer(); + condWrite(); + it = mCurrentValidations.erase(it); + } else - { - if (pair.oldest) - { -#ifdef VC_DEBUG - Log(lsTRACE) << "VC: OLD " << pair.oldest->getLedgerHash() << " " << - boost::lexical_cast(pair.oldest->getCloseTime()); -#endif - ++ret[pair.oldest->getLedgerHash()]; - } - if (pair.newest) - { -#ifdef VC_DEBUG - Log(lsTRACE) << "VC: NEW " << pair.newest->getLedgerHash() << " " << - boost::lexical_cast(pair.newest->getCloseTime()); -#endif - ++ret[pair.newest->getLedgerHash()]; - } + { // contains a live record + ++ret[it->second->getLedgerHash()]; ++it; } } @@ -213,14 +179,12 @@ void ValidationCollection::addDeadLedger(const uint256& ledger) void ValidationCollection::flush() { boost::mutex::scoped_lock sl(mValidationLock); - boost::unordered_map::iterator it = mCurrentValidations.begin(); + boost::unordered_map::iterator it = mCurrentValidations.begin(); bool anyNew = false; while (it != mCurrentValidations.end()) { - if (it->second.oldest) - mStaleValidations.push_back(it->second.oldest); - if (it->second.newest) - mStaleValidations.push_back(it->second.newest); + if (it->second) + mStaleValidations.push_back(it->second); ++it; anyNew = true; } @@ -247,7 +211,7 @@ void ValidationCollection::condWrite() void ValidationCollection::doWrite() { static boost::format insVal("INSERT INTO LedgerValidations " - "(LedgerHash,NodePubKey,Flags,CloseTime,Signature) VALUES ('%s','%s','%u','%u',%s);"); + "(LedgerHash,NodePubKey,Flags,SignTime,Signature) VALUES ('%s','%s','%u','%u',%s);"); boost::mutex::scoped_lock sl(mValidationLock); assert(mWriting); @@ -265,7 +229,7 @@ void ValidationCollection::doWrite() BOOST_FOREACH(const SerializedValidation::pointer& it, vector) db->executeSQL(boost::str(insVal % it->getLedgerHash().GetHex() - % it->getSignerPublic().humanNodePublic() % it->getFlags() % it->getCloseTime() + % it->getSignerPublic().humanNodePublic() % it->getFlags() % it->getSignTime() % db->escape(strCopy(it->getSignature())))); db->executeSQL("END TRANSACTION;"); } diff --git a/src/ValidationCollection.h b/src/ValidationCollection.h index bdd224a040..4a5912abfe 100644 --- a/src/ValidationCollection.h +++ b/src/ValidationCollection.h @@ -12,23 +12,15 @@ typedef boost::unordered_map ValidationSet; -class ValidationPair -{ -public: - SerializedValidation::pointer oldest, newest; - - ValidationPair(const SerializedValidation::pointer& v) : newest(v) { ; } -}; - class ValidationCollection { protected: boost::mutex mValidationLock; - boost::unordered_map mValidations; - boost::unordered_map mCurrentValidations; - std::vector mStaleValidations; - std::list mDeadLedgers; + boost::unordered_map mValidations; + boost::unordered_map mCurrentValidations; + std::vector mStaleValidations; + std::list mDeadLedgers; bool mWriting;