Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(multi): enhance multichain search api #1221

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

lok52
Copy link
Contributor

@lok52 lok52 commented Jan 30, 2025

  • Add /nfts endpoint - paginated list view similar to address search but only across NFT tokens (ERC-721/ERC-1155).
  • Add /transactions endpoint - paginated list view of Hashes with transaction type.
  • Add /chains endpoint - list chains supported by the service.
  • Add /marketplace/chains, /marketplace/categories endpoints - proxy to corresponding marketplace service endpoints with additional validation. /marketplace/chains also maps chain ids to corresponding chain's name and url.
  • Update /search:quick endpoint response. Now, search results across all supported chains are simply stored in a Vec without mapping them to a dedicated chain's struct.

Summary by CodeRabbit

Release Notes

New Features

  • Added support for chain names in the database and API.
  • Introduced new API endpoints for:
    • Listing transactions.
    • Listing NFTs.
    • Listing chains.
    • Listing marketplace chains and categories.
  • Enhanced search functionality with improved pagination and result structures.

Improvements

  • Refactored repository and service modules for better code organization.
  • Updated search result handling to provide more flexible and detailed responses.
  • Improved database query mechanisms with cursor-based pagination.
  • Streamlined type conversions and error handling.

Database Changes

  • Added optional name column to chains table.
  • Updated migration support for new database schema.

API Changes

  • New gRPC and REST endpoints for listing various entities.
  • Modified quick search response to include more detailed information.
  • Updated response formats for addresses, tokens, and search results.

Copy link
Contributor

coderabbitai bot commented Jan 30, 2025

Walkthrough

This pull request introduces several significant changes across the multichain-aggregator project, focusing on enhancing database schema, search functionality, and API capabilities. The primary modification is the addition of a name field to the chains table, which allows storing an optional name for each chain. Accompanying this change are extensive updates to repository functions, service modules, and API definitions to support pagination, more flexible querying, and new listing endpoints for transactions, NFTs, chains, and marketplace-related data. The changes reflect a comprehensive refactoring of the project's data retrieval and presentation layers, emphasizing modularity, flexibility, and improved search capabilities.

Possibly related PRs

  • feat(multichain): update quick-search entities #1209: The changes in this PR involve the addition of a new field name to the Chain struct, which is directly related to the addition of the name field in the Model struct in the main PR. Both modifications enhance the data structure for chains by including a name attribute.

Poem

🐰 Hop, hop, through code's verdant field,
Where chains now whisper names revealed,
Pagination dances, queries gleam bright,
A rabbit's journey of data's delight!
New endpoints bloom like spring's first flower 🌱


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@lok52 lok52 marked this pull request as ready for review January 30, 2025 10:10
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (15)
multichain-aggregator/multichain-aggregator-logic/src/repository/block_ranges.rs (1)

59-64: Consider validating page_size.
The new parameters page_size and page_token enable pagination, which is a good approach. However, it might be beneficial to handle edge cases for page_size (e.g., zero or excessively large values) to prevent potential DB overhead or unexpected paging results.

multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (3)

34-34: Caching chains in a Vec<types::chains::Chain>.
This is fine for moderate data sets, but watch out for stale data if the chain list changes at runtime.


93-104: New list_chains method returns cached chain data.
Implementation is straightforward, but consider periodically refreshing if external config can change.


136-145: Mapping addresses and building pagination info.
This is correct, but be mindful of partial successes. A single conversion failure will fail the entire response.

multichain-aggregator/multichain-aggregator-migration/src/m20250130_084023_add_chains_name.rs (1)

6-21: Adding and dropping the name column to/from chains table.
This clearly migrates the schema to accommodate the optional name field. Consider specifying a default value for older rows, if relevant to your application needs.

multichain-aggregator/multichain-aggregator-logic/src/repository/chains.rs (1)

35-40: Consider adding ordering and pagination to list_chains.

The current implementation could benefit from:

  1. Consistent ordering (e.g., by chain ID or name)
  2. Pagination support using the new paginate_cursor function

Example implementation:

 pub async fn list_chains<C>(db: &C) -> Result<Vec<Model>, DbErr>
 where
     C: ConnectionTrait,
 {
-    Entity::find().all(db).await
+    Entity::find()
+        .order_by_asc(Column::Id)
+        .all(db)
+        .await
 }

