3bee563c81 refactor(wallet)!: remove TxOrdering::Bip69Lexicographic (nymius)
e5cb7b2066 feat(wallet): add TxOrdering::Custom (FadedCoder)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Resolves https://github.com/bitcoindevkit/bdk/issues/534.
Resumes from the work in https://github.com/bitcoindevkit/bdk/pull/556.
Add custom sorting function for inputs and outputs through `TxOrdering::Custom` and deprecates `TxOrdering::Bip69Lexicographic`.
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
I tried consider all discussions in https://github.com/bitcoindevkit/bdk/issues/534 while implementing some changes to the original PR. I created a summary of the considerations I had while implementing this:
##### Why use smart pointers?
The size of enums and structs should be known at compilation time. A struct whose fields implements some kind of trait cannot be specified without using a smart pointer because the size of the implementations of the trait cannot be known beforehand.
##### Why `Arc` or `Rc` instead of `Box`?
The majority of the useful smart pointers that I know (`Arc`, `Box`, `Rc`) for this case implement `Drop` which rules out the implementation of `Copy`, making harder to manipulate a simple enum like `TxOrdering`. `Clone` can be used instead, implemented by `Arc` and `Rc`, but not implemented by `Box`.
##### Why `Arc` instead of `Rc`?
Multi threading I guess.
##### Why using a type alias like `TxVecSort`?
cargo-clippy was accusing a too complex type if using the whole type inlined in the struct inside the enum.
##### Why `Fn` and not `FnMut`?
`FnMut` is not allowed inside `Arc`. I think this is due to the `&mut self` ocupies the first parameter of the `call` method when desugared (https://rustyyato.github.io/rust/syntactic/sugar/2019/01/17/Closures-Magic-Functions.html), which doesn't respects `Arc` limitation of not having mutable references to data stored inside `Arc`:
Quoting the [docs](https://doc.rust-lang.org/std/sync/struct.Arc.html):
> you cannot generally obtain a mutable reference to something inside an `Arc`.
`FnOnce` > `FnMut` > `Fn`, where `>` stands for "is supertrait of", so, `Fn` can be used everywhere `FnMut` is expected.
##### Why not `&'a dyn FnMut`?
It needs to include a lifetime parameter in `TxOrdering`, which will force the addition of a lifetime parameter in `TxParams`, which will require the addition of a lifetime parameter in a lot of places more. **Which one is preferable?**
<!-- In this section you can include notes directed to the reviewers, like explaining why some parts
of the PR were done in a specific way -->
### Changelog notice
- Adds new `TxOrdering` variant: `TxOrdering::Custom`. A structure that stores the ordering functions to sort the inputs and outputs of a transaction.
- Deprecates `TxOrdering::Bip69Lexicographic`.
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
### Checklists
#### All Submissions:
* [ ] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [ ] I've added docs for the new feature
Top commit has no ACKs.
Tree-SHA512: 0d3e3ea9aee3a6c9e9d5e1ae93215be84bd1bd99907a319976517819aeda768a7166860a48a8d24abb30c516e0129decb6a6aebd8f24783ea2230143e6dcd72a
BIP 69 proposed a deterministic way to sort transaction inputs and
outputs with the idea of enhancing privacy. It was later discovered
there was no such enhancement but rather a decrement in privacy due to
this sorting.
To avoid the promotion of bad practices, the
TxOrdering::Bip69Lexicographic variant which implemented this BIP for
BDK is removed with this change.
Notice that you can still produce a BIP 69 compliant transaction
defining order functions for TxOrdering::Custom.
Signed-off-by: Steve Myers <steve@notmandatory.org>
The deterministic sorting of transaction inputs and outputs proposed in
BIP 69 doesn't improve the privacy of transactions as originally
intended. In the search of not spreading bad practices but still provide
flexibility for possible use cases depending on particular order of the
inputs/outpus of a transaction, a new TxOrdering variant has been added
to allow the implementation of these orders through the definition of
comparison functions.
Signed-off-by: Steve Myers <steve@notmandatory.org>
af75817d4b ref(tx_graph): Change last_seen to `HashMap<Txid, u64>` (valued mammal)
6204d2c766 feat(tx_graph): Add method `txs_with_no_anchor_or_last_seen` (valued mammal)
496601b8b1 test(tx_graph): Add test for `list_canonical_txs` (valued mammal)
c4057297a9 wallet: delete method `insert_anchor` (valued mammal)
b34790c6b6 ref(tx_graph)!: Rename `list_chain_txs` to `list_canonical_txs` (valued mammal)
2ce4bb4dfc test(indexed_tx_graph): Add test_get_chain_position (valued mammal)
36f58870cb test(wallet): Add test_insert_tx_balance_and_utxos (valued mammal)
bbc19c3536 fix(tx_graph)!: Change tx_last_seen to `Option<u64>` (valued mammal)
324eeb3eb4 fix(wallet)!: Rework `Wallet::insert_tx` to no longer insert anchors (valued mammal)
Pull request description:
The PR changes the type of last_seen to `Option<u64>` for `txs` member of `TxGraph`.
This fixes an issue where unbroadcast and otherwise non-canonical transactions were returned from methods `list_chain_txs` and `Wallet::transactions` because every new tx inserted had a last_seen of 0 making it appear unconfirmed.
fixes#1446fixes#1396
### Notes to the reviewers
### Changelog notice
Changed
- Member `last_seen_unconfirmed` of `TxNode` is changed to `Option<u64>`
- Renamed `TxGraph` method `list_chain_txs` to `list_canonical_txs`
- Changed `Wallet::insert_tx` to take a single `tx: Transaction` as parameter
Added
- Add method `txs_with_no_anchor_or_last_seen` for `TxGraph`
- Add method `unbroadcast_transactions` for `Wallet`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
notmandatory:
Re ACK af75817d4b
Tree-SHA512: e664b3b49e2f547873923f15dffbbc7fa032b6240e5b856b180e9e26123ca141864d10448912dc4a31bbb200c75bef4251a910a4330dac17ee6841b564612d13
Also fixup `test_list_owned_txouts` to check that the right
outputs, utxos, and balance are returned at different local
chain heights.
This fixes an issue where unbroadcast and otherwise non-canonical
transactions were returned from methods `list_chain_txs` and
`Wallet::transactions` because every tx inserted had a last_seen
of 0 making it appear unconfirmed.
Note this commit changes the way `Balance` is represented due to
new logic in `try_get_chain_position` that no longer considers
txs with non-canonical anchors. Before this change, a tx anchored
to a block that is reorged out had a permanent effect on the
pending balance, and now only txs with a last_seen time or an
anchor confirmed in the best chain will return a `ChainPosition`.
8f5b172e59 test(wallet): verify wallet panics in dev mode if using keychains with duplicate spks (Steve Myers)
46c6f18cc3 refactor(chain): calculate DescriptorId as sha256 hash of spk at index 0 (Steve Myers)
Pull request description:
### Description
Rename `DescriptorId` to `KeychainId` and `descriptor_id()` to `keychain_id()`.
Calculate keychain ids as the hash of the spk derived from its descriptor as index 0.
Added docs to `Wallet` and `KeychainTxOutIndex::insert_descriptor()` explaining that it's the users responsibility not to use wildcard and non-wildcard descriptors that can derive the same script pubkey. I also recommended for `Wallet` that legacy non-wildcard descriptors be added to a temporary `Wallet` and swept into a modern wildcard descriptor.
### Notes to the reviewers
fixes#1483
### Changelog notice
changed
- Renamed DescriptorId to KeychainId, DescriptorExt::descriptor_id() to keychain_id().
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
LLFourn:
ACK 8f5b172e59
oleonardolima:
Concept ACK 8f5b172e59
Tree-SHA512: 07defa208d9cfcd61bc6e31ded06a287c392c51bcc4949f601ecfac635c3443e7d08c62d92618ed894dc5ef13cdcf784771a6bf8904a5397110bedb1563f52d4
cf7aca84d1 Remove usage of blockdata:: from bitcoin paths (Tobin C. Harding)
Pull request description:
### Description
In `rust-bitcoin` the `blockdata` module is a code organisation thing, it should never have been public. One day those guys would like to remove it, so as not to be a PITA for `bdk` when they do lets remove all usage of `blockdata::` now.
### Changelog notice
Internal change only, no externally visible changes.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
Top commit has no ACKs.
Tree-SHA512: 4c5e54a2ac3f71835c25ce97ad0acd1859e5ae8d45ebfde087e19e9494754e0aa70a47ca5f3d6a3b509f27e28ef9d4a5269573c686250f43834d09378155681c
In `rust-bitcoin` the `blockdata` module is a code organisation thing,
it should never have been public. One day those guys would like to
remove it, so as not to be a PITA for `bdk` when they do lets remove all
usage of `blockdata::` now.
Internal change only, no externally visible changes.
438cd4682d refactor(wallet)!: change WeightedUtxo to use Weight type (Leonardo Lima)
Pull request description:
fixes#1466
depends on #1448
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
This PR is a follow-up on top of #1448, and should be rebased and merged after it, it uses the rust-bitcoin `Weight` type instead of the current `usize` usage.
NOTE: ~~It also adds a new `MiniscriptError` variant, and remove the `.unwrap()` usage.~~ As suggested I'll address this on another issue #1485, trying to reproduce the error first.
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
It should be ready to review after #1448 gets merged.
<!-- In this section you can include notes directed to the reviewers, like explaining why some parts
of the PR were done in a specific way -->
### Changelog notice
- Change `WeightedUtxo` `satisfaction_weight` has been changed to use `Weight` type.
- Change `TxBuilder` methods `add_foreign_utxo` and `add_foreign_utxo_with_sequence` to expect `Weight` as `satisfaction_weight` type.
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
storopoli:
Anyways, ACK 438cd4682d
notmandatory:
ACK 438cd4682d
Tree-SHA512: 1998fe659833da890ce07aa746572ae24d035e636732c1a11b7828ffed48e753adb4108f42d00b7cd05e6f45831a7a9840faa26f94058fc13760497837af002f
55a17293a4 ref(signer): Use `Psbt::sighash_ecdsa` for computing sighashes (valued mammal)
f2a2dae84c refactor(signer): Remove trait ComputeSighash (valued mammal)
Pull request description:
This PR does some cleanup of the `bdk_wallet` signer module most notably by removing the internal trait `ComputeSighash` and replacing old code for computing the sighash (for legacy and segwit context) with a single method [`Psbt::sighash_ecdsa`](https://docs.rs/bitcoin/0.31.2/bitcoin/psbt/struct.Psbt.html#method.sighash_ecdsa). The logic for computing the taproot sighash is unchanged and extracted to a new helper function `compute_tap_sighash`.
- [x] Unimplement `ComputeSighash`
- [x] Try de-duplicating code by using `Psbt::sighash_ecdsa`. see https://github.com/bitcoindevkit/bdk/pull/1023#discussion_r1263140218
- Not done in this PR: Consider removing unused `SignerError` variants
fixes#1038
### Notes to the reviewers
### Changelog notice
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
Top commit has no ACKs.
Tree-SHA512: 56af3c9c463513ca3bae5480aa5b90d78de119c3c09c824a7220eb6832d5f403b172afc8168228918ea1adabb4bf8fca858790adfebf84fc334b4fc1cc99d3cd
- Change param `hash` to `&Message` in `sign_psbt_ecdsa`
- Remove unused methods `compute_legacy_sighash`,
and `compute_segwitv0_sighash`.
- Match on `self.ctx` when signing for `SignerWrapper<PrivateKey>`
since we'd be lacking context that should normally occur during
sync with a chain source. The logic for inserting a graph
anchor from a `ConfirmationTime` is moved to the wallet common
test module in order to simulate receiving new txs and
confirming them.
4bddb0de62 feat(wallet): add back TxBuilder finish() and sort_tx() with thread_rng() (Steve Myers)
45c0cae0a4 fix(bdk): remove rand dependency (rustaceanrob)
Pull request description:
### Description
WIP towards removing `rand` fixes#871
The `rand` dependency was imported explicitly, but `rand` is also implicitly used through the `rand-std` feature flag on `bitcoin`.
### Notes to he reviewers
**Updated:**
`rand` was used primarily in two parts of `bdk`. Particularly in signing and in building a transaction.
Signing:
- Used implicitly in [`sign_schnorr`](https://docs.rs/bitcoin/latest/bitcoin/key/struct.Secp256k1.html#method.sign_schnorr), but nowhere else within `signer`.
Transaction ordering:
- Used to shuffle the inputs and outputs of a transaction, the default
- Used in the single random draw __as a fallback__ to branch and bound during coin selection. Branch and bound is the default coin selection option.
See conversation for proposed solutions.
### Changelog notice
- Remove the `rand` dependency from `bdk`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
ValuedMammal:
ACK 4bddb0de62
notmandatory:
ACK 4bddb0de62
Tree-SHA512: 662d9bcb1e02f8195d73df16789b8c2aba8ccd7b37ba713ebb0bfd19c66163acbcb6f266b64f88347cbb1f96b88c8a150581012cbf818d1dc8b4437b3e53fc62
996605f2bf fix(wallet)!: Simplify `SignOptions` and improve finalization logic (valued mammal)
Pull request description:
Rather than comingle various `SignOptions` with the finalization step, we simply clear all fields when finalizing as per the PSBT spec in BIPs 174 and 371 which is more in line with user expectations.
### Notes to the reviewers
I chose to re-implement some parts of [`finalize_input`](https://docs.rs/miniscript/latest/src/miniscript/psbt/finalizer.rs.html#434) since it's fairly straightforward, see also https://github.com/bitcoindevkit/bdk/issues/1461#issuecomment-2171983426. I had to refactor some wallet tests but didn't go out of my way to add additional tests.
closes#1461
### Changelog notice
- Removed field `remove_partial_sigs` from `SignOptions`
- Removed field `remove_taproot_extras` from `SignOptions`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
evanlinjin:
re-ACK 996605f2bf
Tree-SHA512: 63e78e75c22031424e87fcc26cd6b0015c626cd57c02680256bad9d1783cef71f4048b2d7ce5d0425cd4239351e37dd0e2a626dda7e8417af7fc52cb0afe6933
Rather than comingle various `SignOptions` with the finalization
step, we simply clear all fields when finalizing as per the PSBT
spec in BIPs 174 and 371 which is more in line with user
expectations.
e21affdbbb Bump bdk version to 1.0.0-alpha.13 (Steve Myers)
Pull request description:
### Description
Bump bdk version to 1.0.0-alpha.13
bdk_chain to 0.16.0
bdk_bitcoind_rpc to 0.12.0
bdk_electrum to 0.15.0
bdk_esplora to 0.15.0
bdk_file_store to 0.13.0
bdk_sqlite keep at 0.2.0
bdk_testenv to 0.6.0
bdk_hwi to 0.3.0
fixes#1471
Top commit has no ACKs.
Tree-SHA512: 987adf5084dc84261fb3767d8b1f8ebe3dd94a8a4eb30b8529b70c055e4f122cb48063d205e90831169c2d7b2a1509aef1966857bd6b67e78cfb0d52144822d9
bdk_chain to 0.16.0
bdk_bitcoind_rpc to 0.12.0
bdk_electrum to 0.15.0
bdk_esplora to 0.15.0
bdk_file_store to 0.13.0
bdk_sqlite keep at 0.2.0
bdk_testenv to 0.6.0
bdk_hwi to 0.3.0
a0bf45bef1 docs: remove PersistBackend from docs, Cargo.toml and wallet README.md (Steve Myers)
feb27df180 feat(chain)!: add `take` convenience method to `Append` trait (志宇)
1eca568be5 feat!: rm `persist` submodule (志宇)
Pull request description:
### Description
@LLFourn suggested these changes which greatly simplifies the amount of code we have to maintain and removes the `async-trait` dependency. We remove `PersistBackend`, `PersistBackendAsync`, `StageExt` and `StageExtAsync` completely. Instead, we introduce `Wallet::take_staged(&mut self) -> Option<ChangeSet>`.
The original intention to keep a staging area (`StageExt`, `StageExtAsync`) is to enforce:
1. The caller will not persist an empty changeset.
2. The caller does not take the staged changeset if the database (`PersistBackend`) fails.
We achieve `1.` by returning `None` if the staged changeset is empty.
`2.` is not too important. The caller can try figure out what to do with the changeset if persisting to db fails.
### Notes to the reviewers
I added a `take` convenience method to the `Append` trait. I thought it would be handy for the caller to do a staging area with this.
### Changelog notice
* Remove `persist` submodule from `bdk_chain`.
* Change `Wallet` to outsource it's persistence logic by introducing `Wallet::take_staged`.
* Add `take` convenience method to `Append` trait.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
~* [ ] I've added tests for the new feature~
* [x] I've added docs for the new feature
ACKs for top commit:
notmandatory:
ACK a0bf45bef1
evanlinjin:
self-ACK a0bf45bef1
Tree-SHA512: 38939ab446c84d9baecd4cd36a7b66add5a121212ad5e06ade04a60f7903133dd9a20219e230ab8a40404c47e07b946ccd43085572d71c3a2a80240a2223a500
Remove `PersistBackend`, `PersistBackendAsync`, `StageExt` and
`StageExtAsync`. Remove `async` feature flag and dependency. Update
examples and wallet.
20341a3ca1 fix: typo on `SignedAmount` instead of `Amount` (Leonardo Lima)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
It fixes the typo on the `expect()` message introduced on #1426, noticed at https://github.com/bitcoindevkit/bdk/pull/1426#discussion_r1626761825
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
storopoli:
ACK 20341a3ca1
notmandatory:
ACK 20341a3ca1
Tree-SHA512: 62deb7308b2a6891eeb4598ebdf3c5ee1541fc52fd454bca2816b819bd7f6ab25c18e10c4166c80c4e553bcb3ce2207323376d260484a956837ac857854cbd6b
ec36c7ecca feat(example): use changeset staging with rpc polling example (志宇)
19328d4999 feat(wallet)!: change persist API to use `StageExt` and `StageExtAsync` (志宇)
2e40b0118c feat(chain): reintroduce a way to stage changesets before persisting (志宇)
36e82ec686 chore(chain): relax `miniscript` feature flag scope (志宇)
9e97ac0330 refactor(persist): update file_store, sqlite, wallet to use bdk_chain::persist (Steve Myers)
54b0c11cbe feat(persist): add PersistAsync trait and StagedPersistAsync struct (Steve Myers)
aa640ab277 refactor(persist): rename PersistBackend to Persist, move to chain crate (Steve Myers)
Pull request description:
### Description
Sorry to submit another refactor PR for the persist related stuff, but I think it's worth revisiting. My primary motivations are:
1. remove `db` from `Wallet` so users have the ability to use `async` storage crates, for example using `sqlx`. I updated docs and examples to let users know they are responsible for persisting changes.
2. remove the `anyhow` dependency everywhere (except as a dev test dependency). It really doesn't belong in a lib and by removing persistence from `Wallet` it isn't needed.
3. remove the `bdk_persist` crate and revert back to the original design with generic error types. I kept the `Debug` and `Display` constrains on persist errors so they could still be used with the `anyhow!` macro.
### Notes to the reviewers
I also replaced/renamed old `Persist` with `StagedPersist` struct inspired by #1453, it is only used in examples. The `Wallet` handles it's own staging.
### Changelog notice
Changed
- Removed `db` from `Wallet`, users are now responsible for persisting changes, see docs and examples.
- Removed the `bdk_persist` crate and moved logic back to `bdk_chain::persist` module.
- Renamed `PersistBackend` trait to `Persist`
- Replaced `Persist` struct with `StagedPersist`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
ValuedMammal:
ACK ec36c7ecca
Tree-SHA512: 380b94ae42411ea174720b7c185493c640425f551742f25576a6259a51c1037b441a238c6043f4fdfbf1490aa15f948a34139f1850d0c18d285110f6a9f36018
Still enable the `persist` submodule without `miniscript` feature flag.
Only disable `CombinedChangeSet`.
Also stop `cargo clippy` from complaining about unused imports when
`miniscript` is disabled.
8dd174479f refactor(chain): compute txid once for `KeychainTxOutIndex::index_tx` (志宇)
639d735ca0 refactor(chain): change field names to be more sane (志宇)
5a02f40122 docs(chain): fix docs (志宇)
c77e12bae7 refactor(chain): `KeychainTxOutIndex` use `HashMap` for fields (志宇)
4d3846abf4 chore(chain): s/replenish_lookahead/replenish_inner_index/ (LLFourn)
8779afdb0b chore(chain): document insert_descriptor invariants better (LLFourn)
69f2a695f7 refactor(chain): improve replenish lookeahd internals (LLFourn)
5a584d0fd8 chore(chain): Fix Indexed and KeychainIndexed documentaion (Lloyd Fournier)
b8ba5a0206 chore(chain): Improve documentation of keychain::ChangeSet (LLFourn)
101a09a97f chore(chain): Standardise KeychainTxOutIndex return types (LLFourn)
bce070b1d6 chore(chain): add type IndexSpk, fix clippy type complexity warning (Steve Myers)
4d2442c37f chore(chain): misc docs and insert_descriptor fixes (LLFourn)
bc2a8be979 refactor(keychain): Fix KeychainTxOutIndex range queries (LLFourn)
3b2ff0cc95 Write failing test for keychain range querying (LLFourn)
Pull request description:
Fixes#1459
This reverts part of the changes in #1203. There the `SpkTxOutIndex<(K,u32)>` was changed to `SpkTxOutIndex<(DescriptorId, u32>)`. This led to a complicated translation logic in `KeychainTxOutIndex` (where the API is based on `K`) to transform calls to it to calls to the underlying `SpkTxOutIndex` (which now indexes by `DescriptorId`). The translation layer was broken when it came to translating range queries from the `KeychainTxOutIndex`. My solution was just to revert this part of the change and remove the need for a translation layer (almost) altogether. A thin translation layer remains to ensure that un-revealed spks are filtered out before being returned from the `KeychainTxOutIndex` methods.
I feel like this PR could be extended to include a bunch of ergonomics improvements that are easier to implement now. But I think that's the point of https://github.com/bitcoindevkit/bdk/pull/1451 so I held off and should probably go and scope creep that one instead.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
evanlinjin:
ACK 8dd174479f
Tree-SHA512: 283e6b6d4218902298e2e848fe847a6c85e27af4eee3e4337e3dad6eacf9beaa08ac99b1dce7b6fb199ca53931e543ea365728a81c41567a2e510cce77b12ac0
