feat: add onchain proofs to events#1484
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR extends two core event signatures across the Enclave system by adding ZK proof parameters: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
agent/flow-trace/04_DKG_AND_COMPUTATION.md (2)
482-509:⚠️ Potential issue | 🟡 MinorFix the documented
publishCommitteecall shape.Line 483 and Line 487 still describe
publishCommittee(..., pkHash), but the ABI checked in with this PR now expects(e3Id, nodes, publicKey, proof, foldProof). The event line was updated to carry the C5 proof, so the surrounding flow should show where both proof blobs are supplied too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent/flow-trace/04_DKG_AND_COMPUTATION.md` around lines 482 - 509, Update the flow text to reflect the corrected publishCommittee signature and where proofs are supplied: change references to publishCommittee(e3Id, nodes, pkHash) to publishCommittee(e3Id, nodes, publicKey, proof, foldProof) (as invoked by CiphernodeRegistrySolWriter) and ensure the CommitteePublished emission and surrounding steps note that the C5 proof and foldProof are passed into the contract call and included in the emitted event alongside the full publicKey; also adjust the Enclave.onCommitteePublished / stored fields description to remain accurate (public key stored as pkHash) while documenting that the proofs are supplied to the on-chain publish call and event.
677-709:⚠️ Potential issue | 🟡 MinorThis plaintext publication flow still omits
foldProof.Line 678 and Line 682 document a 3-arg
publishPlaintextOutputcall, whilepackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonnow defines(e3Id, plaintextOutput, proof, foldProof). Please sync this section so the flow doc matches the contract ABI and the earlier C7a/C7b discussion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent/flow-trace/04_DKG_AND_COMPUTATION.md` around lines 677 - 709, The flow doc's plaintext publication sequence references publishPlaintextOutput as a 3-arg call but the contract ABI (IEnclave.json) and earlier C7a/C7b discussion require a 4th foldProof parameter; update the diagram and text around EnclaveSolWriter / publishPlaintextOutput(e3Id, output, proof) to include foldProof (e.g., publishPlaintextOutput(e3Id, plaintextOutput, proof, foldProof)), ensure steps that mention decryptionVerifier.verify or the emitted PlaintextOutputPublished event reflect the additional foldProof argument, and align the narrative with the C7a/C7b fold proof usage so the doc matches the current contract interface.
🧹 Nitpick comments (1)
crates/evm-helpers/src/events.rs (1)
61-64: Consider single-sourcing these event ABIs.Adding
proofhere required matching edits in the contract artifacts and both fake Solidity fixtures. Since this file is still a hand-maintained ABI copy, the drift risk behind the TODO is already showing up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/evm-helpers/src/events.rs` around lines 61 - 64, The manually maintained event ABI in events.rs (the PlaintextOutputPublished and CommitteePublished event definitions) is drifting from the actual contract artifacts; replace the hand-written ABI with a single-sourced generation approach: remove the manual event declarations and instead derive/load the event ABIs from the canonical contract artifact (e.g., via abigen!/ethers-abi codegen or include_str! on the compiled contract JSON and parse it in a build script) so changes to events like the added proof field on PlaintextOutputPublished/CommitteePublished come from one source of truth and propagate to both runtime code and test fixtures automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@agent/flow-trace/04_DKG_AND_COMPUTATION.md`:
- Around line 482-509: Update the flow text to reflect the corrected
publishCommittee signature and where proofs are supplied: change references to
publishCommittee(e3Id, nodes, pkHash) to publishCommittee(e3Id, nodes,
publicKey, proof, foldProof) (as invoked by CiphernodeRegistrySolWriter) and
ensure the CommitteePublished emission and surrounding steps note that the C5
proof and foldProof are passed into the contract call and included in the
emitted event alongside the full publicKey; also adjust the
Enclave.onCommitteePublished / stored fields description to remain accurate
(public key stored as pkHash) while documenting that the proofs are supplied to
the on-chain publish call and event.
- Around line 677-709: The flow doc's plaintext publication sequence references
publishPlaintextOutput as a 3-arg call but the contract ABI (IEnclave.json) and
earlier C7a/C7b discussion require a 4th foldProof parameter; update the diagram
and text around EnclaveSolWriter / publishPlaintextOutput(e3Id, output, proof)
to include foldProof (e.g., publishPlaintextOutput(e3Id, plaintextOutput, proof,
foldProof)), ensure steps that mention decryptionVerifier.verify or the emitted
PlaintextOutputPublished event reflect the additional foldProof argument, and
align the narrative with the C7a/C7b fold proof usage so the doc matches the
current contract interface.
---
Nitpick comments:
In `@crates/evm-helpers/src/events.rs`:
- Around line 61-64: The manually maintained event ABI in events.rs (the
PlaintextOutputPublished and CommitteePublished event definitions) is drifting
from the actual contract artifacts; replace the hand-written ABI with a
single-sourced generation approach: remove the manual event declarations and
instead derive/load the event ABIs from the canonical contract artifact (e.g.,
via abigen!/ethers-abi codegen or include_str! on the compiled contract JSON and
parse it in a build script) so changes to events like the added proof field on
PlaintextOutputPublished/CommitteePublished come from one source of truth and
propagate to both runtime code and test fixtures automatically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ebd90a6f-c5cb-497e-bd71-2a8e14a0e0ca
📒 Files selected for processing (27)
agent/flow-trace/00_INDEX.mdagent/flow-trace/04_DKG_AND_COMPUTATION.mdagent/flow-trace/06_DEACTIVATION_AND_COMPLETION.mdcrates/events/src/enclave_event/committee_published.rscrates/events/src/enclave_event/plaintext_output_published.rscrates/evm-helpers/src/events.rscrates/evm-helpers/tests/fixtures/fake_enclave.solcrates/indexer/src/indexer.rscrates/indexer/tests/fixtures/fake_enclave.solcrates/indexer/tests/integration.rsdocs/pages/building-with-enclave.mdxdocs/pages/computation-flow.mdxpackages/enclave-contracts/artifacts/contracts/Enclave.sol/Enclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.jsonpackages/enclave-contracts/artifacts/contracts/registry/CiphernodeRegistryOwnable.sol/CiphernodeRegistryOwnable.jsonpackages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.jsonpackages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/contracts/interfaces/ICiphernodeRegistry.solpackages/enclave-contracts/contracts/interfaces/IEnclave.solpackages/enclave-contracts/contracts/registry/CiphernodeRegistryOwnable.solpackages/enclave-contracts/deployed_contracts.jsonpackages/enclave-contracts/test/Enclave.spec.tspackages/enclave-contracts/test/Registry/CiphernodeRegistryOwnable.spec.tspackages/enclave-sdk/src/events/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores