diff --git a/src/libxrpl/tx/invariants/MPTInvariant.cpp b/src/libxrpl/tx/invariants/MPTInvariant.cpp index 229f5572c0..184fa9fbf6 100644 --- a/src/libxrpl/tx/invariants/MPTInvariant.cpp +++ b/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) { diff --git a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp b/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp index c9b9409452..0577d5ab30 100644 --- a/src/libxrpl/tx/transactors/token/ConfidentialMPTSend.cpp +++ b/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 diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index a3218c1455..8dcf2372b0 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/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; diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 6060fc8950..744d8773a3 100644 --- a/src/test/app/Invariants_test.cpp +++ b/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 "