Skip to content

Conversation

@JonoPrest
Copy link
Collaborator

@JonoPrest JonoPrest commented Oct 31, 2025

Summary by CodeRabbit

  • New Features

    • Configurable caching for Cap’n Proto requests to improve repeat-query performance.
    • Public conversion utilities for interoperability with external tooling.
    • Expanded request and field-selection APIs for more flexible query construction.
  • Chores

    • Dependency updates: added a hashing library and moved compression lib to dev-dependencies.
  • Compatibility

    • Serialization variant now includes a cache toggle — client configurations may require adjustment.

@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Walkthrough

Adds Cap'n Proto caching: makes SerializationFormat::CapnProto carry should_cache_queries, implements a two-stage cached-query then full-query flow in the client, splits BuilderReader into CapnpBuilder/CapnpReader, extends schema with CachedQueryResponse and Query.body union, and adds QueryId/Request/FieldSelection and related builders/readers.

Changes

Cohort / File(s) Summary
Config & Example
examples/all_erc20/src/main.rs, hypersync-client/src/config.rs
SerializationFormat::CapnProto changed from a unit variant to struct-like CapnProto { should_cache_queries: bool }; example now constructs ClientConfig with CapnProto explicitly.
Client: caching flow & parsing
hypersync-client/src/lib.rs, hypersync-client/src/parse_response.rs
Client implements a two-stage flow when CapnProto caching is enabled: send QueryId cached request -> parse CachedQueryResponse via new read_query_response -> fall back to full query on miss/error. Added helper to check caching flag and Cap'n Proto response handling.
Type conversions / API surface
hypersync-client/src/to_ethers.rs, hypersync-client/tests/api_test.rs
Added ConversionError enum, TryIntoEthers trait, and access_conversion function; tests updated for the new CapnProto { should_cache_queries: ... } variant and conversion adjustments.
Cap'n Proto schema
hypersync-net-types/hypersync_net_types.capnp
Added CachedQueryResponse union (queryResponse
Net types: Capnp trait refactor
hypersync-net-types/src/lib.rs, hypersync-net-types/src/block.rs, hypersync-net-types/src/log.rs, hypersync-net-types/src/trace.rs, hypersync-net-types/src/transaction.rs
Replaced BuilderReader with public CapnpBuilder and CapnpReader traits; updated implementations across filters/selections to add populate_builder and from_reader.
Query types & hashing
hypersync-net-types/src/query.rs
Introduced QueryId, BlockRange, Request enum (QueryBody vs QueryId), FieldSelection, revamped Query representation, canonicalization and hashing to produce QueryId, and Cap'n Proto builder/reader workflows plus tests.
Net-types benches & tooling
hypersync-net-types/benches/compression.rs
Added Cap'n Proto helper utilities for message construction/serialization and refactored benchmarks to use unified Cap'n Proto helper functions.
Manifests / deps
hypersync-net-types/Cargo.toml, hypersync-client/Cargo.toml
Bumped versions, moved zstd to dev-dependencies, added xxhash-rust dep, and updated hypersync-net-types dependency version in client Cargo.toml.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server

    rect rgb(220,235,255)
    note over Client,Server: Two-stage cached query flow (when CapnProto.should_cache_queries = true)
    Client->>Client: Check serialization_format.should_cache_queries
    alt caching enabled
        Client->>Client: Build QueryId cached request
        Client->>Server: Send cached-query request (Query.body = queryId, cache headers)
        Server->>Server: Lookup cache by QueryId
        alt cache hit
            Server-->>Client: CachedQueryResponse { queryResponse }
            Client->>Client: read_query_response -> parse into ArrowResponse
            Client-->>Client: return ArrowResponse
        else cache miss
            Server-->>Client: CachedQueryResponse { notCached }
            Client->>Client: Fall back to full Query path
        end
    end
    end

    rect rgb(235,220,255)
    note over Client,Server: Full query flow
    Client->>Client: Build full Query request (body = query)
    Client->>Server: Send full query
    Server->>Server: Execute query
    Server-->>Client: QueryResponse
    Client->>Client: parse_query_response -> ArrowResponse
    Client-->>Client: return ArrowResponse
    end
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

  • Focus review on:
    • hypersync-net-types/src/query.rs (canonicalization, QueryId generation, hashing, builder/reader correctness, and tests).
    • hypersync-client/src/lib.rs (two-stage cached vs full query control flow, error handling, fallback semantics).
    • Schema/codegen alignment for hypersync-net-types/hypersync_net_types.capnp and all CapnpBuilder/CapnpReader implementations across modules.

Possibly related PRs

  • Capnp encoding for queries #75 — Earlier Cap'n Proto integration work; touches serialization/Cap'n Proto codepaths closely related to this PR's caching, QueryId, and schema changes.

Suggested reviewers

  • JasoonS

Poem

🐰 I nibble at schemas, stitch bytes neat,

QueryIds hop—hashed paws quick and fleet.
Builders split, readers learn to sing,
Cached replies dart in on velvet wing.
Hop, cache, parse — then out the carrot treat! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'Query caching' accurately and concisely summarizes the primary change in the changeset. The title directly corresponds to the main objectives of implementing query caching functionality, which is evidenced by the substantial changes across multiple files to add Cap'n Proto-based caching infrastructure, including modifications to serialization formats, query structures, and response handling with caching logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jp/query-caching

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a098504 and 203d31b.

📒 Files selected for processing (4)
  • examples/all_erc20/src/main.rs (2 hunks)
  • hypersync-client/Cargo.toml (2 hunks)
  • hypersync-client/src/config.rs (2 hunks)
  • hypersync-net-types/Cargo.toml (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • examples/all_erc20/src/main.rs
⏰ 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: lint
🔇 Additional comments (4)
hypersync-client/Cargo.toml (1)

3-3: Version bumps coordinated with hypersync-net-types.

The version increments align the client crate with the updated net-types dependency to support the query caching feature. Path dependencies are preserved.

Also applies to: 50-50

hypersync-net-types/Cargo.toml (1)

27-27: Dependency relocation is appropriate.

Moving zstd to dev-dependencies is reasonable if it is only used for compression testing or benchmarking and not required for production functionality.

hypersync-client/src/config.rs (2)

30-30: LGTM: Default derive improves ergonomics.

Adding Default derive with Json as the default variant is a sensible improvement that enhances usability without breaking existing code.

Also applies to: 33-33


78-78: LGTM: Default derive with sensible default variant.

Adding Default derive with NoEncode as the default is a good choice—no transformation is the safest default for binary data.

Also applies to: 81-81


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@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: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
hypersync-client/src/to_ethers.rs (2)

400-407: Fix incorrect error message.

The error message says "Failed to convert field value" but the field being converted is gas_price.

Apply this diff to fix the error message:

             gas_price: value
                 .gas_price
                 .map(|gas_price| {
                     gas_price.try_into().map_err(|_| {
-                        ConversionError::ConvertError("Failed to convert field `value`".into())
+                        ConversionError::ConvertError("Failed to convert field `gas_price`".into())
                     })
                 })
                 .transpose()?,

730-741: Fix incorrect error message.

The error message says "Field value is null" but the field being checked is reward_type.

Apply this diff to fix the error message:

             reward_type: {
                 let reward_type = reward_type.ok_or(ConversionError::MissingValue(
-                    "Field `value` is null".into(),
+                    "Field `reward_type` is null".into(),
                 ))?;
hypersync-net-types/src/transaction.rs (1)

351-354: Add the missing ? when reading authorization items.

auth_list.get(i) returns a Result; without ? the code won’t compile because AuthorizationSelection::from_reader expects the reader, not the wrapper. Please propagate the result before invoking the reader.

-            let auth_reader = auth_list.get(i);
-            let auth_selection = AuthorizationSelection::from_reader(auth_reader)?;
+            let auth_reader = auth_list.get(i)?;
+            let auth_selection = AuthorizationSelection::from_reader(auth_reader)?;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5fb50f and 863db4a.

⛔ Files ignored due to path filters (1)
  • hypersync-net-types/src/__generated__/hypersync_net_types_capnp.rs is excluded by !**/__generated__/**
📒 Files selected for processing (14)
  • examples/all_erc20/src/main.rs (2 hunks)
  • hypersync-client/src/config.rs (1 hunks)
  • hypersync-client/src/lib.rs (6 hunks)
  • hypersync-client/src/parse_response.rs (3 hunks)
  • hypersync-client/src/to_ethers.rs (2 hunks)
  • hypersync-client/tests/api_test.rs (1 hunks)
  • hypersync-net-types/Cargo.toml (1 hunks)
  • hypersync-net-types/hypersync_net_types.capnp (2 hunks)
  • hypersync-net-types/src/block.rs (3 hunks)
  • hypersync-net-types/src/lib.rs (2 hunks)
  • hypersync-net-types/src/log.rs (3 hunks)
  • hypersync-net-types/src/query.rs (6 hunks)
  • hypersync-net-types/src/trace.rs (3 hunks)
  • hypersync-net-types/src/transaction.rs (5 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
hypersync-net-types/src/block.rs (1)
hypersync-net-types/src/lib.rs (2)
  • populate_builder (52-52)
  • populate_builder (66-81)
hypersync-client/src/parse_response.rs (2)
hypersync-client/src/lib.rs (3)
  • message (467-467)
  • message (526-526)
  • message_reader (496-497)
hypersync-net-types/src/query.rs (8)
  • message (50-50)
  • message (367-367)
  • message (389-389)
  • message (743-743)
  • message (764-764)
  • message_reader (84-84)
  • message_reader (382-382)
  • message_reader (404-404)
examples/all_erc20/src/main.rs (1)
hypersync-client/src/config.rs (2)
  • default (42-45)
  • default (95-97)
hypersync-net-types/src/lib.rs (1)
hypersync-net-types/src/query.rs (4)
  • populate_builder (525-568)
  • from_reader (90-117)
  • from_reader (236-256)
  • from_reader (572-613)
hypersync-client/src/lib.rs (1)
hypersync-client/src/parse_response.rs (2)
  • read_query_response (29-90)
  • message_reader (98-99)
hypersync-net-types/src/query.rs (1)
hypersync-net-types/src/lib.rs (4)
  • from_reader (56-58)
  • from_reader (89-99)
  • populate_builder (52-52)
  • populate_builder (66-81)
⏰ 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: test_release
  • GitHub Check: test_dev
🔇 Additional comments (3)
hypersync-client/src/to_ethers.rs (3)

15-15: LGTM! Unused import removed.

The removal of Write from the import list is appropriate as it's not used in this module.


772-776: LGTM! Idiomatic Rust refactor.

The simplified filter_map with Option::map is more concise and idiomatic than a manual unwrapping approach. It correctly filters out None topics while converting Some values.


478-484: Verify silent defaulting to zero address.

The address field uses unwrap_or(ethers::types::Address::zero()) instead of returning an error when the address is missing. This silently defaults to the zero address, which could mask missing data or cause unexpected behavior if the caller expects an error for missing addresses.

Confirm whether this silent fallback is intentional for access list items.

Copy link

@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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 863db4a and a098504.

⛔ Files ignored due to path filters (1)
  • hypersync-net-types/src/__generated__/hypersync_net_types_capnp.rs is excluded by !**/__generated__/**
📒 Files selected for processing (5)
  • hypersync-client/src/lib.rs (5 hunks)
  • hypersync-net-types/Cargo.toml (2 hunks)
  • hypersync-net-types/benches/compression.rs (2 hunks)
  • hypersync-net-types/hypersync_net_types.capnp (2 hunks)
  • hypersync-net-types/src/query.rs (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • hypersync-net-types/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (3)
hypersync-client/src/lib.rs (1)
hypersync-client/src/parse_response.rs (2)
  • read_query_response (29-90)
  • message_reader (98-99)
hypersync-net-types/benches/compression.rs (1)
hypersync-net-types/src/query.rs (12)
  • message (49-49)
  • message (566-566)
  • message (692-692)
  • message (713-713)
  • to_capnp_bytes (563-575)
  • from_capnp_bytes (68-73)
  • from_capnp_bytes (577-588)
  • message_reader (71-71)
  • message_reader (583-584)
  • from_reader (77-109)
  • from_reader (234-254)
  • from_reader (499-540)
hypersync-net-types/src/query.rs (4)
hypersync-client/src/lib.rs (4)
  • message (449-449)
  • message (508-508)
  • message_reader (478-479)
  • new (71-94)
hypersync-net-types/benches/compression.rs (5)
  • message (329-329)
  • from_capnp_bytes (348-354)
  • message_reader (350-351)
  • message_reader (359-360)
  • to_capnp_bytes (336-340)
hypersync-format/src/types/fixed_size_data.rs (1)
  • FixedSizeData (105-105)
hypersync-net-types/src/lib.rs (4)
  • from_reader (56-58)
  • from_reader (89-99)
  • populate_builder (52-52)
  • populate_builder (66-81)
⏰ 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). (3)
  • GitHub Check: lint
  • GitHub Check: test_dev
  • GitHub Check: test_release
🔇 Additional comments (3)
hypersync-net-types/hypersync_net_types.capnp (1)

26-31: Schema updates LGTM

CachedQueryResponse and the Query union align cleanly with the caching flow; no issues spotted.

hypersync-client/src/lib.rs (1)

425-503: Cap'n Proto caching flow looks solid

Thanks for threading the cached-query preflight and cleanly falling back—nothing concerning stood out in this segment.

hypersync-net-types/benches/compression.rs (1)

327-388: Benchmark helpers look good

The shared helpers tidy up the Cap’n Proto setup without changing behavior—looks good to me.

@JonoPrest JonoPrest merged commit 969852e into main Nov 3, 2025
4 checks passed
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.

3 participants