Compress ElGamal Public Keys and Pedersen Commitments + Add Validation (#6385)

src/libxrpl/protocol/ConfidentialTransfer.cpp

@@ -122,6 +122,20 @@ isValidCiphertext(Slice const& buffer)
     return makeEcPair(buffer, key1, key2);
 }
 
+bool
+isValidCompressedECPoint(Slice const& buffer)
+{
+    if (buffer.size() != compressedECPointLength)
+        return false;
+
+    // Compressed EC points must start with 0x02 or 0x03
+    if (buffer[0] != 0x02 && buffer[0] != 0x03)
+        return false;
+
+    secp256k1_pubkey point;
+    return secp256k1_ec_pubkey_parse(secp256k1Context(), &point, buffer.data(), buffer.size()) == 1;
+}
+
 TER
 homomorphicAdd(Slice const& a, Slice const& b, Buffer& out)
 {
@@ -192,8 +206,12 @@ encryptAmount(uint64_t const amt, Slice const& pubKeySlice, Slice const& blindin
     if (blindingFactor.size() != ecBlindingFactorLength)
         return std::nullopt;
 
+    if (pubKeySlice.size() != ecPubKeyLength)
+        return std::nullopt;
+
     secp256k1_pubkey c1, c2, pubKey;
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return std::nullopt;
 
     if (!secp256k1_elgamal_encrypt(secp256k1Context(), &c1, &c2, &pubKey, amt, blindingFactor.data()))
         return std::nullopt;
@@ -212,14 +230,15 @@ encryptCanonicalZeroAmount(Slice const& pubKeySlice, AccountID const& account, M
         return std::nullopt;  // LCOV_EXCL_LINE
 
     secp256k1_pubkey c1, c2, pubKey;
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return std::nullopt;  // LCOV_EXCL_LINE
 
     if (!generate_canonical_encrypted_zero(secp256k1Context(), &c1, &c2, &pubKey, account.data(), mptId.data()))
-        return std::nullopt;
+        return std::nullopt;  // LCOV_EXCL_LINE
 
     Buffer buf(ecGamalEncryptedTotalLength);
     if (!serializeEcPair(c1, c2, buf))
-        return std::nullopt;
+        return std::nullopt;  // LCOV_EXCL_LINE
 
     return buf;
 }
@@ -234,7 +253,8 @@ verifySchnorrProof(Slice const& pubKeySlice, Slice const& proofSlice, uint256 co
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
     secp256k1_pubkey pubKey;
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     if (secp256k1_mpt_pok_sk_verify(secp256k1Context(), proofSlice.data(), &pubKey, contextHash.data()) != 1)
         return tecBAD_PROOF;
@@ -256,7 +276,8 @@ verifyElGamalEncryption(
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
     secp256k1_pubkey pubKey;
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     secp256k1_pubkey c1, c2;
     if (!makeEcPair(ciphertext, c1, c2))
@@ -339,7 +360,8 @@ verifyMultiCiphertextEqualityProof(
         if (recipient.publicKey.size() != ecPubKeyLength)
             return tecINTERNAL;  // LCOV_EXCL_LINE
 
-        std::memcpy(pk[i].data, recipient.publicKey.data(), ecPubKeyLength);
+        if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pk[i], recipient.publicKey.data(), ecPubKeyLength) != 1)
+            return tecINTERNAL;  // LCOV_EXCL_LINE
     }
 
     int const result = secp256k1_mpt_verify_same_plaintext_multi(
@@ -363,8 +385,12 @@ verifyClawbackEqualityProof(
     if (!makeEcPair(ciphertext, c1, c2))
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
+    if (pubKeySlice.size() != ecPubKeyLength)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
+
     secp256k1_pubkey pubKey;
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     // Note: c2, c1 order - the proof is generated with c2 first (the encrypted
     // message component) because the equality proof structure expects the
@@ -413,16 +439,19 @@ verifyAmountPcmLinkage(
     if (!makeEcPair(encAmt, c1, c2))
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    secp256k1_pubkey pubKey;
     if (pubKeySlice.size() != ecPubKeyLength)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    secp256k1_pubkey pcm;
     if (pcmSlice.size() != ecPedersenCommitmentLength)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
-    std::memcpy(pcm.data, pcmSlice.data(), ecPedersenCommitmentLength);
+    secp256k1_pubkey pubKey;
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
+
+    secp256k1_pubkey pcm;
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pcm, pcmSlice.data(), ecPedersenCommitmentLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     if (secp256k1_elgamal_pedersen_link_verify(
             secp256k1Context(), proof.data(), &c1, &c2, &pubKey, &pcm, contextHash.data()) != 1)
@@ -450,16 +479,19 @@ verifyBalancePcmLinkage(
     if (!makeEcPair(encAmt, c1, c2))
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    secp256k1_pubkey pubKey;
     if (pubKeySlice.size() != ecPubKeyLength)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    secp256k1_pubkey pcm;
     if (pcmSlice.size() != ecPedersenCommitmentLength)
         return tecINTERNAL;  // LCOV_EXCL_LINE
 
-    std::memcpy(pubKey.data, pubKeySlice.data(), ecPubKeyLength);
-    std::memcpy(pcm.data, pcmSlice.data(), ecPubKeyLength);
+    secp256k1_pubkey pubKey;
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pubKey, pubKeySlice.data(), ecPubKeyLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
+
+    secp256k1_pubkey pcm;
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pcm, pcmSlice.data(), ecPedersenCommitmentLength) != 1)
+        return tecINTERNAL;  // LCOV_EXCL_LINE
 
     // Note: c2, c1 order - the linkage proof expects the message-containing
     // component (c2 = m*G + r*Pk) before the blinding component (c1 = r*G).

src/test/app/ConfidentialTransfer_test.cpp

@@ -48,6 +48,26 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
         return trivialCiphertext;
     }
 
+    // Returns a valid compressed EC point (33 bytes) that can pass preflight
+    // validation but contains invalid data for preclaim test purposes.
+    static Buffer const&
+    getTrivialCommitment()
+    {
+        static Buffer const trivialCommitment = []() {
+            Buffer buf(ecPedersenCommitmentLength);
+            std::memset(buf.data(), 0, ecPedersenCommitmentLength);
+
+            // 0x02 prefix for compressed EC point with even y-coordinate
+            buf.data()[0] = 0x02;
+            // Set last byte to make it a valid x-coordinate on the curve
+            buf.data()[ecPedersenCommitmentLength - 1] = 0x01;
+
+            return buf;
+        }();
+
+        return trivialCommitment;
+    }
+
     std::string
     getTrivialSendProofHex(size_t nRecipients)
     {
@@ -276,6 +296,9 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
 
             // Holder public key is invalid (empty buffer)
             mptAlice.convert({.account = bob, .amt = 10, .holderPubKey = Buffer{}, .err = temMALFORMED});
+
+            // Holder public key has correct length but invalid EC point data
+            mptAlice.convert({.account = bob, .amt = 10, .holderPubKey = Buffer(ecPubKeyLength), .err = temMALFORMED});
         }
 
         // when registering holder pub key, the transaction must include a
@@ -388,9 +411,12 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
             mptAlice.generateKeyPair(alice);
             mptAlice.generateKeyPair(bob);
 
-            // Pub key is invalid
+            // Issuer pub key is invalid (empty)
             mptAlice.set({.account = alice, .issuerPubKey = Buffer{}, .err = temMALFORMED});
 
+            // Issuer pub key has correct length but invalid EC point data
+            mptAlice.set({.account = alice, .issuerPubKey = Buffer(ecPubKeyLength), .err = temMALFORMED});
+
             // Auditor key is invalid length
             mptAlice.set(
                 {.account = alice,
@@ -398,6 +424,13 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .auditorPubKey = Buffer(10),
                  .err = temMALFORMED});
 
+            // Auditor key has correct length but invalid EC point data
+            mptAlice.set(
+                {.account = alice,
+                 .issuerPubKey = mptAlice.getPubKey(alice),
+                 .auditorPubKey = Buffer(ecPubKeyLength),
+                 .err = temMALFORMED});
+
             // Cannot set auditor key without issuer key
             mptAlice.set({.account = alice, .auditorPubKey = mptAlice.getPubKey(alice), .err = temMALFORMED});
 
@@ -1135,8 +1168,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(3),
                  .senderEncryptedAmt = Buffer(ecGamalEncryptedTotalLength),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temBAD_CIPHERTEXT});
             // dest encrypted amount malformed
             mptAlice.send(
@@ -1145,8 +1178,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(3),
                  .destEncryptedAmt = Buffer(ecGamalEncryptedTotalLength),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temBAD_CIPHERTEXT});
             // issuer encrypted amount malformed
             mptAlice.send(
@@ -1155,8 +1188,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(3),
                  .issuerEncryptedAmt = Buffer(ecGamalEncryptedTotalLength),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temBAD_CIPHERTEXT});
 
             // invalid proof length
