diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index d2de6663994..0532f991869 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -85,7 +85,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -134,7 +133,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -173,6 +171,23 @@ class ConfidentialTransfer_test : public beast::unit_test::suite testcase("Convert preflight"); using namespace test::jtx; + // Alice (issuer) tries to convert her own tokens - should fail + { + Env env{*this, features}; + Account const alice("alice"); + MPTTester mptAlice(env, alice); + + mptAlice.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mptAlice.generateKeyPair(alice); + + mptAlice.convert( + {.account = alice, + .amt = 10, + .holderPubKey = mptAlice.getPubKey(alice), + .err = temMALFORMED}); + } + { Env env{*this, features - featureConfidentialTransfer}; Account const alice("alice"); @@ -180,9 +195,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {bob}}); mptAlice.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanLock}); + {.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock}); mptAlice.authorize({.account = bob}); mptAlice.pay(alice, bob, 100); @@ -209,9 +222,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {bob}}); mptAlice.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanLock}); + {.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock}); mptAlice.authorize({.account = bob}); mptAlice.pay(alice, bob, 100); @@ -233,6 +244,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .blindingFactor = Buffer(10), .err = temMALFORMED}); + // Holder encrypted amount is empty (length 0) mptAlice.convert( {.account = bob, .amt = 10, @@ -240,6 +252,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .holderEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT}); + // Issuer encrypted amount is empty (length 0) mptAlice.convert( {.account = bob, .amt = 10, @@ -247,12 +260,30 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .issuerEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT}); + // Auditor encrypted amount has invalid length (must be 66 bytes) + mptAlice.convert( + {.account = bob, + .amt = 10, + .holderPubKey = mptAlice.getPubKey(bob), + .auditorEncryptedAmt = Buffer(10), + .err = temBAD_CIPHERTEXT}); + + // Auditor encrypted amount has correct length but invalid data + mptAlice.convert( + {.account = bob, + .amt = 10, + .holderPubKey = mptAlice.getPubKey(bob), + .auditorEncryptedAmt = getBadCiphertext(), + .err = temBAD_CIPHERTEXT}); + + // Amount exceeds maximum allowed MPT amount mptAlice.convert( {.account = bob, .amt = maxMPTokenAmount + 1, .holderPubKey = mptAlice.getPubKey(bob), .err = temBAD_AMOUNT}); + // Holder encrypted amount has correct length but invalid data mptAlice.convert( {.account = bob, .amt = 1, @@ -260,6 +291,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .holderEncryptedAmt = getBadCiphertext(), .err = temBAD_CIPHERTEXT}); + // Issuer encrypted amount has correct length but invalid data (not + // a valid EC point) mptAlice.convert( {.account = bob, .amt = 1, @@ -267,7 +300,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .issuerEncryptedAmt = getBadCiphertext(), .err = temBAD_CIPHERTEXT}); - // invalid pub key + // Holder public key is invalid (empty buffer) mptAlice.convert( {.account = bob, .amt = 10, @@ -285,7 +318,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -330,7 +362,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -403,10 +434,38 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.generateKeyPair(alice); mptAlice.generateKeyPair(bob); + // Pub key is invalid mptAlice.set( {.account = alice, .issuerPubKey = Buffer{}, .err = temMALFORMED}); + + // Auditor key is invalid length + mptAlice.set( + {.account = alice, + .issuerPubKey = mptAlice.getPubKey(alice), + .auditorPubKey = Buffer(10), + .err = temMALFORMED}); + + // Cannot set auditor key without issuer key + mptAlice.set( + {.account = alice, + .auditorPubKey = mptAlice.getPubKey(alice), + .err = temMALFORMED}); + + // Cannot set Holder and issuer Keys in the same transaction + mptAlice.set( + {.account = alice, + .holder = bob, + .issuerPubKey = mptAlice.getPubKey(alice), + .err = temMALFORMED}); + + // Cannot set Holder and auditor Keys in the same transaction + mptAlice.set( + {.account = alice, + .holder = bob, + .auditorPubKey = mptAlice.getPubKey(alice), + .err = temMALFORMED}); } // issuance has disabled confidential transfer @@ -684,6 +743,104 @@ class ConfidentialTransfer_test : public beast::unit_test::suite }); } + // cannot convert if auditor key is set, but auditor amount is not + // provided + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const auditor("auditor"); + MPTTester mptAlice( + env, alice, {.holders = {bob}, .auditor = auditor}); + + mptAlice.create( + {.ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(auditor); + + mptAlice.set( + {.account = alice, + .issuerPubKey = mptAlice.getPubKey(alice), + .auditorPubKey = mptAlice.getPubKey(auditor)}); + + // no auditor encrypted amt provided + mptAlice.convert( + {.account = bob, + .amt = 10, + .fillAuditorEncryptedAmt = false, + .holderPubKey = mptAlice.getPubKey(bob), + .err = tecNO_PERMISSION}); + } + + // cannot convert if tx include auditor ciphertext, but does not have + // auditing enabled + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + MPTTester mptAlice(env, alice, {.holders = {bob}}); + + mptAlice.create( + {.ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + + // there is no auditor key set + mptAlice.set( + {.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + + mptAlice.convert( + {.account = bob, + .amt = 10, + .holderPubKey = mptAlice.getPubKey(bob), + .auditorEncryptedAmt = getTrivialCiphertext(), + .err = tecNO_PERMISSION}); + } + + // Auditor key set successfully, auditor ciphertext mathematically + // correct, but contains invalid data (mismatching amount). + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const auditor("auditor"); + MPTTester mptAlice( + env, alice, {.holders = {bob}, .auditor = auditor}); + + mptAlice.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + 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 = 10, + .holderPubKey = mptAlice.getPubKey(bob), + .auditorEncryptedAmt = getTrivialCiphertext(), + .err = tecBAD_PROOF}); + } + // invalid proof when registering holder pub key { Env env{*this, features}; @@ -693,7 +850,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -802,7 +958,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -825,9 +980,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {bob}}); mptAlice.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanLock}); + {.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock}); mptAlice.authorize({.account = bob}); mptAlice.pay(alice, bob, 100); @@ -847,7 +1000,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.generateKeyPair(alice); @@ -867,7 +1019,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1561,7 +1712,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1604,7 +1754,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1676,7 +1825,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1719,7 +1867,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1764,9 +1911,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {bob}}); mptAlice.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanLock}); + {.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock}); mptAlice.authorize({.account = bob}); mptAlice.pay(alice, bob, 100); @@ -1786,7 +1931,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1850,6 +1994,18 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 30, .issuerEncryptedAmt = getBadCiphertext(), .err = temBAD_CIPHERTEXT}); + + mptAlice.convertBack( + {.account = bob, + .amt = 30, + .auditorEncryptedAmt = Buffer(10), + .err = temBAD_CIPHERTEXT}); + + mptAlice.convertBack( + {.account = bob, + .amt = 30, + .auditorEncryptedAmt = getBadCiphertext(), + .err = temBAD_CIPHERTEXT}); } } @@ -1868,7 +2024,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1892,9 +2047,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite MPTTester mptAlice(env, alice, {.holders = {bob}}); mptAlice.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanLock}); + {.ownerCount = 1, .flags = tfMPTCanTransfer | tfMPTCanLock}); mptAlice.authorize({.account = bob}); mptAlice.pay(alice, bob, 100); @@ -1915,7 +2068,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.generateKeyPair(alice); @@ -1937,7 +2089,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -1964,7 +2115,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); mptAlice.authorize({.account = bob}); @@ -2009,7 +2159,6 @@ class ConfidentialTransfer_test : public beast::unit_test::suite mptAlice.create( {.ownerCount = 1, - .holderCount = 0, .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTRequireAuth | tfMPTCanPrivacy}); @@ -2058,6 +2207,88 @@ class ConfidentialTransfer_test : public beast::unit_test::suite .amt = 10, }); } + + // Alice has NOT set an auditor key, but Bob provides + // auditorEncryptedAmt + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + MPTTester mptAlice(env, alice, {.holders = {bob}}); + + mptAlice.create( + {.ownerCount = 1, + .flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.set( + {.account = alice, .issuerPubKey = mptAlice.getPubKey(alice)}); + + // Bob converts funds to confidential so he has something to convert + // back + mptAlice.convert( + {.account = bob, + .amt = 50, + .holderPubKey = mptAlice.getPubKey(bob)}); + mptAlice.mergeInbox({.account = bob}); + + mptAlice.convertBack( + {.account = bob, + .amt = 10, + // Provide valid ciphertext to pass preflight + .auditorEncryptedAmt = getTrivialCiphertext(), + .err = tecNO_PERMISSION}); + } + + // we set the auditor key, but convertBack omits auditorEncryptedAmt + { + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + Account const auditor("auditor"); + MPTTester mptAlice( + env, alice, {.holders = {bob}, .auditor = auditor}); + + mptAlice.create( + {.flags = tfMPTCanTransfer | tfMPTCanLock | tfMPTCanPrivacy}); + mptAlice.authorize({.account = bob}); + mptAlice.pay(alice, bob, 100); + + mptAlice.generateKeyPair(alice); + mptAlice.generateKeyPair(bob); + mptAlice.generateKeyPair(auditor); + mptAlice.set( + {.account = alice, + .issuerPubKey = mptAlice.getPubKey(alice), + .auditorPubKey = mptAlice.getPubKey(auditor)}); + + // Convert funds so Bob has a balance + mptAlice.convert({ + .account = bob, + .amt = 50, + .holderPubKey = mptAlice.getPubKey(bob), + }); + mptAlice.mergeInbox({.account = bob}); + + // ConvertBack WITHOUT auditorEncryptedAmt + mptAlice.convertBack({ + .account = bob, + .amt = 10, + .fillAuditorEncryptedAmt = false, + .err = tecNO_PERMISSION, + }); + + // ConvertBack where auditor ciphertext mathematically + // correct, but contains invalid data (mismatching amount). + mptAlice.convertBack( + {.account = bob, + .amt = 10, + .auditorEncryptedAmt = getTrivialCiphertext(), + .err = tecBAD_PROOF}); + } } void diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 9dc90414619..38a877b5241 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -482,7 +482,7 @@ MPTTester::set(MPTSet const& arg) return forObject([&](SLEP const& sle) -> bool { if (sle) { - if (!auditor_) + if (!auditor_.has_value()) Throw( "MPTTester::set: auditor is not set"); @@ -1078,8 +1078,8 @@ MPTTester::fillConversionCiphertexts( // Handle Auditor if (arg.auditorEncryptedAmt) auditorCiphertext = *arg.auditorEncryptedAmt; - else if (auditor()) - auditorCiphertext = encryptAmount(*auditor(), *arg.amt, blindingFactor); + else if (auditor_.has_value() && *arg.fillAuditorEncryptedAmt) + auditorCiphertext = encryptAmount(*auditor_, *arg.amt, blindingFactor); // Update auditor JSON only if ciphertext exists if (auditorCiphertext) @@ -1288,7 +1288,7 @@ MPTTester::send(MPTConfidentialSend const& arg) std::optional auditorAmt; if (arg.auditorEncryptedAmt) auditorAmt = arg.auditorEncryptedAmt; - else if (auditor_) + else if (auditor_.has_value()) auditorAmt = encryptAmount(*auditor_, *arg.amt, blindingFactor); jv[sfSenderEncryptedAmount] = strHex(senderAmt); diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index da1bf9e2a7b..e60aef50ded 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -172,7 +172,7 @@ struct MPTConvert std::optional id = std::nullopt; std::optional amt = std::nullopt; std::optional proof = std::nullopt; - + std::optional fillAuditorEncryptedAmt = true; // indicates whether to autofill schnorr proof. // default : auto generate proof if holderPubKey is present. // true: force proof generation. @@ -232,6 +232,8 @@ struct MPTConvertBack std::optional holderEncryptedAmt = std::nullopt; std::optional issuerEncryptedAmt = std::nullopt; std::optional auditorEncryptedAmt = std::nullopt; + std::optional fillAuditorEncryptedAmt = true; + // not an txn param, only used for autofilling std::optional blindingFactor = std::nullopt; std::optional pedersenCommitment = std::nullopt; std::optional ownerCount = std::nullopt; @@ -373,12 +375,6 @@ class MPTTester return issuer_; } - std::optional const& - auditor() const - { - return auditor_; - } - Account const& holder(std::string const& h) const; diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index e33a59a26f8..8f9e93c11db 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -323,7 +323,7 @@ MPTokenIssuanceSet::preclaim(PreclaimContext const& ctx) if (ctx.tx.isFieldPresent(sfAuditorElGamalPublicKey) && sleMptIssuance->isFieldPresent(sfAuditorElGamalPublicKey)) { - return tecNO_PERMISSION; + return tecNO_PERMISSION; // LCOV_EXCL_LINE } if (ctx.tx.isFieldPresent(sfIssuerElGamalPublicKey) &&