Skip to content

fix: add missing domain separation to Bulletproof Fiat-Shamir challenges - #143

Open
tesseract-ripple wants to merge 2 commits into
XRPLF:mainfrom
tesseract-ripple:fix-bp-x-challenge-domain-tag
Open

tesseract-ripple wants to merge 2 commits into
XRPLF:mainfrom
tesseract-ripple:fix-bp-x-challenge-domain-tag

Conversation

@tesseract-ripple

Copy link
Copy Markdown
Collaborator

Summary

The y and z challenges in the aggregated Bulletproof are domain-separated with "MPT_BULLETPROOF_RANGE", but x is not, in either the prover or the verifier. Both sides omit it consistently so there is no soundness break, but the transcript loses protocol separation, which is what #50 reports. While confirming that, I found the two IPA challenge derivations have the same gap: derive_ipa_binding_challenge (ux) hashes commit_inp || dot and derive_ipa_round_challenge hashes last_challenge || L || R, neither with a tag.

The README already describes the Fiat-Shamir construction as having strict domain separation, so this brings the code in line with what we claim.

Changes

First commit adds "MPT_BULLETPROOF_RANGE" as the leading input to the x challenge in both the prover and the verifier, matching how y and z already do it. Closes #50.

Second commit adds "MPT_BULLETPROOF_IPA" to the two IPA helpers. Those are each single functions called from both sides, so prover and verifier stay in step by construction. It is a separate commit so it can be dropped independently if you would rather keep this PR to the filed issue.

This changes the challenge values, so proofs generated before it will not verify after it and vice versa. No test vectors in the repo pin bulletproof bytes (the two files with fixed vectors, test_pok_sk.c and test_ct_tweak_mul.c, do not touch bulletproofs), and the bulletproof tests are round-trip, so nothing here needed regenerating. The rippled side is the part that needs coordinating, same class of thing as #92. Worth deciding whether this wants to ride along with an amendment boundary.

One open question for review: I used the existing protocol-level tag for x rather than a distinct per-challenge label, on the grounds that it matches y and z and keeps the diff minimal. Distinct labels per challenge role would be the stricter construction. Happy to switch if you prefer that.

Testing

Built Debug on macOS with the full suite, 12/12 ctest pass. pre-commit run --all-files clean.

The y and z Fiat-Shamir challenges are domain-separated with
"MPT_BULLETPROOF_RANGE" but x was not, in either the prover or the
verifier. Both sides omitted it consistently so there is no soundness
break, but the transcript loses protocol separation.

Closes XRPLF#50.
derive_ipa_binding_challenge (ux) and derive_ipa_round_challenge (u_r)
both hashed their inputs with no domain tag. Adds "MPT_BULLETPROOF_IPA"
to each. Both helpers are shared by prover and verifier, so the two
sides stay in step by construction.

Separate from the x-challenge fix so it can be dropped independently,
though both land in the same transcript-changing release either way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Bulletproof x-challenge hash missing domain separation tag

1 participant