<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
This PR adds a unit test that checks the satisfaction of timestamp-based timelocks. The goal is to test the absolute time therefore the variable passed to the miniscript fragment `after` has to be equal to or greater that 500_000_000 otherwise it would be checking the block height.
### 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 -->
This unit test tries to check if #642 is still an issue.
### 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 -->
It adds a secondary MSRV version of `1.75.0` for the `bdk_electrum` crate, excluding it from the `1.63.0` MSRV dependency pinning and CI step.
As it's the new CI will run without `bdk_electrum` for `1.63.0` MSRV step, will run another step for `1.75.0` MSRV and the latest stable.
Currently,, the only affected crate is `electrum-client`, therefore `bdk-electrum`, as it relies directly on `rustls` which migrated to `1.71.0` MSRV. The `esplora-client` relies on `rustls` as a dependency of `minreq` or `reqwest`, but those crates didn't upgraded it's `rustls` versions yet.
In a further PR it should also bump the `electrum-client` crate to it's latest version, which relies on `1.75.0` MSRV, see https://github.com/bitcoindevkit/rust-electrum-client/pull/159.
### Notes to the reviewers
<!-- In this section you can include notes directed to the reviewers, like explaining why some parts
of the PR were done in a specific way -->
It's open for discussion if this approach is the expected one, or another one would be better.
### Changelog notice
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
- Add a secondary MSRV of `1.75.0` for `bdk_electrum`.
### 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
Automated update to Github CI workflow `cont_integration.yml` by [create-pull-request](https://github.com/peter-evans/create-pull-request) GitHub action
This PR adds a unit test for checking that the fee calculation of `bdk_electrum` is correct.
> a "preliminary" tx to an address in Core's wallet such that the output created becomes the input of the next tx. This preliminary tx is the prev_tx that needs to be fetched in order to calculate the fee of the tx sent to the receiver. This way is better because the required outpoint is less determined. Then we assert we have the right (outpoint, txout) in the update TxGraph https://github.com/bitcoindevkit/bdk/issues/1444#issuecomment-2112751282
### Notes to the reviewers
fixes: #1444
### 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
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Changes the default version number to version 2 to enchance privacy for the type of wallet used
Fixes #1676
### Changelog notice
- Changed the default transaction version number in the transaction builder.
- Updated the test to test for the default transaction number.
### 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. -->
fix #1711
### Description
This PR replaces how currently potentially bug-prone PSBT's input access is done using a more carefully approach.
### 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 -->
### 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
benalleng [Thu, 2 Jan 2025 17:09:15 +0000 (12:09 -0500)]
fix(tx_builder): change default tx to version 2
This change improves privacy since >85% of transactions on the network are version 2. Version 2 is also necessary to eventually implement BIP 326 nSequence-based anti-fee-sniping.
Opening this up as a record of past decisions. The template is inspired by the one used by uniffi. Whether or not we decide to add these to the repo, it's an opportunity to learn, discuss, and collab when it comes to design and architecture.
The first 2 ADRs pertain to bdk's approach to data persistence. Another recent suggestion was to document how we're thinking about single vs multiple descriptor wallets. Feedback welcome.
Cleanup and remove unused code in `Wallet::create_tx`, this was noticed during review of #1763. See: https://github.com/bitcoindevkit/bdk/pull/1763#discussion_r1876740140
fixes #1710
### Notes to the reviewers
In addition to removing the unneeded assignments to `fee_amount` and `received` I also refactored creation of the change output to be an `if let` instead of `match` statement since it only needs to do something if there is `Excess::Change`.
I should have done this cleanup as part of #1048.
### Changelog notice
None, only internal cleanup.
### 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 cleans up the `test_wallet_transactions_relevant` test based on suggestions that didn't make it into #1779.
### Notes to the reviewers
Also included a small use cleanup in wallet/mod.
### 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
Steve Myers [Thu, 19 Dec 2024 03:55:18 +0000 (21:55 -0600)]
Bump bdk_wallet version to 1.0.0
bdk_core to 0.4.1
bdk_chain to 0.21.1
bdk_bitcoind_rpc to 0.17.1
bdk_electrum to 0.20.1
bdk_esplora to 0.20.1
bdk_file_store to 0.18.1
bdk_testenv to 0.11.1
Currently the behavior of `Wallet::transactions` is not well defined and unintuitive.
The approach taken in this PR is to make `Wallet::transactions` return what most wallets would like the caller to see (i.e. transactions that are part of the canonical history and spend from/to a tracked spk). A.k.a make the method more restrictive.
Documentation is updated to refer the caller to the underlying `bdk_chain` structures for any over usecase.
After #1765 gets merged, the behavior of `Wallet::transactions` will become even more unintuitive. Refer to https://github.com/bitcoindevkit/bdk/pull/1765#pullrequestreview-2503968540.
### Notes to the reviewers
**Why not have multiple methods in `Wallet` that return different sets of transactions?**
I think it's better to only provide common usecase histories from `Wallet` and I can only think of one.
### Changelog notice
* Change `Wallet::transactions` to only include "relevant" transactions.
### 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 manually fixed new clippy warnings.
I also changed CI to:
~~1. not build or run tests or test dependencies when building with MSRV. This reduced the number of dependencies we need to pin for MSRV and fixes 1398.~~ I removed these changes.
2. use the same version or rust as in the `rust-version` file for all jobs instead of just for clippy. I made this change to prevent CI breaks when stable changes before we have a chance to fix the auto created PR that bumps the `rust-version` file.
I noticed these warnings while publishing beta.6 and would like to get them fixed before the final `bdk_wallet` 1.0.0. With default features we should have any build warnings. To reproduce:
* [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
Adds an example on what `used` stands for, and make it explicit that it
has the same behavior as `Wallet::reveal_next_address` in the scenario
where all previously revealed addresses have been used.
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
Is there any other behavior of `next_unused_address` we'd need to make clear through documentation ?
<!-- 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 -->
- Improve the `Wallet::next_unused_address` documentation to better describe expected behavior/usage.
### 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
Leonardo Lima [Wed, 4 Dec 2024 18:33:45 +0000 (15:33 -0300)]
docs(wallet): reword the `next_unused_address` doc
docs(wallet): reword the `next_unused_address` doc
Adds an example on what `used` stands for, and make it explicit that it
has the same behavior as `Wallet::reveal_next_address` in the scenario
where all previously revealed addresses have been used.
Steve Myers [Thu, 12 Dec 2024 00:45:01 +0000 (18:45 -0600)]
Bump bdk_wallet version to 1.0.0-beta.6
bdk_core to 0.4.0
bdk_chain to 0.21.0
bdk_bitcoind_rpc to 0.17.0
bdk_electrum to 0.20.0
bdk_esplora to 0.20.0
bdk_file_store to 0.18.0
bdk_testenv to 0.11.0
refactor(coin_selection)!: use Amount and SignedAmount for API and internally
refactor(chain)!: use Amount for DescriptorExt::dust_value()
Using named types make the API and internal code easier to read and reason about since it makes it clear that the values are bitcoin amounts. Also to create these types the units (ie .from_sat()) must be specified.
### Notes to the reviewers
For coin_selection using Amount and SignedAmount makes internal code safer against overflow errors. In particular because these types will panic if an amount overflow occurs. Using u64/i64 on the other hand can silently rollover. See: https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow
This is a continuation of the work done in #1595. Since this is an API breaking change I would like to include it in the 1.0.0-beta milestone if possible.
### Changelog notice
- Change coin_selection to use Amount instead of u64 for all bitcoin amounts.
- Change DescriptorExt::dust_value() to return an Amount instead of u64.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
Steve Myers [Sun, 8 Dec 2024 01:28:24 +0000 (19:28 -0600)]
refactor(coin_selection)!: use Amount and SignedAmount for API and internally
Using named types make the API and internal code easier to read and reason
about since it makes it clear that the values are bitcoin amounts. Also to
create these types the units (ie .from_sat()) must be specified.
Using Amount and SignedAmount also makes internal code safer against overflow
errors. In particular because these types will panic if an amount overflow
occurs. Using u64/i64 on the otherhand can silently rollover. See:
https://doc.rust-lang.org/book/ch03-02-data-types.html#integer-overflow
Previously, getting the canonical history of transactions/UTXOs required calling `TxGraph::get_chain_position` on each transaction. This was highly inefficient and resulted in an `O(n^2)` algorithm. The situation is especially problematic when we have many unconfirmed conflicts.
This PR introduces an `O(n)` algorithm to determine the canonical set of transactions in `TxGraph`. The algorithm's premise is as follows:
1. If transaction `A` is determined to be canonical, all of `A`'s ancestors must also be canonical.
2. If transaction `B` is determined to be NOT canonical, all of `B`'s descendants must also be NOT canonical.
3. If a transaction is anchored in the best chain, it is canonical.
4. If a transaction conflicts with a canonical transaction, it is NOT canonical.
5. A transaction with a higher last-seen has precedence.
6. Last-seen values are transitive. A transaction's collective last-seen value is the max of it's last-seen value and all of it's descendants.
We maintain two mutually-exclusive `txid` sets: `canoncial` and `not_canonical`.
Imagine a method `mark_canonical(A)` that is based on premise 1 and 2. This method will mark transaction `A` and all of it's ancestors as canonical. For each transaction that is marked canonical, we can iterate all of it's conflicts and mark those as `non_canonical`. If a transaction already exists in `canoncial` or `not_canonical`, we can break early, avoiding duplicate work.
This algorithm iterates transactions in 3 runs.
1. Iterate over all transactions with anchors in descending anchor-height order. For any transaction that has an anchor pointing to the best chain, we call `mark_canonical` on it. We iterate in descending-height order to reduce the number of anchors we need to check against the `ChainOracle` (premise 1). The purpose of this run is to populate `non_canonical` with all transactions that directly conflict with anchored transactions and populate `canonical` with all anchored transactions and ancestors of anchors transactions (transitive anchors).
2. Iterate over all transactions with last-seen values, in descending last-seen order. We can call `mark_canonical` on all of these that do not already exist in `canonical` or `not_canonical`.
3. Iterate over remaining transactions that contains anchors (but not in the best chain) and have no last-seen value. We treat these transactions in the same way as we do in run 2.
```
many_conflicting_unconfirmed::list_canonical_txs
time: [709.46 us 710.36 us 711.35 us]
many_conflicting_unconfirmed::filter_chain_txouts
time: [712.59 us 713.23 us 713.90 us]
many_conflicting_unconfirmed::filter_chain_unspents
time: [709.95 us 711.16 us 712.45 us]
many_chained_unconfirmed::list_canonical_txs
time: [2.2604 ms 2.2641 ms 2.2680 ms]
many_chained_unconfirmed::filter_chain_txouts
time: [3.5763 ms 3.5869 ms 3.5979 ms]
many_chained_unconfirmed::filter_chain_unspents
time: [3.5540 ms 3.5596 ms 3.5652 ms]
nested_conflicts_unconfirmed::list_canonical_txs
time: [660.06 us 661.75 us 663.60 us]
nested_conflicts_unconfirmed::filter_chain_txouts
time: [650.15 us 651.36 us 652.71 us]
nested_conflicts_unconfirmed::filter_chain_unspents
time: [658.37 us 661.54 us 664.81 us]
```
```
many_conflicting_unconfirmed::list_canonical_txs
time: [94.618 ms 94.966 ms 95.338 ms]
many_conflicting_unconfirmed::filter_chain_txouts
time: [159.31 ms 159.76 ms 160.22 ms]
many_conflicting_unconfirmed::filter_chain_unspents
time: [163.29 ms 163.61 ms 163.96 ms]
# I gave up running the rest of the benchmarks since they were taking too long.
```
### Notes to the reviewers
* ***PLEASE MERGE #1733 BEFORE THIS PR!*** We had to change the signature of `ChainPosition` to account for transitive anchors and unconfirmed transactions with no `last-seen` value.
* The canonicalization algorithm is contained in `/crates/chain/src/canonical_iter.rs`.
* Since the algorithm requires traversing transactions ordered by anchor height, and then last-seen values, we introduce two index fields in `TxGraph`; `txs_by_anchor` and `txs_by_last_seen`. Methods `insert_anchor` and `insert_seen_at` are changed to populate these index fields.
* An ADR is added: `docs/adr/0003_canonicalization_algorithm.md`. This is based on the work in #1592.
### Changelog notice
* Added: Introduce an `O(n)` canonicalization algorithm. This logic is contained in `/crates/chain/src/canonical_iter.rs`.
* Added: Indexing fields in `TxGraph`; `txs_by_anchor_height` and `txs_by_last_seen`. Pre-indexing allows us to construct the canonical history more efficiently.
* Removed: `TxGraph` methods: `try_get_chain_position` and `get_chain_position`. This is superseded by the new canonicalization algorithm.
### 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
Jiri Jakes [Thu, 5 Dec 2024 08:51:37 +0000 (16:51 +0800)]
refactor(wallet): Reuse chain position instead of obtaining new one
In `Wallet::preselect_utxos()`, the code used to obtain chain position
of the UTXO's transaction from the graph, however the chain position
is already recorded within the UTXO's representation (`LocalOutput`).
This patch reuses the existing chain position instead of obtaining a
fresh one.
perf(chain): add benchmarks for canonicalization logic
This is mostly taken from #1735 except we inline many of the functions
and test `list_canonical_txs`, `filter_chain_unspents` and
`filter_chain_txouts` on all scenarios.
This is an O(n) algorithm to determine the canonical set of txids.
* Run 1: Iterate txs with anchors, starting from highest anchor height
txs.
* Run 2: Iterate txs with last-seen values, starting from highest
last-seen values.
* Run 3: Iterate txs that are remaining from run 1 which are not
anchored in the best chain.
Since all transitively-anchored txs are added to the `canonical` set in
run 1, and anything that conflicts to anchored txs are already added to
`not_canonial`, we can guarantee that run 2 will not traverse anything
that directly or indirectly conflicts anything that is anchored.
Run 3 is needed in case a tx does not have a last-seen value, but is
seen in a conflicting chain.
`TxGraph` is updated to include indexes `txids_by_anchor_height` and
`txids_by_last_seen`. These are populated by the `insert_anchor` and
`insert_seen_at` methods. Generic constaints needed to be tightened as
these methods need to be aware of the anchor height to create
`LastSeenIn`.
志宇 [Wed, 16 Oct 2024 14:28:23 +0000 (14:28 +0000)]
feat(chain)!: `TxGraph` contain anchors in one field
Previously, the field `TxGraph::anchors` existed because we assumed
there was use in having a partially-chronological order of transactions
there. Turns out it wasn't used at all.
This commit removes anchors from the `txs` field and reworks `anchors`
field to be a map of `txid -> set<anchors>`.
This is a breaking change since the signature of `all_anchors()` is
changed.
Update `TxGraph` field docs for `empty_outspends` and `empty_anchors`.
As expressed by @LLFourn _We want to not depend on serde_json. If we keep it around for serializing anchors we won't be able to remove it in the future because it will always be needed to do migrations_
Currently there is only one widely used anchor, `ConfirmationBlockTime`.
The decision was to constrain support to just be for a single anchor type ConfirmationBlockTime. The anchor table will represent all fields of `ConfirmationBlockTime`, each one in its own column.
The reasons:
- No one is using rusqlite with any other anchor type, and if they are, they can do something custom anyway.
- The anchor representation may change in the future, supporting for multiple Anchor types here will cause more problems for migration later on.
Resolves #1695
### Notes to the reviewers
<details>
<summary>Why the type of the confirmation_time column is INTEGER?</summary>
By [sqlite3 docs](https://www.sqlite.org/datatype3.html):
> Each value stored in an SQLite database (or manipulated by the database engine) has one of the following storage classes:
> ...
> INTEGER. The value is a *signed integer*, stored in 0, 1, 2, 3, 4, 6, or 8 bytes depending on the magnitude of the value.
(Remember confirmation time is `u64`)
> REAL. The value is a floating point value, stored as an 8-byte IEEE floating point number.
> ...
> SQLite uses a more general dynamic type system. In SQLite, the datatype of a value is associated with the value itself, not with its container.
> ...
> In order to maximize compatibility between SQLite and other database engines, ..., SQLite supports the concept of "type affinity" on columns. The type affinity of a column is the recommended type for data stored in that column. The important idea here is that the type is recommended, not required. Any column can still store any type of data. It is just that some columns, given the choice, will prefer to use one storage class over another. The preferred storage class for a column is called its "affinity".
> ...
> A column with NUMERIC affinity may contain values using all five storage classes. When text data is inserted into a NUMERIC column, **the storage class of the text is converted to INTEGER or REAL (in order of preference)** if the text is a well-formed integer or real literal, respectively.
> A column that uses INTEGER affinity behaves the same as a column with NUMERIC affinity.
But anchor table was defined using the STRICT keyword, again, by sqlite docs:
> ... The STRICT keyword causes the following differences:
> ...
> SQLite attempts to coerce the data into the appropriate type using the usual affinity rules, as PostgreSQL, MySQL, SQL Server, and Oracle all do. *If the value cannot be losslessly converted* in the specified datatype, then an SQLITE_CONSTRAINT_DATATYPE error is raised.
So, the *TLDR*, with some help of this [blog post](https://jakegoulding.com/blog/2011/02/06/sqlite-64-bit-integers/) is:
- SQLite INTEGER supported values range from `-2**63 to (2**63-1)`
- If the number is bigger it will treat the number as REAL.
- As the SQLite table is defined with the STRICT keyword, it should enforce a losslessly conversion or fail with SQLITE_CONSTRAINT_DATATYPE.
</details>
<details>
<summary>Why not setting confirmation_time as BLOB or NUMERIC?</summary>
I don't have a strong opinion on this. INTEGER was the first numeric type I found, then later I found NUMERIC and they seemed to behave in the same way, so I didn't change the type.
I discarded BLOB and ANY first because they didn't provide a clear idea of what was being stored in the column, but in retrospective they seem to remove all the considerations that I had to do above, so maybe they are better fitted for the type.
</details>
<details>
<summary>Why adding a default value to confirmation_time column if the anchor column constraint is to be NOT NULL so all copied values will be filled?</summary>
`confirmation_time` should have the same constraint as `anchor`, to be `NOT NULL`, but as UPDATE statements might be executed in already filled tables, you must provide a default value for all the rows you are going to add the new column to. As the `confirmation_time` extraction of the json blob in anchor cannot be performed in the same step, I had to add this default value.
This is flexibilizing the schema of the tables and extending the bug surface it may have, but I'm assuming the application layer will enforce the addition of a valid `confirmation_time` always.
</details>
<details>
<summary>Why the default value of confirmation_time column is -1?</summary>
Considering the other alternatives were to use the max value, min value or zero and confirmation time should always be positive, I considered `-1` just to be computer and human readable enough to perceive there must be something wrong if the `ConfirmationBlockTime` retrieved by the load of the wallet has this value set as the confirmation time.
</details>
<details>
<summary>Why to not be STRICT with each statement?</summary>
It is a constraint only applicable to tables on creation.
</details>
<details>
<summary>Why not creating a whole new table without anchor column and with the confirmation_time column, copy the content from one to the other and drop the former table?</summary>
Computation cost. I didn't benchmark it, and I don't know how efficient is SQLite engine under the hood, but at first sight it seems copying a single column is better than copying four.
</details>
<!-- 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
- Reduce `rusqlite` implementation only to `ChangeSet<ConfirmationBlockTime>`.
- Replace `anchor` column in sqlite by `confirmation_time`.
- Modify `migrate_schema` `versioned_script` parameter's type to `&[&str]`.
- Transformed existing schemas in methods to allow their individual application.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [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
`fetch_prev_txout` should not try to query the prevouts of coinbase transactions, as it may result in the Electrum server returning an error which would cause the overall `sync`/`full_scan` to fail. Without this critical bug fix, `bdk_electrum` will crash when someone tracks an address being mined to.
### Notes to the reviewers
### Changelog notice
* `fetch_prev_txout` no longer queries coinbase transactions.
### 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
Wei Chen [Wed, 6 Nov 2024 16:33:51 +0000 (00:33 +0800)]
fix(electrum): prevent `fetch_prev_txout` from querying coinbase transactions
\`fetch_prev_txout\` should not try to query the prevouts of coinbase
transactions, as it may result in the Electrum server returning an error
which would cause the overall `sync` to fail.
Update `bdk_esplora` to depend on esplora-client 0.11.0
### Notes to the reviewers
bitcoindevkit/rust-esplora-client#103 added a generic type parameter to `AsyncClient` representing a user-defined `Sleeper` and that change is reflected here in order to call the underlying API methods. We also add a new build feature "tokio" that enables the corresponding feature in rust-esplora-client.
closes #1742
### Changelog notice
`bdk_esplora`: Bump `esplora-client` to 0.11.0
### 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] This pull request breaks the existing API
* [x] I'm linking the issue being fixed by this PR
nymius [Wed, 4 Dec 2024 15:36:57 +0000 (12:36 -0300)]
test(chain): add compatibility test for v0 to v1 sqlite schema migration
Why just v0 to v1 test and not a general backward compatibility test?
Is harder to craft a general compatibility test without prior knowledge
of how future schemas would look like. Also, the creation of a backward
compatibility test for each new schema change will allow the execution
of incremental backward compatibility tests with better granularity.
nymius [Mon, 2 Dec 2024 15:26:55 +0000 (12:26 -0300)]
refactor(chain)!: move schemas to their own methods
We would like to test backward compatibility of new schemas.
To do so, we should be able to apply schemas independently.
Why to change `rusqlite::execute` by `rusqlite::execute_batch`?
- we are going to need to return the statements of the schemas as
Strings, because we are now returning them from methods, it seemed
redundant to keep getting references to something is already
referencing data, i.e., keep moving around &String instead of String
(consider `rusqlite::execute_batch` takes multiple statements as a
single String)
- we were calling `rusqlite::execute` with empty params, so we weren't
trapped by the method's signature
- `rustqlite::execute_batch` does the same than we were doing applying
statements secuentially in a loop
- the code doesn't lose error expresivity: `rusqlite::execute_batch` is
going to fail with the same errors `rusqlite::execute` does
BREAKING CHANGE: changes public method `migrate_schema` signature.
nymius [Thu, 28 Nov 2024 23:58:36 +0000 (20:58 -0300)]
refactor(chain)!: remove AnchorImpl wrapper for Anchor implementors
AnchorImpl was a wrapper created to allow the implementation of foreign
traits, like From/ToJson from serde_json for external unknown structs
implementing the Anchor trait.
As the Anchor generic in the rusqlite implementation for anchored
ChangeSets was the only place where this AnchorImpl was used and it has
been fixed to the anchor ConfirmationBlockTime, there is no more reason
to keep this wrapper around.
nymius [Wed, 27 Nov 2024 01:05:08 +0000 (22:05 -0300)]
refactor(chain)!: impl sqlite for ConfirmationBlockTime anchored changesets
We want to not depend on serde_json. If we keep it around for
serializing anchors we won't be able to remove it in the future because
it will always be needed to do migrations.
Currently there is only one widely used anchor, ConfirmationBlockTime.
The desicion was to constrain support to just be for a single anchor
type ConfirmationBlockTime. The anchor table will represent all fields
of ConfirmationBlockTime, each one in its own column.
The reasons:
- No one is using rusqlite with any other anchor type, and if they are,
they can do something custom anyway.
- The anchor representation may change in the future, supporting for
multiple Anchor types here will cause more problems for migration
later on.
I had to remove the `Clone` trait on `TxBuilder` but it was only being used in tests.
### Changelog notice
- TxBuilder is now Send safe and does not implement the Clone trait
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [ ] I've added docs for the new feature
Updates the `electrum-client` dependency to 0.22.0 for `bdk_electrum`.
### Notes to the reviewers
### Changelog notice
* Updated `electrum-client` dependency to 0.22.0.
### 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
Change `ChainPosition` to be able to represent transitive anchors and unconfirm-without-last-seen values.
As mentioned in https://github.com/bitcoindevkit/bdk/pull/1670#issuecomment-2484535021, we want this merged first so that we have minimal changes to the API after 1670 is merged.
### Changelog notice
* Change `ChainPosition` so that it is able to represent transitive anchors and unconfirmed-without-last-seen values.
### 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
The recent release of `rustls` (0.23.19) reverts it's MSRV to 1.63 so the pin is no longer necessary.
### Notes to the reviewers
Some context:
* https://github.com/bitcoindevkit/rust-electrum-client/pull/158
* https://github.com/rustls/rustls/pull/2244
### Changelog notice
* Revert MSRV pin of `rustls`.
### 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
Previously, we may error when we insert an anchor where the txid being anchored has no corresponding tx.
closes #1712
replaces #961
### 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 -->
### 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
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
<!-- 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 -->
`rustls` version 0.23.18 raised msrv to 1.71.
Version 0.23.17 was pinned to 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 rustls 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
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
As mentioned in issue #1689 the `debug_assert!` is being used on other `LocalChain` methods, such as: `from_changeset`, and `apply_changeset` but it was missing on `apply_update`.
<!-- 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
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
- Add usage of `debug_assert!()` to `LocalChain::apply_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
* [ ] 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
Bump [`hashbrown`](https://crates.io/crates/hashbrown) to v0.15
I ran [`cargo build-all-features` and `cargo test-all-features`](https://github.com/frewsxcv/cargo-all-features) locally for the `core` crate with no issues. Here's the output for the former:
```
cargo build-all-features
Building crate=bdk_core features=[]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[hashbrown]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[serde]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.05s
Building crate=bdk_core features=[std]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[hashbrown,serde]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[hashbrown,std]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[serde,std]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
Building crate=bdk_core features=[hashbrown,serde,std]
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s
```
The relevant changelog can be found between [here](https://github.com/rust-lang/hashbrown/blob/master/CHANGELOG.md#v0100---2021-01-16) and [here](https://github.com/rust-lang/hashbrown/blob/master/CHANGELOG.md#v0151---2024-11-03)
The only notable breaking change I found was that the MSRV was bumped to v1.63. However, this is the same MSRV as we already use, so it's not a breaking change for us.
Any implementation overflowing the `i64` type would be clearly faulty, so I just pass up the error if the conversion fails.
### 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
projects / bdk / log