Merge pull request #7048 from PeterChen13579/add_defensive_checks

add defensive checks

src/libxrpl/tx/invariants/MPTInvariant.cpp

@@ -448,10 +448,10 @@ ValidConfidentialMPToken::visitEntry(
 
         bool const hasAnyHolder = hasHolderInbox || hasHolderSpending;
 
-        if (hasAnyHolder != hasIssuerBalance)
-        {
+        // 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;
@@ -592,6 +592,23 @@ ValidConfidentialMPToken::finalize(
                 return false;
             }
         }
+        else if (
+            std::ranges::find(confidentialMPTTxTypes, tx.getTxnType()) !=
+            confidentialMPTTxTypes.end())
+        {
+            // 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
+            // modify sfOutstandingAmount
+            if (checks.outstandingDelta != 0)
+            {
+                JLOG(j.fatal()) << "Invariant failed: OutstandingAmount changed "
+                                   "by confidential transaction that should not "
+                                   "modify it for MPT "
+                                << to_string(id);
+                return false;
+            }
+        }
 
         if (checks.badVersion)
         {

src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp

@@ -243,7 +243,7 @@ ConfidentialMPTSend::doApply()
     auto sleSenderMPToken = view().peek(keylet::mptoken(mptIssuanceID, account_));
     auto sleDestinationMPToken = view().peek(keylet::mptoken(mptIssuanceID, destination));
 
-    auto const sleDestAcct = view().peek(keylet::account(destination));
+    auto const sleDestAcct = view().read(keylet::account(destination));
 
     if (!sleSenderMPToken || !sleDestinationMPToken || !sleDestAcct)
         return tecINTERNAL;  // LCOV_EXCL_LINE

src/test/app/ConfidentialTransfer_test.cpp

@@ -2090,7 +2090,11 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 60}, {carol, 50, 20}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob, .payAmount = 100, .convertAmount = 60},
+             {.account = carol, .payAmount = 50, .convertAmount = 20}}};
         auto& mptAlice = confEnv.mpt;
 
         // bob sends 10 to carol
@@ -3029,7 +3033,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 1000, 60}, {carol, 1000, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob, .payAmount = 1000, .convertAmount = 60}, {carol, 1000, 50}}};
         auto& mptAlice = confEnv.mpt;
 
         {
@@ -3098,7 +3105,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
             // Register keys only (amt=0) for both parties — spending stays 0.
             Env env2{*this, features};
             Account const alice2("alice"), bob2("bob"), carol2("carol");
-            ConfidentialEnv zeroEnv{env2, alice2, {{bob2, 100, 0}, {carol2, 50, 0}}};
+            ConfidentialEnv zeroEnv{
+                env2,
+                alice2,
+                {{.account = bob2, .payAmount = 100, .convertAmount = 0}, {carol2, 50, 0}}};
             auto& mptAlice2 = zeroEnv.mpt;
 
             // Trying to send any amount with 0 spending balance must fail:
@@ -6263,7 +6273,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 50}}};
+        ConfidentialEnv confEnv{
+            env, alice, {{.account = bob, .payAmount = 100, .convertAmount = 50}}};
         auto& mptAlice = confEnv.mpt;
 
         auto const spendingBalance =
@@ -6322,7 +6333,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob");
-        ConfidentialEnv confEnv{env, alice, {{bob, 1000, 100}}};
+        ConfidentialEnv confEnv{
+            env, alice, {{.account = bob, .payAmount = 1000, .convertAmount = 100}}};
         auto& mptAlice = confEnv.mpt;
 
         auto const versionV = mptAlice.getMPTokenVersion(bob);
@@ -6394,7 +6406,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 50}}};
+        ConfidentialEnv confEnv{
+            env, alice, {{.account = bob, .payAmount = 100, .convertAmount = 50}}};
         auto& mptAlice = confEnv.mpt;
 
         // Prepare valid parameters for a ConvertBack of 10
