chore: store block numbers#1101
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughA straightforward enhancement that captures and stores the current blockchain block number during contract deployments for HonkVerifier, CRISPProgram, RiscZeroGroth16Verifier, and MockRISC0Verifier in the deployment metadata. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
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 (1)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (1)
58-65: Consider using deployment receipts (and/or helpers) for more precise block numbersThe new
blockNumber: await ethers.provider.getBlockNumber()fields are functionally fine and will store the latest mined block at the time eachstoreDeploymentArgscall runs. However, two small improvements are worth considering:
Precision of “deployment block”
getBlockNumber()returns the current head of the chain on the connected provider, not necessarily the block that mined the specific deployment transaction.(eeth.dev)
If the goal is to record the actual deployment block for each contract (HonkVerifier, CRISPProgram, RiscZeroGroth16Verifier, MockRISC0Verifier), consider instead:
- Grabbing the deployment transaction via
contract.deploymentTransaction()and waiting on its receipt, then- Storing
receipt.blockNumberas the deployment block in your metadata.This avoids any drift if additional blocks are mined between deployment and the metadata write.
Minor RPC efficiency / consistency
EachstoreDeploymentArgscall independently awaitsethers.provider.getBlockNumber(). That’s cheap, but if you end up doing more of this, it may be cleaner to:
- Capture the relevant block number right after each deployment (or from the receipt as above), and
- Pass the captured value into
storeDeploymentArgs, rather than calling the provider again inside the object literal.These are refinements rather than correctness issues, so fine to defer, but they’ll make the recorded
blockNumbermore robust and self-explanatory in the long run.Also applies to: 78-91, 127-134, 147-154
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-11T13:09:03.800Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.800Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts
⏰ 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: build_e3_support_risc0
- GitHub Check: crisp_unit
- GitHub Check: integration_prebuild
- GitHub Check: build_sdk
- GitHub Check: build_enclave_cli
- GitHub Check: rust_integration
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: test_net
🔇 Additional comments (1)
README.md (1)
388-389: Non-semantic line reflow looks goodThis is purely formatting (line wrap) around the
mainbranch + feature flags sentence; content and meaning are unchanged. ✅
ctrlc03
force-pushed
the
chore/store-block-numbers-crisp
branch
from
5381ce1 to
cd85da1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (2)
55-65: Retrieve block number from deployment transaction receipt for accuracy.The current implementation retrieves the block number from the current chain state after deployment, which may result in a block number that's later than the actual deployment block—especially on fast networks or during high traffic. Additionally, there's no
waitForDeployment()call to ensure the transaction is mined.Apply this diff to retrieve the accurate deployment block number:
const honkVerifierFactory = await ethers.getContractFactory('HonkVerifier', { libraries: { 'project/contracts/CRISPVerifier.sol:ZKTranscriptLib': zkTranscriptLibAddress, }, }) const honkVerifier = await honkVerifierFactory.deploy() +await honkVerifier.waitForDeployment() const honkVerifierAddress = await honkVerifier.getAddress() +const deploymentTx = honkVerifier.deploymentTransaction() +const receipt = await deploymentTx?.wait() storeDeploymentArgs( { address: honkVerifierAddress, - blockNumber: await ethers.provider.getBlockNumber(), + blockNumber: receipt?.blockNumber, }, 'HonkVerifier', chain, )
75-91: Retrieve block number from deployment transaction receipt for accuracy.Similar to the HonkVerifier deployment, the block number is retrieved from the current chain state without waiting for deployment confirmation. This could result in storing an inaccurate block number.
Apply this diff to retrieve the accurate deployment block number:
const crisp = await crispFactory.deploy(enclaveAddress, verifier, honkVerifierAddress, IMAGE_ID) - +await crisp.waitForDeployment() const crispAddress = await crisp.getAddress() +const deploymentTx = crisp.deploymentTransaction() +const receipt = await deploymentTx?.wait() + storeDeploymentArgs( { address: crispAddress, - blockNumber: await ethers.provider.getBlockNumber(), + blockNumber: receipt?.blockNumber, constructorArgs: { enclave: enclaveAddress, verifierAddress: verifier, honkVerifierAddress, imageId: IMAGE_ID, }, }, 'CRISPProgram', chain, )
🧹 Nitpick comments (2)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts (2)
122-135: Consider retrieving block number from deployment transaction receipt.While this implementation correctly waits for deployment, it still retrieves the block number from the current chain state rather than from the deployment transaction receipt. For maximum accuracy, consider using the transaction receipt.
Apply this diff to retrieve the block number from the transaction receipt:
const verifierFactory = await ethers.getContractFactory('RiscZeroGroth16Verifier') const verifier = await verifierFactory.deploy() await verifier.waitForDeployment() const address = await verifier.getAddress() +const deploymentTx = verifier.deploymentTransaction() +const receipt = await deploymentTx?.wait() storeDeploymentArgs( { address, - blockNumber: await ethers.provider.getBlockNumber(), + blockNumber: receipt?.blockNumber, }, 'RiscZeroGroth16Verifier', chain, )
143-154: Consider retrieving block number from deployment transaction receipt.Similar to the RiscZeroGroth16Verifier deployment, this correctly waits for deployment but retrieves the block number from current chain state. Using the transaction receipt would provide more accurate block numbers.
Apply this diff to retrieve the block number from the transaction receipt:
const mockVerifierFactory = await ethers.getContractFactory('MockRISC0Verifier') const mockVerifier = await mockVerifierFactory.deploy() await mockVerifier.waitForDeployment() const mockVerifierAddress = await mockVerifier.getAddress() +const deploymentTx = mockVerifier.deploymentTransaction() +const receipt = await deploymentTx?.wait() + storeDeploymentArgs( { address: mockVerifierAddress, - blockNumber: await ethers.provider.getBlockNumber(), + blockNumber: receipt?.blockNumber, }, 'MockRISC0Verifier', hre.globalOptions.network, )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-11T13:09:03.800Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.800Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
examples/CRISP/packages/crisp-contracts/deploy/crisp.ts
⏰ 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). (8)
- GitHub Check: crisp_unit
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: build_enclave_cli
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: integration_prebuild
- GitHub Check: rust_integration
ctrlc03
force-pushed
the
chore/store-block-numbers-crisp
branch
from
cd85da1 to
59dbbbb
Compare
2 participants
Store block numbers for crisp contracts
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.