Uh oh!
There was an error while loading. Please reload this page.
chore: re-merge v0.42-dev (real merge commit) - #699
Merged
QuantumExplorer merged 4 commits intoApr 28, 2026
Merged
Conversation
Co-authored-by: QuantumExplorer <11468583+QuantumExplorer@users.noreply.github.com>
* feat: make wallet events atomic
Reshape `WalletEvent` so each variant carries the records or context needed to persist a wallet update atomically off a single event, alongside the post-change balance.
The variant set is now:
- `TransactionReceived { wallet_id, record, balance }`. Fires when the wallet first sees an off-chain transaction.
- `TransactionStatusChanged { wallet_id, txid, context, balance }`. Fires when a known off-chain transaction has its state change. Currently fires only for InstantSend locks.
- `BlockUpdate { wallet_id, height, inserted, updated, matured, balance }`. Carries records bucketed by what happened to them in the block, plus the post-block balance.
- `SyncHeightUpdate { wallet_id, height }`. Marks a filter-batch checkpoint.
`TransactionRecord` carries `account_type` directly, identifying the owning account. `WalletInfoInterface` gains a `matured_coinbase_records` method that enumerates coinbase records crossing the maturity threshold during a height advance, populating `BlockUpdate.matured`.
The FFI groups the flattened account-discriminator fields into an `FFIAccountType` struct and renames the prior discriminant enum to `FFIAccountKind`.
* fix: record balance before bumping `block_processed_wallet_count`
Tests wait on `block_processed_wallet_count` and then read `last_confirmed`/`last_unconfirmed`. Bumping the counter before storing the balance snapshot left those reads racey. Reorder so the balance is recorded first.
Addresses CodeRabbit review comment on PR #696#696 (comment)
* fix: place `IdentityTopUp.registration_index` in `index_secondary`
The `FFIAccountType` doc states `index_secondary` carries `registration_index` for `IdentityTopUp` and `index = 0` for variants without a meaningful primary index, matching the parallel encoding in `FFIAccountKind::from_account_type`. The `From<&AccountType>` impl wrote `registration_index` into `index` instead, breaking the documented FFI contract.
Addresses CodeRabbit review comment on PR #696#696 (comment)
* fix: route confirmation backfills to `new_records`
`is_new` is wallet-wide (set on the first matching account, then breaks), so the per-account `else` branch can run for an account that did not previously hold the record. `confirm_transaction` backfills via `record_transaction` in that case, but the post-call record was always pushed onto `updated_records`, breaking the atomic `inserted`/`updated` contract consumed by `WalletEvent::BlockUpdate`. Capture `existed_before` per account and route to `new_records` when the record was just created.
Addresses CodeRabbit review comment on PR #696#696 (comment)
* refactor(key-wallet-manager): extract `finalize_block_advance` helper
`process_block` and `update_last_processed_height` duplicated the entire balance-snapshot, prior-heights collection, matured-coinbase window, height advance, and per-wallet `BlockUpdate` emission. Extract the shared tail into a private `WalletManager::finalize_block_advance` helper that takes the inserted/updated maps. `update_last_processed_height` becomes a one-line call with empty maps; `process_block` keeps only its txdata loop before delegating.
* refactor: rename wallet events for clearer semantics
Rename `WalletEvent` variants and the matching FFI callbacks to past-participle names that say what happened, replacing vague "Update" suffixes:
- `TransactionReceived` -> `TransactionDetected`. "Received" implied incoming funds, but the event fires for any first-time off-chain sighting (incoming or outgoing).
- `TransactionStatusChanged` -> `TransactionInstantLocked`. The event only ever fires for an InstantSend lock applied to a known mempool tx, so name it for what it actually is. Drop the `status: TransactionContext` field and carry the `InstantLock` directly.
- `BlockUpdate` -> `BlockProcessed`. Mirrors `process_block` and matches the past-participle pattern.
- `SyncHeightUpdate` -> `SyncHeightAdvanced`. Conveys monotonic forward motion.
FFI rename mirrors the Rust side: the IS callback now takes `islock_data: *const u8` + `islock_len: usize` instead of an `FFITransactionContext`, removing a discriminant that was always `InstantSend`. The wallet-side `OnBlockProcessedCallback` becomes `OnWalletBlockProcessedCallback` to disambiguate from the existing sync-event type with the same name.
* fix: record balance before bumping IS-locked counter in test callback
Addresses CodeRabbit review comment on PR #696#696 (review)
The instant_locked callback bumped `transaction_instant_send_locked_count` before calling `record_balance`. Tests that wait on the counter and then read `last_confirmed`/`last_unconfirmed` could observe the previous balance snapshot. Match the ordering used by the other callbacks: store the balance first, then bump the counter.
* fix: backfill missing transaction record in InstantSend path
Addresses CodeRabbit review comment on PR #696#696 (review)
The IS-lock branch in `WalletTransactionChecker::check_core_transaction` only updated accounts that already held a `TransactionRecord` for the txid. When wallet-level `is_new` was `false` (because at least one account had the record) but another matched account did not, the latter was silently skipped: no record was created and `mark_utxos_instant_send` ran against an empty UTXO set on that account.
Mirror the confirmation path: when the affected account lacks the record, call `record_transaction` to register the record and its UTXOs, then mark them IS-locked. This ordering ensures the freshly registered UTXOs receive the IS-lock flag too. The backfilled record is pushed into `new_records` to match the existing convention from commit 659a6d5.
Add `test_instantsend_backfills_missing_record_in_other_account` covering the multi-account scenario.…685) * feat(key-wallet-manager): add mutable-pair accessors + insert_wallet Adds three accessors on WalletManager that exploit the split borrow between the independent `wallets` and `wallet_infos` maps: - `get_wallet_and_info_mut` — `(&Wallet, &mut T)` for mutations that read wallet config while writing info (UTXOs, balance, tx history). - `get_wallet_mut_and_info_mut` — `(&mut Wallet, &mut T)` for cases that need to mutate the wallet itself (e.g. idempotent re-derivation of HD accounts during changeset replay). - `insert_wallet` — register a pre-built `(Wallet, T)` pair, returns `WalletExists` on duplicate and bumps `structural_revision` on success. Enables restore paths that reconstruct wallets from persistence without going through the mnemonic-based builder. Unit tests cover dedup error, split-borrow mutation across both maps, and the monitor-revision bump on insert. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(key-wallet-manager): drop redundant accessor tests Per ZocoLini's review on #685: the type system already proves the mutable-borrow accessors return the right shape, and monitor_revision is covered elsewhere. Keep insert_wallet_rejects_duplicate since it exercises actual error-path logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…into-per-wallet-filter-scan-v2
Contributor
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
QuantumExplorer
merged commit Apr 28, 2026
8f112f1
into
feat/per-wallet-filter-scan
32 of 33 checks passed
Uh oh!
There was an error while loading. Please reload this page.
QuantumExplorer
deleted the
merge/v0.42-dev-into-per-wallet-filter-scan-v2
branch
April 28, 2026 05:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #698. That PR was squash-merged, which flattened the two-parent merge commit into a single linear commit. As a result,
feat/per-wallet-filter-scan's history no longer recordsv0.42-devas an ancestor, and #694 re-computes the same conflicts even though the content is already integrated.This PR creates a real merge commit so the parent link is recorded.
Do NOT use "Squash and merge" — squashing again would recreate the same problem and leave #694 in
CONFLICTINGstate. Use the dropdown next to the merge button to pick "Create a merge commit".What's in this merge
key-wallet-manager/src/{lib,process_block,event_tests}.rs) get re-flagged by git's 3-way merge as conflicts because the squashed commit looks like an independent change to git, not a merge. Resolved withgit merge -X ourssincefeat/per-wallet-filter-scanalready has the correct merged content from #698.insert_wallet) which landed onv0.42-devafter #698 was merged. That's the only net content change —key-wallet-manager/src/accessors.rs+68 lines.Verified
cargo build --workspacecargo test -p key-wallet-manager— 31 unit + integration tests passa8e381ad(current feat tip) +da28aca1(current v0.42-dev tip), so #694's merge-base will bev0.42-dev's tip, eliminating the phantom conflicts.Test plan
MERGEABLE🤖 Generated with Claude Code