@@ -1165,8 +1198,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .dest = carol,
                  .amt = 10,
                  .proof = std::string(10, 'A'),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temMALFORMED});
 
             // invalid amount Pedersen commitment length
@@ -1176,7 +1209,7 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(3),
                  .amountCommitment = Buffer(100),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temMALFORMED});
 
             // invalid balance Pedersen commitment length
@@ -1185,9 +1218,29 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .dest = carol,
                  .amt = 10,
                  .proof = getTrivialSendProofHex(3),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
                  .balanceCommitment = Buffer(100),
                  .err = temMALFORMED});
+
+            // amount Pedersen commitment has correct length but invalid EC point data
+            mptAlice.send(
+                {.account = bob,
+                 .dest = carol,
+                 .amt = 10,
+                 .proof = getTrivialSendProofHex(3),
+                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
+                 .balanceCommitment = getTrivialCommitment(),
+                 .err = temMALFORMED});
+
+            // balance Pedersen commitment has correct length but invalid EC point data
+            mptAlice.send(
+                {.account = bob,
+                 .dest = carol,
+                 .amt = 10,
+                 .proof = getTrivialSendProofHex(3),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .err = temMALFORMED});
         }
 
         // test bad ciphertext
@@ -1234,8 +1287,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(4),
                  .auditorEncryptedAmt = Buffer(10),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temBAD_CIPHERTEXT});
 
             // auditor encrypted amount (correct length, invalid data)
