Skip to content

Use unique ID for tx items - #468

Merged
bordalix merged 1 commit into
masterfrom
fix-tx-list-id
Mar 23, 2026
Merged

bordalix merged 1 commit into
masterfrom
fix-tx-list-id

Conversation

@pietro909

@pietro909 pietro909 commented Mar 23, 2026 •

Copy link
Copy Markdown
Contributor

Test it with an old wallet, >1k txs

Summary by CodeRabbit

  • Bug Fixes
    • Improved transaction list stability and rendering by enhancing how transaction items are identified and tracked.

@pietro909
pietro909 requested a review from bordalix March 23, 2026 18:13
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-bitcoin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0484b88
Status: ✅  Deploy successful!
Preview URL: https://713da07c.wallet-bitcoin.pages.dev
Branch Preview URL: https://fix-tx-list-id.wallet-bitcoin.pages.dev

View logs

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying wallet-mutinynet with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0484b88
Status: ✅  Deploy successful!
Preview URL: https://da31d4e3.arkade-wallet.pages.dev
Branch Preview URL: https://fix-tx-list-id.arkade-wallet.pages.dev

View logs

@coderabbitai

coderabbitai Bot commented Mar 23, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 17e14d09-5a91-493f-aa29-f6aa8fb1a2ff

📥 Commits

Reviewing files that changed from the base of the PR and between 8053a23 and 0484b88.

📒 Files selected for processing (1)
  • src/components/TransactionsList.tsx

Walkthrough

The list row key generation in TransactionsList.tsx was updated to join all available transaction identifiers (roundTxid, redeemTxid, boardingTxid) into a single key separated by dashes, filtering out falsy values, with a fallback to index-based key when none exist. The previous approach used a left-to-right chain of || operators.

Changes

Cohort / File(s) Summary
Transaction List Key Generation
src/components/TransactionsList.tsx
Updated row key generation to join all truthy transaction identifiers separated by - instead of selecting the first available identifier via || chain.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • #340: Modifies the same TransactionsList.tsx key generation logic, changing the fallback operator approach (from ?? to \|\|).
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: updating transaction list item key generation to use unique IDs instead of the previous fallback logic.

✏️ 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 fix-tx-list-id

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.

@ghost

ghost commented Mar 23, 2026

Copy link
Copy Markdown

🔍 Arkana PR Review

Summary: Changes the React list key for transaction items from a fallback-chain (roundTxid || redeemTxid || boardingTxid || index) to a composite key joining all available txids with -.

Assessment: ✅ Good fix

Why this matters: The previous key strategy could produce duplicate keys when a transaction had, say, a roundTxid that matched another tx's roundTxid but they differed on redeemTxid. The || chain stops at the first truthy value, so two different transactions could share the same key if their first non-null txid matched. The new approach joins all available ids, producing a more unique composite key.

Edge case check:

  • If a tx has all three txids, key = round-redeem-boarding — unique and stable ✓
  • If a tx has only roundTxid, key = roundTxid — same as before ✓
  • If a tx has none, falls back to tx-${index} — same as before ✓
  • If two txs share the same roundTxid but differ on redeemTxid, they now get different keys ✓

One thing to verify: With >1k txs (as the PR body suggests), confirm virtualized list performance is not impacted by longer key strings. Given that react-virtual uses keys for identity rather than DOM diffing, this should be fine.

@bordalix
bordalix merged commit ca3da8d into master Mar 23, 2026
5 checks passed
TaprootFreak pushed a commit to TaprootFreak/wallet that referenced this pull request Mar 27, 2026
@bordalix
bordalix deleted the fix-tx-list-id branch May 18, 2026 16:15
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