From 243e181c08db6203f8882bd843f6e858d0d9a25b Mon Sep 17 00:00:00 2001 From: Joe Loser Date: Mon, 21 May 2018 22:39:24 -0400 Subject: [PATCH] Replace uses of dirDelete with ApplyView::dirRemove --- src/ripple/app/tx/impl/CancelCheck.cpp | 14 +- src/ripple/app/tx/impl/CancelTicket.cpp | 11 +- src/ripple/app/tx/impl/CashCheck.cpp | 14 +- src/ripple/app/tx/impl/Escrow.cpp | 45 ++-- src/ripple/app/tx/impl/OfferStream.cpp | 2 +- src/ripple/app/tx/impl/PayChan.cpp | 14 +- src/ripple/app/tx/impl/SetSignerList.cpp | 30 +-- src/ripple/ledger/View.h | 11 - src/ripple/ledger/impl/View.cpp | 267 ++++------------------- 9 files changed, 107 insertions(+), 301 deletions(-) diff --git a/src/ripple/app/tx/impl/CancelCheck.cpp b/src/ripple/app/tx/impl/CancelCheck.cpp index 09143c10f..28be52ffe 100644 --- a/src/ripple/app/tx/impl/CancelCheck.cpp +++ b/src/ripple/app/tx/impl/CancelCheck.cpp @@ -18,8 +18,10 @@ //============================================================================== #include + #include #include +#include #include #include #include @@ -104,22 +106,18 @@ CancelCheck::doApply () if (srcId != dstId) { std::uint64_t const page {(*sleCheck)[sfDestinationNode]}; - TER const ter {dirDelete (view(), true, page, - keylet::ownerDir (dstId), checkId, false, false, viewJ)}; - if (! isTesSuccess (ter)) + if (! view().dirRemove (keylet::ownerDir(dstId), page, checkId, true)) { JLOG(j_.warn()) << "Unable to delete check from destination."; - return ter; + return tefBAD_LEDGER; } } { std::uint64_t const page {(*sleCheck)[sfOwnerNode]}; - TER const ter {dirDelete (view(), true, page, - keylet::ownerDir (srcId), checkId, false, false, viewJ)}; - if (! isTesSuccess (ter)) + if (! view().dirRemove (keylet::ownerDir(srcId), page, checkId, true)) { JLOG(j_.warn()) << "Unable to delete check from owner."; - return ter; + return tefBAD_LEDGER; } } diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index b3d5544e8..e2185300c 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -80,15 +80,18 @@ CancelTicket::doApply () std::uint64_t const hint (sleTicket->getFieldU64 (sfOwnerNode)); - auto viewJ = ctx_.app.journal ("View"); - TER const result = dirDelete (ctx_.view (), false, hint, - keylet::ownerDir (ticket_owner), ticketId, false, (hint == 0), viewJ); + if (! ctx_.view().dirRemove( + keylet::ownerDir(ticket_owner), hint, ticketId, false)) + { + return tefBAD_LEDGER; + } + auto viewJ = ctx_.app.journal ("View"); adjustOwnerCount(view(), view().peek( keylet::account(ticket_owner)), -1, viewJ); ctx_.view ().erase (sleTicket); - return result; + return tesSUCCESS; } } diff --git a/src/ripple/app/tx/impl/CashCheck.cpp b/src/ripple/app/tx/impl/CashCheck.cpp index 526805a3e..acbb0b9f3 100644 --- a/src/ripple/app/tx/impl/CashCheck.cpp +++ b/src/ripple/app/tx/impl/CashCheck.cpp @@ -375,23 +375,21 @@ CashCheck::doApply () if (srcId != account_) { std::uint64_t const page {(*sleCheck)[sfDestinationNode]}; - TER const ter {dirDelete (psb, true, page, - keylet::ownerDir (account_), checkKey, false, false, viewJ)}; - if (! isTesSuccess (ter)) + if (! ctx_.view().dirRemove( + keylet::ownerDir(account_), page, checkKey, true)) { JLOG(j_.warn()) << "Unable to delete check from destination."; - return ter; + return tefBAD_LEDGER; } } // Remove check from check owner's directory. { std::uint64_t const page {(*sleCheck)[sfOwnerNode]}; - TER const ter {dirDelete (psb, true, page, - keylet::ownerDir (srcId), checkKey, false, false, viewJ)}; - if (! isTesSuccess (ter)) + if (! ctx_.view().dirRemove( + keylet::ownerDir(srcId), page, checkKey, true)) { JLOG(j_.warn()) << "Unable to delete check from owner."; - return ter; + return tefBAD_LEDGER; } } // If we succeeded, update the check owner's reserve. diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 4d97b030b..c05b45ebe 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -18,18 +18,20 @@ //============================================================================== #include + #include #include #include #include #include +#include +#include #include #include #include #include #include #include -#include // During an EscrowFinish, the transaction must specify both // a condition and a fulfillment. We track whether that @@ -476,21 +478,21 @@ EscrowFinish::doApply() // Remove escrow from owner directory { auto const page = (*slep)[sfOwnerNode]; - TER const ter = dirDelete(ctx_.view(), true, - page, keylet::ownerDir(account), - k.key, false, page == 0, ctx_.app.journal ("View")); - if (! isTesSuccess(ter)) - return ter; + if (! ctx_.view().dirRemove( + keylet::ownerDir(account), page, k.key, true)) + { + return tefBAD_LEDGER; + } } // Remove escrow from recipient's owner directory, if present. if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) { - TER const ter = dirDelete(ctx_.view(), true, - (*slep)[sfDestinationNode], keylet::ownerDir(destID), - k.key, false, false, ctx_.app.journal ("View")); - if (! isTesSuccess(ter)) - return ter; + auto const page = (*slep)[sfDestinationNode]; + if (! ctx_.view().dirRemove(keylet::ownerDir(destID), page, k.key, true)) + { + return tefBAD_LEDGER; + } } // Transfer amount to destination @@ -561,21 +563,22 @@ EscrowCancel::doApply() // Remove escrow from owner directory { auto const page = (*slep)[sfOwnerNode]; - TER const ter = dirDelete(ctx_.view(), true, - page, keylet::ownerDir(account), - k.key, false, page == 0, ctx_.app.journal ("View")); - if (! isTesSuccess(ter)) - return ter; + if (! ctx_.view().dirRemove( + keylet::ownerDir(account), page, k.key, true)) + { + return tefBAD_LEDGER; + } } // Remove escrow from recipient's owner directory, if present. if (ctx_.view ().rules().enabled(fix1523) && (*slep)[~sfDestinationNode]) { - TER const ter = dirDelete(ctx_.view(), true, - (*slep)[sfDestinationNode], keylet::ownerDir((*slep)[sfDestination]), - k.key, false, false, ctx_.app.journal ("View")); - if (! isTesSuccess(ter)) - return ter; + auto const page = (*slep)[sfDestinationNode]; + if (! ctx_.view().dirRemove( + keylet::ownerDir((*slep)[sfDestination]), page, k.key, true)) + { + return tefBAD_LEDGER; + } } // Transfer amount back to owner, decrement owner count diff --git a/src/ripple/app/tx/impl/OfferStream.cpp b/src/ripple/app/tx/impl/OfferStream.cpp index d1fa55613..dcdeb1dae 100644 --- a/src/ripple/app/tx/impl/OfferStream.cpp +++ b/src/ripple/app/tx/impl/OfferStream.cpp @@ -42,7 +42,7 @@ template void TOfferStreamBase::erase (ApplyView& view) { - // NIKB NOTE This should be using ApplyView::dirDelete, which would + // NIKB NOTE This should be using ApplyView::dirRemove, which would // correctly remove the directory if its the last entry. // Unfortunately this is a protocol breaking change. diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index a91bf5ad7..cffd2da72 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -18,18 +18,18 @@ //============================================================================== #include - #include #include +#include +#include #include -#include #include #include #include #include +#include #include #include -#include namespace ripple { @@ -133,10 +133,10 @@ closeChannel ( // Remove PayChan from owner directory { auto const page = (*slep)[sfOwnerNode]; - TER const ter = dirDelete (view, true, page, keylet::ownerDir (src), - key, false, page == 0, j); - if (!isTesSuccess (ter)) - return ter; + if (! view.dirRemove(keylet::ownerDir(src), page, key, true)) + { + return tefBAD_LEDGER; + } } // Transfer amount back to owner, decrement owner count diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 1773be349..0c3d4e96d 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -18,15 +18,17 @@ //============================================================================== #include + #include -#include -#include -#include -#include -#include #include -#include +#include +#include +#include +#include +#include +#include #include +#include namespace ripple { @@ -287,17 +289,19 @@ SetSignerList::removeSignersFromLedger (Keylet const& accountKeylet, // Remove the node from the account directory. auto const hint = (*signers)[sfOwnerNode]; - auto viewJ = ctx_.app.journal ("View"); - TER const result = dirDelete(ctx_.view(), false, hint, - ownerDirKeylet, signerListKeylet.key, false, (hint == 0), viewJ); + if (! ctx_.view().dirRemove( + ownerDirKeylet, hint, signerListKeylet.key, false)) + { + return tefBAD_LEDGER; + } - if (result == tesSUCCESS) - adjustOwnerCount(view(), - view().peek(accountKeylet), removeFromOwnerCount, viewJ); + auto viewJ = ctx_.app.journal("View"); + adjustOwnerCount( + view(), view().peek(accountKeylet), removeFromOwnerCount, viewJ); ctx_.view().erase (signers); - return result; + return tesSUCCESS; } void diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 965bee0e4..7bc9e0fe3 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -236,17 +236,6 @@ dirAdd (ApplyView& view, std::function fDescriber, beast::Journal j); -// deprecated -TER -dirDelete (ApplyView& view, - const bool bKeepRoot, - std::uint64_t uNodeDir, // Node item is mentioned in. - Keylet const& uRootIndex, - uint256 const& uLedgerIndex, // Item being deleted - const bool bStable, - const bool bSoft, - beast::Journal j); - // VFALCO NOTE Both STAmount parameters should just // be "Amount", a unit-less number. // diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index 79ab3a534..ee7ce57a6 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -911,201 +911,6 @@ dirAdd (ApplyView& view, return uNodeDir; } -// Ledger must be in a state for this to work. -TER -dirDelete (ApplyView& view, - const bool bKeepRoot, // --> True, if we never completely clean up, after we overflow the root node. - std::uint64_t uNodeDir, // --> Node containing entry. - Keylet const& root, // --> The index of the base of the directory. Nodes are based off of this. - uint256 const& uLedgerIndex, // --> Value to remove from directory. - const bool bStable, // --> True, not to change relative order of entries. - const bool bSoft, // --> True, uNodeDir is not hard and fast (pass uNodeDir=0). - beast::Journal j) -{ - if (view.rules().enabled(featureSortedDirectories)) - { - if (view.dirRemove(root, uNodeDir, uLedgerIndex, bKeepRoot)) - return tesSUCCESS; - - return tefBAD_LEDGER; - } - - std::uint64_t uNodeCur = uNodeDir; - SLE::pointer sleNode = - view.peek(keylet::page(root, uNodeCur)); - - if (!sleNode) - { - JLOG (j.warn()) << "dirDelete: no such node:" << - " root=" << to_string (root.key) << - " uNodeDir=" << strHex (uNodeDir) << - " uLedgerIndex=" << to_string (uLedgerIndex); - - if (!bSoft) - { - assert (false); - return tefBAD_LEDGER; - } - else if (uNodeDir < 20) - { - // Go the extra mile. Even if node doesn't exist, try the next node. - - return dirDelete (view, bKeepRoot, - uNodeDir + 1, root, uLedgerIndex, bStable, true, j); - } - else - { - return tefBAD_LEDGER; - } - } - - STVector256 svIndexes = sleNode->getFieldV256 (sfIndexes); - - auto it = std::find (svIndexes.begin (), svIndexes.end (), uLedgerIndex); - - if (svIndexes.end () == it) - { - if (!bSoft) - { - assert (false); - JLOG (j.warn()) << "dirDelete: no such entry"; - return tefBAD_LEDGER; - } - - if (uNodeDir < 20) - { - // Go the extra mile. Even if entry not in node, try the next node. - return dirDelete (view, bKeepRoot, uNodeDir + 1, - root, uLedgerIndex, bStable, true, j); - } - - return tefBAD_LEDGER; - } - - // Remove the element. - if (svIndexes.size () > 1) - { - if (bStable) - { - svIndexes.erase (it); - } - else - { - *it = svIndexes[svIndexes.size () - 1]; - svIndexes.resize (svIndexes.size () - 1); - } - } - else - { - svIndexes.clear (); - } - - sleNode->setFieldV256 (sfIndexes, svIndexes); - view.update(sleNode); - - if (svIndexes.empty ()) - { - // May be able to delete nodes. - std::uint64_t uNodePrevious = sleNode->getFieldU64 (sfIndexPrevious); - std::uint64_t uNodeNext = sleNode->getFieldU64 (sfIndexNext); - - if (!uNodeCur) - { - // Just emptied root node. - - if (!uNodePrevious) - { - // Never overflowed the root node. Delete it. - view.erase(sleNode); - } - // Root overflowed. - else if (bKeepRoot) - { - // If root overflowed and not allowed to delete overflowed root node. - } - else if (uNodePrevious != uNodeNext) - { - // Have more than 2 nodes. Can't delete root node. - } - else - { - // Have only a root node and a last node. - auto sleLast = view.peek(keylet::page(root, uNodeNext)); - - assert (sleLast); - - if (sleLast->getFieldV256 (sfIndexes).empty ()) - { - // Both nodes are empty. - - view.erase (sleNode); // Delete root. - view.erase (sleLast); // Delete last. - } - else - { - // Have an entry, can't delete root node. - } - } - } - // Just emptied a non-root node. - else if (uNodeNext) - { - // Not root and not last node. Can delete node. - - auto slePrevious = view.peek(keylet::page(root, uNodePrevious)); - auto sleNext = view.peek(keylet::page(root, uNodeNext)); - assert (slePrevious); - if (!slePrevious) - { - JLOG (j.warn()) << "dirDelete: previous node is missing"; - return tefBAD_LEDGER; - } - assert (sleNext); - if (!sleNext) - { - JLOG (j.warn()) << "dirDelete: next node is missing"; - return tefBAD_LEDGER; - } - - // Fix previous to point to its new next. - slePrevious->setFieldU64 (sfIndexNext, uNodeNext); - view.update (slePrevious); - - // Fix next to point to its new previous. - sleNext->setFieldU64 (sfIndexPrevious, uNodePrevious); - view.update (sleNext); - - view.erase(sleNode); - } - // Last node. - else if (bKeepRoot || uNodePrevious) - { - // Not allowed to delete last node as root was overflowed. - // Or, have pervious entries preventing complete delete. - } - else - { - // Last and only node besides the root. - auto sleRoot = view.peek(root); - assert (sleRoot); - - if (sleRoot->getFieldV256 (sfIndexes).empty ()) - { - // Both nodes are empty. - - view.erase(sleRoot); // Delete root. - view.erase(sleNode); // Delete last. - } - else - { - // Root has an entry, can't delete. - } - } - } - - return tesSUCCESS; -} - TER trustCreate (ApplyView& view, const bool bSrcHigh, @@ -1216,41 +1021,37 @@ trustDelete (ApplyView& view, beast::Journal j) { // Detect legacy dirs. - bool bLowNode = sleRippleState->isFieldPresent (sfLowNode); - bool bHighNode = sleRippleState->isFieldPresent (sfHighNode); std::uint64_t uLowNode = sleRippleState->getFieldU64 (sfLowNode); std::uint64_t uHighNode = sleRippleState->getFieldU64 (sfHighNode); - TER terResult; JLOG (j.trace()) << "trustDelete: Deleting ripple line: low"; - terResult = dirDelete(view, - false, - uLowNode, - keylet::ownerDir (uLowAccountID), - sleRippleState->key(), - false, - !bLowNode, - j); - if (tesSUCCESS == terResult) - { - JLOG (j.trace()) - << "trustDelete: Deleting ripple line: high"; - terResult = dirDelete (view, - false, - uHighNode, - keylet::ownerDir (uHighAccountID), + if (! view.dirRemove( + keylet::ownerDir(uLowAccountID), + uLowNode, sleRippleState->key(), - false, - !bHighNode, - j); + false)) + { + return tefBAD_LEDGER; + } + + JLOG (j.trace()) + << "trustDelete: Deleting ripple line: high"; + + if (! view.dirRemove( + keylet::ownerDir(uHighAccountID), + uHighNode, + sleRippleState->key(), + false)) + { + return tefBAD_LEDGER; } JLOG (j.trace()) << "trustDelete: Deleting ripple line: state"; view.erase(sleRippleState); - return terResult; + return tesSUCCESS; } TER @@ -1264,22 +1065,32 @@ offerDelete (ApplyView& view, auto owner = sle->getAccountID (sfAccount); // Detect legacy directories. - bool bOwnerNode = sle->isFieldPresent (sfOwnerNode); uint256 uDirectory = sle->getFieldH256 (sfBookDirectory); - TER terResult = dirDelete (view, false, sle->getFieldU64 (sfOwnerNode), - keylet::ownerDir (owner), offerIndex, false, !bOwnerNode, j); - TER terResult2 = dirDelete (view, false, sle->getFieldU64 (sfBookNode), - keylet::page (uDirectory), offerIndex, true, false, j); + if (! view.dirRemove( + keylet::ownerDir(owner), + sle->getFieldU64(sfOwnerNode), + offerIndex, + false)) + { + return tefBAD_LEDGER; + } - if (tesSUCCESS == terResult) - adjustOwnerCount(view, view.peek( - keylet::account(owner)), -1, j); + if (! view.dirRemove( + keylet::page(uDirectory), + sle->getFieldU64(sfBookNode), + offerIndex, + false)) + { + return tefBAD_LEDGER; + } + + adjustOwnerCount(view, view.peek( + keylet::account(owner)), -1, j); view.erase(sle); - return (terResult == tesSUCCESS) ? - terResult2 : terResult; + return tesSUCCESS; } // Direct send w/o fees: