feat: add unbounded partial addresses matching over address filters #953
Conversation
WalkthroughThe changes introduce and refine support for partial account address matching using the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Storage
participant Utils
Client->>API: Query accounts/transactions with address filter (e.g., "wallet:xxx:...")
API->>Storage: Build dataset/query with address filter
Storage->>Utils: Call isPartialAddress(address)
Utils-->>Storage: Return true if address ends with "..."
Storage->>Storage: Construct query for partial match
Storage-->>API: Return filtered results
API-->>Client: Respond with filtered accounts/transactions
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (1)
internal/storage/ledger/transactions.go (1)
297-344: Consider adding input validation for malformed addresses.While the current logic handles the main use cases correctly, consider adding validation for potentially malformed address patterns to provide better error handling.
func filterAccountAddressOnTransactions(address string, source, destination bool) string { + if address == "" { + return "" + } + src := strings.Split(address, ":")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.modis excluded by!**/*.modtools/generator/go.modis excluded by!**/*.mod
📒 Files selected for processing (6)
internal/storage/ledger/accounts_test.go(1 hunks)internal/storage/ledger/resource_aggregated_balances.go(2 hunks)internal/storage/ledger/resource_volumes.go(1 hunks)internal/storage/ledger/transactions.go(2 hunks)internal/storage/ledger/transactions_test.go(1 hunks)internal/storage/ledger/utils.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/storage/ledger/transactions_test.go (2)
internal/storage/common/resource.go (2)
ColumnPaginatedQuery(406-415)ResourceQuery(359-365)internal/transaction.go (2)
Transaction(38-50)Transaction(52-59)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Tests
- GitHub Check: Dirty
🔇 Additional comments (10)
internal/storage/ledger/transactions_test.go (1)
780-788: LGTM! Good test coverage for unbounded partial address matching.The new test case correctly validates the "..." syntax for partial address matching. The expected result
[tx5, tx4, tx3]aligns with the test data where all these transactions involve accounts with the "users:" prefix.internal/storage/ledger/resource_volumes.go (2)
69-71: LGTM! Good type safety improvement.The explicit string casting before calling
isPartialAddressimproves type safety and makes the code more robust.
113-115: LGTM! Consistent type safety pattern.The same type safety improvement is applied consistently throughout the file, which is good for maintainability.
internal/storage/ledger/accounts_test.go (1)
182-191: LGTM! Good test coverage for unbounded partial address matching.The new test case correctly validates the "..." syntax for account address filtering. The expected result of 3 accounts matches the test data where accounts "account:1", "account:2", and "account:3" exist.
internal/storage/ledger/resource_aggregated_balances.go (2)
77-79: LGTM! Good type safety improvement.The explicit string casting before calling
isPartialAddressimproves type safety and makes the code more robust. This change is consistent with the pattern used in other resource handlers.
113-115: LGTM! Consistent type safety pattern.The same type safety improvement is applied consistently, matching the approach used in the PIT branch above and in other resource handlers.
internal/storage/ledger/utils.go (2)
8-21: LGTM! Enhanced partial address detection logic.The updated
isPartialAddressfunction correctly implements unbounded partial matching by checking if the last segment is "...". The logic properly handles both traditional partial addresses (with empty segments) and the new unbounded syntax.
33-34: Consistent filtering logic for unbounded partial addresses.The updated condition correctly skips both empty segments and "..." segments during filtering, which aligns with the unbounded partial matching feature. This ensures that "..." segments don't generate specific position constraints.
internal/storage/ledger/transactions.go (2)
300-301: Improved consistency by usingisPartialAddressfunction.Good refactoring to use the centralized
isPartialAddressfunction instead of manual segment checking. The change from initializingmwith a length entry to an empty map is correct for unbounded partial address handling.
308-315: Correct implementation of unbounded partial address logic.The logic properly handles unbounded partial addresses by:
- Breaking early when "..." is encountered as the last segment (lines 308-310)
- Only adding the length constraint when the last segment is not "..." (lines 313-315)
This ensures that addresses ending with "..." can match any number of trailing segments.
| if src[len(src)-1] != "" { | ||
| parts = append(parts, fmt.Sprintf("jsonb_array_length(%s_array) = %d", key, len(src))) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify edge case handling for address ending with ":"
The condition src[len(src)-1] != "" prevents adding the array length constraint when the last segment is "...". However, consider the edge case where an address ends with ":" (e.g., "account:"), which would create an empty last segment.
🏁 Script executed:
#!/bin/bash
# Test edge cases for addresses ending with colon
echo "Testing address patterns that end with colon..."
# Search for test cases that might cover this scenario
rg -A 10 -B 2 ".*:\"?\s*$" --type go
# Look for existing test coverage of edge cases
ast-grep --pattern 'func $_(t *testing.T) {
$$$
address := "$_:"
$$$
}'Length of output: 72841
#!/bin/bash
Show the implementation of filterAccountAddress in utils.go
rg -n "func filterAccountAddress" -A 50 internal/storage/ledger/utils.go
Also dump the file around the array-length check
sed -n '1,200p' internal/storage/ledger/utils.go | sed -n '1,80p'
#!/bin/bash
Extract the full implementation of filterAccountAddress to inspect edge-case handling
rg -n "func filterAccountAddress" -A50 -B2 internal/storage/ledger/utils.go
Add tests and clarify trailing-colon behavior in filterAccountAddress
The if src[len(src)-1] != "" check in internal/storage/ledger/utils.go (lines ~28–30) intentionally skips the length constraint for addresses ending with “:”, but we have no coverage or documentation for this edge case.
• File: internal/storage/ledger/utils.go, around lines 27–30
if src[len(src)-1] != "" {
parts = append(parts, fmt.Sprintf("jsonb_array_length(%s_array) = %d", key, len(src)))
}• Add unit tests for inputs like "account:" to verify the generated SQL (it should only enforce prefix matching, no array-length check).
• Document in code (or GoDoc) that a trailing colon enables prefix/sub-account matching by design.
🤖 Prompt for AI Agents
In internal/storage/ledger/utils.go around lines 27 to 30, the condition
src[len(src)-1] != "" skips adding the array length constraint for addresses
ending with a colon, but this behavior is undocumented and lacks test coverage.
Add unit tests for inputs like "account:" to verify that the generated SQL
enforces only prefix matching without the array-length check. Also, update the
function's comments or GoDoc to clearly document that a trailing colon enables
prefix or sub-account matching by design.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #953 +/- ##
==========================================
+ Coverage 82.36% 82.42% +0.06%
==========================================
Files 141 141
Lines 7829 7835 +6
==========================================
+ Hits 6448 6458 +10
+ Misses 1056 1054 -2
+ Partials 325 323 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Fixes LX-46