chore: add endpoints to filter round data by requester#1152
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
📝 WalkthroughWalkthroughAdds requester-based filtering: new client env var and constant; client hooks now POST requester payloads. Server models, repository, and routes accept requester(s) and return requester-scoped round and result data. Endpoints Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant EnclaveAPI as Enclave Server (HTTP)
participant Repo as CurrentRoundRepository
participant CrispRepo as CrispE3Repository
Note over Client,EnclaveAPI: Client POSTs requesters payload
Client->>EnclaveAPI: POST /rounds/current { requesters: [...] }
alt requester provided
EnclaveAPI->>Repo: get_current_round_for_requester(requester)
Repo->>CrispRepo: iterate rounds -> fetch lite state per round
CrispRepo-->>Repo: lite state / errors
Repo-->>EnclaveAPI: Option<CurrentRound>
else no requester
EnclaveAPI->>Repo: get_current_round()
Repo-->>EnclaveAPI: Option<CurrentRound>
end
EnclaveAPI-->>Client: 200 OK / 404 NotFound
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Fix all issues with AI agents
In @examples/CRISP/client/src/utils/constants.ts:
- Line 8: ROUND_REQUESTERS currently splits VITE_E3_REQUESTERS by commas but
doesn't trim entries, so addresses like "0x123, 0x456" will include whitespace;
update the initialization of ROUND_REQUESTERS to map over the split results and
call .trim() on each entry and filter out any empty strings (i.e., use
import.meta.env.VITE_E3_REQUESTERS.split(',').map(s =>
s.trim()).filter(Boolean)) so stored addresses have no leading/trailing
whitespace.
In @examples/CRISP/server/src/server/routes/rounds.rs:
- Around line 77-91: The handler get_current_round accepts
RoundRequestWithRequester with a Vec<String> but only uses the first element;
either change the API/model to accept a single optional requester (e.g.,
RoundRequestWithRequester.requester: Option<String>) and update the handler to
use that value, or implement true multi-requester support by iterating over
incoming.requesters and applying OR/combined logic before calling the store;
also remove the unnecessary clone by changing get_current_round_for_requester to
accept &str or &String (or pass a string slice) so you can borrow instead of
cloning.
In @examples/CRISP/server/src/server/routes/state.rs:
- Line 15: Remove the unused import of the Encodable trait from rlp: in the use
statement that currently imports alloy::{primitives::{Address, Bytes, U256},
rlp::Encodable}, delete the rlp::Encodable part so only Address, Bytes, and U256
are imported; ensure no other code in this module references Encodable before
removing.
🧹 Nitpick comments (3)
examples/CRISP/client/.env.example (1)
6-7: Consider reordering environment variables alphabetically.Static analysis suggests ordering VITE_E3_REQUESTERS before VITE_ENCLAVE_API for consistency. While this doesn't affect functionality, alphabetical ordering can improve maintainability in environment files.
examples/CRISP/server/src/server/repo.rs (1)
54-76: Consider indexing by requester for improved performance.The linear scan through all rounds works correctly but could become slow as the number of rounds grows. The reverse iteration optimizes for the common case (finding recent rounds), which is good.
For future optimization, consider maintaining an index mapping requesters to their round IDs to avoid the O(n) scan.
examples/CRISP/server/src/server/routes/state.rs (1)
222-257: LGTM with optional optimization.The filtering logic correctly implements requester-based filtering. The approach of returning all results when
requestersis empty is intuitive.For scenarios with many requesters, consider converting
requestersto aHashSetbefore the loop to improve lookup performance from O(n) to O(1) per check.♻️ Optional optimization using HashSet
async fn get_all_round_results(data: web::Json::<RoundRequestWithRequester>, store: web::Data<AppData>) -> impl Responder { let incoming = data.into_inner(); let round_count = match store.current_round().get_current_round_id().await { Ok(count) => count, Err(e) => { info!("Error retrieving round count: {:?}", e); return HttpResponse::InternalServerError().body("Failed to retrieve round count"); } }; let mut states = Vec::new(); let requesters = incoming.requesters; + let requester_set: std::collections::HashSet<_> = requesters.iter().collect(); // FIXME: This assumes ids are ordered for i in 0..round_count + 1 { match store.e3(i).get_web_result_request().await { Ok(w) => { if !requesters.is_empty() { // if we have any requesters to filter by, do it - if requesters.contains(&w.requester) { + if requester_set.contains(&w.requester) { states.push(w); } } else { states.push(w); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/CRISP/client/.env.exampleexamples/CRISP/client/src/hooks/enclave/useEnclaveServer.tsexamples/CRISP/client/src/utils/constants.tsexamples/CRISP/server/src/server/models.rsexamples/CRISP/server/src/server/repo.rsexamples/CRISP/server/src/server/routes/rounds.rsexamples/CRISP/server/src/server/routes/state.rs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2024-11-05T06:48:58.177Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-11-05T06:51:46.701Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/evm/src/event_reader.rs:270-286
Timestamp: 2024-11-05T06:51:46.701Z
Learning: In `packages/ciphernode/evm/src/event_reader.rs`, the `state.ids` HashSet is intended to grow indefinitely to store all processed event IDs for deduplication purposes until an alternative like a bloom filter is implemented.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
🧬 Code graph analysis (5)
examples/CRISP/server/src/server/models.rs (1)
examples/CRISP/server/src/server/indexer.rs (1)
e3(340-340)
examples/CRISP/server/src/server/routes/rounds.rs (1)
examples/CRISP/server/src/server/repo.rs (3)
get_current_round(36-45)store(38-40)store(113-115)
examples/CRISP/server/src/server/repo.rs (1)
examples/CRISP/client/src/model/vote.model.ts (1)
CurrentRound(12-14)
examples/CRISP/server/src/server/routes/state.rs (1)
examples/CRISP/server/src/server/repo.rs (2)
store(38-40)store(113-115)
examples/CRISP/client/src/hooks/enclave/useEnclaveServer.ts (3)
examples/CRISP/client/src/model/vote.model.ts (2)
CurrentRound(12-14)VoteStateLite(42-57)examples/CRISP/client/src/utils/constants.ts (1)
ROUND_REQUESTERS(8-8)examples/CRISP/client/src/model/poll.model.ts (1)
PollRequestResult(22-30)
🪛 dotenv-linter (4.0.0)
examples/CRISP/client/.env.example
[warning] 7-7: [UnorderedKey] The VITE_E3_REQUESTERS key should go before the VITE_ENCLAVE_API key
(UnorderedKey)
⏰ 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_enclave_cli
- GitHub Check: build_e3_support_dev
- GitHub Check: test_net
- GitHub Check: build_sdk
- GitHub Check: build_crisp_sdk
- GitHub Check: rust_unit
- GitHub Check: integration_prebuild
- GitHub Check: rust_integration
- GitHub Check: Build & Push Image
🔇 Additional comments (5)
examples/CRISP/server/src/server/routes/rounds.rs (1)
25-25: Breaking change: GET to POST is correctly coordinated with client updates.The route change from GET to POST is a breaking change for API consumers. The client-side hooks in
examples/CRISP/client/src/hooks/enclave/useEnclaveServer.tshave been updated accordingly, ensuring coordination across the stack.examples/CRISP/server/src/server/models.rs (2)
108-111: Model structure is consistent with usage patterns.The
RoundRequestWithRequesterstruct withVec<String>aligns with the route handlers inexamples/CRISP/server/src/server/routes/rounds.rs. However, note the comment on rounds.rs regarding whether aVec<String>is the most appropriate type when only the first element is used.
164-164: Properly extends WebResultRequest with requester field.The addition of the
requesterfield and its corresponding mapping in theFrom<E3>implementation correctly propagates requester information through the request flow.Also applies to: 254-254
examples/CRISP/client/src/hooks/enclave/useEnclaveServer.ts (1)
19-19: Client hooks correctly updated to POST requesters payload.The changes properly align with the server-side API modifications:
- Import of
ROUND_REQUESTERSprovides the requester addresses from environment configuration- Both
getCurrentRoundandgetWebResultcorrectly POST the requesters array- Type signatures accurately reflect the new payload structure
The implementation is sound, though it inherits the potential whitespace issue from
ROUND_REQUESTERSin constants.ts (addressed in that file's review).Also applies to: 39-39, 42-43
examples/CRISP/server/src/server/repo.rs (1)
243-256: LGTM!The addition of the
requesterfield toWebResultRequestis straightforward and correctly sources the data frome3_crisp.requester.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
examples/CRISP/client/src/utils/constants.ts (1)
8-10: Add filtering to remove empty strings.While
.trim()correctly handles whitespace (addressing the previous review comment), the implementation should also filter out empty strings that can result from trailing commas ("0x123,"), consecutive commas ("0x123,,0x456"), or leading commas. Empty strings in the array could cause validation failures or unexpected filtering behavior.♻️ Proposed fix
export const ROUND_REQUESTERS = import.meta.env.VITE_E3_REQUESTERS - ? import.meta.env.VITE_E3_REQUESTERS.split(',').map((s: string) => s.trim()) + ? import.meta.env.VITE_E3_REQUESTERS.split(',').map((s: string) => s.trim()).filter(Boolean) : []examples/CRISP/server/src/server/routes/state.rs (2)
217-221: Update function documentation.The documentation doesn't mention the new requester-based filtering capability. Update it to describe the
RoundRequestWithRequesterparameter and the filtering behavior.📝 Suggested documentation update
/// Get all the results for all rounds /// +/// # Arguments +/// +/// * `RoundRequestWithRequester` - Optional list of requester addresses to filter results +/// /// # Returns /// /// * A JSON response containing the results for all rounds +/// If requesters are provided, only returns results matching those requesters +/// If no requesters are provided, returns all results
222-256: Consider address normalization for requester matching.The filtering logic is correct, but the string comparison at line 242 is case-sensitive. For Ethereum addresses, this could cause mismatches if the addresses in the payload use different casing than those stored in the results. Consider normalizing addresses to lowercase (or using checksummed addresses consistently throughout) to ensure reliable matching.
♻️ Example normalization approach
async fn get_all_round_results(data: web::Json::<RoundRequestWithRequester>, store: web::Data<AppData>) -> impl Responder { let incoming = data.into_inner(); let round_count = match store.current_round().get_current_round_id().await { Ok(count) => count, Err(e) => { info!("Error retrieving round count: {:?}", e); return HttpResponse::InternalServerError().body("Failed to retrieve round count"); } }; let mut states = Vec::new(); - let requesters = incoming.requesters; + // Normalize addresses to lowercase for case-insensitive comparison + let requesters: Vec<String> = incoming.requesters.iter().map(|r| r.to_lowercase()).collect(); // FIXME: This assumes ids are ordered for i in 0..round_count + 1 { match store.e3(i).get_web_result_request().await { Ok(w) => { if !requesters.is_empty() { // if we have any requesters to filter by, do it - if requesters.contains(&w.requester) { + if requesters.contains(&w.requester.to_lowercase()) { states.push(w); } } else { states.push(w); } } Err(e) => { info!("Error retrieving state for round {}: {:?}", i, e); continue; } } } HttpResponse::Ok().json(states) }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/CRISP/client/src/utils/constants.tsexamples/CRISP/server/src/server/routes/state.rs
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-11-05T06:48:58.177Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-11-05T06:49:46.285Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/enclave_node/src/datastore.rs:14-16
Timestamp: 2024-11-05T06:49:46.285Z
Learning: In `packages/ciphernode/enclave_node/src/datastore.rs`, for internal functions like `get_in_mem_store`, adding extensive documentation and error handling may not be necessary if they are not client-facing.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-10-16T09:51:10.038Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/router/src/committee_meta.rs:43-43
Timestamp: 2024-10-16T09:51:10.038Z
Learning: In `packages/ciphernode/router/src/committee_meta.rs`, avoid suggesting making the `on_event` method in `CommitteMetaFeature` asynchronous or adding error handling for the `write` operation to the data store.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
📚 Learning: 2024-11-05T06:51:46.701Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/evm/src/event_reader.rs:270-286
Timestamp: 2024-11-05T06:51:46.701Z
Learning: In `packages/ciphernode/evm/src/event_reader.rs`, the `state.ids` HashSet is intended to grow indefinitely to store all processed event IDs for deduplication purposes until an alternative like a bloom filter is implemented.
Applied to files:
examples/CRISP/server/src/server/routes/state.rs
🧬 Code graph analysis (1)
examples/CRISP/server/src/server/routes/state.rs (1)
examples/CRISP/server/src/server/repo.rs (2)
store(38-40)store(113-115)
⏰ 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_dev
- GitHub Check: build_crisp_sdk
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: rust_integration
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (2)
examples/CRISP/server/src/server/routes/state.rs (2)
10-12: LGTM!The import changes correctly add
RoundRequestWithRequesterto support the new filtering functionality.
26-26: Client is already updated to use POST with the requester payload.The
/state/allendpoint uses POST and the client atexamples/CRISP/client/src/hooks/enclave/useEnclaveServer.ts(line 43) is already calling it with the correct'post'method and{ requesters: ROUND_REQUESTERS }payload. The server and client changes are coordinated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/CRISP/scripts/dev_client.sh (1)
7-7: LGTM! The .env file check is a helpful addition.The logic correctly ensures
.envexists before starting the dev server, which is essential now that the client relies onVITE_E3_REQUESTERSfor requester filtering. The implementation won't overwrite existing developer customizations.Optional: Consider adding user feedback for better developer experience.
While not essential, adding a brief message when the file is copied would help developers understand what's happening, and explicitly checking for
.env.examplewould provide a clearer error if it's missing.💡 Optional enhancement for better DX
-(cd ./client && if [[ ! -f .env ]]; then cp .env.example .env; fi && pnpm dev-static) +(cd ./client && \ + if [[ ! -f .env ]]; then \ + if [[ ! -f .env.example ]]; then \ + echo "Error: .env.example not found in client directory" >&2; \ + exit 1; \ + fi; \ + echo "Creating .env from .env.example..."; \ + cp .env.example .env; \ + fi && \ + pnpm dev-static)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/CRISP/scripts/dev_client.sh
⏰ 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: build_sdk
- GitHub Check: build_crisp_sdk
- GitHub Check: rust_integration
- GitHub Check: test_net
- GitHub Check: rust_unit
- GitHub Check: build_e3_support_dev
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
2 participants
add requesters array filtering to frontend and server endpoints
Summary by CodeRabbit
New Features
Configuration
API Changes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.