feat(key-wallet): add keep_txs_in_memory Cargo feature

[QuantumExplorer]

Summary

• Adds an opt-in Cargo feature keep_txs_in_memory (NOT enabled by default) that gates the ManagedCoreAccount::transactions field at compile time. With the feature off, the field doesn't exist and processing transactions only updates UTXOs and balance.
• key-wallet-ffi enables the feature explicitly to keep its FFI surface unchanged. Other crates can opt in the same way.
• Exposes ManagedCoreAccount::has_transaction(&Txid) / get_transaction(&Txid) helpers that work in both configurations (returning false / None when the feature is off) so call sites stay readable.
• Routes the production paths in wallet_checker and wallet_info_interface through the helpers or #[cfg] blocks. Wallet-level history queries (transaction_history, immature_transactions, matured_coinbase_records) return empty Vecs when the feature is off.
• Trait methods ManagedAccountTrait::transactions() / transactions_mut() are also feature-gated.
• Marks the key-wallet dev-dep self-reference as default-features = false so the feature can actually be disabled when running tests.

Test plan

• cargo build --workspace (default features)
• cargo test --workspace --no-run builds all tests across the workspace
• cargo test -p key-wallet --lib (feature off) — 434 passed
• cargo test -p key-wallet --features keep_txs_in_memory --lib — 464 passed
• cargo test -p key-wallet --all-features --lib — 469 passed
• cargo clippy -p key-wallet --tests and --features keep_txs_in_memory --tests — clean
• cargo clippy --workspace --tests — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

• Chores
  • Introduced a build-time option to control in-memory transaction history, allowing memory-optimized builds while preserving transaction processing and balance accuracy.
• New Features
  • Wallet APIs now adapt when transaction history is disabled (history queries return empty results).
• Bug Fixes
  • Ensures re-processing the same transaction does not double-count UTXOs or balances.
• Tests
  • Added feature-gated tests validating balance/UTXO correctness and record behavior.

[coderabbitai]

Warning

Rate limit exceeded

@QuantumExplorer has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 38 minutes and 22 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 42d18170-90b7-421c-a598-50de2740cff3

📥 Commits

Reviewing files that changed from the base of the PR and between ce2ba4b and ed46023.

📒 Files selected for processing (12)

• key-wallet-ffi/Cargo.toml
• key-wallet-manager/Cargo.toml
• key-wallet/Cargo.toml
• key-wallet/src/managed_account/managed_account_trait.rs
• key-wallet/src/managed_account/managed_core_funds_account.rs
• key-wallet/src/managed_account/managed_core_keys_account.rs
• key-wallet/src/test_utils/wallet.rs
• key-wallet/src/tests/keep_txs_in_memory_tests.rs
• key-wallet/src/tests/mod.rs
• key-wallet/src/transaction_checking/transaction_router/tests/mod.rs
• key-wallet/src/transaction_checking/wallet_checker.rs
• key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs

📝 Walkthrough

Walkthrough

Per-account in-memory transaction retention for ManagedCoreAccount is now controlled by a new Cargo feature keep_txs_in_memory. When disabled, TransactionRecord storage and related accessors are omitted at compile time; deduplication and UTXO/balance updates continue via always-present processed_txids and reconstructed spent_outpoints. Serde, tests, and downstream manifests were updated accordingly.

Changes

In-memory transaction feature gating (single cohort)

