Skip to content

fix: Correct hybrid offer deletion on credential expiry #6843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kassaking7 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

fix: Correct hybrid offer deletion on credential expiry #6843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00fadb5
fix: correct hybrid offer deletion on credential expiry
Kassaking7 Apr 9, 2026
3853a28
amendment gate
Kassaking7 Apr 13, 2026
02db687
new amendment and test fix
Kassaking7 Apr 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/xrpl/protocol/detail/features.macro
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// Add new amendments to the top of this list.
// Keep it sorted in reverse chronological order.

XRPL_FIX (PermissionDEX1_1, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(MPTokensV2, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (Security3_1_3, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
Expand Down
9 changes: 8 additions & 1 deletion src/libxrpl/tx/paths/OfferStream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,14 @@ TOfferStreamBase<TIn, TOut>::step()
continue;
}

if (entry->isFieldPresent(sfDomainID) &&
// Without fixPermissionDEX1_1: validate domain membership for any book
// (original behaviour).
// With fixPermissionDEX1_1: only validate when walking a domain book.
// Hybrid offers carry sfDomainID but also participate in the open
// book; expiry of the owner's domain credential should not evict
// the offer from the open book.
if ((!view_.rules().enabled(fixPermissionDEX1_1) || book_.domain.has_value()) &&
entry->isFieldPresent(sfDomainID) &&
!permissioned_dex::offerInDomain(
view_, entry->key(), entry->getFieldH256(sfDomainID), j_))
{
Expand Down
186 changes: 159 additions & 27 deletions src/test/app/PermissionedDEX_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1076,8 +1076,8 @@ class PermissionedDEX_test : public beast::unit_test::suite
testcase("Hybrid invalid offer");

// bob has a hybrid offer and then he is removed from domain.
// in this case, the hybrid offer will be considered as unfunded even in
// a regular payment
// Domain payments are blocked once the credential is revoked, but
// the hybrid offer remains crossable in the open book.
Env env(*this, features);
auto const& [gw, domainOwner, alice, bob, carol, USD, domainID, credType] =
PermissionedDEX(env);
Expand All @@ -1090,8 +1090,9 @@ class PermissionedDEX_test : public beast::unit_test::suite
env(credentials::deleteCred(domainOwner, bob, domainOwner, credType));
env.close();

// bob's hybrid offer is unfunded and can not be consumed in a domain
// payment
// bob's hybrid offer cannot be consumed via a domain payment —
// the domain book evicts it, but since the payment fails the sandbox
// is discarded and the offer remains intact.
env(pay(alice, carol, USD(5)),
path(~USD),
sendmax(XRP(5)),
Expand All @@ -1100,33 +1101,70 @@ class PermissionedDEX_test : public beast::unit_test::suite
env.close();
BEAST_EXPECT(checkOffer(env, bob, hybridOfferSeq, XRP(50), USD(50), lsfHybrid, true));

// bob's unfunded hybrid offer can't be consumed even with a regular
// payment
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)), ter(tecPATH_PARTIAL));
env.close();
BEAST_EXPECT(checkOffer(env, bob, hybridOfferSeq, XRP(50), USD(50), lsfHybrid, true));
if (features[fixPermissionDEX1_1])
{
// With the fix: hybrid offer CAN still be consumed via a regular
// open-book payment even though the domain credential was revoked.
auto const carolBalBefore = env.balance(carol, USD);
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)));
env.close();
BEAST_EXPECT(env.balance(carol, USD) - carolBalBefore == USD(5));
BEAST_EXPECT(checkOffer(env, bob, hybridOfferSeq, XRP(45), USD(45), lsfHybrid, true));

// create a regular offer
auto const regularOfferSeq{env.seq(bob)};
env(offer(bob, XRP(10), USD(10)));
env.close();
BEAST_EXPECT(offerExists(env, bob, regularOfferSeq));
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(10), USD(10)));
// create a regular offer alongside the hybrid one
auto const regularOfferSeq{env.seq(bob)};
env(offer(bob, XRP(10), USD(10)));
env.close();
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(10), USD(10)));

