mirror of
https://github.com/XRPLF/rippled.git
synced 2025-12-06 17:27:55 +00:00
Remove directory size limit (#5935)
This change introduces the `fixDirectoryLimit` amendment to remove the directory pages limit. We found that the directory size limit is easier to hit than originally assumed, and there is no good reason to keep this limit, since the object reserve provides the necessary incentive to avoid creating unnecessary objects on the ledger.
This commit is contained in:
@@ -55,7 +55,10 @@ 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 */
|
||||
/** The maximum number of pages allowed in a directory
|
||||
|
||||
Made obsolete by fixDirectoryLimit amendment.
|
||||
*/
|
||||
std::uint64_t constexpr dirNodeMaxPages = 262144;
|
||||
|
||||
/** The maximum number of items in an NFT page */
|
||||
|
||||
@@ -29,9 +29,8 @@
|
||||
|
||||
// Add new amendments to the top of this list.
|
||||
// Keep it sorted in reverse chronological order.
|
||||
// If you add an amendment here, then do not forget to increment `numFeatures`
|
||||
// in include/xrpl/protocol/Feature.h.
|
||||
|
||||
XRPL_FIX (DirectoryLimit, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (IncludeKeyletFields, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(DynamicMPT, Supported::no, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (TokenEscrowV1, Supported::yes, VoteBehavior::DefaultNo)
|
||||
|
||||
@@ -22,6 +22,9 @@
|
||||
#include <xrpl/ledger/ApplyView.h>
|
||||
#include <xrpl/protocol/Protocol.h>
|
||||
|
||||
#include <limits>
|
||||
#include <type_traits>
|
||||
|
||||
namespace ripple {
|
||||
|
||||
std::optional<std::uint64_t>
|
||||
@@ -91,8 +94,21 @@ ApplyView::dirAdd(
|
||||
return page;
|
||||
}
|
||||
|
||||
// We rely on modulo arithmetic of unsigned integers (guaranteed in
|
||||
// [basic.fundamental] paragraph 2) to detect page representation overflow.
|
||||
// For signed integers this would be UB, hence static_assert here.
|
||||
static_assert(std::is_unsigned_v<decltype(page)>);
|
||||
// Defensive check against breaking changes in compiler.
|
||||
static_assert([]<typename T>(std::type_identity<T>) constexpr -> T {
|
||||
T tmp = std::numeric_limits<T>::max();
|
||||
return ++tmp;
|
||||
}(std::type_identity<decltype(page)>{}) == 0);
|
||||
++page;
|
||||
// Check whether we're out of pages.
|
||||
if (++page >= dirNodeMaxPages)
|
||||
if (page == 0)
|
||||
return std::nullopt;
|
||||
if (!rules().enabled(fixDirectoryLimit) &&
|
||||
page >= dirNodeMaxPages) // Old pages limit
|
||||
return std::nullopt;
|
||||
|
||||
// We are about to create a new node; we'll link it to
|
||||
|
||||
@@ -558,6 +558,39 @@ struct Credentials_test : public beast::unit_test::suite
|
||||
jle[jss::result][jss::node]["CredentialType"] ==
|
||||
strHex(std::string_view(credType)));
|
||||
}
|
||||
|
||||
{
|
||||
testcase("Credentials fail, directory full");
|
||||
std::uint32_t const issuerSeq{env.seq(issuer) + 1};
|
||||
env(ticket::create(issuer, 63));
|
||||
env.close();
|
||||
|
||||
// Everything below can only be tested on open ledger.
|
||||
auto const res1 = directory::bumpLastPage(
|
||||
env,
|
||||
directory::maximumPageIndex(env),
|
||||
keylet::ownerDir(issuer.id()),
|
||||
directory::adjustOwnerNode);
|
||||
BEAST_EXPECT(res1);
|
||||
|
||||
auto const jv = credentials::create(issuer, subject, credType);
|
||||
env(jv, ter(tecDIR_FULL));
|
||||
// Free one directory entry by using a ticket
|
||||
env(noop(issuer), ticket::use(issuerSeq + 40));
|
||||
|
||||
// Fill subject directory
|
||||
env(ticket::create(subject, 63));
|
||||
auto const res2 = directory::bumpLastPage(
|
||||
env,
|
||||
directory::maximumPageIndex(env),
|
||||
keylet::ownerDir(subject.id()),
|
||||
directory::adjustOwnerNode);
|
||||
BEAST_EXPECT(res2);
|
||||
env(jv, ter(tecDIR_FULL));
|
||||
|
||||
// End test
|
||||
env.close();
|
||||
}
|
||||
}
|
||||
|
||||
{
|
||||
@@ -1084,6 +1117,7 @@ struct Credentials_test : public beast::unit_test::suite
|
||||
testSuccessful(all);
|
||||
testCredentialsDelete(all);
|
||||
testCreateFailed(all);
|
||||
testCreateFailed(all - fixDirectoryLimit);
|
||||
testAcceptFailed(all);
|
||||
testDeleteFailed(all);
|
||||
testFeatureFailed(all - featureCredentials);
|
||||
|
||||
@@ -39,6 +39,7 @@
|
||||
#include <test/jtx/delivermin.h>
|
||||
#include <test/jtx/deposit.h>
|
||||
#include <test/jtx/did.h>
|
||||
#include <test/jtx/directory.h>
|
||||
#include <test/jtx/domain.h>
|
||||
#include <test/jtx/escrow.h>
|
||||
#include <test/jtx/fee.h>
|
||||
|
||||
81
src/test/jtx/directory.h
Normal file
81
src/test/jtx/directory.h
Normal file
@@ -0,0 +1,81 @@
|
||||
//------------------------------------------------------------------------------
|
||||
/*
|
||||
This file is part of rippled: https://github.com/ripple/rippled
|
||||
Copyright (c) 2025 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.
|
||||
*/
|
||||
//==============================================================================
|
||||
|
||||
#ifndef RIPPLE_TEST_JTX_DIRECTORY_H_INCLUDED
|
||||
#define RIPPLE_TEST_JTX_DIRECTORY_H_INCLUDED
|
||||
|
||||
#include <test/jtx/Env.h>
|
||||
|
||||
#include <xrpl/basics/Expected.h>
|
||||
#include <xrpl/protocol/Feature.h>
|
||||
#include <xrpl/protocol/Indexes.h>
|
||||
|
||||
#include <cstdint>
|
||||
#include <limits>
|
||||
|
||||
namespace ripple::test::jtx {
|
||||
|
||||
/** Directory operations. */
|
||||
namespace directory {
|
||||
|
||||
enum Error {
|
||||
DirectoryRootNotFound,
|
||||
DirectoryTooSmall,
|
||||
DirectoryPageDuplicate,
|
||||
DirectoryPageNotFound,
|
||||
InvalidLastPage,
|
||||
AdjustmentError
|
||||
};
|
||||
|
||||
/// Move the position of the last page in the user's directory on open ledger to
|
||||
/// newLastPage. Requirements:
|
||||
/// - directory must have at least two pages (root and one more)
|
||||
/// - adjust should be used to update owner nodes of the objects affected
|
||||
/// - newLastPage must be greater than index of the last page in the directory
|
||||
///
|
||||
/// Use this to test tecDIR_FULL errors in open ledger.
|
||||
/// NOTE: effects will be DISCARDED on env.close()
|
||||
auto
|
||||
bumpLastPage(
|
||||
Env& env,
|
||||
std::uint64_t newLastPage,
|
||||
Keylet directory,
|
||||
std::function<bool(ApplyView&, uint256, std::uint64_t)> adjust)
|
||||
-> Expected<void, Error>;
|
||||
|
||||
/// Implementation of adjust for the most common ledger entry, i.e. one where
|
||||
/// page index is stored in sfOwnerNode (and only there). Pass this function
|
||||
/// to bumpLastPage if the last page of directory has only objects
|
||||
/// of this kind (e.g. ticket, DID, offer, deposit preauth, MPToken etc.)
|
||||
bool
|
||||
adjustOwnerNode(ApplyView& view, uint256 key, std::uint64_t page);
|
||||
|
||||
inline auto
|
||||
maximumPageIndex(Env const& env) -> std::uint64_t
|
||||
{
|
||||
if (env.enabled(fixDirectoryLimit))
|
||||
return std::numeric_limits<std::uint64_t>::max();
|
||||
return dirNodeMaxPages - 1;
|
||||
}
|
||||
|
||||
} // namespace directory
|
||||
|
||||
} // namespace ripple::test::jtx
|
||||
|
||||
#endif
|
||||
145
src/test/jtx/impl/directory.cpp
Normal file
145
src/test/jtx/impl/directory.cpp
Normal file
@@ -0,0 +1,145 @@
|
||||
//------------------------------------------------------------------------------
|
||||
/*
|
||||
This file is part of rippled: https://github.com/ripple/rippled
|
||||
Copyright (c) 2025 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 <test/jtx/directory.h>
|
||||
|
||||
#include <xrpl/ledger/Sandbox.h>
|
||||
|
||||
namespace ripple::test::jtx {
|
||||
|
||||
/** Directory operations. */
|
||||
namespace directory {
|
||||
|
||||
auto
|
||||
bumpLastPage(
|
||||
Env& env,
|
||||
std::uint64_t newLastPage,
|
||||
Keylet directory,
|
||||
std::function<bool(ApplyView&, uint256, std::uint64_t)> adjust)
|
||||
-> Expected<void, Error>
|
||||
{
|
||||
Expected<void, Error> res{};
|
||||
env.app().openLedger().modify(
|
||||
[&](OpenView& view, beast::Journal j) -> bool {
|
||||
Sandbox sb(&view, tapNONE);
|
||||
|
||||
// Find the root page
|
||||
auto sleRoot = sb.peek(directory);
|
||||
if (!sleRoot)
|
||||
{
|
||||
res = Unexpected<Error>(DirectoryRootNotFound);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Find last page
|
||||
auto const lastIndex = sleRoot->getFieldU64(sfIndexPrevious);
|
||||
if (lastIndex == 0)
|
||||
{
|
||||
res = Unexpected<Error>(DirectoryTooSmall);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (sb.exists(keylet::page(directory, newLastPage)))
|
||||
{
|
||||
res = Unexpected<Error>(DirectoryPageDuplicate);
|
||||
return false;
|
||||
}
|
||||
|
||||
if (lastIndex >= newLastPage)
|
||||
{
|
||||
res = Unexpected<Error>(InvalidLastPage);
|
||||
return false;
|
||||
}
|
||||
|
||||
auto slePage = sb.peek(keylet::page(directory, lastIndex));
|
||||
if (!slePage)
|
||||
{
|
||||
res = Unexpected<Error>(DirectoryPageNotFound);
|
||||
return false;
|
||||
}
|
||||
|
||||
// Copy its data and delete the page
|
||||
auto indexes = slePage->getFieldV256(sfIndexes);
|
||||
auto prevIndex = slePage->at(~sfIndexPrevious);
|
||||
auto owner = slePage->at(~sfOwner);
|
||||
sb.erase(slePage);
|
||||
|
||||
// Create new page to replace slePage
|
||||
auto sleNew =
|
||||
std::make_shared<SLE>(keylet::page(directory, newLastPage));
|
||||
sleNew->setFieldH256(sfRootIndex, directory.key);
|
||||
sleNew->setFieldV256(sfIndexes, indexes);
|
||||
if (owner)
|
||||
sleNew->setAccountID(sfOwner, *owner);
|
||||
if (prevIndex)
|
||||
sleNew->setFieldU64(sfIndexPrevious, *prevIndex);
|
||||
sb.insert(sleNew);
|
||||
|
||||
// Adjust root previous and previous node's next
|
||||
sleRoot->setFieldU64(sfIndexPrevious, newLastPage);
|
||||
if (prevIndex.value_or(0) == 0)
|
||||
sleRoot->setFieldU64(sfIndexNext, newLastPage);
|
||||
else
|
||||
{
|
||||
auto slePrev = sb.peek(keylet::page(directory, *prevIndex));
|
||||
if (!slePrev)
|
||||
{
|
||||
res = Unexpected<Error>(DirectoryPageNotFound);
|
||||
return false;
|
||||
}
|
||||
slePrev->setFieldU64(sfIndexNext, newLastPage);
|
||||
sb.update(slePrev);
|
||||
}
|
||||
sb.update(sleRoot);
|
||||
|
||||
// Fixup page numbers in the objects referred by indexes
|
||||
if (adjust)
|
||||
for (auto const key : indexes)
|
||||
{
|
||||
if (!adjust(sb, key, newLastPage))
|
||||
{
|
||||
res = Unexpected<Error>(AdjustmentError);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
sb.apply(view);
|
||||
return true;
|
||||
});
|
||||
|
||||
return res;
|
||||
}
|
||||
|
||||
bool
|
||||
adjustOwnerNode(ApplyView& view, uint256 key, std::uint64_t page)
|
||||
{
|
||||
auto sle = view.peek({ltANY, key});
|
||||
if (sle && sle->isFieldPresent(sfOwnerNode))
|
||||
{
|
||||
sle->setFieldU64(sfOwnerNode, page);
|
||||
view.update(sle);
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
} // namespace directory
|
||||
|
||||
} // namespace ripple::test::jtx
|
||||
@@ -22,9 +22,11 @@
|
||||
#include <xrpl/ledger/Sandbox.h>
|
||||
#include <xrpl/protocol/Feature.h>
|
||||
#include <xrpl/protocol/Protocol.h>
|
||||
#include <xrpl/protocol/TER.h>
|
||||
#include <xrpl/protocol/jss.h>
|
||||
|
||||
#include <algorithm>
|
||||
#include <limits>
|
||||
|
||||
namespace ripple {
|
||||
namespace test {
|
||||
@@ -489,6 +491,91 @@ struct Directory_test : public beast::unit_test::suite
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testDirectoryFull()
|
||||
{
|
||||
using namespace test::jtx;
|
||||
Account alice("alice");
|
||||
|
||||
auto const testCase = [&, this](FeatureBitset features, auto setup) {
|
||||
using namespace test::jtx;
|
||||
|
||||
Env env(*this, features);
|
||||
env.fund(XRP(20000), alice);
|
||||
env.close();
|
||||
|
||||
auto const [lastPage, full] = setup(env);
|
||||
|
||||
// Populate root page and last page
|
||||
for (int i = 0; i < 63; ++i)
|
||||
env(credentials::create(alice, alice, std::to_string(i)));
|
||||
env.close();
|
||||
|
||||
// NOTE, everything below can only be tested on open ledger because
|
||||
// there is no transaction type to express what bumpLastPage does.
|
||||
|
||||
// Bump position of last page from 1 to highest possible
|
||||
auto const res = directory::bumpLastPage(
|
||||
env,
|
||||
lastPage,
|
||||
keylet::ownerDir(alice.id()),
|
||||
[lastPage, this](
|
||||
ApplyView& view, uint256 key, std::uint64_t page) {
|
||||
auto sle = view.peek({ltCREDENTIAL, key});
|
||||
if (!BEAST_EXPECT(sle))
|
||||
return false;
|
||||
|
||||
BEAST_EXPECT(page == lastPage);
|
||||
sle->setFieldU64(sfIssuerNode, page);
|
||||
// sfSubjectNode is not set in self-issued credentials
|
||||
view.update(sle);
|
||||
return true;
|
||||
});
|
||||
BEAST_EXPECT(res);
|
||||
|
||||
// Create one more credential
|
||||
env(credentials::create(alice, alice, std::to_string(63)));
|
||||
|
||||
// Not enough space for another object if full
|
||||
auto const expected = full ? ter{tecDIR_FULL} : ter{tesSUCCESS};
|
||||
env(credentials::create(alice, alice, "foo"), expected);
|
||||
|
||||
// Destroy all objects in directory
|
||||
for (int i = 0; i < 64; ++i)
|
||||
env(credentials::deleteCred(
|
||||
alice, alice, alice, std::to_string(i)));
|
||||
|
||||
if (!full)
|
||||
env(credentials::deleteCred(alice, alice, alice, "foo"));
|
||||
|
||||
// Verify directory is empty.
|
||||
auto const sle = env.le(keylet::ownerDir(alice.id()));
|
||||
BEAST_EXPECT(sle == nullptr);
|
||||
|
||||
// Test completed
|
||||
env.close();
|
||||
};
|
||||
|
||||
testCase(
|
||||
testable_amendments() - fixDirectoryLimit,
|
||||
[this](Env&) -> std::tuple<std::uint64_t, bool> {
|
||||
testcase("directory full without fixDirectoryLimit");
|
||||
return {dirNodeMaxPages - 1, true};
|
||||
});
|
||||
testCase(
|
||||
testable_amendments(), //
|
||||
[this](Env&) -> std::tuple<std::uint64_t, bool> {
|
||||
testcase("directory not full with fixDirectoryLimit");
|
||||
return {dirNodeMaxPages - 1, false};
|
||||
});
|
||||
testCase(
|
||||
testable_amendments(), //
|
||||
[this](Env&) -> std::tuple<std::uint64_t, bool> {
|
||||
testcase("directory full with fixDirectoryLimit");
|
||||
return {std::numeric_limits<std::uint64_t>::max(), true};
|
||||
});
|
||||
}
|
||||
|
||||
void
|
||||
run() override
|
||||
{
|
||||
@@ -497,6 +584,7 @@ struct Directory_test : public beast::unit_test::suite
|
||||
testRipd1353();
|
||||
testEmptyChain();
|
||||
testPreviousTxnID();
|
||||
testDirectoryFull();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -162,7 +162,7 @@ CredentialCreate::doApply()
|
||||
<< to_string(credentialKey.key) << ": "
|
||||
<< (page ? "success" : "failure");
|
||||
if (!page)
|
||||
return tecDIR_FULL; // LCOV_EXCL_LINE
|
||||
return tecDIR_FULL;
|
||||
sleCred->setFieldU64(sfIssuerNode, *page);
|
||||
|
||||
adjustOwnerCount(view(), sleIssuer, 1, j_);
|
||||
@@ -182,7 +182,7 @@ CredentialCreate::doApply()
|
||||
<< to_string(credentialKey.key) << ": "
|
||||
<< (page ? "success" : "failure");
|
||||
if (!page)
|
||||
return tecDIR_FULL; // LCOV_EXCL_LINE
|
||||
return tecDIR_FULL;
|
||||
sleCred->setFieldU64(sfSubjectNode, *page);
|
||||
view().update(view().peek(keylet::account(subject)));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user