Layer / File(s)	Summary
Feature & Manifests key-wallet/Cargo.toml, key-wallet-ffi/Cargo.toml, key-wallet-manager/Cargo.toml	Adds keep_txs_in_memory = [] to key-wallet; downstream crates opt into the feature. key-wallet dev self-dependency now sets default-features = false and enables getrandom among dev features.
Data Shape key-wallet/src/managed_account/mod.rs	transactions: BTreeMap<Txid, TransactionRecord> is present only under #[cfg(feature = "keep_txs_in_memory")]; processed_txids, spent_outpoints, and other core fields remain unconditional. new initializes transactions conditionally.
Public API / Accessors key-wallet/src/managed_account/mod.rs, key-wallet/src/managed_account/managed_account_trait.rs	has_transaction now reports from processed_txids unconditionally; get_transaction returns Some only when the feature is enabled. Trait method transactions(&self) is feature-gated; transactions_mut(&mut self) remains unconditional.
Core Logic Changes key-wallet/src/managed_account/mod.rs, key-wallet/src/transaction_checking/wallet_checker.rs	record_transaction always updates processed_txids and computes UTXO/balance state but inserts/upgrades TransactionRecord into transactions only when the feature is enabled. confirm_transaction and callers in wallet_checker no longer assume an in-memory record exists and use has_transaction/get_transaction or feature-gated branches for InstantSend and confirmation-path updates.
Serde / Persistence key-wallet/src/managed_account/mod.rs	Deserialization uses a Helper with #[serde(default)] transactions; spent_outpoints and processed_txids are rebuilt from helper data and transactions is populated conditionally under the feature.
Wallet info / Querying key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs	Methods that return transaction-history-derived results (transaction_history, immature_transactions, matured_coinbase_records) are feature-gated: return in-memory-derived results when enabled, otherwise return empty collections. mark_instant_send_utxos updates per-transaction context only when the feature is enabled while still marking UTXOs and updating cached balance.
Tests & Test Utilities key-wallet/src/tests/*, key-wallet/src/test_utils/wallet.rs, key-wallet/src/transaction_checking/transaction_router/tests/mod.rs	New tests exercise behavior with feature enabled/disabled; test modules and helpers referencing TransactionRecord are conditionally compiled under the feature. Some existing tests now also mutate processed_txids in setups to simulate missing-record scenarios.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I stash some hops behind a little flag,
some carrots kept, some left to lag.
UTXOs hop on, records may hide,
balances bloom whatever’s applied.
A twitch, a build — a rabbit’s pride. 🥕

🚥 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 clearly and concisely summarizes the main change: adding a new Cargo feature to control in-memory transaction retention in the key-wallet crate.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/gracious-dirac-44a53b

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 your free trial and get 200 agent minutes per Slack user (a $50 value).

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

❌ Patch coverage is 94.59459% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.06%. Comparing base (806ec4d) to head (ed46023).

Files with missing lines	Patch %	Lines
...t/src/managed_account/managed_core_keys_account.rs	91.17%	3 Missing ⚠️
...allet/managed_wallet_info/wallet_info_interface.rs	96.77%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           v0.42-dev     #709      +/-   ##
=============================================
- Coverage      71.17%   71.06%   -0.12%     
=============================================
  Files            320      320              
  Lines          68610    68647      +37     
=============================================
- Hits           48836    48785      -51     
- Misses         19774    19862      +88     

Flag	Coverage Δ
core	76.19% <ø> (ø)
ffi	44.89% <ø> (-0.86%)	⬇️
rpc	20.00% <ø> (ø)
spv	87.44% <ø> (-0.09%)	⬇️
wallet	69.84% <94.59%> (+0.07%)	⬆️

Files with missing lines	Coverage Δ
...allet/src/managed_account/managed_account_trait.rs	37.24% <ø> (ø)
.../src/managed_account/managed_core_funds_account.rs	77.54% <100.00%> (+1.75%)	⬆️
...-wallet/src/transaction_checking/wallet_checker.rs	99.38% <100.00%> (ø)
...allet/managed_wallet_info/wallet_info_interface.rs	83.22% <96.77%> (ø)
...t/src/managed_account/managed_core_keys_account.rs	66.07% <91.17%> (+10.94%)	⬆️

... and 18 files with indirect coverage changes

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

key-wallet/src/tests/keep_txs_in_memory_tests.rs (1)

52-75: ⚡ Quick win

Add a re-confirmation idempotency test to lock in correct behavior.

The existing test verifies single-pass behavior well, but there's no coverage for calling confirm_transaction (or the equivalent through the wallet pipeline) a second time for the same transaction when keep_txs_in_memory is false. Such a test would catch the deduplication regression identified above and serve as a regression guard once it's fixed.

📝 Suggested additional test

#[tokio::test]
async fn confirm_transaction_is_idempotent_when_disabled() {
    let mut ctx = TestWalletContext::new_random();
    ctx.managed_wallet
        .first_bip44_managed_account_mut()
        .expect("Should have BIP44 account")
        .set_keep_txs_in_memory(false);

    let tx = Transaction::dummy(&ctx.receive_address, 0..1, &[200_000]);

    // First pass: mempool
    let r1 = ctx.check_transaction(&tx, TransactionContext::Mempool).await;
    assert!(r1.is_relevant);

    // Second pass: block confirmation of the same tx
    let r2 = ctx.check_transaction(&tx, TransactionContext::InBlock(/* block info */)).await;

    let account = ctx.bip44_account();
    // UTXO count must not double
    assert_eq!(account.utxos.len(), 1);
    // Balance must remain the same
    assert_eq!(account.balance.spendable(), 200_000);
    // monitor_revision must not be bumped on the second pass if no UTXO actually changed
    // (once the deduplication bug is fixed)
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@key-wallet/src/tests/keep_txs_in_memory_tests.rs` around lines 52 - 75, Add
an idempotency test that confirms calling ctx.check_transaction twice for the
same Transaction (first with TransactionContext::Mempool, then with
TransactionContext::InBlock) does not double-count UTXOs or change balance when
keep_txs_in_memory is false: create a TestWalletContext, call
.first_bip44_managed_account_mut().set_keep_txs_in_memory(false), build the same
Transaction::dummy, call ctx.check_transaction(&tx,
TransactionContext::Mempool).await then ctx.check_transaction(&tx,
TransactionContext::InBlock(...)).await, then assert on
bip44_account().utxos.len() == 1 and bip44_account().balance.spendable() ==
200_000 and (if available) that bip44_account().monitor_revision was not
incremented on the second pass; name the test
confirm_transaction_is_idempotent_when_disabled and add it to
keep_txs_in_memory_tests.rs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@key-wallet/src/managed_account/mod.rs`:
- Around line 594-596: The deduplication bug occurs because confirm_transaction
currently checks self.transactions.contains_key(&tx.txid()) which is empty when
keep_txs_in_memory is false; add a new field processed_txids: HashSet<Txid>
(annotated like spent_outpoints with serde(skip_serializing)) to track seen
txids independent of stored transactions, always insert tx.txid() into
processed_txids inside record_transaction (even when keep_txs_in_memory is
false), change confirm_transaction to use processed_txids.contains(&tx.txid())
as the early-return guard, initialize processed_txids = HashSet::new() in new()
and reconstruct it from helper.transactions during Deserialize (same place
transactions and spent_outpoints are rebuilt) so behavior is consistent after
load.

