feat(wallet)!: include non-canonical txs in wallet history

[noahjoeris]

Description

WalletTx was an alias for CanonicalTx, so wallet.transactions() / wallet.get_tx(txid) only returned canonical wallet-relevant txs. Replaced/evicted txs disappeared even when still wallet-relevant.

This PR renames WalletTx → TransactionInfo and surfaces all wallet-relevant txs, including non-canonical ones:

pub struct TransactionInfo {
    pub details: TxDetails,              // includes chain_position: Option<…>
    pub anchors: BTreeSet<ConfirmationBlockTime>,
    pub first_seen: Option<u64>,
    pub last_seen: Option<u64>,
    pub evicted_at: Option<u64>,
    pub conflicts: Vec<Txid>,            // direct input conflicts
}

Fixes #295

Supported use cases

• Keep dropped txs visible in wallet history.
• Show whether an evicted tx has conflicts.
• Let users decide when it is safe to forget an old dropped tx.
• Give users enough information to warn before accidentally paying twice.

Notes to the reviewers

• Performance of transactions() takes a small hit. Worth confirming whether it's an issue in practice.
• Still need to add tests.

Changelog notice

- Changed: `WalletTx` → owned `TransactionInfo` (details, anchors/seen/evicted, direct conflicts).
- Changed: `transactions()` / `get_tx()` also return non-canonical wallet-relevant txs.
- Changed: `TxDetails::chain_position` → `Option<ChainPosition<_>>` (`None` = non-canonical).
- Added: `Wallet::conflicts(txid)` (ancestor-aware).

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran just p before pushing

New Features:

• I've added tests for the new feature
• I've added docs for the new feature
• This pull request breaks the existing API

[codecov]

Codecov Report

❌ Patch coverage is 97.79412% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.47%. Comparing base (75e0d30) to head (404f0fa).

Files with missing lines	Patch %	Lines
src/wallet/error.rs	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   81.15%   81.47%   +0.31%     
==========================================
  Files          24       24              
  Lines        5552     5647      +95     
  Branches      247      250       +3     
==========================================
+ Hits         4506     4601      +95     
  Misses        970      970              
  Partials       76       76              

Flag	Coverage Δ
rust	81.47% <97.79%> (+0.31%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ValuedMammal]

Concept ACK

[noahjoeris]

After some ideas by ValuedMammal I simplified the model a bit:

• Renamed WalletTx → TransactionInfo, WalletTxStatus → TxCanonicality.
• Replaced canonical_blockers() with a direct-only conflicts: Vec<Txid> field; moved last_evicted to a top-level field on TransactionInfo.

PR description updated accordingly.

[ValuedMammal]

canonical_blockers was a nice idea because it could tell you if a tx is indirectly conflicted through one of its ancestors, even if it has no direct conflicts of its own. How can we capture that information so it's clear why a wallet tx isn't canonical? Maybe include the tx's unconfirmed ancestor and descendant sets somewhere so we can trace its ancestry. But as mentioned before we may not want to bake a lot of txgraph traversal into every wallet.transactions() call

[noahjoeris]

Yep I agree. I added canonical_blockers() back.

[oleonardolima]

I don't fully grasp the rationale for the changes. I mean if you need specific group of transactions (e.g non-canonical, evicted, and so on), you could have other methods for those specific scenarios, and even a newer single one that list everything (same as accessing the inner TxGraph).

It's valid to notice that TxGraph is monotone, so if we are getting all transactions for the Wallet being canonical or not, so the list can be really big, and basically a duplicate of what already exists there.

[noahjoeris]

Thanks for the reviews!

I don't fully grasp the rationale for the changes. I mean if you need specific group of transactions (e.g non-canonical, evicted, and so on), you could have other methods for those specific scenarios, and even a newer single one that list everything (same as accessing the inner TxGraph).

It's valid to notice that TxGraph is monotone, so if we are getting all transactions for the Wallet being canonical or not, so the list can be really big, and basically a duplicate of what already exists there.

It addresses the problem stated in #295 that relevant non-canonical txs (fee-evicted, replaced, reorged) disappear from transactions(), so callers can miss them and accidentally double-pay. Ideal behavior is to keep them in the default wallet history with status/conflicts so apps can act accordingly. A separate method / raw TxGraph leaves that safe path opt-in.

Users can rebuild this from existing APIs, but they could get it wrong and it's more convenient this way.
And I assume non-canonical relevant txs are usually rare so the list of txs won't be much longer. Perf-wise we can lighten transactions() and move sent/received/fee/fee_rate/net_value to on-demand tx_details if we want.