@@ -1245,8 +1298,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(4),
                  .auditorEncryptedAmt = getBadCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = temBAD_CIPHERTEXT});
         }
     }
@@ -1321,8 +1374,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
             jv[sfSenderEncryptedAmount] = strHex(getTrivialCiphertext());
             jv[sfDestinationEncryptedAmount] = strHex(getTrivialCiphertext());
             jv[sfIssuerEncryptedAmount] = strHex(getTrivialCiphertext());
-            jv[sfAmountCommitment] = strHex(Buffer(ecPedersenCommitmentLength));
-            jv[sfBalanceCommitment] = strHex(Buffer(ecPedersenCommitmentLength));
+            jv[sfAmountCommitment] = strHex(getTrivialCommitment());
+            jv[sfBalanceCommitment] = strHex(getTrivialCommitment());
             jv[sfZKProof] = getTrivialSendProofHex(3);
 
             env(jv, ter(tecOBJECT_NOT_FOUND));
@@ -1339,8 +1392,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .senderEncryptedAmt = getTrivialCiphertext(),
                  .destEncryptedAmt = getTrivialCiphertext(),
                  .issuerEncryptedAmt = getTrivialCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = tecNO_TARGET});
         }
 
@@ -1354,8 +1407,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .senderEncryptedAmt = getTrivialCiphertext(),
                  .destEncryptedAmt = getTrivialCiphertext(),
                  .issuerEncryptedAmt = getTrivialCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = tecNO_PERMISSION});
             mptAlice.send(
                 {.account = dave,
@@ -1365,8 +1418,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .senderEncryptedAmt = getTrivialCiphertext(),
                  .destEncryptedAmt = getTrivialCiphertext(),
                  .issuerEncryptedAmt = getTrivialCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = tecNO_PERMISSION});
         }
 
@@ -1380,8 +1433,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .senderEncryptedAmt = getTrivialCiphertext(),
                  .destEncryptedAmt = getTrivialCiphertext(),
                  .issuerEncryptedAmt = getTrivialCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = tecOBJECT_NOT_FOUND});
         }
 
@@ -1585,8 +1638,8 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
                  .amt = 10,
                  .proof = getTrivialSendProofHex(4),
                  .auditorEncryptedAmt = getTrivialCiphertext(),
-                 .amountCommitment = Buffer(ecPedersenCommitmentLength),
-                 .balanceCommitment = Buffer(ecPedersenCommitmentLength),
+                 .amountCommitment = getTrivialCommitment(),
+                 .balanceCommitment = getTrivialCommitment(),
                  .err = tecBAD_PROOF});
         }
     }
@@ -1862,6 +1915,13 @@ class ConfidentialTransfer_test : public beast::unit_test::suite
             // invalid blinding factor length
             mptAlice.convertBack({.account = alice, .amt = 30, .blindingFactor = Buffer{}, .err = temMALFORMED});
 
+            // Balance commitment has correct length but invalid EC point data
+            mptAlice.convertBack(
+                {.account = bob,
+                 .amt = 30,
+                 .pedersenCommitment = Buffer(ecPedersenCommitmentLength),
+                 .err = temMALFORMED});
+
             mptAlice.convertBack({.account = bob, .amt = 30, .holderEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT});
 
             mptAlice.convertBack({.account = bob, .amt = 30, .issuerEncryptedAmt = Buffer{}, .err = temBAD_CIPHERTEXT});