---

Nitpick comments:
In `@key-wallet/src/tests/keep_txs_in_memory_tests.rs`:
- Around line 52-75: Add an idempotency test that confirms calling
ctx.check_transaction twice for the same Transaction (first with
TransactionContext::Mempool, then with TransactionContext::InBlock) does not
double-count UTXOs or change balance when keep_txs_in_memory is false: create a
TestWalletContext, call
.first_bip44_managed_account_mut().set_keep_txs_in_memory(false), build the same
Transaction::dummy, call ctx.check_transaction(&tx,
TransactionContext::Mempool).await then ctx.check_transaction(&tx,
TransactionContext::InBlock(...)).await, then assert on
bip44_account().utxos.len() == 1 and bip44_account().balance.spendable() ==
200_000 and (if available) that bip44_account().monitor_revision was not
incremented on the second pass; name the test
confirm_transaction_is_idempotent_when_disabled and add it to
keep_txs_in_memory_tests.rs.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 31ffa800-a57f-42b1-886b-975dd1a4bcff

📥 Commits

Reviewing files that changed from the base of the PR and between ca507a9 and 7896c7f.

📒 Files selected for processing (3)

• key-wallet/src/managed_account/mod.rs
• key-wallet/src/tests/keep_txs_in_memory_tests.rs
• key-wallet/src/tests/mod.rs

[coderabbitai]

♻️ Duplicate comments (1)

key-wallet/src/managed_account/mod.rs (1)

103-117: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Deduplication is still broken when keep_txs_in_memory is disabled.

Line 114-Line 115 makes has_transaction always false, and Line 506-Line 507 makes confirm_transaction always re-record and report a change. The same tx replay (e.g., mempool → block, rescans) is treated as new repeatedly, which can inflate wallet-level state-change signaling and transaction counters downstream.

🔧 Fix direction (dedupe independent of tx record retention)

 pub struct ManagedCoreAccount {
+    /// Txids processed by this account regardless of record retention policy.
+    #[cfg_attr(feature = "serde", serde(skip_serializing))]
+    processed_txids: HashSet<Txid>,
     #[cfg(feature = "keep_txs_in_memory")]
     pub transactions: BTreeMap<Txid, TransactionRecord>,
     ...
 }

 impl ManagedCoreAccount {
     pub fn has_transaction(&self, txid: &Txid) -> bool {
-        #[cfg(feature = "keep_txs_in_memory")]
-        { self.transactions.contains_key(txid) }
-        #[cfg(not(feature = "keep_txs_in_memory"))]
-        { let _ = txid; false }
+        self.processed_txids.contains(txid)
     }

     pub(crate) fn confirm_transaction(...) -> bool {
-        #[cfg(feature = "keep_txs_in_memory")]
-        {
-            if !self.transactions.contains_key(&tx.txid()) { ... }
-            ...
-        }
-        #[cfg(not(feature = "keep_txs_in_memory"))]
-        {
-            self.record_transaction(...);
-            true
-        }
+        if !self.processed_txids.contains(&tx.txid()) {
+            self.record_transaction(...);
+            return true;
+        }
+        // existing context/utxo update logic
     }

     pub(crate) fn record_transaction(...) -> TransactionRecord {
+        self.processed_txids.insert(tx.txid());
         #[cfg(feature = "keep_txs_in_memory")]
         self.transactions.insert(tx.txid(), tx_record);
         ...
     }
 }

Also applies to: 466-508

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@key-wallet/src/managed_account/mod.rs` around lines 103 - 117,
has_transaction currently returns false when keep_txs_in_memory is disabled,
causing confirm_transaction to always treat repeats as new; add a lightweight
persistent "seen txid" pathway so deduplication works regardless of tx record
retention: introduce a HashSet<FieldName like seen_txids> on ManagedAccount (or
reuse an existing persistent index/map), update has_transaction to check
seen_txids under the #[cfg(not(feature = "keep_txs_in_memory"))] branch, and
modify confirm_transaction to consult seen_txids before re-recording/reporting a
change and to insert the txid into seen_txids when first processed so subsequent
mempool→block or rescan events are deduplicated.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@key-wallet/src/managed_account/mod.rs`:
- Around line 103-117: has_transaction currently returns false when
keep_txs_in_memory is disabled, causing confirm_transaction to always treat
repeats as new; add a lightweight persistent "seen txid" pathway so
deduplication works regardless of tx record retention: introduce a
HashSet<FieldName like seen_txids> on ManagedAccount (or reuse an existing
persistent index/map), update has_transaction to check seen_txids under the
#[cfg(not(feature = "keep_txs_in_memory"))] branch, and modify
confirm_transaction to consult seen_txids before re-recording/reporting a change
and to insert the txid into seen_txids when first processed so subsequent
mempool→block or rescan events are deduplicated.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5c09014e-2a07-44d5-85c9-24a384785a1b

📥 Commits

Reviewing files that changed from the base of the PR and between 7896c7f and 0282498.

📒 Files selected for processing (9)

• key-wallet/Cargo.toml
• key-wallet/src/managed_account/managed_account_trait.rs
• key-wallet/src/managed_account/mod.rs
• key-wallet/src/test_utils/wallet.rs
• key-wallet/src/tests/keep_txs_in_memory_tests.rs
• key-wallet/src/tests/mod.rs
• key-wallet/src/transaction_checking/transaction_router/tests/mod.rs
• key-wallet/src/transaction_checking/wallet_checker.rs
• key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• key-wallet/src/tests/keep_txs_in_memory_tests.rs

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[github-actions]

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

[QuantumExplorer]

Closing because I prefer a new approach I made in #733