feat(multi): add check redirect endpoint #1563
Conversation
WalkthroughThis PR adds a redirect-check feature to the multichain aggregator. Changes include: new protobuf RPC and messages ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
190-223: Redirect model + conversion toCheckRedirectResponselook correct and frontend‑friendlyUsing
#[derive(strum::Display)]with#[strum(serialize_all = "lowercase")]onRedirectTypeensures thetypestring matches the expected"block" | "transaction" | "address"shape, and theFrom<Redirect>impl always setsredirect = truewith optionalparameter/chain_id, leaving the “no redirect” case to the default response.If you anticipate logging or reusing
Redirect/RedirectTypein tests, consider addingDebug/Clonederives to both types for convenience; otherwise this minimal API is sufficient. Please also confirm that consumers expecting lowercase type strings (e.g., frontend proxyResponse types) are aligned with this serialization.multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (1)
119-126: CentralizingSearchContextconstruction for quick search and redirects is a solid cleanupThe new
search_context(&self, is_aggregated)helper cleanly encapsulatesSearchContextwiring (cluster reference, cloned DB wrapped inArc, anddomain_primary_chain_id), and reusing it in bothquick_searchandcheck_redirectkeeps those flows consistent and avoids duplicating setup logic.If
SearchContextusage expands further, you could consider storingArc<DatabaseConnection>directly onClusterto avoid repeatedArc::new(self.db.clone()), but that’s a micro-optimization and not required now.Also applies to: 855-876
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (1)
254-260: Use limit=1 for efficiency in Domain redirect search.The Domain search uses
QUICK_SEARCH_NUM_ITEMS(50) but only needs the first result via.next(). This fetches unnecessary data.Apply this diff to improve efficiency:
SearchTerm::Domain(query) => search_context .cluster .search_domains_cached( query, vec![search_context.domain_primary_chain_id], - QUICK_SEARCH_NUM_ITEMS, + 1, None, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
multichain-aggregator/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
multichain-aggregator/Cargo.toml(1 hunks)multichain-aggregator/multichain-aggregator-logic/Cargo.toml(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs(6 hunks)multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs(4 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs(1 hunks)multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml(1 hunks)multichain-aggregator/multichain-aggregator-proto/proto/v1/cluster-explorer.proto(2 hunks)multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml(2 hunks)multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-10T16:24:23.028Z
Learnt from: bragov4ik
Repo: blockscout/blockscout-rs PR: 1137
File: stats/stats/Cargo.toml:17-17
Timestamp: 2024-12-10T16:24:23.028Z
Learning: When a dependency like `trait-variant` is correctly defined under `[workspace.dependencies]` in the root `Cargo.toml`, it should not be flagged as improperly defined in workspace crates.
Applied to files:
multichain-aggregator/multichain-aggregator-logic/Cargo.toml
📚 Learning: 2025-09-25T16:24:01.081Z
Learnt from: lok52
Repo: blockscout/blockscout-rs PR: 1518
File: multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs:664-670
Timestamp: 2025-09-25T16:24:01.081Z
Learning: In multichain-aggregator token search caching, the cluster name (self.name) provides cluster-level cache isolation but does not prevent intra-cluster cache collisions when different chain_ids subsets are used with identical query parameters.
Applied to files:
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs
🧬 Code graph analysis (4)
multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs (3)
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (2)
check_redirect(872-876)new(74-110)multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (1)
check_redirect(68-78)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
new(205-211)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
multichain-search/frontend/types/proxyResponse.ts (3)
Block(52-57)Transaction(59-63)Address(45-50)
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (3)
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (5)
check_redirect(872-876)search_context(119-126)active_chain_ids(129-141)active_chain_ids(152-152)new(74-110)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (2)
address(109-120)new(205-211)multichain-aggregator/multichain-aggregator-logic/src/services/filecoin.rs (1)
try_filecoin_address_to_evm_address(6-31)
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (1)
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (3)
quick_search(24-66)try_parse_address(278-282)check_redirect(68-78)
⏰ 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). (2)
- GitHub Check: Linting / lint
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (13)
multichain-aggregator/Cargo.toml (1)
79-79: strum workspace dependency and features look correctly wiredDefining
strum = { version = "0.27", default-features = false, features = ["derive"] }at the workspace level matches the usage pattern in the logic crate (derive macros viastrum) and keeps default features off while enabling the proc-macro exports.If you haven’t already, please run
cargo tree -d strumto confirm there’s a single strum version and feature set in the workspace.multichain-aggregator/multichain-aggregator-logic/Cargo.toml (1)
34-34: Logic crate’s use of workspacestrumwith extrastdfeature is appropriateOpting into
strum = { workspace = true, features = ["std"] }here correctly augments the workspace-levelstrumdefinition withderiveso theDisplayderive insearch_results.rsworks under a unified feature set, without re-specifying version or defaults. Based on learnings.When you run CI or
cargo build -p multichain-aggregator-logic, please confirm there are no MSRV or feature-compat issues withstrum 0.27.multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml (1)
104-106: HTTP rule forCheckRedirectmatches proto and existing conventionsThe selector and path (
/api/v1/clusters/{cluster_id}/search:check-redirect) are consistent with theClusterExplorerServiceproto and mirror the existingsearch:quickmapping;qwill correctly flow as a query parameter from the request message.multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs (1)
354-369:check_redirecthandler correctly normalizesOption<Redirect>into the responseDelegating to
cluster.check_redirect(&inner.q)and using.map(|r| r.into()).unwrap_or_default()cleanly mapsSometo a populatedCheckRedirectResponseandNoneto the default “no redirect” payload, while reusing the existingServiceError→Statusconversion.multichain-aggregator/multichain-aggregator-proto/proto/v1/cluster-explorer.proto (1)
34-35:CheckRedirectRPC and its request/response types align with server and logic layersThe new
CheckRedirect(CheckRedirectRequest) returns (CheckRedirectResponse)method and message shapes (cluster_id/q in the request; redirect flag plus optional type/parameter/chain_id in the response) match the logic layer’sRedirectconversion and the server’sClusterExplorer::check_redirectimplementation.Also applies to: 327-337
multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (1)
666-688: Swagger path andv1CheckRedirectResponseschema are consistent with the protoThe
/api/v1/clusters/{cluster_id}/search:check-redirectoperation (withcluster_idpath and optionalqquery) and thev1CheckRedirectResponsedefinition accurately reflect theCheckRedirectRequest/CheckRedirectResponseproto messages and the server handler’s behavior.Also applies to: 1471-1481
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (1)
578-626: UsingSearchTerm::try_parse_addressunifies address parsing across search flowsSwitching
prepare_addresses_querytoSearchTerm::try_parse_address(&query)ensures address detection for address/domain/contract-name searches matches the same heuristics used by quick search and redirect resolution, while preserving the existing domain-with-TLD and contract-name fallback behavior.multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (6)
8-13: LGTM! Imports updated correctly for redirect functionality.The new imports for
RedirectandRedirectTypeare necessary for the redirect-check capability introduced in this PR.
68-78: LGTM! Clean implementation of redirect-check capability.The function correctly delegates parsing and redirect resolution to helper methods, with appropriate error handling.
237-239: Verify the optimistic redirect behavior for AddressHash.Unlike
HashandDomaincases,AddressHashcreates a redirect without verifying the address exists in the cluster. This could redirect to non-existent addresses.If this optimistic behavior is intentional (e.g., addresses conceptually always exist), consider adding a comment explaining the design decision. Otherwise, consider checking address existence similar to the other cases.
274-286: LGTM! Good extraction of parsing utilities.The new public parsing helpers improve code reusability and maintain existing functionality (including Filecoin address support). The
try_*naming convention clearly indicates fallible operations.
289-307: Verify the Domain catch-all behavior and BlockNumber exclusion.This function treats any query ≥
MIN_QUERY_LENGTHthat's not a hash or address as aDomain. This means:
- Numeric queries like
"1234"are treated as domains for redirects (not block numbers)BlockNumberis excluded from redirect logic entirelyThis differs from
parse_search_terms(Line 324) which would parse"1234"as aBlockNumber. Confirm this is the intended redirect behavior.If intentional, consider adding a doc comment explaining that redirects only support Hash, Address, and Domain types.
309-337: LGTM! Good refactoring to use parsing helpers.The function now reuses the extracted parsing utilities, improving maintainability while preserving existing behavior.
bragov4ik
left a comment
bragov4ik
left a comment
There was a problem hiding this comment.
I'm not really in the context of what's the meaning of redirect and checking, but LGTM :)
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs
Show resolved
Hide resolved
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (2)
701-723: Add enum constraint to thetypefield and document response fields.The endpoint definition looks structurally sound. However, the response schema needs refinement:
typefield should define valid enum values (Block, Transaction, Address per the PR summary) rather than leaving it as a bare string. This clarifies the API contract for consumers.Missing descriptions: All response fields lack descriptions. Add brief docs explaining what each field represents and when it's populated (e.g., whether type/parameter/chain_id are only present when redirect=true).
/api/v1/clusters/{cluster_id}/search:check-redirect: get: operationId: ClusterExplorerService_CheckRedirect responses: "200": description: A successful response. schema: $ref: '#/definitions/v1CheckRedirectResponse'
1518-1528: Define enum constraint fortypefield and add field descriptions.The response schema definition should document each field and constrain the
typefield to valid redirect type values:v1CheckRedirectResponse: type: object properties: redirect: type: boolean + description: Whether the search query should redirect to a specific entity. type: type: string + description: The type of entity to redirect to (Block, Transaction, or Address). Only present if redirect is true. + enum: + - Block + - Transaction + - Address parameter: type: string + description: The parsed parameter value (e.g., block number, transaction hash, or address). Only present if redirect is true. chain_id: type: string + description: The chain ID for the redirect target. Only present if redirect is true.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
multichain-aggregator/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
multichain-aggregator/Cargo.toml(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs(6 hunks)multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml(1 hunks)multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml(2 hunks)multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T16:24:01.081Z
Learnt from: lok52
Repo: blockscout/blockscout-rs PR: 1518
File: multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs:664-670
Timestamp: 2025-09-25T16:24:01.081Z
Learning: In multichain-aggregator token search caching, the cluster name (self.name) provides cluster-level cache isolation but does not prevent intra-cluster cache collisions when different chain_ids subsets are used with identical query parameters.
Applied to files:
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs
🧬 Code graph analysis (2)
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (2)
multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (3)
quick_search(24-66)try_parse_address(278-282)check_redirect(68-78)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (2)
new(205-211)address(109-120)
multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs (3)
multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (2)
check_redirect(873-877)new(75-111)multichain-aggregator/multichain-aggregator-logic/src/services/quick_search.rs (1)
check_redirect(68-78)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
new(205-211)
⏰ 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). (1)
- GitHub Check: Docker build and docker push / build-and-push
🔇 Additional comments (7)
multichain-aggregator/Cargo.toml (1)
80-80: The strum dependency addition is well-placed and uses appropriate feature flags. The latest stable version is 0.27.2, so the specified 0.27 constraint is current, and there are no security advisories for strum.multichain-aggregator/multichain-aggregator-logic/src/services/cluster.rs (5)
18-18: LGTM: Import additions support new redirect functionality.The new imports (SearchContext, SearchTerm, Redirect) align with the redirect-checking feature introduced in this PR.
Also applies to: 30-30
120-127: LGTM: Helper method reduces duplication.The
search_contexthelper centralizesSearchContextconstruction, improving maintainability. The Arc wrapping of the database connection aligns with the SearchContext API requirements shown in the relevant snippets.
589-589: LGTM: Centralized address parsing improves consistency.Using
SearchTerm::try_parse_addressconsolidates address parsing logic (including Filecoin address support) in one place, as shown in the relevant snippets.
862-862: LGTM: Refactored to use the new search_context helper.This reduces code duplication and improves maintainability.
873-877: Confirm the intentional design of hardcodedis_aggregated=falseincheck_redirect.The method hardcodes
is_aggregated=false, ensuring redirects always flatten aggregated addresses. This differs fromquick_search, which parameterizes this behavior. Verify this design choice is intentional and document why redirects should not support aggregation modes.multichain-aggregator/multichain-aggregator-server/src/services/cluster_explorer.rs (1)
353-368: No action needed—input handling and default response semantics are correct.The query string is properly validated: it's trimmed at both the
check_redirectentry point (query.trim()) and withinparse_redirect_search_term, which returnsNonefor empty or too-short inputs. This prevents invalid queries from propagating downstream.The default response semantics are sound:
CheckRedirectResponsedefaults toredirect: falsewith optional fields empty, which correctly represents "no redirect available" whenOption::Noneis encountered. TheFrom<Redirect>implementation setsredirect: trueand populates the optional type, parameter, and chain_id fields only when a redirect exists.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.