src/test/jtx/impl/mpt.cpp

@@ -698,7 +698,11 @@ MPTTester::getClawbackProof(
         return std::nullopt;
     }
 
-    std::memcpy(pk.data, pubKeyBlob.data(), ecPubKeyLength);
+    if (!secp256k1_ec_pubkey_parse(ctx, &pk, pubKeyBlob.data(), ecPubKeyLength))
+    {
+        return std::nullopt;
+    }
+
     Buffer proof(ecEqualityProofLength);
 
     if (secp256k1_equality_plaintext_prove(
@@ -722,7 +726,8 @@ MPTTester::getSchnorrProof(Account const& account, uint256 const& ctxHash) const
         return std::nullopt;
 
     secp256k1_pubkey pk;
-    std::memcpy(pk.data, pubKey->data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(secp256k1Context(), &pk, pubKey->data(), ecPubKeyLength) != 1)
+        return std::nullopt;
 
     Buffer proof(ecSchnorrProofLength);
 
@@ -785,7 +790,8 @@ MPTTester::getConfidentialSendProof(
             return std::nullopt;
         }
 
-        std::memcpy(pk[i].data, recipient.publicKey.data(), ecPubKeyLength);
+        if (!secp256k1_ec_pubkey_parse(ctx, &pk[i], recipient.publicKey.data(), ecPubKeyLength))
+            return std::nullopt;
 
         sr.insert(sr.end(), blindingFactor.data(), blindingFactor.data() + ecBlindingFactorLength);
     }
@@ -839,9 +845,16 @@ MPTTester::getPedersenCommitment(std::uint64_t const amount, Buffer const& peder
     if (pedersenBlindingFactor.size() != ecBlindingFactorLength)
         Throw<std::runtime_error>("Invalid blinding factor size");
 
-    // current pedersen generation implementation fails if amount is 0
+    // secp256k1_mpt_pedersen_commit doesn't handle amount 0, return a trivial
+    // valid commitment for test purposes
     if (amount == 0)
-        return Buffer{ecPedersenCommitmentLength};
+    {
+        Buffer buf(ecPedersenCommitmentLength);
+        std::memset(buf.data(), 0, ecPedersenCommitmentLength);
+        buf.data()[0] = 0x02;
+        buf.data()[ecPedersenCommitmentLength - 1] = 0x01;
+        return buf;
+    }
 
     secp256k1_pubkey commitment;
     auto const ctx = secp256k1Context();
@@ -852,7 +865,16 @@ MPTTester::getPedersenCommitment(std::uint64_t const amount, Buffer const& peder
         Throw<std::runtime_error>("Pedersen commitment generation failed");
     }
 
-    return Buffer{commitment.data, ecPedersenCommitmentLength};
+    // Serialize commitment to compressed format (33 bytes)
+    unsigned char compressedCommitment[ecPedersenCommitmentLength];
+    size_t outLen = ecPedersenCommitmentLength;
+    if (secp256k1_ec_pubkey_serialize(ctx, compressedCommitment, &outLen, &commitment, SECP256K1_EC_COMPRESSED) != 1 ||
+        outLen != ecPedersenCommitmentLength)
+    {
+        Throw<std::runtime_error>("Pedersen commitment serialization failed");
+    }
+
+    return Buffer{compressedCommitment, ecPedersenCommitmentLength};
 }
 
 Buffer
@@ -1427,7 +1449,15 @@ MPTTester::generateKeyPair(Account const& account)
     if (!secp256k1_elgamal_generate_keypair(secp256k1Context(), privKey, &pubKey))
         Throw<std::runtime_error>("failed to generate key pair");
 
-    pubKeys.insert({account.id(), Buffer{pubKey.data, ecPubKeyLength}});
+    // Serialize public key to compressed format (33 bytes)
+    unsigned char compressedPubKey[ecPubKeyLength];
+    size_t outLen = ecPubKeyLength;
+    if (secp256k1_ec_pubkey_serialize(
+            secp256k1Context(), compressedPubKey, &outLen, &pubKey, SECP256K1_EC_COMPRESSED) != 1 ||
+        outLen != ecPubKeyLength)
+        Throw<std::runtime_error>("failed to serialize public key");
+
+    pubKeys.insert({account.id(), Buffer{compressedPubKey, ecPubKeyLength}});
     privKeys.insert({account.id(), Buffer{privKey, ecPrivKeyLength}});
 }
 
@@ -1752,10 +1782,12 @@ MPTTester::getAmountLinkageProof(
     }
 
     secp256k1_pubkey pk;
