diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 12a150a4a5..7ba059881c 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/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); diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index ad08b18dd5..7476698ee3 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -341,7 +341,8 @@ LedgerMaster::setValidLedger(std::shared_ptr 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 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 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) diff --git a/src/ripple/app/misc/NegativeUNLVote.cpp b/src/ripple/app/misc/NegativeUNLVote.cpp index fba02637e5..5c133403b4 100644 --- a/src/ripple/app/misc/NegativeUNLVote.cpp +++ b/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()]; diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index 46bf4322a5..a9dbd5585e 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -1049,10 +1049,11 @@ public: /** 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 - getTrustedForLedger(ID const& ledgerID) + getTrustedForLedger(ID const& ledgerID, Seq const& seq) { std::vector res; std::lock_guard lock{mutex_}; @@ -1061,7 +1062,7 @@ public: 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()); }); diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index 79de1fc806..7dc2086e55 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/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> trustedValidations; + hash_map, std::vector> + 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,7 +655,7 @@ 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}) @@ -661,13 +663,14 @@ class Validations_test : public beast::unit_test::suite 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 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()); };