Skip to content

Comments

DotRing: Milestone 1 Delivery#8

Open
prasad-kumkar wants to merge 3 commits intoPolkadotOpenSourceGrants:masterfrom
Chainscore:Dot-Ring---Milestone1
Open

DotRing: Milestone 1 Delivery#8
prasad-kumkar wants to merge 3 commits intoPolkadotOpenSourceGrants:masterfrom
Chainscore:Dot-Ring---Milestone1

Conversation

@prasad-kumkar
Copy link

Milestone Delivery Checklist

  • The delivery template has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • The delivery is according to the delivery guidelines.

Link to the application pull request: PolkadotOpenSourceGrants/apply#14

@ditavia-br ditavia-br self-assigned this Oct 10, 2025
@ditavia-br
Copy link

@prasad-kumkar Thanks for the milestone delivery. Please take a look at the evaluation document and provide proper improvements and fixes. Some curves/modules need test coverage improvements. Let me know when I can evaluate this milestone again.

@konasiva7
Copy link

@ditavia-br Thanks for the review and suggestions! We’ll work on the improvements and test coverage updates, and let you know once everything is ready for reevaluation.

@konasiva7
Copy link

  • Added additional tests (including property based) , ensuring test coverage >80% including glv.py, bandersnatch.py, baby_jubjub.py, all other curves and curve families including mg_curve.py, mg_affine_point.py.
  • Improved coverage of field_element.py; coverage of kzg.py improved where possible.
  • Ensured the Ring proof Implementation and tests coverage >90%.
  • Added Dockerfile for container setup with all dependencies, and test execution.

@ditavia-br Ready for evaluation

@ditavia-br
Copy link

@konasiva7 Thanks for the improvements and fixes. Some files still have low coverage, please take a look at the evaluation document and either improve them or explain why this is the case. I’ll kindly ask @drskalman and @davxy, who reviewed the application, to take a look and share their opinions. Let me know when I can review it again.

@konasiva7
Copy link

konasiva7 commented Oct 20, 2025

@ditavia-br Thanks for the suggestions! please take a look at the updates below:

  • Made sure the coverage is > 80%
  • Skipping tests when the third-party msm is absent, we have this setup already covered in our readme, let us know if anything extra is required. (We tried this msm as one way of optimization while we do have the bulitin msm as well which is being used when flag is not set True)
  • Added property based tests as suggested

Ready for review

@ditavia-br
Copy link

@konasiva7 Thanks for the improvements. From my side, the delivery looks good, but let’s wait for the review from @drskalman and @davxy. I’ll ask to have them added as reviewers on the PR, since I don’t have permission to do it myself.

@drskalman
Copy link

Hello there, could you please add a test to deserialize this proof and verify it? also if you could write a test which serialize the proof generated by DotRing, I could test it in rust implementation.
ring_proof_and_key.json

Thank you.

@semuelle
Copy link

pinging @prasad-kumkar & @konasiva7

@prasad-kumkar
Copy link
Author

I'm on it @semuelle ; apologies for the delay, have been focused on optimization and had a restructuring underway

@prasad-kumkar
Copy link
Author

Hello @drskalman, may i know the structure of verifier_key.verification_key to deserialize? Does it contain the result point and ring commitments? Thank you

@prasad-kumkar
Copy link
Author

Sharing DotRing generated vectors here: https://github.com/Chainscore/dot-ring/tree/main/tests/vectors/dot-ring @drskalman

@prasad-kumkar
Copy link
Author

Hello @drskalman, may i know the structure of verifier_key.verification_key to deserialize? Does it contain the result point and ring commitments? Thank you

@drskalman would be great to have this ^

Also wanted to share that we have been working on performance optimisation, and with significant improvements we have added our benchmarks, which are comparable with ark-vrf bench.

We also submitted our Milestone 2 with release of the library

@prasad-kumkar
Copy link
Author

ping @drskalman

@drskalman
Copy link

Hello @drskalman, may i know the structure of verifier_key.verification_key to deserialize? Does it contain the result point and ring commitments? Thank you

Yeah. It is the point and the commitment to the fixed_columns: https://github.com/w3f/ring-proof/blob/skalman--export-json-proof/w3f-ring-proof/src/piop/mod.rs#L184

It is where it gets serialized:
https://github.com/w3f/ring-proof/blob/skalman--export-json-proof/w3f-ring-proof/examples/export_sample_ring_proof.rs#L123

@diogo-w3f
Copy link

ping @prasad-kumkar

