Skip to content

Support compact AND-composed sigma proof#6861

Merged
kennyzlei merged 2 commits intoXRPLF:ripple/attackathon-april-2026from
yinyiqian1:confidential-mpt-compact-proof
Apr 11, 2026
Merged

Support compact AND-composed sigma proof#6861
kennyzlei merged 2 commits intoXRPLF:ripple/attackathon-april-2026from
yinyiqian1:confidential-mpt-compact-proof

Conversation

@yinyiqian1
Copy link
Copy Markdown
Collaborator

@yinyiqian1 yinyiqian1 commented Apr 10, 2026

This PR is to integrate the new changes from mpt-crypto SDK mpt_utility:

The mpt_utility SDK change can be found here: XRPLF/mpt-crypto#24

For reference, the updated proof sizes per transaction are:

ConfidentialMPTConvert: 64 byte.
ConfidentialMPTSend: 192 byte compact sigma + 754 byte bullet proof = 946 byte.
ConfidentialMPTConvertBack: 128 byte compact sigma + 688 byte bullet proof = 816 byte
ConfidentialMPTClawback: 64 byte.

High Level Overview of Change

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@yinyiqian1 yinyiqian1 marked this pull request as ready for review April 10, 2026 23:30
@kennyzlei kennyzlei added the Attackathon Used to label PRs created for Attackathon branch label Apr 10, 2026
@kennyzlei
Copy link
Copy Markdown
Collaborator

/ai-review

Copy link
Copy Markdown
Contributor

@xrplf-ai-reviewer xrplf-ai-reviewer bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two high-severity issues in the crypto verification path and a stale lock file entry — see inline comments.

Review by Claude Opus 4.6 · Prompt: V14

2 * ecPedersenProofLength + ecDoubleBulletproofLength;

if (proof.size() != expectedProofSize || sender.publicKey.size() != ecPubKeyLength ||
if (proof.size() != ecCompactSendProofLength || sender.publicKey.size() != ecPubKeyLength ||
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proof-size check ignores auditor presence — ecCompactSendProofLength is a single constant, but recipientCount varies. Use the dynamic calculation:

Suggested change
if (proof.size() != ecCompactSendProofLength || sender.publicKey.size() != ecPubKeyLength ||
if (proof.size() != getEqualityProofSize(recipientCount) + 2 * ecPedersenProofLength + ecDoubleBulletproofLength || sender.publicKey.size() != ecPubKeyLength ||

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recipientCount is no longer valid. The new design provides a fixed length. Please check my PR in the mpt-crypto library: XRPLF/mpt-crypto#24

The core changes are in that PR

@@ -403,7 +404,6 @@ verifySendProof(

if (mpt_verify_send_proof(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proof.size() removed from mpt_verify_send_proof — if the C library doesn't infer length from a constant, this risks a buffer overread in the crypto path. Confirm the library API changed, or restore the size argument:

    if (mpt_verify_send_proof(
            proof.data(),
            proof.size(),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the API changed, can be found XRPLF/mpt-crypto#24

"protobuf/6.33.5#d96d52ba5baaaa532f47bda866ad87a5%1774467363.12",
"openssl/3.6.1#e6399de266349245a4542fc5f6c71552%1774458290.139",
"nudb/2.0.9#11149c73f8f2baff9a0198fe25971fc7%1774883011.384",
"mpt-crypto/0.2.0-rc2#2236dfc0cc11be70d84a5fa17a50bfb1%1775853786.434",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both rc1 and rc2 are present in the lock file — rc1 should be removed. Regenerate with conan lock create rather than hand-editing:

        "mpt-crypto/0.2.0-rc2#2236dfc0cc11be70d84a5fa17a50bfb1%1775853786.434",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 11, 2026

Codecov Report

❌ Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (ripple/attackathon-april-2026@ac4f142). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/libxrpl/protocol/ConfidentialTransfer.cpp 90.0% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                       @@
##             ripple/attackathon-april-2026   #6861   +/-   ##
===============================================================
  Coverage                                 ?   82.1%           
===============================================================
  Files                                    ?    1036           
  Lines                                    ?   78672           
  Branches                                 ?    7660           
===============================================================
  Hits                                     ?   64590           
  Misses                                   ?   14082           
  Partials                                 ?       0           
Files with missing lines Coverage Δ
include/xrpl/protocol/ConfidentialTransfer.h 83.3% <ø> (ø)
include/xrpl/protocol/Protocol.h 100.0% <ø> (ø)
...de/xrpl/tx/transactors/token/ConfidentialMPTSend.h 100.0% <ø> (ø)
...l/tx/transactors/token/ConfidentialMPTClawback.cpp 97.3% <100.0%> (ø)
...x/transactors/token/ConfidentialMPTConvertBack.cpp 99.0% <100.0%> (ø)
...bxrpl/tx/transactors/token/ConfidentialMPTSend.cpp 96.7% <100.0%> (ø)
src/libxrpl/protocol/ConfidentialTransfer.cpp 92.9% <90.0%> (ø)

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@PeterChen13579 PeterChen13579 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I found some dead code in mpt_crypto lib but we can clean it up later

@kennyzlei kennyzlei merged commit a1613e5 into XRPLF:ripple/attackathon-april-2026 Apr 11, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Attackathon Used to label PRs created for Attackathon branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants