refactor(torii-client): cleanup & remove rpc #3019
Conversation
Larkooo
changed the title
|
Ohayo sensei! WalkthroughThis pull request updates the client module in the Torii crate. It removes the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant G as gRPC Client
C->>C: Call metadata() (acquire write lock)
C->>G: Request updated metadata asynchronously
G-->>C: Return WorldMetadata or Error
Possibly related PRs
Suggested reviewers
🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/torii/client/src/client/mod.rs (2)
8-8: Ohayo sensei! Nice switch totokio::sync::RwLock.
Switching from alternative locks totokio::sync::RwLockis a valid move for asynchronous contexts. Just be mindful of whether you really need distinct read/write phases or if a simplerMutexsuffices.
69-72: Ohayo sensei! Consider using a read lock for metadata fetching.
Right now, you acquire a write lock to callmetadata(). Ifmetadata()doesn’t mutate theWorldClient, you might use a read lock, allowing multiple concurrent metadata reads. Otherwise, this is a clean migration away from storing cached metadata.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/torii/client/src/client/mod.rs(3 hunks)
🔇 Additional comments (2)
crates/torii/client/src/client/mod.rs (2)
24-24: Ohayo sensei! Good call on unifying your error handling.
Bringing in the localErrortype keeps everything consistent and easier to maintain.
42-46: Ohayo sensei! Verify that thetorii_urlis a valid gRPC endpoint.
You’re now constructing theWorldClientwithtorii_urland removing JSON-RPC. This is great for streamlining, but consider adding validation or logging if an invalid URL is passed.
| /// The grpc client. | ||
| inner: AsyncRwLock<torii_grpc::client::WorldClient>, | ||
| inner: RwLock<torii_grpc::client::WorldClient>, |
There was a problem hiding this comment.
Ohayo sensei! Ensure references to the removed world_reader field are cleaned up.
Excellent approach to storing WorldClient in a RwLock. However, referencing self.world_reader.address around line 216 looks like a leftover. This will cause a compile error now that world_reader is gone.
As a follow-up fix outside the changed lines, consider updating the code to reference the new client’s address field (assuming one exists). For instance:
- subscribe_indexer(contract_address.unwrap_or(self.world_reader.address))
+ subscribe_indexer(contract_address.unwrap_or(grpc_client.world_address))Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/torii/client/src/client/mod.rs (1)
62-65: Ohayo sensei! Consider using read lock for metadata fetch!While the change to fetch metadata directly is good, using a write lock (
write().await) might be unnecessary since we're only reading data. Consider usingread().awaitinstead for better concurrency.- let mut grpc_client = self.inner.write().await; + let grpc_client = self.inner.read().await;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/torii/client/src/client/mod.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: docs
- GitHub Check: build
- GitHub Check: ensure-wasm
🔇 Additional comments (4)
crates/torii/client/src/client/mod.rs (4)
8-8: Ohayo sensei! LGTM on the import simplification!The direct import of
RwLockimproves code readability by avoiding unnecessary type aliases.
30-30: Ohayo sensei! Clean struct refactoring!The field type change aligns with the import simplification, and the removal of
world_readersuccessfully achieves the PR's objective of removing RPC.
37-41: Ohayo sensei! Excellent initialization simplification!The removal of
rpc_urlparameter and direct gRPC client initialization aligns perfectly with the PR's objective of removing RPC dependencies.
208-208: Ohayo sensei! Good fallback handling!The change to use
unwrap_or_default()is a safe and clean way to handle the default case after removingworld_reader.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3019 +/- ##
==========================================
+ Coverage 56.24% 56.25% +0.01%
==========================================
Files 436 436
Lines 58829 58813 -16
==========================================
- Hits 33087 33086 -1
+ Misses 25742 25727 -15 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit