fix(sdk): address-sync no longer silently discards balance changes for post-snapshot addresses (Found-025)#3650
fix(sdk): address-sync no longer silently discards balance changes for post-snapshot addresses (Found-025)#3650lklimek wants to merge 2 commits into
Conversation
… for post-snapshot addresses (Found-025) `incremental_catch_up` built its `key_to_tag` lookup once from a single pre-RPC `provider.pending_addresses()` snapshot and passed it by immutable reference into both apply loops. The `if let Some(..) = address_lookup.get(..)` predicate had no `else`, so any balance change the platform returned for an address derived *after* the snapshot was dropped with no log, metric, or error — `result.found` never got it and `on_address_found` was never called. Under concurrent multi-identity funding the derive-fund-sync interleave is routine, which is why e2e gates TK-001/007/013/014 and id_005 flaked here. Extract the two inline apply loops into a pure `pub(crate) apply_address_changes` seam (no Sdk, no network, no async) that returns applied updates plus the addresses absent from the snapshot. The new `apply_block_changes` re-polls `pending_addresses()` when an unknown address appears (mirroring the tree-scan refresh) and replays only the previously-unknown subset, so a fresh receive address is recovered and known-address `AddToCredits` deltas are never double-counted. An address still unknown after the refresh is logged at `warn` — observable, never silently dropped. Known-address behavior is byte-for-byte identical. Adds three deterministic `#[cfg(test)]` regression guards on the pure seam (no proof/Sdk needed): unknown-address surfacing, post-snapshot recovery through the refresh, and delta double-count safety. All three fail on the pre-fix silent-discard logic and pass post-fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Review GateCommit:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3650 +/- ##
============================================
- Coverage 88.06% 88.05% -0.01%
============================================
Files 2521 2521
Lines 308995 308781 -214
============================================
- Hits 272122 271908 -214
Misses 36873 36873
🚀 New features to boost your workflow:
|
lklimek
changed the title
2 participants
Issue being fixed or feature implemented
Fixes the rs-sdk address-sync silent-discard defect (Found-025): the SDK
silently drops platform-returned balance updates for any address absent
from the pre-RPC
pending_addresses()snapshot.Summary
sync_address_balances(packages/rs-sdk/src/platform/address_sync/mod.rs)builds a
key_to_taglookup once from a single pre-RPCprovider.pending_addresses()snapshot and passes it by immutablereference into the private
incremental_catch_up. In both the compactedapply loop and the recent apply loop the predicate
if let Some(&(tag, address)) = address_lookup.get(&addr_bytes)had noelsebranch: any balance change the platform returned for an addressderived after the snapshot was dropped with no log, metric, or error —
result.foundnever got it andon_address_foundwas never called.Unlike the tree-scan path,
incremental_catch_upnever re-polledpending_addresses()(contrastafter_branch_iteration, which refreshesduring the branch loop).
Root cause
key_to_tagsnapshot: built once from a single pre-RPCprovider.pending_addresses()call and never refreshed insideincremental_catch_up.else: theif let Some(..) = address_lookup.get(..)predicatehad no fallback, so a snapshot miss was a no-op.
post-snapshot address went to neither
result.foundnoron_address_found, and the wallet's local sync map showed the addressempty.
Reproduction
Aafter thepass's
pending_addresses()snapshot is taken (routine underconcurrent multi-identity funding).
A; let it chain-confirm.A's balance, but theSDK discards it:
result.foundnever getsA,on_address_foundisnever called, the wallet's local sync map shows
Aempty.On single-threaded runs the derive usually precedes the snapshot, hiding
the bug — hence the symptom is flaky, not always red, in live e2e.
Under concurrent multi-identity funding the derive→fund→sync interleave is
routine, which is why the rs-platform-wallet e2e funding gates
TK-001 / TK-007 / TK-013 / TK-014 / id_005 flaked here (PASS
single-threaded, flaky under 14-thread concurrency).
What was done?
pub(crate) apply_address_changesseam — noSdk, no network, noasync. It returns the applied
(tag, address, funds)updates plus theaddresses the platform reported a change for that were absent from the
snapshot (
unknown), instead of silently dropping them.apply_block_changesdrives the asyncon_address_foundcallbacksoutside the pure seam, and on an unknown-address miss re-polls
provider.pending_addresses()(mirroring the existingafter_branch_iterationtree-scan refresh) and replays only thepreviously-unknown subset — so a freshly-derived receive address is
recovered while known-address
AddToCreditsdeltas are neverdouble-counted.
warn(observable, never silently dropped) per project
tracingconventions.(verified by the 27 pre-existing
address_synctests, all still green).How Has This Been Tested?
cargo build -p dash-sdk,cargo clippy -p dash-sdk --all-targets(clean),
cargo test -p dash-sdk --lib address_sync— 30/30 pass,including the 27 pre-existing address_sync tests (no regression from the
refactor).
Three new deterministic
#[cfg(test)]regression guards on the pure seam(no proof/Sdk/network):
found_025_apply_address_changes_surfaces_unknown_address— a knownaddress still applies identically; a post-snapshot address is surfaced
in
unknowninstead of silently dropped.found_025_apply_block_changes_recovers_post_snapshot_address— aprovider that derived an address mid-pass gets the balance applied and
on_address_foundfired after the in-pass refresh (the exact Found-025scenario).
found_025_known_delta_not_double_counted_on_refresh— a knownAddToCreditsdelta applies exactly once across the refresh+replay.Red-pre / green-post proof: restoring the silent-discard behavior
(drop the
else, no refresh) makes guards 1 and 2 FAIL(
unknown == [],result.found == None); the fix makes all three PASS.Would have caught Found-025 at unit level. The extracted
pub(crate)pure seam makes the discard a pure-data assertion (no
Sdk/proof) pinnedin
rs-sdk's own test module.This is the
v3.1-devproduction PR. Applying onto #3549 for combinede2e validation is a separate sequenced step.
Test plan
cargo build -p dash-sdkcargo clippy -p dash-sdk --all-targets(clean)cargo test -p dash-sdk --lib address_sync— 30/30 passfound_025_apply_address_changes_surfaces_unknown_addressfound_025_apply_block_changes_recovers_post_snapshot_addressfound_025_known_delta_not_double_counted_on_refreshsilent-discard logic)
Breaking Changes
None. Public API unchanged; the new seam is
pub(crate). Known-addressbehavior is identical.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Co-authored by Claudius the Magnificent AI Agent