@@ -6468,7 +6481,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 100}, {carol, 50, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob, .payAmount = 100, .convertAmount = 100}, {carol, 50, 50}}};
         auto& mptAlice = confEnv.mpt;
 
         // Bob sends 10 to carol.  The send amount (10) and Bob's remaining balance
@@ -6525,7 +6541,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob");
-        ConfidentialEnv confEnv{env, alice, {{bob, 10, 10}}};
+        ConfidentialEnv confEnv{
+            env, alice, {{.account = bob, .payAmount = 10, .convertAmount = 10}}};
         auto& mptAlice = confEnv.mpt;
 
         // Converting back 1 from 10 leaves remaining balance = 9 (non-negative).
@@ -6612,7 +6629,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         {
             Account const alice("alice"), bob("bob"), carol("carol");
             Env env{*this, features};
-            ConfidentialEnv confEnv{env, alice, {{bob, 100, 60}, {carol, 50, 30}}};
+            ConfidentialEnv confEnv{
+                env,
+                alice,
+                {{.account = bob, .payAmount = 100, .convertAmount = 60}, {carol, 50, 30}}};
             auto& mptAlice = confEnv.mpt;
 
             // sender's encrypted amount has an invalid coordinate
@@ -6688,7 +6708,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         {
             Account const alice("alice"), bob("bob"), carol("carol");
             Env env{*this, features};
-            ConfidentialEnv confEnv{env, alice, {{bob, 100, 60}, {carol, 50, 30}}};
+            ConfidentialEnv confEnv{
+                env,
+                alice,
+                {{.account = bob, .payAmount = 100, .convertAmount = 60}, {carol, 50, 30}}};
             auto& mptAlice = confEnv.mpt;
 
             Buffer badProof(ecSendProofLength);
@@ -6711,7 +6734,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         {
             Account const alice("alice"), bob("bob"), carol("carol");
             Env env{*this, features};
-            ConfidentialEnv confEnv{env, alice, {{bob, 100, 60}, {carol, 50, 30}}};
+            ConfidentialEnv confEnv{
+                env,
+                alice,
+                {{.account = bob, .payAmount = 100, .convertAmount = 60}, {carol, 50, 30}}};
             auto& mptAlice = confEnv.mpt;
 
             // getTrivialCiphertext() has both C1 and C2 as valid (but trivial)
@@ -6802,7 +6828,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 60}, {carol, 50, 30}}};
+        ConfidentialEnv confEnv{
+            env, alice, {{.account = bob, .payAmount = 100, .convertAmount = 60}, {carol, 50, 30}}};
         auto& mptAlice = confEnv.mpt;
 
         // The x-coordinate of the NIST P-256 generator point — a real,
@@ -6983,7 +7010,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 100, 100}, {carol, 50, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob, .payAmount = 100, .convertAmount = 100}, {carol, 50, 50}}};
         auto& mptAlice = confEnv.mpt;
 
         auto const bobSpendingBefore =
@@ -7372,7 +7402,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
         ConfidentialEnv confEnv{
-            env, alice, {{bob}, {carol, 1000, 50}}, tfMPTCanTransfer | tfMPTCanConfidentialAmount};
+            env,
+            alice,
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}},
+            tfMPTCanTransfer | tfMPTCanConfidentialAmount};
         auto& mptAlice = confEnv.mpt;
 
         // Set RequireDest on carol
@@ -8994,7 +9027,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         // Bob converts exactly 10 tokens, leaving honest remaining = 0.
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob, 1000, 10}, {carol, 1000, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob, .payAmount = 1000, .convertAmount = 10}, {carol, 1000, 50}}};
         auto& mptAlice = confEnv.mpt;
 
         uint64_t const sendAmount = 10;
@@ -9053,7 +9089,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob}, {carol, 1000, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}}};
         auto& mptAlice = confEnv.mpt;
 
         ConfidentialSendSetup setup(mptAlice, bob, carol, alice, 10);
