志宇 [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
志宇 [Thu, 27 Feb 2025 23:47:20 +0000 (10:47 +1100)]
chore(wallet): Pin `bdk_chain` version to latest release
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.
The `bdk_esplora` crate depends on `reqwest` which now pins it's `idna_adapter` version to support newer versions of the `url` crate when testing with it's MSRV. I fixed our CI 1.75 MSRV build by also pinning the `idna_adapter`.
### Notes to the reviewers
The MSRV break was fixed by the `reqwest` team with seanmonstar/reqwest#2470.
This change was made 2 mo ago, I'm not sure why our CI only broke now.
### Changelog notice
- Pin idna_adapter dep to fix reqwest with MSRV 1.75.
### 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 is required since we are using Network::Testnet4
### Changelog notice
bdk_wallet:
- deps: Bump `bitcoin` to 0.32.4
### 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
### Description
`flate2` 1.1.0 raised msrv to 1.67. Previous version has been pinned.
### Changelog notice
* Pinned flate2 dependency to 1.0.35.
### 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 fixes a problem with `bdk_electrum` making bogus `transaction_get_merkle` calls to the Electrum server.
In electrum, heights returned alongside txs may be 0 or -1. 0 means the tx is unconfirmed. -1 means the tx is unconfirmed and spends an unconfirmed tx.
Previously, the codebase assumed that heights cannot be negative and used a `i32 as usize` cast (which would lead to the casted height being 18446744073709551615). Then the codebase will try to call `transaction_get_merkle` with the overflowed height.
### Notes to the reviewers
The test introduced in this PR does not fail with the old codebase. What ends up happening is that `transaction_get_merke` will be called with the overflow height and the Electrum server will return with "merkle not found".
To see the failure, you can change the `height as usize` casts to use `.try_into().expect()` then you will see a panic.
These changes should be applied as `1.0.1` and `1.1.1` fixes.
### Changelog notice
* Fix `bdk_electrum` handling of negative spk-history height.
### 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] I've added tests to reproduce the issue which are now passing
~* [ ] This pull request breaks the existing API~
~* [ ] I'm linking the issue being fixed by this PR~
志宇 [Sun, 16 Feb 2025 04:14:41 +0000 (15:14 +1100)]
test(electrum): Chained mempool sync
Is an spk history contains a tx that spends another unconfirmed tx, the
Electrum API will return the tx with a negative height. This should
succeed and not panic.
志宇 [Sat, 15 Feb 2025 14:47:20 +0000 (01:47 +1100)]
fix(electrum): Handle negative heights properly
In electrum, heights returned alongside txs may be 0 or -1. 0 means the
tx is unconfirmed. -1 means the tx is unconfirmed and spends an
unconfirmed tx.
Previously, the codebase assumed that heights cannot be negative and
used a `i32 as usize` cast (which would lead to panic if the i32 is
negative).
* CI complained about docs here. https://github.com/bitcoindevkit/bdk/actions/runs/13420654916/job/37492201157
* CI complained about MSRV not being satisfied. https://github.com/bitcoindevkit/bdk/actions/runs/13420654913/job/37492254119
### 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
As I was developing the changes in #1798 I discover issue #1810. So I introduced the fixes in that PR but later I split them in two to ease the review by suggestion of @oleonardolima .
The `preselect_utxos` method has an off-by-one error that is making the selection of optional UTxOs too restrictive, by
requiring the coinbase outputs to surpass or equal coinbase maturity time at the current height of the selection, and
not in the block in which the transaction may be included in the blockchain.
The changes in this commit fix it by considering the maturity of the coinbase output at the spending height and not
the transaction creation height, this means, a +1 at the considered height at the moment of building the
transaction.
Fixes #1810.
### Notes to the reviewers
Tests for issue #1810 have not been explicitly added, as there already was a `text_spend_coinbase` test which was corrected to ensure coinbase maturation is considered in alignment with the new logic.
Changes are not breaking but I'm modifying slightly the documentation for the public method `TxBuilder::current_height` to adjust to the fixed code. Does this merit an entry in the CHANGELOG?
### Changelog notice
`Wallet` now considers a utxo originated from a coinbase transaction (`coinbase utxo`) as available for selection if it will mature in the next block after the height provided to the selection, the current height by default. The previous behavior allowed selecting a `coinbase utxo` only when the height at the moment of selection was equal to maturity height or greater.
### 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] I've updated existing tests to match the fix
* [x] I've updated docs to match the fix logic
* [x] This pull request DOES NOT break the existing API
* [x] I'm linking the issue being fixed by this PR
nymius [Wed, 12 Feb 2025 19:44:15 +0000 (16:44 -0300)]
fix(wallet): off-by-one error checking coinbase maturity in optional UTxOs
The `preselect_utxos` method has an off-by-one error that is making the
selection of optional UTxOs too restrictive, by requiring the coinbase
outputs to surpass or equal coinbase maturity time at the current height
of the selection, and not in the block in which the transaction may be
included in the blockchain.
The changes in this commit fix it by considering the maturity of the
coinbase output at the spending height and not the transaction creation
height, this means, a +1 at the considered height at the moment of
building the transaction.
### Description
`cc` dependency was unpinned to fix a clash with `cmake` that was causing CI errors.
### Changelog notice
* Unpinned cc dependency.
### 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
Steve Myers [Tue, 4 Feb 2025 17:11:51 +0000 (11:11 -0600)]
Merge bitcoindevkit/bdk#1738: test(core): add unit tests for `merge` trait
fcc0a3409aa390e9f32a3308dd5fe0e2e33b9b44 test: add tests for trait of operations on various data structures, including conflict scenarios and checking for empty structures (pluveto)
Pull request description:
### Description
This update adds unit tests for the `Merge` trait implementations on `BTreeMap`, `BTreeSet`, `Vec`, and tuples. It also includes tests for the `is_empty` and `take` methods to ensure their correct functionality.
### Notes to the Reviewers
- The tests cover basic scenarios for merging collections and checking their emptiness.
- Consider the potential method name conflict with `BTreeSet`'s existing `take` method.
### Changelog Notice
- Added unit tests for merging functionality and utility methods (`is_empty`, `take`) on various collection types.
### 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
I used `zizmor` on all current CI workflows, it's a tool that helps detecting possible vulnerabilities in our CI jobs, see https://woodruffw.github.io/zizmor/.
It can run against most of it's audit rules, however the ones that require the GitHub API Token would require some with access to it in order to test against it. So this PR does not cover for impostor-commit, ref-confusion known-vulnerable-actions audit rules.
<!-- 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
- Do not persist credentials on GitHub Actions.
<!-- 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
Leonardo Lima [Tue, 28 Jan 2025 14:06:55 +0000 (11:06 -0300)]
fix(cont-integration): template injection audit
- fixes the `template_injection` audit failure due to
`matrix.rust.version` usage, use an environement var instead
see: https://woodruffw.github.io/zizmor/audits/#template-injection
The PR adds a `bip158` module to the `bdk_bitcoind_rpc` crate along with a new type `FilterIter` that can be used for retrieving blocks from a full node which contain transactions relevant to a list of script pubkeys.
### Notes to the reviewers
### Changelog notice
`bdk_bitcoind_rpc`: Added `bip158` module as a means of updating `bdk_chain` 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
Previously we failed to remove the change output if the wallet has no internal keychain which caused tx building to fail at the new higher feerate. Fix this by mapping the internal keychain to the de-facto change keychain so that the drain output can be recalculated.
fixes #1807
### Changelog notice
Fixed
- wallet: Fixed an issue preventing `build_fee_bump` from re-targeting the drain value for some wallets
### 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] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
Bumps [Swatinem/rust-cache](https://github.com/swatinem/rust-cache) from 2.7.5 to 2.7.7.
<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.7</h2>
<p><strong>Full Changelog</strong>: <a href="https://github.com/Swatinem/rust-cache/compare/v2.7.6...v2.7.7">https://github.com/Swatinem/rust-cache/compare/v2.7.6...v2.7.7</a></p>
<h2>v2.7.6</h2>
<h2>What's Changed</h2>
<ul>
<li>Updated artifact upload action to v4 by <a href="https://github.com/guylamar2006"><code>@guylamar2006</code></a> in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/212">Swatinem/rust-cache#212</a></li>
<li>Adds an option to do lookup-only of the cache by <a href="https://github.com/danlec"><code>@danlec</code></a> in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/217">Swatinem/rust-cache#217</a></li>
<li>add runner OS in cache key by <a href="https://github.com/rnbguy"><code>@rnbguy</code></a> in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/220">Swatinem/rust-cache#220</a></li>
<li>Allow opting out of caching $CARGO_HOME/bin. by <a href="https://github.com/benjyw"><code>@benjyw</code></a> in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/216">Swatinem/rust-cache#216</a></li>
</ul>
<h2>New Contributors</h2>
<ul>
<li><a href="https://github.com/guylamar2006"><code>@guylamar2006</code></a> made their first contribution in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/212">Swatinem/rust-cache#212</a></li>
<li><a href="https://github.com/danlec"><code>@danlec</code></a> made their first contribution in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/217">Swatinem/rust-cache#217</a></li>
<li><a href="https://github.com/rnbguy"><code>@rnbguy</code></a> made their first contribution in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/220">Swatinem/rust-cache#220</a></li>
<li><a href="https://github.com/benjyw"><code>@benjyw</code></a> made their first contribution in <a href="https://redirect.github.com/Swatinem/rust-cache/pull/216">Swatinem/rust-cache#216</a></li>
</ul>
<p><strong>Full Changelog</strong>: <a href="https://github.com/Swatinem/rust-cache/compare/v2.7.5...v2.7.6">https://github.com/Swatinem/rust-cache/compare/v2.7.5...v2.7.6</a></p>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/Swatinem/rust-cache/commit/f0deed1e0edfc6a9be95417288c0e1099b1eeec3"><code>f0deed1</code></a> 2.7.7</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/008623fb834cadde1d7ccee1a26dc84acb660ec3"><code>008623f</code></a> also cache <code>cargo install</code> metadata</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/720f7e45ccee46c12a7b1d7bed2ab733be9be5a1"><code>720f7e4</code></a> 2.7.6</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/4b1f006ad2112a11d66969e219444096a98af937"><code>4b1f006</code></a> update dependencies, in particular <code>@actions/cache</code></li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/e8e63cdbf2788df3801e6f9a81516b2ca8391886"><code>e8e63cd</code></a> Allow opting out of caching $CARGO_HOME/bin. (<a href="https://redirect.github.com/swatinem/rust-cache/issues/216">#216</a>)</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/9a2e0d32122f6883cb48fad7a1ac5c49f25b7661"><code>9a2e0d3</code></a> add runner OS in cache key (<a href="https://redirect.github.com/swatinem/rust-cache/issues/220">#220</a>)</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/c00f3025caeee0e9c78c18c43de11ab15fd3b486"><code>c00f302</code></a> Adds an option to do lookup-only of the cache (<a href="https://redirect.github.com/swatinem/rust-cache/issues/217">#217</a>)</li>
<li><a href="https://github.com/Swatinem/rust-cache/commit/68b3cb7503c78e67dae8373749990a220eb65352"><code>68b3cb7</code></a> Updated artifact upload action to v4 (<a href="https://redirect.github.com/swatinem/rust-cache/issues/212">#212</a>)</li>
<li>See full diff in <a href="https://github.com/swatinem/rust-cache/compare/v2.7.5...v2.7.7">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)
projects / bdk / log