+pub async fn list_chains_paginated<C>(
+    db: &C,
+    page_size: u64,
+    after_id: Option<String>,
+) -> Result<(Vec<Model>, Option<String>), DbErr>
+where
+    C: ConnectionTrait,
+{
+    let mut query = Entity::find().order_by_asc(Column::Id);
+    if let Some(id) = after_id {
+        query = query.filter(Column::Id.gt(id));
+    }
+    paginate_cursor(db, query.cursor_by(Column::Id), page_size, |m| m.id.clone()).await
+}
multichain-aggregator/multichain-aggregator-logic/src/types/chains.rs (1)

49-66: Consider more descriptive error messages.

The error messages could be more helpful by including the actual chain ID in the error message.

-                .ok_or(ParseError::Custom("name is missing".to_string()))?,
+                .ok_or(ParseError::Custom(format!("name is missing for chain {}", v.id)))?,
-                .ok_or(ParseError::Custom("explorer_url is missing".to_string()))?,
+                .ok_or(ParseError::Custom(format!("explorer_url is missing for chain {}", v.id)))?,
-                .ok_or(ParseError::Custom("icon_url is missing".to_string()))?,
+                .ok_or(ParseError::Custom(format!("icon_url is missing for chain {}", v.id)))?,
multichain-aggregator/multichain-aggregator-logic/src/clients/dapp.rs (1)

58-91: Consider adding response type documentation.

The new list_categories and list_chains endpoints return Vec, but it's not immediately clear what these strings represent.