-    std::memcpy(pk.data, pubKey.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(ctx, &pk, pubKey.data(), ecPubKeyLength) != 1)
+        return Buffer();
 
     secp256k1_pubkey pcm;
-    std::memcpy(pcm.data, params.pedersenCommitment.data(), ecPedersenCommitmentLength);
+    if (secp256k1_ec_pubkey_parse(ctx, &pcm, params.pedersenCommitment.data(), ecPedersenCommitmentLength) != 1)
+        return Buffer();
 
     Buffer proof(ecPedersenProofLength);
     if (secp256k1_elgamal_pedersen_link_prove(
@@ -1798,10 +1830,12 @@ MPTTester::getBalanceLinkageProof(
     }
 
     secp256k1_pubkey pk;
-    std::memcpy(pk.data, pubKey.data(), ecPubKeyLength);
+    if (secp256k1_ec_pubkey_parse(ctx, &pk, pubKey.data(), ecPubKeyLength) != 1)
+        return Buffer();
 
     secp256k1_pubkey pcm;
-    std::memcpy(pcm.data, params.pedersenCommitment.data(), ecPedersenCommitmentLength);
+    if (secp256k1_ec_pubkey_parse(ctx, &pcm, params.pedersenCommitment.data(), ecPedersenCommitmentLength) != 1)
+        return Buffer();
 
     Buffer proof(ecPedersenProofLength);
 

src/xrpld/app/tx/detail/ConfidentialMPTConvert.cpp

@@ -28,7 +28,7 @@ ConfidentialMPTConvert::preflight(PreflightContext const& ctx)
 
     if (ctx.tx.isFieldPresent(sfHolderElGamalPublicKey))
     {
-        if (ctx.tx[sfHolderElGamalPublicKey].length() != ecPubKeyLength)
+        if (!isValidCompressedECPoint(ctx.tx[sfHolderElGamalPublicKey]))
             return temMALFORMED;
 
         // proof of knowledge of the secret key corresponding to the provided

src/xrpld/app/tx/detail/ConfidentialMPTConvertBack.cpp

@@ -28,7 +28,7 @@ ConfidentialMPTConvertBack::preflight(PreflightContext const& ctx)
     if (ctx.tx[sfBlindingFactor].size() != ecBlindingFactorLength)
         return temMALFORMED;
 
-    if (ctx.tx[sfBalanceCommitment].size() != ecPedersenCommitmentLength)
+    if (!isValidCompressedECPoint(ctx.tx[sfBalanceCommitment]))
         return temMALFORMED;
 
     // check encrypted amount format after the above basic checks

src/xrpld/app/tx/detail/ConfidentialMPTSend.cpp

@@ -47,9 +47,8 @@ ConfidentialMPTSend::preflight(PreflightContext const& ctx)
     if (ctx.tx[sfZKProof].length() != sizeEquality + sizePedersenLinkage)
         return temMALFORMED;
 
-    // Check the length of Pedersen commitments
-    if (ctx.tx[sfBalanceCommitment].size() != ecPedersenCommitmentLength ||
-        ctx.tx[sfAmountCommitment].size() != ecPedersenCommitmentLength)
+    // Check the Pedersen commitments are valid
+    if (!isValidCompressedECPoint(ctx.tx[sfBalanceCommitment]) || !isValidCompressedECPoint(ctx.tx[sfAmountCommitment]))
         return temMALFORMED;
 
     // Check the encrypted amount formats, this is more expensive so put it at

src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp

@@ -1,6 +1,7 @@
 #include <xrpld/app/misc/DelegateUtils.h>
 #include <xrpld/app/tx/detail/MPTokenIssuanceSet.h>
 
+#include <xrpl/protocol/ConfidentialTransfer.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/TxFlags.h>
@@ -127,10 +128,10 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx)
     if (hasAuditorElGamalKey && !hasIssuerElGamalKey)
         return temMALFORMED;
 
-    if (hasIssuerElGamalKey && ctx.tx[sfIssuerElGamalPublicKey].length() != ecPubKeyLength)
+    if (hasIssuerElGamalKey && !isValidCompressedECPoint(ctx.tx[sfIssuerElGamalPublicKey]))
         return temMALFORMED;
 
-    if (hasAuditorElGamalKey && ctx.tx[sfAuditorElGamalPublicKey].length() != ecPubKeyLength)
+    if (hasAuditorElGamalKey && !isValidCompressedECPoint(ctx.tx[sfAuditorElGamalPublicKey]))
         return temMALFORMED;
 
     return tesSUCCESS;