From 381606aba2efcd179517f2497dc6e8c2770c679e Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Fri, 16 Nov 2018 02:24:23 -0800 Subject: [PATCH] Harden validations: This commit introduces the "HardenedValidations" amendment which, if enabled, allows validators to include additional information in their validations that can increase the robustness of consensus. Specifically, the commit introduces a new optional field that can be set in validation messages can be used to attest to the hash of the latest ledger that a validator considers to be fully validated. Additionally, the commit leverages the previously introduced "cookie" field to improve the robustness of the network by making it possible for servers to automatically detect accidental misconfiguration which results in two or more validators using the same validation key. --- src/ripple/app/consensus/RCLConsensus.cpp | 82 +++++++++----- src/ripple/app/consensus/RCLConsensus.h | 3 + src/ripple/app/consensus/RCLValidations.cpp | 40 +++++-- src/ripple/app/consensus/RCLValidations.h | 18 ++- src/ripple/app/misc/AmendmentTable.h | 4 +- src/ripple/app/misc/FeeVote.h | 6 +- src/ripple/app/misc/FeeVoteImpl.cpp | 30 ++--- src/ripple/app/misc/NetworkOPs.cpp | 12 +- src/ripple/app/misc/NetworkOPs.h | 6 +- src/ripple/app/misc/impl/AmendmentTable.cpp | 4 +- src/ripple/consensus/Validations.h | 68 +++++++++++- src/ripple/overlay/impl/PeerImp.cpp | 7 +- src/ripple/overlay/impl/PeerImp.h | 2 +- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/SField.h | 1 + src/ripple/protocol/SOTemplate.h | 8 +- src/ripple/protocol/STValidation.h | 94 ++++++++-------- src/ripple/protocol/impl/Feature.cpp | 4 +- src/ripple/protocol/impl/SField.cpp | 1 + src/ripple/protocol/impl/STValidation.cpp | 116 +++++--------------- src/test/app/AmendmentTable_test.cpp | 16 +-- src/test/app/RCLValidations_test.cpp | 9 +- src/test/consensus/Validations_test.cpp | 8 +- src/test/csf/Validation.h | 14 ++- src/test/rpc/Subscribe_test.cpp | 3 +- 25 files changed, 313 insertions(+), 246 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 06c6bbec0..2c3c6a1e7 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -85,7 +86,14 @@ RCLConsensus::Adaptor::Adaptor( , nodeID_{validatorKeys.nodeID} , valPublic_{validatorKeys.publicKey} , valSecret_{validatorKeys.secretKey} + , valCookie_{ + rand_int(1, std::numeric_limits::max())} { + assert(valCookie_ != 0); + + JLOG(j_.info()) << "Consensus engine started" + << " (Node: " << to_string(nodeID_) + << ", Cookie: " << valCookie_ << ")"; } boost::optional @@ -753,41 +761,63 @@ RCLConsensus::Adaptor::validate( bool proposing) { using namespace std::chrono_literals; + auto validationTime = app_.timeKeeper().closeTime(); if (validationTime <= lastValidationTime_) validationTime = lastValidationTime_ + 1s; lastValidationTime_ = validationTime; - STValidation::FeeSettings fees; - std::vector amendments; - - auto const& feeTrack = app_.getFeeTrack(); - std::uint32_t fee = - std::max(feeTrack.getLocalFee(), feeTrack.getClusterFee()); - - if (fee > feeTrack.getLoadBase()) - fees.loadFee = fee; - - // next ledger is flag ledger - if (((ledger.seq() + 1) % 256) == 0) - { - // Suggest fee changes and new features - feeVote_->doValidation(ledger.ledger_, fees); - amendments = app_.getAmendmentTable().doValidation( - getEnabledAmendments(*ledger.ledger_)); - } - auto v = std::make_shared( - ledger.id(), - ledger.seq(), - txns.id(), - validationTime, + lastValidationTime_, valPublic_, valSecret_, nodeID_, - proposing /* full if proposed */, - fees, - amendments); + [&](STValidation& v) { + v.setFieldH256(sfLedgerHash, ledger.id()); + v.setFieldH256(sfConsensusHash, txns.id()); + + v.setFieldU32(sfLedgerSequence, ledger.seq()); + + if (proposing) + v.setFlag(vfFullValidation); + + if (ledger.ledger_->rules().enabled(featureHardenedValidations)) + { + // Attest to the hash of what we consider to be the last fully + // validated ledger. This may be the hash of the ledger we are + // validating here, and that's fine. + if (auto const vl = ledgerMaster_.getValidatedLedger()) + v.setFieldH256(sfValidatedHash, vl->info().hash); + + v.setFieldU64(sfCookie, valCookie_); + } + + // Report our load + { + auto const& ft = app_.getFeeTrack(); + auto const fee = std::max(ft.getLocalFee(), ft.getClusterFee()); + if (fee > ft.getLoadBase()) + v.setFieldU32(sfLoadFee, fee); + } + + // If the next ledger is a flag ledger, suggest fee changes and + // new features: + if ((ledger.seq() + 1) % 256 == 0) + { + // Fees: + feeVote_->doValidation(ledger.ledger_->fees(), v); + + // Amendments + // FIXME: pass `v` and have the function insert the array + // directly? + auto const amendments = app_.getAmendmentTable().doValidation( + getEnabledAmendments(*ledger.ledger_)); + + if (!amendments.empty()) + v.setFieldV256( + sfAmendments, STVector256(sfAmendments, amendments)); + } + }); // suppress it if we receive it app_.getHashRouter().addSuppression( diff --git a/src/ripple/app/consensus/RCLConsensus.h b/src/ripple/app/consensus/RCLConsensus.h index 2a932ed60..f06dc5e5a 100644 --- a/src/ripple/app/consensus/RCLConsensus.h +++ b/src/ripple/app/consensus/RCLConsensus.h @@ -66,6 +66,9 @@ class RCLConsensus PublicKey const valPublic_; SecretKey const valSecret_; + // A randomly selected non-zero value used to tag our validations + std::uint64_t const valCookie_; + // Ledger we most recently needed to acquire LedgerHash acquiringLedger_; ConsensusParms parms_; diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index 9940f2a54..71b3879fa 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -151,15 +151,14 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash) bool handleNewValidation( Application& app, - STValidation::ref val, + std::shared_ptr const& val, std::string const& source) { PublicKey const& signingKey = val->getSignerPublic(); uint256 const& hash = val->getLedgerHash(); // Ensure validation is marked as trusted if signer currently trusted - boost::optional masterKey = - app.validators().getTrustedKey(signingKey); + auto masterKey = app.validators().getTrustedKey(signingKey); if (!val->isTrusted() && masterKey) val->setTrusted(); @@ -172,13 +171,15 @@ handleNewValidation( beast::Journal const j = validations.adaptor().journal(); auto dmp = [&](beast::Journal::Stream s, std::string const& msg) { - s << "Val for " << hash - << (val->isTrusted() ? " trusted/" : " UNtrusted/") - << (val->isFull() ? "full" : "partial") << " from " - << (masterKey ? toBase58(TokenType::NodePublic, *masterKey) - : "unknown") - << " signing key " << toBase58(TokenType::NodePublic, signingKey) - << " " << msg << " src=" << source; + std::string id = toBase58(TokenType::NodePublic, signingKey); + + if (masterKey) + id += ":" + toBase58(TokenType::NodePublic, *masterKey); + + s << (val->isTrusted() ? "trusted" : "untrusted") << " " + << (val->isFull() ? "full" : "partial") << " validation: " << hash + << " from " << id << " via " << source << ": " << msg << "\n" + << " [" << val->getSerializer().getHex() << "]"; }; if (!val->isFieldPresent(sfLedgerSequence)) @@ -192,13 +193,30 @@ handleNewValidation( if (masterKey) { ValStatus const outcome = validations.add(calcNodeID(*masterKey), val); + if (j.debug()) dmp(j.debug(), to_string(outcome)); + if (outcome == ValStatus::conflicting && j.warn()) + { + auto const seq = val->getFieldU32(sfLedgerSequence); + dmp(j.warn(), + "conflicting validations issued for " + to_string(seq) + + " (likely from a Byzantine validator)"); + } + + if (outcome == ValStatus::multiple && j.warn()) + { + auto const seq = val->getFieldU32(sfLedgerSequence); + dmp(j.warn(), + "multiple validations issued for " + to_string(seq) + + " (multiple validators operating with the same key?)"); + } + if (outcome == ValStatus::badSeq && j.warn()) { auto const seq = val->getFieldU32(sfLedgerSequence); - dmp(j.warn(), + dmp(j.debug(), "already validated sequence at or past " + std::to_string(seq)); } diff --git a/src/ripple/app/consensus/RCLValidations.h b/src/ripple/app/consensus/RCLValidations.h index 4cce72f03..3f0245d67 100644 --- a/src/ripple/app/consensus/RCLValidations.h +++ b/src/ripple/app/consensus/RCLValidations.h @@ -33,12 +33,11 @@ class Application; /** Wrapper over STValidation for generic Validation code - Wraps an STValidation::pointer for compatibility with the generic validation - code. + Wraps an STValidation for compatibility with the generic validation code. */ class RCLValidation { - STValidation::pointer val_; + std::shared_ptr val_; public: using NodeKey = ripple::PublicKey; @@ -48,7 +47,7 @@ public: @param v The validation to wrap. */ - RCLValidation(STValidation::pointer const& v) : val_{v} + RCLValidation(std::shared_ptr const& v) : val_{v} { } @@ -127,8 +126,15 @@ public: return ~(*val_)[~sfLoadFee]; } + /// Get the cookie specified in the validation (0 if not set) + std::uint64_t + cookie() const + { + return (*val_)[sfCookie]; + } + /// Extract the underlying STValidation being wrapped - STValidation::pointer + std::shared_ptr unwrap() const { return val_; @@ -243,7 +249,7 @@ using RCLValidations = Validations; bool handleNewValidation( Application& app, - STValidation::ref val, + std::shared_ptr const& val, std::string const& source); } // namespace ripple diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index c56a1d58b..22e226b66 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -103,7 +103,7 @@ public: NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, - std::vector const& valSet) = 0; + std::vector> const& valSet) = 0; // Called by the consensus code when we need to // add feature entries to a validation @@ -126,7 +126,7 @@ public: void doVoting( std::shared_ptr const& lastClosedLedger, - std::vector const& parentValidations, + std::vector> const& parentValidations, std::shared_ptr const& initialPosition) { // Ask implementation what to do diff --git a/src/ripple/app/misc/FeeVote.h b/src/ripple/app/misc/FeeVote.h index abcb02cce..543f4cdb6 100644 --- a/src/ripple/app/misc/FeeVote.h +++ b/src/ripple/app/misc/FeeVote.h @@ -60,9 +60,7 @@ public: @param baseValidation */ virtual void - doValidation( - std::shared_ptr const& lastClosedLedger, - STValidation::FeeSettings& fees) = 0; + doValidation(Fees const& lastFees, STValidation& val) = 0; /** Cast our local vote on the fee. @@ -72,7 +70,7 @@ public: virtual void doVoting( std::shared_ptr const& lastClosedLedger, - std::vector const& parentValidations, + std::vector> const& parentValidations, std::shared_ptr const& initialPosition) = 0; }; diff --git a/src/ripple/app/misc/FeeVoteImpl.cpp b/src/ripple/app/misc/FeeVoteImpl.cpp index cb48dc29b..ce3da19b2 100644 --- a/src/ripple/app/misc/FeeVoteImpl.cpp +++ b/src/ripple/app/misc/FeeVoteImpl.cpp @@ -93,14 +93,12 @@ public: FeeVoteImpl(Setup const& setup, beast::Journal journal); void - doValidation( - std::shared_ptr const& lastClosedLedger, - STValidation::FeeSettings& fees) override; + doValidation(Fees const& lastFees, STValidation& val) override; void doVoting( std::shared_ptr const& lastClosedLedger, - std::vector const& parentValidations, + std::vector> const& parentValidations, std::shared_ptr const& initialPosition) override; }; @@ -112,39 +110,43 @@ FeeVoteImpl::FeeVoteImpl(Setup const& setup, beast::Journal journal) } void -FeeVoteImpl::doValidation( - std::shared_ptr const& lastClosedLedger, - STValidation::FeeSettings& fees) +FeeVoteImpl::doValidation(Fees const& lastFees, STValidation& v) { - if (lastClosedLedger->fees().base != target_.reference_fee) + // Values should always be in a valid range (because the voting process + // will ignore out-of-range values) but if we detect such a case, we do + // not send a value. + if (lastFees.base != target_.reference_fee) { JLOG(journal_.info()) << "Voting for base fee of " << target_.reference_fee; - fees.baseFee = target_.reference_fee; + if (auto const f = target_.reference_fee.dropsAs()) + v.setFieldU64(sfBaseFee, *f); } - if (lastClosedLedger->fees().accountReserve(0) != target_.account_reserve) + if (lastFees.accountReserve(0) != target_.account_reserve) { JLOG(journal_.info()) << "Voting for base reserve of " << target_.account_reserve; - fees.reserveBase = target_.account_reserve; + if (auto const f = target_.account_reserve.dropsAs()) + v.setFieldU32(sfReserveBase, *f); } - if (lastClosedLedger->fees().increment != target_.owner_reserve) + if (lastFees.increment != target_.owner_reserve) { JLOG(journal_.info()) << "Voting for reserve increment of " << target_.owner_reserve; - fees.reserveIncrement = target_.owner_reserve; + if (auto const f = target_.owner_reserve.dropsAs()) + v.setFieldU32(sfReserveIncrement, *f); } } void FeeVoteImpl::doVoting( std::shared_ptr const& lastClosedLedger, - std::vector const& set, + std::vector> const& set, std::shared_ptr const& initialPosition) { // LCL must be flag ledger diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 077e0ac1a..359f2cd15 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -362,7 +362,9 @@ public: std::shared_ptr set) override; bool - recvValidation(STValidation::ref val, std::string const& source) override; + recvValidation( + std::shared_ptr const& val, + std::string const& source) override; std::shared_ptr getTXMap(uint256 const& hash); @@ -550,7 +552,7 @@ public: std::shared_ptr const& stTxn, TER terResult) override; void - pubValidation(STValidation::ref val) override; + pubValidation(std::shared_ptr const& val) override; //-------------------------------------------------------------------------- // @@ -2047,7 +2049,7 @@ NetworkOPsImp::pubConsensus(ConsensusPhase phase) } void -NetworkOPsImp::pubValidation(STValidation::ref val) +NetworkOPsImp::pubValidation(std::shared_ptr const& val) { // VFALCO consider std::shared_mutex std::lock_guard sl(mSubLock); @@ -2473,7 +2475,9 @@ NetworkOPsImp::getTxsAccountB( } bool -NetworkOPsImp::recvValidation(STValidation::ref val, std::string const& source) +NetworkOPsImp::recvValidation( + std::shared_ptr const& val, + std::string const& source) { JLOG(m_journal.debug()) << "recvValidation " << val->getLedgerHash() << " from " << source; diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index e27f25cc4..630d32c7e 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -175,7 +175,9 @@ public: std::shared_ptr set) = 0; virtual bool - recvValidation(STValidation::ref val, std::string const& source) = 0; + recvValidation( + std::shared_ptr const& val, + std::string const& source) = 0; virtual void mapComplete(std::shared_ptr const& map, bool fromAcquire) = 0; @@ -309,7 +311,7 @@ public: std::shared_ptr const& stTxn, TER terResult) = 0; virtual void - pubValidation(STValidation::ref val) = 0; + pubValidation(std::shared_ptr const& val) = 0; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index bf6ee723e..099d656fe 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -232,7 +232,7 @@ public: NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, - std::vector const& validations) override; + std::vector> const& validations) override; }; //------------------------------------------------------------------------------ @@ -455,7 +455,7 @@ AmendmentTableImpl::doVoting( NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, - std::vector const& valSet) + std::vector> const& valSet) { JLOG(j_.trace()) << "voting at " << closeTime.time_since_epoch().count() << ": " << enabledAmendments.size() << ", " diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index 17a9bb4c9..8626d6806 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -167,7 +167,11 @@ enum class ValStatus { /// Not current or was older than current from this node stale, /// A validation violates the increasing seq requirement - badSeq + badSeq, + /// Multiple validations for the same ledger from multiple validators + multiple, + /// Multiple validations for different ledgers by a single validator + conflicting }; inline std::string @@ -181,6 +185,10 @@ to_string(ValStatus m) return "stale"; case ValStatus::badSeq: return "badSeq"; + case ValStatus::multiple: + return "multiple"; + case ValStatus::conflicting: + return "conflicting"; default: return "unknown"; } @@ -306,6 +314,14 @@ class Validations beast::uhash<>> byLedger_; + // Partial and full validations indexed by sequence + beast::aged_unordered_map< + Seq, + hash_map, + std::chrono::steady_clock, + beast::uhash<>> + bySequence_; + // Represents the ancestry of validated ledgers LedgerTrie trie_; @@ -546,7 +562,10 @@ public: ValidationParms const& p, beast::abstract_clock& c, Ts&&... ts) - : byLedger_(c), parms_(p), adaptor_(std::forward(ts)...) + : byLedger_(c) + , bySequence_(c) + , parms_(p) + , adaptor_(std::forward(ts)...) { } @@ -598,11 +617,48 @@ public: std::lock_guard lock{mutex_}; // Check that validation sequence is greater than any non-expired - // validations sequence from that validator + // validations sequence from that validator; if it's not, perform + // additional work to detect Byzantine validations auto const now = byLedger_.clock().now(); - SeqEnforcer& enforcer = seqEnforcers_[nodeID]; - if (!enforcer(now, val.seq(), parms_)) + + auto const [seqit, seqinserted] = + bySequence_[val.seq()].emplace(nodeID, val); + + if (!seqinserted) + { + // Check if the entry we're already tracking was signed + // long enough ago that we can disregard it. + auto const diff = + std::max(seqit->second.signTime(), val.signTime()) - + std::min(seqit->second.signTime(), val.signTime()); + + if (diff > parms_.validationCURRENT_WALL && + val.signTime() > seqit->second.signTime()) + seqit->second = val; + } + + // Enforce monotonically increasing sequences for validations + // by a given node: + if (auto& enf = seqEnforcers_[nodeID]; !enf(now, val.seq(), parms_)) + { + // If the validation is for the same sequence as one we are + // tracking, check it closely: + if (seqit->second.seq() == val.seq()) + { + // Two validations for the same sequence but for different + // ledgers. This could be the result of misconfiguration + // but it can also mean a Byzantine validator. + if (seqit->second.ledgerID() != val.ledgerID()) + return ValStatus::conflicting; + + // Two validations for the same sequence but with different + // cookies. This is probably accidental misconfiguration. + if (seqit->second.cookie() != val.cookie()) + return ValStatus::multiple; + } + return ValStatus::badSeq; + } byLedger_[val.ledgerID()].insert_or_assign(nodeID, val); @@ -626,6 +682,7 @@ public: updateTrie(lock, nodeID, val, boost::none); } } + return ValStatus::current; } @@ -639,6 +696,7 @@ public: { std::lock_guard lock{mutex_}; beast::expire(byLedger_, parms_.validationSET_EXPIRES); + beast::expire(bySequence_, parms_.validationSET_EXPIRES); } /** Update trust status of validations diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 61f749396..ada1916b2 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -2117,7 +2117,7 @@ PeerImp::onMessage(std::shared_ptr const& m) try { - STValidation::pointer val; + std::shared_ptr val; { SerialIter sit(makeSlice(m->validation())); val = std::make_shared( @@ -2453,7 +2453,6 @@ PeerImp::checkPropose( << "Checking " << (isTrusted ? "trusted" : "UNTRUSTED") << " proposal"; assert(packet); - protocol::TMProposeSet& set = *packet; if (!cluster() && !peerPos.checkSign()) { @@ -2474,7 +2473,7 @@ PeerImp::checkPropose( { // relay untrusted proposal JLOG(p_journal_.trace()) << "relaying UNTRUSTED proposal"; - overlay_.relay(set, peerPos.suppressionID()); + overlay_.relay(*packet, peerPos.suppressionID()); } else { @@ -2485,7 +2484,7 @@ PeerImp::checkPropose( void PeerImp::checkValidation( - STValidation::pointer val, + std::shared_ptr const& val, std::shared_ptr const& packet) { try diff --git a/src/ripple/overlay/impl/PeerImp.h b/src/ripple/overlay/impl/PeerImp.h index e302c6a60..ef30c5c00 100644 --- a/src/ripple/overlay/impl/PeerImp.h +++ b/src/ripple/overlay/impl/PeerImp.h @@ -592,7 +592,7 @@ private: void checkValidation( - STValidation::pointer val, + std::shared_ptr const& val, std::shared_ptr const& packet); void diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index a325de309..6da2cc559 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -111,7 +111,7 @@ class FeatureCollections "RequireFullyCanonicalSig", "fix1781", // XRPEndpointSteps should be included in the circular // payment check - }; + "HardenedValidations"}; std::vector features; boost::container::flat_map featureToIndex; @@ -366,6 +366,7 @@ extern uint256 const featureDeletableAccounts; extern uint256 const fixQualityUpperBound; extern uint256 const featureRequireFullyCanonicalSig; extern uint256 const fix1781; +extern uint256 const featureHardenedValidations; // The following amendments have been active for at least two years. // Their pre-amendment code has been removed. diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 3ad477899..38628f852 100644 --- a/src/ripple/protocol/SField.h +++ b/src/ripple/protocol/SField.h @@ -432,6 +432,7 @@ extern SF_U256 const sfDigest; extern SF_U256 const sfPayChannel; extern SF_U256 const sfConsensusHash; extern SF_U256 const sfCheckID; +extern SF_U256 const sfValidatedHash; // currency amount (common) extern SF_Amount const sfAmount; diff --git a/src/ripple/protocol/SOTemplate.h b/src/ripple/protocol/SOTemplate.h index 45a10bbab..4c0ad0b3f 100644 --- a/src/ripple/protocol/SOTemplate.h +++ b/src/ripple/protocol/SOTemplate.h @@ -50,7 +50,13 @@ public: : sField_(fieldName), style_(style) { if (!sField_.get().isUseful()) - Throw("SField in SOElement must be useful."); + { + auto nm = std::to_string(fieldName.getCode()); + if (fieldName.hasName()) + nm += ": '" + fieldName.getName() + "'"; + Throw( + "SField (" + nm + ") in SOElement must be useful."); + } } SField const& diff --git a/src/ripple/protocol/STValidation.h b/src/ripple/protocol/STValidation.h index 35a313e93..841086000 100644 --- a/src/ripple/protocol/STValidation.h +++ b/src/ripple/protocol/STValidation.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,8 +33,12 @@ namespace ripple { // Validation flags -const std::uint32_t vfFullyCanonicalSig = - 0x80000000; // signature is fully canonical + +// This is a full (as opposed to a partial) validation +constexpr std::uint32_t vfFullValidation = 0x00000001; + +// The signature is fully canonical +constexpr std::uint32_t vfFullyCanonicalSig = 0x80000000; class STValidation final : public STObject, public CountedObject { @@ -44,11 +49,6 @@ public: return "STValidation"; } - using pointer = std::shared_ptr; - using ref = const std::shared_ptr&; - - enum { kFullFlag = 0x1 }; - /** Construct a STValidation from a peer. Construct a STValidation from serialized data previously shared by a @@ -70,71 +70,63 @@ public: SerialIter& sit, LookupNodeID&& lookupNodeID, bool checkSignature) - : STObject(getFormat(), sit, sfValidation) + : STObject(validationFormat(), sit, sfValidation) { auto const spk = getFieldVL(sfSigningPubKey); if (publicKeyType(makeSlice(spk)) != KeyType::secp256k1) { - JLOG(debugLog().error()) << "Invalid public key in validation" + JLOG(debugLog().error()) << "Invalid public key in validation: " << getJson(JsonOptions::none); Throw("Invalid public key in validation"); } if (checkSignature && !isValid()) { - JLOG(debugLog().error()) << "Invalid signature in validation" + JLOG(debugLog().error()) << "Invalid signature in validation: " << getJson(JsonOptions::none); Throw("Invalid signature in validation"); } - mNodeID = lookupNodeID(PublicKey(makeSlice(spk))); - assert(mNodeID.isNonZero()); + nodeID_ = lookupNodeID(PublicKey(makeSlice(spk))); + assert(nodeID_.isNonZero()); } - /** Fees to set when issuing a new validation. + /** Construct, sign and trust a new STValidation issued by this node. - 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 - - @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. + @param f callback function to "fill" the validation with necessary data */ + template STValidation( - uint256 const& ledgerHash, - std::uint32_t ledgerSeq, - uint256 const& consensusHash, NetClock::time_point signTime, - PublicKey const& publicKey, - SecretKey const& secretKey, + PublicKey const& pk, + SecretKey const& sk, NodeID const& nodeID, - bool isFull, - FeeSettings const& fees, - std::vector const& amendments); + F&& f) + : STObject(validationFormat(), sfValidation) + , nodeID_(nodeID) + , seenTime_(signTime) + { + // First, set our own public key: + if (publicKeyType(pk) != KeyType::secp256k1) + LogicError( + "We can only use secp256k1 keys for signing validations"); + + setFieldVL(sfSigningPubKey, pk.slice()); + setFieldU32(sfSigningTime, signTime.time_since_epoch().count()); + + // Perform additional initialization + f(*this); + + // Finally, sign the validation and mark it as trusted: + setFlag(vfFullyCanonicalSig); + setFieldVL(sfSignature, signDigest(pk, sk, getSigningHash())); + setTrusted(); + } STBase* copy(std::size_t n, void* buf) const override @@ -168,7 +160,7 @@ public: NodeID getNodeID() const { - return mNodeID; + return nodeID_; } bool @@ -201,7 +193,7 @@ public: void setSeen(NetClock::time_point s) { - mSeen = s; + seenTime_ = s; } Blob @@ -212,11 +204,11 @@ public: private: static SOTemplate const& - getFormat(); + validationFormat(); - NodeID mNodeID; + NodeID nodeID_; bool mTrusted = false; - NetClock::time_point mSeen = {}; + NetClock::time_point seenTime_ = {}; }; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 0ea3085a8..521d34bdf 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -130,7 +130,7 @@ detail::supportedAmendments() "fixQualityUpperBound", "RequireFullyCanonicalSig", "fix1781", - }; + "HardenedValidations"}; return supported; } @@ -187,6 +187,8 @@ uint256 const fixQualityUpperBound = uint256 const featureRequireFullyCanonicalSig = *getRegisteredFeature("RequireFullyCanonicalSig"); uint256 const fix1781 = *getRegisteredFeature("fix1781"); +uint256 const featureHardenedValidations = + *getRegisteredFeature("HardenedValidations"); // The following amendments have been active for at least two years. // Their pre-amendment code has been removed. diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index eea1e24be..06a85ca6b 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/src/ripple/protocol/impl/SField.cpp @@ -168,6 +168,7 @@ SF_U256 const sfDigest(access, STI_HASH256, 21, "Digest"); SF_U256 const sfPayChannel(access, STI_HASH256, 22, "Channel"); SF_U256 const sfConsensusHash(access, STI_HASH256, 23, "ConsensusHash"); SF_U256 const sfCheckID(access, STI_HASH256, 24, "CheckID"); +SF_U256 const sfValidatedHash(access, STI_HASH256, 25, "ValidatedHash"); // currency amount (common) SF_Amount const sfAmount(access, STI_AMOUNT, 1, "Amount"); diff --git a/src/ripple/protocol/impl/STValidation.cpp b/src/ripple/protocol/impl/STValidation.cpp index 9de595a96..4612baf69 100644 --- a/src/ripple/protocol/impl/STValidation.cpp +++ b/src/ripple/protocol/impl/STValidation.cpp @@ -25,68 +25,32 @@ namespace ripple { -STValidation::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, - FeeSettings const& fees, - std::vector const& amendments) - : STObject(getFormat(), sfValidation), mNodeID(nodeID), mSeen(signTime) +SOTemplate const& +STValidation::validationFormat() { - // This is our own public key and it should always be valid. - if (!publicKeyType(publicKey)) - LogicError("Invalid validation public key"); - assert(mNodeID.isNonZero()); - setFieldH256(sfLedgerHash, ledgerHash); - setFieldH256(sfConsensusHash, consensusHash); - setFieldU32(sfSigningTime, signTime.time_since_epoch().count()); + // We can't have this be a magic static at namespace scope because + // it relies on the SField's below being initialized, and we can't + // guarantee the initialization order. + static SOTemplate const format{ + {sfFlags, soeREQUIRED}, + {sfLedgerHash, soeREQUIRED}, + {sfLedgerSequence, soeOPTIONAL}, + {sfCloseTime, soeOPTIONAL}, + {sfLoadFee, soeOPTIONAL}, + {sfAmendments, soeOPTIONAL}, + {sfBaseFee, soeOPTIONAL}, + {sfReserveBase, soeOPTIONAL}, + {sfReserveIncrement, soeOPTIONAL}, + {sfSigningTime, soeREQUIRED}, + {sfSigningPubKey, soeREQUIRED}, + {sfSignature, soeOPTIONAL}, + {sfConsensusHash, soeOPTIONAL}, + {sfCookie, soeDEFAULT}, + {sfValidatedHash, soeOPTIONAL}, + }; - setFieldVL(sfSigningPubKey, publicKey.slice()); - if (isFull) - setFlag(kFullFlag); - - setFieldU32(sfLedgerSequence, ledgerSeq); - - if (fees.loadFee) - setFieldU32(sfLoadFee, *fees.loadFee); - - // IF any of the values are out of the valid range, don't send a value. - // They should not be an issue, though, because the voting - // process (FeeVoteImpl) ignores any out of range values. - if (fees.baseFee) - { - if (auto const v = fees.baseFee->dropsAs()) - setFieldU64(sfBaseFee, *v); - } - - if (fees.reserveBase) - { - if (auto const v = fees.reserveBase->dropsAs()) - setFieldU32(sfReserveBase, *v); - } - - if (fees.reserveIncrement) - { - if (auto const v = fees.reserveIncrement->dropsAs()) - setFieldU32(sfReserveIncrement, *v); - } - - if (!amendments.empty()) - setFieldV256(sfAmendments, STVector256(sfAmendments, amendments)); - - setFlag(vfFullyCanonicalSig); - - auto const signingHash = getSigningHash(); - setFieldVL( - sfSignature, signDigest(getSignerPublic(), secretKey, signingHash)); - - setTrusted(); -} + return format; +}; uint256 STValidation::getSigningHash() const @@ -115,7 +79,7 @@ STValidation::getSignTime() const NetClock::time_point STValidation::getSeenTime() const { - return mSeen; + return seenTime_; } bool @@ -148,7 +112,7 @@ STValidation::getSignerPublic() const bool STValidation::isFull() const { - return (getFlags() & kFullFlag) != 0; + return (getFlags() & vfFullValidation) != 0; } Blob @@ -165,32 +129,4 @@ STValidation::getSerialized() const return s.peekData(); } -SOTemplate const& -STValidation::getFormat() -{ - struct FormatHolder - { - SOTemplate format{ - {sfFlags, soeREQUIRED}, - {sfLedgerHash, soeREQUIRED}, - {sfLedgerSequence, soeOPTIONAL}, - {sfCloseTime, soeOPTIONAL}, - {sfLoadFee, soeOPTIONAL}, - {sfAmendments, soeOPTIONAL}, - {sfBaseFee, soeOPTIONAL}, - {sfReserveBase, soeOPTIONAL}, - {sfReserveIncrement, soeOPTIONAL}, - {sfSigningTime, soeREQUIRED}, - {sfSigningPubKey, soeREQUIRED}, - {sfSignature, soeOPTIONAL}, - {sfConsensusHash, soeOPTIONAL}, - {sfCookie, soeOPTIONAL}, - }; - }; - - static const FormatHolder holder; - - return holder.format; -} - } // namespace ripple diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 0fbba0192..f0e9f401a 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -372,7 +373,7 @@ public: auto const roundTime = weekTime(week); // Build validations - std::vector validations; + std::vector> validations; validations.reserve(validators.size()); int i = 0; @@ -391,16 +392,15 @@ public: } auto v = std::make_shared( - uint256(), - i, - uint256(), - roundTime, + ripple::NetClock::time_point{}, val.first, val.second, calcNodeID(val.first), - true, - STValidation::FeeSettings{}, - field); + [&field](STValidation& v) { + if (!field.empty()) + v.setFieldV256( + sfAmendments, STVector256(sfAmendments, field)); + }); validations.emplace_back(v); } diff --git a/src/test/app/RCLValidations_test.cpp b/src/test/app/RCLValidations_test.cpp index d6190ca6d..a84b44c30 100644 --- a/src/test/app/RCLValidations_test.cpp +++ b/src/test/app/RCLValidations_test.cpp @@ -35,16 +35,11 @@ class RCLValidations_test : public beast::unit_test::suite testcase("Change validation trusted status"); auto keys = randomKeyPair(KeyType::secp256k1); auto v = std::make_shared( - uint256(), - 1, - uint256(), - NetClock::time_point(), + ripple::NetClock::time_point{}, keys.first, keys.second, calcNodeID(keys.first), - true, - STValidation::FeeSettings{}, - std::vector{}); + [&](STValidation& v) {}); BEAST_EXPECT(v->isTrusted()); v->setUntrusted(); diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index 3bc0a3b3b..9455d3931 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/src/test/consensus/Validations_test.cpp @@ -54,7 +54,7 @@ class Validations_test : public beast::unit_test::suite clock_type const& c_; PeerID nodeID_; bool trusted_ = true; - std::size_t signIdx_ = 1; + std::size_t signIdx_{1}; boost::optional loadFee_; public: @@ -394,7 +394,7 @@ class Validations_test : public beast::unit_test::suite // If we advance far enough for AB to expire, we can fully // validate or partially validate that sequence number again - BEAST_EXPECT(ValStatus::badSeq == process(ledgerAZ)); + BEAST_EXPECT(ValStatus::conflicting == process(ledgerAZ)); harness.clock().advance( harness.parms().validationSET_EXPIRES + 1ms); BEAST_EXPECT(ValStatus::current == process(ledgerAZ)); @@ -683,8 +683,10 @@ class Validations_test : public beast::unit_test::suite trustedValidations[val.ledgerID()].emplace_back(val); } // d now thinks ledger 1, but cannot re-issue a previously used seq + // and attempting it should generate a conflict. { - BEAST_EXPECT(ValStatus::badSeq == harness.add(d.partial(ledgerA))); + BEAST_EXPECT( + ValStatus::conflicting == harness.add(d.partial(ledgerA))); } // e only issues partials { diff --git a/src/test/csf/Validation.h b/src/test/csf/Validation.h index 8d73b4ffe..490c082cd 100644 --- a/src/test/csf/Validation.h +++ b/src/test/csf/Validation.h @@ -55,6 +55,7 @@ class Validation bool trusted_ = false; bool full_ = false; boost::optional loadFee_; + std::uint64_t cookie_{0}; public: using NodeKey = PeerKey; @@ -68,7 +69,8 @@ public: PeerKey key, PeerID nodeID, bool full, - boost::optional loadFee = boost::none) + boost::optional loadFee = boost::none, + std::uint64_t cookie = 0) : ledgerID_{id} , seq_{seq} , signTime_{sign} @@ -77,6 +79,7 @@ public: , nodeID_{nodeID} , full_{full} , loadFee_{loadFee} + , cookie_{cookie} { } @@ -128,6 +131,12 @@ public: return full_; } + std::uint64_t + cookie() const + { + return cookie_; + } + boost::optional loadFee() const { @@ -191,4 +200,5 @@ public: } // namespace csf } // namespace test } // namespace ripple -#endif \ No newline at end of file + +#endif diff --git a/src/test/rpc/Subscribe_test.cpp b/src/test/rpc/Subscribe_test.cpp index 741796973..22a4afb29 100644 --- a/src/test/rpc/Subscribe_test.cpp +++ b/src/test/rpc/Subscribe_test.cpp @@ -370,6 +370,7 @@ public: // Check stream update using namespace std::chrono_literals; + BEAST_EXPECT(wsc->findMsg(5s, [&](auto const& jv) { return jv[jss::type] == "validationReceived" && jv[jss::validation_public_key].asString() == valPublicKey && @@ -378,7 +379,7 @@ public: jv[jss::ledger_index] == std::to_string(env.closed()->info().seq) && jv[jss::flags] == - (vfFullyCanonicalSig | STValidation::kFullFlag) && + (vfFullyCanonicalSig | vfFullValidation) && jv[jss::full] == true && !jv.isMember(jss::load_fee) && jv[jss::signature] && jv[jss::signing_time]; }));