feat(multi): domains integration#1318
Conversation
WalkthroughThis update applies a series of changes across multiple repositories and modules. In several Cargo.toml files, dependency declarations are refined by adding explicit version tags and integrating new dependencies like Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
multichain-aggregator/multichain-aggregator-logic/src/types/domains.rs (1)
37-46: ConvertDomainto protobuf.Field mappings are straightforward. However, using
.expect("...")can terminate the process if protocol deserialization fails. Consider handling parse failure more gracefully.For example, you could default to an empty JSON object if an unexpected error occurs:
- protocol: serde_json::from_value(v.protocol).expect("failed to deserialize protocol"), + protocol: serde_json::from_value(v.protocol).unwrap_or_default(),multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (1)
125-130: Passing BENS client and NonTokenSearch configuration.Using
bens_clientwith theNonTokenSearchconfig and setting a domain lookup limit of 1 is a practical initial approach. If broader TLD or multi-domain support is needed later, consider making the limit configurable or retrieving it from settings.multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (1)
30-33: TODO for chain-agnostic BENS handling.The comment indicates an upcoming requirement to generalize domain resolution. Consider tracking this in an issue or roadmap to ensure it’s not overlooked.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
blockscout-ens/Cargo.lockis excluded by!**/*.lockmultichain-aggregator/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
blockscout-ens/Cargo.toml(1 hunks)multichain-aggregator/Cargo.toml(1 hunks)multichain-aggregator/multichain-aggregator-logic/Cargo.toml(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/clients/bens.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/clients/mod.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/error.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs(2 hunks)multichain-aggregator/multichain-aggregator-logic/src/services/search.rs(9 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs(4 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/batch_import_request.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/domains.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/mod.rs(1 hunks)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs(4 hunks)multichain-aggregator/multichain-aggregator-proto/Cargo.toml(2 hunks)multichain-aggregator/multichain-aggregator-proto/build.rs(2 hunks)multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml(1 hunks)multichain-aggregator/multichain-aggregator-proto/proto/v1/multichain-aggregator.proto(5 hunks)multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml(6 hunks)multichain-aggregator/multichain-aggregator-server/src/server.rs(2 hunks)multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs(6 hunks)multichain-aggregator/multichain-aggregator-server/src/settings.rs(5 hunks)multichain-aggregator/multichain-aggregator-server/tests/quick_search.rs(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (2)
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (11)
addresses(118-121)addresses(423-426)addresses(428-437)addresses(438-442)addresses(494-503)addresses(530-533)block_ranges(456-462)domains(89-94)domains(517-517)hashes(198-201)hashes(400-403)blockscout-ens/graph-node/subgraphs/zeta-subgraph/generated/schema.ts (1)
Domain(14-294)
multichain-aggregator/multichain-aggregator-server/src/server.rs (4)
multichain-aggregator/multichain-aggregator-logic/src/clients/bens.rs (1)
new_client(5-8)multichain-aggregator/multichain-aggregator-logic/src/clients/dapp.rs (1)
new_client(9-12)multichain-aggregator/multichain-aggregator-logic/src/clients/token_info.rs (1)
new_client(10-13)multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (1)
new(28-47)
multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs (7)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
from(117-128)multichain-aggregator/multichain-aggregator-logic/src/types/token_info.rs (1)
from(36-45)multichain-aggregator/multichain-aggregator-logic/src/types/hashes.rs (2)
from(15-22)from(38-44)multichain-aggregator/multichain-aggregator-logic/src/types/dapp.rs (1)
from(28-36)multichain-aggregator/multichain-aggregator-logic/src/types/chains.rs (3)
from(15-24)from(28-35)from(39-46)multichain-aggregator/multichain-aggregator-logic/src/types/api_keys.rs (1)
from(33-38)multichain-aggregator/multichain-aggregator-logic/src/types/block_ranges.rs (3)
from(13-21)from(25-31)from(41-46)
multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (2)
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (7)
addresses(118-121)addresses(423-426)addresses(428-437)addresses(438-442)addresses(494-503)addresses(530-533)token_types(49-54)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
chain_ids(76-80)
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (5)
multichain-aggregator/multichain-aggregator-logic/src/clients/bens.rs (2)
query(33-61)query(89-99)multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
chain_ids(76-80)multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs (1)
try_from(49-61)multichain-aggregator/multichain-aggregator-logic/src/types/domains.rs (1)
try_from(15-34)multichain-aggregator/multichain-aggregator-logic/src/types/dapp.rs (1)
try_from(16-24)
multichain-aggregator/multichain-aggregator-logic/src/clients/bens.rs (2)
multichain-aggregator/multichain-aggregator-server/src/settings.rs (2)
default(57-62)default(76-108)libs/api-client-framework/src/endpoint.rs (1)
serialize_query(61-63)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (77)
multichain-aggregator/multichain-aggregator-logic/src/types/mod.rs (1)
7-7: LGTM: New domains module added successfullyThe addition of the
domainsmodule aligns with the PR objective of integrating domain functionality into the system.multichain-aggregator/multichain-aggregator-logic/src/error.rs (1)
34-35: LGTM: JSON parsing error variant addedThe addition of the
Jsonerror variant to handle JSON parsing errors is appropriate and follows the same pattern as existing error variants. This will be useful for processing domain-related JSON data.multichain-aggregator/multichain-aggregator-proto/Cargo.toml (2)
11-12: LGTM: Protocol Buffer dependencies addedThe addition of
prost-wktandprost-wkt-typesis appropriate for handling Well-Known Types in Protocol Buffers, which will be useful for the domains integration.
21-21: LGTM: Protocol Buffer build dependency addedThe addition of
prost-wkt-buildis consistent with the runtime dependencies and necessary for compiling Protocol Buffer definitions with Well-Known Types.multichain-aggregator/multichain-aggregator-server/src/settings.rs (5)
43-44: LGTM: Optional bens_protocols field with default valueAdding the
bens_protocolsfield with a default value is a good approach. This allows for configuration flexibility while providing sensible defaults.
96-98: LGTM: Default bens_client configurationThe default configuration for
bens_clientis consistent with other client defaults and provides a reasonable starting point.
105-105: LGTM: Default bens_protocols settingSetting the default
bens_protocolsvalue is consistent with the rest of the defaults configuration.
125-127: LGTM: Default ENS protocol implementationThe default protocols implementation currently includes only "ens", which is a reasonable starting point for domain resolution. As more domain protocols are supported, this list can be expanded.
32-35:Details
❓ Verification inconclusive
Good refactoring of client settings, but consider backward compatibility
Consolidating
DappClientSettingsandTokenInfoClientSettingsinto the genericHttpApiClientSettingstype is a good refactoring. The addition ofbens_clientfollows the same pattern.However, this change might affect existing configuration files. Have you considered backward compatibility or migration steps for existing deployments?
🏁 Script executed:
#!/bin/bash # Check if there are any existing configuration files that might be affected by the type change find . -name "*.yaml" -o -name "*.yml" -o -name "*.json" -o -name "*.toml" | xargs grep -l "dapp_client\|token_info_client"Length of output: 198
Action Required: Address Backward Compatibility for Client Settings
Great job on refactoring by consolidating
DappClientSettingsandTokenInfoClientSettingsinto the unifiedHttpApiClientSettings, along with adding the newbens_client. However, the discovery of an existing configuration file (multichain-aggregator/multichain-aggregator-server/config/example.toml) that referencesdapp_client(ortoken_info_client) suggests that deployments relying on older configuration settings may break.
- Review Configuration Files: Verify that all consumers of the configuration (e.g., example.toml) are either updated or have a clear migration path.
- Migration/Deprecation Strategy: Consider including migration steps, fallback mechanisms, or deprecation warnings to guide users through updating their configs.
Please address these concerns to ensure backward compatibility.
multichain-aggregator/multichain-aggregator-logic/src/clients/mod.rs (1)
1-1: Clean addition of the newbensmoduleThis addition of the
bensmodule integrates domain functionality into the client system. The module follows the established pattern of client module exports.multichain-aggregator/multichain-aggregator-logic/Cargo.toml (1)
9-9: Appropriate addition of thebens-protodependencyThe dependency is correctly added with
workspace = true, consistent with other dependencies in this project. This addition supports the new domain functionality being integrated.multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml (1)
37-39: Consistent addition of the new/api/v1/domainsendpointThe new HTTP endpoint for listing domains follows the same RESTful pattern as other endpoints in this service. The placement before the Health section maintains the logical organization of the API configuration.
multichain-aggregator/multichain-aggregator-logic/src/types/batch_import_request.rs (1)
56-56: Properly updated Address construction to use the new domain modelThe code has been updated to initialize
domain_info: Noneinstead of the previousens_namefield, reflecting the architectural shift from a simple string field to a more structured representation of domain information.multichain-aggregator/multichain-aggregator-server/src/server.rs (3)
16-16: Properly integrated the new bens client importThe import statement has been correctly updated to include the new
bensclient alongside the existingdappandtoken_infoclients.
72-72: Well-structured client initializationThe initialization of the
bens_clientfollows the same pattern as the existing clients, maintaining code consistency.
78-82: Appropriate integration of the bens client in the MultichainAggregator constructorBoth the new
bens_clientandbens_protocolsparameters have been correctly added to theMultichainAggregator::new()call, enabling the domain lookup functionality.multichain-aggregator/Cargo.toml (3)
16-16: Correctly referenced the bens-proto dependencyThe addition of the
bens-protodependency is appropriate for integrating the domain functionality.
28-30: Well-structured addition of prost-wkt dependenciesThe addition of the
prost-wkt,prost-wkt-types, andprost-wkt-builddependencies is appropriate for improved protocol buffer handling with well-known types.
20-20:Details
❓ Verification inconclusive
Verify if removing the 'conversions' feature is intentional
The
features = ["conversions"]attribute has been removed from theactix-prost-builddependency. Make sure this doesn't impact any functionality that might depend on these conversions.
🏁 Script executed:
#!/bin/bash # Check if the "conversions" feature is used elsewhere or if it's redundant # Search for references to actix-prost-build conversions rg -A 3 -B 3 "conversions" --glob "*.rs" --glob "*.toml" # Check if there are any compilation errors related to missing conversions cargo check -p multichain-aggregator-proto 2>&1 | grep -i conversionLength of output: 1655
ACTION: Verify that removing the 'conversions' feature does not introduce any hidden issues.
Based on the checks performed:
- The
cargo check -p multichain-aggregator-protocommand produced no conversion-related compilation errors.- The
rgcommand returned several references to "conversions" in unrelated modules (e.g., in tests and in other packages), but none of these clearly indicate a dependency on theactix-prost-buildconversions feature.It appears that the removal of
features = ["conversions"]from theactix-prost-builddependency is likely intentional. However, because there are some conversion-related references elsewhere in the codebase, please perform a manual verification to ensure that no functionality relying on the now-disabled conversions is impacted.multichain-aggregator/multichain-aggregator-server/tests/quick_search.rs (3)
23-28: Properly implemented mock bens serverThe mock server for domain lookups has been correctly integrated into the test setup, following the same pattern as the existing mock servers.
47-52: Comprehensive test cases for domain functionalityThe test assertions for both empty domains and a domain search query are well-structured and verify the expected behavior of the new functionality.
113-147: Well-structured mock implementation for bens serverThe
mock_bens_serverfunction is well-implemented with a complete and realistic response payload that includes all the necessary fields for domain information. The mock follows the same pattern as the other mock servers in this file.multichain-aggregator/multichain-aggregator-proto/build.rs (6)
3-7: Added appropriate imports for enhanced Protocol Buffer handlingThe new imports for
prost_wkt_buildand additional standard library modules are correctly added to support the enhanced Protocol Buffer compilation process.
15-16: Well-structured output directory configurationThe output directory and file descriptor set path are properly configured using environment variables.
21-21: Appropriately configured file descriptor set pathThe file descriptor set path is correctly added to the configuration, enabling additional protocol buffer processing.
29-29: Added proper serialization for Google Protocol Buffer typesThe type attribute for
.google.protobufenables proper serialization and deserialization withserde, which is essential for correctly handling Protocol Buffer messages.
43-44: Correctly configured extern path for Google Protocol Buffer typesThe
extern_pathconfiguration properly links the Google Protocol Buffer types to theprost-wkt-typescrate.
47-50: Well-implemented serde support for Protocol Buffer typesThe implementation to read the descriptor bytes, decode them, and add serde support for the generated types is correct and follows best practices.
multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs (6)
7-13: AddDomainInfostruct.This new struct cleanly encapsulates domain-related data like address, name, expiry date, and names count. It appears well-structured and complements the existing address data model.
19-19: Attachdomain_infotoAddress.Adding
domain_infoas an optional field inAddressaligns with evolving requirements to store richer domain information. This change looks coherent with the new domain-based approach.
33-33: Verify ENS name handling.Setting
ens_nametoNoneabandons any previously stored ENS data. If there's a requirement to preserve or migrate ENS information, consider verifying the impact on existing records or providing a migration path.
53-53: Clarify ignoringdomain_infofrom the database.Currently assigning
Nonetodomain_infomeans the existing database records do not store or load domain details. If domain metadata will be persisted in the future, consider extending the model accordingly.
64-73: ImplementFrom<DomainInfo>forproto::DomainInfo.This straightforward field mapping consistently follows the code’s existing conversion pattern for protobuf structs. Implementation looks well-aligned with best practices.
79-79: Mapdomain_infoto protobuf.Using
map(|d| d.into())is a neat way to conditionally convert domain info. This approach avoids boilerplate and neatly handles the optional field.blockscout-ens/Cargo.toml (1)
14-16: Pin dependencies to a specific tag.Specifying
tag = "v1.0.3"ensures reproducible builds and avoids unexpected upstream changes. This is a recommended practice for more predictable dependency management.multichain-aggregator/multichain-aggregator-logic/src/types/domains.rs (2)
1-10: Introduce newDomainstruct.This struct neatly groups domain-related attributes including address, name, expiry date, and protocol. It provides a sound foundation for supporting domain-centric features.
12-35: ImplementTryFrom<BensDomain>with error checks.The conversion logic properly validates missing fields using
ok_or_else, ensuring robust error handling. This design prevents invalid data from silently propagating.multichain-aggregator/multichain-aggregator-logic/src/clients/bens.rs (2)
1-8: LGTM: Good setup for API clientThe HTTP client setup and configuration initialization are properly implemented.
65-101: Good implementation of the GetAddress endpointClean implementation of the Endpoint trait for the GetAddress struct. The path construction and query parameter handling are done correctly.
multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (6)
122-149: LGTM: Well-defined domains endpoint with proper parametersThe new
/api/v1/domainsendpoint is properly defined with appropriate query parameters for searching and pagination. The structure follows the same pattern as other endpoints in the API.
370-382: LGTM: Proper definition of protobufNullValue enumThe protobufNullValue enum is properly defined with good documentation explaining its purpose for JSON representation.
400-401: LGTM: Good replacement of ens_name with domain_infoThe replacement of
ens_namewithdomain_inforeferencing thev1DomainInfoschema is a good enhancement that will provide more comprehensive domain information.
454-476: LGTM: Well-structured domain schemasThe
v1Domainandv1DomainInfoschemas are well-structured with appropriate fields for domain information.
538-547: LGTM: Properly defined ListDomainsResponse with pagination supportThe
v1ListDomainsResponseschema is correctly structured with an items array and pagination support, following the same pattern as other list response schemas.
637-641: LGTM: Properly added domains to QuickSearchResponseThe domains field is properly added to the QuickSearchResponse schema.
multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (3)
198-206: LGTM: Improved list function signatureThe function signature has been improved by replacing option types with more straightforward vector types. This simplifies the API and makes the code more readable.
216-222: LGTM: Simplified chain_ids filtering logicThe conditional logic for applying chain_ids filter has been improved to directly check if the vector is non-empty.
229-241: LGTM: Improved addresses filtering implementationThe changes correctly update the address filtering logic to handle multiple addresses efficiently.
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (4)
1-9: LGTM: Good imports organizationThe imports are well-organized, including the new Domain type from domains module.
19-20: LGTM: Added domains field to QuickSearchResultThe domains field has been correctly added to the QuickSearchResult struct.
31-33: LGTM: Updated merge method to include domainsThe merge method has been properly updated to extend the domains vector.
125-126: LGTM: Updated QuickSearchResponse conversionThe From implementation for proto::QuickSearchResponse has been correctly updated to include the domains field.
multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (4)
21-24: New fields added to support BENS client and protocols.The introduction of
bens_clientandbens_protocolsis a clean approach for domain lookups. Initialization within the struct and constructor is straightforward, and there are no obvious concurrency or maintainability issues.Also applies to: 32-35, 42-45
163-164: NFT-specific address search logic.Here,
search::AddressSearchConfig::NFTSearchproperly toggles token types. The pattern is consistent with the non-token search implementation above.Also applies to: 166-166
228-231: Quick search domain support.Extending
quick_searchto passbens_clientandbens_protocolsensures domain queries can be resolved in a unified manner alongside other resources.
326-350: Newlist_domainsRPC method.This addition seamlessly integrates domain search into the aggregator service, returning properly paginated data. The error propagation and pagination logic match existing patterns, making it maintainable and consistent.
multichain-aggregator/multichain-aggregator-proto/proto/v1/multichain-aggregator.proto (7)
7-7: Importinggoogle/protobuf/struct.proto.This import is necessary for storing arbitrary protocol data in the new Domain message.
22-22: AddedListDomainsRPC.The new RPC signature aligns with its server-side implementation. The pairing of
ListDomainsRequestandListDomainsResponseis consistent with existing methods.
43-49: NewDomainInfomessage.This struct captures details about a domain. There’s no immediate concern, though be aware that “optional” in proto3 has nuances, and you might need to confirm how default/empty states are handled.
52-52: Replaced ENS field withdomain_info.Embedding domain details through
DomainInfois more flexible than a single ENS name string. This design supports multi-field expansions in the future.
91-97: NewDomainmessage.Including a
google.protobuf.Struct protocolfield allows flexible handling of domain protocol metadata. This is a good choice for unstructured data that may evolve.
144-144:domainsfield inQuickSearchResponse.Augmenting quick search results with discovered domains expands the aggregator features. Ensure any front-end consumers can handle this new field gracefully.
232-242:ListDomainsRequestandListDomainsResponse.The request/response pairing for domain listings is consistent with the rest of the API. Paginated output supports large domain sets.
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (14)
2-5: New imports for BENS queries, domain types, and collections.These additions cleanly bring in the required dependencies for domain lookups. The standard library’s
OnceLockusage for regex initialization is also well-structured.Also applies to: 10-13, 22-25
35-39:domain_name_regexinitialization.Using
OnceLock<Regex>is a good pattern for performance and thread safety when repeatedly matching domain strings.
40-55:AddressSearchConfigenum integration.Splitting search logic between
NFTSearchandNonTokenSearchavoids scattering conditional checks. Delegating token type resolution totoken_types()is clear.
59-60: Extendedsearch_addresseswith BENS params.Adding
bens_clientandAddressSearchConfigbroadens the search logic. No immediate concurrency or error-handling pitfalls are evident here.Also applies to: 62-62
70-73: Closure-based address parsing.This is a neat approach: either parse a valid address or treat it as a potential name-based query. No obvious concerns.
75-106: Conditional domain resolution.When
NonTokenSearchis used and the query looks like a domain, the code callssearch_domainsand retrieves addresses. This branching helps unify domain and direct address queries while avoiding extra domain lookups for non-domain inputs.
107-112: Loading addresses from DB and preloading domain info.Passing
addressesand optionalcontract_name_queryis consistent for both domain-based and raw address searches. Post-processing withpreload_domain_infois a logical next step, ensuring domain data is enriched.Also applies to: 118-126
128-179:preload_domain_infousage and error handling.Fetching domain info for each address in parallel is efficient. Logging failures ensures visibility without blocking the entire operation. Confirm that partial errors are acceptable for your use cases.
280-314:search_domainsfunction.This new function neatly mirrors the existing search pattern, returning domain objects and a next-page token. The error handling and usage of
map_errmatch standard patterns.
321-324: Added BENS client and protocol arguments forquick_search.Incorporating domain logic into your quick search broadens coverage for domain queries without complicating preexisting structures.
343-346: Concurrent join for quick search jobs.This approach merges partial results from each search term. The concurrency pattern is standard and well-managed, folding results into a single structure.
Also applies to: 351-352
367-367: NewDomainvariant inSearchTermand search context.These changes extend your search domain to actual domains (pun intended). The usage is consistent with existing variants.
Also applies to: 375-375
507-542:SearchTerm::Domainbranch logic.Looks correct for retrieving domain-based addresses and then appending them to
results.addresseswhile also populatingresults.domains. This ensures domain queries are fully reflected in the final quick search output.
624-632: New test for domain queries.This test confirms that a domain-like string is parsed into the correct search terms, including the new
SearchTerm::Domain. Great for ensuring future refactors don’t break domain detection.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
111-113: Balance entities update addresses previous review commentThe
balance_entitiesmacro call has been correctly updated to include the newdomainsfield, addressing the comment from the previous review. This ensures domains are properly considered in the balancing logic.
🧹 Nitpick comments (1)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)
35-88: Well-implemented filtering and sorting functionalityThe new
filter_and_sort_entities_by_prioritymethod and its helper function are well-designed. The macro-based approach efficiently applies the filtering and sorting logic across multiple entity types. The implementation properly handles:
- Filtering to keep only one item per chain_id
- Sorting based on the priority defined by the order of chain IDs
However, I notice that
domainsis not included in the list of fields to filter and sort. Is this intentional? If domains have a chain_id field and should be filtered/sorted like other entities, consider including them.If domains should be included in the filtering and sorting, update the macro call:
filter_and_sort_by_priority!( self, [ addresses, blocks, transactions, block_numbers, dapps, tokens, - nfts + nfts, + domains ] );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (4)
multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (1)
addresses(234-237)multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (6)
addresses(118-121)addresses(423-426)addresses(428-437)addresses(438-442)addresses(494-503)addresses(530-533)multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (1)
new(28-47)multichain-aggregator/multichain-aggregator-logic/src/services/api_key_manager.rs (1)
new(12-14)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Unit, doc and integration tests
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (3)
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (3)
1-8: Integration of Domain support looks good!The addition of the
domainsfield toQuickSearchResultstruct and the corresponding import ofdomains::Domainare properly implemented. This is a necessary foundation for the domains integration feature.Also applies to: 20-20
32-32: Proper update to merge methodThe
mergemethod has been correctly updated to extend the newdomainsvector when merging anotherQuickSearchResultinstance, consistent with how other fields are handled.
127-127: Proto response mapping for domains is correctThe
Fromimplementation for converting toproto::QuickSearchResponsehas been properly updated to include domain information in the response. This ensures the new domain data will be correctly exposed through the API.
2 participants
Summary by CodeRabbit
New Features
Refactor