diff --git a/include/xrpl/ledger/Ledger.h b/include/xrpl/ledger/Ledger.h index e1dd2c422e..ed3a5bcc44 100644 --- a/include/xrpl/ledger/Ledger.h +++ b/include/xrpl/ledger/Ledger.h @@ -330,6 +330,18 @@ public: void updateSkipList(); + /** + * Check that every node of both of this ledger's maps is available. + * + * The function walks the state map and then the transaction map, and logs + * what it could not read. + * + * @param j The journal to log missing nodes to. + * @param parallel Walk the state map on several threads. The transaction + * map is always walked on one thread. + * @return True when both maps are complete. False when either map is + * missing a node. + */ bool walkLedger(beast::Journal j, bool parallel = false) const; diff --git a/src/libxrpl/ledger/Ledger.cpp b/src/libxrpl/ledger/Ledger.cpp index 188fb3cd2c..c06ce81fc3 100644 --- a/src/libxrpl/ledger/Ledger.cpp +++ b/src/libxrpl/ledger/Ledger.cpp @@ -743,18 +743,22 @@ Ledger::walkLedger(beast::Journal j, bool parallel) const std::vector missingNodes1; std::vector missingNodes2; + // Returning walkMapParallel's result directly would skip the transaction map walk + // below, and dropping it would lose an answer missingNodes1 does not carry: a worker + // that cannot read the node store reports that through the return value alone. + bool stateComplete = true; + if (stateMap_.getHash().isZero() && !header_.accountHash.isZero() && !stateMap_.fetchRoot(SHAMapHash{header_.accountHash}, nullptr)) { missingNodes1.emplace_back(SHAMapType::STATE, SHAMapHash{header_.accountHash}); } + else if (parallel) + { + stateComplete = stateMap_.walkMapParallel(missingNodes1, 32); + } else { - if (parallel) - { - return stateMap_.walkMapParallel(missingNodes1, 32); - } - stateMap_.walkMap(missingNodes1, 32); } @@ -785,7 +789,7 @@ Ledger::walkLedger(beast::Journal j, bool parallel) const stream << "First: " << missingNodes2[0].what(); } } - return missingNodes1.empty() && missingNodes2.empty(); + return stateComplete && missingNodes1.empty() && missingNodes2.empty(); } bool diff --git a/src/tests/libxrpl/ledger/WalkLedger.cpp b/src/tests/libxrpl/ledger/WalkLedger.cpp new file mode 100644 index 0000000000..9f75908dab --- /dev/null +++ b/src/tests/libxrpl/ledger/WalkLedger.cpp @@ -0,0 +1,181 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include // SHAMapType +#include + +#include + +#include +#include +#include + +#include +#include +#include + +namespace xrpl::tests { + +namespace { + +// Fixed seed, so a failure reproduces when the test runs on its own. +constexpr std::uint32_t kSeed = 0x5eed1234U; + +/** + * Build a SHAMapItem holding random data. + * + * @param engine The random engine to draw the data from. + * @return The new item. + */ +boost::intrusive_ptr +makeRandomItem(beast::xor_shift_engine& engine) +{ + static constexpr auto kWordsPerItem = 3; + + Serializer s; + for (auto word = 0; word < kWordsPerItem; ++word) + s.add32(randInt(engine)); + return makeShamapitem(s.getSHA512Half(), s.slice()); +} + +/** + * Write a map's nodes into a family's node store. + * + * @param map The map to read. It must be complete. + * @param family The family whose store receives the nodes. + * @param rootOnly Write only the root node, leaving every node below it absent. + */ +void +storeMap(SHAMap const& map, test::TestFamily& family, bool rootOnly) +{ + // visitNodes reports the root first, so rootOnly lets that first call through and + // stops at the one after it. + int stored = 0; + map.visitNodes([&family, rootOnly, &stored](SHAMapTreeNode& node) { + if (rootOnly && stored > 0) + return false; + + Serializer s; + node.serializeWithPrefix(s); + family.db().store( + NodeObjectType::AccountNode, std::move(s.modData()), node.getHash().asUInt256(), 0); + ++stored; + return true; + }); +} + +} // namespace + +// walkLedger must not report a ledger whose transaction map is missing a node as +// complete. The parallel path used to return the state map's result directly and never +// walk the transaction map at all, so Application::loadOldLedger accepted such a ledger. +// Both paths must agree, so this checks the serial one on the same ledger. +TEST(WalkLedger, parallel_walk_checks_the_transaction_map) +{ + static constexpr auto kItems = 200; + + beast::Journal const j{TestSink::instance()}; + beast::xor_shift_engine engine{kSeed}; + test::TestFamily source{j}; + test::TestFamily dest{j}; + + // Two complete maps, built in the source family. + SHAMap stateMap{SHAMapType::STATE, source}; + SHAMap txMap{SHAMapType::TRANSACTION, source}; + for (auto i = 0; i < kItems; ++i) + { + stateMap.addItem(SHAMapNodeType::TnAccountState, makeRandomItem(engine)); + txMap.addItem(SHAMapNodeType::TnTransactionNm, makeRandomItem(engine)); + } + stateMap.setImmutable(); + txMap.setImmutable(); + + // Read both hashes before storing anything. getHash() is what computes the node + // hashes, through unshare(), so storing first would write every node under a stale + // hash and nothing would be findable afterwards. + LedgerHeader header; + header.seq = 1; + header.accountHash = stateMap.getHash().asUInt256(); + header.txHash = txMap.getHash().asUInt256(); + header.hash = calculateLedgerHash(header); + + // The destination family gets the whole state map, so the state walk succeeds, and + // only the transaction map's root, so every node below it is unreadable. The first + // unreadable node makes SHAMap::finishFetch call TestFamily::missingNodeAcquireBySeq, + // which throws, so expect one "finishFetch exception" warning in the log below. + storeMap(stateMap, dest, false); + storeMap(txMap, dest, true); + + bool loaded = false; + Ledger const ledger{ + header, + loaded, + false, + Rules{std::unordered_set>{}}, + Fees{}, + dest, + j}; + + // Both roots are readable, so the ledger loads. + ASSERT_TRUE(loaded); + + EXPECT_FALSE(ledger.walkLedger(j, true)); + EXPECT_FALSE(ledger.walkLedger(j, false)); +} + +// A ledger whose maps are both complete must pass, on the parallel path too. +TEST(WalkLedger, parallel_walk_accepts_a_complete_ledger) +{ + static constexpr auto kItems = 200; + + beast::Journal const j{TestSink::instance()}; + beast::xor_shift_engine engine{kSeed}; + test::TestFamily source{j}; + test::TestFamily dest{j}; + + SHAMap stateMap{SHAMapType::STATE, source}; + SHAMap txMap{SHAMapType::TRANSACTION, source}; + for (auto i = 0; i < kItems; ++i) + { + stateMap.addItem(SHAMapNodeType::TnAccountState, makeRandomItem(engine)); + txMap.addItem(SHAMapNodeType::TnTransactionNm, makeRandomItem(engine)); + } + stateMap.setImmutable(); + txMap.setImmutable(); + + // getHash() before storeMap: see the note in the test above. + LedgerHeader header; + header.seq = 1; + header.accountHash = stateMap.getHash().asUInt256(); + header.txHash = txMap.getHash().asUInt256(); + header.hash = calculateLedgerHash(header); + + storeMap(stateMap, dest, false); + storeMap(txMap, dest, false); + + bool loaded = false; + Ledger const ledger{ + header, + loaded, + false, + Rules{std::unordered_set>{}}, + Fees{}, + dest, + j}; + ASSERT_TRUE(loaded); + + EXPECT_TRUE(ledger.walkLedger(j, true)); + EXPECT_TRUE(ledger.walkLedger(j, false)); +} + +} // namespace xrpl::tests