feat: update submission deadline task#1102
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThis PR introduces a new Hardhat task Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/enclave-contracts/tasks/ciphernode.ts (1)
536-573: Consider validatingnewWindowand double-checking deploy helper semanticsThe task wiring and flow look good: you obtain PoseidonT3, get the
ciphernodeRegistryvia the existing deploy helper, then callsetSortitionSubmissionWindow(newWindow)and wait for confirmation.Two points worth tightening up:
- Guard against invalid
newWindowvaluesRight now,
newWindowis an unvalidatedINT. A negative or zero value could cause an unexpected revert or underflow in the contract, depending on howsetSortitionSubmissionWindowis implemented. Adding a quick check before calling the contract would make the task safer for admins:.setAction(async () => ({ default: async ({ newWindow }, hre) => { + if (newWindow <= 0) { + throw new Error("newWindow must be a positive integer (e.g. number of blocks)."); + } + const { deployAndSaveCiphernodeRegistryOwnable } = await import( "../scripts/deployAndSave/ciphernodeRegistryOwnable" );
- Confirm that
deployAndSaveCiphernodeRegistryOwnablewon’t redeploy unintentionallyHere you call:
const { ciphernodeRegistry } = await deployAndSaveCiphernodeRegistryOwnable({ hre, poseidonT3Address: poseidonT3, });Given that
deployAndSaveCiphernodeRegistryOwnablecomparesenclaveAddress,owner, andsubmissionWindowwith stored values to decide whether to redeploy, please double-check that omitting those arguments here will only attach to the existing proxy and not trigger a redeploy with defaulted values. If it can redeploy, you may want a dedicated “attach-only” helper (or explicitly pass the stored args) so calling this admin task doesn’t accidentally wipe/replace registry state.examples/CRISP/packages/crisp-contracts/package.json (1)
36-40: Deployment and window-update scripts look consistent with the core package
- Updating
deploy:contracts:fullto also setUSE_MOCK_VERIFIER=truekeeps the example aligned with the deployment expectations in this repo.- The new
updateSubmissionWindowscript mirrors the enclave-contracts package and should invoke the same Hardhat task.Only caveat: like the existing
export-based scripts, this is POSIX-shell–oriented; if you ever need Windows support, a cross-platform env setter (or npmcross-env) would be needed, but that’s consistent with the rest of the current setup.packages/enclave-contracts/hardhat.config.ts (1)
21-22: Core Hardhat config correctly registersupdateSubmissionWindowAdding
updateSubmissionWindowto the ciphernode imports and including it in thetasksarray cleanly exposes the new admin task in the main contracts workspace. Functionally this is solid.If you care about readability, you might consider grouping
updateSubmissionWindownext to the other ciphernode tasks in the array (e.g., right afterciphernodeSiblings) rather than aftercleanDeploymentsTask, but that’s purely organizational.Also applies to: 91-106
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
examples/CRISP/packages/crisp-contracts/hardhat.config.ts(2 hunks)examples/CRISP/packages/crisp-contracts/package.json(1 hunks)packages/enclave-contracts/hardhat.config.ts(2 hunks)packages/enclave-contracts/package.json(1 hunks)packages/enclave-contracts/tasks/ciphernode.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-11T13:21:31.031Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/tasks/utils.ts:7-8
Timestamp: 2025-09-11T13:21:31.031Z
Learning: In Hardhat v3, the task API syntax has changed significantly from v2. The new syntax uses:
- `.addOption({ name, description, defaultValue, type })` instead of `.addOptionalParam()`
- `.setAction(async () => ({ default: (args, hre) => { ... } }))` instead of direct `.setAction((args, hre) => { ... })`
- `.build()` is required to finalize task definitions
- `ArgumentType.STRING` is used for option types instead of `types.string`
Applied to files:
examples/CRISP/packages/crisp-contracts/hardhat.config.tspackages/enclave-contracts/hardhat.config.tspackages/enclave-contracts/tasks/ciphernode.ts
📚 Learning: 2025-10-10T12:56:40.538Z
Learnt from: 0xjei
Repo: gnosisguild/enclave PR: 830
File: templates/default/README.md:123-128
Timestamp: 2025-10-10T12:56:40.538Z
Learning: In the Enclave repository, the hard-coded Hardhat development private key `0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80` is acceptable in template README files and documentation for local testing/interaction purposes.
Applied to files:
examples/CRISP/packages/crisp-contracts/package.json
🧬 Code graph analysis (2)
packages/enclave-contracts/hardhat.config.ts (1)
packages/enclave-contracts/tasks/ciphernode.ts (1)
updateSubmissionWindow(537-573)
packages/enclave-contracts/tasks/ciphernode.ts (3)
packages/enclave-contracts/scripts/deployAndSave/poseidonT3.ts (1)
deployAndSavePoseidonT3(19-66)packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
deployAndSaveCiphernodeRegistryOwnable(31-129)crates/net/src/net_interface.rs (1)
tx(106-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: crisp_unit
- GitHub Check: test_contracts
- GitHub Check: test_net
- GitHub Check: rust_integration
- GitHub Check: build_enclave_cli
- GitHub Check: build_ciphernode_image
- GitHub Check: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: rust_unit
🔇 Additional comments (3)
packages/enclave-contracts/package.json (1)
173-175: Scripts wiring for verification and window update looks consistent
verify:contractsandupdateSubmissionWindowcorrectly map to existing Hardhat entry points and follow the existing naming pattern for scripts. No issues from a contracts/tooling perspective.packages/enclave-contracts/tasks/ciphernode.ts (1)
9-9: ArgumentType import aligns with existing Hardhat v3 task styleBringing in
ArgumentTypefor typed options matches the pattern used in other tasks in this repo; nothing to change here.examples/CRISP/packages/crisp-contracts/hardhat.config.ts (1)
9-9: CRISP Hardhat config correctly exposes the new taskImporting
updateSubmissionWindowand adding it to thetasksarray keeps the CRISP example in sync with the core enclave-contracts config. This should makehardhat ciphernode:windowavailable out of the box for the example project.Also applies to: 66-67
2 participants
add task to update submission deadline from admin
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.