From 71983a59b6d2073aee8c0f4f73ad7596f650679e Mon Sep 17 00:00:00 2001 From: Peter Chen <34582813+PeterChen13579@users.noreply.github.com> Date: Wed, 13 May 2026 09:57:18 -0400 Subject: [PATCH] add public MPT txn after Clearing Confidential Flag (#7113) --- include/xrpl/tx/invariants/MPTInvariant.h | 6 +- src/libxrpl/tx/invariants/MPTInvariant.cpp | 40 +++++-- src/test/app/ConfidentialTransfer_test.cpp | 125 ++++++++++++++++++++- src/test/app/Invariants_test.cpp | 18 +++ 4 files changed, 176 insertions(+), 13 deletions(-) diff --git a/include/xrpl/tx/invariants/MPTInvariant.h b/include/xrpl/tx/invariants/MPTInvariant.h index 492f102800..5607bb72a0 100644 --- a/include/xrpl/tx/invariants/MPTInvariant.h +++ b/include/xrpl/tx/invariants/MPTInvariant.h @@ -65,8 +65,8 @@ public: * Cannot delete if sfIssuerEncryptedBalance exists * Cannot delete if sfConfidentialBalanceInbox and sfConfidentialBalanceSpending exist * - Privacy flag consistency: - * MPToken can only have encrypted fields if lsfMPTCanConfidentialAmount is set on - * issuance. + * MPToken confidential balance fields can only be created or changed if + * lsfMPTCanConfidentialAmount is set on the issuance. * - Encrypted field existence consistency: * If sfConfidentialBalanceSpending/sfConfidentialBalanceInbox exists, then * sfIssuerEncryptedBalance must also exist (and vice versa). @@ -86,7 +86,7 @@ class ValidConfidentialMPToken bool deletedWithEncrypted = false; bool badConsistency = false; bool badCOA = false; - bool requiresPrivacyFlag = false; + bool changesConfidentialFields = false; bool badVersion = false; }; std::map changes_; diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 8d94238252..1754a30120 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/src/libxrpl/tx/invariants/MPTInvariant.cpp @@ -450,17 +450,29 @@ ValidConfidentialMPToken::visitEntry( bool const hasHolderInbox = after->isFieldPresent(sfConfidentialBalanceInbox); bool const hasHolderSpending = after->isFieldPresent(sfConfidentialBalanceSpending); - bool const hasAnyHolder = hasHolderInbox || hasHolderSpending; - // sfIssuerEncryptedBalance, sfConfidentialBalanceInbox, and sfConfidentialBalanceSpending // must all exist or not exist same time. if (hasHolderInbox != hasHolderSpending || hasHolderInbox != hasIssuerBalance) changes_[id].badConsistency = true; - // Privacy flag consistency - bool const hasEncrypted = hasAnyHolder || hasIssuerBalance; - if (hasEncrypted) - changes_[id].requiresPrivacyFlag = true; + auto const confidentialBalanceFieldChanged = [&before, &after](auto const& field) { + auto const afterValue = (*after)[~field]; + if (!afterValue) + return false; + + if (!before || before->getType() != ltMPTOKEN) + return true; // LCOV_EXCL_LINE + + return (*before)[~field] != afterValue; + }; + + if (confidentialBalanceFieldChanged(sfConfidentialBalanceInbox) || + confidentialBalanceFieldChanged(sfConfidentialBalanceSpending) || + confidentialBalanceFieldChanged(sfIssuerEncryptedBalance) || + confidentialBalanceFieldChanged(sfAuditorEncryptedBalance)) + { + changes_[id].changesConfidentialFields = true; + } } if (before && before->getType() == ltMPTOKEN_ISSUANCE) @@ -563,8 +575,10 @@ ValidConfidentialMPToken::finalize( return false; } - // Privacy flag consistency - if (checks.requiresPrivacyFlag) + // Confidential balance fields may remain on a holder MPToken after all + // confidential balances have returned to zero. Only creating or + // changing those fields requires the issuance privacy flag. + if (checks.changesConfidentialFields) { if (!issuance->isFlag(lsfMPTCanConfidentialAmount)) { @@ -600,6 +614,16 @@ ValidConfidentialMPToken::finalize( std::ranges::find(kCONFIDENTIAL_MPT_TX_TYPES, tx.getTxnType()) != kCONFIDENTIAL_MPT_TX_TYPES.end()) { + // Confidential Txns should not modify public MPTAmount balance + // if Confidential Amount Delta is 0 + if (checks.mptAmountDelta != 0) + { + JLOG(j.fatal()) << "Invariant failed: MPTAmount changed by confidential " + "transaction that should not modify this field." + << to_string(id); + return false; + } + // Among confidential MPT transactions, only ConfidentialMPTSend and // ConfidentialMPTMergeInbox leave coaDelta unmodified. Therefore, if a confidential MPT // transaction reaches here, it must be one of these two types, neither of which will diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index c9112d3b9f..bc3c5aaa2d 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -5325,6 +5325,126 @@ class ConfidentialTransfer_test : public beast::unit_test::Suite } } + void + testPublicTransfersAfterClearingConfidentialFlag(FeatureBitset features) + { + testcase("Public transfers after clearing Confidential Flag"); + using namespace test::jtx; + + Account const alice("alice"); + Account const bob("bob"); + Account const carol("carol"); + + // After clearing the confidential flag, all four public MPT operations + // must succeed regardless of which confidential path left encrypted-zero + // fields on bob's MPToken. + auto runPublicPayments = [&](MPTTester& mpt) { + mpt.pay(bob, carol, 10); + mpt.pay(carol, bob, 5); + mpt.pay(alice, bob, 1); + mpt.pay(carol, alice, 5); + }; + + auto drainAndDeleteBobMPToken = [&](Env& env, MPTTester& mpt) { + auto const bobBalance = mpt.getBalance(bob); + BEAST_EXPECT(bobBalance > 0); + + mpt.pay(bob, alice, bobBalance); + BEAST_EXPECT(mpt.getBalance(bob) == 0); + + mpt.authorize({.account = bob, .flags = tfMPTUnauthorize}); + BEAST_EXPECT(!env.le(keylet::mptoken(mpt.issuanceID(), bob.id()))); + }; + + // Alice pays Bob 100 public, Bob converts 50 confidential + // Bob converts 50 back to public, and make sure can receive public payments + { + Env env{*this, features}; + ConfidentialEnv ct{ + env, + alice, + {{.account = bob, .payAmount = 100, .convertAmount = 50}}, + tfMPTCanTransfer | tfMPTCanConfidentialAmount}; + + env.fund(XRP(1'000), carol); + ct.mpt.authorize({.account = carol}); + ct.mpt.pay(alice, carol, 50); + + ct.mpt.convertBack({.account = bob, .amt = 50}); + ct.mpt.set({ + .account = alice, + .mutableFlags = tmfMPTClearCanConfidentialAmount, + }); + + runPublicPayments(ct.mpt); + drainAndDeleteBobMPToken(env, ct.mpt); + } + + // Same path as above but with Auditor + { + Env env{*this, features}; + Account const auditor("auditor"); + MPTTester mptAlice(env, alice, {.holders = {bob, carol}, .auditor = auditor}); + + mptAlice.create({ + .ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanConfidentialAmount, + }); + + mptAlice.authorize({.account = bob}); + mptAlice.authorize({.account = carol}); + mptAlice.pay(alice, bob, 100); + mptAlice.pay(alice, carol, 50); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(auditor); + mptAlice.set( + {.account = alice, + .issuerPubKey = mptAlice.getPubKey(alice), + .auditorPubKey = mptAlice.getPubKey(auditor)}); + + mptAlice.convert({ + .account = bob, + .amt = 50, + .holderPubKey = mptAlice.getPubKey(bob), + }); + mptAlice.mergeInbox({.account = bob}); + mptAlice.convertBack({.account = bob, .amt = 50}); + mptAlice.set({ + .account = alice, + .mutableFlags = tmfMPTClearCanConfidentialAmount, + }); + + runPublicPayments(mptAlice); + drainAndDeleteBobMPToken(env, mptAlice); + } + + // Confidential clawback leaves encrypted-zero fields; + // the public balance remaining after the clawback must stay usable. + { + Env env{*this, features}; + ConfidentialEnv ct{ + env, + alice, + {{.account = bob, .payAmount = 100, .convertAmount = 50}}, + tfMPTCanTransfer | tfMPTCanClawback | tfMPTCanConfidentialAmount}; + + env.fund(XRP(1'000), carol); + ct.mpt.authorize({.account = carol}); + ct.mpt.pay(alice, carol, 50); + + ct.mpt.confidentialClaw({.account = alice, .holder = bob, .amt = 50}); + ct.mpt.set({ + .account = alice, + .mutableFlags = tmfMPTClearCanConfidentialAmount, + }); + + runPublicPayments(ct.mpt); + drainAndDeleteBobMPToken(env, ct.mpt); + } + } + void testMutatePrivacy(FeatureBitset features) { @@ -10114,10 +10234,11 @@ class ConfidentialTransfer_test : public beast::unit_test::Suite testIdentityElementRejection(features); testSendWrongIssuerPublicKey(features); - // Replay Tests - testMutatePrivacy(features); + // public and private txns + testPublicTransfersAfterClearingConfidentialFlag(features); // Replay tests + testMutatePrivacy(features); testProofContextBinding(features); testProofCiphertextBinding(features); testProofVersionMismatch(features); diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 6462fca7e6..aa776f5166 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -4515,6 +4515,24 @@ class Invariants_test : public beast::unit_test::Suite {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, precloseConfidential); + // Send/MergeInbox and zero-COA-delta confidential transactions must not + // change public holder MPTAmount. + doInvariantCheck( + {"Invariant failed: MPTAmount changed by confidential " + "transaction that should not modify this field."}, + [&mptID](Account const& a1, Account const& a2, ApplyContext& ac) { + auto sleToken = ac.view().peek(keylet::mptoken(mptID, a2.id())); + if (!sleToken) + return false; + sleToken->setFieldU64(sfMPTAmount, sleToken->getFieldU64(sfMPTAmount) + 1); + ac.view().update(sleToken); + return true; + }, + XRPAmount{}, + STTx{ttCONFIDENTIAL_MPT_SEND, [](STObject&) {}}, + {tecINVARIANT_FAILED, tecINVARIANT_FAILED}, + precloseConfidential); + // badVersion doInvariantCheck( {"MPToken sfConfidentialBalanceVersion not updated when sfConfidentialBalanceSpending "