@prasad-kumkar
Copy link
Author

@drskalman we were unable to verify, but we noticed the export file uses random srs params (I couldn't follow if they're deterministic?) - https://github.com/w3f/ring-proof/blob/b165b1d2a2ecf95cb88212033c4451775fbb509d/w3f-ring-proof/examples/export_sample_ring_proof.rs#L94-L96

However, we have successfully verified the test vectors generated by ark-vrf, which does inherit w3f/ring-proof, using constant SRS params from zcash ceremony here - https://github.com/Chainscore/dot-ring/blob/main/tests/test_ring_vrf/test_ring_vrf.py

Would it be acceptable for this milestone?

@prasad-kumkar
Copy link
Author

ping @drskalman

@ditavia-br
Copy link

@prasad-kumkar I spoke with @drskalman and he explained me that the verification key, which is part of srs that is needed for verification, is given. So, it should be possible to verify. The code should be able work with srs different than Zcash ones. Can you elaborate on that?

I asked an AI to evaluate the code and suggest the changes. Here is the answer:

Right now the ring‑proof KZG SRS is hard‑wired to the bundled Zcash file (dot_ring/vrf/data/bls12-381-srs-2-11-uncompressed-zcash.bin) and used directly by KZG.default() → SRS.from_loaded() → g1_points/g2_points. So verification only works if proofs were generated from that same SRS.

If you want to support a non‑Zcash SRS, you’d need to make the SRS configurable and thread it through ring‑proof creation and verification. A minimal path:

  1. Make the SRS file path configurable
    In dot_ring/ring_proof/pcs/load_powers.py, add a parameter or env var to choose the SRS file instead of always loading the Zcash file.

  2. Load SRS on demand instead of at import time
    Right now g1_points/g2_points are loaded at module import. Change that so read_srs_file(path) returns (g1_points, g2_points) and is called by SRS.from_loaded() or a new SRS.from_file(path, max_deg).

  3. Allow callers to pass an SRS
    Add optional parameters to KZG.default()/RingVrf methods (or a constructor) so the caller can pass a custom SRS file or preloaded SRS. Then use that SRS for both proof creation and verification.

  4. Plumb through ring‑proof verify
    In dot_ring/ring_proof/gotos.py, verify_signature and ring_vrf_proof_verify build verifier_key using g1_points/g2_points. Change those to use the passed SRS (or KZG instance) so verification matches the proof’s SRS.

Can you move forward to implement this way?

@prasad-kumkar
Copy link
Author

@ditavia-br you are right! It does include the the SRS params, I misunderstood it to be the result point - which was infact missing and deterministically randomly generated so I executed the rust export script locally to get those values. At the end we are able to verify the given proof in our implementation. Following is the test script:

uv run pytest tests/test_verify_rust_proof.py

@drskalman
Copy link

@prasad-kumkar Thank you! I'll take a look at the verification. Meanwhile could you generate a proof and export it in the same json format so I can verify it on the rust side? Thanks a lot.

@prasad-kumkar
Copy link
Author

@drskalman definitely! I generated a couple for diff domain sizes: https://github.com/Chainscore/dot-ring/tree/main/tests/vectors/others

I also cross-checked the rust implementation with this alongside the export script - https://gist.github.com/prasad-kumkar/600c60e2bccd3b2788ef7dcaaa80650f

@ditavia-br
Copy link

@prasad-kumkar Thanks for the changes. I was able to verify the proofs. I’ll wait for @drskalman to double-check before approving this milestone. He mentioned that he will review it in the next few days.

In the meantime, do you think any adjustments are needed to have Milestone 2 ready for evaluation?

@prasad-kumkar
Copy link
Author

Amazing, thank you @ditavia-br :)

All changes are merged on main branch so both milestones can be evaluated. For context - M2 is mostly performance changes & production release - here are the benchmarking results dotring comparable with ark-vrf. Additionally we have shared PyPi release and documentation for review.

@ditavia-br
Copy link

@prasad-kumkar thanks for the information. I'll check that soon. By the way, could you please fix the Dockerfile in this project? We should be able to generate and verify the proofs using docker to have isolation of the code from the local environment. Thanks!

@prasad-kumkar
Copy link
Author

@ditavia-br Dockerfile should be fixed now. We've also added a workflow which tests in a containerised environment as such - https://github.com/Chainscore/dot-ring/actions/runs/21911120890/job/63265025224?pr=26#logs

@prasad-kumkar
Copy link
Author

ping @ditavia-br @drskalman

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.

6 participants