Adds missing persistence for `first_seen`, which was not included in #1950.
### Changelog notice
- Adds `first_seen` column to the `bdk_txs` table via schema v3 migration.
- Updates `from_sqlite()` and `persist_to_sqlite()` to handle `first_seen`.
- Updates the v0-to-v3 migration test to verify compatibility with older schemas.
### 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 +nightly 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
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
`is_empty` methods are helpful in various contexts so I added them.
### Changelog notice
```md
Added
- `is_empty` methods to `TxUpdate`, `SyncResponse` and `FullScanResponse`
```
### 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
This PR adds basic formatting configuration for uniform comments. Note that the nightly toolchain is necessary.
### Changelog:
- Create rustfmt.toml with this config:
```
comment_width = 100
format_code_in_doc_comments = true
wrap_comments = true
```
- Apply rustfmt.toml.
- Update the fmt CI job to use the nightly toolchain.
- Update PR template to use nightly toolchain.
Note: I had to add #[rustfmt::skip] to test_extract_satisfaction_timelock() because the base64 PSBT would get wrapped due to the slashes.
#### 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 +nightly fmt` and `cargo clippy` before committing
ci: add zizmor github actions security analysis workflow and fix possible vulnerabilities
### 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
Update `rust-version` to 1.86.0 and address some clippy lints, in particular [`clippy::doc_overindented_list_items`](https://rust-lang.github.io/rust-clippy/master/index.html#doc_overindented_list_items).
fix #1940
### 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. -->
### Description
It's a minor documentation fix, as reported during the audit and referenced in bitcoindevkit/bdk_wallet#59.
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
I'm unsure if anything else / any other explanation should be included in the documentation. Let me know if you think more context should be added to it.
<!-- 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 -->
<!-- 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
This PR solves issue #1947 by implementing `first_seen` tracking.
* Added `first_seen` field to `TxGraph` and `ChangeSet` so that `first_seen` timestamp can be added when inserting a new seen-at using `insert_seen_at`.
* `first_seen` added to `TxNode` as a way to retrieve the first-seen timestamp for a transaction.
* `first_seen` added to `ChainPosition::Unconfirmed` to order unconfirmed transactions by `first_seen`.
* New tests have been added for the above described functionalities.
### Changelog notice
* Add tracking first-seen timestamps of transactions
<!-- 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
Work for this began as part of #1811, based on this [comment](https://github.com/bitcoindevkit/bdk/pull/1811#discussion_r1945941691).
`Emitter::mempool` now returns a `MempoolEvent` which provides the data for tracking `evicted_at`:
```
pub struct MempoolEvent {
/// Unemitted transactions or transactions with ancestors that are unseen by the receiver.
///
/// To understand the second condition, consider a receiver which filters transactions based on
/// whether it alters the UTXO set of tracked script pubkeys. If an emitted mempool transaction
/// spends a tracked UTXO which is confirmed at height `h`, but the receiver has only seen up to
/// block of height `h-1`, we want to re-emit this transaction until the receiver has seen the
/// block at height `h`.
pub new_txs: Vec<(Transaction, u64)>,
/// [`Txid`]s of all transactions that have been evicted from mempool.
pub evicted_txids: HashSet<Txid>,
/// The latest timestamp of when a transaction entered the mempool.
///
/// This is useful for setting the timestamp for evicted transactions.
pub latest_update_time: u64,
}
```
### Changelog notice
* Change `Emitter::mempool` to return `MempoolEvent`s which contain mempool-eviction data.
* Change `Emitter::client` to have more relaxed generic bounds. `C: Deref, C::Target: RpcApi` are the new bounds.
* Add conversion impls for `CanonicalTx` to `Txid`/`Arc<Transaction>`.
* Add `ChainPosition::is_unconfirmed` method.
### 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
feat(bitcoind_rpc)!: Reduce friction of `Emitter` API.
* Change signature of `Emitter::new` so that `expected_mempool_txids`
can be more easily constructed from `TxGraph` methods.
* Change generic bounds of `C` within `Emitter<C>` to be `C: DeRef,
C::Target: RpcApi`. This allows the caller to have `Arc<Client>` as
`C` and does not force to caller to hold a lifetimed reference.
Add the ability to modify the canonicalization algorithm. Right now, the only modifier is `assume_canonical` which takes in a `Vec` (ordered list) of txids and superimposes it on the canonicalization algorithm. Txs later in the list (higher index) have a higher priority (in case of conflicts).
### Notes to the reviewers
None at the moment.
### Changelog notice
* Added `CanonicalizationParams` to allow the caller to modify the canonicalization algorithm. This in a new parameter on `CanonicalIter::new`.
* Changed `TxGraph::insert_tx` to allow for updating a transaction's witness field. This is useful for initially introducing an unsigned tx and adding witnesses later on.
### 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
志宇 [Thu, 23 Jan 2025 04:16:46 +0000 (15:16 +1100)]
feat(chain)!: Add ability to modify canonicalization algorithm
Introduce `CanonicalizationParams` which is passed in to
`CanonicalIter::new`.
`CanonicalizationParams::assume_canonical` is the only field right now.
This contains a list of txids that we assume to be canonical,
superceding any other canonicalization rules.
Bumps [Swatinem/rust-cache](https://github.com/swatinem/rust-cache) from 2.7.7 to 2.7.8.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a href="https://github.com/swatinem/rust-cache/releases">Swatinem/rust-cache's releases</a>.</em></p>
<blockquote>
<h2>v2.7.8</h2>
<h2>What's Changed</h2>
<ul>
<li>Include CPU arch in the cache key for arm64 Linux runners by <a href="https://github.com/rhysd"><code>@rhysd</code></a> in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/228">Swatinem/rust-cache#228</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a href="https://github.com/Swatinem/rust-cache/compare/v2.7.7...v2.7.8">https://github.com/Swatinem/rust-cache/compare/v2.7.7...v2.7.8</a></p>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/Swatinem/rust-cache/blob/master/CHANGELOG.md">Swatinem/rust-cache's changelog</a>.</em></p>
<blockquote>
<h2>2.7.8</h2>
<ul>
<li>Include CPU arch in the cache key</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/Swatinem/rust-cache/commit/9d47c6ad4b02e050fd481d890b2ea34778fd09d6"><code>9d47c6a</code></a> 2.7.8</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/27b8ea9368cf428f0bfe41b0876b1a7e809d9844"><code>27b8ea9</code></a> Include CPU arch in the cache key (<a href="https://redirect.github.com/swatinem/rust-cache/issues/228">#228</a>)</li>
<li>See full diff in <a href="https://github.com/swatinem/rust-cache/compare/v2.7.7...v2.7.8">compare view</a></li>
</ul>
</details>
<br />
Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.
<details>
<summary>Dependabot commands and options</summary>
<br />
You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
When we initially created the canonicalization algorithm, we didn't expect callers to insert invalid transactions into the graph. However, user error happens and we should handle it correctly.
Before this PR, when inserting transactions that double-spend themselves (where two or more different inputs of the transactions conflict), the canonicalization result will have inconsistencies.
### Notes to the reviewers
Logic changes are all contained in `CanonicalIter::mark_canonical`. `mark_canonical` will detect whether the `tx` being passed in double spends itself. If so, we abort and undo all changes made so far.
There is a slight <2% degradation in performance with this change (except in two cases where there is a performance improvement of ~10%).
* Fix canonicalization mess-up when transactions that conflict with itself are inserted.
### 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
* [x] I'm linking the issue being fixed by this PR
志宇 [Fri, 28 Mar 2025 06:06:26 +0000 (17:06 +1100)]
fix(chain): Correctly handle txs that double spend themselves
Detect self-double-spends in `CanonicalIter::mark_canonical`. Disregard
the tx that self-double-spends and all it's attached meta data (such as
timestamps and anchors).
Add new test cases on `test_tx_conflict_handling` for transactions that
self-double-spend.
### Description
* Added a `from_changeset()` function to `TxGraph` implementation
* This function initializes a new `TxGraph` and applies the given `ChangeSet`
* The method allows constructing a `TxGraph` directly from a `ChangeSet`,
simplifying graph creation.
#### 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
Removes the `bdk_wallet` crate from the workspace of this repo now that it's been moved to it's own repo.
### Notes to the reviewers
I did a bit of extra cleanup on the README file. I replaced the crates list with a table that also includes badges for the crate version and docs. I also renamed the `example-crates` folder to simply `examples`, a nit of mine that I already fixed in the new `bdk_wallet` repo.
See: bitcoindevkit/bdk_wallet#18
### Changelog notice
For the changelog for the next version of `bdk_wallet`:
- The `bdk_wallet` crate has been removed from the `bdk` repository and move to it's own `bdk_wallet` repository.
### 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
feat(chain): Add method for constructing `TxGraph` from a `ChangeSet`
* Added a `from_changeset()` function to `TxGraph` implementation
* This function initializes a new `TxGraph` and applies the given `ChangeSet`
* The method allows constructing a `TxGraph` directly from a `ChangeSet`,
simplifying graph creation.
* [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
The PR aims to add a `--feerate` option to the psbt `New` command.
Also in this PR:
- bump `bdk_coin_select` to 0.4
- fix collecting `Assets` and enable support for more descriptor types
### 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
Automated update to Github CI workflow `cont_integration.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
I removed the nightly_docs workflow since it's not needed. The latest published bdk crate docs can be found on docs.rs.
### Notes to the reviewers
We early in the project life we published nightly docs since the API was changing often and releases were infrequent. But now that the api is more stable and we make regular releases nightly docs aren't needed.
### 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. -->
### Description
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
The publish_docs workflow pushes changes to this repository: "bitcoindevkit/bitcoindevkit.org" which requires valid credentials. By setting the persist-credentials to false in #1778, the credentials required are not made available.
### 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 -->
To fix the issue I added a write permission to the publish_jobs, which will allow it to push changes without the credentials.
### 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:
* [ ] 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
This PR bumps the `check-wasm` job runner image to `ubuntu-24.04`. It was previously running `ubuntu-20.04`, which is set to be unsupported in a 3 weeks (see https://github.com/actions/runner-images/issues/11101); `clang` gets bumped to `clang-14` because of this.
### 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 allows the receiving structures (`bdk_chain`, `bdk_wallet`) to detect and evict incoming transactions that are double spent (cancelled).
We add a new field to `TxUpdate` (`TxUpdate::evicted_ats`), which in turn, updates the `last_evicted` timestamps that are tracked/persisted by `TxGraph`. This is similar to how `TxUpdate::seen_ats` update the `last_seen` timestamp in `TxGraph`. Transactions with a `last_evicted` timestamp higher than their `last_seen` timestamp are considered evicted.
`SpkWithExpectedTxids` is introduced in `SpkClient` to track expected `Txid`s for each `spk`. During a sync, if any `Txid`s from `SpkWithExpectedTxids` are not in the current history of an `spk` obtained from the chain source, those `Txid`s are considered missing. Support for `SpkWithExpectedTxids` has been added to both `bdk_electrum` and `bdk_esplora` chain source crates.
The canonicalization algorithm is updated to disregard transactions with a `last_evicted` timestamp greater than or equal to their `last_seen` timestamp, except in cases where transitivity rules apply.
### Notes to the reviewers
This PR does not fix #1740 for block-by-block chain source (such as `bdk_bitcoind_rpc`). This work is done in a separate PR (#1857).
### Changelog notice
* Add `TxUpdate::evicted_ats` which tracks transactions that have been replaced and are no longer present in mempool.
* Change `TxGraph` to track `last_evicted` timestamps. This is when a transaction is last marked as missing from the mempool.
* The canonicalization algorithm now disregards transactions with a `last_evicted` timestamp greater than or equal to it's `last_seen` timestamp, except when a canonical descendant exists due to rules of transitivity.
* Add `SpkWithExpectedTxids` in `spk_client` which keeps track of expected `Txid`s for each `spk`.
* Change `bdk_electrum` and `bdk_esplora` to understand `SpkWithExpectedTxids`.
* Add `SyncRequestBuilder::expected_txids_of_spk` method which adds an association between `txid`s and `spk`s.
### 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
志宇 [Fri, 24 Jan 2025 08:45:00 +0000 (19:45 +1100)]
feat(chain)!: Change `TxGraph` to understand evicted-at & last-evicted
The evicted-at and last-evicted timestamp informs the `TxGraph` when the
transaction was last deemed as missing (evicted) from the mempool.
The canonicalization algorithm is changed to disregard transactions with
a last-evicted timestamp greater or equal to it's last-seen timestamp.
The exception is when we have a canonical descendant due to rules of
transitivity.
Update rusqlite_impl to persist `last_evicted`.
Also update docs:
* Remove duplicate paragraphs about `ChangeSet`s.
* Add "Canonicalization" section which expands on methods that require
canonicalization and the associated data types used in the
canonicalization algorithm.
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
- Pin `minreq` to `2.13.2` because `2.13.3` uses the unstable `std::sync::LazyLock` (https://github.com/bitcoindevkit/bdk/pull/1886#issuecomment-2714736611).
- Pin `base64ct` to `1.6.0` for MSRV.
Required for #1885 (`ubuntu-24.04` on all CI workflows).
### 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
fixes #1873 based on solution proposed by @praveenperera .
### Notes to the reviewers
This is not a breaking change since it's only changing the internal `_marker` field's type.
### Changelog notice
- Fix PersistedWallet to be Send + Sync, even when used with a !Sync persister type such as rusqlite::Connection.
### 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
* [x] I'm linking the issue being fixed by this PR
Steve Myers [Thu, 6 Mar 2025 03:12:02 +0000 (21:12 -0600)]
fix(wallet): allow PersistedWallet<P> to be Send + Sync even if P is !Sync
The goal of this change is to ensure that PersistWallet<P> remains Send and Sync, even if the functions implemented on it
have a &mut P parameter and P is not Sync.
The reason to change PersistWallet<P>'s _marker field type from PhatonData<P> to PhantomData<fn(&mut P)> is to tell the
Rust compiler that this struct is a "consumer" of &mut P, but it does not own P and therefore should not be tied to its
lifetime.
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
- Removed `persist` crate from `README.md`
### 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. -->
### Description
- Removed mentions of a future 1.0 release
- Fixed broken links to example 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
* [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 would imagine many users would be handling a `Address<NetworkChecked>` when building a transaction. They may pass this structure directly with this patch, or continue to use `ScriptBuf`.
### Notes to the reviewers
To my knowledge this is non-breaking, but it is a change in the function signature so I am not sure.
### Changelog notice
Accept any type that is convertible to a `ScriptBuf` in `TxBuilder::add_recipient`
### 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:
* [ ] 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
This PR makes passing a `CHANGE_DESCRIPTOR` optional on `example_wallet_rpc`, as per #1533.
### Notes to the reviewers
Before instantiating a `Wallet` it checks if `args.change_descriptor` is Some. If true calls `Wallet::create`, else calls `Wallet::create_single`.
#### 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
* [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 makes `Regtest` the default network on `wallet_example_rpc` and updates the README accordingly.
### Notes to the reviewers
It had `Testnet` as the default network, `8332` as the default port and `481824` as the starting height; now everything is `Regtest`.
### 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 makes the discussion in #1762 easier to find for new contributors.
### 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
The `Store::open` method doesn't recovers the previous `Store` state saved in the file and emplaces the file pointer just after the magic bytes prefix, this later is agravated by `Store::append_changeset` which sets the file pointer after the last written changeset. The combination of both causes the lost of any previous changeset there may have been in the file.
Is natural to think this shouldn't be the expected behavior, as @KnowWhoami pointed out in #1517, and the `Store` should recover the previous changesets stored in the file store.
The approach proposed in #1632 triggered a discussion about more changes in `Store`, which motivated the current refactor.
Besides the fix for #1517, the following methods have been changed/renamed/repurposed in Store:
- `create`: create file and retrieve store, if exists fail.
- `load`: load changesets from file, aggregate them and return aggregated changeset and `Store`. If there are problems with decoding, fail.
- `dump`: aggregate all changesets and return them.
- `load_or_create`: try load or fallback to create.
Fixes #1517.
Overrides #1632.
### Notes to the reviewers
**Load** return type is `Result<(Option<C>, Store), StoreErrorWithDump>` to allow methods which doesn't use `WalletPersister` to get the aggregated changeset right away.
**Dump** is kept to allow `WalletPersister::initialize` method to retrieve the aggregated changeset without forcing the inclusion of the additional parameters needed by `store::load` to the trait, which would also affect the `rusqlite` implementation.
### Changelog notice
#### Added:
- `StoreError` enum, which includes `Io`, `Bincode` and `InvalidMagicBytes`.
- "not intended for production" notice in general `README` for `file store`.
#### Changed:
- `Store::create_new` to `Store::create`, with new return type: `Result<Self, StoreError>`
- `Store::open` to `Store::load`, with new return type: `Result<(Self, Option<C>), StoreErrorWithDump<C>>`
- `Store::open_or_create` to `Store::load_or_create`, with new return type: `Result<(Option<C>, Self), StoreErrorWithDump<C>>
`
- `Store::aggregate_changesets` to `Store::dump`, with new return type: `Result<Option<C>, StoreErrorWithDump<C>>`
- `FileError` to `StoreError`
- `AggregateChangesetsError` to `StoreErrorWithDump`, which now can include all the variants of `StoreError` in the error field.
#### Deleted:
- `IterError` deleted.
<!-- 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~~
* ~~I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)~~
* ~~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~~
There were multiple calls for de-duplication of selected UTxOs in `Wallet::create_tx`: ([1](https://github.com/bitcoindevkit/bdk/blob/abc305612160c0e2ce85c7bba4c2c162ff488adc/crates/wallet/src/wallet/mod.rs#L2016-L2020)) and ([2](https://github.com/bitcoindevkit/bdk/blob/abc305612160c0e2ce85c7bba4c2c162ff488adc/crates/wallet/src/wallet/mod.rs#L1452)).
As the test [`test_filter_duplicates`](https://github.com/bitcoindevkit/bdk/blob/master/crates/wallet/src/wallet/coin_selection.rs#L1666-L1695) shows, there are four possible cases for duplication of UTxOs while feeding the coin selection algorithms.
1. no duplication: out of concern
2. duplication in the required utxos only: covered by the source of `required_utxos`, `Wallet::list_unspent`, which [roots back the provided `UTxOs` to a `HashMap`](https://github.com/bitcoindevkit/bdk/blob/a5335a18431dfecea1f524af44c953b79a96867c/crates/chain/src/tx_graph.rs#L911-L912) which should avoid any duplication by definition
3. duplication in the optional utxos only: is the only one possible as optional `UTxOs` are stored in a `Vec` and no checks are performed about the duplicity of their members.
4. duplication across the required and optional utxos: is already covered by `Wallet::preselect_utxos`, which avoid the processing of required UTxOs while listing the unspent available UTxOs in the wallet.
This refactor does the following:
- Refactors `TxParams::utxos` type to be `HashSet<LocalOutput>` avoiding the duplication case 3
- Keeps avoiding case 4 without overhead and allowing a query closer to O(1) on avg. to cover duplication case 4 (before was O(n) where n is the size of required utxos) thanks to the above change.
- Still covers case 2 because the [source of `required_utxos`, `Wallet::list_unspent`](https://github.com/bitcoindevkit/bdk/blob/a5335a18431dfecea1f524af44c953b79a96867c/crates/chain/src/tx_graph.rs#L911-L912) comes from a `HashMap` which should avoid duplication by definition.
- Moves the computation of the `WeightedUtxos` to the last part of UTxO filtering, allowing the unification of the computation for local outputs.
- Removes some extra allocations done for helpers structures or intermediate results while filtering UTxOs.
- Allows for future integration of UTxO filtering methods for other utilities, e.g.: filter locked utxos
```rust
.filter(|utxo| !self.is_utxo_locked(utxo.outpoint, self.latest_checkpoint().height()))
```
- Adds more comments for each filtering step.
- Differentiates `foreign_utxos`, which should include a provided satisfation weight to use them effectively, and `utxos`, manually selected UTxOs for which the wallet can compute their satisfaction weight without external resources.
With these changes all four cases would be covered, and `coin_selection::filter_duplicates` is no longer needed.
Fixes #1794.
### Notes to the reviewers
I added three test to cover the interesting cases for duplication:
- there are no duplicates in required utxos
- there are no duplicates in optional utxos
- there are no duplicates across optional and required utxos
the three of them have been prefixed with `not_duplicated_utxos*` to allow its joint execution under the command:
```bash
cargo test -- not_duplicated_utxos
```
because the guarantees for the three conditions above are spread in different parts of the code.
### Changelog notice
No changes to public APIs.
### 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
* [x] This pull request does not break 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
nymius [Fri, 28 Feb 2025 04:39:57 +0000 (15:39 +1100)]
refactor(store)!: change Store's method and error names
The changes in this commit were motivated due to a bug in the
`StoreFile` which caused old data to be lost if the file was `open`
instead of created and new data was appended. The bugfix later motivated
a general name cleanup in StoreFile's methods and errors and some minor
changes in their signatures. FileError was renamed to StoreError, which
now includes the IterError variants, allowing the remplacement of the
former form. The new StoreFile methods are:
- create: create file in write only mode or fail if file exists.
- load: open existing file, check integrity of content and retrieve
Store.
- append: add new changesets to Store. Do nothing if changeset is empty.
- dump: aggregate and retrieve all stored changesets in store.
- load_or_create: load if file exists, create if not, and retrieve
Store.
As the code changed and there is not a single point to verificate the
following properties:
- there are no duplicates in required utxos
- there are no duplicates in optional utxos
- there are no duplicates across optional and required utxos
anymore, test have been prefixed with `not_duplicated_utxos*` to allow
its joint execution by using the following command:
cargo test -- not_duplicated_utxos
nymius [Mon, 13 Jan 2025 15:40:13 +0000 (12:40 -0300)]
refactor(wallet): use iterators and adaptors in preselect_utxos
There were multiple calls for de-duplication of selected UTxOs.
As the test `test_filter_duplicates` shows, there are four possible
cases for duplication of UTxOs while feeding the coin selection
algorithms.
1. no duplication: out of concern
2. duplication in the required utxos only: covered by the source of
`required_utxos`, `Wallet::list_unspent`, which roots back the
provided `UTxOs` to a `HashMap` which should avoid any duplication by
definition
3. duplication in the optional utxos only: is the only one possible as
optional `UTxOs` are stored in a `Vec` and no checks are performed
about the duplicity of their members.
4. duplication across the required and optional utxos: is already
covered by `Wallet::preselect_utxos`, which avoid the processing of
required UTxOs while listing the unspent available UTxOs in the
wallet.
This refactor changes:
- `TxParams::utxos` type to be `HashSet<LocalOutput>` avoiding the
duplication case 3, and allowing a query closer to O(1) on avg. to
cover duplication case 4 (before was O(n) where n is the size of
required utxos).
- Moves the computation of the `WeightedUtxos` to the last part of UTxO
filtering, allowing the unification of the computation for local
outputs.
- Removes some extra allocations done for helpers structures or
intermediate results while filtering UTxOs.
- Allows for future integration of UTxO filtering methods for other
utilities.
- Adds more comments for each filtering step.
With these changes all four cases would be covered, and
`coin_selection::filter_duplicates` would be no longer needed.
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
It looks like CI is failing due to the latest release of https://github.com/trifectatechfoundation/bzip2-rs/releases/tag/v0.5.2 which is a dependency of zip and therefore from electrsd.
It's failing on MSRV 1.63.0 step raising the following error: `error[E0658]: use of unstable library feature 'core_ffi_c'`.
This PR adds bzip2-sys to the pinned dependency list, pinning it to `0.1.12`.
<!-- 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 -->
### Changelog notice
- Adds `bzip2-sys` to the pinned dependency list, pinning it to `0.1.12`.
<!-- 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
#1811 introduces the `evicted-at` concept. While adding this to `TxUpdate`, I realized there were some shortcomings in our full-scan & sync flow:
* Chain sources that use `TxUpdate` to convey updates cannot choose to add transactions without a `seen_at` timestamp as the `TxGraph::apply_update_at` logic adds this timestamp for all unanchored txs if the `seen_at` parameter is specified. Purposefully adding uncanonical txs is useful for wallets that want to know about replaced txs.
* The `TxGraph::apply_update_at` logic is hard to reason about. `TxUpdate::seen_ats` already has the `seen_at` timestamp per tx, but `apply_update_at()` also takes in a `seen_at` timestamp`.
* `TxUpdate::seen_ats` currently forces us to only have one `seen-at` per tx as it's a map of `txid -> seen_at`. However, in the future we want to add a `first-seen` timestamp to `TxGraph` which is basically the earliest `seen-at` timestamp introduced so we may want to have multiple `seen_at`s per tx. The other issue is if we merge `TxUpdate`s, we will loose data. I.e. we can only keep the `last_seen` or the `first_seen`.
These problems were exacerbated when introducing `evicted-at`. In the old design, the chain-source has no concept of sync time (as sync time was introduced after-the-fact when applying the `TxUpdate`). Therefore the introduced `TxUpdate::evicted` field was a `HashSet<Txid>` (no timestamps) and relied on the `TxGraph::apply_upate_at` logic to introduce the `evicted-at` timestamp. All this makes the sync logic harder to understand. What happens if the `seen_at` input is `None`? What happens if updates were applied out of order? What happens when we merge `TxUpdates` before applying?
The following changes are made in this PR to simplify the sync/full-scan logic to be easier to understand and robust:
* The `sync_time` is added to the `SyncRequest` and `FullScanRequest`. `sync_time` is mandatory and is added as an input of `builder()`. If the `std` feature is enabled, the `builder_now()` is available which uses the current timestamp. The chain source is now responsible to add this `sync_time` timestamp as `seen_at` for mempool txs. Non-canonical and non-anchored txs can be added without the `seen_at` timestamp.
* `TxUpdate::seen_ats` is now a `HashSet` of `(Txid, u64)`. This allows multiple `seen_at`s per tx. This is also a much easier to use API as the chain source can just insert into this `HashSet` without checking previous data.
* `TxGraph::apply_update_at` is removed as we no longer needs to introduce a fallback `seen_at` timestamp after-the-fact. `TxGraph::apply_update` is no longer `std`-only and the logic of applying updates is greatly simplified.
Additionally, `TxUpdate` is updated to be `#[non_exhaustive]` for backwards-compatibility protection.
### Notes to the reviewers
This is based on #1837. Merge that first.
These are breaking changes to `bdk_core`. It needs to be breaking to properly fix all the issues.
As mentioned by @notmandatory, `bdk_wallet` changes will be added to a new PR once the new `bdk_wallet` repo is ready. But the PR won't be merged until we're ready for a `bdk_wallet` 2.0.
### Changelog notice
* Change `FullScanRequest::builder` and `SyncRequest::builder` methods to depend on `feature = "std"`. This is because requests now have a `start_time`, instead of specifying a `seen_at` when applying the update.
* Add `FullScanRequest::builder_at` and `SyncRequest::builder_at` methods which are the non-std version of the `..Request::builder` methods.
* Change `TxUpdate::seen_ats` field to be a `HashSet` of `(Txid, u64)`. This allows a single update to have multiple `seen_at`s per tx.
* Change `TxUpdate` to be `non-exhaustive`.
* Remove `apply_update_at` as we no longer need to apply with a timestamp after-the-fact.
### 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~ No tests needed as it's more of a refactor.
志宇 [Thu, 13 Feb 2025 11:08:18 +0000 (22:08 +1100)]
feat!: Improve spk-based syncing flow
* Change `TxUpdate::seen_ats` to be a `HashSet<(Txid, u64)>` so we can
introduce multiple timestamps per tx. This is useful to introduce both
`first_seen` and `last_seen` timestamps to `TxGraph`. This is also a
better API for chain-sources as they can just insert timestamps into
the field without checking previous values.
* Change sync/full-scan flow to have the request structure introduce the
sync time instead of introducing the timestamp when apply the
`TxUpdate`. This simplifies the `apply_update{_at}` logic and makes
`evicted_at` easier to reason about (in the future).
Remove the use of Cargo `path` ref for the `bdk_chain` and `bdk_file_store` dependencies. This is the first step to move `bdk_wallet` into a new workspace.
Also fixed CI failures as we were testing no-std without hashbrown.
### 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