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

@@ -32,6 +32,7 @@
 #include <ripple/app/misc/TxQ.h>
 #include <ripple/app/misc/ValidatorKeys.h>
 #include <ripple/app/misc/ValidatorList.h>
+#include <ripple/basics/random.h>
 #include <ripple/beast/core/LexicalCast.h>
 #include <ripple/consensus/LedgerTiming.h>
 #include <ripple/nodestore/DatabaseShard.h>
@@ -85,7 +86,14 @@ RCLConsensus::Adaptor::Adaptor(
     , nodeID_{validatorKeys.nodeID}
     , valPublic_{validatorKeys.publicKey}
     , valSecret_{validatorKeys.secretKey}
+    , valCookie_{
+          rand_int<std::uint64_t>(1, std::numeric_limits<std::uint64_t>::max())}
 {
+    assert(valCookie_ != 0);
+
+    JLOG(j_.info()) << "Consensus engine started"
+                    << " (Node: " << to_string(nodeID_)
+                    << ", Cookie: " << valCookie_ << ")";
 }
 
 boost::optional<RCLCxLedger>
@@ -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<uint256> 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<STValidation>(
-        ledger.id(),
+        lastValidationTime_,
-        ledger.seq(),
-        txns.id(),
-        validationTime,
         valPublic_,
         valSecret_,
         nodeID_,
-        proposing /* full if proposed */,
+        [&](STValidation& v) {
-        fees,
+            v.setFieldH256(sfLedgerHash, ledger.id());
-        amendments);
+            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(

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_;

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<STValidation> 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<PublicKey> masterKey =
+    auto masterKey = app.validators().getTrustedKey(signingKey);
-        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
+        std::string id = toBase58(TokenType::NodePublic, signingKey);
-          << (val->isTrusted() ? " trusted/" : " UNtrusted/")
+
-          << (val->isFull() ? "full" : "partial") << " from "
+        if (masterKey)
-          << (masterKey ? toBase58(TokenType::NodePublic, *masterKey)
+            id += ":" + toBase58(TokenType::NodePublic, *masterKey);
-                        : "unknown")
+
-          << " signing key " << toBase58(TokenType::NodePublic, signingKey)
+        s << (val->isTrusted() ? "trusted" : "untrusted") << " "
-          << " " << msg << " src=" << source;
+          << (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));
         }
 

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
+    Wraps an STValidation for compatibility with the generic validation code.
-    code.
 */
 class RCLValidation
 {
-    STValidation::pointer val_;
+    std::shared_ptr<STValidation> 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<STValidation> 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<STValidation>
     unwrap() const
     {
         return val_;
@@ -243,7 +249,7 @@ using RCLValidations = Validations<RCLValidationsAdaptor>;
 bool
 handleNewValidation(
     Application& app,
-    STValidation::ref val,
+    std::shared_ptr<STValidation> const& val,
     std::string const& source);
 
 }  // namespace ripple

src/ripple/app/misc/AmendmentTable.h

@@ -103,7 +103,7 @@ public:
         NetClock::time_point closeTime,
         std::set<uint256> const& enabledAmendments,
         majorityAmendments_t const& majorityAmendments,
-        std::vector<STValidation::pointer> const& valSet) = 0;
+        std::vector<std::shared_ptr<STValidation>> 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<ReadView const> const& lastClosedLedger,
-        std::vector<STValidation::pointer> const& parentValidations,
+        std::vector<std::shared_ptr<STValidation>> const& parentValidations,
         std::shared_ptr<SHAMap> const& initialPosition)
     {
         // Ask implementation what to do

src/ripple/app/misc/FeeVote.h

@@ -60,9 +60,7 @@ public:
         @param baseValidation
     */
     virtual void
-    doValidation(
+    doValidation(Fees const& lastFees, STValidation& val) = 0;
-        std::shared_ptr<ReadView const> const& lastClosedLedger,
-        STValidation::FeeSettings& fees) = 0;
 
     /** Cast our local vote on the fee.
 
@@ -72,7 +70,7 @@ public:
     virtual void
     doVoting(
         std::shared_ptr<ReadView const> const& lastClosedLedger,
-        std::vector<STValidation::pointer> const& parentValidations,
+        std::vector<std::shared_ptr<STValidation>> const& parentValidations,
         std::shared_ptr<SHAMap> const& initialPosition) = 0;
 };
 

src/ripple/app/misc/FeeVoteImpl.cpp

@@ -93,14 +93,12 @@ public:
     FeeVoteImpl(Setup const& setup, beast::Journal journal);
 
     void
-    doValidation(
+    doValidation(Fees const& lastFees, STValidation& val) override;
-        std::shared_ptr<ReadView const> const& lastClosedLedger,
-        STValidation::FeeSettings& fees) override;
 
     void
     doVoting(
         std::shared_ptr<ReadView const> const& lastClosedLedger,
-        std::vector<STValidation::pointer> const& parentValidations,
+        std::vector<std::shared_ptr<STValidation>> const& parentValidations,
         std::shared_ptr<SHAMap> const& initialPosition) override;
 };
 
@@ -112,39 +110,43 @@ FeeVoteImpl::FeeVoteImpl(Setup const& setup, beast::Journal journal)
 }
 
 void
-FeeVoteImpl::doValidation(
+FeeVoteImpl::doValidation(Fees const& lastFees, STValidation& v)
-    std::shared_ptr<ReadView const> const& lastClosedLedger,
-    STValidation::FeeSettings& fees)
 {
-    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<std::uint64_t>())
+            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<std::uint32_t>())
+            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<std::uint32_t>())
+            v.setFieldU32(sfReserveIncrement, *f);
     }
 }
 
 void
 FeeVoteImpl::doVoting(
     std::shared_ptr<ReadView const> const& lastClosedLedger,
-    std::vector<STValidation::pointer> const& set,
+    std::vector<std::shared_ptr<STValidation>> const& set,
     std::shared_ptr<SHAMap> const& initialPosition)
 {
     // LCL must be flag ledger

src/ripple/app/misc/NetworkOPs.cpp

@@ -362,7 +362,9 @@ public:
         std::shared_ptr<protocol::TMProposeSet> set) override;
 
     bool
-    recvValidation(STValidation::ref val, std::string const& source) override;
+    recvValidation(
+        std::shared_ptr<STValidation> const& val,
+        std::string const& source) override;
 
     std::shared_ptr<SHAMap>
     getTXMap(uint256 const& hash);
@@ -550,7 +552,7 @@ public:
         std::shared_ptr<STTx const> const& stTxn,
         TER terResult) override;
     void
-    pubValidation(STValidation::ref val) override;
+    pubValidation(std::shared_ptr<STValidation> const& val) override;
 
     //--------------------------------------------------------------------------
     //
@@ -2047,7 +2049,7 @@ NetworkOPsImp::pubConsensus(ConsensusPhase phase)
 }
 
 void
-NetworkOPsImp::pubValidation(STValidation::ref val)
+NetworkOPsImp::pubValidation(std::shared_ptr<STValidation> 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<STValidation> const& val,
+    std::string const& source)
 {
     JLOG(m_journal.debug())
         << "recvValidation " << val->getLedgerHash() << " from " << source;

src/ripple/app/misc/NetworkOPs.h

@@ -175,7 +175,9 @@ public:
         std::shared_ptr<protocol::TMProposeSet> set) = 0;
 
     virtual bool
-    recvValidation(STValidation::ref val, std::string const& source) = 0;
+    recvValidation(
+        std::shared_ptr<STValidation> const& val,
+        std::string const& source) = 0;
 
     virtual void
     mapComplete(std::shared_ptr<SHAMap> const& map, bool fromAcquire) = 0;
@@ -309,7 +311,7 @@ public:
         std::shared_ptr<STTx const> const& stTxn,
         TER terResult) = 0;
     virtual void
-    pubValidation(STValidation::ref val) = 0;
+    pubValidation(std::shared_ptr<STValidation> const& val) = 0;
 };
 
 //------------------------------------------------------------------------------

src/ripple/app/misc/impl/AmendmentTable.cpp

@@ -232,7 +232,7 @@ public:
         NetClock::time_point closeTime,
         std::set<uint256> const& enabledAmendments,
         majorityAmendments_t const& majorityAmendments,
-        std::vector<STValidation::pointer> const& validations) override;
+        std::vector<std::shared_ptr<STValidation>> const& validations) override;
 };
 
 //------------------------------------------------------------------------------
@@ -455,7 +455,7 @@ AmendmentTableImpl::doVoting(
     NetClock::time_point closeTime,
     std::set<uint256> const& enabledAmendments,
     majorityAmendments_t const& majorityAmendments,
-    std::vector<STValidation::pointer> const& valSet)
+    std::vector<std::shared_ptr<STValidation>> const& valSet)
 {
     JLOG(j_.trace()) << "voting at " << closeTime.time_since_epoch().count()
                      << ": " << enabledAmendments.size() << ", "

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<NodeID, Validation>,
+        std::chrono::steady_clock,
+        beast::uhash<>>
+        bySequence_;
+
     // Represents the ancestry of validated ledgers
     LedgerTrie<Ledger> trie_;
 
@@ -546,7 +562,10 @@ public:
         ValidationParms const& p,
         beast::abstract_clock<std::chrono::steady_clock>& c,
         Ts&&... ts)
-        : byLedger_(c), parms_(p), adaptor_(std::forward<Ts>(ts)...)
+        : byLedger_(c)
+        , bySequence_(c)
+        , parms_(p)
+        , adaptor_(std::forward<Ts>(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<Seq>& 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

src/ripple/overlay/impl/PeerImp.cpp

@@ -2117,7 +2117,7 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMValidation> const& m)
 
     try
     {
-        STValidation::pointer val;
+        std::shared_ptr<STValidation> val;
         {
             SerialIter sit(makeSlice(m->validation()));
             val = std::make_shared<STValidation>(
@@ -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<STValidation> const& val,
     std::shared_ptr<protocol::TMValidation> const& packet)
 {
     try

src/ripple/overlay/impl/PeerImp.h

@@ -592,7 +592,7 @@ private:
 
     void
     checkValidation(
-        STValidation::pointer val,
+        std::shared_ptr<STValidation> const& val,
         std::shared_ptr<protocol::TMValidation> const& packet);
 
     void

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<uint256> features;
     boost::container::flat_map<uint256, std::size_t> 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.

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;

src/ripple/protocol/SOTemplate.h

@@ -50,7 +50,13 @@ public:
         : sField_(fieldName), style_(style)
     {
         if (!sField_.get().isUseful())
-            Throw<std::runtime_error>("SField in SOElement must be useful.");
+        {
+            auto nm = std::to_string(fieldName.getCode());
+            if (fieldName.hasName())
+                nm += ": '" + fieldName.getName() + "'";
+            Throw<std::runtime_error>(
+                "SField (" + nm + ") in SOElement must be useful.");
+        }
     }
 
     SField const&

src/ripple/protocol/STValidation.h

@@ -25,6 +25,7 @@
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/STObject.h>
 #include <ripple/protocol/SecretKey.h>
+#include <cassert>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -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<STValidation>
 {
@@ -44,11 +49,6 @@ public:
         return "STValidation";
     }
 
-    using pointer = std::shared_ptr<STValidation>;
-    using ref = const std::shared_ptr<STValidation>&;
-
-    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<std::runtime_error>("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<std::runtime_error>("Invalid signature in validation");
         }
 
-        mNodeID = lookupNodeID(PublicKey(makeSlice(spk)));
+        nodeID_ = lookupNodeID(PublicKey(makeSlice(spk)));
-        assert(mNodeID.isNonZero());
+        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<std::uint32_t> loadFee;
-        boost::optional<XRPAmount> baseFee;
-        boost::optional<XRPAmount> reserveBase;
-        boost::optional<XRPAmount> 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 f callback function to "fill" the validation with necessary data
-        @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.
     */
+    template <typename F>
     STValidation(
-        uint256 const& ledgerHash,
-        std::uint32_t ledgerSeq,
-        uint256 const& consensusHash,
         NetClock::time_point signTime,
-        PublicKey const& publicKey,
+        PublicKey const& pk,
-        SecretKey const& secretKey,
+        SecretKey const& sk,
         NodeID const& nodeID,
-        bool isFull,
+        F&& f)
-        FeeSettings const& fees,
+        : STObject(validationFormat(), sfValidation)
-        std::vector<uint256> const& amendments);
+        , 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

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.

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");