auto const sleHybridOffer = env.le(keylet::offer(bob.id(), hybridOfferSeq));
BEAST_EXPECT(sleHybridOffer);
auto const openDir =
sleHybridOffer->getFieldArray(sfAdditionalBooks)[0].getFieldH256(sfBookDirectory);
BEAST_EXPECT(checkDirectorySize(env, openDir, 2));
auto const sleHybridOffer = env.le(keylet::offer(bob.id(), hybridOfferSeq));
BEAST_EXPECT(sleHybridOffer);
auto const openDir =
sleHybridOffer->getFieldArray(sfAdditionalBooks)[0].getFieldH256(sfBookDirectory);
// both offers are in the open book directory
BEAST_EXPECT(checkDirectorySize(env, openDir, 2));

// this normal payment should consume the regular offer and remove the
// unfunded hybrid offer
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)));
env.close();
// A regular payment crosses the hybrid offer first (FIFO, older
// offer), then stops — the regular offer is untouched.
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)));
env.close();

BEAST_EXPECT(checkOffer(env, bob, hybridOfferSeq, XRP(40), USD(40), lsfHybrid, true));
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(10), USD(10)));
BEAST_EXPECT(checkDirectorySize(env, openDir, 2));
}
else
{
// Without the fix (original behaviour): the open-book traversal
// also runs the offerInDomain eviction check, so the hybrid offer
// is treated as unfunded and the regular payment fails.
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)), ter(tecPATH_PARTIAL));
env.close();
BEAST_EXPECT(checkOffer(env, bob, hybridOfferSeq, XRP(50), USD(50), lsfHybrid, true));

// create a regular offer
auto const regularOfferSeq{env.seq(bob)};
env(offer(bob, XRP(10), USD(10)));
env.close();
BEAST_EXPECT(offerExists(env, bob, regularOfferSeq));
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(10), USD(10)));

auto const sleHybridOffer = env.le(keylet::offer(bob.id(), hybridOfferSeq));
BEAST_EXPECT(sleHybridOffer);
auto const openDir =
sleHybridOffer->getFieldArray(sfAdditionalBooks)[0].getFieldH256(sfBookDirectory);
BEAST_EXPECT(checkDirectorySize(env, openDir, 2));

BEAST_EXPECT(!offerExists(env, bob, hybridOfferSeq));
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(5), USD(5)));
BEAST_EXPECT(checkDirectorySize(env, openDir, 1));
// This payment crosses the regular offer and permanently evicts the
// hybrid offer from the open book (since the payment succeeds, the
// sandbox — including the hybrid eviction — is committed).
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)));
env.close();

BEAST_EXPECT(!offerExists(env, bob, hybridOfferSeq));
BEAST_EXPECT(checkOffer(env, bob, regularOfferSeq, XRP(5), USD(5)));
BEAST_EXPECT(checkDirectorySize(env, openDir, 1));
}
}

void
Expand Down Expand Up @@ -1287,6 +1325,96 @@ class PermissionedDEX_test : public beast::unit_test::suite
}
}

