Merge remote-tracking branch 'XRPLF/develop' into ximinez/lending-XLS-66

* XRPLF/develop:
  refactor: Improve txset handling (5951)
  Remove directory size limit (5935)
  fix: Change Credential sfSubjectNode to optional (5936)
  refactor: Add support for extra transaction signature validation (5851)
  refactor: Retire fixQualityUpperBound amendment (5960)
  refactor: Retire fix1623 amendment (5928)
  refactor: Retire fixTakerDryOfferRemoval amendment (5958)
  ci: Check whether test failures are caused by port exhaustion (5938)
  chore: Use new prepare-runner (5970)
This commit is contained in:
Ed Hennis
2025-10-31 13:50:46 -04:00
22 changed files with 525 additions and 189 deletions

View File

@@ -63,7 +63,7 @@ jobs:
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
- name: Prepare runner
uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
uses: XRPLF/actions/.github/actions/prepare-runner@99685816bb60a95a66852f212f382580e180df3a
with:
disable_ccache: false

View File

@@ -100,3 +100,12 @@ jobs:
"$test_file"
fi
done
- name: Debug failure (Linux)
if: ${{ failure() && runner.os == 'Linux' && inputs.run_tests }}
shell: bash
run: |
echo "IPv4 local port range:"
cat /proc/sys/net/ipv4/ip_local_port_range
echo "Netstat:"
netstat -an

View File

@@ -66,7 +66,7 @@ jobs:
uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0
- name: Prepare runner
uses: XRPLF/actions/.github/actions/prepare-runner@638e0dc11ea230f91bd26622fb542116bb5254d5
uses: XRPLF/actions/.github/actions/prepare-runner@99685816bb60a95a66852f212f382580e180df3a
with:
disable_ccache: false

View File

@@ -56,7 +56,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 */

View File

@@ -33,6 +33,7 @@
// Keep it sorted in reverse chronological order.
XRPL_FEATURE(LendingProtocol, Supported::no, VoteBehavior::DefaultNo)
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)
@@ -98,14 +99,11 @@ XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYe
XRPL_FIX (AmendmentMajorityCalc, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(HardenedValidations, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (QualityUpperBound, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (PayChanRecipientOwnerDir, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (MasterKeyAsRegularKey, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (TakerDryOfferRemoval, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(MultiSignReserve, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(DepositPreauth, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FIX (1623, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(DepositAuth, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes)
@@ -128,8 +126,10 @@ XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete)
// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
// All known amendments and amendments that may appear in a validated
// ledger must be registered either here or above with the "active" amendments
// All known amendments and amendments that may appear in a validated ledger
// must be registered either here or above with the "active" amendments
//
// Please keep this list sorted alphabetically for convenience.
XRPL_RETIRE(fix1201)
XRPL_RETIRE(fix1368)
XRPL_RETIRE(fix1373)
@@ -141,10 +141,13 @@ XRPL_RETIRE(fix1528)
XRPL_RETIRE(fix1543)
XRPL_RETIRE(fix1571)
XRPL_RETIRE(fix1578)
XRPL_RETIRE(fix1623)
XRPL_RETIRE(fix1781)
XRPL_RETIRE(fixCheckThreading)
XRPL_RETIRE(fixQualityUpperBound)
XRPL_RETIRE(fixRmSmallIncreasedQOffers)
XRPL_RETIRE(fixSTAmountCanonicalize)
XRPL_RETIRE(fixTakerDryOfferRemoval)
XRPL_RETIRE(CryptoConditions)
XRPL_RETIRE(Escrow)
XRPL_RETIRE(EnforceInvariants)

View File

@@ -458,7 +458,7 @@ LEDGER_ENTRY(ltCREDENTIAL, 0x0081, Credential, credential, ({
{sfExpiration, soeOPTIONAL},
{sfURI, soeOPTIONAL},
{sfIssuerNode, soeREQUIRED},
{sfSubjectNode, soeREQUIRED},
{sfSubjectNode, soeOPTIONAL},
{sfPreviousTxnID, soeREQUIRED},
{sfPreviousTxnLgrSeq, soeREQUIRED},
}))

View File

@@ -22,6 +22,9 @@
#include <xrpl/ledger/ApplyView.h>
#include <xrpl/protocol/Protocol.h>
#include <limits>
#include <type_traits>
namespace ripple {
namespace directory {
@@ -110,11 +113,22 @@ insertPage(
Keylet const& directory,
std::function<void(std::shared_ptr<SLE> const&)> const& describe)
{
// 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 (!view.rules().enabled(fixDirectoryLimit) &&
page >= dirNodeMaxPages) // Old pages limit
return std::nullopt;
}
// We are about to create a new node; we'll link it to
// the chain first:

View File

@@ -1867,49 +1867,35 @@ class Check_test : public beast::unit_test::suite
}
void
testFix1623Enable(FeatureBitset features)
testDeliveredAmountForCheckCashTxn(FeatureBitset features)
{
testcase("Fix1623 enable");
testcase("DeliveredAmount For CheckCash Txn");
using namespace test::jtx;
Account const alice{"alice"};
Account const bob{"bob"};
auto testEnable = [this](
FeatureBitset const& features, bool hasFields) {
// Unless fix1623 is enabled a "tx" RPC command should return
// neither "DeliveredAmount" nor "delivered_amount" on a CheckCash
// transaction.
Account const alice{"alice"};
Account const bob{"bob"};
Env env{*this, features};
Env env{*this, features};
env.fund(XRP(1000), alice, bob);
env.close();
env.fund(XRP(1000), alice, bob);
env.close();
uint256 const chkId{getCheckIndex(alice, env.seq(alice))};
env(check::create(alice, bob, XRP(200)));
env.close();
uint256 const chkId{getCheckIndex(alice, env.seq(alice))};
env(check::create(alice, bob, XRP(200)));
env.close();
env(check::cash(bob, chkId, check::DeliverMin(XRP(100))));
env(check::cash(bob, chkId, check::DeliverMin(XRP(100))));
// Get the hash for the most recent transaction.
std::string const txHash{
env.tx()->getJson(JsonOptions::none)[jss::hash].asString()};
// Get the hash for the most recent transaction.
std::string const txHash{
env.tx()->getJson(JsonOptions::none)[jss::hash].asString()};
env.close();
Json::Value const meta = env.rpc("tx", txHash)[jss::result][jss::meta];
// DeliveredAmount and delivered_amount are either present or
// not present in the metadata returned by "tx" based on fix1623.
env.close();
Json::Value const meta =
env.rpc("tx", txHash)[jss::result][jss::meta];
BEAST_EXPECT(
meta.isMember(sfDeliveredAmount.jsonName) == hasFields);
BEAST_EXPECT(meta.isMember(jss::delivered_amount) == hasFields);
};
// Run both the disabled and enabled cases.
testEnable(features - fix1623, false);
testEnable(features, true);
// DeliveredAmount and delivered_amount are present.
BEAST_EXPECT(meta.isMember(sfDeliveredAmount.jsonName));
BEAST_EXPECT(meta.isMember(jss::delivered_amount));
}
void
@@ -2711,7 +2697,7 @@ class Check_test : public beast::unit_test::suite
testCashInvalid(features);
testCancelValid(features);
testCancelInvalid(features);
testFix1623Enable(features);
testDeliveredAmountForCheckCashTxn(features);
testWithTickets(features);
}

View File

@@ -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);

View File

@@ -2010,7 +2010,7 @@ class Invariants_test : public beast::unit_test::suite
{
// Create the directory
BEAST_EXPECT(
directory::createRoot(
::ripple::directory::createRoot(
ac.view(),
dirKeylet,
loanBrokerKeylet.key,
@@ -2037,7 +2037,7 @@ class Invariants_test : public beast::unit_test::suite
describeOwnerDir(slePseudo->at(sfAccount));
BEAST_EXPECT(
directory::insertPage(
::ripple::directory::insertPage(
ac.view(),
0,
sleDir,
@@ -2070,7 +2070,7 @@ class Invariants_test : public beast::unit_test::suite
// Put some extra garbage into the directory
for (auto const& key : {slePseudo->key(), sleDir->key()})
{
directory::insertKey(
::ripple::directory::insertKey(
ac.view(), sleDir, 0, false, indexes, key);
}
@@ -2101,7 +2101,7 @@ class Invariants_test : public beast::unit_test::suite
// Put one meaningless key into the directory
auto const key =
keylet::account(Account("random").id()).key;
directory::insertKey(
::ripple::directory::insertKey(
ac.view(), sleDir, 0, false, indexes, key);
return true;
@@ -2127,7 +2127,7 @@ class Invariants_test : public beast::unit_test::suite
// failure.
STVector256 indexes;
directory::insertKey(
::ripple::directory::insertKey(
ac.view(), sleDir, 0, false, indexes, slePseudo->key());
return true;

View File

@@ -5294,14 +5294,12 @@ public:
{
using namespace jtx;
static FeatureBitset const all{testable_amendments()};
static FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval};
static FeatureBitset const immediateOfferKilled{
featureImmediateOfferKilled};
FeatureBitset const fillOrKill{fixFillOrKill};
FeatureBitset const permDEX{featurePermissionedDEX};
static std::array<FeatureBitset, 6> const feats{
all - takerDryOffer - immediateOfferKilled - permDEX,
static std::array<FeatureBitset, 5> const feats{
all - immediateOfferKilled - permDEX,
all - immediateOfferKilled - fillOrKill - permDEX,
all - fillOrKill - permDEX,
@@ -5323,7 +5321,7 @@ public:
}
};
class OfferWTakerDryOffer_test : public OfferBaseUtil_test
class OfferWOSmallQOffers_test : public OfferBaseUtil_test
{
void
run() override
@@ -5332,7 +5330,7 @@ class OfferWTakerDryOffer_test : public OfferBaseUtil_test
}
};
class OfferWOSmallQOffers_test : public OfferBaseUtil_test
class OfferWOFillOrKill_test : public OfferBaseUtil_test
{
void
run() override
@@ -5341,7 +5339,7 @@ class OfferWOSmallQOffers_test : public OfferBaseUtil_test
}
};
class OfferWOFillOrKill_test : public OfferBaseUtil_test
class OfferWOPermDEX_test : public OfferBaseUtil_test
{
void
run() override
@@ -5350,21 +5348,12 @@ class OfferWOFillOrKill_test : public OfferBaseUtil_test
}
};
class OfferWOPermDEX_test : public OfferBaseUtil_test
{
void
run() override
{
OfferBaseUtil_test::run(4);
}
};
class OfferAllFeatures_test : public OfferBaseUtil_test
{
void
run() override
{
OfferBaseUtil_test::run(5, true);
OfferBaseUtil_test::run(4, true);
}
};
@@ -5376,7 +5365,6 @@ class Offer_manual_test : public OfferBaseUtil_test
using namespace jtx;
FeatureBitset const all{testable_amendments()};
FeatureBitset const immediateOfferKilled{featureImmediateOfferKilled};
FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval};
FeatureBitset const fillOrKill{fixFillOrKill};
FeatureBitset const permDEX{featurePermissionedDEX};
@@ -5385,13 +5373,10 @@ class Offer_manual_test : public OfferBaseUtil_test
testAll(all - fillOrKill - permDEX);
testAll(all - permDEX);
testAll(all);
testAll(all - takerDryOffer - permDEX);
}
};
BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, app, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWTakerDryOffer, app, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, app, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, app, ripple, 2);
BEAST_DEFINE_TESTSUITE_PRIO(OfferWOPermDEX, app, ripple, 2);

View File

@@ -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
View 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

View 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

View File

@@ -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();
}
};

View File

@@ -144,8 +144,8 @@ class Feature_test : public beast::unit_test::suite
BEAST_EXPECT(featureToName(featureFlow) == "Flow");
BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL");
BEAST_EXPECT(
featureToName(fixTakerDryOfferRemoval) ==
"fixTakerDryOfferRemoval");
featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields");
BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow");
}
void

View File

@@ -844,24 +844,6 @@ DirectStepI<TDerived>::qualityUpperBound(
{
auto const dir = this->debtDirection(v, StrandDirection::forward);
if (!v.rules().enabled(fixQualityUpperBound))
{
std::uint32_t const srcQOut = [&]() -> std::uint32_t {
if (redeems(prevStepDir) && issues(dir))
return transferRate(v, src_).value;
return QUALITY_ONE;
}();
auto dstQIn = static_cast<TDerived const*>(this)->quality(
v, QualityDirection::in);
if (isLast_ && dstQIn > QUALITY_ONE)
dstQIn = QUALITY_ONE;
Issue const iss{currency_, src_};
return {
Quality(getRate(STAmount(iss, srcQOut), STAmount(iss, dstQIn))),
dir};
}
auto const [srcQOut, dstQIn] = redeems(dir)
? qualitiesSrcRedeems(v)
: qualitiesSrcIssues(v, prevStepDir);

View File

@@ -276,7 +276,6 @@ CashCheck::doApply()
// work to do...
auto viewJ = ctx_.app.journal("View");
auto const optDeliverMin = ctx_.tx[~sfDeliverMin];
bool const doFix1623{psb.rules().enabled(fix1623)};
if (srcId != account_)
{
@@ -311,7 +310,7 @@ CashCheck::doApply()
return tecUNFUNDED_PAYMENT;
}
if (optDeliverMin && doFix1623)
if (optDeliverMin)
// Set the DeliveredAmount metadata.
ctx_.deliver(xrpDeliver);
@@ -461,7 +460,7 @@ CashCheck::doApply()
<< "flow did not produce DeliverMin.";
return tecPATH_PARTIAL;
}
if (doFix1623 && !checkCashMakesTrustLine)
if (!checkCashMakesTrustLine)
// Set the delivered_amount metadata.
ctx_.deliver(result.actualAmountOut);
}

View File

@@ -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)));
}

View File

@@ -1228,7 +1228,16 @@ OverlayImpl::relay(
{
auto& txn = tx->get();
SerialIter sit(makeSlice(txn.rawtransaction()));
relay = !isPseudoTx(STTx{sit});
try
{
relay = !isPseudoTx(STTx{sit});
}
catch (std::exception const&)
{
// Could not construct STTx, not relaying
JLOG(journal_.debug()) << "Could not construct STTx: " << hash;
return;
}
}
Overlay::PeerSequence peers = {};

View File

@@ -78,51 +78,25 @@ getDeliveredAmount(
}
// Returns true if transaction meta could contain a delivered amount field,
// based on transaction type, transaction result and whether fix1623 is enabled
template <class GetFix1623Enabled>
// based on transaction type and transaction result
bool
canHaveDeliveredAmountHelp(
GetFix1623Enabled const& getFix1623Enabled,
canHaveDeliveredAmount(
std::shared_ptr<STTx const> const& serializedTx,
TxMeta const& transactionMeta)
{
if (!serializedTx)
return false;
TxType const tt{serializedTx->getTxnType()};
// Transaction type should be ttPAYMENT, ttACCOUNT_DELETE or ttCHECK_CASH
// and if the transaction failed nothing could have been delivered.
if ((tt == ttPAYMENT || tt == ttCHECK_CASH || tt == ttACCOUNT_DELETE) &&
transactionMeta.getResultTER() == tesSUCCESS)
{
TxType const tt{serializedTx->getTxnType()};
if (tt != ttPAYMENT && tt != ttCHECK_CASH && tt != ttACCOUNT_DELETE)
return false;
if (tt == ttCHECK_CASH && !getFix1623Enabled())
return false;
return true;
}
// if the transaction failed nothing could have been delivered.
if (transactionMeta.getResultTER() != tesSUCCESS)
return false;
return true;
}
// Returns true if transaction meta could contain a delivered amount field,
// based on transaction type, transaction result and whether fix1623 is enabled
bool
canHaveDeliveredAmount(
RPC::Context const& context,
std::shared_ptr<STTx const> const& serializedTx,
TxMeta const& transactionMeta)
{
// These lambdas are used to compute the values lazily
auto const getFix1623Enabled = [&context]() -> bool {
auto const view = context.app.openLedger().current();
if (!view)
return false;
return view->rules().enabled(fix1623);
};
return canHaveDeliveredAmountHelp(
getFix1623Enabled, serializedTx, transactionMeta);
return false;
}
void
@@ -133,12 +107,8 @@ insertDeliveredAmount(
TxMeta const& transactionMeta)
{
auto const info = ledger.info();
auto const getFix1623Enabled = [&ledger] {
return ledger.rules().enabled(fix1623);
};
if (canHaveDeliveredAmountHelp(
getFix1623Enabled, serializedTx, transactionMeta))
if (canHaveDeliveredAmount(serializedTx, transactionMeta))
{
auto const getLedgerIndex = [&info] { return info.seq; };
auto const getCloseTime = [&info] { return info.closeTime; };
@@ -167,7 +137,7 @@ getDeliveredAmount(
TxMeta const& transactionMeta,
GetLedgerIndex const& getLedgerIndex)
{
if (canHaveDeliveredAmount(context, serializedTx, transactionMeta))
if (canHaveDeliveredAmount(serializedTx, transactionMeta))
{
auto const getCloseTime =
[&context,
@@ -212,7 +182,7 @@ insertDeliveredAmount(
std::shared_ptr<STTx const> const& transaction,
TxMeta const& transactionMeta)
{
if (canHaveDeliveredAmount(context, transaction, transactionMeta))
if (canHaveDeliveredAmount(transaction, transactionMeta))
{
auto amt = getDeliveredAmount(
context, transaction, transactionMeta, [&transactionMeta]() {

View File

@@ -18,6 +18,7 @@
//==============================================================================
#include <xrpld/shamap/SHAMap.h>
#include <xrpld/shamap/SHAMapLeafNode.h>
#include <xrpld/shamap/SHAMapSyncFilter.h>
#include <xrpl/basics/random.h>
@@ -591,16 +592,16 @@ SHAMap::addKnownNode(
}
auto const generation = f_.getFullBelowCache()->getGeneration();
SHAMapNodeID iNodeID;
auto iNode = root_.get();
SHAMapNodeID currNodeID;
auto currNode = root_.get();
while (iNode->isInner() &&
!static_cast<SHAMapInnerNode*>(iNode)->isFullBelow(generation) &&
(iNodeID.getDepth() < node.getDepth()))
while (currNode->isInner() &&
!static_cast<SHAMapInnerNode*>(currNode)->isFullBelow(generation) &&
(currNodeID.getDepth() < node.getDepth()))
{
int branch = selectBranch(iNodeID, node.getNodeID());
int const branch = selectBranch(currNodeID, node.getNodeID());
XRPL_ASSERT(branch >= 0, "ripple::SHAMap::addKnownNode : valid branch");
auto inner = static_cast<SHAMapInnerNode*>(iNode);
auto inner = static_cast<SHAMapInnerNode*>(currNode);
if (inner->isEmptyBranch(branch))
{
JLOG(journal_.warn()) << "Add known node for empty branch" << node;
@@ -614,58 +615,84 @@ SHAMap::addKnownNode(
}
auto prevNode = inner;
std::tie(iNode, iNodeID) = descend(inner, iNodeID, branch, filter);
std::tie(currNode, currNodeID) =
descend(inner, currNodeID, branch, filter);
if (iNode == nullptr)
if (currNode != nullptr)
continue;
auto newNode = SHAMapTreeNode::makeFromWire(rawNode);
if (!newNode || childHash != newNode->getHash())
{
auto newNode = SHAMapTreeNode::makeFromWire(rawNode);
JLOG(journal_.warn()) << "Corrupt node received";
return SHAMapAddNode::invalid();
}
if (!newNode || childHash != newNode->getHash())
// In rare cases, a node can still be corrupt even after hash
// validation. For leaf nodes, we perform an additional check to
// ensure the node's position in the tree is consistent with its
// content to prevent inconsistencies that could
// propagate further down the line.
if (newNode->isLeaf())
{
auto const& actualKey =
static_cast<SHAMapLeafNode const*>(newNode.get())
->peekItem()
->key();
// Validate that this leaf belongs at the target position
auto const expectedNodeID =
SHAMapNodeID::createID(node.getDepth(), actualKey);
if (expectedNodeID.getNodeID() != node.getNodeID())
{
JLOG(journal_.warn()) << "Corrupt node received";
JLOG(journal_.debug())
<< "Leaf node position mismatch: "
<< "expected=" << expectedNodeID.getNodeID()
<< ", actual=" << node.getNodeID();
return SHAMapAddNode::invalid();
}
}
// Inner nodes must be at a level strictly less than 64
// but leaf nodes (while notionally at level 64) can be
// at any depth up to and including 64:
if ((iNodeID.getDepth() > leafDepth) ||
(newNode->isInner() && iNodeID.getDepth() == leafDepth))
{
// Map is provably invalid
state_ = SHAMapState::Invalid;
return SHAMapAddNode::useful();
}
if (iNodeID != node)
{
// Either this node is broken or we didn't request it (yet)
JLOG(journal_.warn()) << "unable to hook node " << node;
JLOG(journal_.info()) << " stuck at " << iNodeID;
JLOG(journal_.info()) << "got depth=" << node.getDepth()
<< ", walked to= " << iNodeID.getDepth();
return SHAMapAddNode::useful();
}
if (backed_)
canonicalize(childHash, newNode);
newNode = prevNode->canonicalizeChild(branch, std::move(newNode));
if (filter)
{
Serializer s;
newNode->serializeWithPrefix(s);
filter->gotNode(
false,
childHash,
ledgerSeq_,
std::move(s.modData()),
newNode->getType());
}
// Inner nodes must be at a level strictly less than 64
// but leaf nodes (while notionally at level 64) can be
// at any depth up to and including 64:
if ((currNodeID.getDepth() > leafDepth) ||
(newNode->isInner() && currNodeID.getDepth() == leafDepth))
{
// Map is provably invalid
state_ = SHAMapState::Invalid;
return SHAMapAddNode::useful();
}
if (currNodeID != node)
{
// Either this node is broken or we didn't request it (yet)
JLOG(journal_.warn()) << "unable to hook node " << node;
JLOG(journal_.info()) << " stuck at " << currNodeID;
JLOG(journal_.info()) << "got depth=" << node.getDepth()
<< ", walked to= " << currNodeID.getDepth();
return SHAMapAddNode::useful();
}
if (backed_)
canonicalize(childHash, newNode);
newNode = prevNode->canonicalizeChild(branch, std::move(newNode));
if (filter)
{
Serializer s;
newNode->serializeWithPrefix(s);
filter->gotNode(
false,
childHash,
ledgerSeq_,
std::move(s.modData()),
newNode->getType());
}
return SHAMapAddNode::useful();
}
JLOG(journal_.trace()) << "got node, already had it (late)";