Refactor getTrustedForLedger() (XRPLF#4424)

Look for validations associated with a specific ledger ID and sequence
number.

src/ripple/app/consensus/RCLConsensus.cpp

@@ -337,7 +337,7 @@ RCLConsensus::Adaptor::onClose(
             // pseudo-transactions
             auto validations = app_.validators().negativeUNLFilter(
                 app_.getValidations().getTrustedForLedger(
-                    prevLedger->info().parentHash));
+                    prevLedger->info().parentHash, prevLedger->seq() - 1));
             if (validations.size() >= app_.validators().quorum())
             {
                 feeVote_->doVoting(prevLedger, validations, initialSet);

src/ripple/app/ledger/impl/LedgerMaster.cpp

@@ -341,7 +341,8 @@ LedgerMaster::setValidLedger(std::shared_ptr<Ledger const> const& l)
     if (!standalone_)
     {
         auto validations = app_.validators().negativeUNLFilter(
-            app_.getValidations().getTrustedForLedger(l->info().hash));
+            app_.getValidations().getTrustedForLedger(
+                l->info().hash, l->info().seq));
         times.reserve(validations.size());
         for (auto const& val : validations)
             times.push_back(val->getSignTime());
@@ -987,7 +988,7 @@ LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq)
             return;
 
         auto validations = app_.validators().negativeUNLFilter(
-            app_.getValidations().getTrustedForLedger(hash));
+            app_.getValidations().getTrustedForLedger(hash, seq));
         valCount = validations.size();
         if (valCount >= app_.validators().quorum())
         {
@@ -1053,7 +1054,8 @@ LedgerMaster::checkAccept(std::shared_ptr<Ledger const> const& ledger)
 
     auto const minVal = getNeededValidations();
     auto validations = app_.validators().negativeUNLFilter(
-        app_.getValidations().getTrustedForLedger(ledger->info().hash));
+        app_.getValidations().getTrustedForLedger(
+            ledger->info().hash, ledger->info().seq));
     auto const tvc = validations.size();
     if (tvc < minVal)  // nothing we can do
     {
@@ -1128,7 +1130,7 @@ LedgerMaster::checkAccept(std::shared_ptr<Ledger const> const& ledger)
         {
             // Have not printed the warning before, check if need to print.
             auto const vals = app_.getValidations().getTrustedForLedger(
-                ledger->info().parentHash);
+                ledger->info().parentHash, ledger->info().seq - 1);
             std::size_t higherVersionCount = 0;
             std::size_t rippledCount = 0;
             for (auto const& v : vals)

src/ripple/app/misc/NegativeUNLVote.cpp

@@ -198,7 +198,7 @@ NegativeUNLVote::buildScoreTable(
     for (int i = 0; i < FLAG_LEDGER_INTERVAL; ++i)
     {
         for (auto const& v : validations.getTrustedForLedger(
-                 ledgerAncestors[numAncestors - 1 - i]))
+                 ledgerAncestors[numAncestors - 1 - i], seq - 2 - i))
         {
             if (scoreTable.count(v->getNodeID()))
                 ++scoreTable[v->getNodeID()];

src/ripple/consensus/Validations.h

@@ -1049,10 +1049,11 @@ class Validations
     /**  Get trusted full validations for a specific ledger
 
          @param ledgerID The identifier of ledger of interest
+         @param seq The sequence number of ledger of interest
          @return Trusted validations associated with ledger
     */
     std::vector<WrappedValidationType>
-    getTrustedForLedger(ID const& ledgerID)
+    getTrustedForLedger(ID const& ledgerID, Seq const& seq)
     {
         std::vector<WrappedValidationType> res;
         std::lock_guard lock{mutex_};
@@ -1061,7 +1062,7 @@ class Validations
             ledgerID,
             [&](std::size_t numValidations) { res.reserve(numValidations); },
             [&](NodeID const&, Validation const& v) {
-                if (v.trusted() && v.full())
+                if (v.trusted() && v.full() && v.seq() == seq)
                     res.emplace_back(v.unwrap());
             });
 

src/test/consensus/Validations_test.cpp

@@ -613,7 +613,8 @@ class Validations_test : public beast::unit_test::suite
         c.setLoadFee(12);
         e.setLoadFee(12);
 
-        hash_map<Ledger::ID, std::vector<Validation>> trustedValidations;
+        hash_map<std::pair<Ledger::ID, Ledger::Seq>, std::vector<Validation>>
+            trustedValidations;
 
         //----------------------------------------------------------------------
         // checkers
@@ -624,14 +625,15 @@ class Validations_test : public beast::unit_test::suite
         auto compare = [&]() {
             for (auto& it : trustedValidations)
             {
-                auto const& id = it.first;
+                auto const& id = it.first.first;
+                auto const& seq = it.first.second;
                 auto const& expectedValidations = it.second;
 
                 BEAST_EXPECT(
                     harness.vals().numTrustedForLedger(id) ==
                     expectedValidations.size());
                 BEAST_EXPECT(
-                    sorted(harness.vals().getTrustedForLedger(id)) ==
+                    sorted(harness.vals().getTrustedForLedger(id, seq)) ==
                     sorted(expectedValidations));
 
                 std::uint32_t baseFee = 0;
@@ -653,21 +655,22 @@ class Validations_test : public beast::unit_test::suite
         Ledger ledgerAC = h["ac"];
 
         // Add a dummy ID to cover unknown ledger identifiers
-        trustedValidations[Ledger::ID{100}] = {};
+        trustedValidations[{Ledger::ID{100}, Ledger::Seq{100}}] = {};
 
         // first round a,b,c agree
         for (auto const& node : {a, b, c})
         {
             auto const val = node.validate(ledgerA);
             BEAST_EXPECT(ValStatus::current == harness.add(val));
             if (val.trusted())
-                trustedValidations[val.ledgerID()].emplace_back(val);
+                trustedValidations[{val.ledgerID(), val.seq()}].emplace_back(
+                    val);
         }
         // d disagrees
         {
             auto const val = d.validate(ledgerB);
             BEAST_EXPECT(ValStatus::current == harness.add(val));
-            trustedValidations[val.ledgerID()].emplace_back(val);
+            trustedValidations[{val.ledgerID(), val.seq()}].emplace_back(val);
         }
         // e only issues partials
         {
@@ -681,7 +684,8 @@ class Validations_test : public beast::unit_test::suite
             auto const val = node.validate(ledgerAC);
             BEAST_EXPECT(ValStatus::current == harness.add(val));
             if (val.trusted())
-                trustedValidations[val.ledgerID()].emplace_back(val);
+                trustedValidations[{val.ledgerID(), val.seq()}].emplace_back(
+                    val);
         }
         // d now thinks ledger 1, but cannot re-issue a previously used seq
         // and attempting it should generate a conflict.
@@ -1035,6 +1039,9 @@ class Validations_test : public beast::unit_test::suite
                            std::vector<Validation> const& trustedVals) {
             Ledger::ID testID = trustedVals.empty() ? this->genesisLedger.id()
                                                     : trustedVals[0].ledgerID();
+            Ledger::Seq testSeq = trustedVals.empty()
+                ? this->genesisLedger.seq()
+                : trustedVals[0].seq();
             BEAST_EXPECT(vals.currentTrusted() == trustedVals);
             BEAST_EXPECT(vals.getCurrentNodeIDs() == listed);
             BEAST_EXPECT(
@@ -1046,7 +1053,8 @@ class Validations_test : public beast::unit_test::suite
             else
                 BEAST_EXPECT(
                     vals.getPreferred(this->genesisLedger)->second == testID);
-            BEAST_EXPECT(vals.getTrustedForLedger(testID) == trustedVals);
+            BEAST_EXPECT(
+                vals.getTrustedForLedger(testID, testSeq) == trustedVals);
             BEAST_EXPECT(
                 vals.numTrustedForLedger(testID) == trustedVals.size());
         };