Removed the persistence module in `bdk_chain` (which contained the `PersistWith` and `PersistAsyncWith` traits and a `Persisted<T>` wrapper). Replaced it with simplified versions that are `Wallet`-specific.
The new traits (`WalletPersister` and `AsyncWalletPersister`) are simpler since they have less type-parameters (being wallet-specific). The old versions were more complex as they were intended to be used with any type. However, we need more time to finalize a works-for-all-bdk-types persistence API.
Additionally, `WalletPersister` and `AsyncWalletPersister` also introduce the `initialize` method. It is handy to contain db-initialization (i.e. create tables for SQL) here. We can call `initialize` in `PersistedWallet` constructors. So the `PersistedWallet::persist` method does not need to ensure the database is initialized. To accommodate this, I made the `.init_sqlite_table` methods on changeset-types public, and removed the call of this in `from_sqlite` and `.persist_to_sqlite`. The `initialize` method now loads from the persister, so the `load` associated function is removed.
There was a bug in the old `PersistAsyncWith` trait where the lifetime bounds were not strict enough (refer to the conversation in #1552). This is now fixed in the new `AsyncWalletPersister` trait.
Docs for the new types are clearer (hopefully). I mentioned implementation details about the new traits and what safety checks were guaranteed in `PersistedWallet`.
I think it makes sense just to have a wallet-specific persistence API which we will maintain alongside wallet for v1.0. This is less baggage for users and maintainers alike.
### Notes to the reviewers
**How breaking are these changes?**
Unless if you were implementing your own persistence, then not breaking at all! If you were implementing your own persistence for BDK, the breakages are minimal. The main change is introducing the `initialize` method to persistence traits (which replaces `load`).
**Acknowledgements**
I came up with the idea of this while responding to @tnull's suggestions on Discord. Unfortunately we cannot fulfill all of his requests. However, I do believe that this will make things easier for our users.
### Changelog notice
* Removed the `persist` module in `bdk_chain`.
* Added `WalletPersister`/`AsyncWalletPersister` traits and `PersistedWallet` struct to `bdk_wallet`. These are simplified and safer versions of old structs provided by the `persist` module in `bdk_chain`.
* Change `.init_sqlite_tables` method to be public on changeset types. `from_sqlite` and `persist_into_sqlite` methods no longer call `.init_sqlite_tables` internally.
### 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
志宇 [Thu, 15 Aug 2024 05:49:18 +0000 (05:49 +0000)]
feat(wallet)!: add persister (`P`) type param to `PersistedWallet<P>`
This forces the caller to use the same persister type that they used for
loading/creating when calling `.persist` on `PersistedWallet`.
This is not totally fool-proof since we can have multiple instances of
the same persister type persisting to different databases. However, it
does further enforce some level of safety.
Changeset methods `.persist_to_sqlite` and `from_sqlite` no longer
internally call `.init_sqlite_tables`. Instead, it is up to the caller
to call `.init_sqlite_tables` beforehand.
This allows us to utilize `WalletPersister::initialize`, instead of
calling `.init_sqlite_tables` every time we persist/load.
Fix building `example_cli` by adding the bitcoin and rand dependencies so it, and the crates that depend on it, can be built independently and not only from the workspace.
### Notes to the reviewers
I also added the build-examples CI job to verify we can build examples individually.
### Changelog notice
None.
### 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
* [x] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
This PR is a follow up to the dev call discussion where we decided it was better to PR these changes separate from #1533. This is a breaking change, but I think it's worth it because those will potentially cause runtime errors for users that expect one thing to happen and realize it's something else.
Left out of this PR but as surfaced in the call probably worth discussing is whether these methods make sense at all or whether they should be removed entirely. What does it mean to return an error when someone loads a wallet from persistence for which the genesis hash doesn't match the one persisted? Maybe worth a new issue; this PR simply attempts to name them correctly.
### Description
See [Q1 in comment here](https://github.com/bitcoindevkit/bdk/pull/1533#issuecomment-2267641603) for more context into the initial question.
Two of the methods on the builder that loads a wallet were checkers/validators rather than setters:
- `network()`
- `genesis_hash()`
This is confusing for users because when loading a wallet from persistence those things are persisted and cannot be changed, and yet seemingly the option to do that is there with those methods (so now you're re-thinking what you think you know about the changeset and persistence). Moreover, the fields on the [`LoadParams` type](https://docs.rs/bdk_wallet/1.0.0-beta.1/src/bdk_wallet/wallet/params.rs.html#116-124) are correctly named `check_network` and `check_genesis_hash`. This PR simply brings those two methods in line with what they actually do and set on the struct they modify.
This modification was not done on the `descriptors()` method, because that one is both a validator and a setter if the descriptors passed contain private keys.
Note that I chose the names `validate_network` and `validate_genesis_hash` but I'm not married to them. If others prefer `check_network` and `check_genesis_hash`, I'm happy to fix them that way instead!
### Changelog notice
```md
Breaking:
- The `LoadParams` type used in the wallet load builder renamed its `network()` and `genesis_hash` methods to `check_network()` and `check_genesis_hash`. [#1537]
* [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
* [ ] 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
* [ ] I'm linking the issue being fixed by this PR
Some users use `bdk_esplora` to update `bdk_chain` structures *without* a `LocalChain`. ~~This PR introduces "low-level" methods to `EsploraExt` and `EsploraAsyncExt` which populates a `TxGraph` update with associated data.~~
We change `FullScanRequest`/`SyncRequest` to take in the `chain_tip` parameter as an option. Spk-based chain sources (`bdk_electrum` and `bdk_esplora`) will not fetch a chain-update if `chain_tip` is `None`, allowing callers to opt-out of receiving updates for `LocalChain`.
We change `FullScanRequest`/`SyncRequest` to have better ergonomics when inspecting the progress of syncing (refer to #1528).
We change `FullScanRequest`/`SyncRequest` to be constructed with a builder pattern. This is a better API since we separate request-construction and request-consumption.
Additionally, much of the `bdk_esplora` logic has been made more efficient (less calls to Esplora) by utilizing the `/tx/:txid` endpoint (`get_tx_info` method). This method returns the tx and tx_status in one call. The logic for fetching updates for outpoints has been reworked to support parallelism.
Documentation has also been updated.
### Notes to reviewers
This PR has evolved somewhat. Initially, we were adding more methods on `EsploraExt`/`EsploraAsyncExt` to make syncing/scanning more modular. However, it turns out we can just make the `chain_tip` parameter of the request structures optional. Since we are changing the request structures, we might as well go further and improve the ergonomics of the whole structures (refer to #1528). This is where we are at with this PR.
Unfortunately, the changes are now breaking. I think this is an okay tradeoff for the API improvements (before we get to stable).
### Changelog notice
* Change request structures in `bdk_chain::spk_client` to be constructed via a builder pattern, make providing a `chain_tip` optional, and have better ergonomics for inspecting progress while syncing.
* Change `bdk_esplora` to be more efficient by reducing the number of calls via the `/tx/:txid` endpoint. The logic for fetching outpoint updates has been reworked to support parallelism.
* Change `bdk_esplora` to always add prev-txouts to the `TxGraph` update.
### 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
Change `FullScanRequest` and `SyncRequest` take in a `chain_tip` as an
option. In turn, `FullScanResult` and `SyncResult` are also changed to
return the update `chain_tip` as an option. This allows the caller to
opt-out of getting a `LocalChain` update.
Rework `FullScanRequest` and `SyncRequest` to have better ergonomics
when inspecting the progress of items of a sync request. Richer progress
data is provided to the inspect closure.
Introduce `FullScanRequestBuilder` and `SyncRequestBuilder`. Separating
out request-construction and request-consumption in different structs
simplifies the API and method names.
Simplify `EsploraExt` and `EsploraAsyncExt` back to having two methods
(`full_scan` and `sync`). The caller can still opt out of fetching a
`LocalChain` update with the new `FullScanRequest` and `SyncRequest`.
Prevouts are needed to calculate fees for transactions. They are
introduced as floating txouts in the update `TxGraph`. A helper method
`insert_prevouts` is added to insert the floating txouts using the
`Vin`s returned from Esplora.
Also replaced `anchor_from_status` with `insert_anchor_from_status` as
we always insert the anchor into the update `TxGraph` after getting it.
Also removed `bitcoin` dependency as `bdk_chain` already depends on
`bitcoin` (and it's re-exported).
志宇 [Sun, 16 Jun 2024 07:37:11 +0000 (15:37 +0800)]
feat(esplora): make ext traits more flexible
Some users would like to use esplora updates with custom `ChainOracle`s
(not `LocalChain`). We introduce "low-level" extension methods that
populate an update `TxGraph` with associated data.
Additionally, much of the logic has been made more efficient. We make
use of the `/tx/:txid` endpoint (`get_tx_info` method) to do a single
call to get both the tx and tx_status. If the tx already exists, we only
fetch the tx_status. The logic for fetching data based on outpoints has
been reworked to be more efficient and it now supports parallelism
Additionally, the logic for fetching data based on outpoints has been
reworked to be more efficient and it now supports parallelism.
Documentation has also been reworked.
Note that this is NOT a breaking change because the API of `full_scan`
and `sync` are not changed.
The change descriptor is made optional, making this an effective reversion of #1390 and enables creating wallets with a single descriptor.
fixes #1511
### Notes to the reviewers
PR 1390 also removed an error case [`ChangePolicyDescriptor`](https://github.com/bitcoindevkit/bdk/blob/8eef350bd08057acc39b6fc50b1217db5e29b968/crates/wallet/src/wallet/mod.rs#L1529-L1533) and this can possibly be added back. In the case the wallet only has a single descriptor we allow any utxos to fund a tx that aren't specifically marked unspendable regardless of the change spend policy.
### Changelog notice
* Added method `Wallet::create_single` that expects a single `D: IntoWalletDescriptor` as input and enables building a wallet with no internal keychain.
### 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] This pull request breaks the existing API
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
to just `descriptor` that takes a `KeychainKind` and optional
`D: IntoWalletDescriptor` representing the expected value of
the descriptor in the changeset.
Add method `LoadParams::extract_keys` that will use any private
keys in the provided descriptors to add a signer to the wallet.
valued mammal [Tue, 6 Aug 2024 18:52:14 +0000 (14:52 -0400)]
fix(wallet): Change FromSql type to `Option<_>`
when selecting a wallet row from sqlite. This is consistent with
the structure of the wallet `ChangeSet` where the fields
`descriptor`, `change_descriptor`, and `network` are all optional.
This PR is a companion to bitcoindevkit/rust-electrum-client#135. It enables choosing the `ring` dependency on rustls instead of the new default (as of 0.23.0) `aws-lc-rs`. The AWS dependency breaks the Android and Swift builds. I wrote a more detailed explanation on [#135](https://github.com/bitcoindevkit/rust-electrum-client/pull/135).
### Notes to the reviewers
Do not merge before:
- [x] [#135](https://github.com/bitcoindevkit/rust-electrum-client/pull/135) is merged
- [x] A new version of rust-electrum-client is released (will be 0.21.0)
- [x] The dependency points to the new version of the client rather than my fork of it.
### Changelog notice
```md
Added
- bdk_electrum now enables choosing either the `use-rustls` or `use-rustls-ring` feature
```
### 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
This PR also serves as an issue; it can't really be merged as is. I just didn't want to open an issue and just ask for better docs and instead decided to open a PR with some unfinished new API docs.
I am working on a page for the Book of BDK that focuses on bdk_wallet + bdk_electrum syncing. A few things have been unclear to me, and I think slight additions to the API docs would fix that for others.
~~1. I was wondering what exactly this `confirmation_time` field on the `bdk_chain::ConfirmationTimeHeightAnchor` type represents. After digging into it (at least for the electrum client), it represents the timestamp specified by the block header where the tx was confirmed. _Question: I'd like to confirm that this always the case, e.g. that this is the timestamp used in cases where your client is an Esplora or bitcoind RPC node?_ If so, my addition to the sentence will make sense and is ready to go.~~ Edit: this is no longer a type after the rebase.
2. I think it'd be great to add context to the `fetch_prev_txouts` argument on the full_scan and sync methods on the `BdkElectrumClient`. It says that this is necessary for "fee calculation". What does that mean? I assume it means "for calculating the fee rate on the given transactions"? (let me know if that's wrong). Even so, I'm left wondering what happens if I don't fetch them. Will my fee calculations be just plain wrong? Or will they be unavailable? A bit more context for the caller of the method would be great here. If someone knows the definite answer to this let me know and feel free to propose a doc line and I'll amend the commit!
### Checklists
* [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
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
Closes #1125.
### Description
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
Add new test for `bdk_electrum` to make sure previously unconfirmed transactions get confirmed again in both reorg and no-reorg situations.
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
* Added `wait_until_electrum_sees_txid` method to `TestEnv`.
* `wait_until_electrum_sees_block` now has a `Duration` input for timeout.
* Removed exponential polling delay in `wait_until_electrum_sees_block`.
* Added `test_sync` to `bdk_electrum` to make sure previously unconfirmed transactions get confirmed in both reorg and no-org situations.
### 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
I think this will be useful for passing a custom bitcoin config including e.g. `-blockfilterindex` that would not otherwise be included in the default config.
* [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
Wei Chen [Thu, 8 Aug 2024 14:17:01 +0000 (22:17 +0800)]
feat(testenv): Added `bdk_electrum` wait for Txid method
Added `wait_until_electrum_sees_txid` method to `TestEnv`. Both
`bdk_electrum` wait methods now have a `timeout` option. Removed
the exponential polling delay in lieu of a fixed delay inside the
`bdk_electrum` wait methods.
- Adds two commands `init` and `generate`. Loading database doesn't require descriptors
- Replaces `send` command with `psbt` (new, sign, and extract). Supports tap key spend
- Uses `bdk-coin-select` and miniscript `plan` module
fixes #1469
partially addresses #1462
### Notes to the reviewers
Note the `example_cli` lib now defines the `ChangeSet` and `Anchor` type whereas before these were generic
### Changelog notice
* Improvements to `example_cli` that include generating descriptors and creating PSBTs
### 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
valued mammal [Fri, 9 Aug 2024 02:43:29 +0000 (22:43 -0400)]
feat(testenv): Add method `new_with_config`
This allows passing a custom config for either `BitcoinD` or
`ElectrsD`. We also add a new `pub struct Config` for holding
each of `bitcoind::Conf` and `electrsd::Conf`.
[Rust 1.60 introduces *"namespaced features"*](https://blog.rust-lang.org/2022/04/07/Rust-1.60.0.html#new-syntax-for-cargo-features) so we can add a dependency without implicitly also defining a feature of the same name.
We can now get rid of crate-renaming and use "namespaced features" instead.
Thanks to @LLFourn for pointing this out.
### Changelog notice
* Remove crate-renaming in `bdk_chain` and use "namespaced features" 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
Skimming through the BDK code today i noticed the descriptor checksum algorithm is re-implemented instead of using the implementation from rust-miniscript. Re-implementing it here is more code to review and maintain, more room for mistakes and overall less eyes over the code than centralizing a single implementation over at `rust-miniscript`.
While doing this i noticed that one of the variants was completely unnecessary (`calc_checksum_bytes`), so i removed it. I realise it's breaking the API, so if we want to avoid that i can remove this part. I also added a commit to remove redundant calls to `calc_checsum`.
Overall this is just a quick PoC as i noticed it i figured i'd send a PR, my bad if i missed anything!
* [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
* [ ] 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
* [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
* [ ] 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
Antoine Poinsot [Thu, 25 Jul 2024 14:51:16 +0000 (16:51 +0200)]
bdk_wallet: remove unnecessary calls to calc_checksum
This code was:
1. Removing the checksum from the descriptor string, asserting it was
present
2. Re-calculating the checksum on the descriptor string absent the
checksum
Instead, just use the existing checksum? If we don't trust it we
probably shouldn't assert its presence in the first place.
The latest tokio minor version update from 1.38.1 to 1.39.1 changed it's MSRV from 1.63.0 to 1.70.0, breaking our CI MSRV 1.63 testing. This PR pins `tokio` back to 1.38.1 for our CI MSRV testing.
### Notes to the reviewers
https://github.com/tokio-rs/tokio/pull/6645
https://crates.io/crates/tokio/versions
### 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
bdk_chain to 0.17.0
bdk_bitcoind_rpc to 0.13.0
bdk_electrum to 0.16.0
bdk_esplora to 0.16.0
bdk_file_store to 0.14.0
bdk_testenv to 0.7.0
bdk_hwi to 0.4.0
### Notes to the reviewers
This release completes the 1.0.0-alpha milestone and starts our new 1.0.0-beta milestone series of releases.
The beta releases will maintain a stable `bdk_wallet` API and be focused on improving testing, docs, CI, and fixing any newly discovered bugs.
Steve Myers [Mon, 22 Jul 2024 02:47:39 +0000 (21:47 -0500)]
Bump bdk version to 1.0.0-beta.1
bdk_chain to 0.17.0
bdk_bitcoind_rpc to 0.13.0
bdk_electrum to 0.16.0
bdk_esplora to 0.16.0
bdk_file_store to 0.14.0
bdk_testenv to 0.7.0
bdk_hwi to 0.4.0
Make all method signatures of `KeychainTxOutIndex` take owned `K` and use `ScriptBuf` instead of its borrowed counterpart `&Script`. Fixes #1482
### Notes to the reviewers
Steve also added a CI fix as well
### Changelog notice
- Make all method signatures of `KeychainTxOutIndex` take owned `K`
- Update `KeychainTxOutIndex` methods to use `ScriptBuf`
### 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
* [ ] 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
Rework sqlite: Instead of only supported one schema (defined in `bdk_sqlite`), we have a schema per changeset type for more flexiblity.
* rm `bdk_sqlite` crate (as we don't need `bdk_sqlite::Store` anymore).
* add `sqlite` feature on `bdk_chain` which adds methods on each changeset type for initializing tables, loading the changeset and writing.
Rework changesets: Some callers may want to use `KeychainTxOutIndex` where `K` may change per descriptor on every run. So we only want to persist the last revealed indices by `DescriptorId` (which uniquely-ish identifies the descriptor).
* rm `keychain_added` field from `keychain_txout`'s changeset.
* Add `keychain_added` to `CombinedChangeSet` (which is renamed to `WalletChangeSet`).
Rework persistence: add back some safety and convenience when persisting our types. Working with changeset directly (as we were doing before) can be cumbersome.
* Intoduce `struct Persisted<T>` which wraps a type `T` which stores staged changes to it. This adds safety when creating and or loading `T` from db.
* `struct Persisted<T>` methods, `create`, `load` and `persist`, are available if `trait PersistWith<Db>` is implemented for `T`. `Db` represents the database connection and `PersistWith` should be implemented per database-type.
* For async, we have `trait PersistedAsyncWith<Db>`.
* `Wallet` has impls of `PersistedWith<rusqlite::Connection>`, `PersistedWith<rusqlite::Transaction>` and `PersistedWith<bdk_file_store::Store>` by default.
Rework wallet-construction: Before, we had multiple methods for loading and creating with different input-counts so it would be unwieldly to add more parameters in the future. This also makes it difficult to impl `PersistWith` (which has a single method for `load` that takes in `PersistWith::LoadParams` and a single method for `create` that takes in `PersistWith::CreateParams`).
* Introduce a builder pattern when constructing a `Wallet`. For loading from persistence or `ChangeSet`, we have `LoadParams`. For creating a new wallet, we have `CreateParams`.
### Notes to the reviewers
TODO
### Changelog notice
```
### Added
- Add `sqlite` feature to `bdk_chain` which introduces methods on changeset types that encode/decode changesets to SQLite database.
* Introduce `PersistWith<Db>` and `PersistAsyncWith<Db>` traits and a `Persisted` wrapper. This ergonomically makes sure user inits the db before reading/writing to it.
### Changed
- Moved `bdk_chain::CombinedChangeSet` to `bdk_wallet::ChangeSet` and added `keychain_added` field.
- `bdk_wallet::Wallet` construction now uses a builder API using the newly introduced `CreateParams` and `LoadParams`.
### Removed
- Remove `keychains_added` field from `bdk_chain::keychain_txout::ChangeSet`.
```
### 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
feat(chain,wallet)!: Change persist-traits to be "safer"
Previously, `Persist{Async}With::persist` can be directly called as a
method on the type (i.e. `Wallet`). However, the `db: Db` (which is an
input) may not be initialized. We want a design which makes it harder
for the caller to make this mistake.
We change `Persist{Async}With::persist` to be an "associated function"
which takes two inputs: `db: &mut Db` and `changeset`. However, the
implementer cannot take directly from `Self` (as it's no longer an
input). So we introduce a new trait `Staged` which defines the staged
changeset type and a method that gives us a `&mut` of the staged
changes.
feat!: Rework sqlite, changesets, persistence and wallet-construction
Rework sqlite: Instead of only supported one schema (defined in
`bdk_sqlite`), we have a schema per changeset type for more flexiblity.
* rm `bdk_sqlite` crate (as we don't need `bdk_sqlite::Store` anymore).
* add `sqlite` feature on `bdk_chain` which adds methods on each
changeset type for initializing tables, loading the changeset and
writing.
Rework changesets: Some callers may want to use `KeychainTxOutIndex`
where `K` may change per descriptor on every run. So we only want to
persist the last revealed indices by `DescriptorId` (which uniquely-ish
identifies the descriptor).
* rm `keychain_added` field from `keychain_txout`'s changeset.
* Add `keychain_added` to `CombinedChangeSet` (which is renamed to
`WalletChangeSet`).
Rework persistence: add back some safety and convenience when persisting
our types. Working with changeset directly (as we were doing before) can
be cumbersome.
* Intoduce `struct Persisted<T>` which wraps a type `T` which stores
staged changes to it. This adds safety when creating and or loading
`T` from db.
* `struct Persisted<T>` methods, `create`, `load` and `persist`, are
avaliable if `trait PersistWith<Db>` is implemented for `T`. `Db`
represents the database connection and `PersistWith` should be
implemented per database-type.
* For async, we have `trait PersistedAsyncWith<Db>`.
* `Wallet` has impls of `PersistedWith<rusqlite::Connection>`,
`PersistedWith<rusqlite::Transaction>` and
`PersistedWith<bdk_file_store::Store>` by default.
Rework wallet-construction: Before, we had multiple methods for loading
and creating with different input-counts so it would be unwieldly to add
more parameters in the future. This also makes it difficult to impl
`PersistWith` (which has a single method for `load` that takes in
`PersistWith::LoadParams` and a single method for `create` that takes in
`PersistWith::CreateParams`).
* Introduce a builder pattern when constructing a `Wallet`. For loading
from persistence or `ChangeSet`, we have `LoadParams`. For creating a
new wallet, we have `CreateParams`.
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
Fixes #980.
### Description
This PR is the first step in reworking `bdk_electrum` to use merkle proofs. When we fetch a transaction, we now also obtain the merkle proof and block header for verification. We then insert an anchor only after validation that the transaction exists in a confirmed block. The loop logic that previously existed in `full_scan` to account for re-orgs has also been removed as part of this rework.
This is a breaking change because `graph_update`s now provide the full `ConfirmationTimeHeightAnchor` type. This removes the need for the `ElectrumFullScanResult` and `ElectrumSyncResult` structs that existed only to provide the option for converting the anchor type from `ConfirmationHeightAnchor` into `ConfirmationTimeHeightAnchor`.
### 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 -->
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
* `ConfirmationTimeHeightAnchor` and `ConfirmationHeightAnchor` have been removed.
* `ConfirmationBlockTime` has been introduced as a new anchor type.
* `bdk_electrum`'s `full_scan` and `sync` now return `graph_update`s with `ConfirmationBlockTime`.
### 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
Both `bdk_electrum` and `bdk_esplora` now report the exact block
that the transaction is in, which removes the need for having the
old `ConfirmationTimeHeightAnchor` and `ConfirmationHeightAnchor`.
This PR introduces a new, simpler anchor type that can be modified
to support additional data in the future.
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
`cc` version 1.0.106 raised `msrv` to 1.67.
The previous working version 1.0.105 was pinned for CI to continue working.
### 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 -->
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
* Pinned cc dependency version to build with rust 1.63.
### 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
Simplifies `public_descriptor` function by using `get_descriptor` and removes `get_descriptor_for_keychain`.
### Notes to the reviewers
Tested with `cargo test --all-features`.
### Changelog notice
- Simplifies `public_descriptor` function and removes `get_descriptor_for_keychain`
### 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
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
Fixes #1467.
### Description
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
Renames the `Append` trait to `Merge`.
### 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 -->
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
* Rename `bdk_chain::Append` to `bdk_chain::Merge`.
### 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
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
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>
FadedCoder [Sun, 27 Feb 2022 05:49:53 +0000 (11:19 +0530)]
feat(wallet): add TxOrdering::Custom
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>
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 #1446
fixes #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
valued mammal [Thu, 23 May 2024 21:33:45 +0000 (17:33 -0400)]
fix(tx_graph)!: Change tx_last_seen to `Option<u64>`
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`.
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
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
Tobin C. Harding [Wed, 26 Jun 2024 04:33:00 +0000 (14:33 +1000)]
Remove usage of blockdata:: from bitcoin paths
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.
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
projects / bdk / log