diff --git a/src/test/app/ConfidentialTransfer_test.cpp b/src/test/app/ConfidentialTransfer_test.cpp index db3697eea8..9d3e4b28bf 100644 --- a/src/test/app/ConfidentialTransfer_test.cpp +++ b/src/test/app/ConfidentialTransfer_test.cpp @@ -1360,6 +1360,56 @@ class ConfidentialTransfer_test : public beast::unit_test::suite // todo: test with convert back and delete } + void + testConvertBack(FeatureBitset features) + { + testcase("Convert back"); + using namespace test::jtx; + Env env{*this, features}; + Account const alice("alice"); + Account const bob("bob"); + MPTTester mptAlice(env, alice, {.holders = {bob}}); + + mptAlice.create( + {.ownerCount = 1, + .holderCount = 0, + .flags = tfMPTCanTransfer | tfMPTCanLock}); + + mptAlice.authorize({.account = bob}); + env.close(); + mptAlice.pay(alice, bob, 100); + env.close(); + + mptAlice.generateKeyPair(alice); + + mptAlice.set({.account = alice, .pubKey = mptAlice.getPubKey(alice)}); + + mptAlice.generateKeyPair(bob); + + mptAlice.convert({ + .account = bob, + .amt = 40, + .proof = "123", + .holderPubKey = mptAlice.getPubKey(bob), + }); + + mptAlice.mergeInbox({ + .account = bob, + }); + + mptAlice.convertBack({ + .account = bob, + .amt = 30, + .proof = "123", + }); + + // mptAlice.convertBack({ + // .account = bob, + // .amt = 10, + // .proof = "123", + // }); + } + void testWithFeats(FeatureBitset features) { @@ -1379,6 +1429,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite testSendPreclaim(features); testDelete(features); + + testConvertBack(features); } public: diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 1d0fc3f220..212df2829e 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -980,6 +980,91 @@ MPTTester::getIssuanceOutstandingBalance() const return (*sle)[sfOutstandingAmount]; } +void +MPTTester::convertBack(MPTConvertBack const& arg) +{ + Json::Value jv; + if (arg.account) + jv[sfAccount] = arg.account->human(); + else + Throw("Account not specified"); + + jv[jss::TransactionType] = jss::ConfidentialConvertBack; + if (arg.id) + jv[sfMPTokenIssuanceID] = to_string(*arg.id); + else + { + if (!id_) + Throw("MPT has not been created"); + jv[sfMPTokenIssuanceID] = to_string(*id_); + } + + if (arg.amt) + jv[sfMPTAmount.jsonName] = std::to_string(*arg.amt); + if (arg.holderEncryptedAmt) + jv[sfHolderEncryptedAmount.jsonName] = strHex(*arg.holderEncryptedAmt); + else + jv[sfHolderEncryptedAmount.jsonName] = + strHex(encryptAmount(*arg.account, *arg.amt)); + + if (arg.issuerEncryptedAmt) + jv[sfIssuerEncryptedAmount.jsonName] = strHex(*arg.issuerEncryptedAmt); + else + jv[sfIssuerEncryptedAmount.jsonName] = + strHex(encryptAmount(issuer_, *arg.amt)); + + if (arg.proof) + jv[sfZKProof.jsonName] = *arg.proof; + + auto const holderAmt = getBalance(*arg.account); + auto const prevConfidentialOutstanding = getIssuanceConfidentialBalance(); + + uint64_t prevInboxBalance = + getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_INBOX); + uint64_t prevSpendingBalance = + getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_SPENDING); + uint64_t prevIssuerBalance = + getDecryptedBalance(*arg.account, ISSUER_ENCRYPTED_BALANCE); + + if (submit(arg, jv) == tesSUCCESS) + { + auto const postConfidentialOutstanding = + getIssuanceConfidentialBalance(); + env_.require(mptbalance(*this, *arg.account, holderAmt + *arg.amt)); + env_.require(requireAny([&]() -> bool { + return prevConfidentialOutstanding - *arg.amt == + postConfidentialOutstanding; + })); + + uint64_t postInboxBalance = + getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_INBOX); + uint64_t postIssuerBalance = + getDecryptedBalance(*arg.account, ISSUER_ENCRYPTED_BALANCE); + uint64_t postSpendingBalance = + getDecryptedBalance(*arg.account, HOLDER_ENCRYPTED_SPENDING); + + // inbox balance should not change + env_.require(requireAny( + [&]() -> bool { return postInboxBalance == prevInboxBalance; })); + + // issuer's encrypted balance is updated correctly + env_.require(requireAny([&]() -> bool { + return prevIssuerBalance - *arg.amt == postIssuerBalance; + })); + + // holder's spending balance is updated correctly + env_.require(requireAny([&]() -> bool { + return prevSpendingBalance - *arg.amt == postSpendingBalance; + })); + + // sum of holder's inbox and spending balance should equal to issuer's + // encrypted balance + env_.require(requireAny([&]() -> bool { + return postInboxBalance + postSpendingBalance == postIssuerBalance; + })); + } +} + } // namespace jtx } // namespace test } // namespace ripple diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index 12c5410db2..4033ed2288 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -196,6 +196,19 @@ struct MPTConfidentialSend std::optional flags = std::nullopt; std::optional err = std::nullopt; }; +struct MPTConvertBack +{ + std::optional account = std::nullopt; + std::optional id = std::nullopt; + std::optional amt = std::nullopt; + std::optional proof = std::nullopt; + std::optional holderEncryptedAmt = std::nullopt; + std::optional issuerEncryptedAmt = std::nullopt; + std::optional ownerCount = std::nullopt; + std::optional holderCount = std::nullopt; + std::optional flags = std::nullopt; + std::optional err = std::nullopt; +}; class MPTTester { @@ -237,6 +250,9 @@ public: void send(MPTConfidentialSend const& arg = MPTConfidentialSend{}); + void + convertBack(MPTConvertBack const& arg = MPTConvertBack{}); + [[nodiscard]] bool checkDomainID(std::optional expected) const; diff --git a/src/xrpld/app/tx/detail/ConfidentialConvert.cpp b/src/xrpld/app/tx/detail/ConfidentialConvert.cpp index 7c04021551..3158d115c6 100644 --- a/src/xrpld/app/tx/detail/ConfidentialConvert.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialConvert.cpp @@ -70,6 +70,11 @@ ConfidentialConvert::preclaim(PreclaimContext const& ctx) if (sleIssuance->isFlag(lsfMPTNoConfidentialTransfer)) return tecNO_PERMISSION; + // already checked in preflight, but should also check that issuer on the + // issuance isn't the account either + if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount]) + return tefINTERNAL; // LCOV_EXCL_LINE + // issuer has not uploaded their pub key yet if (!sleIssuance->isFieldPresent(sfIssuerElGamalPublicKey)) return tecNO_PERMISSION; diff --git a/src/xrpld/app/tx/detail/ConfidentialConvertBack.cpp b/src/xrpld/app/tx/detail/ConfidentialConvertBack.cpp index 7331c46d78..21a6e4ced0 100644 --- a/src/xrpld/app/tx/detail/ConfidentialConvertBack.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialConvertBack.cpp @@ -19,6 +19,7 @@ #include +#include #include #include #include @@ -43,13 +44,17 @@ ConfidentialConvertBack::preflight(PreflightContext const& ctx) ctx.tx[sfIssuerEncryptedAmount].length() != ecGamalEncryptedTotalLength) return temMALFORMED; - if (ctx.tx[sfMPTAmount] == 0) + if (ctx.tx[sfMPTAmount] == 0 || ctx.tx[sfMPTAmount] > maxMPTokenAmount) return temMALFORMED; + if (!isValidCiphertext(ctx.tx[sfHolderEncryptedAmount]) || + !isValidCiphertext(ctx.tx[sfIssuerEncryptedAmount])) + return temBAD_CIPHERTEXT; + // todo: update with correct size of proof since it might also contain range // proof - if (ctx.tx[sfZKProof].length() != ecEqualityProofLength) - return temMALFORMED; + // if (ctx.tx[sfZKProof].length() != ecEqualityProofLength) + // return temMALFORMED; return tesSUCCESS; } @@ -66,6 +71,11 @@ ConfidentialConvertBack::preclaim(PreclaimContext const& ctx) if (sleIssuance->isFlag(lsfMPTNoConfidentialTransfer)) return tecNO_PERMISSION; + // already checked in preflight, but should also check that issuer on the + // issuance isn't the account either + if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount]) + return tefINTERNAL; // LCOV_EXCL_LINE + auto const sleMptoken = ctx.view.read( keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], ctx.tx[sfAccount])); if (!sleMptoken) @@ -86,27 +96,41 @@ ConfidentialConvertBack::preclaim(PreclaimContext const& ctx) return tecINSUFFICIENT_FUNDS; } - // todo: need addtional parsing, the proof should contain multiple proofs - auto checkEqualityProof = [&](auto const& encryptedAmount, - auto const& pubKey) -> TER { - return proveEquality( - ctx.tx[sfZKProof], - encryptedAmount, - pubKey, - ctx.tx[sfMPTAmount], - ctx.tx.getTransactionID(), - (*sleMptoken)[~sfConfidentialBalanceVersion].value_or(0)); - }; + auto const mptIssuanceID = ctx.tx[sfMPTokenIssuanceID]; + auto const account = ctx.tx[sfAccount]; - if (!isTesSuccess(checkEqualityProof( - ctx.tx[sfHolderEncryptedAmount], - (*sleMptoken)[sfHolderElGamalPublicKey])) || - !isTesSuccess(checkEqualityProof( - ctx.tx[sfIssuerEncryptedAmount], - (*sleIssuance)[sfIssuerElGamalPublicKey]))) - { - return tecBAD_PROOF; - } + // Check lock + MPTIssue const mptIssue(mptIssuanceID); + if (auto const ter = checkFrozen(ctx.view, account, mptIssue); + ter != tesSUCCESS) + return ter; + + // Check auth + if (auto const ter = requireAuth(ctx.view, mptIssue, account); + !isTesSuccess(ter)) + return ter; + + // todo: need addtional parsing, the proof should contain multiple proofs + // auto checkEqualityProof = [&](auto const& encryptedAmount, + // auto const& pubKey) -> TER { + // return proveEquality( + // ctx.tx[sfZKProof], + // encryptedAmount, + // pubKey, + // ctx.tx[sfMPTAmount], + // ctx.tx.getTransactionID(), + // (*sleMptoken)[~sfConfidentialBalanceVersion].value_or(0)); + // }; + + // if (!isTesSuccess(checkEqualityProof( + // ctx.tx[sfHolderEncryptedAmount], + // (*sleMptoken)[sfHolderElGamalPublicKey])) || + // !isTesSuccess(checkEqualityProof( + // ctx.tx[sfIssuerEncryptedAmount], + // (*sleIssuance)[sfIssuerElGamalPublicKey]))) + // { + // return tecBAD_PROOF; + // } // todo: also check range proof that // sfHolderEncryptedAmount <= sfConfidentialBalanceSpending AND @@ -139,32 +163,31 @@ ConfidentialConvertBack::doApply() (*sleMptoken)[sfConfidentialBalanceVersion] = (*sleMptoken)[~sfConfidentialBalanceVersion].value_or(0u) + 1u; - // todo: support homomophic sub - // // homomorphically subtract holder's encrypted balance - // { - // Buffer res(ecGamalEncryptedTotalLength); - // if (TER const ter = homomorphicSub( - // (*sleMptoken)[sfConfidentialBalanceSpending], - // ctx_.tx[sfHolderEncryptedAmount], - // res); - // isTesSuccess(ter)) - // return tecINTERNAL; + // homomorphically subtract holder's encrypted balance + { + Buffer res(ecGamalEncryptedTotalLength); + if (TER const ter = homomorphicSubtract( + (*sleMptoken)[sfConfidentialBalanceSpending], + ctx_.tx[sfHolderEncryptedAmount], + res); + !isTesSuccess(ter)) + return tecINTERNAL; - // (*sleMptoken)[sfConfidentialBalanceSpending] = res; - // } + (*sleMptoken)[sfConfidentialBalanceSpending] = res; + } - // // homomorphically subtract issuer's encrypted balance - // { - // Buffer res(ecGamalEncryptedTotalLength); - // if (TER const ter = homomorphicSub( - // (*sleMptoken)[sfIssuerEncryptedBalance], - // ctx_.tx[sfIssuerEncryptedAmount], - // res); - // isTesSuccess(ter)) - // return tecINTERNAL; + // homomorphically subtract issuer's encrypted balance + { + Buffer res(ecGamalEncryptedTotalLength); + if (TER const ter = homomorphicSubtract( + (*sleMptoken)[sfIssuerEncryptedBalance], + ctx_.tx[sfIssuerEncryptedAmount], + res); + !isTesSuccess(ter)) + return tecINTERNAL; - // (*sleMptoken)[sfIssuerEncryptedBalance] = res; - // } + (*sleMptoken)[sfIssuerEncryptedBalance] = res; + } view().update(sleIssuance); view().update(sleMptoken); diff --git a/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp b/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp index 23c852eba9..880d336fc6 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp @@ -52,6 +52,11 @@ ConfidentialMergeInbox::preclaim(PreclaimContext const& ctx) if (sleIssuance->isFlag(lsfMPTNoConfidentialTransfer)) return tecNO_PERMISSION; + // already checked in preflight, but should also check that issuer on the + // issuance isn't the account either + if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount]) + return tefINTERNAL; // LCOV_EXCL_LINE + auto const sleMptoken = ctx.view.read( keylet::mptoken(ctx.tx[sfMPTokenIssuanceID], ctx.tx[sfAccount])); if (!sleMptoken) diff --git a/src/xrpld/app/tx/detail/ConfidentialSend.cpp b/src/xrpld/app/tx/detail/ConfidentialSend.cpp index c5b947d279..c2530de8bd 100644 --- a/src/xrpld/app/tx/detail/ConfidentialSend.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialSend.cpp @@ -97,6 +97,11 @@ ConfidentialSend::preclaim(PreclaimContext const& ctx) if (!sleIssuance->isFieldPresent(sfIssuerElGamalPublicKey)) return tecNO_PERMISSION; + // already checked in preflight, but should also check that issuer on the + // issuance isn't the account either + if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount]) + return tefINTERNAL; // LCOV_EXCL_LINE + // Check sender's MPToken auto const sleSenderMPToken = ctx.view.read(keylet::mptoken(mptIssuanceID, account)); @@ -178,7 +183,7 @@ ConfidentialSend::doApply() Slice const destEc = ctx_.tx[sfDestinationEncryptedAmount]; Slice const issuerEc = ctx_.tx[sfIssuerEncryptedAmount]; - // Substract from sender's spending balance + // Subtract from sender's spending balance { Slice const curSpending = (*sleSender)[sfConfidentialBalanceSpending]; Buffer newSpending(ecGamalEncryptedTotalLength); @@ -191,7 +196,7 @@ ConfidentialSend::doApply() (*sleSender)[sfConfidentialBalanceSpending] = newSpending; } - // Substract from issuer's balance + // Subtract from issuer's balance { Slice const curIssuerEnc = (*sleSender)[sfIssuerEncryptedBalance]; Buffer newIssuerEnc(ecGamalEncryptedTotalLength); @@ -206,7 +211,7 @@ ConfidentialSend::doApply() // Increment version (*sleSender)[sfConfidentialBalanceVersion] = - (*sleSender)[sfConfidentialBalanceVersion] + 1; + (*sleSender)[~sfConfidentialBalanceVersion].value_or(0u) + 1u; // Add to destination's inbox balance {