src/ripple/protocol/impl/STValidation.cpp

@@ -25,68 +25,32 @@
 
 namespace ripple {
 
-STValidation::STValidation(
+SOTemplate const&
-    uint256 const& ledgerHash,
+STValidation::validationFormat()
-    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<uint256> const& amendments)
-    : STObject(getFormat(), sfValidation), mNodeID(nodeID), mSeen(signTime)
 {
-    // This is our own public key and it should always be valid.
+    // We can't have this be a magic static at namespace scope because
-    if (!publicKeyType(publicKey))
+    // it relies on the SField's below being initialized, and we can't
-        LogicError("Invalid validation public key");
+    // guarantee the initialization order.
-    assert(mNodeID.isNonZero());
+    static SOTemplate const format{
-    setFieldH256(sfLedgerHash, ledgerHash);
+        {sfFlags, soeREQUIRED},
-    setFieldH256(sfConsensusHash, consensusHash);
+        {sfLedgerHash, soeREQUIRED},
-    setFieldU32(sfSigningTime, signTime.time_since_epoch().count());
+        {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());
+    return format;
-    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<std::uint64_t>())
-            setFieldU64(sfBaseFee, *v);
-    }
-
-    if (fees.reserveBase)
-    {
-        if (auto const v = fees.reserveBase->dropsAs<std::uint32_t>())
-            setFieldU32(sfReserveBase, *v);
-    }
-
-    if (fees.reserveIncrement)
-    {
-        if (auto const v = fees.reserveIncrement->dropsAs<std::uint32_t>())
-            setFieldU32(sfReserveIncrement, *v);
-    }
-
-    if (!amendments.empty())
-        setFieldV256(sfAmendments, STVector256(sfAmendments, amendments));
-
-    setFlag(vfFullyCanonicalSig);
-
-    auto const signingHash = getSigningHash();
-    setFieldVL(
-        sfSignature, signDigest(getSignerPublic(), secretKey, signingHash));
-
-    setTrusted();
-}
 
 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

src/test/app/AmendmentTable_test.cpp

@@ -25,6 +25,7 @@
 #include <ripple/core/ConfigSections.h>
 #include <ripple/protocol/Feature.h>
 #include <ripple/protocol/PublicKey.h>
+#include <ripple/protocol/STValidation.h>
 #include <ripple/protocol/SecretKey.h>
 #include <ripple/protocol/TxFlags.h>
 #include <ripple/protocol/digest.h>
@@ -372,7 +373,7 @@ public:
         auto const roundTime = weekTime(week);
 
         // Build validations
-        std::vector<STValidation::pointer> validations;
+        std::vector<std::shared_ptr<STValidation>> validations;
         validations.reserve(validators.size());
 
         int i = 0;
@@ -391,16 +392,15 @@ public:
             }
 
             auto v = std::make_shared<STValidation>(
-                uint256(),
+                ripple::NetClock::time_point{},
-                i,
-                uint256(),
-                roundTime,
                 val.first,
                 val.second,
                 calcNodeID(val.first),
-                true,
+                [&field](STValidation& v) {
-                STValidation::FeeSettings{},
+                    if (!field.empty())
-                field);
+                        v.setFieldV256(
+                            sfAmendments, STVector256(sfAmendments, field));
+                });
 
             validations.emplace_back(v);
         }

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<STValidation>(
-            uint256(),
+            ripple::NetClock::time_point{},
-            1,
-            uint256(),
-            NetClock::time_point(),
             keys.first,
             keys.second,
             calcNodeID(keys.first),
-            true,
+            [&](STValidation& v) {});
-            STValidation::FeeSettings{},
-            std::vector<uint256>{});
 
         BEAST_EXPECT(v->isTrusted());
         v->setUntrusted();

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<std::uint32_t> 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
         {

src/test/csf/Validation.h

@@ -55,6 +55,7 @@ class Validation
     bool trusted_ = false;
     bool full_ = false;
     boost::optional<std::uint32_t> loadFee_;
+    std::uint64_t cookie_{0};
 
 public:
     using NodeKey = PeerKey;
@@ -68,7 +69,8 @@ public:
         PeerKey key,
         PeerID nodeID,
         bool full,
-        boost::optional<std::uint32_t> loadFee = boost::none)
+        boost::optional<std::uint32_t> 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<std::uint32_t>
     loadFee() const
     {
@@ -191,4 +200,5 @@ public:
 }  // namespace csf
 }  // namespace test
 }  // namespace ripple
-#endif
+
+#endif

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];
             }));