Skip to content

fixes #213 feat(circuits): parameterize hidden_cards hand size in Circom - #286

Open
dhareymu wants to merge 4 commits into
salazarsebas:mainfrom
dhareymu:hidden-cards
Open

fixes #213 feat(circuits): parameterize hidden_cards hand size in Circom#286
dhareymu wants to merge 4 commits into
salazarsebas:mainfrom
dhareymu:hidden-cards

Conversation

@dhareymu

Copy link
Copy Markdown

fixes #213

@drips-wave

drips-wave Bot commented Jul 26, 2026

Copy link
Copy Markdown

@dhareymu Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@salazarsebas

Copy link
Copy Markdown
Owner

Thanks for the work here! Looking at this against #213's acceptance criteria, there's a gap: the actual compiled circuit (hidden_cards.circom) still hardcodes component main {...} = HiddenCards(52, 5) — only the internal HandUnique template in deck.circom was generalized to a loop. The issue asks for builders to accept a variable hand_size on-chain, which needs:

  • hidden_cards.circom's main component to actually take a parameterized hand size (not just the helper template)
  • Corresponding changes on the Rust/on-chain side (builders + verifier) so a non-default hand_size can be requested and verified end-to-end
  • Real regenerated fixtures showing a hand_size ≠ 5 working through the full pipeline (the current hidden_cards.sym is 0 bytes, which looks like a broken/stale artifact rather than a real regeneration)

Could you extend the parameterization to the compiled circuit and wire it through to the Rust side? Happy to take another look once that's in.

@salazarsebas

Copy link
Copy Markdown
Owner

Appreciate the fixture-generation update, but I don't think this fully closes #213 yet — generate-fixtures.mjs now reads HAND_SIZE from an env var, but hidden_cards.circom itself still has component main {...} = HiddenCards(52, 5) hardcoded, so the compiled circuit (and the on-chain verifier) is still fixed at hand size 5. Could you rebase onto main and also parameterize the actual circuit instantiation (and regenerate the corresponding proving/verifying artifacts) so hand_size is genuinely configurable?

… pipeline artifacts

- hidden_cards.circom main component is now parameterized via compile.sh/compile.ps1
  with DECK_SIZE and HAND_SIZE env vars (default 52, 5) substituted before circom runs
- Remove stale 0-byte hidden_cards.sym artifact
- Regenerate generated.rs and generated_fixtures.rs from full circom pipeline
  (compile -> ptau power=16 -> setup -> prove -> verify -> export-soroban)
  All 4 circuits verified: hidden_cards, fog_of_war, fair_dice, sealed_bid
- Add hidden_cards_supports_non_default_hand_size unit test (hand_size=7)
- Fix Cargo.toml dev-dependencies to include cougr-core with testutils feature
  so integration tests in tests/ resolve test_fixtures without --features flag
- Fix examples/hidden_hand Cargo.toml to use local path dependency
- Fix PTAU_POWER default to 16 in setup.sh and download-ptau.sh (circuit needs 2^16)
- Add lib.sh repo-local circom binary path (target/circom_bin/bin)

Closes salazarsebas#213
@dhareymu

Copy link
Copy Markdown
Author

@salazarsebas Apologies for the late response. The highlighted issues have been resolved now. I had some technical issues from my end, that was why the delay.

@dhareymu

dhareymu commented Aug 3, 2026

Copy link
Copy Markdown
Author

@salazarsebas kindly help check

@dhareymu

dhareymu commented Aug 6, 2026

Copy link
Copy Markdown
Author

@salazarsebas I'm still waiting for you to check and merge

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.

feat(circuits): parameterize hidden_cards hand size in Circom

3 participants