ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-02 16:26:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Fix wrong hybrid offer orderbook placement and update LedgerStateFix to amend ExchangeRate meta (#7087)

Browse Source 
Co-authored-by: Peter Chen <ychen@ripple.com>
This commit is contained in:
Shawn Xie
2026-05-21 02:19:04 -04:00
committed by GitHub
parent a830ab10ef
commit 28cc20c816
15 changed files with 669 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
include/xrpl/protocol/detail/transactions.macro
View File

@@ -688,6 +688,7 @@ TRANSACTION(ttLEDGER_STATE_FIX, 53, LedgerStateFix,
({
{sfLedgerFixType, SoeRequired},
{sfOwner, SoeOptional},
{sfBookDirectory, SoeOptional},
}))
/** This transaction type creates a MPTokensIssuance instance */

37
include/xrpl/protocol_autogen/transactions/LedgerStateFix.h
View File

@@ -83,6 +83,32 @@ public:
{
return this->tx_->isFieldPresent(sfOwner);
}
/**
* @brief Get sfBookDirectory (SoeOptional)
* @return The field value, or std::nullopt if not present.
*/
[[nodiscard]]
protocol_autogen::Optional<SF_UINT256::type::value_type>
getBookDirectory() const
{
if (hasBookDirectory())
{
return this->tx_->at(sfBookDirectory);
}
return std::nullopt;
}
/**
* @brief Check if sfBookDirectory is present.
* @return True if the field is present, false otherwise.
*/
[[nodiscard]]
bool
hasBookDirectory() const
{
return this->tx_->isFieldPresent(sfBookDirectory);
}
};
/**
@@ -149,6 +175,17 @@ public:
return *this;
}
/**
* @brief Set sfBookDirectory (SoeOptional)
* @return Reference to this builder for method chaining.
*/
LedgerStateFixBuilder&
setBookDirectory(std::decay_t<typename SF_UINT256::type::value_type> const& value)
{
object_[sfBookDirectory] = value;
return *this;
}
/**
* @brief Build and return the LedgerStateFix wrapper.
* @param publicKey The public key for signing.

27
include/xrpl/tx/invariants/DirectoryInvariant.h Normal file
View File

@@ -0,0 +1,27 @@
#pragma once
#include <xrpl/basics/base_uint.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <memory>
namespace xrpl {
class ValidBookDirectory
{
bool badBookDirectory_ = false;
hash_set<uint256> rootIndexes_;
public:
void
visitEntry(bool, std::shared_ptr<SLE const> const&, std::shared_ptr<SLE const> const&);
bool
finalize(STTx const&, TER const, XRPAmount const, ReadView const&, beast::Journal const&);
};
} // namespace xrpl

2
include/xrpl/tx/invariants/InvariantCheck.h
View File

@@ -6,6 +6,7 @@
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/tx/invariants/AMMInvariant.h>
#include <xrpl/tx/invariants/DirectoryInvariant.h>
#include <xrpl/tx/invariants/FreezeInvariant.h>
#include <xrpl/tx/invariants/LoanBrokerInvariant.h>
#include <xrpl/tx/invariants/LoanInvariant.h>
@@ -393,6 +394,7 @@ using InvariantChecks = std::tuple<
ValidMPTIssuance,
ValidPermissionedDomain,
ValidPermissionedDEX,
ValidBookDirectory,
ValidAMM,
NoModifiedUnmodifiableFields,
ValidPseudoAccounts,

1
include/xrpl/tx/transactors/dex/OfferCreate.h
View File

@@ -85,6 +85,7 @@ private:
Keylet const& offerIndex,
STAmount const& saTakerPays,
STAmount const& saTakerGets,
std::uint64_t openRate,
std::function<void(SLE::ref, std::optional<uint256>)> const& setDir);
};

1
include/xrpl/tx/transactors/system/LedgerStateFix.h
View File

@@ -9,6 +9,7 @@ class LedgerStateFix : public Transactor
public:
enum class FixType : std::uint16_t {
NfTokenPageLink = 1,
BookExchangeRate = 2,
};
static constexpr auto kConsequencesFactory = ConsequencesFactoryType::Normal;

96
src/libxrpl/tx/invariants/DirectoryInvariant.cpp Normal file
View File

@@ -0,0 +1,96 @@
#include <xrpl/tx/invariants/DirectoryInvariant.h>
#include <xrpl/basics/Log.h>
#include <xrpl/beast/utility/Journal.h>
#include <xrpl/ledger/ReadView.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/XRPAmount.h>
#include <memory>
namespace xrpl {
namespace {
[[nodiscard]] bool
isRootBookDirectory(SLE const& dir)
{
// Child page keys do not encode book quality.
return dir.isFieldPresent(sfExchangeRate) || dir.isFieldPresent(sfTakerPaysCurrency) ||
dir.isFieldPresent(sfTakerPaysIssuer) || dir.isFieldPresent(sfTakerPaysMPT) ||
dir.isFieldPresent(sfTakerGetsCurrency) || dir.isFieldPresent(sfTakerGetsIssuer) ||
dir.isFieldPresent(sfTakerGetsMPT) || dir.isFieldPresent(sfDomainID);
}
[[nodiscard]] bool
badExchangeRate(SLE const& dir)
{
return isRootBookDirectory(dir) &&
(!dir.isFieldPresent(sfExchangeRate) ||
dir.getFieldU64(sfExchangeRate) != getQuality(dir.key()));
}
} // namespace
void
ValidBookDirectory::visitEntry(
bool,
std::shared_ptr<SLE const> const& before,
std::shared_ptr<SLE const> const& after)
{
// New root directories must have matching exchange-rate metadata. New
// child directories must point to an existing root.
// Only validate newly-created directories; LedgerStateFix handles legacy
// bad exchange-rate metadata.
if (badBookDirectory_ || before || !after || after->getType() != ltDIR_NODE)
return;
auto const rootIndex = after->getFieldH256(sfRootIndex);
if (after->key() == rootIndex && !badBookDirectory_)
{
badBookDirectory_ = badBookDirectory_ || badExchangeRate(*after);
return;
}
rootIndexes_.insert(rootIndex);
}
bool
ValidBookDirectory::finalize(
STTx const&,
TER const,
XRPAmount const,
ReadView const& view,
beast::Journal const& j)
{
if (!view.rules().enabled(fixCleanup3_2_0))
return true;
if (badBookDirectory_)
{
JLOG(j.fatal()) << "Invariant failed: book directory exchange rate "
"does not match directory quality";
return false;
}
for (auto const& rootIndex : rootIndexes_)
{
auto const root = view.read(Keylet(ltDIR_NODE, rootIndex));
if (!root)
{
JLOG(j.fatal()) << "Invariant failed: book directory root missing";
return false;
}
}
return true;
}
} // namespace xrpl

13
src/libxrpl/tx/transactors/dex/OfferCreate.cpp
View File

@@ -547,6 +547,7 @@ OfferCreate::applyHybrid(
Keylet const& offerKey,
STAmount const& saTakerPays,
STAmount const& saTakerGets,
std::uint64_t openRate,
std::function<void(SLE::ref, std::optional<uint256>)> const& setDir)
{
if (!sleOffer->isFieldPresent(sfDomainID))
@@ -558,7 +559,7 @@ OfferCreate::applyHybrid(
// if offer is hybrid, need to also place into open offer dir
Book const book{saTakerPays.asset(), saTakerGets.asset(), std::nullopt};
auto dir = keylet::quality(keylet::kBook(book), getRate(saTakerGets, saTakerPays));
auto dir = keylet::quality(keylet::kBook(book), openRate);
bool const bookExists = sb.exists(dir);
auto const bookNode = sb.dirAppend(dir, offerKey, [&](SLE::ref sle) {
@@ -924,8 +925,16 @@ OfferCreate::applyGuts(Sandbox& sb, Sandbox& sbCancel)
// if it's a hybrid offer, set hybrid flag, and create an open dir
if (bHybrid)
{
// Pre-fixCleanup3_2_0: the open-book directory quality was computed
// from post-crossing amounts, which may differ from the original rate
// due to rounding in rate preservation. Post-fixCleanup3_2_0: use the
// original placement rate so the open-book directory quality matches
// the domain-book directory.
auto const openRate = ctx_.view().rules().enabled(fixCleanup3_2_0)
? uRate
: getRate(saTakerGets, saTakerPays);
auto const res =
applyHybrid(sb, sleOffer, offerIndex, saTakerPays, saTakerGets, setBookDir);
applyHybrid(sb, sleOffer, offerIndex, saTakerPays, saTakerGets, openRate, setBookDir);
if (!isTesSuccess(res))
return {res, true}; // LCOV_EXCL_LINE
}

86
src/libxrpl/tx/transactors/system/LedgerStateFix.cpp
View File

@@ -4,7 +4,9 @@
#include <xrpl/ledger/ReadView.h>
#include <xrpl/ledger/helpers/NFTokenHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STLedgerEntry.h>
#include <xrpl/protocol/STTx.h>
@@ -12,24 +14,72 @@
#include <xrpl/protocol/XRPAmount.h>
#include <xrpl/tx/Transactor.h>
#include <algorithm>
#include <array>
#include <memory>
#include <utility>
namespace xrpl {
namespace {
using FixType = LedgerStateFix::FixType;
std::array<std::pair<FixType, SField const*>, 2> const kLedgerFixFields = {{
{FixType::NfTokenPageLink, &sfOwner},
{FixType::BookExchangeRate, &sfBookDirectory},
}};
[[nodiscard]] SField const*
fixField(FixType const fixType)
{
auto const iter = std::ranges::find_if(
kLedgerFixFields, [fixType](auto const& entry) { return entry.first == fixType; });
if (iter == kLedgerFixFields.end())
return nullptr; // LCOV_EXCL_LINE
return iter->second;
}
[[nodiscard]] bool
hasUnexpectedFixField(STTx const& tx, SField const& expected)
{
return std::ranges::any_of(kLedgerFixFields, [&tx, &expected](auto const& entry) {
auto const field = entry.second;
return field != &expected && tx.isFieldPresent(*field);
});
}
} // namespace
NotTEC
LedgerStateFix::preflight(PreflightContext const& ctx)
{
switch (static_cast<FixType>(ctx.tx[sfLedgerFixType]))
auto const fixType = static_cast<FixType>(ctx.tx[sfLedgerFixType]);
switch (fixType)
{
case FixType::NfTokenPageLink:
if (!ctx.tx.isFieldPresent(sfOwner))
return temINVALID;
break;
case FixType::BookExchangeRate:
if (!ctx.rules.enabled(fixCleanup3_2_0))
return temDISABLED;
break;
default:
return tefINVALID_LEDGER_FIX_TYPE;
}
auto const expectedField = fixField(fixType);
if (expectedField == nullptr)
return tefINVALID_LEDGER_FIX_TYPE; // LCOV_EXCL_LINE
// Each fix type allows exactly one fix-specific field.
if (!ctx.tx.isFieldPresent(*expectedField) || hasUnexpectedFixField(ctx.tx, *expectedField))
return temINVALID;
return tesSUCCESS;
}
@@ -53,6 +103,24 @@ LedgerStateFix::preclaim(PreclaimContext const& ctx)
return tesSUCCESS;
}
if (static_cast<FixType>(ctx.tx[sfLedgerFixType]) == FixType::BookExchangeRate)
{
auto const dirKey = ctx.tx.getFieldH256(sfBookDirectory);
auto const sle = ctx.view.read(Keylet(ltDIR_NODE, dirKey));
if (!sle)
return tecOBJECT_NOT_FOUND;
// Must be the first page of a book directory (has sfExchangeRate).
if (!sle->isFieldPresent(sfExchangeRate))
return tecNO_PERMISSION;
// ExchangeRate is already correct, nothing to fix.
if (getQuality(sle->key()) == sle->getFieldU64(sfExchangeRate))
return tecNO_PERMISSION;
return tesSUCCESS;
}
// preflight is supposed to verify that only valid FixTypes get to preclaim.
return tecINTERNAL; // LCOV_EXCL_LINE
}
@@ -68,6 +136,18 @@ LedgerStateFix::doApply()
return tesSUCCESS;
}
if (static_cast<FixType>(ctx_.tx[sfLedgerFixType]) == FixType::BookExchangeRate)
{
auto const dirKey = ctx_.tx.getFieldH256(sfBookDirectory);
auto sle = view().peek(Keylet(ltDIR_NODE, dirKey));
if (!sle)
return tecINTERNAL; // LCOV_EXCL_LINE
sle->setFieldU64(sfExchangeRate, getQuality(sle->key()));
view().update(sle);
return tesSUCCESS;
}
// preflight is supposed to verify that only valid FixTypes get to doApply.
return tecINTERNAL; // LCOV_EXCL_LINE
}

7
src/test/app/FixNFTokenPageLinks_test.cpp
View File

@@ -173,6 +173,13 @@ class FixNFTokenPageLinks_test : public beast::unit_test::Suite
tx.removeMember(sfOwner.jsonName);
env(tx, Fee(linkFixFee), Ter(temINVALID));
}
{
// NFTokenPageLink fixes require sfOwner and reject fields that
// belong to other LedgerStateFix types.
json::Value tx = ledgerStateFix::nftPageLinks(alice, alice);
tx[sfBookDirectory.jsonName] = to_string(uint256{1});
env(tx, Fee(linkFixFee), Ter(temINVALID));
}
{
// Invalid LedgerFixType codes.
json::Value tx = ledgerStateFix::nftPageLinks(alice, alice);

103
src/test/app/Invariants_test.cpp
View File

@@ -24,6 +24,7 @@
#include <xrpl/ledger/helpers/DirectoryHelpers.h>
#include <xrpl/ledger/helpers/RippleStateHelpers.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Book.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/InnerObjectFormats.h>
@@ -50,6 +51,7 @@
#include <xrpl/tx/ApplyContext.h>
#include <xrpl/tx/Transactor.h>
#include <xrpl/tx/applySteps.h>
#include <xrpl/tx/invariants/DirectoryInvariant.h>
#include <xrpl/tx/invariants/VaultInvariant.h>
#include <algorithm>
@@ -2037,6 +2039,106 @@ class Invariants_test : public beast::unit_test::Suite
}
}
void
testBookDirectoryExchangeRate()
{
using namespace test::jtx;
testcase << "book directory exchange rate";
auto const getBookRootKey = [](Account const& account, std::uint64_t quality) {
Book const book{xrpIssue(), account["USD"], std::nullopt};
return keylet::quality(keylet::kBook(book), quality);
};
// Root book-directory pages carry exchange-rate metadata that must
// match the quality encoded in the directory key.
auto const makeRootPage = [](Keylet const& dir, std::uint64_t exchangeRate) {
auto sleDir = std::make_shared<SLE>(dir);
sleDir->setFieldH256(sfRootIndex, dir.key);
STVector256 indexes;
indexes.pushBack(uint256{1});
sleDir->setFieldV256(sfIndexes, indexes);
sleDir->setFieldU64(sfExchangeRate, exchangeRate);
return sleDir;
};
// Child pages do not carry quality metadata; they only point back to
// the root directory.
auto const makeChildPage = [](Keylet const& rootDir) {
auto sleDir = std::make_shared<SLE>(keylet::page(rootDir, 1));
sleDir->setFieldH256(sfRootIndex, rootDir.key);
STVector256 indexes;
indexes.pushBack(uint256{2});
sleDir->setFieldV256(sfIndexes, indexes);
return sleDir;
};
auto const makeOfferCreateTx = [] {
return STTx{ttOFFER_CREATE, [](STObject& tx) {
Account const account{"A1"};
tx.setFieldAmount(sfTakerPays, XRP(1));
tx.setFieldAmount(sfTakerGets, account["USD"](1));
}};
};
std::initializer_list<TER> const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED};
// Creating a root book directory with mismatched exchange-rate
// metadata violates the invariant.
doInvariantCheck(
{{"book directory exchange rate does not match directory quality"}},
[&](Account const& a1, Account const&, ApplyContext& ac) {
auto const directoryQuality = STAmount::kURateOne;
auto const dir = getBookRootKey(a1, directoryQuality);
ac.view().insert(makeRootPage(dir, directoryQuality + 1));
return true;
},
XRPAmount{},
makeOfferCreateTx(),
failTers);
// A new child page must point to an existing root page.
doInvariantCheck(
{{"book directory root missing"}},
[&](Account const& a1, Account const&, ApplyContext& ac) {
auto const directoryQuality = STAmount::kURateOne;
auto const rootDir = getBookRootKey(a1, directoryQuality);
// Insert only the child page. It points at rootDir, but the
// corresponding root page is intentionally missing.
ac.view().insert(makeChildPage(rootDir));
return true;
},
XRPAmount{},
makeOfferCreateTx(),
failTers);
// Legacy bad-root tolerance:
// - The view contains a pre-existing root page with bad sfExchangeRate
// metadata.
// - The simulated transaction only creates a child page pointing to
// that root.
// - The invariant must pass because this transaction did not create
// the bad root, only adding a child page.
{
Env env{*this, defaultAmendments()};
Account const a1{"A1"};
env.fund(XRP(1000), a1);
env.close();
OpenView view{*env.current()};
auto const directoryQuality = STAmount::kURateOne;
auto const rootDir = getBookRootKey(a1, directoryQuality);
view.rawInsert(makeRootPage(rootDir, directoryQuality + 1));
ValidBookDirectory invariant;
invariant.visitEntry(false, nullptr, makeChildPage(rootDir));
test::StreamSink sink{beast::Severity::Warning};
beast::Journal const jlog{sink};
BEAST_EXPECT(
invariant.finalize(makeOfferCreateTx(), tesSUCCESS, XRPAmount{}, view, jlog));
}
}
Keylet
createLoanBroker(jtx::Account const& a, jtx::Env& env, jtx::PrettyAsset const& asset)
{
@@ -4489,6 +4591,7 @@ public:
testPermissionedDomainInvariants(defaultAmendments() - fixCleanup3_1_3);
testPermissionedDEX(defaultAmendments() | fixCleanup3_1_3);
testPermissionedDEX(defaultAmendments() - fixCleanup3_1_3);
testBookDirectoryExchangeRate();
testNoModifiedUnmodifiableFields();
testValidPseudoAccounts();
testValidLoanBroker();

262
src/test/app/PermissionedDEX_test.cpp
View File

@@ -7,7 +7,9 @@
#include <test/jtx/balance.h>
#include <test/jtx/credentials.h>
#include <test/jtx/domain.h>
#include <test/jtx/fee.h>
#include <test/jtx/jtx_json.h>
#include <test/jtx/ledgerStateFix.h>
#include <test/jtx/offer.h>
#include <test/jtx/owners.h> // IWYU pragma: keep
#include <test/jtx/paths.h>
@@ -1457,6 +1459,263 @@ class PermissionedDEX_test : public beast::unit_test::Suite
}
}
void
testHybridOfferCrossingQuality(FeatureBitset features)
{
bool const fixEnabled = features[fixCleanup3_2_0];
testcase << "Hybrid offer crossing quality"
<< (fixEnabled ? " (fixCleanup3_2_0)" : " (pre-fix)");
// Partially-crossed hybrid offer should have consistent quality
// across both book directories.
//
// Steps:
// - Bob places a hybrid offer.
// - Alice places an opposing hybrid offer that partially crosses.
//
// Verify:
// - Domain-book key quality == its sfExchangeRate.
// - Post-fix: open-book key quality == domain-book key quality.
// - Pre-fix: open-book key quality != domain-book key quality
// (key used post-crossing rate, sfExchangeRate used pre-crossing).
Env env(*this, features);
auto const& [gw_, domainOwner, alice_, bob_, carol_, USD, domainID, credType] =
PermissionedDEX(env);
// Bob places a hybrid offer: TakerPays = XRP(100), TakerGets = USD(40)
auto const bobOfferSeq{env.seq(bob_)};
env(offer(bob_, XRP(100), USD(40)), Txflags(tfHybrid), Domain(domainID));
env.close();
BEAST_EXPECT(offerExists(env, bob_, bobOfferSeq));
// Alice places a hybrid offer in the opposite direction that
// partially crosses Bob's offer.
// Alice: TakerPays = USD(100), TakerGets = XRP(300) (rate = 3 XRP/USD)
// Bob's offer is at a better rate (2.5 XRP/USD) so crossing occurs.
auto const aliceOfferSeq{env.seq(alice_)};
env(offer(alice_, USD(100), XRP(300)), Txflags(tfHybrid), Domain(domainID));
env.close();
// After crossing, Alice's remaining offer should be placed.
auto const sle = env.le(keylet::offer(alice_.id(), aliceOfferSeq));
BEAST_EXPECT(sle);
BEAST_EXPECT(sle->isFieldPresent(sfAdditionalBooks));
BEAST_EXPECT(sle->getFieldArray(sfAdditionalBooks).size() == 1);
auto const domainDirKey = sle->getFieldH256(sfBookDirectory);
auto const openDirKey =
sle->getFieldArray(sfAdditionalBooks)[0].getFieldH256(sfBookDirectory);
auto const domainQuality = getQuality(domainDirKey);
auto const openQuality = getQuality(openDirKey);
// Read the directory SLEs and check sfExchangeRate vs key quality.
auto const domainDirSle = env.le(Keylet(ltDIR_NODE, domainDirKey));
auto const openDirSle = env.le(Keylet(ltDIR_NODE, openDirKey));
BEAST_EXPECT(domainDirSle);
BEAST_EXPECT(openDirSle);
auto const domainExRate = domainDirSle->getFieldU64(sfExchangeRate);
auto const openExRate = openDirSle->getFieldU64(sfExchangeRate);
auto const preCrossingQuality = std::uint64_t{5623825668291712342ULL};
auto const postCrossingQuality = std::uint64_t{5623825668291712341ULL};
// Domain directory: sfExchangeRate should always match key quality
// (both use the pre-crossing rate). Correct behavior.
BEAST_EXPECT(domainQuality == preCrossingQuality);
BEAST_EXPECT(domainExRate == preCrossingQuality);
BEAST_EXPECT(domainExRate == domainQuality);
if (fixEnabled)
{
// Correct behavior: both directory keys use the pre-crossing rate.
BEAST_EXPECT(openQuality == preCrossingQuality);
BEAST_EXPECT(domainQuality == openQuality);
// sfExchangeRate matches key quality on both directories.
BEAST_EXPECT(openExRate == preCrossingQuality);
BEAST_EXPECT(openExRate == openQuality);
}
else
{
// Wrong legacy behavior: the open-book directory key uses the
// post-crossing rate instead of the domain-book rate.
BEAST_EXPECT(openQuality == postCrossingQuality);
BEAST_EXPECT(domainQuality != openQuality);
// The open-book sfExchangeRate still uses the pre-crossing rate,
// so it no longer matches the actual quality encoded in the
// open-book directory key.
BEAST_EXPECT(openExRate == preCrossingQuality);
BEAST_EXPECT(openExRate != openQuality);
BEAST_EXPECT(openExRate == domainQuality);
}
}
void
testBookExchangeRateFix(FeatureBitset features)
{
testcase("LedgerStateFix BookExchangeRate");
// Use the pre-fix path to create a hybrid offer with a mismatched
// sfExchangeRate, then apply LedgerStateFix to correct it.
//
// Steps:
// - Create a partially-crossed hybrid offer (pre-fixCleanup3_2_0)
// so the open-book directory has wrong sfExchangeRate.
// - Re-enable fixCleanup3_2_0 and submit a LedgerStateFix to
// repair the open-book directory's sfExchangeRate.
//
// Verify:
// - Before fix: sfExchangeRate != getQuality(key).
// - After fix: sfExchangeRate == getQuality(key).
{
// Amendment gate: BookExchangeRate fixes require fixCleanup3_2_0.
Env env(*this, features - fixCleanup3_2_0);
Account const carol{"carol"};
env.fund(XRP(1000), carol);
env.close();
env(ledgerStateFix::bookExchangeRate(carol, uint256{1}), Ter(temDISABLED));
}
{
// Preflight check: BookExchangeRate fixes only accept their
// required fix-specific field.
Env env(*this, features);
Account const carol{"carol"};
env.fund(XRP(1000), carol);
env.close();
// BookExchangeRate fixes require sfBookDirectory.
auto missingBookDirectory = ledgerStateFix::bookExchangeRate(carol, uint256{1});
missingBookDirectory.removeMember(sfBookDirectory.jsonName);
env(missingBookDirectory, Ter(temINVALID));
// BookExchangeRate fixes reject fields that belong to other
// LedgerStateFix types.
auto extraOwner = ledgerStateFix::bookExchangeRate(carol, uint256{1});
extraOwner[sfOwner.jsonName] = carol.human();
env(extraOwner, Ter(temINVALID));
}
{
Env env(*this, features);
auto const setup = PermissionedDEX(env);
auto const fixFee = drops(env.current()->fees().increment);
{
// Preclaim check: the target directory must exist.
env(ledgerStateFix::bookExchangeRate(setup.carol, uint256{1}),
Fee(fixFee),
Ter(tecOBJECT_NOT_FOUND));
}
{
// Preclaim check: the target directory must be a book root
// page. Owner directories are ltDIR_NODE entries, but they do
// not carry sfExchangeRate.
auto const ownerDir = keylet::ownerDir(setup.bob.id());
auto const ownerDirSle = env.le(ownerDir);
BEAST_EXPECT(ownerDirSle);
BEAST_EXPECT(!ownerDirSle->isFieldPresent(sfExchangeRate));
env(ledgerStateFix::bookExchangeRate(setup.carol, ownerDir.key),
Fee(fixFee),
Ter(tecNO_PERMISSION));
}
{
// Preclaim check: a correct sfExchangeRate leaves nothing to
// repair.
auto const bobOfferSeq{env.seq(setup.bob)};
env(offer(setup.bob, XRP(100), setup.usd(40)));
env.close();
auto const sle = env.le(keylet::offer(setup.bob.id(), bobOfferSeq));
BEAST_EXPECT(sle);
auto const dirKey = sle->getFieldH256(sfBookDirectory);
{
auto const dirSle = env.le(Keylet(ltDIR_NODE, dirKey));
BEAST_EXPECT(dirSle);
auto const exchangeRate = dirSle->getFieldU64(sfExchangeRate);
auto const quality = getQuality(dirKey);
BEAST_EXPECT(exchangeRate == quality);
}
env(ledgerStateFix::bookExchangeRate(setup.carol, dirKey),
Fee(fixFee),
Ter(tecNO_PERMISSION));
}
}
{
// Repair path: start without fixCleanup3_2_0 to produce the
// mismatch, then enable the amendment and fix it.
Env env(*this, features - fixCleanup3_2_0);
auto const& [gw_, domainOwner, alice_, bob_, carol_, USD, domainID, credType] =
PermissionedDEX(env);
// Bob places a hybrid offer.
env(offer(bob_, XRP(100), USD(40)), Txflags(tfHybrid), Domain(domainID));
env.close();
// Alice partially crosses Bob.
auto const aliceOfferSeq{env.seq(alice_)};
env(offer(alice_, USD(100), XRP(300)), Txflags(tfHybrid), Domain(domainID));
env.close();
auto const sle = env.le(keylet::offer(alice_.id(), aliceOfferSeq));
BEAST_EXPECT(sle);
auto const openDirKey =
sle->getFieldArray(sfAdditionalBooks)[0].getFieldH256(sfBookDirectory);
auto const preCrossingQuality = std::uint64_t{5623825668291712342ULL};
auto const postCrossingQuality = std::uint64_t{5623825668291712341ULL};
// Confirm mismatch exists.
{
auto const dirSle = env.le(Keylet(ltDIR_NODE, openDirKey));
BEAST_EXPECT(dirSle);
auto const exchangeRate = dirSle->getFieldU64(sfExchangeRate);
auto const quality = getQuality(openDirKey);
BEAST_EXPECT(exchangeRate == preCrossingQuality);
BEAST_EXPECT(quality == postCrossingQuality);
BEAST_EXPECT(exchangeRate != quality);
}
// Enable fixCleanup3_2_0 and apply the LedgerStateFix.
env.enableFeature(fixCleanup3_2_0);
env.close();
auto const fixFee = drops(env.current()->fees().increment);
env(ledgerStateFix::bookExchangeRate(carol_, openDirKey), Fee(fixFee));
env.close();
// Confirm sfExchangeRate now matches the key quality.
{
auto const dirSle = env.le(Keylet(ltDIR_NODE, openDirKey));
BEAST_EXPECT(dirSle);
auto const exchangeRate = dirSle->getFieldU64(sfExchangeRate);
auto const quality = getQuality(openDirKey);
BEAST_EXPECT(exchangeRate == postCrossingQuality);
BEAST_EXPECT(quality == postCrossingQuality);
BEAST_EXPECT(exchangeRate == quality);
}
// Submitting again should fail — nothing to fix.
env(ledgerStateFix::bookExchangeRate(carol_, openDirKey),
Fee(fixFee),
Ter(tecNO_PERMISSION));
}
}
void
testCancelRegularOfferWithDomainCreate(FeatureBitset features)
{
@@ -1528,6 +1787,9 @@ public:
testHybridOfferDirectories(all);
testHybridMalformedOffer(all);
testHybridMalformedOffer(all - fixCleanup3_1_3);
testHybridOfferCrossingQuality(all);
testHybridOfferCrossingQuality(all - fixCleanup3_2_0);
testBookExchangeRateFix(all);
// Cancelling a regular offer in a domain OfferCreate is allowed
// only after fixCleanup3_2_0.

13
src/test/jtx/impl/ledgerStateFixes.cpp
View File

@@ -1,6 +1,7 @@
#include <test/jtx/Account.h>
#include <test/jtx/ledgerStateFix.h>
#include <xrpl/basics/base_uint.h>
#include <xrpl/json/json_value.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/jss.h>
@@ -22,4 +23,16 @@ nftPageLinks(jtx::Account const& acct, jtx::Account const& owner)
return jv;
}
// Fix sfExchangeRate on a book directory. acct pays fee.
json::Value
bookExchangeRate(jtx::Account const& acct, uint256 const& bookDir)
{
json::Value jv;
jv[sfAccount.jsonName] = acct.human();
jv[sfLedgerFixType.jsonName] = static_cast<uint16_t>(LedgerStateFix::FixType::BookExchangeRate);
jv[sfBookDirectory.jsonName] = to_string(bookDir);
jv[sfTransactionType.jsonName] = jss::LedgerStateFix;
return jv;
}
} // namespace xrpl::test::jtx::ledgerStateFix

4
src/test/jtx/ledgerStateFix.h
View File

@@ -10,4 +10,8 @@ namespace xrpl::test::jtx::ledgerStateFix {
json::Value
nftPageLinks(jtx::Account const& acct, jtx::Account const& owner);
/** Repair sfExchangeRate on a book directory's first page. */
json::Value
bookExchangeRate(jtx::Account const& acct, uint256 const& bookDir);
} // namespace xrpl::test::jtx::ledgerStateFix

