Skip to content

feat(platform-wallet): anchor fresh wallets at a checkpoint, set birth height on import#4063

Open
xdustinface wants to merge 6 commits into
v4.1-devfrom
feat/wallet-birth-height-ui
Open

feat(platform-wallet): anchor fresh wallets at a checkpoint, set birth height on import#4063
xdustinface wants to merge 6 commits into
v4.1-devfrom
feat/wallet-birth-height-ui

Conversation

@xdustinface

@xdustinface xdustinface commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
  • Fresh wallet: a wallet created at app startup registers before SPV has synced any headers, so the tip read as 0 and it anchored at genesis and rescanned the whole chain. It now falls back to the network's latest checkpoint, so a new wallet syncs from near the chain head.
  • Imported wallet: thread a birth_height through the swift-sdk import FFI and the example app, so a recovered wallet scans from its birth height instead of from scratch.
  • Dependency: bump rust-dashcore to the branch carrying the wallet birth-height checkpoint anchoring engine (dash-spv).

Summary by CodeRabbit

  • New Features

    • Added an optional Birth height field when importing an existing wallet, allowing users to choose the sync start point.
    • Wallet import now carries the selected birth height through to the underlying SDK.
  • Bug Fixes

    • Fresh wallet syncing now prefers the latest available checkpoint when the SPV tip height isn’t usable, reducing unnecessary catch-up from genesis.
    • Import now safely defaults when the height field is empty, and importing a provided birth height across multiple networks is now blocked with a clear error.

The import flow already anchored at genesis (birthHeight 0) for imported wallets, forcing a full-chain scan. Add an optional birth-height field so the user can pin a height instead: SPV then anchors at the nearest checkpoint at or before it and skips the earlier chain. Left empty it still falls back to genesis so a wallet of unknown age sees all its history. A freshly generated wallet is unchanged (nil, scans from the tip).
…from_bytes`

The key-wallet-ffi import-from-bytes export gained a `birth_height` parameter, so the Swift wrapper must supply it. Add an optional `birthHeight` (default 0 = scan from genesis) and forward it.
…PV tip unknown

A wallet created at app startup registers before SPV has synced any headers, so the header tip reads 0 and the fresh-wallet birth height fell back to genesis, making the client rescan the whole chain. Fall back to the latest hardcoded checkpoint for the network instead, so a new wallet anchors near the chain head.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@xdustinface, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4fd57c7d-d85d-4e17-8c6d-394ed302609c

📥 Commits

Reviewing files that changed from the base of the PR and between d529dad and 6cff00e.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • Cargo.toml
📝 Walkthrough

Walkthrough

This PR updates dashcore-family dependency revisions, changes wallet birth-height fallback to use checkpoints when SPV tip height is unavailable, and threads an optional birth height through the Swift import API and example app import flow.

Changes

Wallet birth-height resolution and import wiring

Layer / File(s) Summary
Dependency revision bump
Cargo.toml
Pins the dashcore-family workspace dependencies to a new git revision.
Checkpoint fallback in wallet lifecycle
packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs
Adds CheckpointManager, updates the birth-height documentation, and changes wallet registration to fall back to the latest checkpoint height when SPV tip height is not positive.
Swift import API birth height
packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift
Adds a birthHeight parameter to importWallet and passes it through to the FFI import call.
Example app import birth height
packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift
Adds optional import birth-height state and UI, parses the entered value, and forwards it into imported wallet creation.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CreateWalletView
  participant WalletManager
  participant register_wallet
  participant CheckpointManager

  User->>CreateWalletView: Enter birth height and import wallet
  CreateWalletView->>WalletManager: createWallet(birthHeight)
  WalletManager->>register_wallet: register_wallet(birth_height_override)
  register_wallet->>register_wallet: resolve SPV tip height
  alt tip > 0
    register_wallet-->>WalletManager: use SPV tip height
  else tip unavailable or 0
    register_wallet->>CheckpointManager: last_checkpoint()
    CheckpointManager-->>register_wallet: checkpoint height
    register_wallet-->>WalletManager: use checkpoint height
  end
  WalletManager-->>CreateWalletView: wallet created
Loading

Possibly related PRs

Suggested labels: ready for final review

Suggested reviewers: QuantumExplorer, shumkov, thepastaclaw

🚥 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 accurately summarizes the two main changes: checkpoint anchoring for fresh wallets and birth height handling for imports.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/wallet-birth-height-ui

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.

Point the rust-dashcore git deps at the `dev` tip, which carries the merged wallet birth-height checkpoint anchoring (#848) that the fresh-wallet checkpoint fallback relies on.
@xdustinface xdustinface marked this pull request as ready for review July 9, 2026 14:21
@xdustinface xdustinface force-pushed the feat/wallet-birth-height-ui branch from a6d79e0 to 6150161 Compare July 9, 2026 14:23
@thepastaclaw

thepastaclaw commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

✅ Review complete (commit 6cff00e)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs`:
- Around line 153-177: Update the doc comments for create_wallet_from_mnemonic
and create_wallet_from_seed_bytes so they match the current birth-height logic
in wallet_lifecycle::create_wallet_from_mnemonic/create_wallet_from_seed_bytes:
when birth_height_override is absent, the wallet uses the SPV confirmed header
tip if available, and otherwise falls back to the latest network checkpoint via
CheckpointManager instead of genesis/0. Keep the wording aligned with the actual
fallback path used in the birth_height computation so the comments no longer
describe the old behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0e8591bf-00b5-4463-aad3-87a3aad0315f

