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,13 @@ 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>
@@ -745,40 +752,60 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger,
     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 v = std::make_shared<STValidation>(
+        lastValidationTime_, valPublic_, valSecret_, nodeID_,
+        [&](STValidation& v)
+        {
+            v.setFieldH256(sfLedgerHash, ledger.id());
+            v.setFieldH256(sfConsensusHash, txns.id());
 
-    auto const& feeTrack = app_.getFeeTrack();
-    std::uint32_t fee =
-        std::max(feeTrack.getLocalFee(), feeTrack.getClusterFee());
+            v.setFieldU32(sfLedgerSequence, ledger.seq());
 
-    if (fee > feeTrack.getLoadBase())
-        fees.loadFee = fee;
+            if (proposing)
+                v.setFlag(vfFullValidation);
 
-    // 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_));
-    }
+            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);
 
-    auto v = std::make_shared<STValidation>(
-        ledger.id(),
-        ledger.seq(),
-        txns.id(),
-        validationTime,
-        valPublic_,
-        valSecret_,
-        nodeID_,
-        proposing /* full if proposed */,
-        fees,
-        amendments);
+                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

@@ -65,6 +65,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

@@ -149,15 +149,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 =
-        app.validators().getTrustedKey(signingKey);
+    auto masterKey = app.validators().getTrustedKey(signingKey);
     if (!val->isTrusted() && masterKey)
         val->setTrusted();
 
@@ -169,15 +168,17 @@ handleNewValidation(Application& app,
     RCLValidations& validations = app.getValidations();
     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;
+    auto dmp = [&](beast::Journal::Stream s, std::string const& msg)
+    {
+        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;
     };
 
     if(!val->isFieldPresent(sfLedgerSequence))
@@ -191,9 +192,26 @@ handleNewValidation(Application& app,
     if (masterKey)
     {
         ValStatus const outcome = validations.add(calcNodeID(*masterKey), val);
+
         if(j.debug())
             dmp(j.debug(), to_string(outcome));
 
+        if (outcome == ValStatus::conflicting && j.fatal())
+        {
+            auto const seq = val->getFieldU32(sfLedgerSequence);
+            dmp(j.fatal(),
+                "conflicting validations issued for " + to_string(seq) +
+                " (likely from a Byzantine validator)");
+        }
+
+        if (outcome == ValStatus::multiple && j.fatal())
+        {
+            auto const seq = val->getFieldU32(sfLedgerSequence);
+            dmp(j.fatal(),
+                "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);

src/ripple/app/consensus/RCLValidations.h

@@ -33,12 +33,12 @@ 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<STValidation> val_;
+
 public:
     using NodeKey = ripple::PublicKey;
     using NodeID = ripple::NodeID;
@@ -47,7 +47,7 @@ class RCLValidation
 
         @param v The validation to wrap.
     */
-    RCLValidation(STValidation::pointer const& v) : val_{v}
+    RCLValidation(std::shared_ptr<STValidation> const& v) : val_{v}
     {
     }
 
@@ -126,13 +126,19 @@ class RCLValidation
         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_;
     }
-
 };
 
 /** Wraps a ledger instance for use in generic Validations LedgerTrie.
@@ -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

@@ -93,7 +93,7 @@ class AmendmentTable
         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
@@ -116,7 +116,7 @@ class AmendmentTable
     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

@@ -61,8 +61,9 @@ class FeeVote
     */
     virtual
     void
-    doValidation (std::shared_ptr<ReadView const> const& lastClosedLedger,
-        STValidation::FeeSettings & fees) = 0;
+    doValidation (
+        Fees const& lastFees,
+        STValidation& val) = 0;
 
     /** Cast our local vote on the fee.
 
@@ -72,7 +73,7 @@ class FeeVote
     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

@@ -96,12 +96,13 @@ class FeeVoteImpl : public FeeVote
     FeeVoteImpl (Setup const& setup, beast::Journal journal);
 
     void
-    doValidation (std::shared_ptr<ReadView const> const& lastClosedLedger,
-        STValidation::FeeSettings& fees) override;
+    doValidation (
+        Fees const& lastFees,
+        STValidation& val) 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;
 };
 
@@ -113,40 +114,45 @@ FeeVoteImpl::FeeVoteImpl (Setup const& setup, beast::Journal journal)
 {
 }
 
-void
-FeeVoteImpl::doValidation(
-    std::shared_ptr<ReadView const> const& lastClosedLedger,
-        STValidation::FeeSettings& fees)
+void 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<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

@@ -333,7 +333,8 @@ class NetworkOPsImp final
         std::shared_ptr<protocol::TMProposeSet> set) override;
 
     bool recvValidation (
-        STValidation::ref val, std::string const& source) override;
+        std::shared_ptr<STValidation> const& val,
+        std::string const& source) override;
 
     std::shared_ptr<SHAMap> getTXMap (uint256 const& hash);
     bool hasTXSet (
@@ -480,7 +481,7 @@ class NetworkOPsImp final
         std::shared_ptr<ReadView const> const& lpCurrent,
         std::shared_ptr<STTx const> const& stTxn, TER terResult) override;
     void pubValidation (
-        STValidation::ref val) override;
+        std::shared_ptr<STValidation> const& val) override;
 
     //--------------------------------------------------------------------------
     //
@@ -1884,7 +1885,7 @@ void NetworkOPsImp::pubConsensus (ConsensusPhase phase)
 }
 
 
-void NetworkOPsImp::pubValidation (STValidation::ref val)
+void NetworkOPsImp::pubValidation (std::shared_ptr<STValidation> const& val)
 {
     // VFALCO consider std::shared_mutex
     std::lock_guard sl (mSubLock);
@@ -2290,7 +2291,7 @@ NetworkOPsImp::getTxsAccountB(
 }
 
 bool NetworkOPsImp::recvValidation (
-    STValidation::ref val, std::string const& source)
+    std::shared_ptr<STValidation> const& val, std::string const& source)
 {
     JLOG(m_journal.debug()) << "recvValidation " << val->getLedgerHash ()
                           << " from " << source;