From a6f2f9f27a467617d995df8348fea73c6a453d4e Mon Sep 17 00:00:00 2001 From: Richard Holland Date: Wed, 5 Mar 2025 20:51:16 +1100 Subject: [PATCH] move away from using bare pointers, use Ledger const references where possible --- src/ripple/rpc/handlers/Catalogue.cpp | 40 ++++++++++----------------- src/ripple/rpc/impl/RPCHelpers.cpp | 35 +++++++++++++++++++++-- src/ripple/shamap/SHAMap.h | 9 ++++-- src/ripple/shamap/impl/SHAMap.cpp | 19 ++++++++----- 4 files changed, 66 insertions(+), 37 deletions(-) diff --git a/src/ripple/rpc/handlers/Catalogue.cpp b/src/ripple/rpc/handlers/Catalogue.cpp index 329a45021..10b785d55 100644 --- a/src/ripple/rpc/handlers/Catalogue.cpp +++ b/src/ripple/rpc/handlers/Catalogue.cpp @@ -22,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -68,7 +67,7 @@ struct CATLHeader class CatalogueProcessor { private: - std::vector>& ledgers; + std::vector>& ledgers; std::ofstream& outfile; beast::Journal journal; std::atomic aborted{false}; @@ -88,7 +87,10 @@ private: } bool - outputLedger(ReadView const& ledger, SHAMap const* prevStateMap = nullptr) + outputLedger( + Ledger const& ledger, + std::optional> prevStateMap = + std::nullopt) { uint32_t seq = ledger.info().seq; try @@ -127,15 +129,10 @@ private: return false; } - auto& stateMap = - static_cast(ledger.get())->stateMap(); - size_t stateNodesWritten = - stateMap.serializeToStream(outfile, prevStateMap); + ledger->stateMap().serializeToStream(outfile, prevStateMap); - auto& txMap = static_cast(ledger.get())->txMap(); - - size_t txNodesWritten = txMap.serializeToStream(outfile, nullptr); + size_t txNodesWritten = ledger->txMap().serializeToStream(outfile); JLOG(journal.info()) << "Ledger " << seq << ": Wrote " << stateNodesWritten << " state nodes, " @@ -154,7 +151,7 @@ private: public: CatalogueProcessor( - std::vector>& ledgers_, + std::vector>& ledgers_, std::ofstream& outfile_, beast::Journal journal_) : ledgers(ledgers_), outfile(outfile_), journal(journal_) @@ -178,18 +175,11 @@ public: if (!outputLedger(*ledgers.front())) return false; - // For subsequent ledgers, process as deltas from the previous - SHAMap const* prevStateMap = - &static_cast(ledgers.front().get())->stateMap(); - for (size_t i = 1; i < ledgers.size(); ++i) { auto ledger = ledgers[i]; - if (!outputLedger(*ledger, prevStateMap)) + if (!outputLedger(*ledger, ledgers[i - 1]->stateMap())) return false; - - prevStateMap = - &static_cast(ledger.get())->stateMap(); } return !aborted; @@ -251,14 +241,14 @@ doCatalogueCreate(RPC::JsonContext& context) if (min_ledger > max_ledger) return rpcError(rpcINVALID_PARAMS, "min_ledger must be <= max_ledger"); - std::vector> lpLedgers; + std::vector> lpLedgers; // Grab all ledgers of interest lpLedgers.reserve(max_ledger - min_ledger + 1); for (auto i = min_ledger; i <= max_ledger; ++i) { - std::shared_ptr ptr; + std::shared_ptr ptr; auto status = RPC::getLedger(ptr, i, context); if (status.toErrorCode() != rpcSUCCESS) // Status isn't OK return rpcError(status); @@ -440,8 +430,9 @@ doCatalogueLoad(RPC::JsonContext& context) ledger->setLedgerInfo(info); // Apply delta (only leaf-node changes) - if (!ledger->stateMap().deserializeFromStream( - infile, &prevLedger->stateMap())) + SHAMap const& prevMap = prevLedger->stateMap(); + + if (!ledger->stateMap().deserializeFromStream(infile, prevMap)) { JLOG(context.j.error()) << "Failed to apply delta to ledger " << info.seq; @@ -450,8 +441,7 @@ doCatalogueLoad(RPC::JsonContext& context) } // pull in the tx map - if (!ledger->txMap().deserializeFromStream( - infile, &prevLedger->txMap())) + if (!ledger->txMap().deserializeFromStream(infile, prevLedger->txMap())) { JLOG(context.j.error()) << "Failed to apply delta to ledger " << info.seq; diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 26d279dbd..f3d25bd60 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -570,6 +570,19 @@ getLedger(T& ledger, uint256 const& ledgerHash, Context& context) return Status::OK; } +// Helper function to determine if the types are compatible for assignment +template +struct is_assignable_shared_ptr : std::false_type +{ +}; + +template +struct is_assignable_shared_ptr< + std::shared_ptr&, + std::shared_ptr> : std::is_convertible +{ +}; + template Status getLedger(T& ledger, uint32_t ledgerIndex, Context& context) @@ -579,10 +592,14 @@ getLedger(T& ledger, uint32_t ledgerIndex, Context& context) { if (context.app.config().reporting()) return {rpcLGR_NOT_FOUND, "ledgerNotFound"}; + auto cur = context.ledgerMaster.getCurrentLedger(); if (cur->info().seq == ledgerIndex) { - ledger = cur; + if constexpr (is_assignable_shared_ptr::value) + { + ledger = cur; + } } } @@ -635,7 +652,12 @@ getLedger(T& ledger, LedgerShortcut shortcut, Context& context) return { rpcLGR_NOT_FOUND, "Reporting does not track current ledger"}; - ledger = context.ledgerMaster.getCurrentLedger(); + auto cur = context.ledgerMaster.getCurrentLedger(); + if constexpr (is_assignable_shared_ptr::value) + { + ledger = cur; + } + assert(ledger->open()); } else if (shortcut == LedgerShortcut::CLOSED) @@ -685,6 +707,15 @@ getLedger<>( template Status getLedger<>(std::shared_ptr&, uint256 const&, Context&); +template Status +getLedger<>(std::shared_ptr&, uint32_t, Context&); + +template Status +getLedger<>(std::shared_ptr&, LedgerShortcut shortcut, Context&); + +template Status +getLedger<>(std::shared_ptr&, uint256 const&, Context&); + bool isValidated( LedgerMaster& ledgerMaster, diff --git a/src/ripple/shamap/SHAMap.h b/src/ripple/shamap/SHAMap.h index 3abce21ff..ed28ec154 100644 --- a/src/ripple/shamap/SHAMap.h +++ b/src/ripple/shamap/SHAMap.h @@ -380,8 +380,10 @@ public: * @return Number of nodes written */ std::size_t - serializeToStream(std::ostream& stream, SHAMap const* baseSHAMap = nullptr) - const; + serializeToStream( + std::ostream& stream, + std::optional> baseSHAMap = + std::nullopt) const; /** * Deserialize a SHAMap from a stream @@ -394,7 +396,8 @@ public: bool deserializeFromStream( std::istream& stream, - SHAMap const* baseSHAMap = nullptr); + std::optional> baseSHAMap = + std::nullopt); private: using SharedPtrNodeStack = diff --git a/src/ripple/shamap/impl/SHAMap.cpp b/src/ripple/shamap/impl/SHAMap.cpp index baad7d172..ca77fd15b 100644 --- a/src/ripple/shamap/impl/SHAMap.cpp +++ b/src/ripple/shamap/impl/SHAMap.cpp @@ -1243,7 +1243,9 @@ SHAMap::invariants() const } std::size_t -SHAMap::serializeToStream(std::ostream& stream, SHAMap const* baseSHAMap) const +SHAMap::serializeToStream( + std::ostream& stream, + std::optional> baseSHAMap) const { std::unordered_set> writtenNodes; @@ -1286,15 +1288,16 @@ SHAMap::serializeToStream(std::ostream& stream, SHAMap const* baseSHAMap) const }; // If we're creating a delta, first compute the differences - if (baseSHAMap && baseSHAMap->root_) + if (baseSHAMap && baseSHAMap->get().root_) { + const SHAMap& baseMap = baseSHAMap->get(); + // Only compute delta if the maps are different - if (getHash() != baseSHAMap->getHash()) + if (getHash() != baseMap.getHash()) { Delta differences; - if (compare( - *baseSHAMap, differences, std::numeric_limits::max())) + if (compare(baseMap, differences, std::numeric_limits::max())) { // Process each difference for (auto const& [key, deltaItem] : differences) @@ -1387,7 +1390,9 @@ SHAMap::serializeToStream(std::ostream& stream, SHAMap const* baseSHAMap) const } bool -SHAMap::deserializeFromStream(std::istream& stream, SHAMap const* baseSHAMap) +SHAMap::deserializeFromStream( + std::istream& stream, + std::optional> baseSHAMap) { try { @@ -1401,7 +1406,7 @@ SHAMap::deserializeFromStream(std::istream& stream, SHAMap const* baseSHAMap) // If we have a base map, start with a copy of it if (baseSHAMap) { - root_ = baseSHAMap->root_; + root_ = baseSHAMap->get().root_; if (root_) unshare(); }