📥 Commits

Reviewing files that changed from the base of the PR and between 9b5aee8 and 6150161.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • Cargo.toml
  • packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs
  • packages/swift-sdk/Sources/SwiftDashSDK/KeyWallet/WalletManager.swift
  • packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift

Comment thread packages/rs-platform-wallet/src/manager/wallet_lifecycle.rs

@thepastaclaw thepastaclaw left a comment

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.

Code Review

Source: reviewers claude-general=opus (failed: extra-usage quota), codex-general=gpt-5.5, claude-ffi-engineer=opus (failed: extra-usage quota), codex-ffi-engineer=gpt-5.5; verifier=codex gpt-5.5; specialists=ffi-engineer.

Verified the actionable agent finding against the PR diff and the wallet manager implementation. The PR adds a single import birth-height field and passes that same explicit override to every selected network, while the manager treats the override as the per-network SPV scan start. CodeRabbit has no actionable inline findings.

🔴 1 blocking

🤖 Prompt for all review comments 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/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift`:
- [BLOCKING] packages/swift-sdk/SwiftExampleApp/SwiftExampleApp/Core/Views/CreateWalletView.swift:370-379: Single birth height is applied to every selected network
  When an imported mnemonic is created for multiple selected networks, the UI parses one `importBirthHeight` value and passes it unchanged into each network-specific wallet manager. Birth heights are chain-local, and the Rust manager treats `Some(h)` as an explicit scan-start override for that manager, bypassing its own network tip/checkpoint fallback. A height that is correct for one network can be after the wallet's first funding on another network, causing that wallet to start scanning too late and miss earlier history. Collect a birth height per selected network, or reject a non-empty birth height when importing to more than one network; leaving it blank should remain the safe multi-network path because it passes `0` for imported wallets.

…k import, refresh stale docs

Importing a mnemonic to more than one selected network passed a single user-entered birth height unchanged into every network's `createWallet`. Birth heights are chain-local, so a height correct for one network anchors another network's scan too late and misses earlier history. `CreateWalletView` now rejects a non-empty birth height when more than one network is selected, keeping the safe blank-to-genesis path as the multi-network default.

Also refresh the `create_wallet_from_mnemonic` and `create_wallet_from_seed_bytes` doc comments, which still described the old genesis fallback for the SPV-unavailable case that now anchors at the latest network checkpoint.

@thepastaclaw thepastaclaw left a comment

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.

Code Review

Source: reviewers = claude ffi-engineer opus (failed); claude general opus (failed); codex ffi-engineer gpt-5.5; codex general gpt-5.5; verifier = codex gpt-5.5; specialists = ffi-engineer; policy gate = review_policy.enforce_backport_prereq_policy.

Prior reconciliation: prior-1 is fixed; the current Swift code rejects a non-empty import birth height when more than one network is selected before entering the wallet creation loop. Carried-forward prior findings: none. New findings in latest delta: none; the latest delta, cumulative PR diff, and CodeRabbit context produced no verified in-scope issues.

@thepastaclaw thepastaclaw left a comment

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.

Code Review

Source: reviewers = claude-general=opus (failed); codex-general=gpt-5.5; claude-ffi-engineer=opus (failed); codex-ffi-engineer=gpt-5.5; verifier=codex gpt-5.5; specialists=ffi-engineer; policy gate=review_policy.enforce_backport_prereq_policy.

Prior reconciliation: previous d529dadb review found no unresolved findings, so there are no carried-forward prior findings. Historical prior-1 remains fixed unless a finding below states otherwise. New findings in latest delta: none; cumulative current-head review produced no verified in-scope issues.

Previous exact-SHA review at d529dad had no unresolved findings, and I found no regression at 6cff00e. The latest delta only bumps the rust-dashcore revision; cumulative wallet birth-height behavior still rejects a non-empty birth height across multiple selected networks. cargo check -p platform-wallet could not complete because fetching the pinned git dependency requires network access and the escalation request was rejected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants