diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index dc03cf7c59..6d809f7ab5 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -1085,7 +1085,7 @@ TRANSACTION(ttLOAN_PAY, 84, LoanPay, # include #endif TRANSACTION(ttCONFIDENTIAL_MPT_CONVERT, 85, ConfidentialMPTConvert, - Delegation::Delegable, + Delegation::NotDelegable, featureConfidentialTransfer, NoPriv, ({ diff --git a/include/xrpl/protocol_autogen/transactions/ConfidentialMPTConvert.h b/include/xrpl/protocol_autogen/transactions/ConfidentialMPTConvert.h index dec7f733c9..284b7f9e70 100644 --- a/include/xrpl/protocol_autogen/transactions/ConfidentialMPTConvert.h +++ b/include/xrpl/protocol_autogen/transactions/ConfidentialMPTConvert.h @@ -19,7 +19,7 @@ class ConfidentialMPTConvertBuilder; * @brief Transaction: ConfidentialMPTConvert * * Type: ttCONFIDENTIAL_MPT_CONVERT (85) - * Delegable: Delegation::Delegable + * Delegable: Delegation::NotDelegable * Amendment: featureConfidentialTransfer * Privileges: NoPriv * diff --git a/src/test/app/ConfidentialTransferExtended_test.cpp b/src/test/app/ConfidentialTransferExtended_test.cpp index 953325a6e9..fe5e0b3064 100644 --- a/src/test/app/ConfidentialTransferExtended_test.cpp +++ b/src/test/app/ConfidentialTransferExtended_test.cpp @@ -1653,30 +1653,35 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase mptAlice.generateKeyPair(carol); mptAlice.set({.issuerPubKey = mptAlice.getPubKey(alice)}); - // Bob delegates Convert, MergeInbox to dave. - env(delegate::set(bob, dave, {"ConfidentialMPTConvert", "ConfidentialMPTMergeInbox"})); + // ConfidentialMPTConvert is not delegable: attempting to grant it as a + // delegated permission is rejected at preflight of DelegateSet. + env(delegate::set(bob, dave, {"ConfidentialMPTConvert"}), Ter(temMALFORMED)); env.close(); - // Carol has no permission from bob to convert on his behalf. + // Bob delegates MergeInbox to dave. + env(delegate::set(bob, dave, {"ConfidentialMPTMergeInbox"})); + env.close(); + + // A Convert carrying a Delegate is rejected at preflight because the + // transaction type is not delegable at all. mptAlice.convert({ .account = bob, .amt = 10, .holderPubKey = mptAlice.getPubKey(bob), - .delegate = carol, - .err = terNO_DELEGATE_PERMISSION, + .delegate = dave, + .err = temINVALID, }); - // Dave executes Convert on behalf of bob, registering bob's key. + // Bob converts, registering bob's key. mptAlice.convert({ .account = bob, .amt = 100, .holderPubKey = mptAlice.getPubKey(bob), - .delegate = dave, }); env.require(MptBalance(mptAlice, bob, 100)); - // Dave executes Convert again on behalf of bob (no key registration). - mptAlice.convert({.account = bob, .amt = 50, .delegate = dave}); + // Bob converts again (no key registration). + mptAlice.convert({.account = bob, .amt = 50}); // Dave executes MergeInbox on behalf of bob. mptAlice.mergeInbox({.account = bob, .delegate = dave}); @@ -1698,10 +1703,7 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase .err = terNO_DELEGATE_PERMISSION}); // Bob delegates ConfidentialMPTSend to dave. - env(delegate::set( - bob, - dave, - {"ConfidentialMPTConvert", "ConfidentialMPTMergeInbox", "ConfidentialMPTSend"})); + env(delegate::set(bob, dave, {"ConfidentialMPTMergeInbox", "ConfidentialMPTSend"})); env.close(); // Dave executes Send on behalf of bob. @@ -1716,10 +1718,7 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase env(delegate::set( bob, dave, - {"ConfidentialMPTConvert", - "ConfidentialMPTMergeInbox", - "ConfidentialMPTSend", - "ConfidentialMPTConvertBack"})); + {"ConfidentialMPTMergeInbox", "ConfidentialMPTSend", "ConfidentialMPTConvertBack"})); env.close(); // Dave executes ConvertBack on behalf of bob. @@ -1766,16 +1765,15 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase // Creating the Delegate SLE consumes one owner reserve slot for bob. auto const bobOwnersBefore = ownerCount(env, bob); - env(delegate::set(bob, carol, {"ConfidentialMPTConvert", "ConfidentialMPTMergeInbox"})); + env(delegate::set(bob, carol, {"ConfidentialMPTMergeInbox"})); env.close(); env.require(Owners(bob, bobOwnersBefore + 1)); - // Carol converts and merge inbox on behalf of bob. + // Bob converts; carol merges inbox on behalf of bob. mptAlice.convert({ .account = bob, .amt = 50, .holderPubKey = mptAlice.getPubKey(bob), - .delegate = carol, }); mptAlice.mergeInbox({.account = bob, .delegate = carol}); @@ -1784,16 +1782,18 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase env.close(); env.require(Owners(bob, bobOwnersBefore)); - // Carol can no longer convert on behalf of bob. - mptAlice.convert({ + // Bob converts again to populate a fresh inbox. + mptAlice.convert({.account = bob, .amt = 30}); + + // Carol can no longer merge inbox on behalf of bob. + mptAlice.mergeInbox({ .account = bob, - .amt = 30, .delegate = carol, .err = terNO_DELEGATE_PERMISSION, }); - // Bob can still convert by himself. - mptAlice.convert({.account = bob, .amt = 30}); + // Bob can still merge his inbox. + mptAlice.mergeInbox({.account = bob}); } // Verifies that a delegated confidential transfer works correctly when an @@ -1833,16 +1833,15 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase .auditorPubKey = mptAlice.getPubKey(auditor), }); - // Bob delegates Convert and Send permissions to dave. - env(delegate::set(bob, dave, {"ConfidentialMPTSend", "ConfidentialMPTConvert"})); + // Bob delegates Send permission to dave (Convert is not delegable). + env(delegate::set(bob, dave, {"ConfidentialMPTSend"})); env.close(); - // Dave converts on behalf of bob. + // Bob converts. mptAlice.convert({ .account = bob, .amt = 50, .holderPubKey = mptAlice.getPubKey(bob), - .delegate = dave, }); mptAlice.mergeInbox({.account = bob}); @@ -2229,7 +2228,7 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase mpt.pay(alice, frank, 40); mpt.generateKeyPair(frank); - env(delegate::set(bob, dave, {"ConfidentialMPTConvert", "ConfidentialMPTConvertBack"})); + env(delegate::set(bob, dave, {"ConfidentialMPTConvertBack"})); env(delegate::set(carol, erin, {"ConfidentialMPTSend"})); env(delegate::set(bob, erin, {"ConfidentialMPTMergeInbox"})); env.close(); @@ -2238,15 +2237,15 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase auto const bobSeq = env.seq(bob); auto const carolSeq = env.seq(carol); auto const frankSeq = env.seq(frank); - auto const batchFee = batch::calcConfidentialBatchFee(env, 3, 6); + auto const batchFee = batch::calcConfidentialBatchFee(env, 4, 6); - // Dave submits the batch. Bob's convert and convertback use Dave as Delegate; + // Dave submits the batch. Bob's convertback uses Dave as Delegate; + // Convert is not delegable, so Bob signs his own convert inner tx. // Carol's send and Bob's mergeInbox use Erin as Delegate. Frank's // convert and mergeInbox are non-delegated. auto jv1 = mpt.convertBackJV({.account = bob, .amt = 30}, bobSeq); jv1[jss::Delegate] = dave.human(); - auto jv2 = mpt.convertJV({.account = bob, .amt = 20}, bobSeq + 1); - jv2[jss::Delegate] = dave.human(); + auto const jv2 = mpt.convertJV({.account = bob, .amt = 20}, bobSeq + 1); auto jv3 = mpt.sendJV({.account = carol, .dest = bob, .amt = 15}, carolSeq); jv3[jss::Delegate] = erin.human(); auto const jv4 = mpt.convertJV( @@ -2262,7 +2261,7 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase batch::Inner(jv4, frankSeq), batch::Inner(jv5, frankSeq + 1), batch::Inner(jv6, bobSeq + 2), - batch::Sig(erin, frank), + batch::Sig(erin, frank, bob), Ter(tesSUCCESS)); env.close(); @@ -2283,7 +2282,10 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase BEAST_EXPECT(mpt.getIssuanceConfidentialBalance() == 175); } - // Test invalid scenarios for delegation with tickets. + // Test invalid scenarios for delegation with tickets. ConfidentialMPTConvert + // is not delegable, so ConfidentialMPTConvertBack (which is delegable and + // whose ZK proof also binds to the transaction/ticket sequence) is used as + // the delegated operation. Carol acts as bob's delegate throughout. void testInvalidDelegationWithTickets(FeatureBitset features) { @@ -2309,33 +2311,52 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase mptAlice.generateKeyPair(bob); mptAlice.set({.issuerPubKey = mptAlice.getPubKey(alice)}); - // Bob grants carol permissions. - env(delegate::set(bob, carol, {"ConfidentialMPTConvert"})); + // Give bob a confidential spending balance to convert back from. + mptAlice.convert({.account = bob, .amt = 100, .holderPubKey = mptAlice.getPubKey(bob)}); + mptAlice.mergeInbox({.account = bob}); + + // Bob delegates ConfidentialMPTConvertBack to carol. + env(delegate::set(bob, carol, {"ConfidentialMPTConvertBack"})); env.close(); uint64_t const amt = 10; - auto const bf = generateBlindingFactor(); - auto const holderCt = mptAlice.encryptAmount(bob, amt, bf); - auto const issuerCt = mptAlice.encryptAmount(alice, amt, bf); + + // Every case below fails, so bob's spending balance and version never + // change; capture the crypto material needed to build proofs once. + auto const spendingBalance = requireOptional( + mptAlice.getDecryptedBalance(bob, MPTTester::holderEncryptedSpending), + "Missing spending balance."); + auto const encSpending = requireOptional( + mptAlice.getEncryptedBalance(bob, MPTTester::holderEncryptedSpending), + "Missing encrypted spending balance."); + auto const version = mptAlice.getMPTokenVersion(bob); + auto const pcBf = generateBlindingFactor(); + auto const pc = mptAlice.getPedersenCommitment(spendingBalance, pcBf); + + // Build a ConvertBack proof bound to a given sequence. + auto proofForSeq = [&](std::uint32_t seq) { + return mptAlice.getConvertBackProof( + bob, + amt, + getConvertBackContextHash(bob, mptAlice.issuanceID(), seq, version), + { + .pedersenCommitment = pc, + .amt = spendingBalance, + .encryptedAmt = encSpending, + .blindingFactor = pcBf, + }); + }; // Invalid: proof built with wrong ticket sequence (ticketSeq + 1). { auto const ticketSeq = env.seq(bob) + 1; env(ticket::create(bob, 1)); - auto const badCtxHash = - getConvertContextHash(bob, mptAlice.issuanceID(), ticketSeq + 1); - auto const badProof = requireOptional( - mptAlice.getSchnorrProof(bob, badCtxHash), "Missing Schnorr Proof."); - - mptAlice.convert({ + mptAlice.convertBack({ .account = bob, .amt = amt, - .proof = strHex(badProof), - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, + .proof = proofForSeq(ticketSeq + 1), + .pedersenCommitment = pc, .delegate = carol, .ticketSeq = ticketSeq, .err = tecBAD_PROOF, @@ -2346,18 +2367,12 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase { auto const ticketSeq = env.seq(bob) + 1; env(ticket::create(bob, 1)); - auto const badCtxHash = getConvertContextHash(bob, mptAlice.issuanceID(), env.seq(bob)); - auto const badProof = requireOptional( - mptAlice.getSchnorrProof(bob, badCtxHash), "Missing Schnorr Proof."); - mptAlice.convert({ + mptAlice.convertBack({ .account = bob, .amt = amt, - .proof = strHex(badProof), - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, + .proof = proofForSeq(env.seq(bob)), + .pedersenCommitment = pc, .delegate = carol, .ticketSeq = ticketSeq, .err = tecBAD_PROOF, @@ -2366,13 +2381,9 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase // Invalid: ticket sequence is far in the future and hasn't been created yet. { - mptAlice.convert({ + mptAlice.convertBack({ .account = bob, .amt = amt, - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, .delegate = carol, .ticketSeq = env.seq(bob) + 100, .err = terPRE_TICKET, @@ -2381,13 +2392,9 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase // Invalid: ticket sequence is in the past but was never created. { - mptAlice.convert({ + mptAlice.convertBack({ .account = bob, .amt = amt, - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, .delegate = carol, .ticketSeq = 1, .err = tefNO_TICKET, @@ -2395,17 +2402,14 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase } // Invalid: the delegated account, carol, creates a ticket and uses it. + // The ticket must belong to the delegator (bob), not the delegate. { auto const carolTicketSeq = env.seq(carol) + 1; env(ticket::create(carol, 1)); - mptAlice.convert({ + mptAlice.convertBack({ .account = bob, .amt = amt, - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, .delegate = carol, .ticketSeq = carolTicketSeq, .err = tefNO_TICKET, @@ -2418,25 +2422,31 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase auto const ticketSeq = env.seq(bob) + 1; env(ticket::create(bob, 1)); - // Build proof using ticketSeq. - auto const ctxHashForTicket = - getConvertContextHash(bob, mptAlice.issuanceID(), ticketSeq); - auto const proof = requireOptional( - mptAlice.getSchnorrProof(bob, ctxHashForTicket), "Missing Schnorr Proof."); - - // Submit without ticket. - mptAlice.convert({ + // Submit without a ticket; proof is bound to ticketSeq. + mptAlice.convertBack({ .account = bob, .amt = amt, - .proof = strHex(proof), - .holderPubKey = mptAlice.getPubKey(bob), - .holderEncryptedAmt = holderCt, - .issuerEncryptedAmt = issuerCt, - .blindingFactor = bf, + .proof = proofForSeq(ticketSeq), + .pedersenCommitment = pc, .delegate = carol, .err = tecBAD_PROOF, }); } + + // Valid: carol converts back on bob's behalf using a ticket owned by bob, + // with a proof correctly bound to that ticket sequence. bob's spending + // balance drops from 100 to 90. + { + auto const ticketSeq = env.seq(bob) + 1; + env(ticket::create(bob, 1)); + + mptAlice.convertBack({ + .account = bob, + .amt = amt, + .delegate = carol, + .ticketSeq = ticketSeq, + }); + } } // Verifies that delegation works correctly when the delegating account uses @@ -2471,19 +2481,16 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase mptAlice.generateKeyPair(carol); mptAlice.set({.issuerPubKey = mptAlice.getPubKey(alice)}); - // Bob grants dave permissions. + // Bob grants dave permissions (Convert is not delegable). env(delegate::set( bob, dave, - {"ConfidentialMPTConvert", - "ConfidentialMPTMergeInbox", - "ConfidentialMPTSend", - "ConfidentialMPTConvertBack"})); + {"ConfidentialMPTMergeInbox", "ConfidentialMPTSend", "ConfidentialMPTConvertBack"})); // Alice grants dave permission to clawback on her behalf. env(delegate::set(alice, dave, {"ConfidentialMPTClawback"})); env.close(); - // Dave executes Convert on behalf of bob using ticket. + // Bob converts using a ticket. auto ticketSeq = env.seq(bob) + 1; env(ticket::create(bob, 1)); BEAST_EXPECT(env.seq(bob) != ticketSeq); @@ -2491,7 +2498,6 @@ class ConfidentialTransferExtended_test : public ConfidentialTransferTestBase .account = bob, .amt = 100, .holderPubKey = mptAlice.getPubKey(bob), - .delegate = dave, .ticketSeq = ticketSeq, }); env.require(MptBalance(mptAlice, bob, 100)); diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index a3fa2ac0b5..d61220a1ef 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -2750,7 +2750,7 @@ class Delegate_test : public beast::unit_test::Suite // DO NOT modify expectedDelegableCount unless all scenarios, including // edge cases, have been fully tested and verified. // ==================================================================== - std::size_t const expectedDelegableCount = 57; + std::size_t const expectedDelegableCount = 56; BEAST_EXPECTS( delegableCount == expectedDelegableCount,