From 9afa1cf4d15665e5597ef69d91917baa69c79bce Mon Sep 17 00:00:00 2001 From: Kassaking7 <96991820+Kassaking7@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:40:21 -0400 Subject: [PATCH] fix: Update PermissionedDEX invariant domain tracking for valid offer replacement (#7387) Co-authored-by: Bart --- .../tx/invariants/PermissionedDEXInvariant.h | 3 +- .../invariants/PermissionedDEXInvariant.cpp | 14 ++++- src/test/app/PermissionedDEX_test.cpp | 57 +++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h index 2763b80a94..ae0d573385 100644 --- a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h +++ b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h @@ -17,7 +17,8 @@ class ValidPermissionedDEX bool regularOffers_ = false; // post-fixCleanup3_2_0: excludes deleted offers bool badHybridsOld_ = false; // pre-fixCleanup3_1_3: missing field/domain or size > 1 bool badHybrids_ = false; // post-fixCleanup3_1_3: also catches size == 0 (size != 1) - hash_set domains_; + hash_set domainsOld_; // pre-fixCleanup3_4_0: also flags deleted domains + hash_set domains_; // post-fixCleanup3_4_0: excludes deleted domains public: void diff --git a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp index 1014642b36..44f623f284 100644 --- a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include #include @@ -19,17 +20,23 @@ namespace xrpl { void ValidPermissionedDEX::visitEntry(bool isDelete, SLE::const_ref before, SLE::const_ref after) { + auto trackDomain = [this, isDelete](uint256 const& domain) { + domainsOld_.insert(domain); + if (!isDelete) + domains_.insert(domain); + }; + if (after && after->getType() == ltDIR_NODE) { if (after->isFieldPresent(sfDomainID)) - domains_.insert(after->getFieldH256(sfDomainID)); + trackDomain(after->getFieldH256(sfDomainID)); } if (after && after->getType() == ltOFFER) { if (after->isFieldPresent(sfDomainID)) { - domains_.insert(after->getFieldH256(sfDomainID)); + trackDomain(after->getFieldH256(sfDomainID)); } else { @@ -87,7 +94,8 @@ ValidPermissionedDEX::finalize( // for both payment and offercreate, there shouldn't be another domain // that's different from the domain specified - for (auto const& d : domains_) + auto const& domains = view.rules().enabled(fixCleanup3_4_0) ? domains_ : domainsOld_; + for (auto const& d : domains) { if (d != domain) { diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index 998b7b1c7f..67cb7602a0 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -2001,6 +2001,61 @@ class PermissionedDEX_test : public beast::unit_test::Suite } } + void + testReplaceDomainOfferWithOtherDomainOffer(FeatureBitset features) + { + bool const fixEnabled = features[fixCleanup3_4_0]; + + testcase << "Replace domain offer via OfferCreate" + << (fixEnabled ? " (fixCleanup3_4_0 enabled)" : " (fixCleanup3_4_0 disabled)"); + + Env env(*this, features); + auto const& [gw, domainOwner, alice, bob, carol, USD, domainA, credType] = + PermissionedDEX(env); + + Account const domainOwnerB("permdex-domainOwnerB"); + auto const domainB = + setupDomain(env, {alice, bob, carol, gw}, domainOwnerB, "permdex-other-domain"); + BEAST_EXPECT(domainA != domainB); + + auto const oldSeq = env.seq(alice); + env(offer(alice, USD(100), XRP(1)), Domain(domainA)); + env.close(); + + BEAST_EXPECT(checkOffer(env, alice, oldSeq, USD(100), XRP(1), 0, true)); + auto const oldOffer = env.le(keylet::offer(alice.id(), oldSeq)); + if (!BEAST_EXPECT(oldOffer)) + return; + BEAST_EXPECT(oldOffer->getFieldH256(sfDomainID) == domainA); + + auto const newSeq = env.seq(alice); + // The invariant should reject mixing active Permissioned DEX domains, + // not a domain that is only touched because its offer is being deleted. + if (fixEnabled) + { + env(offer(alice, USD(100), XRP(2)), Domain(domainB), Json(jss::OfferSequence, oldSeq)); + env.close(); + + BEAST_EXPECT(!offerExists(env, alice, oldSeq)); + BEAST_EXPECT(checkOffer(env, alice, newSeq, USD(100), XRP(2), 0, true)); + auto const newOffer = env.le(keylet::offer(alice.id(), newSeq)); + if (!BEAST_EXPECT(newOffer)) + return; + BEAST_EXPECT(newOffer->getFieldH256(sfDomainID) == domainB); + } + else + { + env(offer(alice, USD(100), XRP(2)), + Domain(domainB), + Json(jss::OfferSequence, oldSeq), + Ter(tecINVARIANT_FAILED)); + env.close(); + + BEAST_EXPECT(checkOffer(env, alice, oldSeq, USD(100), XRP(1), 0, true)); + BEAST_EXPECT(!offerExists(env, alice, newSeq)); + } + } + public: void run() override @@ -2038,6 +2093,8 @@ public: // only after fixCleanup3_2_0. testCancelRegularOfferWithDomainCreate(all); testCancelRegularOfferWithDomainCreate(all - fixCleanup3_2_0); + testReplaceDomainOfferWithOtherDomainOffer(all); + testReplaceDomainOfferWithOtherDomainOffer(all - fixCleanup3_4_0); } };