From abe735102aed5d2585d8342bce67ddfe6e4de95e Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 23 Jul 2015 12:29:35 -0700 Subject: [PATCH] Fix metadata during apply: Metadata is correctly generated for the case where a ledger entry is only changed as a consequence of threading. This changes the result compared to previous versions, which produced more than necessary for these cases. --- src/ripple/ledger/OpenView.h | 2 +- src/ripple/ledger/detail/ApplyStateTable.h | 5 +- src/ripple/ledger/impl/ApplyStateTable.cpp | 64 ++++++++++++---------- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/ripple/ledger/OpenView.h b/src/ripple/ledger/OpenView.h index 526fba7e9..a267fc0ff 100644 --- a/src/ripple/ledger/OpenView.h +++ b/src/ripple/ledger/OpenView.h @@ -85,7 +85,7 @@ public: Effects: Creates a new object with a copy of - the modificaiton state table. + the modification state table. The objects managed by shared pointers are not duplicated but shared between instances. diff --git a/src/ripple/ledger/detail/ApplyStateTable.h b/src/ripple/ledger/detail/ApplyStateTable.h index a8e934c2f..376578f2e 100644 --- a/src/ripple/ledger/detail/ApplyStateTable.h +++ b/src/ripple/ledger/detail/ApplyStateTable.h @@ -124,9 +124,8 @@ private: static bool - threadTx (TxMeta& meta, - std::shared_ptr const& to, - Mods& mods); + threadItem (TxMeta& meta, + std::shared_ptr const& to); std::shared_ptr getForMod (ReadView const& base, diff --git a/src/ripple/ledger/impl/ApplyStateTable.cpp b/src/ripple/ledger/impl/ApplyStateTable.cpp index 2844b8a13..320181bc5 100644 --- a/src/ripple/ledger/impl/ApplyStateTable.cpp +++ b/src/ripple/ledger/impl/ApplyStateTable.cpp @@ -71,8 +71,6 @@ ApplyStateTable::apply (OpenView& to, if (deliver) meta.setDeliveredAmount(*deliver); Mods newMod; - // VFALCO NOTE getForMod can insert items with - // Action::cache during the loop. for (auto& item : items_) { SField const* type; @@ -136,7 +134,7 @@ ApplyStateTable::apply (OpenView& to, assert (curNode && origNode); if (curNode->isThreadedType ()) // thread transaction to node item modified - threadTx (meta, curNode, newMod); + threadItem (meta, curNode); STObject prevs (sfPreviousFields); for (auto const& obj : *origNode) @@ -166,7 +164,7 @@ ApplyStateTable::apply (OpenView& to, threadOwners (to, meta, curNode, newMod, j); if (curNode->isThreadedType ()) // always thread to self - threadTx (meta, curNode, newMod); + threadItem (meta, curNode); STObject news (sfNewFields); for (auto const& obj : *curNode) @@ -189,7 +187,7 @@ ApplyStateTable::apply (OpenView& to, // add any new modified nodes to the modification set for (auto& mod : newMod) - update (to, mod.second); + to.rawReplace (mod.second); sMeta = std::make_shared(); meta.addRaw (*sMeta, ter, to.txCount()); @@ -477,13 +475,16 @@ ApplyStateTable::destroyXRP(std::uint64_t feeDrops) //------------------------------------------------------------------------------ +/* Add a tx to the account root's thread + Preconditions: + `to` is an account root in newMods or items_ +*/ bool -ApplyStateTable::threadTx (TxMeta& meta, - std::shared_ptr const& to, - Mods& mods) +ApplyStateTable::threadItem (TxMeta& meta, + std::shared_ptr const& to) { key_type prevTxID; - std::uint32_t prevLgrID; + LedgerIndex prevLgrID; if (! to->thread(meta.getTxID(), meta.getLgrSeq(), prevTxID, prevLgrID)) return false; @@ -501,23 +502,6 @@ std::shared_ptr ApplyStateTable::getForMod (ReadView const& base, key_type const& key, Mods& mods, beast::Journal j) { - auto iter = items_.find (key); - if (iter != items_.end ()) - { - auto& item = iter->second; - if (item.first == Action::erase) - { - // VFALCO We need to think about throwing - // an exception or calling LogicError - JLOG(j.fatal) << - "Trying to thread to deleted node"; - return nullptr; - } - // Track when a node gets modified only by metadata - if (item.first == Action::cache) - item.first = Action::modify; - return item.second; - } { auto miter = mods.find (key); if (miter != mods.end ()) @@ -526,9 +510,28 @@ ApplyStateTable::getForMod (ReadView const& base, return miter->second; } } - auto sle = peek(base, - keylet::unchecked(key)); - if (! sle) + { + auto iter = items_.find (key); + if (iter != items_.end ()) + { + auto const& item = iter->second; + if (item.first == Action::erase) + { + // VFALCO We need to think about throwing + // an exception or calling LogicError + JLOG(j.fatal) << + "Trying to thread to deleted node"; + return nullptr; + } + if (item.first != Action::cache) + return item.second; + + // If it's only cached, then the node is being modified only by + // metadata; fall through and track it in the mods table. + } + } + auto c = base.read (keylet::unchecked (key)); + if (! c) { // VFALCO We need to think about throwing // an exception or calling LogicError @@ -536,6 +539,7 @@ ApplyStateTable::getForMod (ReadView const& base, "ApplyStateTable::getForMod: key not found"; return nullptr; } + auto sle = std::make_shared (*c); mods.emplace(key, sle); return sle; } @@ -558,7 +562,7 @@ ApplyStateTable::threadTx (ReadView const& base, return false; } - return threadTx (meta, sle, mods); + return threadItem (meta, sle); } bool