-
Notifications
You must be signed in to change notification settings - Fork 189
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
fix(torii/graphql): filter out tokens with zero balance #2769
Conversation
WalkthroughOhayo, sensei! This pull request modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ErcBalanceObject
participant Database
Client->>ErcBalanceObject: Request token balances
ErcBalanceObject->>Database: Execute SQL query with non-zero balance filter
Database-->>ErcBalanceObject: Return non-zero token balances
ErcBalanceObject-->>Client: Send token balances
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (1)
crates/torii/graphql/src/object/erc/token_balance.rs (1)
109-111
: Ohayo sensei! Consider extracting the zero balance constant.The implementation correctly filters out zero balances, but the hardcoded hex string could be moved to a constant for better maintainability and reusability.
Consider applying this change:
+ const ZERO_BALANCE: &str = "0x0000000000000000000000000000000000000000000000000000000000000000"; // In fetch_token_balances: let mut conditions = vec![ "(b.account_address = ?)".to_string(), "(t.metadata IS NULL OR length(t.metadata) > 0)".to_string(), - "b.balance != '0x0000000000000000000000000000000000000000000000000000000000000000'" + format!("b.balance != '{}'", ZERO_BALANCE) .to_string(), ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
crates/torii/graphql/src/object/erc/token_balance.rs
(1 hunks)
🔇 Additional comments (1)
crates/torii/graphql/src/object/erc/token_balance.rs (1)
109-111
: LGTM! Pagination handling is correct.
The zero balance filter is properly integrated with the existing WHERE clause and pagination mechanism. The implementation correctly:
- Maintains cursor-based navigation
- Updates total_count to exclude zero balances
- Preserves the query structure
Let's verify the query construction:
✅ Verification successful
Ohayo sensei! The pagination implementation is correctly handling the zero balance filter! ✨
The code shows that:
- The zero balance filter is properly added to the
conditions
vector along with other base conditions - All conditions are joined with
AND
in the WHERE clause - The cursor-based pagination logic (
after
/before
) correctly adds its conditions after the zero balance filter - The query construction maintains proper order of operations
This implementation ensures that:
- Zero balances are filtered out consistently
- Pagination cursors work correctly with the filter
- The WHERE clause is properly constructed with all conditions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the SQL query construction and pagination
# Test: Check if the zero balance filter is properly combined with other conditions
rg -A 5 "WHERE.*AND.*balance.*!=" crates/torii/graphql/src/object/erc/token_balance.rs
Length of output: 88
Script:
#!/bin/bash
# Let's check the query construction more broadly
rg -B 5 -A 10 "WHERE" crates/torii/graphql/src/object/erc/token_balance.rs
# Also check how conditions are built
rg -B 3 -A 3 "conditions" crates/torii/graphql/src/object/erc/token_balance.rs
Length of output: 1702
Script:
#!/bin/bash
# Let's check the full token balance query construction
rg -C 20 "let mut conditions = vec!" crates/torii/graphql/src/object/erc/token_balance.rs
Length of output: 1526
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2769 +/- ##
==========================================
- Coverage 55.49% 55.48% -0.01%
==========================================
Files 434 434
Lines 54312 54318 +6
==========================================
Hits 30138 30138
- Misses 24174 24180 +6 ☔ View full report in Codecov by Sentry. |
commit-id:11f63cc1
f4073bc
to
683d381
Compare
@lambda-0x we're sure the value is always padded this way, correct? |
Summary by CodeRabbit
New Features
Bug Fixes