From: merge-script Date: Wed, 21 Jan 2026 11:29:08 +0000 (+0800) Subject: Merge bitcoindevkit/bdk#2074: Fix `ChainPosition` ordering X-Git-Url: http://internal-gitweb-vhost/%22https:/parse/scripts/database/-script/sha1/static/gitweb.css?a=commitdiff_plain;p=bdk Merge bitcoindevkit/bdk#2074: Fix `ChainPosition` ordering 1cc7d606ac55fafff1ccf5bcc7ddabb2c723805e fix(chain): correct unconfirmed `ChainPosition` `last_seen` tiebreaker (志宇) 184db01a38a8970d1628258e22a24941fcbd3a3f fix(chain): correct ChainPosition ordering for wallet transaction display (志宇) Pull request description: ## Description This PR fixes incorrect ordering of `ChainPosition` that affected how wallet transactions are displayed. Previously, unconfirmed transactions that were never seen in the mempool would incorrectly appear before transactions with mempool timestamps due to the derived `Ord` implementation treating `None` as less than `Some(_)`. ### Problem The derived `Ord` implementation for `ChainPosition::Unconfirmed` caused incorrect transaction ordering: - `Unconfirmed { first_seen: None, last_seen: None }` (never in mempool) - Would appear **before** `Unconfirmed { first_seen: Some(10), last_seen: Some(10) }` (seen in mempool) This resulted in a confusing wallet display where transactions never broadcast would appear before pending mempool transactions. ### Solution Implemented manual `Ord` and `PartialOrd` traits for `ChainPosition` with the correct ordering: 1. **Confirmed transactions** (earliest first by block height) 2. **Unconfirmed transactions seen in mempool** (ordered by `first_seen`) 3. **Unconfirmed transactions never seen** (these come last) Additional ordering rules: - At the same confirmation height, transitive confirmations come before direct confirmations (as they may actually be confirmed earlier) - Tie-breaking uses transaction IDs for deterministic ordering ## Changelog notice ### Fixed - Fixed `ChainPosition` ordering so unconfirmed transactions never seen in mempool appear last instead of first ### Changed - `ChainPosition`, `CanonicalTx`, and `FullTxOut` now require `A: Ord` for their `Ord` implementations - Simplified `FullTxOut` ordering to only use essential fields (chain_position, outpoint, spent_by) ## Checklists ### All Submissions: * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) ### Bugfixes: * [ ] This pull request breaks the existing API * [x] I've added tests to reproduce the issue which are now passing * [ ] I'm linking the issue being fixed by this PR Top commit has no ACKs. Tree-SHA512: a8f4f2c73391ea5425870f365ca2b66b48ac9466211e82af6ed832673f18fec7812cf99abed7b1620e8d26ed47ace6b3b75902e50880b411d96946dfb82c6dad --- 226b65c64408628dda91c55b549aef7c524b2f5a