chore: store e3 requester#1151
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughThis PR adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Requester
participant Contract as Enclave Contract
participant Event as E3Requested Event
participant Indexer as Indexer
participant Server as CRISP Server
participant Repo as Repository
User->>Contract: request_e3(...)
activate Contract
Note over Contract: e3.requester = msg.sender
Contract->>Event: emit E3Requested(e3, ...)
deactivate Contract
Event->>Indexer: E3Requested event (includes e3.requester)
activate Indexer
Indexer->>Indexer: Parse and index E3 with requester
deactivate Indexer
Event->>Server: E3Requested event
activate Server
Server->>Contract: fetch full E3 details (contract.get_e3)
activate Contract
Contract-->>Server: return E3 {requester, ...}
deactivate Contract
Note over Server: Extract requester from E3
Server->>Repo: initialize_round(..., requester)
activate Repo
Repo->>Repo: Create E3Crisp with requester field
Repo-->>Server: E3Crisp initialized
deactivate Repo
deactivate Server
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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/server/src/server/indexer.rs (1)
49-78: Avoid redundant RPC call - useevent.e3.requesterdirectly.The
E3Requestedevent already includes the complete E3 struct with therequesterfield. The additionalcontract.get_e3()call introduces unnecessary RPC overhead. Other parts of this handler already useevent.e3directly (lines 61, 100).🔎 Proposed fix to eliminate redundant contract call
- let contract = ctx.contract(); - info!("[e3_id={}] E3Requested: {:?}", e3_id, event); async move { - let e3 = contract.get_e3(event.e3Id).await?; - // Convert custom params bytes back to token address and balance threshold. // Use sol_data types instead of primitives @@ -75,7 +71,7 @@ .with_context(|| "Invalid token address")?; // save the e3 details - repo.initialize_round(custom_params.token_address, custom_params.balance_threshold, e3.requester.to_string()) + repo.initialize_round(custom_params.token_address, custom_params.balance_threshold, event.e3.requester.to_string()) .await?;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
crates/evm-helpers/src/contracts.rscrates/evm-helpers/src/events.rscrates/indexer/src/indexer.rscrates/indexer/src/models.rsexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.solexamples/CRISP/server/src/server/indexer.rsexamples/CRISP/server/src/server/models.rsexamples/CRISP/server/src/server/repo.rspackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonpackages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/contracts/interfaces/IE3.sol
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/core/src/events.rs:323-323
Timestamp: 2024-10-08T01:50:45.185Z
Learning: When suggesting that instances of `E3Requested` should include `src_chain_id`, double-check to ensure that the field is actually missing before making the suggestion.
📚 Learning: 2024-10-22T03:47:04.014Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/router/src/e3_request_router.rs:202-235
Timestamp: 2024-10-22T03:47:04.014Z
Learning: In `packages/ciphernode/router/src/e3_request_router.rs`, within the `E3RequestRouter::from_snapshot` method, errors during context restoration propagate upwards due to the `?` operator, and skipping contexts when `repositories.context(&e3_id).read().await?` returns `Ok(None)` is acceptable, as missing snapshots are expected.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 Learning: 2024-10-22T03:44:33.301Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/router/src/e3_request_router.rs:126-133
Timestamp: 2024-10-22T03:44:33.301Z
Learning: In `E3RequestRouter::handle` method in `e3_request_router.rs`, `self.repository()` cannot fail, so it's not necessary to handle errors when accessing repositories via `self.repository().repositories()`.
Applied to files:
examples/CRISP/server/src/server/indexer.rs
📚 Learning: 2025-09-19T11:16:53.825Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 752
File: packages/enclave-contracts/contracts/Enclave.sol:15-17
Timestamp: 2025-09-19T11:16:53.825Z
Learning: The Enclave contract in the gnosisguild/enclave repository has not been deployed yet as of September 2025, so storage layout considerations for upgradeable contracts don't apply to current changes.
Applied to files:
packages/enclave-contracts/contracts/Enclave.sol
📚 Learning: 2025-12-23T17:18:22.421Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 1142
File: examples/CRISP/packages/crisp-contracts/contracts/CRISPProgram.sol:141-147
Timestamp: 2025-12-23T17:18:22.421Z
Learning: For Solidity contracts that verify Noir circuit proofs, the verifier's public inputs array must include both the circuit's public inputs and the public outputs. Ensure the verification input array length equals pub_inputs + pub_outputs. Example: a Noir circuit with 3 public inputs and 2064 public outputs should pass an array of length 2067 (3 + 2064). Apply this consistently across all contract verifier implementations and adjust tests accordingly.
Applied to files:
packages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/contracts/interfaces/IE3.solexamples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol
🧬 Code graph analysis (3)
crates/evm-helpers/src/contracts.rs (1)
crates/config/src/app_config.rs (1)
address(307-309)
examples/CRISP/server/src/server/indexer.rs (1)
crates/indexer/src/indexer.rs (1)
contract(185-187)
crates/evm-helpers/src/events.rs (2)
crates/evm-helpers/src/contracts.rs (1)
address(195-197)examples/CRISP/crates/evm_helpers/src/lib.rs (1)
address(150-152)
⏰ 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_crisp_sdk
- GitHub Check: build_sdk
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: build_ciphernode_image
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (13)
packages/enclave-contracts/contracts/Enclave.sol (1)
260-260: LGTM!Correctly captures the requester address as
msg.senderduring E3 creation, consistent with the PR objective to enable refunds and requester-based queries.packages/enclave-contracts/contracts/interfaces/IE3.sol (1)
30-47: LGTM!The
requesterfield is properly documented and correctly placed at the end of the struct. Based on learnings, storage layout compatibility isn't a concern since the contract hasn't been deployed yet.crates/evm-helpers/src/contracts.rs (1)
40-58: LGTM!The
requesterfield is correctly added to thesol!macro E3 struct, maintaining field order consistency with the Solidity contract definition inIE3.sol.crates/evm-helpers/src/events.rs (1)
28-45: LGTM!The
requesterfield is correctly added to the E3 struct for event parsing, maintaining consistency with the contract definition and enabling proper decoding ofE3Requestedevents.crates/indexer/src/models.rs (1)
29-29: LGTM!The
requesterfield is correctly added as aStringtype, consistent with how other address fields (e.g.,enclave_address) are stored in this model.crates/indexer/src/indexer.rs (1)
405-423: LGTM!The
requesterfield is correctly populated from the contract call result. UnlikeE3Requested, theE3Activatedevent only containse3Id,expiration, andcommitteePublicKey, so fetching the full E3 from the contract is necessary here.examples/CRISP/packages/crisp-contracts/contracts/Mocks/MockEnclave.sol (1)
40-41: LGTM!The
requesterfield is correctly added to the E3 struct literal, maintaining proper field ordering. Usingaddress(0)as a default value is appropriate for this mock contract.examples/CRISP/server/src/server/models.rs (3)
180-181: LGTM!The
requesterfield is correctly added toE3StateLitefor exposing the requester in lightweight state representations.
217-219: LGTM!Good addition of the
requesterfield with a descriptive comment clarifying its purpose.
235-235: LGTM!The
requesterfield is consistently added toE3Crisp, completing the model updates for the CRISP application.examples/CRISP/server/src/server/repo.rs (2)
225-243: LGTM!The
requesterfield is correctly propagated frome3_crispto theE3StateLiteresponse, ensuring the requester information is available in the lightweight state view.
133-154: No action needed—the existing call site atexamples/CRISP/server/src/server/indexer.rs:78already correctly passes therequesterparameter when callinginitialize_round.Likely an incorrect or invalid review comment.
packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (1)
195-200: LGTM!The ABI artifact correctly reflects the addition of the
requesterfield (typeaddress) to the E3 struct. The field is consistently added as the last member across all struct occurrences:E3Requestedevent,getE3output, andrequestoutput.Also applies to: 533-538, 809-814
2 participants
fix #1150
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.