Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/rs-sdk/src/platform/address_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,13 @@ pub async fn sync_address_balances<P: AddressProvider>(
start_height
);
for (index, key, funds) in provider.current_balances() {
result.found.insert((index, key), funds);
result.found.insert((index, key.clone()), funds);
// Notify the provider so it can update its internal state
// (e.g., populate found_balances, extend gap limit). Without this,
// addresses discovered during a previous full scan are invisible to
// the consumer in incremental-only mode because on_address_found
// is the only path that populates the provider's found set.
provider.on_address_found(index, &key, funds);
}
Comment on lines 343 to 351
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: No test covers the incremental-only sync path where this bug lived

The existing test_sync_address_balances test passes None for last_sync_timestamp, which always triggers a full tree scan. The bug being fixed only manifests in incremental-only mode, yet no test exercises that path.

To regress-protect this fix, a test would need to:

  1. Implement current_balances() and last_sync_height() on the test provider (the current TestAddressProvider returns empty defaults for both, making incremental-only mode unreachable)
  2. Pass Some(recent_timestamp) to sync_address_balances to trigger needs_full_scan == false
  3. Assert that provider.found is populated after sync — i.e., on_address_found was called for each seeded balance

Without this, the fix could silently regress in a future refactor since the existing suite only validates the full-scan code path.

source: ['claude', 'codex']

🤖 Fix this with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `packages/rs-sdk/src/platform/address_sync/mod.rs`:
- [SUGGESTION] lines 343-351: No test covers the incremental-only sync path where this bug lived
  The existing `test_sync_address_balances` test passes `None` for `last_sync_timestamp`, which always triggers a full tree scan. The bug being fixed only manifests in incremental-only mode, yet no test exercises that path.

To regress-protect this fix, a test would need to:
1. Implement `current_balances()` and `last_sync_height()` on the test provider (the current `TestAddressProvider` returns empty defaults for both, making incremental-only mode unreachable)
2. Pass `Some(recent_timestamp)` to `sync_address_balances` to trigger `needs_full_scan == false`
3. Assert that `provider.found` is populated after sync — i.e., `on_address_found` was called for each seeded balance

Without this, the fix could silently regress in a future refactor since the existing suite only validates the full-scan code path.

start_height
} else {
Expand Down
Loading