// Regression: hybrid offer is NOT evicted from the open book when the
// owner's domain credential expires (fixPermissionDEX1_1).
// A domain payment after expiry should fail (domain book evicts the offer),
// but the open book remains usable.
void
testHybridOpenBookAfterCredentialExpiry(FeatureBitset features)
{
testcase("Hybrid open book after credential expiry");

Env env(*this, features);
auto const& [gw, domainOwner, alice, bob, carol, USD, domainID, credType] =
PermissionedDEX(env);

Account const devin("devin");
env.fund(XRP(100000), devin);
env.close();
env.trust(USD(1000), devin);
env.close();
env(pay(gw, devin, USD(100)));
env.close();

// Give devin a credential that expires far enough in the future to
// survive the setup env.close() calls.
auto jv = credentials::create(devin, domainOwner, credType);
uint32_t const t = env.current()->header().parentCloseTime.time_since_epoch().count();
jv[sfExpiration.jsonName] = t + 100;
env(jv);
env.close();
env(credentials::accept(devin, domainOwner, credType));
env.close();

// Devin creates a hybrid offer: sell USD(10) for XRP(10).
// The offer is placed in both the domain book and the open book.
auto const hybridOfferSeq{env.seq(devin)};
env(offer(devin, XRP(10), USD(10)), txflags(tfHybrid), domain(domainID));
env.close();

BEAST_EXPECT(checkOffer(env, devin, hybridOfferSeq, XRP(10), USD(10), lsfHybrid, true));

// A non-domain open-book payment partially crosses the offer while
// devin's credential is still valid.
auto const carolBalBefore = env.balance(carol, USD);
env(pay(alice, carol, USD(5)), path(~USD), sendmax(XRP(5)));
env.close();
BEAST_EXPECT(env.balance(carol, USD) - carolBalBefore == USD(5));
BEAST_EXPECT(checkOffer(env, devin, hybridOfferSeq, XRP(5), USD(5), lsfHybrid, true));

// Advance time so that devin's credential expires.
env.close(std::chrono::seconds(100));

// Confirm devin can no longer create domain offers.
env(offer(devin, XRP(1), USD(1)), domain(domainID), ter(tecNO_PERMISSION));
env.close();

// The hybrid offer must still exist in the open book after expiry.
BEAST_EXPECT(offerExists(env, devin, hybridOfferSeq));

// A non-domain open-book payment must cross (not evict) the
// remaining portion of devin's hybrid offer.
auto const carolBalBefore2 = env.balance(carol, USD);
env(pay(alice, carol, USD(2)), path(~USD), sendmax(XRP(2)));
env.close();

// Carol received USD — the offer was crossed, not evicted.
BEAST_EXPECT(env.balance(carol, USD) - carolBalBefore2 == USD(2));
// Offer still exists with 3 USD / 3 XRP remaining.
BEAST_EXPECT(checkOffer(env, devin, hybridOfferSeq, XRP(3), USD(3), lsfHybrid, true));

// A domain payment now fails because the domain book evicts devin's
// offer (his credential has expired). The eviction is rolled back with
// the failed sandbox, so the offer is NOT permanently removed.
env(pay(alice, carol, USD(1)),
path(~USD),
sendmax(XRP(1)),
domain(domainID),
ter(tecPATH_PARTIAL));
env.close();

// Offer still intact in the open book — domain payment did not
// permanently delete it.
BEAST_EXPECT(checkOffer(env, devin, hybridOfferSeq, XRP(3), USD(3), lsfHybrid, true));

// The open book can still fully consume the remaining portion.
auto const carolBalBefore3 = env.balance(carol, USD);
env(pay(alice, carol, USD(3)), path(~USD), sendmax(XRP(3)));
env.close();
BEAST_EXPECT(env.balance(carol, USD) - carolBalBefore3 == USD(3));
BEAST_EXPECT(!offerExists(env, devin, hybridOfferSeq));
}

void
testHybridOfferDirectories(FeatureBitset features)
{
Expand Down Expand Up @@ -1391,7 +1519,11 @@ class PermissionedDEX_test : public beast::unit_test::suite
// Test hybrid offers
testHybridOfferCreate(all);
testHybridBookStep(all);
// testHybridInvalidOffer: run once without the fix (old behaviour) and
// once with the fix (new behaviour).
testHybridInvalidOffer(all - fixPermissionDEX1_1);
testHybridInvalidOffer(all);
testHybridOpenBookAfterCredentialExpiry(all);
testHybridOfferDirectories(all);
}
};
Expand Down
Loading