21
src/tests/libxrpl/protocol_autogen/transactions/LedgerStateFixTests.cpp
View File

@@ -31,6 +31,7 @@ TEST(TransactionsLedgerStateFixTests, BuilderSettersRoundTrip)
// Transaction-specific field values
auto const ledgerFixTypeValue = canonical_UINT16();
auto const ownerValue = canonical_ACCOUNT();
auto const bookDirectoryValue = canonical_UINT256();
LedgerStateFixBuilder builder{
accountValue,
@@ -41,6 +42,7 @@ TEST(TransactionsLedgerStateFixTests, BuilderSettersRoundTrip)
// Set optional fields
builder.setOwner(ownerValue);
builder.setBookDirectory(bookDirectoryValue);
auto tx = builder.build(publicKey, secretKey);
@@ -72,6 +74,14 @@ TEST(TransactionsLedgerStateFixTests, BuilderSettersRoundTrip)
EXPECT_TRUE(tx.hasOwner());
}
{
auto const& expected = bookDirectoryValue;
auto const actualOpt = tx.getBookDirectory();
ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBookDirectory should be present";
expectEqualField(expected, *actualOpt, "sfBookDirectory");
EXPECT_TRUE(tx.hasBookDirectory());
}
}
// 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper,
@@ -90,6 +100,7 @@ TEST(TransactionsLedgerStateFixTests, BuilderFromStTxRoundTrip)
// Transaction-specific field values
auto const ledgerFixTypeValue = canonical_UINT16();
auto const ownerValue = canonical_ACCOUNT();
auto const bookDirectoryValue = canonical_UINT256();
// Build an initial transaction
LedgerStateFixBuilder initialBuilder{
@@ -100,6 +111,7 @@ TEST(TransactionsLedgerStateFixTests, BuilderFromStTxRoundTrip)
};
initialBuilder.setOwner(ownerValue);
initialBuilder.setBookDirectory(bookDirectoryValue);
auto initialTx = initialBuilder.build(publicKey, secretKey);
@@ -131,6 +143,13 @@ TEST(TransactionsLedgerStateFixTests, BuilderFromStTxRoundTrip)
expectEqualField(expected, *actualOpt, "sfOwner");
}
{
auto const& expected = bookDirectoryValue;
auto const actualOpt = rebuiltTx.getBookDirectory();
ASSERT_TRUE(actualOpt.has_value()) << "Optional field sfBookDirectory should be present";
expectEqualField(expected, *actualOpt, "sfBookDirectory");
}
}
// 3) Verify wrapper throws when constructed from wrong transaction type.
@@ -190,6 +209,8 @@ TEST(TransactionsLedgerStateFixTests, OptionalFieldsReturnNullopt)
// Verify optional fields are not present
EXPECT_FALSE(tx.hasOwner());
EXPECT_FALSE(tx.getOwner().has_value());
EXPECT_FALSE(tx.hasBookDirectory());
EXPECT_FALSE(tx.getBookDirectory().has_value());
}
}