This PR removes `RelevantTxids` from the electrum crate and tracks
transactions in a `TxGraph`. This removes the need to separately
construct a `TxGraph` after a `full_scan` or `sync`.
This fixes the bug with changesets not being monotone. Previously, the
result of applying changesets individually v.s. applying the aggregate
of changesets may result in different `KeychainTxOutIndex` states.
The nature of the changeset allows different keychain types to share the
same descriptor. However, the previous design did not take this into
account. To do this properly, we should keep track of all keychains
currently associated with a given descriptor. However, the API only
allows returning one keychain per spk/txout/outpoint (which is a good
API).
Therefore, we rank keychain variants by `Ord`. Earlier keychain variants
have a higher rank, and the first keychain will be returned.
We only need to loop though entries of `other`. The logic before was
wasteful because we were also looping though all entries of `self` even
if we do not need to modify the `self` entry.
- The KeychainTxOutIndex's internal SpkIterator now uses DescriptorId
instead of K. The DescriptorId -> K translation is made at the
KeychainTxOutIndex level.
- The keychain::Changeset is now a struct, which includes a map for last
revealed indexes, and one for newly added keychains and their
descriptor.
API changes in bdk:
- Wallet::keychains returns a `impl Iterator` instead of `BTreeMap`
- Wallet::load doesn't take descriptors anymore, since they're stored in
the db
- Wallet::new_or_load checks if the loaded descriptor from db is the
same as the provided one
API changes in bdk_chain:
- `ChangeSet` is now a struct, which includes a map for last revealed
indexes, and one for keychains and descriptors.
- `KeychainTxOutIndex::inner` returns a `SpkIterator<(DescriptorId, u32)>`
- `KeychainTxOutIndex::outpoints` returns a `impl Iterator` instead of `&BTreeSet`
- `KeychainTxOutIndex::keychains` returns a `impl Iterator` instead of
`&BTreeMap`
- `KeychainTxOutIndex::txouts` doesn't return a ExactSizeIterator
anymore
- `KeychainTxOutIndex::unbounded_spk_iter` returns an `Option`
- `KeychainTxOutIndex::next_index` returns an `Option`
- `KeychainTxOutIndex::last_revealed_indices` returns a `BTreeMap`
instead of `&BTreeMap`
- `KeychainTxOutIndex::reveal_to_target` returns an `Option`
- `KeychainTxOutIndex::reveal_next_spk` returns an `Option`
- `KeychainTxOutIndex::next_unused_spk` returns an `Option`
- `KeychainTxOutIndex::add_keychain` has been renamed to
`KeychainTxOutIndex::insert_descriptor`, and now it returns a
ChangeSet
We plan to record `Descriptor` additions into persistence. Hence, we
need to add `Descriptor`s to the changeset. This depends on
`miniscript`. Moving this into `txout_index.rs` makes sense as this is
consistent with all the other files. The only reason why this wasn't
this way before, is because the changeset didn't need miniscript.
Co-Authored-By: Daniela Brozzoni <danielabrozzoni@protonmail.com>
a577c22b12 fix(persist): add default feature to enable bdk_chain/std (Steve Myers)
Pull request description:
### Description
This PR adds a `default` feature to `bdk_persist` so it can be build on its own. Once #1422 is done we can remove the `default`again.
### Notes to the reviewers
I need to be able to build `bdk_persist` on its own so I can publish it to crates.io.
### 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 a577c22b12
oleonardolima:
ACK a577c22b12
storopoli:
ACK a577c22b12
Tree-SHA512: 8b07a9e4974dec8812ca19ce7226dcaece064270a0be8b83d3c326fdf1e89b051eb0bd8aa0eda9362b2c8233ecd6003b70c92ee046603973d8d40611418c3841
22aa534d76 feat: use `Amount` on `TxBuilder::add_recipient` (Leonardo Lima)
d5c0e7200c feat: use `Amount` on `spk_txout_index` and related (Leonardo Lima)
8a33d98db9 feat: update `wallet::Balance` to use `bitcoin::Amount` (Leonardo Lima)
Pull request description:
fixes#823
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
It's being used on `Balance`, and throughout the code, an `u64` represents the amount, which relies on the user to infer its sats, not millisats, or any other representation.
It updates the usage of `u64` on `Balance`, and other APIs:
- `TxParams::add_recipient`
- `KeyChainTxOutIndex::sent_and_received`, `KeyChainTxOutIndex::net_value`
- `SpkTxOutIndex::sent_and_received`, `SpkTxOutIndex::net_value`
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
<!-- 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 -->
It updates some of the APIs to expect the `bitcoin::Amount`, but it does not update internal usage of u64, such as `TxParams` still expects and uses `u64`, please see the PR comments for related discussion.
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
- Changed the `keychain::Balance` struct fields to use `Amount` instead of `u64`.
- Changed the `add_recipient` method on `TxBuilder` implementation to expect `bitcoin::Amount`.
- Changed the `sent_and_received`, and `net_value` methods on `KeyChainTxOutIndex` to expect `bitcoin::Amount`.
- Changed the `sent_and_received`, and `net_value` methods on `SpkTxOutIndex` to expect `bitcoin::Amount`.
### 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
* [ ] 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 22aa534d76
Tree-SHA512: c4e8198d96c0d66cc3d2e4149e8a56bb7565b9cd49ff42113eaebd24b1d7bfeecd7124db0b06524b78b8891ee1bde1546705b80afad408f48495cf3c02446d02
f6218e4741 chore: reexport crates in `TestEnv` (Wei Chen)
125959976f chore: remove `anyhow` dev dependency from `electrum`, `esplora`, and `bitcoind_rpc` (Wei Chen)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Reexports `electrsd` in `TestEnv` to remove the `electrsd` dev depedency out of `bdk_electrum` and `bdk_esplora`.
Credit to @ValuedMammal for the idea.
Since `bitcoind` reexports `anyhow`, this dev dependency was also removed from `bdk_electrum`, `bdk_esplora`, and `bdk_bitcoind_rpc`. `bitcoind`, `bitcoincore_rpc` and `electrum_client` were also reexported for convenience.
### Changelog notice
* Change `bdk_testenv` to re-export internally used crates.
### 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:
evanlinjin:
ACK f6218e4741
Tree-SHA512: c7645eb91d08d4ccb80982a992f691b5a8c0df39df506f6b361bc6f2bb076d62cbe5bb5d88b4c684c36e22464c0674f21f6ef4e23733f89b03aa12ec43a67cba
- update all fields `immature`, ` trusted_pending`, `unstrusted_pending`
and `confirmed` to use the `bitcoin::Amount` instead of `u64`
- update all `impl Balance` methods to use `bitcoin::Amount`
- update all tests that relies on `keychain::Balance`
db47347472 test(wallet): add thread safety test (Rob N)
Pull request description:
### Description
`Wallet` auto-implements `Send` and `Sync` after removing the generic. This test is a compile time error if there are changes to `Wallet` in the future that make it unsafe to send between threads. See #1387 for discussion.
### 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
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
evanlinjin:
ACK db47347472
Tree-SHA512: 490e666bc503f15286268db7e5e2f75ee44ad2f80251d6f7a01af2a435023b87607eee33623712433ea8d27511be63c6c1e9cad4159b3fe66a4644cfa9e344fb
bdk_chain to 0.13.0
bdk_bitcoind_rpc to 0.9.0
bdk_electrum to 0.12.0
bdk_esplora to 0.12.0
bdk_file_store to 0.10.0
bdk_testenv to 0.3.0
bdk_persist to 0.2.0
c0374a0eeb feat(chain): `SyncRequest` now uses `ExactSizeIterator`s (志宇)
0f94f24aaf feat(esplora)!: update to use new sync/full-scan structures (志宇)
4c52f3e08e feat(wallet): make wallet compatible with sync/full-scan structures (志宇)
cdfec5f907 feat(chain): add sync/full-scan structures for spk-based syncing (志宇)
Pull request description:
Fixes#1153
Replaces #1194
### Description
Introduce universal structures that represent sync/full-scan requests/results.
### Notes to the reviewers
This is based on #1194 but is different in the following ways:
* The functionality to print scan/sync progress is not reduced.
* `SyncRequest` and `FullScanRequest` is simplified and fields are exposed for more flexibility.
### Changelog notice
* Add universal structures for initiating/receiving sync/full-scan requests/results for spk-based syncing.
* Updated `bdk_esplora` chain-source to make use of new universal sync/full-scan structures.
### 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
ACKs for top commit:
notmandatory:
tACK c0374a0eeb
Tree-SHA512: c2ad66d972a6785079bca615dfd128edcedf6b7a02670651a0ab1ce5b5174dd96f54644680eedbf55e3f1955fe5c34f632eadbd3f71d7ffde658753c6c6d42be
* Changed `Wallet::apply_update` to also take in anything that
implements `Into<Update>`. This allows us to directly apply a
`FullScanResult` or `SyncResult`.
* Added `start_full_scan` and `start_sync_with_revealed_spks` methods to
`Wallet`.
Co-authored-by: Steve Myers <steve@notmandatory.org>
96a9aa6e63 feat(chain): refactor `merge_chains` (志宇)
2f22987c9e chore(chain): fix comment (志宇)
daf588f016 feat(chain): optimize `merge_chains` (志宇)
77d35954c1 feat(chain)!: rm `local_chain::Update` (志宇)
1269b0610e test(chain): fix incorrect test case (志宇)
72fe65b65f feat(esplora)!: simplify chain update logic (志宇)
eded1a7ea0 feat(chain): introduce `CheckPoint::insert` (志宇)
519cd75d23 test(esplora): move esplora tests into src files (志宇)
a6e613e6b9 test(esplora): add `test_finalize_chain_update` (志宇)
494d253493 feat(testenv): add `genesis_hash` method (志宇)
886d72e3d5 chore(chain)!: rm `missing_heights` and `missing_heights_from` methods (志宇)
bd62aa0fe1 feat(esplora)!: remove `EsploraExt::update_local_chain` (志宇)
1e99793983 feat(testenv): add `make_checkpoint_tip` (志宇)
Pull request description:
Fixes#1354
### Description
Built on top of both #1369 and #1373, we simplify the `EsploraExt` API by removing the `update_local_chain` method and having `full_scan` and `sync` update the local chain in the same call. The `full_scan` and `sync` methods now takes in an additional input (`local_tip`) which provides us with the view of the `LocalChain` before the update. These methods now return structs `FullScanUpdate` and `SyncUpdate`.
The examples are updated to use this new API. `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` are no longer needed, therefore they are removed.
Additionally, we used this opportunity to simplify the logic which updates `LocalChain`. We got rid of the `local_chain::Update` struct (which contained the update `CheckPoint` tip and a `bool` which signaled whether we want to introduce blocks below point of agreement). It turns out we can use something like `CheckPoint::insert` so the chain source can craft an update based on the old tip. This way, we can make better use of `merge_chains`' optimization that compares the `Arc` pointers of the local and update chain (before we were crafting the update chain NOT based on top of the previous local chain). With this, we no longer need the `Update::introduce_older_block` field since the logic will naturally break when we reach a matching `Arc` pointer.
### Notes to the reviewers
* Obtaining the `LocalChain`'s update now happens within `EsploraExt::full_scan` and `EsploraExt::sync`. Creating the `LocalChain` update is now split into two methods (`fetch_latest_blocks` and `chain_update`) that are called before and after fetching transactions and anchors.
* We need to duplicate code for `bdk_esplora`. One for blocking and one for async.
### Changelog notice
* Changed `EsploraExt` API so that sync only requires one round of fetching data. The `local_chain_update` method is removed and the `local_tip` parameter is added to the `full_scan` and `sync` methods.
* Removed `TxGraph::missing_heights` and `tx_graph::ChangeSet::missing_heights_from` methods.
* Introduced `CheckPoint::insert` which allows convenient checkpoint-insertion. This is intended for use by chain-sources when crafting an update.
* Refactored `merge_chains` to also return the resultant `CheckPoint` tip.
* Optimized the update `LocalChain` logic - use the update `CheckPoint` as the new `CheckPoint` tip when possible.
### 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
ACKs for top commit:
LLFourn:
ACK 96a9aa6e63
Tree-SHA512: 3d4f2eab08a1fe94eb578c594126e99679f72e231680b2edd4bfb018ba1d998ca123b07acb2d19c644d5887fc36b8e42badba91cd09853df421ded04de45bf69
`merge_chains` now returns a tuple of the resultant checkpoint AND
changeset. This is arguably a more readable/understandable setup.
To do this, we had to create `CheckPoint::apply_changeset` which is kept
as a private method.
Thank you @ValuedMammal for the suggestion.
Co-authored-by: valuedvalued mammal <valuedmammal@protonmail.com>
d3a14d411d fix: enable blocking-https-rustls feature on esplora client (thunderbiscuit)
Pull request description:
The [`blocking` feature on the rust-esplora-client library](https://github.com/bitcoindevkit/rust-esplora-client/blame/master/Cargo.toml#L35) changed from ureq to minreq, which does not come with https enabled by default, breaking previously working code that simply enabled the `blocking` feature on the `bdk_esplora` crate.
This change will enable what is currently the "default https" [flag for the minreq library](https://docs.rs/minreq/latest/minreq/#https-or-https-rustls) when using the `blocking` feature on bdk_esplora, reverting that breaking change.
### Notes to the reviewers
Another way we could do this (let me know if this is preferable) is to add a new feature called `blocking-https-rustls`:
```rust
blocking = ["esplora-client/blocking"]
blocking-https-rustls = ["esplora-client/blocking-https-rustls"]
```
### Changelog notice
<!-- 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
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
storopoli:
ACK d3a14d411d
evanlinjin:
ACK d3a14d411d
Tree-SHA512: d25495186ceba2fcd04bc9ff0aebfb32ac5db6885ef8e4df1e304c5ee5264f6161821e06d29367d2837afcc64a53f1553e7c0bb065e6a2e46dc08b8e04c2ad8e
Introduce a new API for getting addresses from the Wallet that
reflects a similiar interface as the underlying indexer
`KeychainTxOutIndex` in preparation for removing `AddressIndex` enum.
Before this change, the only way to get an address was via the methods
`try_get{_internal}_address` which required a `&mut` reference to the
wallet, matching on the desired AddressIndex variant. This is too
restrictive since for example peeking or listing unused addresses
shouldn't change the state of the wallet. Hence we provide separate
methods for each use case which makes for a more efficient API.
e51af49ffa fix(wallet): remove generic from wallet (Rob N)
Pull request description:
### Description
The `PersistenceBackend` uses generics to describe errors returned while applying the change set to the persistence layer. This change removes generics wherever possible and introduces a new public error enum. Removing the generics from `PersistenceBackend` errors is the first step towards #1363
*Update*: I proceeded with removing the generics from `Wallet` by introducing a `Box<dyn PersistenceBackend>` .
### Notes to the reviewers
This one sort of blew up in the number of changes due to the use of generics for most of the `Wallet` error variants. The generics were only used for the persistence errors, so I removed the generics from higher level errors whenever possible. The error variants of `PersistenceBackend` may also be more expressive, but I will level that up for discussion and make any changes required.
### Changelog notice
- Changed `PersistenceBackend` errors to depend on the `anyhow` crate.
- Remove the generic `T` from `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
#### New Features:
* [ ] 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
* [ ] 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 e51af49ffa
Tree-SHA512: 8ce4f1c495310e16145555f4a6a29a0f42cf8944eda68004595c3532580767f64f779185022147a00d75001c40d69fdf8f8de2d348eb68484b170d2a181117ff
The intention is to remove the `Update::introduce_older_blocks`
parameter and update the local chain directly with `CheckPoint`.
This simplifies the API and there is a way to do this efficiently.
