Skip to content

feat(platform-wallet): watermark monotonic-merge#3647

Open
lklimek wants to merge 1 commit into
v3.1-devfrom
feat/platform-wallet-watermark-monotonic-merge
Open

feat(platform-wallet): watermark monotonic-merge#3647
lklimek wants to merge 1 commit into
v3.1-devfrom
feat/platform-wallet-watermark-monotonic-merge

Conversation

@lklimek
Copy link
Copy Markdown
Contributor

@lklimek lklimek commented May 15, 2026

Issue being fixed or feature implemented

Incremental platform-address sync watermarks were updated by blind
assignment in PlatformPaymentAddressProvider::update_sync_state. When
sync results complete out of order (a stale incremental scan finishing
after a newer one), the older result could roll the watermark
backward, causing the wallet to re-scan or under-report sync
progress.

Standalone carve-out of cluster G from #3549 (the e2e PR), extracted
so this correctness fix can land independently of the e2e test suite.
SHA of origin: 59cba08af5 (+ merges).

What was done?

  • update_sync_state now merges each watermark field with max()
    (sync_height, sync_timestamp, last_known_recent_block) so the
    watermark is monotonic non-decreasing regardless of completion order.
  • Added PlatformPaymentAddressProvider::last_known_recent_block()
    (pub) — read-only mirror so wallet-level helpers can read the
    watermark without going through the AddressProvider trait.
  • Added PlatformAddressWallet::sync_watermark() -> Option<u64>
    (zero reported as None, matching the existing "no stored
    watermark" convention).
  • set_stored_sync_state documented as the unconditional load-time
    overwrite (monotonic invariant enforced at update-time, not load).

Only two production files changed: provider.rs, wallet.rs under
rs-platform-wallet/src/wallet/platform_addresses/. No e2e tests,
no rust-dashcore bump, no other clusters.

How Has This Been Tested?

  • New in-file #[cfg(test)] module: forward advance, full backward
    rejection, per-field merge (advance + regression + tie), and
    unconditional load-time overwrite.
  • cargo build -p platform-wallet — clean on v3.1-dev against
    rust-dashcore 53130869 (the pre-bump baseline; this cluster uses
    only integer max() and pre-existing types, so it does not
    depend on the test(rs-platform-wallet): e2e suite, Found-025 fix + triage pins #3549 rust-dashcore chainlock bump).
  • cargo clippy -p platform-wallet --all-targets — zero warnings.

Breaking Changes

None. Additive accessors plus a stricter (monotonic) internal merge.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Carved out of #3549 (cluster G). 🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added public methods to access and monitor sync watermark information.
  • Bug Fixes

    • Enhanced sync state handling with monotonic merges to prevent watermark regression when processing out-of-order or stale synchronization results.
  • Tests

    • Added comprehensive unit tests validating sync state management, watermark progression, and edge cases.

Review Change Stack

…accessor

Make incremental-sync watermark updates monotonic so concurrent or
out-of-order sync completions can never roll a watermark backward.
`update_sync_state` now takes the per-field `max()` of the existing
watermark and the incoming `AddressSyncResult` (height, timestamp,
last_known_recent_block) instead of blind assignment. Add a
`pub last_known_recent_block()` provider accessor and
`PlatformAddressWallet::sync_watermark()` so callers can read the
watermark without going through the `AddressProvider` trait. Includes
an in-file unit-test module covering forward advance, backward
rejection, per-field merge, and unconditional load-time overwrite.

Carved out of #3549 (cluster G, SHA 59cba08 + merges)
as a standalone production change. No e2e tests, no rust-dashcore bump:
verified `cargo build`/`cargo clippy -p platform-wallet` clean on
v3.1-dev against rust-dashcore 53130869 (the watermark logic uses only
integer max() and pre-existing types).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 54fc39b4-8fea-482d-a562-3f170c135a62

📥 Commits

Reviewing files that changed from the base of the PR and between 54322f7 and 541b329.

📒 Files selected for processing (2)
  • packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs
  • packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs

📝 Walkthrough

Walkthrough

This PR updates sync watermark handling in the platform address provider to use monotonic per-field merges during incremental updates, preventing watermark regression on out-of-order or stale sync completions. A new public last_known_recent_block() getter and async wallet-level sync_watermark() method expose the guarantee.

Changes

Watermark Monotonicity Guarantee

Layer / File(s) Summary
Provider watermark monotonic merge logic
packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs
update_sync_state now applies per-field max merges to sync_height, sync_timestamp, and last_known_recent_block instead of overwriting; documentation distinguishes monotonic update-time behavior from unconditional load-time overwrites via set_stored_sync_state.
Provider watermark accessor and validation tests
packages/rs-platform-wallet/src/wallet/platform_addresses/provider.rs
New public getter last_known_recent_block() exposes the watermark. Unit tests validate forward advancement, rejection of backwards/out-of-order results, per-field merge semantics, and unconditional overwrite of persisted state.
Wallet-level sync watermark exposure
packages/rs-platform-wallet/src/wallet/platform_addresses/wallet.rs
sync_watermark() async method reads the unified provider's watermark under a read lock, returns None when uninitialized or zero, otherwise Some(u64). Minor reformat of set_address_credit_balance call included.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

A rabbit hops with watermark in sight,
Per-field merges keep the path on height,
No more backslips when syncs arrive askew,
Monotonic guarantees, strong and true! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(platform-wallet): watermark monotonic-merge' accurately and specifically describes the main change: implementing monotonic-merge behavior for sync watermarks in the platform wallet.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/platform-wallet-watermark-monotonic-merge

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added this to the v3.1.0 milestone May 15, 2026
@lklimek lklimek changed the title feat(rs-platform-wallet): watermark monotonic-merge feat(platform-wallet): watermark monotonic-merge May 15, 2026
@lklimek lklimek marked this pull request as ready for review May 15, 2026 10:59
@lklimek lklimek requested a review from QuantumExplorer as a code owner May 15, 2026 10:59
@thepastaclaw
Copy link
Copy Markdown
Collaborator

thepastaclaw commented May 15, 2026

Review Gate

Commit: 541b329b

  • Debounce: 1162m ago (need 30m)

  • CI checks: build failure: PR title

  • CodeRabbit review: comment found

  • Off-peak hours: off-peak (11:12 PM PT Friday)

  • Run review now (check to override)

@lklimek lklimek added the ready for final review Ready for the final review. If AI was involved in producing this PR, it has already had a reviewer. label May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for final review Ready for the final review. If AI was involved in producing this PR, it has already had a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants