fix(contracts): let committee:new request the active parameter set - #2134
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 52 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesCommittee parameter-set selection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Committee creation can now select either documented parameter set and stops before sending if it is unavailable. No new merge-blocking behavior is established. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
4102fee to
ca38edd
Compare
d79629a to
47e5bc5
Compare
|
@coderabbitai full review |
|
47e5bc5 to
d979e04
Compare
committee:new always requested BFV parameter set 0. A local deployment from a secure-8192 build registers only set 1, so the task could not request an E3 there. The task takes --param-set, with ACTIVE_BFV_PARAM_SET from scripts/utils.ts as the default, and derives the expected crypto config ID from the selected set. Before it sends a transaction, it rejects an unknown set and a set that the Interfold deployment has not registered. getE3Quote also rejects an unregistered set, but the contracts build strips revert strings, so that revert carries no reason. Refs #2068 [skip-doc-sync]: no statement in agent/ changes; flow-trace 03 still describes the committee:new owner-capacity check correctly.
|
@coderabbitai full review |
d979e04 to
99b2598
Compare
|
What
Fixes the #2068 item "
committee:newalways requests parameter set 0". PR 2 of the Stack 4 stack; base: PR 1.paramSet = 0. A local deployment from a secure-8192 build registers only set 1, so the task could not request an E3 there.--param-set, withACTIVE_BFV_PARAM_SETfromscripts/utils.ts(the generated constant of the active circuit build) as the default. The task derives the expected crypto config ID from the selected set. Before its first transaction, it stops for an unknown set and for a set that the Interfold deployment has not registered (paramSetRegistry(paramSet)is empty).getE3Quotealso rejects an unregistered set, but the contracts build strips revert strings (revertStrings: "strip"), so that revert has no reason.cryptoConfigIdForParamSetstays intasks/interfold.ts, becausescripts/check-committee.shcompares it withActiveCryptoConfig.sol. feat!: upgrade fhe.rs & refresh BFV circuits [skip-line-limit] #1996 changes the same function; the second PR to merge adapts.Verification
The task needs a deployed system on a node (
--network localhost), so it has no Hardhat test. On the test host, against a private anvil, afterdeploy:mocksand threeciphernode:admin-addregistrations:paramSet: 0and config ID0x20d76557…; the E3 is requested.--param-set 2:Unsupported BFV parameter set: 2; no transaction (deployer nonce unchanged).--param-set 1before registration:BFV parameter set 1 is not registered on this Interfold deployment. No E3 was requested.; no transaction. Without the pre-check, the quote reverts with empty data.--param-set 1aftersetParamSet(1, …): the request usesparamSet: 1and config ID0x3115e08e…; the E3 is requested.The issue's own case also ran (Codex review): after
pnpm build:circuits sync-config --preset secure-8192 --committee minimumand a ZK-path deployment, which registers only set 1, the task requested set 1 by default (0x3115e08e…);--param-set 2and the unregistered--param-set 0stopped with no transaction; after registering set 0,--param-set 0requested an E3 (0x20d76557…).Checklist
tsc --noEmit -p packages/interfold-contracts; prettier.agent/changes; the commit carries[skip-doc-sync]with the reason.cryptoConfigIdForParamSetblock thatscripts/check-committee.shreads is unchanged; the config ID still comes from the selected set.Summary by CodeRabbit
New Features
Documentation