@@ -9168,7 +9207,12 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol"), dan("dan");
-        ConfidentialEnv confEnv{env, alice, {{bob}, {carol, 1000, 50}, {dan, 1000, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob},
+             {.account = carol, .payAmount = 1000, .convertAmount = 50},
+             {dan, 1000, 50}}};
         auto& mptAlice = confEnv.mpt;
 
         uint64_t const sendAmount = 10;
@@ -9279,7 +9323,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
-        ConfidentialEnv confEnv{env, alice, {{bob}, {carol, 1000, 50}}};
+        ConfidentialEnv confEnv{
+            env,
+            alice,
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}}};
         auto& mptAlice = confEnv.mpt;
 
         ConfidentialSendSetup setup(mptAlice, bob, carol, alice, 10);
@@ -9472,7 +9519,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         ConfidentialEnv confEnv{
             env,
             alice,
-            {{bob}, {carol, 1000, 50}},
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}},
             tfMPTCanLock | tfMPTCanConfidentialAmount | tfMPTCanTransfer | tfMPTCanClawback};
         auto& mptAlice = confEnv.mpt;
 
@@ -9604,7 +9651,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
         ConfidentialEnv confEnv{
-            env, alice, {{bob}, {carol, 1000, 50}}, tfMPTCanTransfer | tfMPTCanConfidentialAmount};
+            env,
+            alice,
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}},
+            tfMPTCanTransfer | tfMPTCanConfidentialAmount};
         auto& mptAlice = confEnv.mpt;
 
         uint64_t const sendAmount = 10;
@@ -9702,7 +9752,10 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         Env env{*this, features};
         Account const alice("alice"), bob("bob"), carol("carol");
         ConfidentialEnv confEnv{
-            env, alice, {{bob}, {carol, 1000, 50}}, tfMPTCanTransfer | tfMPTCanConfidentialAmount};
+            env,
+            alice,
+            {{.account = bob}, {.account = carol, .payAmount = 1000, .convertAmount = 50}},
+            tfMPTCanTransfer | tfMPTCanConfidentialAmount};
         auto& mptAlice = confEnv.mpt;
 
         uint64_t const sendAmount = 10;

src/test/app/Invariants_test.cpp

@@ -4306,8 +4306,10 @@ class Invariants_test : public beast::unit_test::suite
                 auto sleToken = ac.view().peek(keylet::mptoken(mptID, A2.id()));
                 if (!sleToken)
                     return false;
-                // Inject fields correctly, but the Issuance was built without the privacy flag.
+                // Inject all three encrypted fields consistently (inbox+spending+issuer must be
+                // in sync or badConsistency fires first and masks requiresPrivacyFlag).
                 sleToken->setFieldVL(sfConfidentialBalanceInbox, Blob{0x00});
+                sleToken->setFieldVL(sfConfidentialBalanceSpending, Blob{0x00});
                 sleToken->setFieldVL(sfIssuerEncryptedBalance, Blob{0x00});
                 ac.view().update(sleToken);
                 return true;
@@ -4354,6 +4356,25 @@ class Invariants_test : public beast::unit_test::suite
             {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
             precloseConfidential);
 
+        // Send/MergeInbox must not change OutstandingAmount (coaDelta == 0)
+        doInvariantCheck(
+            {"Invariant failed: OutstandingAmount changed "
+             "by confidential transaction that should not "
+             "modify it for MPT"},
+            [&mptID](Account const& A1, Account const& A2, ApplyContext& ac) {
+                auto sleIssuance = ac.view().peek(keylet::mptIssuance(mptID));
+                if (!sleIssuance)
+                    return false;
+                sleIssuance->setFieldU64(
+                    sfOutstandingAmount, sleIssuance->getFieldU64(sfOutstandingAmount) + 1);
+                ac.view().update(sleIssuance);
+                return true;
+            },
+            XRPAmount{},
+            STTx{ttCONFIDENTIAL_MPT_SEND, [](STObject&) {}},
+            {tecINVARIANT_FAILED, tecINVARIANT_FAILED},
+            precloseConfidential);
+
         // badVersion
         doInvariantCheck(
             {"MPToken sfConfidentialBalanceVersion not updated when sfConfidentialBalanceSpending "