diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index cb964037e..6ddb55787 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -713,104 +713,10 @@ dirAdd (ApplyView& view, 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); - } + if (strictOrder) + return view.dirAppend(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) - { - // No root, make it. - sleRoot = std::make_shared(dir); - sleRoot->setFieldH256 (sfRootIndex, dir.key); - view.insert (sleRoot); - fDescriber (sleRoot); - - STVector256 v; - v.push_back (uLedgerIndex); - sleRoot->setFieldV256 (sfIndexes, v); - - JLOG (j.trace()) << - "dirAdd: created root " << to_string (dir.key) << - " for entry " << to_string (uLedgerIndex); - - return uNodeDir; - } - - SLE::pointer sleNode; - STVector256 svIndexes; - - uNodeDir = sleRoot->getFieldU64 (sfIndexPrevious); // Get index to last directory node. - - if (uNodeDir) - { - // Try adding to last node. - sleNode = view.peek (keylet::page(dir, uNodeDir)); - assert (sleNode); - } - else - { - // Try adding to root. Didn't have a previous set to the last node. - sleNode = sleRoot; - } - - svIndexes = sleNode->getFieldV256 (sfIndexes); - - if (dirNodeMaxEntries != svIndexes.size ()) - { - // Add to current node. - view.update(sleNode); - } - // Add to new node. - else if (!++uNodeDir) - { - return boost::none; - } - else - { - // Have old last point to new node - sleNode->setFieldU64 (sfIndexNext, uNodeDir); - view.update(sleNode); - - // Have root point to new node. - sleRoot->setFieldU64 (sfIndexPrevious, uNodeDir); - view.update (sleRoot); - - // Create the new node. - sleNode = std::make_shared( - keylet::page(dir, uNodeDir)); - sleNode->setFieldH256 (sfRootIndex, dir.key); - view.insert (sleNode); - - if (uNodeDir != 1) - sleNode->setFieldU64 (sfIndexPrevious, uNodeDir - 1); - - fDescriber (sleNode); - - svIndexes = STVector256 (); - } - - svIndexes.push_back (uLedgerIndex); // Append entry. - sleNode->setFieldV256 (sfIndexes, svIndexes); // Save entry. - - JLOG (j.trace()) << - "dirAdd: creating: root: " << to_string (dir.key); - JLOG (j.trace()) << - "dirAdd: appending: Entry: " << to_string (uLedgerIndex); - JLOG (j.trace()) << - "dirAdd: appending: Node: " << strHex (uNodeDir); - - return uNodeDir; + return view.dirInsert(dir, uLedgerIndex, fDescriber); } TER diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 30c18dd47..aa67e9613 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -376,7 +376,7 @@ extern uint256 const retiredEscrow; extern uint256 const featureCryptoConditionsSuite; extern uint256 const retiredFix1373; extern uint256 const retiredEnforceInvariants; -extern uint256 const featureSortedDirectories; +extern uint256 const retiredSortedDirectories; extern uint256 const fix1201; extern uint256 const fix1512; extern uint256 const fix1513; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index fa57a6a5c..3136144b1 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -167,7 +167,7 @@ uint256 const retiredEscrow = *getRegisteredFeature("Escrow"); uint256 const featureCryptoConditionsSuite = *getRegisteredFeature("CryptoConditionsSuite"); uint256 const retiredFix1373 = *getRegisteredFeature("fix1373"); uint256 const retiredEnforceInvariants = *getRegisteredFeature("EnforceInvariants"); -uint256 const featureSortedDirectories = *getRegisteredFeature("SortedDirectories"); +uint256 const retiredSortedDirectories = *getRegisteredFeature("SortedDirectories"); uint256 const fix1201 = *getRegisteredFeature("fix1201"); uint256 const fix1512 = *getRegisteredFeature("fix1512"); uint256 const fix1513 = *getRegisteredFeature("fix1513"); diff --git a/src/test/ledger/Directory_test.cpp b/src/test/ledger/Directory_test.cpp index c3e2ab72d..37a7f4257 100644 --- a/src/test/ledger/Directory_test.cpp +++ b/src/test/ledger/Directory_test.cpp @@ -85,94 +85,61 @@ struct Directory_test : public beast::unit_test::suite auto alice = Account("alice"); auto bob = Account("bob"); + testcase ("Directory Ordering"); + + Env env(*this); + env.fund(XRP(10000000), alice, gw); + + std::uint32_t const firstOfferSeq {env.seq (alice)}; + 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. { - testcase ("Directory Ordering (without 'SortedDirectories' amendment"); + auto const view = env.closed(); - Env env( - *this, - supported_amendments().reset(featureSortedDirectories)); - env.fund(XRP(10000000), alice, bob, gw); + std::uint64_t page = 0; - // Insert 400 offers from Alice, then one from Bob: - std::uint32_t const firstOfferSeq {env.seq (alice)}; - 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. + do { - auto dir = Dir(*env.current(), - keylet::ownerDir(alice)); + auto p = view->read(keylet::page(keylet::ownerDir(alice), page)); - std::uint32_t lastSeq = firstOfferSeq - 1; + // Ensure that the entries in the page are sorted + auto const& v = p->getFieldV256(sfIndexes); + BEAST_EXPECT (std::is_sorted(v.begin(), v.end())); - // 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)); + // Ensure that the page contains the correct orders by + // calculating which sequence numbers belong here. + std::uint32_t const minSeq = + firstOfferSeq + (page * dirNodeMaxEntries); + std::uint32_t const 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); } - BEAST_EXPECT(lastSeq != 1); - } + + 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) { - testcase ("Directory Ordering (with 'SortedDirectories' amendment)"); - - Env env(*this); - env.fund(XRP(10000000), alice, gw); - - std::uint32_t const firstOfferSeq {env.seq (alice)}; - 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 const minSeq = - firstOfferSeq + (page * dirNodeMaxEntries); - std::uint32_t const 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)); - } + count++; + BEAST_EXPECT(offer->getFieldAmount(sfTakerPays) == USD(count)); + BEAST_EXPECT(offer->getFieldAmount(sfTakerGets) == XRP(count)); } }