From f7a4a94c3bda66277431393de0254c1417b1f282 Mon Sep 17 00:00:00 2001 From: Brad Chase Date: Mon, 12 Mar 2018 12:41:09 -0400 Subject: [PATCH] Add cookie to validation (RIPD-1586): Each validator will generate a random cookie on startup that it will include in each of its validations. This will allow validators to detect when more than one validator is accidentally operating with the same validation keys. --- src/ripple/app/consensus/RCLConsensus.cpp | 44 +++++++----- src/ripple/app/consensus/RCLConsensus.h | 1 + src/ripple/app/consensus/RCLValidations.cpp | 13 +++- src/ripple/app/consensus/RCLValidations.h | 7 ++ src/ripple/app/misc/AmendmentTable.h | 13 +--- src/ripple/app/misc/FeeVote.h | 2 +- src/ripple/app/misc/FeeVoteImpl.cpp | 13 ++-- src/ripple/app/misc/ValidatorKeys.h | 4 ++ src/ripple/app/misc/impl/ValidatorKeys.cpp | 2 + src/ripple/consensus/Validations.h | 79 ++++++++++++++++++--- src/ripple/protocol/Feature.h | 2 + src/ripple/protocol/SField.h | 2 + src/ripple/protocol/STValidation.h | 38 +++++++--- src/ripple/protocol/impl/Feature.cpp | 4 +- src/ripple/protocol/impl/SField.cpp | 2 + src/ripple/protocol/impl/STValidation.cpp | 71 +++++++++++------- src/test/app/AmendmentTable_test.cpp | 30 ++++---- src/test/app/RCLValidations_test.cpp | 18 +++-- src/test/consensus/Validations_test.cpp | 51 ++++++++++++- src/test/csf/Peer.h | 14 +++- src/test/csf/Validation.h | 9 +++ 21 files changed, 313 insertions(+), 106 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index d4cdedd8c..c406d8da8 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -83,6 +83,7 @@ RCLConsensus::Adaptor::Adaptor( , nodeID_{validatorKeys.nodeID} , valPublic_{validatorKeys.publicKey} , valSecret_{validatorKeys.secretKey} + , cookie_{validatorKeys.cookie} { } @@ -834,36 +835,45 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, validationTime = lastValidationTime_ + 1s; lastValidationTime_ = validationTime; - // Build validation - auto v = std::make_shared( - ledger.id(), - txns.id(), - validationTime, - valPublic_, - nodeID_, - proposing /* full if proposed */); - v->setFieldU32(sfLedgerSequence, ledger.seq()); + STValidation::FeeSettings fees; + std::vector amendments; - // Add our load fee to the validation auto const& feeTrack = app_.getFeeTrack(); std::uint32_t fee = std::max(feeTrack.getLocalFee(), feeTrack.getClusterFee()); if (fee > feeTrack.getLoadBase()) - v->setFieldU32(sfLoadFee, fee); + fees.loadFee = fee; - if (((ledger.seq() + 1) % 256) == 0) // next ledger is flag ledger + if (((ledger.seq() + 1) % 256) == 0) { // Suggest fee changes and new features - feeVote_->doValidation(ledger.ledger_, *v); - app_.getAmendmentTable().doValidation(ledger.ledger_, *v); + feeVote_->doValidation(ledger.ledger_, fees); + amendments = app_.getAmendmentTable().doValidation (getEnabledAmendments(*ledger.ledger_)); } - auto const signingHash = v->sign(valSecret_); - v->setTrusted(); + boost::optional maybeCookie; + if (ledgerMaster_.getValidatedRules().enabled(featureValidationCookies)) + maybeCookie.emplace(cookie_); + + auto v = std::make_shared( + ledger.id(), + ledger.seq(), + txns.id(), + validationTime, + valPublic_, + valSecret_, + nodeID_, + proposing /* full if proposed */, + fees, + amendments, + maybeCookie); + + + // suppress it if we receive it - FIXME: wrong suppression - app_.getHashRouter().addSuppression(signingHash); + app_.getHashRouter().addSuppression(v->getSigningHash()); handleNewValidation(app_, v, "local"); Blob validation = v->getSerialized(); protocol::TMValidation val; diff --git a/src/ripple/app/consensus/RCLConsensus.h b/src/ripple/app/consensus/RCLConsensus.h index 9903b69d5..2510b5648 100644 --- a/src/ripple/app/consensus/RCLConsensus.h +++ b/src/ripple/app/consensus/RCLConsensus.h @@ -60,6 +60,7 @@ 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 a638d8382..c16d68d38 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -320,9 +320,17 @@ handleNewValidation(Application& app, if(j.debug()) dmp(j.debug(), to_string(outcome)); - if(outcome == ValStatus::badSeq && j.warn()) + 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)); } @@ -333,7 +341,6 @@ handleNewValidation(Application& app, hash, val->getFieldU32(sfLedgerSequence)); shouldRelay = true; } - } else { diff --git a/src/ripple/app/consensus/RCLValidations.h b/src/ripple/app/consensus/RCLValidations.h index 8bdc3418a..b3684c106 100644 --- a/src/ripple/app/consensus/RCLValidations.h +++ b/src/ripple/app/consensus/RCLValidations.h @@ -134,6 +134,13 @@ 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/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index 674fd57bb..e59690f51 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -100,23 +100,12 @@ public: virtual std::vector getDesired () = 0; - // The two function below adapt the API callers expect to the + // The function below adapts the API callers expect to the // internal amendment table API. This allows the amendment // table implementation to be independent of the ledger // implementation. These APIs will merge when the view code // supports a full ledger API - void - doValidation (std::shared_ptr const& lastClosedLedger, - STObject& baseValidation) - { - auto ourAmendments = - doValidation (getEnabledAmendments(*lastClosedLedger)); - if (! ourAmendments.empty()) - baseValidation.setFieldV256 (sfAmendments, - STVector256 (sfAmendments, ourAmendments)); - } - void doVoting ( std::shared_ptr const& lastClosedLedger, diff --git a/src/ripple/app/misc/FeeVote.h b/src/ripple/app/misc/FeeVote.h index 4e8377303..808fd0581 100644 --- a/src/ripple/app/misc/FeeVote.h +++ b/src/ripple/app/misc/FeeVote.h @@ -62,7 +62,7 @@ public: virtual void doValidation (std::shared_ptr const& lastClosedLedger, - STObject& baseValidation) = 0; + STValidation::FeeSettings & fees) = 0; /** Cast our local vote on the fee. diff --git a/src/ripple/app/misc/FeeVoteImpl.cpp b/src/ripple/app/misc/FeeVoteImpl.cpp index 51446a3b8..407234e6e 100644 --- a/src/ripple/app/misc/FeeVoteImpl.cpp +++ b/src/ripple/app/misc/FeeVoteImpl.cpp @@ -98,7 +98,7 @@ public: void doValidation (std::shared_ptr const& lastClosedLedger, - STObject& baseValidation) override; + STValidation::FeeSettings& fees) override; void doVoting (std::shared_ptr const& lastClosedLedger, @@ -117,22 +117,22 @@ FeeVoteImpl::FeeVoteImpl (Setup const& setup, beast::Journal journal) void FeeVoteImpl::doValidation( std::shared_ptr const& lastClosedLedger, - STObject& baseValidation) + STValidation::FeeSettings& fees) { if (lastClosedLedger->fees().base != target_.reference_fee) { JLOG(journal_.info()) << "Voting for base fee of " << target_.reference_fee; - baseValidation.setFieldU64 (sfBaseFee, target_.reference_fee); + fees.baseFee = target_.reference_fee; } if (lastClosedLedger->fees().accountReserve(0) != target_.account_reserve) { JLOG(journal_.info()) << - "Voting for base resrve of " << target_.account_reserve; + "Voting for base reserve of " << target_.account_reserve; - baseValidation.setFieldU32(sfReserveBase, target_.account_reserve); + fees.reserveBase = target_.account_reserve; } if (lastClosedLedger->fees().increment != target_.owner_reserve) @@ -140,8 +140,7 @@ FeeVoteImpl::doValidation( JLOG(journal_.info()) << "Voting for reserve increment of " << target_.owner_reserve; - baseValidation.setFieldU32 (sfReserveIncrement, - target_.owner_reserve); + fees.reserveIncrement = target_.owner_reserve; } } diff --git a/src/ripple/app/misc/ValidatorKeys.h b/src/ripple/app/misc/ValidatorKeys.h index 0f52c27e5..e7ffa01f5 100644 --- a/src/ripple/app/misc/ValidatorKeys.h +++ b/src/ripple/app/misc/ValidatorKeys.h @@ -40,6 +40,10 @@ 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); bool configInvalid() const diff --git a/src/ripple/app/misc/impl/ValidatorKeys.cpp b/src/ripple/app/misc/impl/ValidatorKeys.cpp index 2f96777c4..ce5d2312d 100644 --- a/src/ripple/app/misc/impl/ValidatorKeys.cpp +++ b/src/ripple/app/misc/impl/ValidatorKeys.cpp @@ -21,12 +21,14 @@ #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 0732e82c3..c260d9aa3 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -156,7 +156,9 @@ enum class ValStatus { /// Not current or was older than current from this node stale, /// A validation violates the increasing seq requirement - badSeq + badSeq, + /// A validation used an inconsistent cookie + badCookie }; inline std::string @@ -170,6 +172,8 @@ to_string(ValStatus m) return "stale"; case ValStatus::badSeq: return "badSeq"; + case ValStatus::badCookie: + return "badCookie"; default: return "unknown"; } @@ -240,6 +244,11 @@ 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 }; @@ -304,6 +313,14 @@ 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_; @@ -532,7 +549,7 @@ public: ValidationParms const& p, beast::abstract_clock& c, Ts&&... ts) - : byLedger_(c), parms_(p), adaptor_(std::forward(ts)...) + : byLedger_(c), bySeq_(c), parms_(p), adaptor_(std::forward(ts)...) { } @@ -583,6 +600,34 @@ 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(); @@ -591,9 +636,9 @@ public: return ValStatus::badSeq; // Use insert_or_assign when C++17 supported - auto ret = byLedger_[val.ledgerID()].emplace(nodeID, val); - if (!ret.second) - ret.first->second = val; + auto byLedgerIt = byLedger_[val.ledgerID()].emplace(nodeID, val); + if (!byLedgerIt.second) + byLedgerIt.first->second = val; auto const ins = current_.emplace(nodeID, val); if (!ins.second) @@ -629,6 +674,7 @@ public: { ScopedLock lock{mutex_}; beast::expire(byLedger_, parms_.validationSET_EXPIRES); + beast::expire(bySeq_, parms_.validationSET_EXPIRES); } /** Update trust status of validations @@ -647,12 +693,12 @@ public: for (auto& it : current_) { - if (added.count(it.first)) + if (added.find(it.first) != added.end()) { it.second.setTrusted(); updateTrie(lock, it.first, it.second, boost::none); } - else if (removed.count(it.first)) + else if (removed.find(it.first) != removed.end()) { it.second.setUntrusted(); removeTrie(lock, it.first, it.second); @@ -663,11 +709,26 @@ public: { for (auto& nodeVal : it.second) { - if (added.count(nodeVal.first)) + if (added.find(nodeVal.first) != added.end()) { nodeVal.second.setTrusted(); } - else if (removed.count(nodeVal.first)) + else if (removed.find(nodeVal.first) != removed.end()) + { + nodeVal.second.setUntrusted(); + } + } + } + + 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(); } diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 151fd33f0..c329cc448 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -78,6 +78,7 @@ class FeatureCollections "Checks", "fix1571", "fix1543", + "ValidationCookies" }; std::vector features; @@ -361,6 +362,7 @@ extern uint256 const featureDepositAuth; extern uint256 const featureChecks; extern uint256 const fix1571; extern uint256 const fix1543; +extern uint256 const featureValidationCookies; } // ripple diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index cdbd3cc00..1c11f4852 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -388,6 +388,8 @@ extern SF_U64 const sfExchangeRate; extern SF_U64 const sfLowNode; extern SF_U64 const sfHighNode; extern SF_U64 const sfDestinationNode; +extern SF_U64 const sfCookie; + // 128-bit extern SF_U128 const sfEmailHash; diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index 9e61cf3e0..4b8e69495 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -91,27 +91,50 @@ public: assert(mNodeID.isNonZero()); } - /** Construct a new STValidation + /** Fees to set when issuing a new validation. - Constructs a new STValidation issued by a node. The instance should be - signed before sharing with other nodes. + Optional fees to include when issuing a new validation + */ + struct FeeSettings + { + boost::optional loadFee; + boost::optional baseFee; + boost::optional reserveBase; + boost::optional reserveIncrement; + }; + + /** Construct, sign and trust a new STValidation + + Constructs, signs and trusts new STValidation issued by this node. @param ledgerHash The hash of the validated ledger + @param ledgerSeq The sequence number (index) of the ledger @param consensusHash The hash of the consensus transaction set @param signTime When the validation is signed @param publicKey The current signing public key + @param secretKey The current signing secret key @param nodeID ID corresponding to node's public master key @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 + ledgers only. */ - STValidation( uint256 const& ledgerHash, + std::uint32_t ledgerSeq, uint256 const& consensusHash, NetClock::time_point signTime, PublicKey const& publicKey, + SecretKey const& secretKey, NodeID const& nodeID, - bool isFull); + bool isFull, + FeeSettings const& fees, + std::vector const& amendments, + boost::optional const cookie); STBase* copy(std::size_t n, void* buf) const override @@ -190,11 +213,8 @@ public: Blob getSignature() const; - // Signs the validation and returns the signing hash - uint256 - sign(SecretKey const& secretKey); - private: + static SOTemplate const& getFormat(); diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index a1e9b4728..fd09433ed 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -109,7 +109,8 @@ detail::supportedAmendments () { "F64E1EABBE79D55B3BB82020516CEC2C582A98A6BFE20FBE9BB6A0D233418064 DepositAuth"}, { "157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F97AE4B1F2D1 Checks"}, { "7117E2EC2DBF119CA55181D69819F1999ECEE1A0225A7FD2B9ED47940968479C fix1571" }, - { "CA7C02118BA27599528543DFE77BA6838D1B0F43B447D4D7F53523CE6A0E9AC2 fix1543" } + { "CA7C02118BA27599528543DFE77BA6838D1B0F43B447D4D7F53523CE6A0E9AC2 fix1543" }, + { "8413364A1E27704241F7106789570A4B36EE2AFA14F94828E78BE942AB2F24BE ValidationCookies"} }; return supported; } @@ -161,5 +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"); } // ripple diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index bddf38bea..6b2b5c077 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -142,6 +142,8 @@ SF_U64 const sfExchangeRate = make::one(&sfExchangeRate, S SF_U64 const sfLowNode = make::one(&sfLowNode, STI_UINT64, 7, "LowNode"); SF_U64 const sfHighNode = make::one(&sfHighNode, STI_UINT64, 8, "HighNode"); SF_U64 const sfDestinationNode = make::one(&sfDestinationNode, STI_UINT64, 9, "DestinationNode"); +SF_U64 const sfCookie = make::one(&sfCookie, STI_UINT64, 10,"Cookie"); + // 128-bit SF_U128 const sfEmailHash = make::one(&sfEmailHash, STI_HASH128, 1, "EmailHash"); diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index 9a66ac663..d5178b54b 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -26,40 +26,57 @@ namespace ripple { STValidation::STValidation( - uint256 const& ledgerHash, - uint256 const& consensusHash, - NetClock::time_point signTime, - PublicKey const& publicKey, - NodeID const& nodeID, - bool isFull) - : STObject(getFormat(), sfValidation) - , mNodeID(nodeID) - , mSeen(signTime) + uint256 const& ledgerHash, + std::uint32_t ledgerSeq, + uint256 const& consensusHash, + NetClock::time_point signTime, + PublicKey const& publicKey, + SecretKey const& secretKey, + NodeID const& nodeID, + bool isFull, + FeeSettings const& fees, + std::vector const& amendments, + boost::optional const cookie) + : STObject(getFormat(), sfValidation), mNodeID(nodeID), mSeen(signTime) { // This is our own public key and it should always be valid. if (!publicKeyType(publicKey)) - LogicError ("Invalid validation public key"); - - // Does not sign - setFieldH256 (sfLedgerHash, ledgerHash); - setFieldH256 (sfConsensusHash, consensusHash); - setFieldU32 (sfSigningTime, signTime.time_since_epoch().count()); - setFieldVL (sfSigningPubKey, publicKey.slice()); - - assert (mNodeID.isNonZero ()); + LogicError("Invalid validation public key"); + assert(mNodeID.isNonZero()); + setFieldH256(sfLedgerHash, ledgerHash); + setFieldH256(sfConsensusHash, consensusHash); + setFieldU32(sfSigningTime, signTime.time_since_epoch().count()); + setFieldVL(sfSigningPubKey, publicKey.slice()); if (isFull) - setFlag (kFullFlag); -} + setFlag(kFullFlag); -uint256 STValidation::sign (SecretKey const& secretKey) -{ - setFlag (vfFullyCanonicalSig); + setFieldU32(sfLedgerSequence, ledgerSeq); + if (fees.loadFee) + setFieldU32(sfLoadFee, *fees.loadFee); + + if (fees.baseFee) + setFieldU64(sfBaseFee, *fees.baseFee); + + if (fees.reserveBase) + setFieldU32(sfReserveBase, *fees.reserveBase); + + if (fees.reserveIncrement) + setFieldU32(sfReserveIncrement, *fees.reserveIncrement); + + if (!amendments.empty()) + setFieldV256(sfAmendments, STVector256(sfAmendments, amendments)); + + setFlag(vfFullyCanonicalSig); + + if(cookie != boost::none) + setFieldU64(sfCookie, *cookie); auto const signingHash = getSigningHash(); - setFieldVL (sfSignature, - signDigest (getSignerPublic(), secretKey, signingHash)); - return signingHash; + setFieldVL( + sfSignature, signDigest(getSignerPublic(), secretKey, signingHash)); + + setTrusted(); } uint256 STValidation::getSigningHash () const @@ -156,6 +173,8 @@ SOTemplate const& STValidation::getFormat () format.push_back (SOElement (sfSigningPubKey, SOE_REQUIRED)); format.push_back (SOElement (sfSignature, SOE_OPTIONAL)); format.push_back (SOElement (sfConsensusHash, SOE_OPTIONAL)); + format.push_back (SOElement (sfCookie, SOE_OPTIONAL)); + } }; diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 04c00b23b..5b801a6e1 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -330,14 +330,13 @@ public: BEAST_EXPECT(ret.second == post_state.end()); } - std::vector makeValidators (int num) + std::vector> makeValidators (int num) { - std::vector ret; + std::vector > ret; ret.reserve (num); for (int i = 0; i < num; ++i) { - ret.push_back ( - randomKeyPair(KeyType::secp256k1).first); + ret.emplace_back(randomKeyPair(KeyType::secp256k1)); } return ret; } @@ -351,7 +350,7 @@ public: void doRound( AmendmentTable& table, weeks week, - std::vector const& validators, + std::vector > const& validators, std::vector > const& votes, std::vector & ourVotes, std::set & enabled, @@ -377,11 +376,8 @@ public: int i = 0; for (auto const& val : validators) { - auto v = std::make_shared( - uint256(), uint256(), roundTime, val, calcNodeID(val), true); - ++i; - STVector256 field (sfAmendments); + std::vector field; for (auto const& amendment : votes) { @@ -391,10 +387,20 @@ public: field.push_back (amendment.first); } } - if (!field.empty ()) - v->setFieldV256 (sfAmendments, field); - v->setTrusted(); + auto v = std::make_shared( + uint256(), + i, + uint256(), + roundTime, + val.first, + val.second, + calcNodeID(val.first), + true, + STValidation::FeeSettings{}, + field, + boost::none); + validations.emplace_back(v); } diff --git a/src/test/app/RCLValidations_test.cpp b/src/test/app/RCLValidations_test.cpp index 0deac4ca5..8e7ddea54 100644 --- a/src/test/app/RCLValidations_test.cpp +++ b/src/test/app/RCLValidations_test.cpp @@ -34,12 +34,20 @@ class RCLValidations_test : public beast::unit_test::suite testChangeTrusted() { testcase("Change validation trusted status"); - PublicKey key = derivePublicKey(KeyType::ed25519, randomSecretKey()); - auto v = std::make_shared(uint256(), uint256(), - NetClock::time_point(), key, calcNodeID(key), true); + auto keys = randomKeyPair(KeyType::secp256k1); + auto v = std::make_shared( + uint256(), + 1, + uint256(), + NetClock::time_point(), + keys.first, + keys.second, + calcNodeID(keys.first), + true, + STValidation::FeeSettings{}, + std::vector{}, + 1001 /* cookie */); - BEAST_EXPECT(!v->isTrusted()); - v->setTrusted(); BEAST_EXPECT(v->isTrusted()); v->setUntrusted(); BEAST_EXPECT(!v->isTrusted()); diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index c4c1517c4..dfffec82c 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/src/test/consensus/Validations_test.cpp @@ -16,6 +16,8 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== + +#include #include #include #include @@ -55,9 +57,15 @@ 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) + Node(PeerID nodeID, clock_type const& c) + : c_(c) + , nodeID_(nodeID) + , cookie_(rand_int( + 1, + std::numeric_limits::max())) { } @@ -125,6 +133,7 @@ class Validations_test : public beast::unit_test::suite currKey(), nodeID_, full, + cookie_, loadFee_}; if (trusted_) v.setTrusted(); @@ -1140,6 +1149,45 @@ 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 { @@ -1157,6 +1205,7 @@ 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 91536d232..a77002fc2 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -19,6 +19,7 @@ #ifndef RIPPLE_TEST_CSF_PEER_H_INCLUDED #define RIPPLE_TEST_CSF_PEER_H_INCLUDED +#include #include #include #include @@ -258,6 +259,9 @@ 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 @@ -274,8 +278,8 @@ struct Peer Scheduler& s, LedgerOracle& o, BasicNetwork& n, - TrustGraph & tg, - CollectorRefs & c, + TrustGraph& tg, + CollectorRefs& c, beast::Journal jIn) : sink(jIn, "Peer " + to_string(i) + ": ") , j(sink) @@ -290,6 +294,9 @@ 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; @@ -586,7 +593,8 @@ struct Peer now(), key, id, - isFull}; + isFull, + cookie}; // 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 a65e67b27..0db7a45aa 100644 --- a/src/test/csf/Validation.h +++ b/src/test/csf/Validation.h @@ -55,6 +55,7 @@ class Validation PeerID nodeID_{0}; bool trusted_ = false; bool full_ = false; + std::uint64_t cookie_; boost::optional loadFee_; public: @@ -68,6 +69,7 @@ public: PeerKey key, PeerID nodeID, bool full, + std::uint64_t cookie, boost::optional loadFee = boost::none) : ledgerID_{id} , seq_{seq} @@ -76,6 +78,7 @@ public: , key_{key} , nodeID_{nodeID} , full_{full} + , cookie_{cookie} , loadFee_{loadFee} { } @@ -144,6 +147,12 @@ public: return *this; } + std::uint64_t + cookie() const + { + return cookie_; + } + auto asTie() const {