From 7822a28c87a58726db86c6342292a1a5799c3edf Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Tue, 20 Apr 2021 13:35:43 -0700 Subject: [PATCH] Replace dirAdd() calls with dirAppend() or dirInsert(): --- src/ripple/app/tx/impl/CreateCheck.cpp | 27 +++++++++------------ src/ripple/app/tx/impl/CreateTicket.cpp | 16 +++++-------- src/ripple/app/tx/impl/DepositPreauth.cpp | 8 +++---- src/ripple/app/tx/impl/Escrow.cpp | 23 ++++++------------ src/ripple/app/tx/impl/PayChan.cpp | 24 +++++++------------ src/ripple/app/tx/impl/SetSignerList.cpp | 9 ++----- src/ripple/ledger/ApplyView.h | 19 +++++++-------- src/ripple/ledger/View.h | 10 -------- src/ripple/ledger/impl/View.cpp | 29 ++++------------------- 9 files changed, 51 insertions(+), 114 deletions(-) diff --git a/src/ripple/app/tx/impl/CreateCheck.cpp b/src/ripple/app/tx/impl/CreateCheck.cpp index 509bd2a3e1..da5a70e647 100644 --- a/src/ripple/app/tx/impl/CreateCheck.cpp +++ b/src/ripple/app/tx/impl/CreateCheck.cpp @@ -183,10 +183,11 @@ CreateCheck::doApply() return tecINSUFFICIENT_RESERVE; } - // Note that we we use the value from the sequence or ticket as the + // Note that we use the value from the sequence or ticket as the // Check sequence. For more explanation see comments in SeqProxy.h. std::uint32_t const seq = ctx_.tx.getSeqProxy().value(); - auto sleCheck = std::make_shared(keylet::check(account_, seq)); + Keylet const checkKeylet = keylet::check(account_, seq); + auto sleCheck = std::make_shared(checkKeylet); sleCheck->setAccountID(sfAccount, account_); AccountID const dstAccountId = ctx_.tx[sfDestination]; @@ -209,16 +210,13 @@ CreateCheck::doApply() // destination's owner directory. if (dstAccountId != account_) { - auto const page = dirAdd( - view(), + auto const page = view().dirInsert( keylet::ownerDir(dstAccountId), - sleCheck->key(), - false, - describeOwnerDir(dstAccountId), - viewJ); + checkKeylet, + describeOwnerDir(dstAccountId)); JLOG(j_.trace()) << "Adding Check to destination directory " - << to_string(sleCheck->key()) << ": " + << to_string(checkKeylet.key) << ": " << (page ? "success" : "failure"); if (!page) @@ -228,16 +226,13 @@ CreateCheck::doApply() } { - auto const page = dirAdd( - view(), + auto const page = view().dirInsert( keylet::ownerDir(account_), - sleCheck->key(), - false, - describeOwnerDir(account_), - viewJ); + checkKeylet, + describeOwnerDir(account_)); JLOG(j_.trace()) << "Adding Check to owner directory " - << to_string(sleCheck->key()) << ": " + << to_string(checkKeylet.key) << ": " << (page ? "success" : "failure"); if (!page) diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index 6e074d958d..a901231822 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -117,23 +117,19 @@ CreateTicket::doApply() for (std::uint32_t i = 0; i < ticketCount; ++i) { std::uint32_t const curTicketSeq = firstTicketSeq + i; - - SLE::pointer sleTicket = std::make_shared( - ltTICKET, getTicketIndex(account_, curTicketSeq)); + Keylet const ticketKeylet = keylet::ticket(account_, curTicketSeq); + SLE::pointer sleTicket = std::make_shared(ticketKeylet); sleTicket->setAccountID(sfAccount, account_); sleTicket->setFieldU32(sfTicketSequence, curTicketSeq); view().insert(sleTicket); - auto const page = dirAdd( - view(), + auto const page = view().dirInsert( keylet::ownerDir(account_), - sleTicket->key(), - false, - describeOwnerDir(account_), - viewJ); + ticketKeylet, + describeOwnerDir(account_)); - JLOG(j_.trace()) << "Creating ticket " << to_string(sleTicket->key()) + JLOG(j_.trace()) << "Creating ticket " << to_string(ticketKeylet.key) << ": " << (page ? "success" : "failure"); if (!page) diff --git a/src/ripple/app/tx/impl/DepositPreauth.cpp b/src/ripple/app/tx/impl/DepositPreauth.cpp index 61a5539d12..f49490cf8a 100644 --- a/src/ripple/app/tx/impl/DepositPreauth.cpp +++ b/src/ripple/app/tx/impl/DepositPreauth.cpp @@ -127,8 +127,8 @@ DepositPreauth::doApply() // Preclaim already verified that the Preauth entry does not yet exist. // Create and populate the Preauth entry. AccountID const auth{ctx_.tx[sfAuthorize]}; - auto slePreauth = - std::make_shared(keylet::depositPreauth(account_, auth)); + Keylet const preauthKeylet = keylet::depositPreauth(account_, auth); + auto slePreauth = std::make_shared(preauthKeylet); slePreauth->setAccountID(sfAccount, account_); slePreauth->setAccountID(sfAuthorize, auth); @@ -137,11 +137,11 @@ DepositPreauth::doApply() auto viewJ = ctx_.app.journal("View"); auto const page = view().dirInsert( keylet::ownerDir(account_), - slePreauth->key(), + preauthKeylet, describeOwnerDir(account_)); JLOG(j_.trace()) << "Adding DepositPreauth to owner directory " - << to_string(slePreauth->key()) << ": " + << to_string(preauthKeylet.key) << ": " << (page ? "success" : "failure"); if (!page) diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 1dbc6d5a53..bafa6da04e 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -231,8 +231,9 @@ EscrowCreate::doApply() // Create escrow in ledger. Note that we we use the value from the // sequence or ticket. For more explanation see comments in SeqProxy.h. - auto const slep = std::make_shared( - keylet::escrow(account, ctx_.tx.getSeqProxy().value())); + Keylet const escrowKeylet = + keylet::escrow(account, ctx_.tx.getSeqProxy().value()); + auto const slep = std::make_shared(escrowKeylet); (*slep)[sfAmount] = ctx_.tx[sfAmount]; (*slep)[sfAccount] = account; (*slep)[~sfCondition] = ctx_.tx[~sfCondition]; @@ -246,13 +247,8 @@ EscrowCreate::doApply() // Add escrow to sender's owner directory { - auto page = dirAdd( - ctx_.view(), - keylet::ownerDir(account), - slep->key(), - false, - describeOwnerDir(account), - ctx_.app.journal("View")); + auto page = ctx_.view().dirInsert( + keylet::ownerDir(account), escrowKeylet, describeOwnerDir(account)); if (!page) return tecDIR_FULL; (*slep)[sfOwnerNode] = *page; @@ -261,13 +257,8 @@ EscrowCreate::doApply() // If it's not a self-send, add escrow to recipient's owner directory. if (auto const dest = ctx_.tx[sfDestination]; dest != ctx_.tx[sfAccount]) { - auto page = dirAdd( - ctx_.view(), - keylet::ownerDir(dest), - slep->key(), - false, - describeOwnerDir(dest), - ctx_.app.journal("View")); + auto page = ctx_.view().dirInsert( + keylet::ownerDir(dest), escrowKeylet, describeOwnerDir(dest)); if (!page) return tecDIR_FULL; (*slep)[sfDestinationNode] = *page; diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index ba52b6260f..869b1f472d 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -246,8 +246,10 @@ PayChanCreate::doApply() // // Note that we we use the value from the sequence or ticket as the // payChan sequence. For more explanation see comments in SeqProxy.h. - auto const slep = std::make_shared( - keylet::payChan(account, dst, ctx_.tx.getSeqProxy().value())); + Keylet const payChanKeylet = + keylet::payChan(account, dst, ctx_.tx.getSeqProxy().value()); + auto const slep = std::make_shared(payChanKeylet); + // Funds held in this channel (*slep)[sfAmount] = ctx_.tx[sfAmount]; // Amount channel has already paid @@ -264,13 +266,10 @@ PayChanCreate::doApply() // Add PayChan to owner directory { - auto const page = dirAdd( - ctx_.view(), + auto const page = ctx_.view().dirInsert( keylet::ownerDir(account), - slep->key(), - false, - describeOwnerDir(account), - ctx_.app.journal("View")); + payChanKeylet, + describeOwnerDir(account)); if (!page) return tecDIR_FULL; (*slep)[sfOwnerNode] = *page; @@ -279,13 +278,8 @@ PayChanCreate::doApply() // Add PayChan to the recipient's owner directory if (ctx_.view().rules().enabled(fixPayChanRecipientOwnerDir)) { - auto const page = dirAdd( - ctx_.view(), - keylet::ownerDir(dst), - slep->key(), - false, - describeOwnerDir(dst), - ctx_.app.journal("View")); + auto const page = ctx_.view().dirInsert( + keylet::ownerDir(dst), payChanKeylet, describeOwnerDir(dst)); if (!page) return tecDIR_FULL; (*slep)[sfDestinationNode] = *page; diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index f27fbcc793..e72f37676b 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -342,13 +342,8 @@ SetSignerList::replaceSignerList() auto viewJ = ctx_.app.journal("View"); // Add the signer list to the account's directory. - auto const page = dirAdd( - ctx_.view(), - ownerDirKeylet, - signerListKeylet.key, - false, - describeOwnerDir(account_), - viewJ); + auto const page = ctx_.view().dirInsert( + ownerDirKeylet, signerListKeylet, describeOwnerDir(account_)); JLOG(j_.trace()) << "Create signer list for account " << toBase58(account_) << ": " << (page ? "success" : "failure"); diff --git a/src/ripple/ledger/ApplyView.h b/src/ripple/ledger/ApplyView.h index 25d01fa276..b99a22ae16 100644 --- a/src/ripple/ledger/ApplyView.h +++ b/src/ripple/ledger/ApplyView.h @@ -273,22 +273,19 @@ public: allowable pages. */ /** @{ */ - std::optional - dirAppend( - Keylet const& directory, - uint256 const& key, - std::function const&)> const& describe) - { - return dirAdd(true, directory, key, describe); - } - std::optional dirAppend( Keylet const& directory, Keylet const& key, std::function const&)> const& describe) { - return dirAppend(directory, key.key, describe); + if (key.type != ltOFFER) + { + assert(!"Only Offers are appended to book directories. " + "Call dirInsert() instead."); + return std::nullopt; + } + return dirAdd(true, directory, key.key, describe); } /** @} */ @@ -325,7 +322,7 @@ public: Keylet const& key, std::function const&)> const& describe) { - return dirInsert(directory, key.key, describe); + return dirAdd(false, directory, key.key, describe); } /** @} */ diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 32df1f63a0..d74a0aec67 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -249,16 +249,6 @@ dirNext( [[nodiscard]] std::function describeOwnerDir(AccountID const& account); -// deprecated -std::optional -dirAdd( - ApplyView& view, - Keylet const& uRootIndex, - uint256 const& uLedgerIndex, - bool strictOrder, - std::function fDescriber, - 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 d6a4dec630..ec4bfab332 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -721,21 +721,6 @@ describeOwnerDir(AccountID const& account) }; } -std::optional -dirAdd( - ApplyView& view, - Keylet const& dir, - uint256 const& uLedgerIndex, - bool strictOrder, - std::function fDescriber, - beast::Journal j) -{ - if (strictOrder) - return view.dirAppend(dir, uLedgerIndex, fDescriber); - - return view.dirInsert(dir, uLedgerIndex, fDescriber); -} - TER trustCreate( ApplyView& view, @@ -765,24 +750,18 @@ trustCreate( auto const sleRippleState = std::make_shared(ltRIPPLE_STATE, uIndex); view.insert(sleRippleState); - auto lowNode = dirAdd( - view, + auto lowNode = view.dirInsert( keylet::ownerDir(uLowAccountID), sleRippleState->key(), - false, - describeOwnerDir(uLowAccountID), - j); + describeOwnerDir(uLowAccountID)); if (!lowNode) return tecDIR_FULL; - auto highNode = dirAdd( - view, + auto highNode = view.dirInsert( keylet::ownerDir(uHighAccountID), sleRippleState->key(), - false, - describeOwnerDir(uHighAccountID), - j); + describeOwnerDir(uHighAccountID)); if (!highNode) return tecDIR_FULL;