diff --git a/Builds/VisualStudio2015/RippleD.vcxproj b/Builds/VisualStudio2015/RippleD.vcxproj index d0f2b8e98..0ad8cb7dd 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj +++ b/Builds/VisualStudio2015/RippleD.vcxproj @@ -2117,6 +2117,10 @@ True True + + True + True + True True diff --git a/Builds/VisualStudio2015/RippleD.vcxproj.filters b/Builds/VisualStudio2015/RippleD.vcxproj.filters index f219baaac..c5f03a535 100644 --- a/Builds/VisualStudio2015/RippleD.vcxproj.filters +++ b/Builds/VisualStudio2015/RippleD.vcxproj.filters @@ -2760,6 +2760,9 @@ ripple\ledger\impl + + ripple\ledger\impl + ripple\ledger\impl diff --git a/src/ripple/app/main/Amendments.cpp b/src/ripple/app/main/Amendments.cpp index 7a3d9653c..c286e927b 100644 --- a/src/ripple/app/main/Amendments.cpp +++ b/src/ripple/app/main/Amendments.cpp @@ -55,7 +55,8 @@ supportedAmendments () { "86E83A7D2ECE3AD5FA87AB2195AE015C950469ABF0B72EAACED318F74886AE90 CryptoConditionsSuite" }, { "42EEA5E28A97824821D4EF97081FE36A54E9593C6E4F20CBAE098C69D2E072DC fix1373" }, { "DC9CA96AEA1DCF83E527D1AFC916EFAF5D27388ECA4060A88817C1238CAEE0BF EnforceInvariants" }, - { "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" } + { "3012E8230864E95A58C60FD61430D7E1B4D3353195F2981DC12B0C7C0950FFAC FlowCross" }, + { "CC5ABAE4F3EC92E94A59B1908C2BE82D2228B6485C00AFF8F22DF930D89C194E SortedDirectories" } }; } diff --git a/src/ripple/app/tx/impl/CancelTicket.cpp b/src/ripple/app/tx/impl/CancelTicket.cpp index d4c092661..9f8bb0a2d 100644 --- a/src/ripple/app/tx/impl/CancelTicket.cpp +++ b/src/ripple/app/tx/impl/CancelTicket.cpp @@ -79,7 +79,7 @@ CancelTicket::doApply () auto viewJ = ctx_.app.journal ("View"); TER const result = dirDelete (ctx_.view (), false, hint, - getOwnerDirIndex (ticket_owner), ticketId, false, (hint == 0), viewJ); + keylet::ownerDir (ticket_owner), ticketId, false, (hint == 0), viewJ); adjustOwnerCount(view(), view().peek( keylet::account(ticket_owner)), -1, viewJ); diff --git a/src/ripple/app/tx/impl/CreateOffer.cpp b/src/ripple/app/tx/impl/CreateOffer.cpp index 4fd151dd4..320e03f0d 100644 --- a/src/ripple/app/tx/impl/CreateOffer.cpp +++ b/src/ripple/app/tx/impl/CreateOffer.cpp @@ -1291,74 +1291,74 @@ CreateOffer::applyGuts (Sandbox& sb, Sandbox& sbCancel) // We need to place the remainder of the offer into its order book. auto const offer_index = getOfferIndex (account_, uSequence); - std::uint64_t uOwnerNode; - // Add offer to owner's directory. - std::tie(result, std::ignore) = dirAdd(sb, uOwnerNode, - keylet::ownerDir (account_), offer_index, - describeOwnerDir (account_), viewJ); + auto const ownerNode = dirAdd(sb, keylet::ownerDir (account_), + offer_index, false, describeOwnerDir (account_), viewJ); - if (result == tesSUCCESS) - { - // Update owner count. - adjustOwnerCount(sb, sleCreator, 1, viewJ); - - JLOG (j_.trace()) << - "adding to book: " << to_string (saTakerPays.issue ()) << - " : " << to_string (saTakerGets.issue ()); - - Book const book { saTakerPays.issue(), saTakerGets.issue() }; - std::uint64_t uBookNode; - bool isNewBook; - - // Add offer to order book, using the original rate - // before any crossing occured. - auto dir = keylet::quality (keylet::book (book), uRate); - - std::tie(result, isNewBook) = dirAdd (sb, uBookNode, - dir, offer_index, [&](SLE::ref sle) - { - sle->setFieldH160 (sfTakerPaysCurrency, - saTakerPays.issue().currency); - sle->setFieldH160 (sfTakerPaysIssuer, - saTakerPays.issue().account); - sle->setFieldH160 (sfTakerGetsCurrency, - saTakerGets.issue().currency); - sle->setFieldH160 (sfTakerGetsIssuer, - saTakerGets.issue().account); - sle->setFieldU64 (sfExchangeRate, uRate); - }, viewJ); - - if (result == tesSUCCESS) - { - auto sleOffer = std::make_shared(ltOFFER, offer_index); - sleOffer->setAccountID (sfAccount, account_); - sleOffer->setFieldU32 (sfSequence, uSequence); - sleOffer->setFieldH256 (sfBookDirectory, dir.key); - sleOffer->setFieldAmount (sfTakerPays, saTakerPays); - sleOffer->setFieldAmount (sfTakerGets, saTakerGets); - sleOffer->setFieldU64 (sfOwnerNode, uOwnerNode); - sleOffer->setFieldU64 (sfBookNode, uBookNode); - if (expiration) - sleOffer->setFieldU32 (sfExpiration, *expiration); - if (bPassive) - sleOffer->setFlag (lsfPassive); - if (bSell) - sleOffer->setFlag (lsfSell); - sb.insert(sleOffer); - - if (isNewBook) - ctx_.app.getOrderBookDB().addOrderBook(book); - } - } - - if (result != tesSUCCESS) + if (!ownerNode) { JLOG (j_.debug()) << - "final result: " << transToken (result); + "final result: failed to add offer to owner's directory"; + return { tecDIR_FULL, true }; } - return { result, true }; + // Update owner count. + adjustOwnerCount(sb, sleCreator, 1, viewJ); + + JLOG (j_.trace()) << + "adding to book: " << to_string (saTakerPays.issue ()) << + " : " << to_string (saTakerGets.issue ()); + + Book const book { saTakerPays.issue(), saTakerGets.issue() }; + + // Add offer to order book, using the original rate + // before any crossing occured. + auto dir = keylet::quality (keylet::book (book), uRate); + bool const bookExisted = static_cast(sb.peek (dir)); + + auto const bookNode = dirAdd (sb, dir, offer_index, true, + [&](SLE::ref sle) + { + sle->setFieldH160 (sfTakerPaysCurrency, + saTakerPays.issue().currency); + sle->setFieldH160 (sfTakerPaysIssuer, + saTakerPays.issue().account); + sle->setFieldH160 (sfTakerGetsCurrency, + saTakerGets.issue().currency); + sle->setFieldH160 (sfTakerGetsIssuer, + saTakerGets.issue().account); + sle->setFieldU64 (sfExchangeRate, uRate); + }, viewJ); + + if (!bookNode) + { + JLOG (j_.debug()) << + "final result: failed to add offer to book"; + return { tecDIR_FULL, true }; + } + + auto sleOffer = std::make_shared(ltOFFER, offer_index); + sleOffer->setAccountID (sfAccount, account_); + sleOffer->setFieldU32 (sfSequence, uSequence); + sleOffer->setFieldH256 (sfBookDirectory, dir.key); + sleOffer->setFieldAmount (sfTakerPays, saTakerPays); + sleOffer->setFieldAmount (sfTakerGets, saTakerGets); + sleOffer->setFieldU64 (sfOwnerNode, *ownerNode); + sleOffer->setFieldU64 (sfBookNode, *bookNode); + if (expiration) + sleOffer->setFieldU32 (sfExpiration, *expiration); + if (bPassive) + sleOffer->setFlag (lsfPassive); + if (bSell) + sleOffer->setFlag (lsfSell); + sb.insert(sleOffer); + + if (!bookExisted) + ctx_.app.getOrderBookDB().addOrderBook(book); + + JLOG (j_.debug()) << "final result: success"; + + return { tesSUCCESS, true }; } TER diff --git a/src/ripple/app/tx/impl/CreateTicket.cpp b/src/ripple/app/tx/impl/CreateTicket.cpp index afcbacd28..2220899a7 100644 --- a/src/ripple/app/tx/impl/CreateTicket.cpp +++ b/src/ripple/app/tx/impl/CreateTicket.cpp @@ -99,27 +99,24 @@ CreateTicket::doApply () sleTicket->setAccountID (sfTarget, target_account); } - std::uint64_t hint; - auto viewJ = ctx_.app.journal ("View"); - auto result = dirAdd(view(), hint, keylet::ownerDir (account_), - sleTicket->key(), describeOwnerDir (account_), viewJ); + auto const page = dirAdd(view(), keylet::ownerDir (account_), + sleTicket->key(), false, describeOwnerDir (account_), viewJ); JLOG(j_.trace()) << "Creating ticket " << to_string (sleTicket->key()) << - ": " << transHuman (result.first); + ": " << (page ? "success" : "failure"); - if (result.first == tesSUCCESS) - { - sleTicket->setFieldU64(sfOwnerNode, hint); + if (!page) + return tecDIR_FULL; - // If we succeeded, the new entry counts agains the - // creator's reserve. - adjustOwnerCount(view(), sle, 1, viewJ); - } + sleTicket->setFieldU64(sfOwnerNode, *page); - return result.first; + // If we succeeded, the new entry counts agains the + // creator's reserve. + adjustOwnerCount(view(), sle, 1, viewJ); + return tesSUCCESS; } } diff --git a/src/ripple/app/tx/impl/Escrow.cpp b/src/ripple/app/tx/impl/Escrow.cpp index 18bc1cc85..3b9abf014 100644 --- a/src/ripple/app/tx/impl/Escrow.cpp +++ b/src/ripple/app/tx/impl/Escrow.cpp @@ -255,13 +255,11 @@ EscrowCreate::doApply() // Add escrow to owner directory { - uint64_t page; - auto result = dirAdd(ctx_.view(), page, - keylet::ownerDir(account), slep->key(), - describeOwnerDir(account), ctx_.app.journal ("View")); - if (! isTesSuccess(result.first)) - return result.first; - (*slep)[sfOwnerNode] = page; + auto page = dirAdd(ctx_.view(), keylet::ownerDir(account), slep->key(), + false, describeOwnerDir(account), ctx_.app.journal ("View")); + if (!page) + return tecDIR_FULL; + (*slep)[sfOwnerNode] = *page; } // Deduct owner's balance, increment owner count @@ -431,7 +429,7 @@ EscrowFinish::doApply() { auto const page = (*slep)[sfOwnerNode]; TER const ter = dirDelete(ctx_.view(), true, - page, keylet::ownerDir(account).key, + page, keylet::ownerDir(account), k.key, false, page == 0, ctx_.app.journal ("View")); if (! isTesSuccess(ter)) return ter; @@ -497,7 +495,7 @@ EscrowCancel::doApply() { auto const page = (*slep)[sfOwnerNode]; TER const ter = dirDelete(ctx_.view(), true, - page, keylet::ownerDir(account).key, + page, keylet::ownerDir(account), k.key, false, page == 0, ctx_.app.journal ("View")); if (! isTesSuccess(ter)) return ter; diff --git a/src/ripple/app/tx/impl/PayChan.cpp b/src/ripple/app/tx/impl/PayChan.cpp index 4aeb509dd..f818bbdc9 100644 --- a/src/ripple/app/tx/impl/PayChan.cpp +++ b/src/ripple/app/tx/impl/PayChan.cpp @@ -134,7 +134,7 @@ closeChannel ( // Remove PayChan from owner directory { auto const page = (*slep)[sfOwnerNode]; - TER const ter = dirDelete (view, true, page, keylet::ownerDir (src).key, + TER const ter = dirDelete (view, true, page, keylet::ownerDir (src), key, false, page == 0, j); if (!isTesSuccess (ter)) return ter; @@ -239,13 +239,11 @@ PayChanCreate::doApply() // Add PayChan to owner directory { - uint64_t page; - auto result = dirAdd (ctx_.view (), page, keylet::ownerDir (account), - slep->key (), describeOwnerDir (account), - ctx_.app.journal ("View")); - if (!isTesSuccess (result.first)) - return result.first; - (*slep)[sfOwnerNode] = page; + auto page = dirAdd (ctx_.view(), keylet::ownerDir(account), slep->key(), + false, describeOwnerDir (account), ctx_.app.journal ("View")); + if (!page) + return tecDIR_FULL; + (*slep)[sfOwnerNode] = *page; } // Deduct owner's balance, increment owner count diff --git a/src/ripple/app/tx/impl/SetSignerList.cpp b/src/ripple/app/tx/impl/SetSignerList.cpp index 522327fc0..4a03ec36e 100644 --- a/src/ripple/app/tx/impl/SetSignerList.cpp +++ b/src/ripple/app/tx/impl/SetSignerList.cpp @@ -236,24 +236,21 @@ SetSignerList::replaceSignerList () auto viewJ = ctx_.app.journal ("View"); // Add the signer list to the account's directory. - std::uint64_t hint; - - auto result = dirAdd(ctx_.view (), hint, ownerDirKeylet, - signerListKeylet.key, describeOwnerDir (account_), viewJ); + auto page = dirAdd(ctx_.view (), ownerDirKeylet, + signerListKeylet.key, false, describeOwnerDir (account_), viewJ); JLOG(j_.trace()) << "Create signer list for account " << - toBase58(account_) << ": " << transHuman (result.first); + toBase58(account_) << ": " << (page ? "success" : "failure"); - if (result.first == tesSUCCESS) - { - signerList->setFieldU64 (sfOwnerNode, hint); + if (!page) + return tecDIR_FULL; - // If we succeeded, the new entry counts against the - // creator's reserve. - adjustOwnerCount(view(), sle, addedOwnerCount, viewJ); - } + signerList->setFieldU64 (sfOwnerNode, *page); - return result.first; + // If we succeeded, the new entry counts against the + // creator's reserve. + adjustOwnerCount(view(), sle, addedOwnerCount, viewJ); + return tesSUCCESS; } TER @@ -293,7 +290,7 @@ SetSignerList::removeSignersFromLedger (Keylet const& accountKeylet, auto viewJ = ctx_.app.journal ("View"); TER const result = dirDelete(ctx_.view(), false, hint, - ownerDirKeylet.key, signerListKeylet.key, false, (hint == 0), viewJ); + ownerDirKeylet, signerListKeylet.key, false, (hint == 0), viewJ); if (result == tesSUCCESS) adjustOwnerCount(view(), diff --git a/src/ripple/ledger/ApplyView.h b/src/ripple/ledger/ApplyView.h index 40c83e4b5..c0e1a363c 100644 --- a/src/ripple/ledger/ApplyView.h +++ b/src/ripple/ledger/ApplyView.h @@ -22,6 +22,7 @@ #include #include +#include namespace ripple { @@ -102,8 +103,16 @@ operator&(ApplyFlags const& lhs, class ApplyView : public ReadView { -public: +private: + /** Add an entry to a directory using the specified insert strategy */ + boost::optional + dirAdd ( + bool preserveOrder, + Keylet const& directory, + uint256 const& key, + std::function const&)> const& describe); +public: ApplyView () = default; /** Returns the tx apply flags. @@ -212,6 +221,113 @@ public: std::uint32_t cur, std::uint32_t next) {}; + /** Append an entry to a directory + + Entries in the directory will be stored in order of insertion, i.e. new + entries will always be added at the tail end of the last page. + + @param directory the base of the directory + @param key the entry to insert + @param describe callback to add required entries to a new page + + @return a \c boost::optional which, if insertion was successful, + will contain the page number in which the item was stored. + + @note this function may create a page (including a root page), if no + page with space is available. This function will only fail if the + page counter exceeds the protocol-defined maximum number of + allowable pages. + */ + /** @{ */ + boost::optional + dirAppend ( + Keylet const& directory, + uint256 const& key, + std::function const&)> const& describe) + { + return dirAdd (true, directory, key, describe); + } + + boost::optional + dirAppend ( + Keylet const& directory, + Keylet const& key, + std::function const&)> const& describe) + { + return dirAppend (directory, key.key, describe); + } + /** @} */ + + /** Insert an entry to a directory + + Entries in the directory will be stored in a semi-random order, but + each page will be maintained in sorted order. + + @param directory the base of the directory + @param key the entry to insert + @param describe callback to add required entries to a new page + + @return a \c boost::optional which, if insertion was successful, + will contain the page number in which the item was stored. + + @note this function may create a page (including a root page), if no + page with space is available.this function will only fail if the + page counter exceeds the protocol-defined maximum number of + allowable pages. + */ + /** @{ */ + boost::optional + dirInsert ( + Keylet const& directory, + uint256 const& key, + std::function const&)> const& describe) + { + return dirAdd (false, directory, key, describe); + } + + boost::optional + dirInsert ( + Keylet const& directory, + Keylet const& key, + std::function const&)> const& describe) + { + return dirInsert (directory, key.key, describe); + } + /** @} */ + + /** Remove an entry from a directory + + @param directory the base of the directory + @param page the page number for this page + @param key the entry to remove + @param keepRoot if deleting the last entry, don't + delete the root page (i.e. the directory itself). + + @return \c true if the entry was found and deleted and + \c false otherwise. + + @note This function will remove zero or more pages from the directory; + the root page will not be deleted even if it is empty, unless + \p keepRoot is not set and the directory is empty. + */ + /** @{ */ + bool + dirRemove ( + Keylet const& directory, + std::uint64_t page, + uint256 const& key, + bool keepRoot); + + bool + dirRemove ( + Keylet const& directory, + std::uint64_t page, + Keylet const& key, + bool keepRoot) + { + return dirRemove (directory, page, key.key, keepRoot); + } + /** @} */ }; } // ripple diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 47dc64127..c249c34ad 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -223,36 +223,21 @@ dirNext (ApplyView& view, std::function describeOwnerDir(AccountID const& account); -// <-- uNodeDir: For deletion, present to make dirDelete efficient. -// --> uRootIndex: The index of the base of the directory. Nodes are based off of this. -// --> uLedgerIndex: Value to add to directory. -// Only append. This allow for things that watch append only structure to just monitor from the last node on ward. -// Within a node with no deletions order of elements is sequential. Otherwise, order of elements is random. - -/** Add an entry to directory, creating the directory if necessary - - @param uNodeDir node of entry - makes deletion efficient - @param uRootIndex The index of the base of the directory. - Nodes are based off of this. - @param uLedgerIndex Value to add to directory. - - @return a pair containing a code indicating success or - failure, and if successful, a boolean indicating - whether the directory was just created. -*/ -std::pair +// deprecated +boost::optional dirAdd (ApplyView& view, - std::uint64_t& uNodeDir, // Node of entry. Keylet const& uRootIndex, uint256 const& uLedgerIndex, + bool strictOrder, std::function fDescriber, beast::Journal j); +// deprecated TER dirDelete (ApplyView& view, const bool bKeepRoot, - const std::uint64_t& uNodeDir, // Node item is mentioned in. - uint256 const& uRootIndex, + std::uint64_t uNodeDir, // Node item is mentioned in. + Keylet const& uRootIndex, uint256 const& uLedgerIndex, // Item being deleted const bool bStable, const bool bSoft, diff --git a/src/ripple/ledger/impl/ApplyView.cpp b/src/ripple/ledger/impl/ApplyView.cpp new file mode 100644 index 000000000..d985c3f0d --- /dev/null +++ b/src/ripple/ledger/impl/ApplyView.cpp @@ -0,0 +1,275 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012, 2013 Ripple Labs Inc. + + Permission to use, copy, modify, and/or distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include +#include +#include +#include +#include + +namespace ripple { + +boost::optional +ApplyView::dirAdd ( + bool preserveOrder, + Keylet const& directory, + uint256 const& key, + std::function const&)> const& describe) +{ + auto root = peek(directory); + + if (! root) + { + // No root, make it. + root = std::make_shared(directory); + root->setFieldH256 (sfRootIndex, directory.key); + describe (root); + + STVector256 v; + v.push_back (key); + root->setFieldV256 (sfIndexes, v); + + insert (root); + return std::uint64_t{0}; + } + + std::uint64_t page = root->getFieldU64(sfIndexPrevious); + + auto node = root; + + if (page) + { + node = peek (keylet::page(directory, page)); + if (!node) + LogicError ("Directory chain: root back-pointer broken."); + } + + auto indexes = node->getFieldV256(sfIndexes); + + // If there's space, we use it: + if (indexes.size () < dirNodeMaxEntries) + { + if (preserveOrder) + { + if (std::find(indexes.begin(), indexes.end(), key) != indexes.end()) + LogicError ("dirInsert: double insertion"); + + indexes.push_back(key); + } + else + { + // We can't be sure if this page is already sorted because + // it may be a legacy page we haven't yet touched. Take + // the time to sort it. + std::sort (indexes.begin(), indexes.end()); + + auto pos = std::lower_bound(indexes.begin(), indexes.end(), key); + + if (pos != indexes.end() && key == *pos) + LogicError ("dirInsert: double insertion"); + + indexes.insert (pos, key); + } + + node->setFieldV256 (sfIndexes, indexes); + update(node); + return page; + } + + // Check whether we're out of pages. + if (++page >= dirNodeMaxPages) + return boost::none; + + // We are about to create a new node; we'll link it to + // the chain first: + node->setFieldU64 (sfIndexNext, page); + update(node); + + root->setFieldU64 (sfIndexPrevious, page); + update(root); + + // Insert the new key: + indexes.clear(); + indexes.push_back (key); + + node = std::make_shared(keylet::page(directory, page)); + node->setFieldH256 (sfRootIndex, directory.key); + node->setFieldV256 (sfIndexes, indexes); + + // Save some space by not specifying the value 0 since + // it's the default. + if (page != 1) + node->setFieldU64 (sfIndexPrevious, page - 1); + describe (node); + insert (node); + + return page; +} + +bool +ApplyView::dirRemove ( + Keylet const& directory, + std::uint64_t page, + uint256 const& key, + bool keepRoot) +{ + auto node = peek(keylet::page(directory, page)); + + if (!node) + return false; + + std::uint64_t constexpr rootPage = 0; + + { + auto entries = node->getFieldV256(sfIndexes); + + auto it = std::find(entries.begin(), entries.end(), key); + + if (entries.end () == it) + return false; + + // We always preserve the relative order when we remove. + entries.erase(it); + + node->setFieldV256(sfIndexes, entries); + update(node); + + if (!entries.empty()) + return true; + } + + // The current page is now empty; check if it can be + // deleted, and, if so, whether the entire directory + // can now be removed. + auto prevPage = node->getFieldU64(sfIndexPrevious); + auto nextPage = node->getFieldU64(sfIndexNext); + + // The first page is the directory's root node and is + // treated specially: it can never be deleted even if + // it is empty, unless we plan on removing the entire + // directory. + if (page == rootPage) + { + if (nextPage == page && prevPage != page) + LogicError ("Directory chain: fwd link broken"); + + if (prevPage == page && nextPage != page) + LogicError ("Directory chain: rev link broken"); + + // Older versions of the code would, in some cases, + // allow the last page to be empty. Remove such + // pages if we stumble on them: + if (nextPage == prevPage && nextPage != page) + { + auto last = peek(keylet::page(directory, nextPage)); + if (!last) + LogicError ("Directory chain: fwd link broken."); + + if (last->getFieldV256 (sfIndexes).empty()) + { + // Update the first page's linked list and + // mark it as updated. + node->setFieldU64 (sfIndexNext, page); + node->setFieldU64 (sfIndexPrevious, page); + update(node); + + // And erase the empty last page: + erase(last); + + // Make sure our local values reflect the + // updated information: + nextPage = page; + prevPage = page; + } + } + + if (keepRoot) + return true; + + // If there's no other pages, erase the root: + if (nextPage == page && prevPage == page) + erase(node); + + return true; + } + + // This can never happen for nodes other than the root: + if (nextPage == page) + LogicError ("Directory chain: fwd link broken"); + + if (prevPage == page) + LogicError ("Directory chain: rev link broken"); + + // This node isn't the root, so it can either be in the + // middle of the list, or at the end. Unlink it first + // and then check if that leaves the list with only a + // root: + auto prev = peek(keylet::page(directory, prevPage)); + if (!prev) + LogicError ("Directory chain: fwd link broken."); + // Fix previous to point to its new next. + prev->setFieldU64(sfIndexNext, nextPage); + update (prev); + + auto next = peek(keylet::page(directory, nextPage)); + if (!next) + LogicError ("Directory chain: rev link broken."); + // Fix next to point to its new previous. + next->setFieldU64(sfIndexPrevious, prevPage); + update(next); + + // The page is no longer linked. Delete it. + erase(node); + + // Check whether the next page is the last page and, if + // so, whether it's empty. If it is, delete it. + if (nextPage != rootPage && + next->getFieldU64 (sfIndexNext) == rootPage && + next->getFieldV256 (sfIndexes).empty()) + { + // Since next doesn't point to the root, it + // can't be pointing to prev. + erase(next); + + // The previous page is now the last page: + prev->setFieldU64(sfIndexNext, rootPage); + update (prev); + + // And the root points to the the last page: + auto root = peek(keylet::page(directory, rootPage)); + if (!root) + LogicError ("Directory chain: root link broken."); + root->setFieldU64(sfIndexPrevious, prevPage); + update (root); + + nextPage = rootPage; + } + + // If we're not keeping the root, then check to see if + // it's left empty. If so, delete it as well. + if (!keepRoot && nextPage == rootPage && prevPage == rootPage) + { + if (prev->getFieldV256 (sfIndexes).empty()) + erase(prev); + } + + return true; +} + +} // ripple diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index e76286b51..219402c35 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -19,11 +19,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -803,19 +805,28 @@ describeOwnerDir(AccountID const& account) }; } -std::pair +boost::optional dirAdd (ApplyView& view, - std::uint64_t& uNodeDir, Keylet const& dir, uint256 const& uLedgerIndex, + bool strictOrder, std::function fDescriber, beast::Journal j) { + if (view.rules().enabled(featureSortedDirectories)) + { + if (strictOrder) + return view.dirAppend(dir, uLedgerIndex, fDescriber); + else + return view.dirInsert(dir, uLedgerIndex, fDescriber); + } + JLOG (j.trace()) << "dirAdd:" << " dir=" << to_string (dir.key) << " uLedgerIndex=" << to_string (uLedgerIndex); auto sleRoot = view.peek(dir); + std::uint64_t uNodeDir = 0; if (! sleRoot) { @@ -833,9 +844,7 @@ dirAdd (ApplyView& view, "dirAdd: created root " << to_string (dir.key) << " for entry " << to_string (uLedgerIndex); - uNodeDir = 0; - - return { tesSUCCESS, true }; + return uNodeDir; } SLE::pointer sleNode; @@ -865,7 +874,7 @@ dirAdd (ApplyView& view, // Add to new node. else if (!++uNodeDir) { - return { tecDIR_FULL, false }; + return boost::none; } else { @@ -901,28 +910,36 @@ dirAdd (ApplyView& view, JLOG (j.trace()) << "dirAdd: appending: Node: " << strHex (uNodeDir); - return { tesSUCCESS, false }; + 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. - const std::uint64_t& uNodeDir, // --> Node containing entry. - uint256 const& uRootIndex, // --> The index of the base of the directory. Nodes are based off of this. + 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(uRootIndex, uNodeCur)); + view.peek(keylet::page(root, uNodeCur)); if (!sleNode) { JLOG (j.warn()) << "dirDelete: no such node:" << - " uRootIndex=" << to_string (uRootIndex) << + " root=" << to_string (root.key) << " uNodeDir=" << strHex (uNodeDir) << " uLedgerIndex=" << to_string (uLedgerIndex); @@ -936,7 +953,7 @@ dirDelete (ApplyView& view, // Go the extra mile. Even if node doesn't exist, try the next node. return dirDelete (view, bKeepRoot, - uNodeDir + 1, uRootIndex, uLedgerIndex, bStable, true, j); + uNodeDir + 1, root, uLedgerIndex, bStable, true, j); } else { @@ -961,7 +978,7 @@ dirDelete (ApplyView& view, { // Go the extra mile. Even if entry not in node, try the next node. return dirDelete (view, bKeepRoot, uNodeDir + 1, - uRootIndex, uLedgerIndex, bStable, true, j); + root, uLedgerIndex, bStable, true, j); } return tefBAD_LEDGER; @@ -1015,7 +1032,7 @@ dirDelete (ApplyView& view, else { // Have only a root node and a last node. - auto sleLast = view.peek(keylet::page(uRootIndex, uNodeNext)); + auto sleLast = view.peek(keylet::page(root, uNodeNext)); assert (sleLast); @@ -1037,9 +1054,8 @@ dirDelete (ApplyView& view, { // Not root and not last node. Can delete node. - auto slePrevious = - view.peek(keylet::page(uRootIndex, uNodePrevious)); - auto sleNext = view.peek(keylet::page(uRootIndex, uNodeNext)); + auto slePrevious = view.peek(keylet::page(root, uNodePrevious)); + auto sleNext = view.peek(keylet::page(root, uNodeNext)); assert (slePrevious); if (!slePrevious) { @@ -1072,8 +1088,7 @@ dirDelete (ApplyView& view, else { // Last and only node besides the root. - auto sleRoot = view.peek (keylet::page(uRootIndex)); - + auto sleRoot = view.peek(root); assert (sleRoot); if (sleRoot->getFieldV256 (sfIndexes).empty ()) @@ -1122,86 +1137,77 @@ trustCreate (ApplyView& view, ltRIPPLE_STATE, uIndex); view.insert (sleRippleState); - std::uint64_t uLowNode; - std::uint64_t uHighNode; + auto lowNode = dirAdd (view, keylet::ownerDir (uLowAccountID), + sleRippleState->key(), false, describeOwnerDir (uLowAccountID), j); - TER terResult; + if (!lowNode) + return tecDIR_FULL; - std::tie (terResult, std::ignore) = dirAdd (view, - uLowNode, keylet::ownerDir (uLowAccountID), - sleRippleState->key(), - describeOwnerDir (uLowAccountID), j); + auto highNode = dirAdd (view, keylet::ownerDir (uHighAccountID), + sleRippleState->key(), false, describeOwnerDir (uHighAccountID), j); - if (tesSUCCESS == terResult) + if (!highNode) + return tecDIR_FULL; + + const bool bSetDst = saLimit.getIssuer () == uDstAccountID; + const bool bSetHigh = bSrcHigh ^ bSetDst; + + assert (sleAccount->getAccountID (sfAccount) == + (bSetHigh ? uHighAccountID : uLowAccountID)); + auto slePeer = view.peek (keylet::account( + bSetHigh ? uLowAccountID : uHighAccountID)); + assert (slePeer); + + // Remember deletion hints. + sleRippleState->setFieldU64 (sfLowNode, *lowNode); + sleRippleState->setFieldU64 (sfHighNode, *highNode); + + sleRippleState->setFieldAmount ( + bSetHigh ? sfHighLimit : sfLowLimit, saLimit); + sleRippleState->setFieldAmount ( + bSetHigh ? sfLowLimit : sfHighLimit, + STAmount ({saBalance.getCurrency (), + bSetDst ? uSrcAccountID : uDstAccountID})); + + if (uQualityIn) + sleRippleState->setFieldU32 ( + bSetHigh ? sfHighQualityIn : sfLowQualityIn, uQualityIn); + + if (uQualityOut) + sleRippleState->setFieldU32 ( + bSetHigh ? sfHighQualityOut : sfLowQualityOut, uQualityOut); + + std::uint32_t uFlags = bSetHigh ? lsfHighReserve : lsfLowReserve; + + if (bAuth) { - std::tie (terResult, std::ignore) = dirAdd (view, - uHighNode, keylet::ownerDir (uHighAccountID), - sleRippleState->key(), - describeOwnerDir (uHighAccountID), j); + uFlags |= (bSetHigh ? lsfHighAuth : lsfLowAuth); + } + if (bNoRipple) + { + uFlags |= (bSetHigh ? lsfHighNoRipple : lsfLowNoRipple); + } + if (bFreeze) + { + uFlags |= (!bSetHigh ? lsfLowFreeze : lsfHighFreeze); } - if (tesSUCCESS == terResult) + if ((slePeer->getFlags() & lsfDefaultRipple) == 0) { - const bool bSetDst = saLimit.getIssuer () == uDstAccountID; - const bool bSetHigh = bSrcHigh ^ bSetDst; - - assert (sleAccount->getAccountID (sfAccount) == - (bSetHigh ? uHighAccountID : uLowAccountID)); - auto slePeer = view.peek (keylet::account( - bSetHigh ? uLowAccountID : uHighAccountID)); - assert (slePeer); - - // Remember deletion hints. - sleRippleState->setFieldU64 (sfLowNode, uLowNode); - sleRippleState->setFieldU64 (sfHighNode, uHighNode); - - sleRippleState->setFieldAmount ( - bSetHigh ? sfHighLimit : sfLowLimit, saLimit); - sleRippleState->setFieldAmount ( - bSetHigh ? sfLowLimit : sfHighLimit, - STAmount ({saBalance.getCurrency (), - bSetDst ? uSrcAccountID : uDstAccountID})); - - if (uQualityIn) - sleRippleState->setFieldU32 ( - bSetHigh ? sfHighQualityIn : sfLowQualityIn, uQualityIn); - - if (uQualityOut) - sleRippleState->setFieldU32 ( - bSetHigh ? sfHighQualityOut : sfLowQualityOut, uQualityOut); - - std::uint32_t uFlags = bSetHigh ? lsfHighReserve : lsfLowReserve; - - if (bAuth) - { - uFlags |= (bSetHigh ? lsfHighAuth : lsfLowAuth); - } - if (bNoRipple) - { - uFlags |= (bSetHigh ? lsfHighNoRipple : lsfLowNoRipple); - } - if (bFreeze) - { - uFlags |= (!bSetHigh ? lsfLowFreeze : lsfHighFreeze); - } - - if ((slePeer->getFlags() & lsfDefaultRipple) == 0) - { - // The other side's default is no rippling - uFlags |= (bSetHigh ? lsfLowNoRipple : lsfHighNoRipple); - } - - sleRippleState->setFieldU32 (sfFlags, uFlags); - adjustOwnerCount(view, sleAccount, 1, j); - - // ONLY: Create ripple balance. - sleRippleState->setFieldAmount (sfBalance, bSetHigh ? -saBalance : saBalance); - - view.creditHook (uSrcAccountID, - uDstAccountID, saBalance, saBalance.zeroed()); + // The other side's default is no rippling + uFlags |= (bSetHigh ? lsfLowNoRipple : lsfHighNoRipple); } - return terResult; + sleRippleState->setFieldU32 (sfFlags, uFlags); + adjustOwnerCount(view, sleAccount, 1, j); + + // ONLY: Create ripple balance. + sleRippleState->setFieldAmount (sfBalance, bSetHigh ? -saBalance : saBalance); + + view.creditHook (uSrcAccountID, + uDstAccountID, saBalance, saBalance.zeroed()); + + return tesSUCCESS; } TER @@ -1223,7 +1229,7 @@ trustDelete (ApplyView& view, terResult = dirDelete(view, false, uLowNode, - getOwnerDirIndex (uLowAccountID), + keylet::ownerDir (uLowAccountID), sleRippleState->key(), false, !bLowNode, @@ -1236,7 +1242,7 @@ trustDelete (ApplyView& view, terResult = dirDelete (view, false, uHighNode, - getOwnerDirIndex (uHighAccountID), + keylet::ownerDir (uHighAccountID), sleRippleState->key(), false, !bHighNode, @@ -1261,14 +1267,12 @@ offerDelete (ApplyView& view, // Detect legacy directories. bool bOwnerNode = sle->isFieldPresent (sfOwnerNode); - std::uint64_t uOwnerNode = sle->getFieldU64 (sfOwnerNode); uint256 uDirectory = sle->getFieldH256 (sfBookDirectory); - std::uint64_t uBookNode = sle->getFieldU64 (sfBookNode); - TER terResult = dirDelete (view, false, uOwnerNode, - getOwnerDirIndex (owner), offerIndex, false, !bOwnerNode, j); - TER terResult2 = dirDelete (view, false, uBookNode, - uDirectory, offerIndex, true, false, j); + 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 (tesSUCCESS == terResult) adjustOwnerCount(view, view.peek( diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 74206cf31..219ea3d8a 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -66,7 +66,8 @@ class FeatureCollections "Escrow", "CryptoConditionsSuite", "fix1373", - "EnforceInvariants"}; + "EnforceInvariants", + "SortedDirectories"}; std::vector features; boost::container::flat_map featureToIndex; @@ -158,6 +159,7 @@ extern uint256 const featureEscrow; extern uint256 const featureCryptoConditionsSuite; extern uint256 const fix1373; extern uint256 const featureEnforceInvariants; +extern uint256 const featureSortedDirectories; } // ripple diff --git a/src/ripple/protocol/Protocol.h b/src/ripple/protocol/Protocol.h index 463030098..e3d4ce81c 100644 --- a/src/ripple/protocol/Protocol.h +++ b/src/ripple/protocol/Protocol.h @@ -49,6 +49,9 @@ std::size_t constexpr oversizeMetaDataCap = 5200; /** The maximum number of entries per directory page */ std::size_t constexpr dirNodeMaxEntries = 32; +/** The maximum number of pages allowed in a directory */ +std::uint64_t constexpr dirNodeMaxPages = 262144; + /** A ledger index. */ using LedgerIndex = std::uint32_t; @@ -62,4 +65,4 @@ using TxSeq = std::uint32_t; } // ripple -#endif \ No newline at end of file +#endif diff --git a/src/ripple/protocol/STVector256.h b/src/ripple/protocol/STVector256.h index 24d195309..193eebbd6 100644 --- a/src/ripple/protocol/STVector256.h +++ b/src/ripple/protocol/STVector256.h @@ -144,6 +144,18 @@ public: return mValue; } + std::vector::iterator + insert(std::vector::const_iterator pos, uint256 const& value) + { + return mValue.insert(pos, value); + } + + std::vector::iterator + insert(std::vector::const_iterator pos, uint256&& value) + { + return mValue.insert(pos, std::move(value)); + } + void push_back (uint256 const& v) { diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index db21c3253..129e082ea 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -113,5 +113,6 @@ uint256 const featureEscrow = *getRegisteredFeature("Escrow"); uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoConditionsSuite"); uint256 const fix1373 = *getRegisteredFeature("fix1373"); uint256 const featureEnforceInvariants = *getRegisteredFeature("EnforceInvariants"); +uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); } // ripple diff --git a/src/ripple/unity/ledger.cpp b/src/ripple/unity/ledger.cpp index 82ab45c05..dea59e7f9 100644 --- a/src/ripple/unity/ledger.cpp +++ b/src/ripple/unity/ledger.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index b4045c5e1..f94323679 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -15,69 +15,431 @@ */ //============================================================================== -#include -#include +#include +#include #include +#include +#include +#include +#include +#include +#include namespace ripple { namespace test { struct Directory_test : public beast::unit_test::suite { - void testDirectory() + // Map [0-15576] into a a unique 3 letter currency code + std::string + currcode (std::size_t i) + { + // There are only 17576 possible combinations + BEAST_EXPECT (i < 17577); + + std::string code; + + for (int j = 0; j != 3; ++j) + { + code.push_back ('A' + (i % 26)); + i /= 26; + } + + return code; + } + + // Insert n empty pages, numbered [0, ... n - 1], in the + // specified directory: + void + makePages( + Sandbox& sb, + uint256 const& base, + std::uint64_t n) + { + for (std::uint64_t i = 0; i < n; ++i) + { + auto p = std::make_shared(keylet::page(base, i)); + + p->setFieldV256 (sfIndexes, STVector256{}); + + if (i + 1 == n) + p->setFieldU64 (sfIndexNext, 0); + else + p->setFieldU64 (sfIndexNext, i + 1); + + if (i == 0) + p->setFieldU64 (sfIndexPrevious, n - 1); + else + p->setFieldU64 (sfIndexPrevious, i - 1); + + sb.insert (p); + } + } + + void testDirectoryOrdering() { using namespace jtx; - Env env(*this); + auto gw = Account("gw"); auto USD = gw["USD"]; + auto alice = Account("alice"); + auto bob = Account("bob"); { - auto dir = Dir(*env.current(), - keylet::ownerDir(Account("alice"))); - BEAST_EXPECT(std::begin(dir) == std::end(dir)); - BEAST_EXPECT(std::end(dir) == dir.find(uint256(), uint256())); + testcase ("Directory Ordering (without 'SortedDirectories' amendment"); + + Env env(*this, all_features_except(featureSortedDirectories)); + env.fund(XRP(10000000), alice, bob, gw); + + // Insert 400 offers from Alice, then one from Bob: + for (std::size_t i = 1; i <= 400; ++i) + env(offer(alice, USD(10), XRP(10))); + + // Check Alice's directory: it should contain one + // entry for each offer she added. Within each + // page, the entries should be in sorted order. + { + auto dir = Dir(*env.current(), + keylet::ownerDir(alice)); + + std::uint32_t lastSeq = 1; + + // Check that the orders are sequential by checking + // that their sequence numbers are: + for (auto iter = dir.begin(); iter != std::end(dir); ++iter) { + BEAST_EXPECT(++lastSeq == (*iter)->getFieldU32(sfSequence)); + } + BEAST_EXPECT(lastSeq != 1); + } } - env.fund(XRP(10000), "alice", "bob", gw); - - auto i = 10; - for (; i <= 400; i += 10) - env(offer("alice", USD(i), XRP(10))); - env(offer("bob", USD(500), XRP(10))); - { - auto dir = Dir(*env.current(), - keylet::ownerDir(Account("bob"))); - BEAST_EXPECT(std::begin(dir)->get()-> - getFieldAmount(sfTakerPays) == USD(500)); + testcase ("Directory Ordering (with 'SortedDirectories' amendment)"); + + Env env(*this, with_features(featureSortedDirectories)); + env.fund(XRP(10000000), alice, gw); + + for (std::size_t i = 1; i <= 400; ++i) + env(offer(alice, USD(i), XRP(i))); + env.close(); + + // Check Alice's directory: it should contain one + // entry for each offer she added, and, within each + // page the entries should be in sorted order. + { + auto const view = env.closed(); + + std::uint64_t page = 0; + + do + { + auto p = view->read(keylet::page(keylet::ownerDir(alice), page)); + + // Ensure that the entries in the page are sorted + auto const& v = p->getFieldV256(sfIndexes); + BEAST_EXPECT (std::is_sorted(v.begin(), v.end())); + + // Ensure that the page contains the correct orders by + // calculating which sequence numbers belong here. + std::uint32_t minSeq = 2 + (page * dirNodeMaxEntries); + std::uint32_t maxSeq = minSeq + dirNodeMaxEntries; + + for (auto const& e : v) + { + auto c = view->read(keylet::child(e)); + BEAST_EXPECT(c); + BEAST_EXPECT(c->getFieldU32(sfSequence) >= minSeq); + BEAST_EXPECT(c->getFieldU32(sfSequence) < maxSeq); + } + + page = p->getFieldU64(sfIndexNext); + } while (page != 0); + } + + // Now check the orderbook: it should be in the order we placed + // the offers. + auto book = BookDirs(*env.current(), + Book({xrpIssue(), USD.issue()})); + int count = 1; + + for (auto const& offer : book) + { + count++; + BEAST_EXPECT(offer->getFieldAmount(sfTakerPays) == USD(count)); + BEAST_EXPECT(offer->getFieldAmount(sfTakerGets) == XRP(count)); + } + } + } + + void + testDirIsEmpty() + { + testcase ("dirIsEmpty"); + + using namespace jtx; + auto const alice = Account("alice"); + auto const bob = Account("bob"); + auto const charlie = Account ("charlie"); + auto const gw = Account ("gw"); + + beast::xor_shift_engine eng; + + Env env(*this, with_features(featureSortedDirectories, featureMultiSign)); + + env.fund(XRP(1000000), alice, charlie, gw); + env.close(); + + // alice should have an empty directory. + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + // Give alice a signer list, then there will be stuff in the directory. + env(signers(alice, 1, { { bob, 1} })); + env.close(); + BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + env(signers(alice, jtx::none)); + env.close(); + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + std::vector const currencies = [this,&eng,&gw]() + { + std::vector c; + + c.reserve((2 * dirNodeMaxEntries) + 3); + + while (c.size() != c.capacity()) + c.push_back(gw[currcode(c.size())]); + + return c; + }(); + + // First, Alices creates a lot of trustlines, and then + // deletes them in a different order: + { + auto cl = currencies; + + for (auto const& c : cl) + { + env(trust(alice, c(50))); + env.close(); + } + + BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + std::shuffle (cl.begin(), cl.end(), eng); + + for (auto const& c : cl) + { + env(trust(alice, c(0))); + env.close(); + } + + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); } - auto dir = Dir(*env.current(), - keylet::ownerDir(Account("alice"))); - i = 0; - for (auto const& e : dir) - BEAST_EXPECT(e->getFieldAmount(sfTakerPays) == USD(i += 10)); + // Now, Alice creates offers to buy currency, creating + // implicit trust lines. + { + auto cl = currencies; - BEAST_EXPECT(std::begin(dir) != std::end(dir)); - BEAST_EXPECT(std::end(dir) == - dir.find(std::begin(dir).page().key, - uint256())); - BEAST_EXPECT(std::begin(dir) == - dir.find(std::begin(dir).page().key, - std::begin(dir).index())); - auto entry = std::next(std::begin(dir), 32); - auto it = dir.find(entry.page().key, entry.index()); - BEAST_EXPECT(it != std::end(dir)); - BEAST_EXPECT((*it)->getFieldAmount(sfTakerPays) == USD(330)); + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + for (auto c : currencies) + { + env(trust(charlie, c(50))); + env.close(); + env(pay(gw, charlie, c(50))); + env.close(); + env(offer(alice, c(50), XRP(50))); + env.close(); + } + + BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + + // Now fill the offers in a random order. Offer + // entries will drop, and be replaced by trust + // lines that are implicitly created. + std::shuffle (cl.begin(), cl.end(), eng); + + for (auto const& c : cl) + { + env(offer(charlie, XRP(50), c(50))); + env.close(); + } + BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + // Finally, Alice now sends the funds back to + // Charlie. The implicitly created trust lines + // should drop away: + std::shuffle (cl.begin(), cl.end(), eng); + + for (auto const& c : cl) + { + env(pay(alice, charlie, c(50))); + env.close(); + } + + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + } + } + + void + testRipd1353() + { + testcase("RIPD-1353 Empty Offer Directories"); + + using namespace jtx; + Env env(*this, with_features(featureSortedDirectories)); + + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const USD = gw["USD"]; + + env.fund(XRP(10000), alice, gw); + env.trust(USD(1000), alice); + env(pay(gw, alice, USD(1000))); + + auto const firstOfferSeq = env.seq(alice); + + // Fill up three pages of offers + for (int i = 0; i < 3; ++i) + for (int j = 0; j < dirNodeMaxEntries; ++j) + env(offer(alice, XRP(1), USD(1))); + env.close(); + + // remove all the offers. Remove the middle page last + for (auto page : {0, 2, 1}) + { + for (int i = 0; i < dirNodeMaxEntries; ++i) + { + Json::Value cancelOffer; + cancelOffer[jss::Account] = alice.human(); + cancelOffer[jss::OfferSequence] = + Json::UInt(firstOfferSeq + page * dirNodeMaxEntries + i); + cancelOffer[jss::TransactionType] = "OfferCancel"; + env(cancelOffer); + env.close(); + } + } + + // All the offers have been cancelled, so the book + // should have no entries and be empty: + { + Sandbox sb(env.closed().get(), tapNONE); + uint256 const bookBase = getBookBase({xrpIssue(), USD.issue()}); + + BEAST_EXPECT(dirIsEmpty (sb, keylet::page(bookBase))); + BEAST_EXPECT (!sb.succ(bookBase, getQualityNext(bookBase))); + } + + // Alice returns the USD she has to the gateway + // and removes her trust line. Her owner directory + // should now be empty: + { + env.trust(USD(0), alice); + env(pay(alice, gw, alice["USD"](1000))); + env.close(); + BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); + } + } + + void + testEmptyChain() + { + testcase("Empty Chain on Delete"); + + using namespace jtx; + Env env(*this, with_features(featureSortedDirectories)); + + auto const gw = Account{"gateway"}; + auto const alice = Account{"alice"}; + auto const USD = gw["USD"]; + + env.fund(XRP(10000), alice); + env.close(); + + uint256 base; + base.SetHex("fb71c9aa3310141da4b01d6c744a98286af2d72ab5448d5adc0910ca0c910880"); + + uint256 item; + item.SetHex("bad0f021aa3b2f6754a8fe82a5779730aa0bbbab82f17201ef24900efc2c7312"); + + { + // Create a chain of three pages: + Sandbox sb(env.closed().get(), tapNONE); + makePages (sb, base, 3); + + // Insert an item in the middle page: + { + auto p = sb.peek (keylet::page(base, 1)); + BEAST_EXPECT(p); + + STVector256 v; + v.push_back (item); + p->setFieldV256 (sfIndexes, v); + sb.update(p); + } + + // Now, try to delete the item from the middle + // page. This should cause all pages to be deleted: + BEAST_EXPECT (sb.dirRemove (keylet::page(base, 0), 1, keylet::unchecked(item), false)); + BEAST_EXPECT (!sb.peek(keylet::page(base, 2))); + BEAST_EXPECT (!sb.peek(keylet::page(base, 1))); + BEAST_EXPECT (!sb.peek(keylet::page(base, 0))); + } + + { + // Create a chain of four pages: + Sandbox sb(env.closed().get(), tapNONE); + makePages (sb, base, 4); + + // Now add items on pages 1 and 2: + { + auto p1 = sb.peek (keylet::page(base, 1)); + BEAST_EXPECT(p1); + + STVector256 v1; + v1.push_back (~item); + p1->setFieldV256 (sfIndexes, v1); + sb.update(p1); + + auto p2 = sb.peek (keylet::page(base, 2)); + BEAST_EXPECT(p2); + + STVector256 v2; + v2.push_back (item); + p2->setFieldV256 (sfIndexes, v2); + sb.update(p2); + } + + // Now, try to delete the item from page 2. + // This should cause pages 2 and 3 to be + // deleted: + BEAST_EXPECT (sb.dirRemove (keylet::page(base, 0), 2, keylet::unchecked(item), false)); + BEAST_EXPECT (!sb.peek(keylet::page(base, 3))); + BEAST_EXPECT (!sb.peek(keylet::page(base, 2))); + + auto p1 = sb.peek(keylet::page(base, 1)); + BEAST_EXPECT (p1); + BEAST_EXPECT (p1->getFieldU64 (sfIndexNext) == 0); + BEAST_EXPECT (p1->getFieldU64 (sfIndexPrevious) == 0); + + auto p0 = sb.peek(keylet::page(base, 0)); + BEAST_EXPECT (p0); + BEAST_EXPECT (p0->getFieldU64 (sfIndexNext) == 1); + BEAST_EXPECT (p0->getFieldU64 (sfIndexPrevious) == 1); + } } void run() override { - testDirectory(); + testDirectoryOrdering(); + testDirIsEmpty(); + testRipd1353(); + testEmptyChain(); } }; BEAST_DEFINE_TESTSUITE(Directory,ledger,ripple); -} // test -} // ripple +} +} diff --git a/src/test/ledger/View_test.cpp b/src/test/ledger/View_test.cpp index e0836ea47..42bc8db55 100644 --- a/src/test/ledger/View_test.cpp +++ b/src/test/ledger/View_test.cpp @@ -835,107 +835,8 @@ class GetAmendments_test } }; -class DirIsEmpty_test - : public beast::unit_test::suite -{ - void - testDirIsEmpty() - { - using namespace jtx; - auto const alice = Account("alice"); - auto const bogie = Account("bogie"); - - Env env(*this, with_features(featureMultiSign)); - - env.fund(XRP(10000), alice); - env.close(); - - // alice should have an empty directory. - BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - - // Give alice a signer list, then there will be stuff in the directory. - env(signers(alice, 1, { { bogie, 1} })); - env.close(); - BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - - env(signers(alice, jtx::none)); - env.close(); - BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - - // The next test is a bit awkward. It tests the case where alice - // uses 3 directory pages and then deletes all entries from the - // first 2 pages. dirIsEmpty() should still return false in this - // circumstance. - // - // Fill alice's directory with implicit trust lines (produced by - // taking offers) and then remove all but the last one. - auto const becky = Account ("becky"); - auto const gw = Account ("gw"); - env.fund(XRP(10000), becky, gw); - env.close(); - - static_assert (64 >= (2 * dirNodeMaxEntries), ""); - - // Generate 64 currencies named AAA -> AAP and ADA -> ADP. - std::vector currencies; - currencies.reserve(64); - for (char b = 'A'; b <= 'D'; ++b) - { - for (char c = 'A'; c <= 'P'; ++c) - { - currencies.push_back(gw[std::string("A") + b + c]); - IOU const& currency = currencies.back(); - - // Establish trust lines. - env(trust(becky, currency(50))); - env.close(); - env(pay(gw, becky, currency(50))); - env.close(); - env(offer(alice, currency(50), XRP(10))); - env(offer(becky, XRP(10), currency(50))); - env.close(); - } - } - - // Set up one more currency that alice will hold onto. We expect - // this one to go in the third directory page. - IOU const lastCurrency = gw["ZZZ"]; - env(trust(becky, lastCurrency(50))); - env.close(); - env(pay(gw, becky, lastCurrency(50))); - env.close(); - env(offer(alice, lastCurrency(50), XRP(10))); - env(offer(becky, XRP(10), lastCurrency(50))); - env.close(); - - BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - - // Now alice gives all the currencies except the last one back to becky. - for (auto currency : currencies) - { - env(pay(alice, becky, currency(50))); - env.close(); - } - - // This is the crux of the test. - BEAST_EXPECT(! dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - - // Give the last currency to becky. Now alice's directory is empty. - env(pay(alice, becky, lastCurrency(50))); - env.close(); - - BEAST_EXPECT(dirIsEmpty (*env.closed(), keylet::ownerDir(alice))); - } - - void run() override - { - testDirIsEmpty(); - } -}; - BEAST_DEFINE_TESTSUITE(View,ledger,ripple); BEAST_DEFINE_TESTSUITE(GetAmendments,ledger,ripple); -BEAST_DEFINE_TESTSUITE(DirIsEmpty, ledger,ripple); } // test } // ripple diff --git a/src/test/rpc/AccountLinesRPC_test.cpp b/src/test/rpc/AccountLinesRPC_test.cpp index 477eb6b7c..860726b47 100644 --- a/src/test/rpc/AccountLinesRPC_test.cpp +++ b/src/test/rpc/AccountLinesRPC_test.cpp @@ -33,6 +33,8 @@ class AccountLinesRPC_test : public beast::unit_test::suite public: void testAccountLines() { + testcase ("acccount_lines"); + using namespace test::jtx; Env env(*this); { @@ -284,6 +286,7 @@ public: void testAccountLineDelete() { + testcase ("Entry pointed to by marker is removed"); using namespace test::jtx; Env env(*this); @@ -308,35 +311,35 @@ public: auto const USD = gw1["USD"]; auto const EUR = gw2["EUR"]; - env(trust(alice, EUR(200))); - env(trust(becky, USD(200))); - env(trust(cheri, USD(200))); + env(trust(alice, USD(200))); + env(trust(becky, EUR(200))); + env(trust(cheri, EUR(200))); env.close(); // becky gets 100 USD from gw1. - env(pay(gw1, becky, USD(100))); + env(pay(gw2, becky, EUR(100))); env.close(); - // alice offers to buy 100 USD for 100 XRP. - env(offer(alice, USD(100), XRP(100))); + // alice offers to buy 100 EUR for 100 XRP. + env(offer(alice, EUR(100), XRP(100))); env.close(); - // becky offers to buy 100 XRP for 100 USD. - env(offer(becky, XRP(100), USD(100))); + // becky offers to buy 100 XRP for 100 EUR. + env(offer(becky, XRP(100), EUR(100))); env.close(); // Get account_lines for alice. Limit at 1, so we get a marker. auto const linesBeg = env.rpc ("json", "account_lines", R"({"account": ")" + alice.human() + R"(", )" R"("limit": 1})"); - BEAST_EXPECT(linesBeg[jss::result][jss::lines][0u][jss::currency] == "EUR"); + BEAST_EXPECT(linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD"); BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker)); - // alice pays 100 USD to cheri. - env(pay(alice, cheri, USD(100))); + // alice pays 100 EUR to cheri. + env(pay(alice, cheri, EUR(100))); env.close(); - // Since alice paid all her USD to cheri, alice should no longer + // Since alice paid all her EUR to cheri, alice should no longer // have a trust line to gw1. So the old marker should now be invalid. auto const linesEnd = env.rpc ("json", "account_lines", R"({"account": ")" + alice.human() + R"(", )" @@ -349,6 +352,8 @@ public: // test API V2 void testAccountLines2() { + testcase ("V2: acccount_lines"); + using namespace test::jtx; Env env(*this); { @@ -779,6 +784,8 @@ public: // test API V2 void testAccountLineDelete2() { + testcase ("V2: account_lines with removed marker"); + using namespace test::jtx; Env env(*this); @@ -787,12 +794,12 @@ public: // // It isn't easy to explicitly delete a trust line, so we do so in a // round-about fashion. It takes 4 actors: - // o Gateway gw1 issues USD - // o alice offers to buy 100 USD for 100 XRP. - // o becky offers to sell 100 USD for 100 XRP. - // There will now be an inferred trustline between alice and gw1. - // o alice pays her 100 USD to cheri. - // alice should now have no USD and no trustline to gw1. + // o Gateway gw1 issues EUR + // o alice offers to buy 100 EUR for 100 XRP. + // o becky offers to sell 100 EUR for 100 XRP. + // There will now be an inferred trustline between alice and gw2. + // o alice pays her 100 EUR to cheri. + // alice should now have no EUR and no trustline to gw2. Account const alice {"alice"}; Account const becky {"becky"}; Account const cheri {"cheri"}; @@ -803,21 +810,21 @@ public: auto const USD = gw1["USD"]; auto const EUR = gw2["EUR"]; - env(trust(alice, EUR(200))); - env(trust(becky, USD(200))); - env(trust(cheri, USD(200))); + env(trust(alice, USD(200))); + env(trust(becky, EUR(200))); + env(trust(cheri, EUR(200))); env.close(); - // becky gets 100 USD from gw1. - env(pay(gw1, becky, USD(100))); + // becky gets 100 EUR from gw1. + env(pay(gw2, becky, EUR(100))); env.close(); - // alice offers to buy 100 USD for 100 XRP. - env(offer(alice, USD(100), XRP(100))); + // alice offers to buy 100 EUR for 100 XRP. + env(offer(alice, EUR(100), XRP(100))); env.close(); - // becky offers to buy 100 XRP for 100 USD. - env(offer(becky, XRP(100), USD(100))); + // becky offers to buy 100 XRP for 100 EUR. + env(offer(becky, XRP(100), EUR(100))); env.close(); // Get account_lines for alice. Limit at 1, so we get a marker. @@ -829,17 +836,17 @@ public: R"("params": [ )" R"({"account": ")" + alice.human() + R"(", )" R"("limit": 1}]})"); - BEAST_EXPECT(linesBeg[jss::result][jss::lines][0u][jss::currency] == "EUR"); + BEAST_EXPECT(linesBeg[jss::result][jss::lines][0u][jss::currency] == "USD"); BEAST_EXPECT(linesBeg[jss::result].isMember(jss::marker)); BEAST_EXPECT(linesBeg.isMember(jss::jsonrpc) && linesBeg[jss::jsonrpc] == "2.0"); BEAST_EXPECT(linesBeg.isMember(jss::ripplerpc) && linesBeg[jss::ripplerpc] == "2.0"); BEAST_EXPECT(linesBeg.isMember(jss::id) && linesBeg[jss::id] == 5); // alice pays 100 USD to cheri. - env(pay(alice, cheri, USD(100))); + env(pay(alice, cheri, EUR(100))); env.close(); - // Since alice paid all her USD to cheri, alice should no longer + // Since alice paid all her EUR to cheri, alice should no longer // have a trust line to gw1. So the old marker should now be invalid. auto const linesEnd = env.rpc ("json2", "{ " R"("method" : "account_lines",)" diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 909161bc1..302287e26 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -29,96 +29,85 @@ namespace ripple { namespace test { static char const* bobs_account_objects[] = { -R"json( -{ - "Balance": { - "currency": "USD", - "issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji", - "value": "-1000" - }, - "Flags": 131072, - "HighLimit": { - "currency": "USD", - "issuer": "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "value": "1000" - }, - "HighNode": "0000000000000000", - "LedgerEntryType": "RippleState", - "LowLimit": { - "currency": "USD", - "issuer": "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", - "value": "0" - }, - "LowNode": "0000000000000000", - "index": - "D89BC239086183EB9458C396E643795C1134963E6550E682A190A5F021766D43" -})json" -, -R"json( -{ - "Balance": { - "currency": "USD", - "issuer": "rrrrrrrrrrrrrrrrrrrrBZbvji", - "value": "-1000" - }, - "Flags": 131072, - "HighLimit": { - "currency": "USD", - "issuer": "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "value": "1000" - }, - "HighNode": "0000000000000000", - "LedgerEntryType": "RippleState", - "LowLimit": { - "currency": "USD", - "issuer": "r9cZvwKU3zzuZK9JFovGg1JC5n7QiqNL8L", - "value": "0" - }, - "LowNode": "0000000000000000", - "index": - "D13183BCFFC9AAC9F96AEBB5F66E4A652AD1F5D10273AEB615478302BEBFD4A4" -})json" -, -R"json( -{ - "Account": "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "BookDirectory": - "50AD0A9E54D2B381288D535EB724E4275FFBF41580D28A925D038D7EA4C68000", - "BookNode": "0000000000000000", - "Flags": 65536, - "LedgerEntryType": "Offer", - "OwnerNode": "0000000000000000", - "Sequence": 4, - "TakerGets": { - "currency": "USD", - "issuer": "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "value": "1" - }, - "TakerPays": "100000000", - "index": - "A984D036A0E562433A8377CA57D1A1E056E58C0D04818F8DFD3A1AA3F217DD82" -})json" -, -R"json( -{ - "Account": "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", - "BookDirectory": - "B025997A323F5C3E03DDF1334471F5984ABDE31C59D463525D038D7EA4C68000", - "BookNode": "0000000000000000", - "Flags": 65536, - "LedgerEntryType": "Offer", - "OwnerNode": "0000000000000000", - "Sequence": 5, - "TakerGets": { - "currency": "USD", - "issuer" : "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", +R"json({ + "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "BookDirectory" : "50AD0A9E54D2B381288D535EB724E4275FFBF41580D28A925D038D7EA4C68000", + "BookNode" : "0000000000000000", + "Flags" : 65536, + "LedgerEntryType" : "Offer", + "OwnerNode" : "0000000000000000", + "Sequence" : 4, + "TakerGets" : { + "currency" : "USD", + "issuer" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", "value" : "1" }, "TakerPays" : "100000000", - "index" : - "CAFE32332D752387B01083B60CC63069BA4A969C9730836929F841450F6A718E" -} -)json" + "index" : "A984D036A0E562433A8377CA57D1A1E056E58C0D04818F8DFD3A1AA3F217DD82" +})json" +, +R"json({ + "Account" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "BookDirectory" : "B025997A323F5C3E03DDF1334471F5984ABDE31C59D463525D038D7EA4C68000", + "BookNode" : "0000000000000000", + "Flags" : 65536, + "LedgerEntryType" : "Offer", + "OwnerNode" : "0000000000000000", + "Sequence" : 5, + "TakerGets" : { + "currency" : "USD", + "issuer" : "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", + "value" : "1" + }, + "TakerPays" : "100000000", + "index" : "CAFE32332D752387B01083B60CC63069BA4A969C9730836929F841450F6A718E" +})json" +, +R"json({ + "Balance" : { + "currency" : "USD", + "issuer" : "rrrrrrrrrrrrrrrrrrrrBZbvji", + "value" : "-1000" + }, + "Flags" : 131072, + "HighLimit" : { + "currency" : "USD", + "issuer" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "value" : "1000" + }, + "HighNode" : "0000000000000000", + "LedgerEntryType" : "RippleState", + "LowLimit" : { + "currency" : "USD", + "issuer" : "r9cZvwKU3zzuZK9JFovGg1JC5n7QiqNL8L", + "value" : "0" + }, + "LowNode" : "0000000000000000", + "index" : "D13183BCFFC9AAC9F96AEBB5F66E4A652AD1F5D10273AEB615478302BEBFD4A4" +})json" +, +R"json({ + "Balance" : { + "currency" : "USD", + "issuer" : "rrrrrrrrrrrrrrrrrrrrBZbvji", + "value" : "-1000" + }, + "Flags" : 131072, + "HighLimit" : { + "currency" : "USD", + "issuer" : "rPMh7Pi9ct699iZUTWaytJUoHcJ7cgyziK", + "value" : "1000" + }, + "HighNode" : "0000000000000000", + "LedgerEntryType" : "RippleState", + "LowLimit" : { + "currency" : "USD", + "issuer" : "r32rQHyesiTtdWFU7UJVtff4nCR5SHCbJW", + "value" : "0" + }, + "LowNode" : "0000000000000000", + "index" : "D89BC239086183EB9458C396E643795C1134963E6550E682A190A5F021766D43" +})json" }; class AccountObjects_test : public beast::unit_test::suite @@ -196,7 +185,7 @@ public: env.trust(USD(1000), bob); env(pay(gw, bob, XRP(1))); env(offer(bob, XRP(100), bob["USD"](1)), txflags(tfPassive)); - + Json::Value params; params[jss::account] = bob.human(); params[jss::limit] = 1; @@ -254,7 +243,7 @@ public: env.fund(XRP(1000), gw1, gw2, bob); env.trust(USD1(1000), bob); env.trust(USD2(1000), bob); - + env(pay(gw1, bob, USD1(1000))); env(pay(gw2, bob, USD2(1000))); @@ -280,7 +269,9 @@ public: aobj.removeMember("PreviousTxnID"); aobj.removeMember("PreviousTxnLgrSeq"); - BEAST_EXPECT( aobj == bobj[i]); + if (aobj != bobj[i]) + std::cout << "Fail at " << i << ": " << aobj << std::endl; + BEAST_EXPECT(aobj == bobj[i]); } } // test request with type parameter as filter, unstepped @@ -298,7 +289,7 @@ public: aobj.removeMember("PreviousTxnID"); aobj.removeMember("PreviousTxnLgrSeq"); - BEAST_EXPECT( aobj == bobj[i]); + BEAST_EXPECT( aobj == bobj[i+2]); } } // test stepped one-at-a-time with limit=1, resume from prev marker @@ -316,7 +307,8 @@ public: aobj.removeMember("PreviousTxnID"); aobj.removeMember("PreviousTxnLgrSeq"); - BEAST_EXPECT( aobj == bobj[i]); + + BEAST_EXPECT(aobj == bobj[i]); auto resume_marker = resp[jss::result][jss::marker]; params[jss::marker] = resume_marker; diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 7d76dc64c..de574cbc2 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -108,21 +108,21 @@ public: { BEAST_EXPECT(jro[0u][jss::quality] == "100000000"); BEAST_EXPECT(jro[0u][jss::taker_gets][jss::currency] == "USD"); - BEAST_EXPECT(jro[0u][jss::taker_gets][jss::issuer] == bob.human()); + BEAST_EXPECT(jro[0u][jss::taker_gets][jss::issuer] == gw.human()); BEAST_EXPECT(jro[0u][jss::taker_gets][jss::value] == "1"); BEAST_EXPECT(jro[0u][jss::taker_pays] == "100000000"); - BEAST_EXPECT(jro[1u][jss::quality] == "100000000"); + BEAST_EXPECT(jro[1u][jss::quality] == "5000000"); BEAST_EXPECT(jro[1u][jss::taker_gets][jss::currency] == "USD"); BEAST_EXPECT(jro[1u][jss::taker_gets][jss::issuer] == gw.human()); - BEAST_EXPECT(jro[1u][jss::taker_gets][jss::value] == "1"); - BEAST_EXPECT(jro[1u][jss::taker_pays] == "100000000"); + BEAST_EXPECT(jro[1u][jss::taker_gets][jss::value] == "2"); + BEAST_EXPECT(jro[1u][jss::taker_pays] == "10000000"); - BEAST_EXPECT(jro[2u][jss::quality] == "5000000"); + BEAST_EXPECT(jro[2u][jss::quality] == "100000000"); BEAST_EXPECT(jro[2u][jss::taker_gets][jss::currency] == "USD"); - BEAST_EXPECT(jro[2u][jss::taker_gets][jss::issuer] == gw.human()); - BEAST_EXPECT(jro[2u][jss::taker_gets][jss::value] == "2"); - BEAST_EXPECT(jro[2u][jss::taker_pays] == "10000000"); + BEAST_EXPECT(jro[2u][jss::taker_gets][jss::issuer] == bob.human()); + BEAST_EXPECT(jro[2u][jss::taker_gets][jss::value] == "1"); + BEAST_EXPECT(jro[2u][jss::taker_pays] == "100000000"); } { diff --git a/src/test/rpc/OwnerInfo_test.cpp b/src/test/rpc/OwnerInfo_test.cpp index fd99ceb97..49b44178b 100644 --- a/src/test/rpc/OwnerInfo_test.cpp +++ b/src/test/rpc/OwnerInfo_test.cpp @@ -120,26 +120,26 @@ class OwnerInfo_test : public beast::unit_test::suite BEAST_EXPECT ( lines[0u][sfBalance.fieldName] == - (STAmount{Issue{to_currency("USD"), noAccount()}, 0} - .value().getJson(0))); - BEAST_EXPECT ( - lines[0u][sfHighLimit.fieldName] == - alice["USD"](1000).value().getJson(0)); - BEAST_EXPECT ( - lines[0u][sfLowLimit.fieldName] == - USD(0).value().getJson(0)); - - BEAST_EXPECT ( - lines[1u][sfBalance.fieldName] == (STAmount{Issue{to_currency("CNY"), noAccount()}, 0} .value().getJson(0))); BEAST_EXPECT ( - lines[1u][sfHighLimit.fieldName] == + lines[0u][sfHighLimit.fieldName] == alice["CNY"](1000).value().getJson(0)); BEAST_EXPECT ( - lines[1u][sfLowLimit.fieldName] == + lines[0u][sfLowLimit.fieldName] == gw["CNY"](0).value().getJson(0)); + BEAST_EXPECT ( + lines[1u][sfBalance.fieldName] == + (STAmount{Issue{to_currency("USD"), noAccount()}, 0} + .value().getJson(0))); + BEAST_EXPECT ( + lines[1u][sfHighLimit.fieldName] == + alice["USD"](1000).value().getJson(0)); + BEAST_EXPECT ( + lines[1u][sfLowLimit.fieldName] == + USD(0).value().getJson(0)); + if (! BEAST_EXPECT (result[jss::accepted].isMember(jss::offers))) return; auto offers = result[jss::accepted][jss::offers]; @@ -163,26 +163,26 @@ class OwnerInfo_test : public beast::unit_test::suite BEAST_EXPECT ( lines[0u][sfBalance.fieldName] == - (STAmount{Issue{to_currency("USD"), noAccount()}, -50} - .value().getJson(0))); - BEAST_EXPECT ( - lines[0u][sfHighLimit.fieldName] == - alice["USD"](1000).value().getJson(0)); - BEAST_EXPECT ( - lines[0u][sfLowLimit.fieldName] == - gw["USD"](0).value().getJson(0)); - - BEAST_EXPECT ( - lines[1u][sfBalance.fieldName] == (STAmount{Issue{to_currency("CNY"), noAccount()}, -50} .value().getJson(0))); BEAST_EXPECT ( - lines[1u][sfHighLimit.fieldName] == + lines[0u][sfHighLimit.fieldName] == alice["CNY"](1000).value().getJson(0)); BEAST_EXPECT ( - lines[1u][sfLowLimit.fieldName] == + lines[0u][sfLowLimit.fieldName] == gw["CNY"](0).value().getJson(0)); + BEAST_EXPECT ( + lines[1u][sfBalance.fieldName] == + (STAmount{Issue{to_currency("USD"), noAccount()}, -50} + .value().getJson(0))); + BEAST_EXPECT ( + lines[1u][sfHighLimit.fieldName] == + alice["USD"](1000).value().getJson(0)); + BEAST_EXPECT ( + lines[1u][sfLowLimit.fieldName] == + gw["USD"](0).value().getJson(0)); + if (! BEAST_EXPECT (result[jss::current].isMember(jss::offers))) return; offers = result[jss::current][jss::offers]; @@ -190,16 +190,14 @@ class OwnerInfo_test : public beast::unit_test::suite if (! BEAST_EXPECT (offers.isArray() && offers.size() == 2)) return; - // first offer is same as in accepted. BEAST_EXPECT ( - offers[0u] == result[jss::accepted][jss::offers][0u]); - // second offer, for CNY + offers[1u] == result[jss::accepted][jss::offers][0u]); BEAST_EXPECT ( - offers[1u][jss::Account] == alice.human()); + offers[0u][jss::Account] == alice.human()); BEAST_EXPECT ( - offers[1u][sfTakerGets.fieldName] == XRP(1000).value().getJson(0)); + offers[0u][sfTakerGets.fieldName] == XRP(1000).value().getJson(0)); BEAST_EXPECT ( - offers[1u][sfTakerPays.fieldName] == CNY(2).value().getJson(0)); + offers[0u][sfTakerPays.fieldName] == CNY(2).value().getJson(0)); } public: