From 45b07ff9ecba8195ab9549edfb35073a83426b1f Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 18 Nov 2015 14:15:04 -0800 Subject: [PATCH] Consensus ledger switch improvements * Expire validations faster based on when we first saw them. * Never jump to a ledger prior to the latest fully-valid ledger * Drop validations with signing times too far in the future immediately --- src/ripple/app/ledger/LedgerTiming.h | 9 ++- .../app/ledger/impl/LedgerConsensusImp.cpp | 5 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 2 +- src/ripple/app/main/Application.cpp | 1 - src/ripple/app/misc/NetworkOPs.cpp | 3 +- src/ripple/app/misc/Validations.cpp | 69 ++++++++++--------- src/ripple/app/misc/Validations.h | 6 +- src/ripple/core/Config.h | 2 - src/ripple/core/impl/Config.cpp | 3 - src/ripple/overlay/impl/PeerImp.cpp | 10 ++- src/ripple/protocol/STValidation.h | 10 ++- src/ripple/protocol/impl/STValidation.cpp | 8 ++- 12 files changed, 77 insertions(+), 51 deletions(-) diff --git a/src/ripple/app/ledger/LedgerTiming.h b/src/ripple/app/ledger/LedgerTiming.h index 8bbbbb7db0..d8d42effc4 100644 --- a/src/ripple/app/ledger/LedgerTiming.h +++ b/src/ripple/app/ledger/LedgerTiming.h @@ -72,11 +72,16 @@ const int LEDGER_IDLE_INTERVAL = 15; // The number of seconds a validation remains current after its ledger's close // time. This is a safety to protect against very old validations and the time // it takes to adjust the close time accuracy window -const int LEDGER_VAL_INTERVAL = 300; +const int VALIDATION_VALID_WALL = 300; + +// The number of seconds a validation remains current after the time we first +// saw it. This provides faster recovery in very rare cases where the number +// of validations produced by the network is lower than normal +const int VALIDATION_VALID_LOCAL = 180; // The number of seconds before a close time that we consider a validation // acceptable. This protects against extreme clock errors -const int LEDGER_EARLY_INTERVAL = 180; +const int VALIDATION_VALID_EARLY = 180; // The number of milliseconds we wait minimum to ensure participation const int LEDGER_MIN_CONSENSUS = 2000; diff --git a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp index d206a4aff8..159b168db6 100644 --- a/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp +++ b/src/ripple/app/ledger/impl/LedgerConsensusImp.cpp @@ -550,8 +550,9 @@ void LedgerConsensusImp::checkLCL () // Get validators that are on our ledger, or "close" to being on // our ledger. hash_map vals = - app_.getValidations ().getCurrentValidations - (favoredLedger, priorLedger); + app_.getValidations ().getCurrentValidations( + favoredLedger, priorLedger, + ledgerMaster_.getValidLedgerIndex ()); for (auto& it : vals) { diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 5b2a713829..267ede2b5f 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -198,7 +198,7 @@ public: if (mStrictValCount) { // If we're only using validation count, then we can't - // reject a ledger even if it's ioncompatible + // reject a ledger even if it's incompatible return true; } diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 9e43e42ffe..d0c937bf37 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1007,7 +1007,6 @@ void ApplicationImp::setup() if (!config_->RUN_STANDALONE) getUNL ().nodeBootstrap (); - mValidations->tune (config_->getSize (siValidationsSize), config_->getSize (siValidationsAge)); m_nodeStore->tune (config_->getSize (siNodeCacheSize), config_->getSize (siNodeCacheAge)); m_ledgerMaster->tune (config_->getSize (siLedgerSize), config_->getSize (siLedgerAge)); family().treecache().setTargetSize (config_->getSize (siTreeCacheSize)); diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index eb999dbeaa..3a9ef72984 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1218,7 +1218,8 @@ bool NetworkOPsImp::checkLastClosedLedger ( hash_map ledgers; { auto current = app_.getValidations ().getCurrentValidations ( - closedLedger, prevClosedLedger); + closedLedger, prevClosedLedger, + m_ledgerMaster.getValidLedgerIndex()); for (auto& it: current) { diff --git a/src/ripple/app/misc/Validations.cpp b/src/ripple/app/misc/Validations.cpp index e9f62805af..b9c6f06056 100644 --- a/src/ripple/app/misc/Validations.cpp +++ b/src/ripple/app/misc/Validations.cpp @@ -75,7 +75,7 @@ private: public: ValidationsImp (Application& app) : app_ (app) - , mValidations ("Validations", 128, 600, stopwatch(), + , mValidations ("Validations", 4096, 600, stopwatch(), app.journal("TaggedCache")) , mWriting (false) , j_ (app.journal ("Validations")) @@ -87,19 +87,11 @@ private: bool addValidation (STValidation::ref val, std::string const& source) override { RippleAddress signer = val->getSignerPublic (); - bool isCurrent = false; + bool isCurrent = current (val); - if (!val->isTrusted() && app_.getUNL().nodeInUNL (signer)) + if (! val->isTrusted() && app_.getUNL().nodeInUNL (signer)) val->setTrusted(); - auto const now = app_.timeKeeper().closeTime().time_since_epoch().count(); - std::uint32_t valClose = val->getSignTime(); - - if ((now > (valClose - LEDGER_EARLY_INTERVAL)) && (now < (valClose + LEDGER_VAL_INTERVAL))) - isCurrent = true; - else - JLOG (j_.warning) << "Received stale validation now=" << now << ", close=" << valClose; - if (!val->isTrusted ()) { JLOG (j_.debug) << "Node " << signer.humanNodePublic () << " not in UNL st=" << val->getSignTime () << @@ -144,7 +136,8 @@ private: } JLOG (j_.debug) << "Val for " << hash << " from " << signer.humanNodePublic () - << " added " << (val->isTrusted () ? "trusted/" : "UNtrusted/") << (isCurrent ? "current" : "stale"); + << " added " << (val->isTrusted () ? "trusted/" : "UNtrusted/") + << (isCurrent ? "current" : "stale"); if (val->isTrusted () && isCurrent) { @@ -174,6 +167,25 @@ private: return ValidationSet (); } + bool current (STValidation::ref val) override + { + // Because this can be called on untrusted, possibly + // malicious validations, we do our math in a way + // that avoids any chance of overflowing or underflowing + // the signing time. + + auto const now = + app_.timeKeeper().now().time_since_epoch().count(); + + auto const signTime = val->getSignTime(); + + return + (signTime > (now - VALIDATION_VALID_EARLY)) && + (signTime < (now + VALIDATION_VALID_WALL)) && + ((val->getSeenTime() == 0) || + (val->getSeenTime() < (now + VALIDATION_VALID_LOCAL))); + } + void getValidationCount (uint256 const& ledger, bool currentOnly, int& trusted, int& untrusted) override { @@ -183,22 +195,14 @@ private: if (set) { - auto const now = - app_.timeKeeper().now().time_since_epoch().count(); for (auto& it: *set) { bool isTrusted = it.second->isTrusted (); - if (isTrusted && currentOnly) + if (isTrusted && currentOnly && ! current (it.second)) { - std::uint32_t closeTime = it.second->getSignTime (); - - if ((now < (closeTime - LEDGER_EARLY_INTERVAL)) || (now > (closeTime + LEDGER_VAL_INTERVAL))) - isTrusted = false; - else - { - JLOG (j_.trace) << "VC: Untrusted due to time " << ledger; - } + JLOG (j_.trace) << "VC: Untrusted due to time " << ledger; + isTrusted = false; } if (isTrusted) @@ -312,9 +316,6 @@ private: std::list getCurrentTrustedValidations () override { - // VFALCO LEDGER_VAL_INTERVAL should be a NetClock::duration - auto const cutoff = app_.timeKeeper().now().time_since_epoch().count() - LEDGER_VAL_INTERVAL; - std::list ret; ScopedLockType sl (mLock); @@ -324,7 +325,7 @@ private: { if (!it->second) // contains no record it = mCurrentValidations.erase (it); - else if (it->second->getSignTime () < cutoff) + else if (! current (it->second)) { // contains a stale record mStaleValidations.push_back (it->second); @@ -346,9 +347,10 @@ private: } LedgerToValidationCounter getCurrentValidations ( - uint256 currentLedger, uint256 priorLedger) override + uint256 currentLedger, + uint256 priorLedger, + LedgerIndex cutoffBefore) override { - auto const cutoff = app_.timeKeeper().now().time_since_epoch().count() - LEDGER_VAL_INTERVAL; bool valCurrentLedger = currentLedger.isNonZero (); bool valPriorLedger = priorLedger.isNonZero (); @@ -361,7 +363,7 @@ private: { if (!it->second) // contains no record it = mCurrentValidations.erase (it); - else if (it->second->getSignTime () < cutoff) + else if (! current (it->second)) { // contains a stale record mStaleValidations.push_back (it->second); @@ -369,7 +371,8 @@ private: condWrite (); it = mCurrentValidations.erase (it); } - else + else if (! it->second->isFieldPresent (sfLedgerSequence) || + (it->second->getFieldU32 (sfLedgerSequence) >= cutoffBefore)) { // contains a live record bool countPreferred = valCurrentLedger && (it->second->getLedgerHash () == currentLedger); @@ -391,6 +394,10 @@ private: ++it; } + else + { + ++it; + } } return ret; diff --git a/src/ripple/app/misc/Validations.h b/src/ripple/app/misc/Validations.h index c9987dcaa9..1f25a7d63d 100644 --- a/src/ripple/app/misc/Validations.h +++ b/src/ripple/app/misc/Validations.h @@ -21,6 +21,7 @@ #define RIPPLE_APP_MISC_VALIDATIONS_H_INCLUDED #include +#include #include #include #include @@ -43,6 +44,8 @@ public: virtual bool addValidation (STValidation::ref, std::string const& source) = 0; + virtual bool current (STValidation::ref) = 0; + virtual ValidationSet getValidations (uint256 const& ledger) = 0; virtual void getValidationCount ( @@ -63,7 +66,8 @@ public: // VFALCO TODO make a type alias for this ugly return value! virtual LedgerToValidationCounter getCurrentValidations ( - uint256 currentLedger, uint256 previousLedger) = 0; + uint256 currentLedger, uint256 previousLedger, + LedgerIndex cutoffBefore) = 0; /** Return the times of all validations for a particular ledger hash. */ virtual std::vector getValidationTimes ( diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index 64392fed36..ab4d6d385d 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -63,8 +63,6 @@ class Rules; enum SizedItemName { siSweepInterval, - siValidationsSize, - siValidationsAge, siNodeCacheSize, siNodeCacheAge, siTreeCacheSize, diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index 8bcf79d14d..a5819e9c68 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -513,9 +513,6 @@ int Config::getSize (SizedItemName item) const { siLedgerFetch, { 2, 2, 3, 3, 3 } }, - { siValidationsSize, { 256, 256, 512, 1024, 1024 } }, - { siValidationsAge, { 500, 500, 500, 500, 500 } }, - { siNodeCacheSize, { 16384, 32768, 131072, 262144, 524288 } }, { siNodeCacheAge, { 60, 90, 120, 900, 1800 } }, diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 0776713aed..5eb72214ce 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -23,11 +23,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -1550,7 +1552,8 @@ void PeerImp::onMessage (std::shared_ptr const& m) { error_code ec; - auto const closeTime = app_.timeKeeper().closeTime().time_since_epoch().count(); + auto const closeTime = + app_.timeKeeper().closeTime().time_since_epoch().count(); if (m->has_hops() && ! slot_->cluster()) m->set_hops(m->hops() + 1); @@ -1569,11 +1572,12 @@ PeerImp::onMessage (std::shared_ptr const& m) SerialIter sit (makeSlice(m->validation())); val = std::make_shared < STValidation> (std::ref (sit), false); + val->setSeen (closeTime); } - if (closeTime > (120 + val->getFieldU32(sfSigningTime))) + if (! app_.getValidations().current (val)) { - p_journal_.trace << "Validation: Too old"; + p_journal_.trace << "Validation: Not current"; fee_ = Resource::feeUnwantedData; return; } diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index cd4faef62f..1132208fab 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -66,6 +66,7 @@ public: uint256 getLedgerHash () const; std::uint32_t getSignTime () const; + std::uint32_t getSeenTime () const; std::uint32_t getFlags () const; RippleAddress getSignerPublic () const; NodeID getNodeID () const @@ -81,10 +82,14 @@ public: uint256 getSigningHash () const; bool isValid (uint256 const& ) const; - void setTrusted () + void setTrusted () { mTrusted = true; } + void setSeen (std::uint32_t s) + { + mSeen = s; + } Blob getSigned () const; Blob getSignature () const; @@ -112,7 +117,8 @@ private: uint256 mPreviousHash; NodeID mNodeID; - bool mTrusted; + bool mTrusted = false; + std::uint32_t mSeen = 0; }; } // ripple diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index 315b2827b9..a8a2907c0b 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -28,7 +28,6 @@ namespace ripple { STValidation::STValidation (SerialIter& sit, bool checkSignature) : STObject (getFormat (), sit, sfValidation) - , mTrusted (false) { mNodeID = RippleAddress::createNodePublic (getFieldVL (sfSigningPubKey)).getNodeID (); assert (mNodeID.isNonZero ()); @@ -44,7 +43,7 @@ STValidation::STValidation ( uint256 const& ledgerHash, std::uint32_t signTime, RippleAddress const& raPub, bool isFull) : STObject (getFormat (), sfValidation) - , mTrusted (false) + , mSeen (signTime) { // Does not sign setFieldH256 (sfLedgerHash, ledgerHash); @@ -85,6 +84,11 @@ std::uint32_t STValidation::getSignTime () const return getFieldU32 (sfSigningTime); } +std::uint32_t STValidation::getSeenTime () const +{ + return mSeen; +} + std::uint32_t STValidation::getFlags () const { return getFieldU32 (sfFlags);