fix: Extend locked-MPToken unauthorize check to fixCleanup3_4_0 (#8004)

This commit is contained in:
Gregory Tsipenyuk
2026-08-17 12:52:20 +00:00
committed by GitHub
parent 5337d028a2
commit c49789086a
2 changed files with 69 additions and 16 deletions

View File

@@ -37,6 +37,7 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
{
auto const accountID = ctx.tx[sfAccount];
auto const holderID = ctx.tx[~sfHolder];
auto const sleMptIssuance = ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
// if non-issuer account submits this tx, then they are trying either:
// 1. Unauthorize/delete MPToken
@@ -51,9 +52,8 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
// There is an edge case where all holders have zero balance, issuance
// is legally destroyed, then outstanding MPT(s) are deleted afterwards.
// Thus, there is no need to check for the existence of the issuance if
// the MPT is being deleted with a zero balance. Check for unauthorize
// before fetching the MPTIssuance object.
// Thus, the unauthorize/delete path below does not require the issuance
// to exist when the MPT is being deleted with a zero balance.
// if holder wants to delete/unauthorize a mpt
if (ctx.tx.isFlag(tfMPTUnauthorize))
@@ -63,8 +63,6 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
if ((*sleMpt)[sfMPTAmount] != 0)
{
auto const sleMptIssuance =
ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tefINTERNAL; // LCOV_EXCL_LINE
@@ -73,21 +71,24 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
if ((*sleMpt)[~sfLockedAmount].value_or(0) != 0)
{
auto const sleMptIssuance =
ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tefINTERNAL; // LCOV_EXCL_LINE
return tecHAS_OBLIGATIONS;
}
if (ctx.view.rules().enabled(featureSingleAssetVault) && sleMpt->isFlag(lsfMPTLocked))
if (ctx.view.rules().enabled(fixCleanup3_4_0))
{
if (sleMptIssuance && sleMpt->isFlag(lsfMPTLocked))
return tecNO_PERMISSION;
}
else if (
ctx.view.rules().enabled(featureSingleAssetVault) && sleMpt->isFlag(lsfMPTLocked))
{
return tecNO_PERMISSION;
}
if (ctx.view.rules().enabled(featureConfidentialTransfer))
{
auto const sleMptIssuance =
ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
// if there still existing encrypted balances of MPT in
// circulation
if (sleMptIssuance &&
@@ -106,9 +107,6 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
}
// Now test when the holder wants to hold/create/authorize a new MPT
auto const sleMptIssuance =
ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tecOBJECT_NOT_FOUND;
@@ -126,7 +124,6 @@ MPTokenAuthorize::preclaim(PreclaimContext const& ctx)
if (!sleHolder)
return tecNO_DST;
auto const sleMptIssuance = ctx.view.read(keylet::mptokenIssuance(ctx.tx[sfMPTokenIssuanceID]));
if (!sleMptIssuance)
return tecOBJECT_NOT_FOUND;

View File

@@ -789,7 +789,7 @@ class MPToken_test : public beast::unit_test::Suite
// locks up bob's mptoken again
mptAlice.set({.account = alice, .holder = bob, .flags = tfMPTLock});
if (!features[featureSingleAssetVault])
if (!features[featureSingleAssetVault] && !features[fixCleanup3_4_0])
{
// Delete bob's mptoken even though it is locked
mptAlice.authorize({.account = bob, .flags = tfMPTUnauthorize});
@@ -7657,6 +7657,56 @@ class MPToken_test : public beast::unit_test::Suite
0, tecNO_PERMISSION, tecNO_PERMISSION, tecNO_PERMISSION, tecNO_PERMISSION);
}
void
testLockedMPTokenDestroyedIssuance(FeatureBitset features)
{
testcase("Locked MPToken with destroyed issuance");
using namespace test::jtx;
Account const alice("alice"); // issuer
Account const bob("bob"); // holder
Env env{*this, features};
env.fund(XRP(1'000), alice, bob);
env.close();
MPTTester mptAlice(
{.env = env, .issuer = alice, .holders = {bob}, .flags = kMptDexFlags | tfMPTCanLock});
// alice locks bob's mptoken individually
mptAlice.set({.account = alice, .holder = bob, .flags = tfMPTLock});
// alice destroys her issuance. This succeeds: MPTokenIssuanceDestroy
// only requires that the issuance has no outstanding balance; it does
// not require that all holder MPTokens have been deleted first.
mptAlice.destroy({.ownerCount = 0});
if (!features[featureSingleAssetVault] || features[fixCleanup3_4_0])
{
// pre SAV or post Cleanup340 amendment: bob deletes the dangling locked MPToken
mptAlice.authorize({.account = bob, .holderCount = 0, .flags = tfMPTUnauthorize});
BEAST_EXPECT(ownerCount(env, bob) == 0);
}
else
{
// bob cannot delete his locked MPToken, even though the issuance
// no longer exists.
mptAlice.authorize(
{.account = bob, .flags = tfMPTUnauthorize, .err = tecNO_PERMISSION});
// and the lock can never be cleared, because unlocking
// requires the (destroyed) issuance
mptAlice.set(
{.account = alice,
.holder = bob,
.flags = tfMPTUnlock,
.err = tecOBJECT_NOT_FOUND});
// the dangling locked MPToken survives
BEAST_EXPECT(env.current()->exists(keylet::mptoken(mptAlice.issuanceID(), bob.id())));
BEAST_EXPECT(ownerCount(env, bob) == 1);
}
}
public:
void
run() override
@@ -7703,7 +7753,9 @@ public:
testSetValidation(all - featurePermissionedDomains);
testSetValidation(all);
testSetEnabled(all - featureSingleAssetVault - fixCleanup3_4_0);
testSetEnabled(all - featureSingleAssetVault);
testSetEnabled(all - fixCleanup3_4_0);
testSetEnabled(all);
// MPT clawback
@@ -7770,6 +7822,10 @@ public:
// Fixes
testFixDoubleOwnerCount(all);
testLockedMPTokenDestroyedIssuance(all);
testLockedMPTokenDestroyedIssuance(all - fixCleanup3_4_0);
testLockedMPTokenDestroyedIssuance(all - featureSingleAssetVault);
testLockedMPTokenDestroyedIssuance(all - featureSingleAssetVault - fixCleanup3_4_0);
}
};