diff --git a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h index 3784a32840..654ed3e009 100644 --- a/include/xrpl/tx/invariants/PermissionedDEXInvariant.h +++ b/include/xrpl/tx/invariants/PermissionedDEXInvariant.h @@ -10,9 +10,10 @@ namespace xrpl { class ValidPermissionedDEX { - bool regularOffers_ = false; - 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) + bool regularOffersOld_ = false; // pre-fixCleanup3_2_0: also flags deleted offers + 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_; public: diff --git a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp index b480b90a31..282df85302 100644 --- a/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp +++ b/src/libxrpl/tx/invariants/PermissionedDEXInvariant.cpp @@ -20,7 +20,7 @@ namespace xrpl { void ValidPermissionedDEX::visitEntry( - bool, + bool isDelete, std::shared_ptr const& before, std::shared_ptr const& after) { @@ -38,7 +38,9 @@ ValidPermissionedDEX::visitEntry( } else { - regularOffers_ = true; + regularOffersOld_ = true; + if (!isDelete) + regularOffers_ = true; } // pre-fixCleanup3_1_3: hybrid offer missing domain, missing @@ -100,7 +102,9 @@ ValidPermissionedDEX::finalize( } } - if (regularOffers_) + bool const hasRegularOffers = + view.rules().enabled(fixCleanup3_2_0) ? regularOffers_ : regularOffersOld_; + if (hasRegularOffers) { JLOG(j.fatal()) << "Invariant failed: domain transaction" " affected regular offers"; diff --git a/src/test/app/PermissionedDEX_test.cpp b/src/test/app/PermissionedDEX_test.cpp index be377c0c1d..3a95fb2f92 100644 --- a/src/test/app/PermissionedDEX_test.cpp +++ b/src/test/app/PermissionedDEX_test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep #include @@ -34,6 +35,7 @@ #include #include #include +#include #include #include @@ -1455,6 +1457,54 @@ class PermissionedDEX_test : public beast::unit_test::Suite } } + void + testCancelRegularOfferWithDomainCreate(FeatureBitset features) + { + bool const fixEnabled = features[fixCleanup3_2_0]; + + testcase << "Cancel regular offer via domain OfferCreate" + << (fixEnabled ? " (fixCleanup3_2_0 enabled)" : " (fixCleanup3_2_0 disabled)"); + + // An OfferCreate with sfDomainID and sfOfferSequence pointing to + // the user's own non-domain offer should atomically cancel the + // regular offer and place the new domain offer. + // + // Pre-fixCleanup3_2_0: ValidPermissionedDEX flagged the deleted + // regular offer, so the transaction failed with tecINVARIANT_FAILED. + // Post-fixCleanup3_2_0: the invariant ignores deletions and the + // transaction succeeds. + + Env env(*this, features); + auto const& [gw, domainOwner, alice, bob, carol, USD, domainID, credType] = + PermissionedDEX(env); + + auto const regularSeq = env.seq(bob); + env(offer(bob, XRP(10), USD(10))); + env.close(); + BEAST_EXPECT(checkOffer(env, bob, regularSeq, XRP(10), USD(10), 0, false)); + + auto const domainSeq = env.seq(bob); + if (fixEnabled) + { + env(offer(bob, XRP(20), USD(20)), + Domain(domainID), + Json(jss::OfferSequence, regularSeq)); + env.close(); + BEAST_EXPECT(!offerExists(env, bob, regularSeq)); + BEAST_EXPECT(checkOffer(env, bob, domainSeq, XRP(20), USD(20), 0, true)); + } + else + { + env(offer(bob, XRP(20), USD(20)), + Domain(domainID), + Json(jss::OfferSequence, regularSeq), + Ter(tecINVARIANT_FAILED)); + env.close(); + BEAST_EXPECT(offerExists(env, bob, regularSeq)); + BEAST_EXPECT(!offerExists(env, bob, domainSeq)); + } + } + public: void run() override @@ -1478,6 +1528,11 @@ public: testHybridOfferDirectories(all); testHybridMalformedOffer(all); testHybridMalformedOffer(all - fixCleanup3_1_3); + + // Cancelling a regular offer in a domain OfferCreate is allowed + // only after fixCleanup3_2_0. + testCancelRegularOfferWithDomainCreate(all); + testCancelRegularOfferWithDomainCreate(all - fixCleanup3_2_0); } };