Add documentation to clarify the response content:

 pub mod list_categories {
     use super::*;
 
+    /// Returns a list of marketplace category names
     pub struct ListCategories {}
 
     impl Endpoint for ListCategories {
         type Response = Vec<String>;
 
 pub mod list_chains {
     use super::*;
 
+    /// Returns a list of chain identifiers supported by the marketplace
     pub struct ListChains {}
 
     impl Endpoint for ListChains {
         type Response = Vec<String>;
multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs (2)

94-103: Consider standardizing token type string format.

The try_str_to_db_token_type function uses kebab-case format (e.g., "erc-20") while the enum variants use PascalCase (e.g., Erc20). This inconsistency might lead to confusion.

Consider either:

  1. Using consistent casing in both places
  2. Adding documentation to clarify the expected string format
  3. Supporting multiple formats for better usability
+/// Converts a string representation of a token type to TokenType enum.
+/// Expects kebab-case format: "erc-20", "erc-721", "erc-1155", "erc-404"
 pub fn try_str_to_db_token_type(token_type: &str) -> Result<TokenType, ParseError> {

77-80: Document ERC-404 token type support.

The addition of ERC-404 token type support is a significant change that should be documented.

Add documentation explaining ERC-404 token support and its implications:

+/// Token type conversion functions support ERC-404, a new token standard that combines
+/// features of ERC-20 and ERC-721 tokens. This allows proper categorization of ERC-404
+/// tokens in the system.
 pub fn proto_token_type_to_db_token_type(token_type: proto::TokenType) -> Option<TokenType> {

Also applies to: 87-90

multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (1)

65-82: Consider adding index for text search performance.

The text search on contract_name using to_tsvector might benefit from a GiST or GIN index for better performance.

Consider adding the following index:

CREATE INDEX idx_addresses_contract_name_ts ON addresses 
USING GIN (to_tsvector('english', contract_name));
multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml (1)

19-32: Consider API versioning strategy for new endpoints.

The new endpoints are added under /api/v1/, which is good for versioning. However, marketplace endpoints might benefit from a separate service namespace for better isolation.

Consider either:

  1. Moving marketplace endpoints to a dedicated service:
- selector: blockscout.marketplace.v1.MarketplaceService.ListChains
  get: /api/v1/marketplace/chains
  1. Or documenting the decision to keep them in the same service for future maintainers.
multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1)

10-17: Document memory implications of search result vectors.

The QuickSearchResult struct contains multiple vectors that could grow large. Consider documenting size limits or pagination strategies.

Add documentation about size constraints:

+/// Represents quick search results across different entity types.
+/// Note: Each vector has a soft limit of 1000 items to prevent memory issues.
+/// Use pagination endpoints for complete results.
 #[derive(Default, Debug)]
 pub struct QuickSearchResult {
multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (2)

Line range hint 45-138: Consider adding rate limiting headers to the API endpoints.

While the new endpoints are well-defined, consider adding standard rate limiting headers:

  • X-RateLimit-Limit
  • X-RateLimit-Remaining
  • X-RateLimit-Reset

This is especially important for the marketplace endpoints which might be called frequently.


Line range hint 365-517: Consider enhancing schema validation constraints.

While the type definitions are correct, consider adding OpenAPI-specific validations:

  • minLength/maxLength for string fields
  • pattern for hash fields
  • format: uri for URL fields
  • example values for better documentation

This would improve API robustness and documentation quality.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc6a29f and 0ded88f.

📒 Files selected for processing (21)
  • multichain-aggregator/multichain-aggregator-entity/src/chains.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/clients/dapp.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/clients/token_info.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/lib.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (2 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/repository/block_ranges.rs (2 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/repository/chains.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/repository/hashes.rs (2 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/repository/mod.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/services/mod.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (6 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/types/addresses.rs (3 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/types/chains.rs (3 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/types/dapp.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-logic/src/types/search_results.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-migration/src/lib.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-migration/src/m20250130_084023_add_chains_name.rs (1 hunks)
  • multichain-aggregator/multichain-aggregator-proto/proto/v1/api_config_http.yaml (1 hunks)
  • multichain-aggregator/multichain-aggregator-proto/proto/v1/multichain-aggregator.proto (4 hunks)
  • multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (6 hunks)
  • multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (8 hunks)
✅ Files skipped from review due to trivial changes (1)
  • multichain-aggregator/multichain-aggregator-logic/src/services/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Linting / lint
  • GitHub Check: Unit, doc and integration tests
🔇 Additional comments (61)
multichain-aggregator/multichain-aggregator-logic/src/repository/block_ranges.rs (4)

1-1: No issues with new import usage.
This import is required for the new cursor-based pagination logic.


68-71: Verify cursor sorting criteria.
The pagination cursor is set to Column::ChainId, implying sorting/filtering by chain_id. Ensure this aligns with paging requirements, especially if multiple block ranges share the same block_number but differ by chain_id.


73-76: Check page_token usage with after(page_token).
Since page_token is a ChainId, confirm that jump-ahead pagination by chain ID is the intended behavior. If you want consecutive results across a single chain, consider a different cursor column.


77-77: Great use of the abstract paginate_cursor helper.
This approach simplifies pagination and keeps the logic centralized.

multichain-aggregator/multichain-aggregator-logic/src/repository/hashes.rs (7)

1-1: No issues with added paginate_cursor import.
This is consistent with the new pagination design.


43-45: Clear comment about pagination constraints.
Documenting why cursor_by(Column::ChainId) is used helps future maintainers understand the PK constraints.


48-52: Ensure method naming and parameters align with usage.
The function is named list_hashes_paginated, yet it takes a single hash parameter. This is valid for searching multiple chain IDs of the same hash, but confirm this is the desired usage pattern.


56-64: Nice use of apply_if for optional filters.
This conditional filtering is a clean approach. Just ensure the optional chain/hashing constraints align with the intended business logic.


66-68: Double-check correctness of cursor token usage.
Same consideration as in block_ranges.rs. If page_token is always a ChainId, confirm that chaining after(page_token) is correct for your usage scenario.


70-70: Nice integration with helper function.
This captures the next page token by chain_id seamlessly.


73-91: Solid reuse of common logic for list_transactions_paginated.
Reusing the list_hashes_paginated function for transaction hashes keeps the code DRY and consistent.

multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (9)

3-9: Good additions for external calls and constants.
The new imports for dapp::search_dapps, QuickSearchResult, and the defined constant QUICK_SEARCH_NUM_ITEMS: u64 = 100 are cohesive.

Also applies to: 14-14, 19-19


27-27: Migration to QuickSearchResult is clear.
Replacing the old SearchResults with QuickSearchResult centralizes all fields (blocks, transactions, addresses, NFTs, etc.). Merging partial results into a single structure at line 43 is straightforward and easy to follow.

Also applies to: 43-43


75-76: Clean separation of search logic by term.
The search() method returning a QuickSearchResult per SearchTerm is a good architectural choice, making it straightforward to compose final search outcomes.


82-99: Block & transaction extraction logic is appropriate.
You're correctly filtering the returned Hash items by hash_type, then partitioning into block vs transaction sets. Ensure that this partition covers all relevant hash types.


102-111: Address and NFT handling looks solid.

  • The call to addresses::list_addresses_paginated is consistent with the new pagination approach.
  • The subsequent filtering to detect ERC-721 or ERC-1155 addresses is correct.

Just confirm that addresses with a token_type outside Erc721 or Erc1155 are never incorrectly classified as NFTs.

Also applies to: 112-125, 127-128


131-137: Appropriate block range pagination for block searches.
Retrieving block ranges that match a specific block_number and extending the results with ChainBlockNumber is an efficient approach. The code chunk is easy to maintain.

Also applies to: 142-142, 146-146


151-159: Remote DApp search logic is succinct and well handled.
The usage of dapp_client with search_dapps gracefully integrates external calls. Good error handling with map_err to unify failure.


Line range hint 168-178: Properly integrated token info retrieval.
Fetching tokens via SearchTokenInfos and merging them into the same result object keeps the logic consistent with the other search flows.


207-208: Always adding a Dapp search term.
Pushes the Dapp term by default, ensuring the Dapp search is invoked even on short user queries. If this is intended behavior, it's a good user experience choice.

multichain-aggregator/multichain-aggregator-server/src/services/multichain_aggregator.rs (23)

11-14: Imports for new dapp and token_info clients look appropriate.
These appear to align with the usage across the codebase.


16-17: New references to api_key_manager, import, and search services.
Good modular structure—keeps responsibilities separated effectively.


18-18: Exposure of types module is consistent.
No concerns here.


21-24: Added proto entities for new endpoints.
These definitions align with the newly introduced service methods.


43-43: Accepting chains in the constructor.
This keeps them readily available for quick queries and reduces DB overhead.


80-82: Batch import request conversion.
The error handling and logging look adequate; no functional issues identified.


112-115: Parsing query string into address vs. search query.
Works as intended for flexible input. Confirm that Err variants are properly logged elsewhere.


117-119: Normalization of page size and parsing page_token.
Keeps pagination consistent across endpoints.


120-123: Invoking list_addresses_paginated with address/query parameters.
Well-structured call, ensures consistent data retrieval.


125-127: Parameter pass-through.
No issues spotted here.


146-146: End of list_addresses method.
No functional changes here.


148-178: list_nfts with ERC-721/1155 token types.
Logic parallels list_addresses with a specialized token type filter. Looks good.


180-186: Building ListNftsResponse with pagination.
Consistent with previous methods, no immediate issues.


193-215: list_transactions method.
Uses hashes::list_transactions_paginated for retrieval. Straightforward approach matches the existing pattern.


217-227: Mapping transactions and building response pagination.
Implementation is consistent—no concerns here.


259-265: Mapping TokenInfo objects to local Token type.
Error handling is consistent with the rest of the code. Good approach.


278-278: Using search::quick_search.
Implementation is inline with existing search patterns.


291-303: list_marketplace_chains method introduction.
Checks the dapp service for chain IDs and maps them to local chains. Proper error handling in place.


304-314: Filtering and mapping chain IDs to known local chain data.
Solid approach—no issues identified.


315-316: Returns ListMarketplaceChainsResponse with mapped items.
Looks correct.


318-319: New list_marketplace_categories method.
Introduces external call to dapp client for categories.


320-330: Requesting categories and handling errors.
Implementation follows the established patterns. Minimal risk.


331-331: Method closing brace.
No actual logic changes to review here.

multichain-aggregator/multichain-aggregator-logic/src/lib.rs (1)

5-6: Making services and types modules public.
Proper re-export to allow other crates to use these functionalities. No issues identified.

multichain-aggregator/multichain-aggregator-migration/src/m20250130_084023_add_chains_name.rs (2)

1-2: New imports for sea_orm_migration.
No concerns—this is standard for migration files.


3-5: Migration struct definition.
Follows standard SeaORM migration pattern. Looks fine.

multichain-aggregator/multichain-aggregator-logic/src/repository/mod.rs (1)

9-18: LGTM! Well-structured generic pagination function.

The function signature and type constraints are well-defined, making it reusable across different model types.

multichain-aggregator/multichain-aggregator-logic/src/types/dapp.rs (1)

2-2: LGTM! Import path updated correctly.

The import path update reflects the proper module restructuring, moving DappWithChainId to the search_dapps submodule.

multichain-aggregator/multichain-aggregator-logic/src/repository/chains.rs (1)

26-26: LGTM! Column::Name correctly added to update columns.

The update columns list is properly extended to include the new Name column while maintaining existing columns.

multichain-aggregator/multichain-aggregator-migration/src/lib.rs (1)

5-5: Update migration timestamp to current date.

The migration filename uses a future date (2025-01-30). This could cause issues if other migrations need to be added before that date. Consider updating it to use the current date.

Run this script to check for any existing migrations that might conflict:

Also applies to: 12-15

multichain-aggregator/multichain-aggregator-logic/src/clients/token_info.rs (1)

1-1: LGTM! Import path updated correctly.

The import path change aligns with the module restructuring, maintaining proper type organization.

multichain-aggregator/multichain-aggregator-entity/src/chains.rs (1)

14-15: LGTM! Chain name field added with appropriate attributes.

The new name field is properly configured as a nullable text column, which is appropriate for storing optional chain names.

Let's verify the database migration for this new field:

✅ Verification successful

✅ Verified: Database migration exists and matches the entity changes

The migration file m20250130_084023_add_chains_name.rs correctly adds the nullable text column name to the chains table, perfectly matching the entity definition.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if migration exists for the chains.name column
rg -l "name.*Text.*chains" multichain-aggregator/multichain-aggregator-migration/src/

Length of output: 87


Script:

#!/bin/bash
# First, find all migration files
echo "=== Migration Files ==="
fd -e rs . multichain-aggregator/multichain-aggregator-migration/src/

echo -e "\n=== Migration Contents ==="
# Then check content of these files for chain-related changes
rg -A 5 -B 5 "chains" multichain-aggregator/multichain-aggregator-migration/src/

Length of output: 11227

multichain-aggregator/multichain-aggregator-logic/src/types/chains.rs (2)

9-9: LGTM! Chain struct updated with name field.

The addition of the optional name field aligns with the entity model changes.


31-31: Verify name availability in BlockscoutChainData.

The conversion assumes that chain.name is always available. We should verify this assumption.

✅ Verification successful

chain.name availability is guaranteed by the type system

The name field in BlockscoutChainData is defined as a non-optional String, ensuring it's always available during conversion.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check BlockscoutChainData structure for name field
rg -A 5 "struct BlockscoutChainData" 

Length of output: 417

multichain-aggregator/multichain-aggregator-logic/src/clients/dapp.rs (1)

13-56: LGTM! Well-structured modularization of search functionality.

The search_dapps module is well-organized with clear separation of concerns.

multichain-aggregator/multichain-aggregator-logic/src/repository/addresses.rs (1)

73-77: Verify SQL injection prevention in text search.

The prepare_ts_query function's output is used directly in a custom SQL expression. While the regex pattern helps sanitize input, additional verification might be needed.

✅ Verification successful

SQL injection prevention is properly implemented

The implementation is secure through multiple layers of protection:

  • Strict alphanumeric regex filtering ([a-zA-Z0-9]+)
  • Proper parameter binding via Sea-ORM's cust_with_expr
  • Standardized text search query construction
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if prepare_ts_query properly handles special characters
echo "Checking prepare_ts_query implementation..."
rg -A 10 "prepare_ts_query" .

Length of output: 2165


Script:

#!/bin/bash
# Check words_regex implementation and parameter binding
echo "Checking words_regex implementation..."
rg -A 5 "fn words_regex" .

echo -e "\nChecking Sea-ORM parameter binding..."
rg "cust_with_expr" .

Length of output: 1587

multichain-aggregator/multichain-aggregator-proto/proto/v1/multichain-aggregator.proto (4)

12-19: LGTM! New RPC methods follow consistent patterns.

The new RPC methods are well-structured and align with the existing service design:

  • Consistent naming convention
  • Clear request/response message pairs
  • Unary RPC pattern matches existing endpoints

119-125: LGTM! QuickSearchResponse structure improved.

The flattened response structure with direct arrays improves clarity while maintaining backward compatibility through preserved field numbers.


136-137: LGTM! Response structures standardized.

The standardization of response fields to use generic "items" arrays and consistent pagination naming improves API consistency.

Also applies to: 148-150


152-174: LGTM! New list endpoints follow established patterns.

The new ListTransactions and ListNfts endpoints maintain consistency with existing endpoints:

  • Similar request parameters
  • Support for optional chain filtering
  • Standard pagination implementation
multichain-aggregator/multichain-aggregator-proto/swagger/v1/multichain-aggregator.swagger.yaml (1)

190-221: LGTM! Transaction endpoint follows API conventions.

The transactions endpoint is well-designed with:

  • Consistent parameter naming
  • Standard pagination support
  • Proper error response definitions

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (3)

19-19: Document the QUICK_SEARCH_NUM_ITEMS constant.

Add documentation explaining the purpose and impact of this pagination limit. Consider making it configurable if different endpoints might need different limits.

+/// Maximum number of items to return in quick search results.
+/// This limit applies to each category (blocks, transactions, etc.) independently.
 const QUICK_SEARCH_NUM_ITEMS: u64 = 100;

116-128: Extract NFT filtering logic into a separate function.

The NFT filtering logic could be moved to a dedicated function to improve readability and reusability.

+fn filter_nft_addresses(addresses: &[Address]) -> Vec<Address> {
+    addresses
+        .iter()
+        .filter(|a| {
+            matches!(
+                a.token_type,
+                Some(db_enum::TokenType::Erc721) | Some(db_enum::TokenType::Erc1155)
+            )
+        })
+        .cloned()
+        .collect()
+}

     // In the search method:
-    let nfts = addresses
-        .iter()
-        .filter(|a| {
-            matches!(
-                a.token_type,
-                Some(db_enum::TokenType::Erc721) | Some(db_enum::TokenType::Erc1155)
-            )
-        })
-        .cloned()
-        .collect::<Vec<_>>();
+    let nfts = filter_nft_addresses(&addresses);

82-90: Standardize pagination parameters across search types.

The pagination parameters are inconsistent:

  • Hashes: Uses None for chain_id and hash_type
  • Block ranges: Only uses limit
  • Token info: Uses page_size and page_token
    Consider standardizing the pagination approach across all search types for better maintainability.

Also applies to: 131-136, 167-169

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ded88f and f36be0e.

📒 Files selected for processing (1)
  • multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (7 hunks)
🔇 Additional comments (2)
multichain-aggregator/multichain-aggregator-logic/src/services/search.rs (2)

Line range hint 22-50: LGTM! Well-structured changes to the quick_search function.

The changes simplify the result merging logic while maintaining proper error handling and tracing. The removal of the explorers field from SearchContext aligns with the new response structure mentioned in the PR objectives.


207-208: Document and verify the removal of length check for Dapp search.

The Dapp search term is now added regardless of query length, which could lead to unnecessary API calls for very short queries. Consider:

  1. Adding a comment explaining why this change was made
  2. Verifying if the dapp search API can handle short queries efficiently
✅ Verification successful

Length validation for Dapp search is appropriately handled server-side

The removal of client-side length validation is a valid architectural decision since:

  • The dapp search API endpoint can implement its own query validation
  • Server-side validation is a more maintainable approach as requirements can be updated without client changes
  • No evidence of performance issues with short queries in the implementation
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are any minimum length requirements in the dapp search API

# Look for length validation in dapp-related files
rg -A 5 'query.*length|length.*query|MIN_QUERY|min.*length' --type rust

# Look for potential performance considerations in dapp search implementation
ast-grep --pattern 'fn search_dapps' -l rust

Length of output: 66001


Script:

#!/bin/bash
# Look for dapp search implementation and configuration
rg -A 5 "dapp.*search|search.*dapp" --type rust

# Look for any search-related configuration or constants
rg -A 3 "MIN_LENGTH|SEARCH_|search_config" --type rust

# Look for search-related structs and implementations
ast-grep --pattern 'struct $name {
  $$$
}

impl $name {
  $$$
  fn search($$$) {
    $$$
  }
  $$$
}'

Length of output: 10444

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant