diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index c406d8da83..0208592e09 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -83,7 +83,6 @@ RCLConsensus::Adaptor::Adaptor( , nodeID_{validatorKeys.nodeID} , valPublic_{validatorKeys.publicKey} , valSecret_{validatorKeys.secretKey} - , cookie_{validatorKeys.cookie} { } @@ -853,10 +852,6 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, amendments = app_.getAmendmentTable().doValidation (getEnabledAmendments(*ledger.ledger_)); } - boost::optional maybeCookie; - if (ledgerMaster_.getValidatedRules().enabled(featureValidationCookies)) - maybeCookie.emplace(cookie_); - auto v = std::make_shared( ledger.id(), ledger.seq(), @@ -867,8 +862,7 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, nodeID_, proposing /* full if proposed */, fees, - amendments, - maybeCookie); + amendments); diff --git a/src/ripple/app/consensus/RCLConsensus.h b/src/ripple/app/consensus/RCLConsensus.h index 2510b56488..9903b69d5a 100644 --- a/src/ripple/app/consensus/RCLConsensus.h +++ b/src/ripple/app/consensus/RCLConsensus.h @@ -60,7 +60,6 @@ class RCLConsensus NodeID const nodeID_; PublicKey const valPublic_; SecretKey const valSecret_; - std::uint64_t const cookie_; // Ledger we most recently needed to acquire LedgerHash acquiringLedger_; diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index c16d68d388..d882747621 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -320,17 +320,9 @@ handleNewValidation(Application& app, if(j.debug()) dmp(j.debug(), to_string(outcome)); - auto const seq = val->getFieldU32(sfLedgerSequence); - - if (outcome == ValStatus::badCookie && j.error()) - { - dmp(j.error(), - "already received validation for seq " + to_string(seq) + - " with different cookie; are two validators running with " - "the same keys?"); - } if (outcome == ValStatus::badSeq && j.warn()) { + auto const seq = val->getFieldU32(sfLedgerSequence); dmp(j.warn(), "already validated sequence at or past " + to_string(seq)); } diff --git a/src/ripple/app/consensus/RCLValidations.h b/src/ripple/app/consensus/RCLValidations.h index b3684c1068..8bdc3418ad 100644 --- a/src/ripple/app/consensus/RCLValidations.h +++ b/src/ripple/app/consensus/RCLValidations.h @@ -134,13 +134,6 @@ public: return val_; } - // Get the validator cookie (or 0 if none set) - std::uint64_t - cookie() const - { - return val_->getFieldU64(sfCookie); - } - }; /** Wraps a ledger instance for use in generic Validations LedgerTrie. diff --git a/src/ripple/app/misc/ValidatorKeys.h b/src/ripple/app/misc/ValidatorKeys.h index e7ffa01f54..96af105624 100644 --- a/src/ripple/app/misc/ValidatorKeys.h +++ b/src/ripple/app/misc/ValidatorKeys.h @@ -40,9 +40,6 @@ public: SecretKey secretKey; NodeID nodeID; std::string manifest; - std::uint64_t cookie; //< Randomly generated at startup to tag validations - //< so nodes can identify unintentional configuration - //< reuse ValidatorKeys(Config const& config, beast::Journal j); diff --git a/src/ripple/app/misc/impl/ValidatorKeys.cpp b/src/ripple/app/misc/impl/ValidatorKeys.cpp index ce5d2312d0..2f96777c4d 100644 --- a/src/ripple/app/misc/impl/ValidatorKeys.cpp +++ b/src/ripple/app/misc/impl/ValidatorKeys.cpp @@ -21,14 +21,12 @@ #include #include -#include #include #include #include namespace ripple { ValidatorKeys::ValidatorKeys(Config const& config, beast::Journal j) - : cookie{rand_int(1, std::numeric_limits::max())} { if (config.exists(SECTION_VALIDATOR_TOKEN) && config.exists(SECTION_VALIDATION_SEED)) diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index c260d9aa33..9e46460c71 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -156,9 +156,7 @@ enum class ValStatus { /// Not current or was older than current from this node stale, /// A validation violates the increasing seq requirement - badSeq, - /// A validation used an inconsistent cookie - badCookie + badSeq }; inline std::string @@ -172,8 +170,6 @@ to_string(ValStatus m) return "stale"; case ValStatus::badSeq: return "badSeq"; - case ValStatus::badCookie: - return "badCookie"; default: return "unknown"; } @@ -244,11 +240,6 @@ to_string(ValStatus m) implementation_specific_t unwrap() -> return the implementation-specific type being wrapped - // Randomly generated cookie for a validator instance used to detect - // if multiple validators are accidentally running with the same - // validator keys - std::uint64_t cookie() const; - // ... implementation specific }; @@ -313,14 +304,6 @@ class Validations beast::uhash<>> byLedger_; - //! Validations from listed nodes indexed by ledger seq (partial and full) - beast::aged_unordered_map< - Seq, - hash_map, - std::chrono::steady_clock, - beast::uhash<>> - bySeq_; - // Represents the ancestry of validated ledgers LedgerTrie trie_; @@ -549,7 +532,7 @@ public: ValidationParms const& p, beast::abstract_clock& c, Ts&&... ts) - : byLedger_(c), bySeq_(c), parms_(p), adaptor_(std::forward(ts)...) + : byLedger_(c), parms_(p), adaptor_(std::forward(ts)...) { } @@ -600,34 +583,6 @@ public: { ScopedLock lock{mutex_}; - // Check if a validator with this nodeID already issued a validation - // for this sequence using a different cookie - auto const bySeqIt = bySeq_[val.seq()].emplace(nodeID, val); - if(!bySeqIt.second) - { - if(bySeqIt.first->second.cookie() != val.cookie()) - { - // Remove prior validation if it was for a different ledger - ID const priorID = bySeqIt.first->second.ledgerID(); - if (priorID != val.ledgerID()) - { - auto const byLedgerIt = byLedger_.find(priorID); - if (byLedgerIt != byLedger_.end()) - byLedger_[priorID].erase(nodeID); - - auto const currIt = current_.find(nodeID); - if (currIt != current_.end() && - currIt->second.ledgerID() == priorID) - { - removeTrie(lock, currIt->first, currIt->second); - adaptor_.onStale(std::move(currIt->second)); - current_.erase(currIt); - } - } - return ValStatus::badCookie; - } - } - // Check that validation sequence is greater than any non-expired // validations sequence from that validator auto const now = byLedger_.clock().now(); @@ -674,7 +629,6 @@ public: { ScopedLock lock{mutex_}; beast::expire(byLedger_, parms_.validationSET_EXPIRES); - beast::expire(bySeq_, parms_.validationSET_EXPIRES); } /** Update trust status of validations @@ -719,21 +673,6 @@ public: } } } - - for (auto& it : bySeq_) - { - for (auto& nodeVal : it.second) - { - if (added.find(nodeVal.first) != added.end()) - { - nodeVal.second.setTrusted(); - } - else if (removed.find(nodeVal.first) != removed.end()) - { - nodeVal.second.setUntrusted(); - } - } - } } Json::Value diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index e94d2fb060..b380f3eaad 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -78,7 +78,6 @@ class FeatureCollections "Checks", "fix1571", "fix1543", - "ValidationCookies", "fix1623" }; @@ -363,7 +362,6 @@ extern uint256 const featureDepositAuth; extern uint256 const featureChecks; extern uint256 const fix1571; extern uint256 const fix1543; -extern uint256 const featureValidationCookies; extern uint256 const fix1623; } // ripple diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index 4b8e69495e..644c9500b6 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -117,7 +117,6 @@ public: @param isFull Whether the validation is full or partial @param fee FeeSettings to include in the validation @param amendments If not empty, the amendments to include in this validation - @param cookie If not none, the validation cookie to set @note The fee and amendment settings are only set if not boost::none. Typically, the amendments and fees are set for validations of flag @@ -133,8 +132,7 @@ public: NodeID const& nodeID, bool isFull, FeeSettings const& fees, - std::vector const& amendments, - boost::optional const cookie); + std::vector const& amendments); STBase* copy(std::size_t n, void* buf) const override diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 776642f03b..4479b8098e 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -110,7 +110,6 @@ detail::supportedAmendments () { "157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F97AE4B1F2D1 Checks" }, { "7117E2EC2DBF119CA55181D69819F1999ECEE1A0225A7FD2B9ED47940968479C fix1571" }, { "CA7C02118BA27599528543DFE77BA6838D1B0F43B447D4D7F53523CE6A0E9AC2 fix1543" }, - { "8413364A1E27704241F7106789570A4B36EE2AFA14F94828E78BE942AB2F24BE ValidationCookies" }, { "58BE9B5968C4DA7C59BA900961828B113E5490699B21877DEF9A31E9D0FE5D5F fix1623" } }; return supported; @@ -163,7 +162,6 @@ uint256 const featureDepositAuth = *getRegisteredFeature("DepositAuth"); uint256 const featureChecks = *getRegisteredFeature("Checks"); uint256 const fix1571 = *getRegisteredFeature("fix1571"); uint256 const fix1543 = *getRegisteredFeature("fix1543"); -uint256 const featureValidationCookies = *getRegisteredFeature("ValidationCookies"); uint256 const fix1623 = *getRegisteredFeature("fix1623"); } // ripple diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index d5178b54b5..92e92c6b9b 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -35,8 +35,7 @@ STValidation::STValidation( NodeID const& nodeID, bool isFull, FeeSettings const& fees, - std::vector const& amendments, - boost::optional const cookie) + std::vector const& amendments) : STObject(getFormat(), sfValidation), mNodeID(nodeID), mSeen(signTime) { // This is our own public key and it should always be valid. @@ -70,8 +69,6 @@ STValidation::STValidation( setFlag(vfFullyCanonicalSig); - if(cookie != boost::none) - setFieldU64(sfCookie, *cookie); auto const signingHash = getSigningHash(); setFieldVL( sfSignature, signDigest(getSignerPublic(), secretKey, signingHash)); diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 1ca523ef03..24f257924d 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -398,8 +398,7 @@ public: calcNodeID(val.first), true, STValidation::FeeSettings{}, - field, - boost::none); + field); validations.emplace_back(v); } diff --git a/src/test/app/RCLValidations_test.cpp b/src/test/app/RCLValidations_test.cpp index 8e7ddea545..d25dcc152a 100644 --- a/src/test/app/RCLValidations_test.cpp +++ b/src/test/app/RCLValidations_test.cpp @@ -45,8 +45,7 @@ class RCLValidations_test : public beast::unit_test::suite calcNodeID(keys.first), true, STValidation::FeeSettings{}, - std::vector{}, - 1001 /* cookie */); + std::vector{}); BEAST_EXPECT(v->isTrusted()); v->setUntrusted(); diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index dfffec82c8..35921c5ba5 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/src/test/consensus/Validations_test.cpp @@ -17,7 +17,6 @@ */ //============================================================================== -#include #include #include #include @@ -57,15 +56,11 @@ class Validations_test : public beast::unit_test::suite bool trusted_ = true; std::size_t signIdx_ = 1; boost::optional loadFee_; - std::uint64_t cookie_; public: Node(PeerID nodeID, clock_type const& c) : c_(c) , nodeID_(nodeID) - , cookie_(rand_int( - 1, - std::numeric_limits::max())) { } @@ -133,7 +128,6 @@ class Validations_test : public beast::unit_test::suite currKey(), nodeID_, full, - cookie_, loadFee_}; if (trusted_) v.setTrusted(); @@ -1149,45 +1143,6 @@ class Validations_test : public beast::unit_test::suite } } - void - testCookie() - { - testcase("Bad cookie"); - - LedgerHistoryHelper h; - TestHarness harness(h.oracle); - Node a = harness.makeNode(); - Node aReuse{a.nodeID(), harness.clock()}; - Node b = harness.makeNode(); - - BEAST_EXPECT(ValStatus::current == harness.add(a.validate(h["a"]))); - BEAST_EXPECT(ValStatus::current == harness.add(b.validate(h["b"]))); - - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["a"].id()) == 1); - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["b"].id()) == 1); - BEAST_EXPECT(harness.vals().currentTrusted().size() == 2); - // Re-issuing for the same ledger gives badCookie status, but does not - // ignore that ledger - BEAST_EXPECT( - ValStatus::badCookie == harness.add(aReuse.validate(h["a"]))); - - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["a"].id()) == 1); - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["b"].id()) == 1); - BEAST_EXPECT(harness.vals().currentTrusted().size() == 2); - - // Re-issuing for a different ledger gives badCookie status and ignores - // the prior validated ledger - BEAST_EXPECT( - ValStatus::badCookie == harness.add(aReuse.validate(h["b"]))); - - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["a"].id()) == 0); - BEAST_EXPECT(harness.vals().numTrustedForLedger(h["b"].id()) == 1); - BEAST_EXPECT(harness.vals().currentTrusted().size() == 1); - - BEAST_EXPECT( - ValStatus::badCookie == harness.add(aReuse.validate(h["b"]))); - } - void run() override { @@ -1205,7 +1160,6 @@ class Validations_test : public beast::unit_test::suite testNumTrustedForLedger(); testSeqEnforcer(); testTrustChanged(); - testCookie(); } }; diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index a77002fc28..c536b25fa3 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -19,7 +19,6 @@ #ifndef RIPPLE_TEST_CSF_PEER_H_INCLUDED #define RIPPLE_TEST_CSF_PEER_H_INCLUDED -#include #include #include #include @@ -259,9 +258,6 @@ struct Peer //! The collectors to report events to CollectorRefs & collectors; - //! Random cookie used to tag validations - std::uint64_t cookie; - /** Constructor @param i Unique PeerID @@ -294,9 +290,6 @@ struct Peer , validations{ValidationParms{}, s.clock(), *this} , fullyValidatedLedger{Ledger::MakeGenesis{}} , collectors{c} - , cookie{rand_int( - 1, - std::numeric_limits::max())} { // All peers start from the default constructed genesis ledger ledgers[lastClosedLedger.id()] = lastClosedLedger; @@ -593,8 +586,7 @@ struct Peer now(), key, id, - isFull, - cookie}; + isFull}; // share the new validation; it is trusted by the receiver share(v); // we trust ourselves diff --git a/src/test/csf/Validation.h b/src/test/csf/Validation.h index 0db7a45aa5..a65e67b275 100644 --- a/src/test/csf/Validation.h +++ b/src/test/csf/Validation.h @@ -55,7 +55,6 @@ class Validation PeerID nodeID_{0}; bool trusted_ = false; bool full_ = false; - std::uint64_t cookie_; boost::optional loadFee_; public: @@ -69,7 +68,6 @@ public: PeerKey key, PeerID nodeID, bool full, - std::uint64_t cookie, boost::optional loadFee = boost::none) : ledgerID_{id} , seq_{seq} @@ -78,7 +76,6 @@ public: , key_{key} , nodeID_{nodeID} , full_{full} - , cookie_{cookie} , loadFee_{loadFee} { } @@ -147,12 +144,6 @@ public: return *this; } - std::uint64_t - cookie() const - { - return cookie_; - } - auto asTie() const {