diff --git a/include/xrpl/protocol/ConfidentialTransfer.h b/include/xrpl/protocol/ConfidentialTransfer.h index 5ac0635d15..943806cd71 100644 --- a/include/xrpl/protocol/ConfidentialTransfer.h +++ b/include/xrpl/protocol/ConfidentialTransfer.h @@ -82,7 +82,7 @@ encryptAmount( Slice const& pubKeySlice, Slice const& blindingFactor); -Buffer +std::optional encryptCanonicalZeroAmount( Slice const& pubKeySlice, AccountID const& account, diff --git a/src/libxrpl/protocol/ConfidentialTransfer.cpp b/src/libxrpl/protocol/ConfidentialTransfer.cpp index 8a6acee0f0..6ebf7ec4ba 100644 --- a/src/libxrpl/protocol/ConfidentialTransfer.cpp +++ b/src/libxrpl/protocol/ConfidentialTransfer.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include @@ -219,15 +218,15 @@ encryptAmount( return buf; } -Buffer +std::optional encryptCanonicalZeroAmount( Slice const& pubKeySlice, AccountID const& account, MPTID const& mptId) { - Buffer buf(ecGamalEncryptedTotalLength); + if (pubKeySlice.size() != ecPubKeyLength) + return std::nullopt; // LCOV_EXCL_LINE - // Allocate ciphertext placeholders secp256k1_pubkey c1, c2; secp256k1_pubkey pubKey; @@ -241,12 +240,13 @@ encryptCanonicalZeroAmount( &pubKey, account.data(), mptId.data())) - Throw("Failed to encrypt amount"); + return std::nullopt; + + Buffer buf(ecGamalEncryptedTotalLength); // Serialize the ciphertext pair into the buffer if (!serializeEcPair(c1, c2, buf)) - Throw( - "Failed to serialize into 66 byte compressed format"); + return std::nullopt; return buf; } diff --git a/src/xrpld/app/tx/detail/ConfidentialClawback.cpp b/src/xrpld/app/tx/detail/ConfidentialClawback.cpp index 7117d71892..a160ceb895 100644 --- a/src/xrpld/app/tx/detail/ConfidentialClawback.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialClawback.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -18,20 +17,21 @@ ConfidentialClawback::preflight(PreflightContext const& ctx) return temDISABLED; auto const account = ctx.tx[sfAccount]; - auto const issuer = MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer(); // Only issuer can clawback - if (account != issuer) + if (account != MPTIssue(ctx.tx[sfMPTokenIssuanceID]).getIssuer()) return temMALFORMED; // Cannot clawback from self if (account == ctx.tx[sfHolder]) return temMALFORMED; + // Check invalid claw amount auto const clawAmount = ctx.tx[sfMPTAmount]; if (clawAmount == 0 || clawAmount > maxMPTokenAmount) return temBAD_AMOUNT; + // Verify proof length if (ctx.tx[sfZKProof].length() != ecEqualityProofLength) return temMALFORMED; @@ -84,13 +84,17 @@ ConfidentialClawback::preclaim(PreclaimContext const& ctx) if (amount > (*sleIssuance)[~sfConfidentialOutstandingAmount].value_or(0)) return tecINSUFFICIENT_FUNDS; - auto const ciphertext = (*sleHolderMPToken)[sfIssuerEncryptedBalance]; - auto const pubKeySlice = (*sleIssuance)[sfIssuerElGamalPublicKey]; - auto const contextHash = getClawbackContextHash( account, ctx.tx[sfSequence], mptIssuanceID, amount, holder); + + // Verify the revealed confidential amount by the issuer matches the exact + // confidential balance of the holder. return verifyClawbackEqualityProof( - amount, ctx.tx[sfZKProof], pubKeySlice, ciphertext, contextHash); + amount, + ctx.tx[sfZKProof], + (*sleIssuance)[sfIssuerElGamalPublicKey], + (*sleHolderMPToken)[sfIssuerEncryptedBalance], + contextHash); } TER @@ -110,53 +114,39 @@ ConfidentialClawback::doApply() Slice const holderPubKey = (*sleHolderMPToken)[sfHolderElGamalPublicKey]; Slice const issuerPubKey = (*sleIssuance)[sfIssuerElGamalPublicKey]; - // Encrypt zero amount - Buffer encZeroForHolder; - Buffer encZeroForIssuer; - try - { - encZeroForHolder = - encryptCanonicalZeroAmount(holderPubKey, holder, mptIssuanceID); - - encZeroForIssuer = - encryptCanonicalZeroAmount(issuerPubKey, holder, mptIssuanceID); - } - catch (std::exception const& e) - { - JLOG(ctx_.journal.error()) - << "ConfidentialClawback: Failed to generate canonical zero: " - << e.what(); + // After clawback, the balance should be encrypted zero. + auto const encZeroForHolder = + encryptCanonicalZeroAmount(holderPubKey, holder, mptIssuanceID); + if (!encZeroForHolder) + return tecINTERNAL; // LCOV_EXCL_LINE + + auto const encZeroForIssuer = + encryptCanonicalZeroAmount(issuerPubKey, holder, mptIssuanceID); + if (!encZeroForIssuer) return tecINTERNAL; // LCOV_EXCL_LINE - } // Set holder's confidential balances to encrypted zero - (*sleHolderMPToken)[sfConfidentialBalanceInbox] = encZeroForHolder; - (*sleHolderMPToken)[sfConfidentialBalanceSpending] = encZeroForHolder; - (*sleHolderMPToken)[sfIssuerEncryptedBalance] = encZeroForIssuer; + (*sleHolderMPToken)[sfConfidentialBalanceInbox] = *encZeroForHolder; + (*sleHolderMPToken)[sfConfidentialBalanceSpending] = *encZeroForHolder; + (*sleHolderMPToken)[sfIssuerEncryptedBalance] = *encZeroForIssuer; (*sleHolderMPToken)[sfConfidentialBalanceVersion] = 0; if (sleHolderMPToken->isFieldPresent(sfAuditorEncryptedBalance)) { - // check that issuance has auditor's pubkey before accessing it, if the - // field is absent, something has gone bad + // Sanity check: the issuance must have an auditor public key if + // auditing is enabled. if (!sleIssuance->isFieldPresent(sfAuditorElGamalPublicKey)) return tecINTERNAL; // LCOV_EXCL_LINE Slice const auditorPubKey = (*sleIssuance)[sfAuditorElGamalPublicKey]; - try - { - Buffer const encZeroForAuditor = encryptCanonicalZeroAmount( - auditorPubKey, holder, mptIssuanceID); - (*sleHolderMPToken)[sfAuditorEncryptedBalance] = encZeroForAuditor; - } - catch (std::exception const& e) - { - JLOG(ctx_.journal.error()) - << "ConfidentialClawback: Failed to generate canonical zero: " - << e.what(); + auto const encZeroForAuditor = + encryptCanonicalZeroAmount(auditorPubKey, holder, mptIssuanceID); + + if (!encZeroForAuditor) return tecINTERNAL; // LCOV_EXCL_LINE - } + + (*sleHolderMPToken)[sfAuditorEncryptedBalance] = *encZeroForAuditor; } // Decrease Global Confidential Outstanding Amount diff --git a/src/xrpld/app/tx/detail/ConfidentialConvert.cpp b/src/xrpld/app/tx/detail/ConfidentialConvert.cpp index fabfc0c61d..f72898084d 100644 --- a/src/xrpld/app/tx/detail/ConfidentialConvert.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialConvert.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -85,13 +84,8 @@ ConfidentialConvert::preclaim(PreclaimContext const& ctx) sleIssuance->isFieldPresent(sfAuditorElGamalPublicKey); // tx must include auditor ciphertext if the issuance has enabled - // auditing - if (requiresAuditor && !hasAuditor) - return tecNO_PERMISSION; - - // if auditing is not supported then user should not upload auditor - // ciphertext - if (!requiresAuditor && hasAuditor) + // auditing, and must not include it if auditing is not enabled + if (requiresAuditor != hasAuditor) return tecNO_PERMISSION; auto const sleMptoken = ctx.view.read(keylet::mptoken(issuanceID, account)); @@ -112,18 +106,21 @@ ConfidentialConvert::preclaim(PreclaimContext const& ctx) return tecINSUFFICIENT_FUNDS; } + auto const hasHolderKeyOnLedger = + sleMptoken->isFieldPresent(sfHolderElGamalPublicKey); + auto const hasHolderKeyInTx = + ctx.tx.isFieldPresent(sfHolderElGamalPublicKey); + // must have pk to convert - if (!sleMptoken->isFieldPresent(sfHolderElGamalPublicKey) && - !ctx.tx.isFieldPresent(sfHolderElGamalPublicKey)) + if (!hasHolderKeyOnLedger && !hasHolderKeyInTx) return tecNO_PERMISSION; // can't update if there's already a pk - if (sleMptoken->isFieldPresent(sfHolderElGamalPublicKey) && - ctx.tx.isFieldPresent(sfHolderElGamalPublicKey)) + if (hasHolderKeyOnLedger && hasHolderKeyInTx) return tecDUPLICATE; Slice holderPubKey; - if (ctx.tx.isFieldPresent(sfHolderElGamalPublicKey)) + if (hasHolderKeyInTx) { holderPubKey = ctx.tx[sfHolderElGamalPublicKey]; @@ -188,7 +185,7 @@ ConfidentialConvert::doApply() Slice const holderEc = ctx_.tx[sfHolderEncryptedAmount]; Slice const issuerEc = ctx_.tx[sfIssuerEncryptedAmount]; - std::optional const auditorEc = ctx_.tx[~sfAuditorEncryptedAmount]; + auto const auditorEc = ctx_.tx[~sfAuditorEncryptedAmount]; // todo: we should check sfConfidentialBalanceSpending depending on // if we encrypt zero amount @@ -243,10 +240,8 @@ ConfidentialConvert::doApply() (*sleMptoken)[sfAuditorEncryptedBalance] = *auditorEc; // encrypt sfConfidentialBalanceSpending with zero balance - auto const zeroBalance = encryptAmount( - 0, - (*sleMptoken)[sfHolderElGamalPublicKey], - generateBlindingFactor()); + auto const zeroBalance = encryptCanonicalZeroAmount( + (*sleMptoken)[sfHolderElGamalPublicKey], account_, mptIssuanceID); if (!zeroBalance) return tecINTERNAL; // LCOV_EXCL_LINE diff --git a/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp b/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp index a19747a3e0..998ea94f27 100644 --- a/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialMergeInbox.cpp @@ -44,7 +44,8 @@ ConfidentialMergeInbox::preclaim(PreclaimContext const& ctx) return tecOBJECT_NOT_FOUND; if (!sleMptoken->isFieldPresent(sfConfidentialBalanceInbox) || - !sleMptoken->isFieldPresent(sfConfidentialBalanceSpending)) + !sleMptoken->isFieldPresent(sfConfidentialBalanceSpending) || + !sleMptoken->isFieldPresent(sfHolderElGamalPublicKey)) return tecNO_PERMISSION; return tesSUCCESS; @@ -58,6 +59,14 @@ ConfidentialMergeInbox::doApply() if (!sleMptoken) return tecINTERNAL; + // sanity check + if (!sleMptoken->isFieldPresent(sfConfidentialBalanceSpending) || + !sleMptoken->isFieldPresent(sfConfidentialBalanceInbox) || + !sleMptoken->isFieldPresent(sfHolderElGamalPublicKey)) + { + return tecINTERNAL; + } + // homomorphically add holder's encrypted balance Buffer sum(ecGamalEncryptedTotalLength); if (TER const ter = homomorphicAdd( @@ -69,17 +78,13 @@ ConfidentialMergeInbox::doApply() (*sleMptoken)[sfConfidentialBalanceSpending] = sum; - try - { - Buffer zeroEncyption; - zeroEncyption = encryptCanonicalZeroAmount( - (*sleMptoken)[sfHolderElGamalPublicKey], account_, mptIssuanceID); - (*sleMptoken)[sfConfidentialBalanceInbox] = zeroEncyption; - } - catch (std::exception const& e) - { - return tecINTERNAL; - } + auto const zeroEncryption = encryptCanonicalZeroAmount( + (*sleMptoken)[sfHolderElGamalPublicKey], account_, mptIssuanceID); + + if (!zeroEncryption) + return tecINTERNAL; // LCOV_EXCL_LINE + + (*sleMptoken)[sfConfidentialBalanceInbox] = *zeroEncryption; // it's fine if it reaches max uint32, it just resets to 0 (*sleMptoken)[sfConfidentialBalanceVersion] = diff --git a/src/xrpld/app/tx/detail/ConfidentialSend.cpp b/src/xrpld/app/tx/detail/ConfidentialSend.cpp index a2f1f9250e..4b63a4f4bf 100644 --- a/src/xrpld/app/tx/detail/ConfidentialSend.cpp +++ b/src/xrpld/app/tx/detail/ConfidentialSend.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -30,6 +29,7 @@ ConfidentialSend::preflight(PreflightContext const& ctx) if (account == ctx.tx[sfDestination]) return temMALFORMED; + // Check the length of the encrypted amounts if (ctx.tx[sfSenderEncryptedAmount].length() != ecGamalEncryptedTotalLength || ctx.tx[sfDestinationEncryptedAmount].length() != @@ -43,17 +43,19 @@ ConfidentialSend::preflight(PreflightContext const& ctx) ecGamalEncryptedTotalLength) return temBAD_CIPHERTEXT; - if (!isValidCiphertext(ctx.tx[sfSenderEncryptedAmount]) || - !isValidCiphertext(ctx.tx[sfDestinationEncryptedAmount]) || - !isValidCiphertext(ctx.tx[sfIssuerEncryptedAmount])) - return temBAD_CIPHERTEXT; - if (hasAuditor && !isValidCiphertext(ctx.tx[sfAuditorEncryptedAmount])) return temBAD_CIPHERTEXT; // if (ctx.tx[sfZKProof].length() != ecEqualityProofLength) // return temMALFORMED; + // Check the encrypted amount formats, this is more expensive so put it at + // the end + if (!isValidCiphertext(ctx.tx[sfSenderEncryptedAmount]) || + !isValidCiphertext(ctx.tx[sfDestinationEncryptedAmount]) || + !isValidCiphertext(ctx.tx[sfIssuerEncryptedAmount])) + return temBAD_CIPHERTEXT; + return tesSUCCESS; } @@ -92,22 +94,16 @@ ConfidentialSend::preclaim(PreclaimContext const& ctx) bool const requiresAuditor = sleIssuance->isFieldPresent(sfAuditorElGamalPublicKey); - // tx must include auditor ciphertext if the issuance has enabled - // auditing - if (requiresAuditor && !hasAuditor) + // Tx must include auditor ciphertext if the issuance has enabled + // auditing, and must not include it if auditing is not enabled + if (requiresAuditor != hasAuditor) return tecNO_PERMISSION; - // if auditing is not supported then user should not upload auditor - // ciphertext - if (!requiresAuditor && hasAuditor) - return tecNO_PERMISSION; - - // already checked in preflight, but should also check that issuer on the - // issuance isn't the account either + // Sanity check: issuer isn't the sender if (sleIssuance->getAccountID(sfIssuer) == ctx.tx[sfAccount]) return tefINTERNAL; // LCOV_EXCL_LINE - // Check sender's MPToken + // Check sender's MPToken existence auto const sleSenderMPToken = ctx.view.read(keylet::mptoken(mptIssuanceID, account)); if (!sleSenderMPToken) @@ -119,7 +115,13 @@ ConfidentialSend::preclaim(PreclaimContext const& ctx) !sleSenderMPToken->isFieldPresent(sfIssuerEncryptedBalance)) return tecNO_PERMISSION; - // Check destination's MPToken + // Sanity check: MPToken's auditor field must be present if auditing is + // enabled + if (requiresAuditor && + !sleSenderMPToken->isFieldPresent(sfAuditorEncryptedBalance)) + return tefINTERNAL; + + // Check destination's MPToken existence auto const sleDestinationMPToken = ctx.view.read(keylet::mptoken(mptIssuanceID, destination)); if (!sleDestinationMPToken) @@ -150,23 +152,6 @@ ConfidentialSend::preclaim(PreclaimContext const& ctx) !isTesSuccess(ter)) return ter; - // todo: check zkproof. equality proof and range proof, combined or separate - // TBD. TER const terProof = verifyConfidentialSendProof( - // ctx.tx[sfZKProof], - // (*sleSender)[sfConfidentialBalanceSpending], - // ctx.tx[sfSenderEncryptedAmount], - // ctx.tx[sfDestinationEncryptedAmount], - // ctx.tx[sfIssuerEncryptedAmount], - // (*sleSender)[sfHolderElGamalPublicKey], - // (*sleDestination)[sfHolderElGamalPublicKey], - // (*sleIssuance)[sfIssuerElGamalPublicKey], - // (*sleSender)[~sfConfidentialBalanceVersion].value_or(0), - // ctx.tx.getTransactionID() - // ); - - // if (!isTesSuccess(terProof)) - // return tecBAD_PROOF; - return tesSUCCESS; } @@ -199,7 +184,7 @@ ConfidentialSend::doApply() Slice const destEc = ctx_.tx[sfDestinationEncryptedAmount]; Slice const issuerEc = ctx_.tx[sfIssuerEncryptedAmount]; - std::optional const auditorEc = ctx_.tx[~sfAuditorEncryptedAmount]; + auto const auditorEc = ctx_.tx[~sfAuditorEncryptedAmount]; // Subtract from sender's spending balance {