3451d1c12e Fix docs.rs features (Alekos Filini)
Pull request description:
### Description
Fix docs.rs features
### 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
* [ ] I've updated `CHANGELOG.md`
ACKs for top commit:
danielabrozzoni:
utACK 3451d1c12e
notmandatory:
ACK 3451d1c12e
Tree-SHA512: 96dc4f816b21cf20fc2828dcfd56865f3f9add8e4aa643205879c810d09e6f2e73d6643061bf3ca98145b37e726907dedd31a145f92d6646c172a43ae2285aa8
7c57965999 Bump version before making release branch, separate patch_release template (Steve Myers)
3d69f1c291 Update DEVELOPMENT_CYCLE.md to work with [patch.crates-io] (Steve Myers)
Pull request description:
### Description
Update DEVELOPMENT_CYCLE and release instructions to make [overriding dependencies] possible for downstream projects with unreleased `bdk` versions for development and testing. Also simplifies the release process by capturing changelog information in the `pull_request_template` and recording release changelog information in the release tag message instead of in a `CHANGELOG.md` file which causes too many merge conflicts and complicates the release process.
Fixes#536Fixes#496
### Notes to the reviewers
The primary changes to our current release process are:
1. Don't add `-dev` or `-rc.x` to unreleased `bdk` cargo versions because those extensions do not work with [overriding dependencies].
2. Increment the `master` branch version as soon as a `release/MAJOR.MINOR` branch is created, the next release `release/MAJOR.MINOR` branch version with be **MAJOR.MINOR.PATCH**, and the `master` branch development version will be **MAJOR.MINOR+1.0**; either version can be used with [overriding dependencies].
4. Remove the `bdk` version from the `src/lib.rs` file so that it doesn't need to be changed on every release, because it isn't needed in the rust docs for most developers and removing it will help simplify the release process.
5. The new release process is now documented as a checklist in a new `release.md` github issue template.
6. Putting changelog information in the release tag message is how the tokio project does it. ~~After this PR is merged I will replace old tags with new ones containing changelog information and then do a new PR to remove the CHANGELOG.md file.~~ After this PR is merged I don't think we need to update old tags, only rename the CHANGELOG.md file to CHANGELOG-OLD.md with a note to check tags for future change log info.
[overriding dependencies]: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK 7c57965999
Tree-SHA512: 818e2f9bc7a629cbbb190a83b9743e8f4de49a4093beae83ed0b9c506f33e6f96b2c1e376f788536d58c46908d278bde08140f43a625515401ea1f9efdb9153f
138acc3b7d Change `populate_test_db` to not return empty input (wszdexdrf)
d6e1dd1040 Change CI to add test using ledger emulator (wszdexdrf)
76034772cb Add a custom signer for hardware wallets (wszdexdrf)
Pull request description:
Also adds a new test in CI for building and testing on a virtual
hardware wallet.
### Description
This PR would enable BDK users to sign transactions using a hardware wallet. It is just the beginning hence there are no complex features, but I hope not for long.
I have added a test in CI for building a ledger emulator and running the new test on it. The test is similar to the one on bitcoindevkit/rust-hwi.
### Notes to the reviewers
The PR is incomplete (and wouldn't work, as the rust-hwi in `cargo.toml` is pointing to a local crate, temporarily) as a small change is required in rust-hwi (https://github.com/bitcoindevkit/rust-hwi/pull/42).
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 138acc3b7d
Tree-SHA512: 54337f06247829242b4dc60f733346173d957de8e9f8b80beb91304d679cfb4e0e4db722c967469265a5b6ede2bd641ba5c089760391c671995dc30de37897de
08668ac462 Set the db sync height (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Fixes#719
Previously we weren't setting the db sync height in populate_test_db
macro even when current height is provided.. This creates a bug that
get_funded_wallet will return 0 balance.
This PR fixes the populate_test_db macro and updates tests which were
previously dependent on the unsynced wallet behavior.
### 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 -->
### 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
ACKs for top commit:
afilini:
ACK 08668ac462
Tree-SHA512: 1dcc968e4b3551e916b450c5ff2fab6636083f104cc982eb3f7602c624382434e0170d9f0c0a356e6c9c5f834eebe5cb1365b37ef73d7b4ef15d652a364dc2ab
7b1ad1b629 Verify signatures after signing (Scott Robinson)
Pull request description:
### Description
Verify signatures after signing
As per [BIP-340, footnote 14][fn]:
> Verifying the signature before leaving the signer prevents random or
> attacker provoked computation errors. This prevents publishing invalid
> signatures which may leak information about the secret key. It is
> recommended, but can be omitted if the computation cost is prohibitive.
[fn]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki#cite_note-14
### Notes to the reviewers
How do we test this?
### Checklists
#### All Submissions:
* [ ] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
re-ACK 7b1ad1b629
Tree-SHA512: 7319db1f8cec2fcfe4ac443ab5728893f9fb6133b33331b35ec6910662c45de8a7cdcf80ac1f3bb435815e914ccf639682a5c07ff0baef42605bf044a34a8232
baf7eaace6 Implement Deref<Target=UrlClient> for EsploraBlockchain (Vladimir Fomene)
Pull request description:
### Description
There is currently no way to access the client from the EsploraBlockchain. This makes it difficult for users to extend it's functionality. This PR exposes both the reqwest and ureq clients. This PR is related to PR #705.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
rajarshimaitra:
tACK baf7eaace6
Tree-SHA512: e2f530058c88e06fc2972edfcd2df1b534d43b0214d710b62e4d5200ac0e38dad6a9f8db1e0c7a7ed19892e59411dcc07f3f6dc8ad58afae9d677169ca98bb38
c5952dd09a Implement `Deref<Target=Client>` for `ElectrumBlockchain` (Alekos Filini)
Pull request description:
### Description
As pointed out in https://github.com/bitcoindevkit/rust-electrum-client/pull/58#issuecomment-1207890096 there was no way to keep using the client once it was given to BDK.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
rajarshimaitra:
ACK c5952dd09a
Tree-SHA512: fbfbada51c9426266c8960da5508ee07b196808f0d670a09a51962bd6eda9ccf585e209f5b99b5ab78a3d17af774bdb3e33ef36ac4f4d1ce7f2c3398ae4f6d0c
a8f9f6c43a RpcBlockchain derefs to the underlying RPC Client (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
For the same reason as #705 and #722 ..
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK a8f9f6c43a
Tree-SHA512: 81e596fe451c275ca0ce27ee7ac9cf7e88433775603021c2dd1cd26a26558531cf74f81ef05d0ae9d5d0e59e91196e3ac6d38c0f4853b1889ddf822d8e63e178
cd078903a7 Fix P2WPKH_SATISFACTION_SIZE in CS tests (Daniela Brozzoni)
Pull request description:
Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8f99 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.
### Description
### Notes to the reviewers
### 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
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
csralvall:
utACK cd078903a7
afilini:
ACK cd078903a7
Tree-SHA512: 3e39735505e411392cf01885b5920443d23fa21d9c20cc7c8fdeaa2698df8bc2da86241b6c20f5e3f5941fe1a0aebe8f957d8145d4f9e7ad3f213e4658d6ea68
8026bd9476 Bump version to 0.21.1-dev (Alekos Filini)
e2bd96012a Bump version to 0.21.0 (Alekos Filini)
2c01b6118f Bump version to 0.21.0-rc.1 (Alekos Filini)
Pull request description:
### Description
Merge the release branch back into master
### 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
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
ACK 8026bd9476
Tree-SHA512: a2a924a60d551a823de035b609d4d51652a165a0695212af76dea87706919c8929dba977bb297f4787708470bf075d14dd0a37657bd3a76e7d44a746fb5439df
134b19a9cb Fix minor typos in docs (thunderbiscuit)
Pull request description:
### Description
This PR fixes:
1. The use of "i.e." in docs, sometimes spelled as "ie."
2. A small typo in the sentence "Note that this methods only operate on the internal database..."
3. A small typo in the sentence "Finish the building the transaction"
I came across these while building docs for bdk-kotlin.
### Notes to the reviewers
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
danielabrozzoni:
ACK 134b19a9cb
Tree-SHA512: 67296999eba8ffe1fe64756fa023d85064774cb9d4c26e99054d467b5024baea4138f11d602d04e695412c61625ee4f5b4687b75f177cfec2604a6c61a5a6216
74e2c477f1 Replace `rpc::CoreTxIter` with `list_transactions` fn. (志宇)
Pull request description:
### Description
This fixes a bug where `CoreTxIter` attempts to call `listtransactions` immediately after a tx result is filtered (instead of being returned), when in fact, the correct logic will be to pop another tx result.
The new logic also ensures that tx results are returned in chonological order. The test `test_list_transactions` verifies this. We also now ensure that `page_size` is between the range `[0 to 1000]` otherwise an error is returned.
Some needless cloning is removed from `from_config` as well as logging improvements.
### Notes to the reviewers
This is an oversight by me (sorry) for PR #683
### 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
ACKs for top commit:
afilini:
ACK 74e2c477f1
Tree-SHA512: f32314a9947067673d19d95da8cde36b350c0bb0ebe0924405ad50602c14590f7ccb09a3e03cdfdd227f938dccd0f556f3a2b4dd7fdd6eba1591c0f8d3e65182
0f03831274 Change get_balance to return in categories. (wszdexdrf)
Pull request description:
### Description
This changes `get_balance()` function so that it returns balance separated in 4 categories:
- available
- trusted-pending
- untrusted-pending
- immature
Fixes#238
### Notes to the reviewers
Based on #614
### 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 updated tests for the new feature
* [x] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 0f03831274
Tree-SHA512: 39f02c22c61b6c73dd8e6d27b1775a72e64ab773ee67c0ad00e817e555c52cdf648f482ca8be5fcc2f3d62134c35b720b1e61b311cb6debb3ad651e79c829b93
5eeba6cced Various `RpcBlockchain` improvements (志宇)
5eb74af414 Rpc: Manually add immature coinbase utxos (志宇)
ac19c19f21 New `RpcBlockchain` implementation with various fixes (志宇)
Pull request description:
Fixes#677
### Description
Unfortunately to fix all the problems, I had to do a complete re-implementation of `RpcBlockchain`.
**The new implementation fixes the following:**
* We can track more than 100 scriptPubKeys
* We can obtain more than 1000 transactions per sync
* Transaction "metadata" for already-syned transactions are updated when we introduce new scriptPubKeys
**`RpcConfig` changes:**
* Introduce `RpcSyncParams`.
* Remove `RpcConfig::skip_blocks` (this is replaced by `RpcSyncParams::start_time`).
### Notes to the reviewers
* The `RpcConfig` structure is changed. It will be good to confirm whether this is an okay change.
### 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
* [x] I've updated `CHANGELOG.md`
#### 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
ACKs for top commit:
afilini:
ACK 5eeba6cced
Tree-SHA512: 7e0c9cfc4ef10fb07e4ac7f6fbf30cf28ca6395495c0237fa5bfa9a2fcbbd4d8ff980ffcf71ddd10bc052e4c07bc2c27f093dd3cd1c69cb29141455c693f2386
We were wrongly considering the sum of "effective value" (i.e. value -
fee cost) when reporting an early "insufficient funds" error in the
branch and bound coin selection.
This commit fixes essentially two issues:
- Very high fee rates could cause a panic during the i64 -> u64
conversion because we assumed the sum of effective values would never
be negative
- Since we were comparing the sum of effective values of *all* the UTXOs
(even the optional UTXOs with negative effective value) with the target
we'd like to reach, we could in some cases error and tell the user we
don't have enough funds, while in fact we do! Since we are not required
to spend any of the optional UTXOs, so we could just ignore the ones
that *cost us* money to spend and excluding them could potentially
allow us to reach the target.
There's a third issue that was present before and remains even with this
fix: when we report the "available" funds in the error, we are ignoring
UTXOs with negative effective value, so it may look like there are less
funds in the wallet than there actually are.
I don't know how to convey the right message the user: if we actually
consider them we just make the "needed" value larger and larger (which
may be confusing, because if the user asks BDK to send 10k satoshis, why
do we say that we actually need 100k?), while if we don't we could report
an incorrect "available" value.
e8df3d2d91 Consolidate `fee_amount` and `amount_needed` (Cesar Alvarez Vallero)
Pull request description:
### Description
Before this commit `fee_amount` and `amount_needed` were passed as independent
parameters. From the perspective of coin selection algorithms, they are always
used jointly for the same purpose, to create a coin selection with a total
effective value greater than it's summed values.
This commit removes the abstraction that the use of the two parameter
introduced by consolidating both into a single parameter, `target_amount`, who
carries their values added up.
Resolves: #641
### Notes to the reviewers
I just updated old tests and didn't create new ones because almost all changes
are renames and "logic changes" (like the addition of the selection fee) are
tested in the modified tests.
### 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
* [x] I've updated `CHANGELOG.md`
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
re-ACK e8df3d2d91 - I tested with the fuzzer, run it for 13,000,000 iterations, couldn't find any crash :)
Tree-SHA512: 64b46473799352c06cc554659e4b159a33812b3d3793c9d436bd1e46b65edd085d71b219f6a0474f6836979ca608aa019a72bdc6915a2cc2d744a76e2a28b889
a63c51f35d Add `electrsd/bitcoind_22_0` to `example\rpcwallet` target (w0xlt)
Pull request description:
On master branch, `example\rpcwallet` fails.
```
$ cargo run --features="keys-bip39 key-value-db rpc" --example rpcwallet
Compiling bitcoin_hashes v0.9.7
Compiling bip39 v1.0.1
Compiling bdk v0.20.1-dev (/home/node01/Dev/wbdk)
Finished dev [unoptimized + debuginfo] target(s) in 19.64s
Running `target/debug/examples/rpcwallet`
>> Setting up bitcoind
thread 'main' panicked at 'We should always have downloaded path: Called a method requiring a feature to be set, but it's not', examples/rpcwallet.rs:56:51
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
```
This PR adds `electrsd/bitcoind_22_0` to `required-features`, making clear that this lib is needed to run this example..
```
$ cargo run --features="keys-bip39 key-value-db rpc electrsd/bitcoind_22_0" --example rpcwallet
Blocking waiting for file lock on package cache
Compiling electrsd v0.19.1
Compiling bdk v0.20.1-dev (/home/node01/Dev/wbdk)
Finished dev [unoptimized + debuginfo] target(s) in 10.27s
Running `target/debug/examples/rpcwallet`
>> Setting up bitcoind
>> bitcoind setup complete
Available coins in Core wallet : 50.00000000 BTC
>> Setting up BDK wallet
>> BDK wallet setup complete.
Available initial coins in BDK wallet : 0 sats
>> Sending coins: Core --> BDK, 10 BTC
>> Received coins in BDK wallet
Available balance in BDK wallet: 1000000000 sats
>> Sending coins: BDK --> Core, 5 BTC
>> Coins sent to Core wallet
Remaining BDK wallet balance: 499999859 sats
Congrats!! you made your first test transaction with bdk and bitcoin core.
```
ACKs for top commit:
afilini:
reACK a63c51f35d
Tree-SHA512: ef13d5e001121c8b1ff6436f9e95b656737bee6692e9b18c4012846a2d2e9e9ad7e6b5cd87cebf4a873335e92a524694e684567a1268f5f0705156659fd9a916
419dc248b6 test: Document `test_bump_fee_add_input_change_dust` (Daniela Brozzoni)
632dabaa07 test: Check tx feerate with longer signatures (Daniela Brozzoni)
2756411ef7 test: Reproduce #660 conditions (Daniela Brozzoni)
50af51da5a test: Fix P2WPKH_FAKE_WITNESS_SIZE (Daniela Brozzoni)
ae919061e2 Take into account the segwit tx header when... ...selecting coins (Daniela Brozzoni)
7ac87b8f99 TXIN_BASE_WEIGHT shouldn't include the script len (Daniela Brozzoni)
ac051d7ae9 Calculate fee amount after output addition (Daniela Brozzoni)
00d426b885 test: Check that the feerate is never below... ...the requested one in assert_fee_rate (Daniela Brozzoni)
42fde6d457 test: Check fee_amount in assert_fee_rate (Daniela Brozzoni)
Pull request description:
### Description
This PR mainly fixes two bugs:
1. TXIN_BASE_WEIGHT wrongly included the `script_len` (Fixes#160)
2. We wouldn't take into account the segwit header in the fee calculation, which could have resulted in a transaction with a lower feerate than the requested one
3. In tests we used to push 108 bytes on the witness as a fake signature, but we should have pushed 106 instead
I also add a test to reproduce the conditions of #660, to check if it's solved. Turns out it's been solved already in #630, but if you're curious about what the bug was, here it is: https://github.com/bitcoindevkit/bdk/issues/660#issuecomment-1196436776
### 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
ACKs for top commit:
afilini:
ACK 419dc248b6
Tree-SHA512: c7b55342eac440a3607a16b94560cb9c08c4805c853432adfda8e21c5177f85d5a8afe0e7e61140e92c8f10934332459c6234fc5f1509ea699d97b1d04f030c6
a713a5a062 Better customize signing in taproot transactions (Daniela Brozzoni)
Pull request description:
Fixes#616
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK a713a5a062
Tree-SHA512: 1100d43cb394b429450fc34f49dd815a024701987c0e6dd163865bd5c4c6f7102127b1ea6e10ced5fdb319874be97baeeb0deea66b4138410871a1d68b4def10
235011feef Add assertions in the FeeRate constructor (Alekos Filini)
Pull request description:
### Description
Disallow negative, NaN, infinite or subnormal fee rate values.
### Notes to the reviewers
This commit is technically an API break because it makes the `FeeRate::from_sat_per_vb` function non-const. I think it's worth it compared to the risk of having completely nonsensical fee rates (that can break the coin selection in interesting ways).
EDIT: it's also a breaking change because our code can now panic in scenarios where it didn't before. Again, I think it's worth it.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
re-ACK 235011feef
Tree-SHA512: c9432956162fadfd255edf20b825635a487adb29c88d791e18f170da79a2aac6f8e745b5e5be09be3c211697d0b1f4bddc1da75c181e8f9fc4fddf566a7a3e5c
32ae95f463 Move change calculus to coin_select (Cesar Alvarez Vallero)
Pull request description:
### Description
The former way to compute and create change was inside `create_tx`, just after
performing coin selection.
It blocked the opportunity to have an "ensemble" algorithm to decide between
multiple coin selection algorithms based on a metric, like Waste.
Now, change is not created inside `coin_select` but the change amount and the
possibility to create change is decided inside the `coin_select` method. In
this way, change is associated with the coin selection algorithm that generated
it, and a method to decide between them can be implemented.
Fixes#147.
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
<!-- 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 -->
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 32ae95f463
Tree-SHA512: 350adb86538949ff50f41151fc46c8d28d9f5fd659e9869882cc3cb30128d76d4b479512c74c721f8beebfdb5423363ad63368e30556efe65ced2b8c52c34ef6
5c940c33cb Fix wallet sync not finding coins of addresses which are not cached (志宇)
Pull request description:
Fixes#521Fixes#451
^ However, only for electrum-based `Blockchain` implementations. For RPC and Compact Block Filters, syncing works differently, and so are the bugs - I have created a separate ticket for this (#677).
### Description
Previously, electrum-based blockchain implementations only synced for `scriptPubKey`s that are already cached in `Database`.
This PR introduces a feedback mechanism, that uses `stop_gap` and the difference between "current index" and "last active index" to determine whether we need to cache more `scriptPubKeys`.
The `WalletSync::wallet_setup` trait now may return an `Error::MissingCachedScripts` error which contains the number of extra `scriptPubKey`s to cache, in order to satisfy `stop_gap` for the next call.
`Wallet::sync` now calls `WalletSync` in a loop, caching in-between subsequent calls (if needed).
#### Notes to reviewers
1. The caveat to this solution is that it is not the most efficient. Every call to `WalletSync::wallet_setup` starts polling the Electrum-based server for `scriptPubKey`s starting from index 0.
However, I feel like this solution is the least "destructive" to the API of `Blockchain`. Also, once the `bdk_core` sync logic is integration, we can select specific ranges of `scriptPubKey`s to sync.
2. Also note that this PR only fixes the issue for electrum-based `Blockchain` implementations (currently `blockchain::electrum` and `blockchain::esplora` only).
3. Another thing to note is that, although `Database` assumes 1-2 keychains, the current `WalletSync` "feedback" only returns one number (which is interpreted as the larger "missing count" of the two keychains). This is done for simplicity, and because we are planning to only have one keychain per database in the future.
https://github.com/bitcoindevkit/bdk/blob/f0c876e7bf38566c0d224cbe421ee312ffc06660/src/blockchain/mod.rs#L157-L161
4. Please have a read of https://github.com/bitcoindevkit/bdk/pull/672#issuecomment-1186929465 for additional context.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK 5c940c33cb
Tree-SHA512: aee917ed4821438fc0675241432a7994603a09a77d5a72e96bad863e7cdd55a9bc6fbd931ce096fef1153905cf1b786e1d8d932dc19032d549480bcda7c75d1b
6db5b4a094 Introduce `get_checksum_bytes` method and improvements (志宇)
Pull request description:
### Description
`get_checksum_bytes()` returns a descriptor checksum as `[u8; 8]` instead of `String`, potentially improving performance and memory usage.
In addition to this, since descriptors only use characters that fit within a UTF-8 8-bit code unit ([US-ASCII](https://www.charset.org/charsets/us-ascii)), there is no need to use the `char` type (which is 4 bytes). This can also potentially bring in some performance and memory-usage benefits.
### Notes to the reviewers
This is useful because we will be using descriptor checksums for indexing operations in the near future (multi-descriptor wallets #486 ).
Refer to comments by @afilini :
* https://github.com/bitcoindevkit/bdk/pull/647#discussion_r921184366
* https://github.com/bitcoindevkit/bdk/pull/647#discussion_r921914696
* https://github.com/bitcoindevkit/bdk/pull/654#discussion_r921980876
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 6db5b4a094
Tree-SHA512: 1cecc3a1514a3ec3ac0a50775f6b3c4dd9785e3606390ceba57cc6248b8ff19c4023add0643c48dd9d84984341c506c036c4880fca4a4358ce1b54ccb4c56687
46c344feb0 Bump version to 0.20.1-dev (Steve Myers)
78d26f6eb3 Bump version to 0.20.0 (Steve Myers)
92b9597f8b Rename `set_current_height` to `current_height` (Alekos Filini)
b5a120c649 Missing newlines (Alekos Filini)
af6bde3997 Fix: Wallet sync may decrement address index (志宇)
45db468c9b Deprecate `AddressValidator` (志宇)
01141bed5a Update CHANGELOG and lib.rs docs version (Steve Myers)
87e8646743 Bump version to 0.20.0-rc.1 (Steve Myers)
Pull request description:
Proposed tweet:
📢 Release 0.20.0 is out! Highlights include bug fixes for the ElectrumBlockchain and descriptor templates, discourage fee sniping in tx building, and new tx signing options. A big thanks to our past and latest new contributors. For all changes see: https://github.com/bitcoindevkit/bdk/releases/tag/v0.20.0
ACKs for top commit:
afilini:
ACK 46c344feb0
Tree-SHA512: 7c36a85611f715d76a37d5a285bc72f1a06297fc06b85cca7e38c3350fcbc0a3e35d38ce617a82d191538776aa49362e523beef70bbe3b93b21d8d28d774b75f
92b9597f8b Rename `set_current_height` to `current_height` (Alekos Filini)
Pull request description:
### Description
Usually we don't have any prefix except for methods that can *add* to a list or replace the list entirely (e.g. `add_recipients` vs `set_recipients`)
I missed this during review of #611
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
danielabrozzoni:
utACK 92b9597f8b - I'm sorry I didn't notice it!
Tree-SHA512: 3391068b2761bcd04d740ef41f9e772039fca7bc0e0736afcbc582ec74b6c91eb155d9e09dd7a07462eec29e32ac86e41ba339d9a550af3f754164cab6bdbf61
af6bde3997 Fix: Wallet sync may decrement address index (志宇)
Pull request description:
### Description
Fixes#649
It is critical to ensure `Wallet::get_address` with `AddressIndex::new` always returns a new and unused address.
This bug seems to be Electrum-specific. The fix is to check address index updates to ensure that newly suggested indexes are not smaller than indexes already in database.
### Notes to the reviewers
I have written new tests in `/testutils/blockchain_tests.rs` that tests all `Blockchain` implementations.
### 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
ACKs for top commit:
afilini:
ACK af6bde3997
Tree-SHA512: d714bebcf7c2836f8b98129b39b4939b0e36726acf0208e52d501f433be6cdb12f1abebc28bd7da0be8b780ccce6e1e42c8fdc6633dd486bf329bc6f88e1ce67
45db468c9b Deprecate `AddressValidator` (志宇)
Pull request description:
### Description
`AddressValidator` should be deprecated as noted by @afilini [on Discord](https://discord.com/channels/753336465005608961/753367451319926827/994899488957272064):
> address validators are supposed to be used for a slightly different thing, which is when you ask the hardware wallet to independently generate the address for a derivation index and then you compare what you see on your computer/phone with what the hardware wallet is displaying
> in the case of change addresses i agree that it's not as important (because as you said the device can just refuse to sign) but for consistency we implemented it for both external and internal addresses
> more broadly, they can be thought of as a way to get a callback every time an address is generated, which may also be useful for other things (for example when i was working on a green-compatible client written in bdk i used that feature to ping the server every time a new address was generated, because that's required in their protocol)
> that said, i think currently pretty much nobody uses them and i am myself moving away from the concept that "everything needs to happen inside bdk": currently my mindset is targeted more towards reducing complexity by breaking down individual parts and wrapping them or making them "extensible" in some way
> that is to say: if you want to verify addresses in your hardware wallet you don't necessarily need bdk to do it for you (actually, you would still have to implement the callback manually), you can just call bdk to get a new addr and then ping the device yourself. and this would allow us to reduce complexity and delete some code
> actually, here's an idea: unless somebody here is opposed to this, i can make a pr to deprecate address validators in the next (0.20) release. if after that again nobody complains we can completely remove them and point users towards different strategies to achieve the same goal
### 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 `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 45db468c9b
Tree-SHA512: 71071f4494537ece9153f5308cb4f576189016afa8ac87bc57bfdcda03ee94d5f7a3477d04f6dd37eeeea2fada6aaad42ad29c964df0971beeda7418ada65f6d
5d00f82388 test that BDK won't add unconf inputs when fee bumping (Daniela Brozzoni)
98748906f6 test: fix populate_test_db conf calculation (Daniela Brozzoni)
1d9fdd01fa Remove wrong TODO comment in build_fee_bump (Daniela Brozzoni)
Pull request description:
### Description
Closes#144
### Notes to reviewers
#144 is describing a bug that doesn't seem to happen in BDK master anymore (BDK not respecting BIP125 rule 2). This PR just adds a test to check that the bug is fixed.
### 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
ACKs for top commit:
afilini:
ACK 5d00f82388
Tree-SHA512: 95833f3566f9716762884d65f3f656346482e45525a3e92efa86710b9f574fdd9af7d235f1f425e4298d6ff380db9af60d1d2008ccde2588d971757db2d136b8
db9d43ed2f use network to set coin type (Esraa Jbara)
Pull request description:
resolves#578
* [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
ACKs for top commit:
danielabrozzoni:
re-ACK db9d43ed2f
afilini:
re-ACK db9d43ed2f
Tree-SHA512: 0310a09ef21c6fc792688a9ccc19221b1cffaeceefd34f4c83f206e965abe963a78f9e4ca53db046b39e7bf1be118a101afe5c08c43f06ecf35ed9536102cd9b
e85aa247cb Avoid using immature coinbase inputs (Daniela Brozzoni)
0e0d5a0e95 populate_test_db accepts a `coinbase` param (Daniela Brozzoni)
Pull request description:
### Description
With this PR we start considering how many confirmations a coinbase has. If it's not mature yet, we don't use it for building transactions.
Fixes#413
### Notes to the reviewers
This PR is based on #611, review that one before reviewing this 😄
007c5a78335a3e9f6c9c28a077793c2ba34bbb4e adds a coinbase parameter to `populate_test_db`, to specify if you want the db to be populated with immature coins. This is useful for `test_spend_coinbase`, but that's probably going to be the only use case.
I don't think it's a big deal to have a test function take an almost_always_useless parameter - it's not an exposed API, anyways. But, if you can come up with a different way of implementing `test_spend_coinbase` that doesn't require 007c5a78335a3e9f6c9c28a077793c2ba34bbb4e, even better! I looked for it for a while, but other than duplicating the whole `populate_test_db` code, which made the test way harder to comprehend, I didn't find any other way.
### 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
ACKs for top commit:
afilini:
ACK e85aa24
Tree-SHA512: 30f470c33f9ffe928500a58f821f8ce445c653766459465eb005031ac523c6f143856fc9ca68a8e1f23a485c6543a9565bd889f9557c92bf5322e81291212a5f
612da165f8 `Blockchain` stop_gap testing improvements (志宇)
8a5f89e129 Fix hang when `ElectrumBlockchainConfig::stop_gap == 0` (志宇)
Pull request description:
* Ensure `chunk_size` is > 0 during wallet sync.
* Slight refactoring for better readability.
* Add test: `test_electrum_blockchain_factory_sync_with_stop_gaps`
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
`Wallet::sync` hangs indefinitely when syncing with Electrum with `stop_gap` set as 0.
The culprit is having `chunk_size` set as `stop_gap`. A zero value results in syncing not being able to progress.
Fixes#651
### 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
ACKs for top commit:
afilini:
ACK 612da165f8
Tree-SHA512: 56f1bff788855facc21856209922594cff9f639c5c58ecd180a0493322a75a564b72ded330ab0b6d6c90007ce859d2b8a5d2870d619bae5ddf9a3d64837f3753
6a15036867 Restrict `drain_to` usage (Daniela Brozzoni)
Pull request description:
### Description
Before this commit, you could create a transaction with `drain_to` set
without specifying recipients, nor `drain_wallet`, nor `utxos`. What would
happen is that BDK would pick one input from the wallet and send
that one to `drain_to`, which is quite weird.
This PR restricts the usage of `drain_to`: if you want to use it as a
change output, you need to set recipients as well. If you want to send
a specific utxo to the `drain_to` address, you specify it through
`add_utxos`. If you want to drain the whole wallet, you set
`drain_wallet`. In any other case, if `drain_to` is set, we return a
`NoRecipients` error.
Fixes#620
### 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 - kinda?
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK 6a15036867
Tree-SHA512: 69076977df37fcaac92dd99d2f2c9c37098971817d5b0629fc7e3069390eb5789331199b3b7c5d0569d70473f4f37e683a5a0b30e2c6b4e2ec22a5ef1d0f2d77
97bc9dc717 Discourage fee sniping with nLockTime (Daniela Brozzoni)
Pull request description:
### Description
By default bdk sets the transaction's nLockTime to current_height
to prevent fee sniping.
current_height can be provided by the user through TxParams; if the user
didn't provide it, we use the last sync height, or 0 if we never synced.
Fixes https://github.com/bitcoindevkit/bdk/issues/533
### Notes to the reviewers:
If you want to know more about fee sniping: https://bitcoinops.org/en/topics/fee-sniping/
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 97bc9dc717
Tree-SHA512: e92d1ae907687d9fee44d120d790f1ebdf14b698194979e1be8433310fd5636afa63808effed12fce6091f968ec6b76b727cfee6fed54068af0a7450239fdd26
77c7d0aae9 Additional comments for `TransactionDetails`. (志宇)
Pull request description:
### Description
I'm not sure if this is needed or helpful, but this PR adds comments to describe how the `sent` and `received` fields of `TransactionDetails` are calculated.
I wasn't sure how it was done until I looked deeper into the codebase (but maybe I am too much of a beginner and this is common sense for most).
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
danielabrozzoni:
re-ACK 77c7d0aae9
Tree-SHA512: 8d29d249a70bc2d0631078b6772c5543bdc61ee43df3810ab666f5e97ca59b0d4cfc8acad14bbaf8674baba319f24fa2781a42740ca42bccd9688831aaedea72
d020dede37 Fix README.md link to rust 1.56.1 blog post (Steve Myers)
Pull request description:
### Description
Fix link to rust blog for 1.56.1 as pointed out by ulrichard: https://github.com/weareseba/bdk-reserves/pull/5#issuecomment-1159212838
### Checklists
#### All Submissions:
* [x] I've signed all my commits
Top commit has no ACKs.
Tree-SHA512: a5bd76fca97dd64c12617b43230dbc36b1178e47224ce324b67cd13999e5f92d2a05d6a9e909841e6d5c6904f2fa426b6bee1001e757d53cc91fb4fd3803f56b
b289c4ec2d Bump MSRV from 1.56.0 to 1.56.1 (Daniela Brozzoni)
Pull request description:
### Description
This PR fixes the CI, which is currently failing after a Github Actions update.
The MSRV is bumped to 1.56.1 (from 1.56.0), since that's `reqwest`'s current MSRV.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK b289c4ec2d
Tree-SHA512: e3aa8638ffd374371037e71d1c9790dd4fb3aa29f0ade5655b47ef5184b26120ee5281cd1745612af2572beb8a6bb43f84da3d60a73b455f9652853fcf31b62d
a85ef62698 fix typo (Buck Perley)
Pull request description:
<!-- 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 -->
just a small typo fix
### 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 -->
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
rajarshimaitra:
ACK a85ef62698
afilini:
ACK a85ef62698
Tree-SHA512: 089de23adae62492a0b39a27c9cb8cb8afc99e5634194118681b8a9a46ff0b073558f9cd515cd4db4c9c6e6f9c813bfa4b193d4e3f9558b34ad29cbd46cf028c
3283a200bc Bump rusqlite (Philipp Hoenisch)
3f9b4cdca9 Bump ahash (Philipp Hoenisch)
Pull request description:
### Description
Not much to say besides that I bumped some version which helps us to resolve some dependency hell :)
### Notes to the reviewers
`ahash` was previously fixed because of an incompatibility with the defined MSRV. The MSRV has been bumped to 1.56 so we can update the 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
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
rajarshimaitra:
ACK 3283a200bc
Tree-SHA512: 4760890529bb4dd87fafdda04188fd06e707398abfa7d36d26077233525e76cc8c7d8888ad996c1cc4ac31ab708ea9a56a602b1d1578b97f9e44b610df3d969b
Only use the old `importmulti` with Core versions that don't support
descriptor-based (sqlite) wallets.
Add an extra feature to test against Core 0.20 called `test-rpc-legacy`
20d36c71d4 Update CHANGELOG.md for Taproot (Alekos Filini)
ef08fbd3c7 Update to the newest release of rust-bitcoin (Alekos Filini)
5320c8353e taproot-tests: validate `tap_tree` in psbt outputs (Alekos Filini)
c1bfaf9b1e Add blockchain tests for parsing, signing, finalizing taproot core psbts (Steve Myers)
0643f76c1f taproot-tests: Add tests for the policy module (Alekos Filini)
89cb425e69 taproot-tests: Add test coverage for tx signing (Alekos Filini)
461397e590 taproot-tests: Test taproot key and script spend in the blockchain tests (Alekos Filini)
c67116fb55 policy: Consider `tap_key_origins` when looking for sigs in PSBTs (Alekos Filini)
572c3ee70d policy: Build `SatisfiableItem::*Signature` based on the context (Alekos Filini)
ff1abc63e0 policy: Refactor `PkOrF` into an enum (Alekos Filini)
308708952b Fix type inference for the `tr()` descriptor, add basic tests (Alekos Filini)
fe1877fb18 Support `tr()` descriptors in dsl (Alekos Filini)
cdc7057813 Add `tr()` descriptors to the `descriptor!()` macro (Alekos Filini)
c121dd0252 Use `tap_key_origins` in PSBTs to derive descriptors (Alekos Filini)
8553821133 Populate more taproot fields in PSBTs (Alekos Filini)
8a5a87b075 Populate `tap_key_origin` in PSBT inputs and outputs (Alekos Filini)
1312184ed7 Attach a context to our software signers (Alekos Filini)
906598ad92 Refactor signer traits, add support for taproot signatures (Alekos Filini)
Pull request description:
### Description
This is a work-in-progress PR to update BDK to rust-bitcoin `0.28` which introduces taproot support and a few other improvements. While updating we also introduce taproot support in BDK.
High level list of subtasks for this PR:
- [x] Update rust-bitcoin and rust-miniscript
- [x] Stop using deprecated structs
- [x] Add taproot metadata to psbts
- [x] Produce schnorr signatures
- [x] Finalize taproot txs
- [x] Support `tr()` descriptors in the `descriptor!()` macro
- [x] Write a lot of tests
- [x] Interoperability with other wallets (Core + ?)
- [x] Signing/finalizing a psbt made by core
- [x] Producing a psbt that core can sign and finalize
- [x] Creating psbts
- [x] Verify the metadata are correct
- [x] Verify sighashes are applied correctly
- [x] Create a tx with a foreign taproot and non-taproot utxo
- [x] Signing psbts
- [x] Signing for a key spend
- [x] Signing for a script spend
- [x] Signing with a single (wif) key
- [x] Signing with an xprv (with and without knowing the utxo being spent in the db)
- [x] Signing with weird sighashes
- [x] Policy module
- [x] Simple key spend
- [x] More complex tap tree with a few keys
- [x] Verify both `contribution` and `satisfaction` of a PSBT input
- [x] Wallet module
- [x] Generate addresses
Fixes#63
### Notes to the reviewers
#### Milestone
I'm adding this to the `0.19` milestone because now that rust-bitcoin and rust-miniscript have been released we should not waiting too long to release a version of BDK that supports the new libraries.
#### API Breaks
Since this is an API-break because of the new version of rust-bitcoin and rust-miniscript, I'm also taking the chance to update a few things in our lib that I had been thinking about for a while.
One example is the signer interface, which had that weird `sign_whole_tx()` method. This has now been removed, and the `Signer` trait replaced with `TransactionSigner` and `InputSigner`. I'm also starting to think that the signer should not only look at the psbt to figure out what to do, but ideally it should also receive some information about the descriptor (for example, the type) to simplify the code.
One option is to add an extra parameter, but that would probably only be used by our internal signers and not much else (for example, if you ask an hardware wallet to sign, it will probably already know what kind of wallet you have).
Another option is to wrap `PrivateKey` and `DescriptorXKey<ExtendedPrivKey>` which are the two internal signers we support with a struct that contains metadata about the descriptor, and then implement the signer traits on that struct. We could construct this in `Wallet::new()`, after miniscript parses the descriptor.
#### MSRV Bump
Due to the update of `rust-electrum-client`, which in turn depends on an updated `webpki`, we will have to bump our MSRV beacuse 1.46 is not supported by the new `webpki` version.
### 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 docs for the new feature
* [ ] I've updated `CHANGELOG.md`
Top commit has no ACKs.
Tree-SHA512: 44ec6c4e7fe0bc87862bb76581ddae7905c8796a4a130c90b48b73b4b7d01ea1a20043b8a0ff449fc2db2f7aecc490def8daee2d420809ded1ece7f085a48f55
This allows the signer to know the signing context precisely without
relying on heuristics on the psbt fields.
Due to the context being static, we still have to look at the PSBT when
producing taproot signatures to determine the set of leaf hashes that
the key can sign for.
82de8b50da Populate the redeemScript for `sh(wsh(sortedmulti()))` (Alekos Filini)
Pull request description:
### Description
Also explicitly match all the individual variants to ensure a similar problem
doesn't happen again.
Fixes#609
### 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
ACKs for top commit:
notmandatory:
ACK 82de8b50da
Tree-SHA512: 7cccae44679089b70e8d9df466a165debe17c4cf30ae11cb27357fbda830d3d7ce53161e414b354b1f2be46efe413b8e44132f5e6d625298b740d44112ca286a
00164588f2 Stop using deprecated structs (Alekos Filini)
a16c18255c Upgrade to rust-bitcoin 0.28 and miniscript 7.0 (Alekos Filini)
Pull request description:
### Description
Upgrade all our dependencies to work with the new release of rust-bitcoin
### Notes to the reviewers
The commits in this pr were originally part of #593
### 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 updated `CHANGELOG.md`
ACKs for top commit:
rajarshimaitra:
ACK 00164588f2
Tree-SHA512: eef7e94246e619686b4dfffd6e4cb685630fe2eaf9447f2f0b49ed2643d67f81c50e0d89b66267db4552a05e58f638d885eb7056270648403f716803fce9e275
b4100a7189 wrong Network path fixed (Pedro Felix)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
This change needs to be made for this example to compile correctly.
<!-- 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 -->
### 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
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
notmandatory:
ACK b4100a7189
Tree-SHA512: b4b7d186cabc7b3ed2d5b6411414d56eeee09c4e8d9d704a26f8817d6573b6b182d7ef1ff8139b6f4f6fb3e4ec99c2c09758804a6bb25051e45b0885c0def5e3
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
blockchains for different descriptors. It's implemented automatically
for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
piece of code that deals with multiple sub-wallets to just get a
`&B: BlockchainFactory` to sync all of them.
These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).
This is the first step for #549, as BIP47 needs to sync many different
descriptors internally.
It's also very useful for #486.
e7a56a9268 Change wallet::get_funded_wallet to return Wallet<AnyDatabase> (Steve Myers)
Pull request description:
### Description
Change testing function `wallet::get_funded_wallet` to return `Wallet<AnyDatabase>` instead of `Wallet<MemoryDatabase>`. This will allow us to use this function for testing `bdk-ffi` which only works with `Wallet<AnyDatabase>`.
### Notes to the reviewers
This is required to complete https://github.com/bitcoindevkit/bdk-ffi/pull/148.
### 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
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK e7a56a9268
Tree-SHA512: 47b53ab6dcee63fc7b24666d3cf9a0ad832782081dd2fe92961c8c9b4c302df90db96b0b518af71d6cbc85319434971219100f8cedb35ce7212d944db29a4295
4628a10191 Fix typo in docs for TxBuilder allow_shrinking method (thunderbiscuit)
Pull request description:
### Description
This PR fixes a small typo in the documentation for the `allow_shrinking()` method on the `TxBuilder`.
### Notes to the reviewers
The sentence
```txt
Explicitly tells the wallet that it is allowed to reduce the fee of the output matching this `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet will attempt to find a change output to shrink instead.
```
was changed for
```txt
Explicitly tells the wallet that it is allowed to reduce the amount of the output matching this `script_pubkey` in order to bump the transaction fee. Without specifying this the wallet will attempt to find a change output to shrink instead.
```
To reflect the fact that it's the _amount_ of the output that is being reduced in order to leave more for the fees.
### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
notmandatory:
ACK 4628a10191
Tree-SHA512: ca135764d78144295fdbdb4673ca6dec153c6f83ee7fb5d7d3051e0d46b04d23b2f3c3fd66e44491da842c778e72b52f8996d00e4c20986d7ccb11587f0d7d27
6931d0bd1f add private function select_sorted_utxso to be resued by multiple CoinSelection impl (KaFai Choi)
545beec743 add OldestFirstCoinSelection (KaFai Choi)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
This PR is to add `OldestFirstCoinSelection`. See this issue for detail https://github.com/bitcoindevkit/bdk/issues/120
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
### Notes to the reviewers
Apologize in advance if the quality of this PR is too low.(I am newbie in both bitcoin wallet and rust).
While this PR seemed very straight-forward to me in the first glance, it's actually a bit more complicated than I thought as it involves calling DB get the blockheight before sorting it.
The current implementation should be pretty naive but I would like to get some opinion to see if I am heading to a right direction first before working on optimizations like
~~1. Avoiding calling DB for optional_utxos if if the amount from required_utxos are already enough.~~ Probably not worth to do such optimization to keep code simpler?
### 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
* [x] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK 6931d0bd1f
Tree-SHA512: d297bbad847d99cfdd8c6b1450c3777c5d55bc51c7934f287975c4d114a21840d428a75a172bfb7eacbac95413535452b644cab971efb8c0b5caf0d06d6d8356
c752ccbdde Expose bip39::Error (志宇)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Expose `bip39::Error` (fixes#581 )
### Notes to the reviewers
I am aware that the `bip39` module plans to be rewritten (as per #561 ), however this seems like a rather straightforward and quick change that may be useful in the short/mid term.
### Checklists
#### Bugfixes:
* [x] Expose `bip39::Error`
ACKs for top commit:
afilini:
ACK c752ccbdde
Tree-SHA512: 98b7ac1ba88aed07d9160830ee80496c32d531c15ada0e9b50a97f0883fbfced22fa83a7c7f8366aadb7e7a667d8a63dde869d31cc375206d277e55b2ec3089d
b5c8ce924b Bump version to 0.19.0-dev (Steve Myers)
e3ce50059f Bump version to 0.18.0 (Steve Myers)
8a2a6bbcee Add `n:` wrapper to vulnerable scripts (Alekos Filini)
122e6e7140 Bump 'miniscript' dependency version to '^6.1' (Steve Myers)
9ed36875f1 export: Rename `WalletExport` to `FullyNodedExport` (Alekos Filini)
f90e3f978e Update changelog with addition of keychain to AddressInfo (Steve Myers)
68e1b32d81 Bump version to 0.18.0-rc.1 (Steve Myers)
Pull request description:
### Description
Release 0.18.0.
### Notes to the reviewers
This branch also includes the following changes which are not yet in the `master` branch:
- Rename `WalletExport` to `FullyNodedExport`, deprecate the former.
- Bump `miniscript` dependency version to `^6.1`.
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK b5c8ce924b
Tree-SHA512: d6a8c1ca2c13d3a84704592053cc450557c96229b0092e655ccf97775c3e0a23e9e1499cf8c9fc5a24fc0665ec0333d0ec6d6b06cb15eb2e14c033f06c3032c2
The `WalletExport` type can still be used as an alias to
`FullyNodedExport` but it has been marked as deprecated and will be
removed in the next release.
This lets us use a tuple of (generated mnemonic, optional passphrase) as
a `DerivableKey` directly, without extracting the inner mnemonic from the
`GeneratedKey` wrapper. For BIP39 keys specifically it doesn't make much
difference because the mnemonic format doesn't encode the network, but in
general this is not the case and having a consistent API will make it
harder for people to make mistakes.
To explain why we should not extract the key: some key formats (like
BIP32 extended keys) are network-specific, meaning that if somebody
tries to use a Testnet key on a Mainnet BDK wallet, it won't work.
However, when we generate a new key we would like to be able to use that
key on any network, but we need to set some kind of placeholder for the
`network` field in the structure. This is why (or at least one of the
reasons why) we wrap the key in the `GeneratedKey` struct: we keep track
of the "valid_networks" separately, which means that even if we set our
BIP32 xprv to be a "Mainnet" key, once we go try creating a wallet with
that key BDK is smart enough to understand that `GeneratedKey`s have
their own separate set of valid networks and it will use that set to
validate whether the key can be used in the wallet or not.
a111d25476 Deprecate Database::flush() function (Steve Myers)
Pull request description:
### Description
The Database::flush() function is only needed for the sled database on mobile, instead for mobile use the sqlite database.
### Notes to the reviewers
This PR is in preparation for removing the Database::flush() function. See https://github.com/bitcoindevkit/bdk/pull/575#issuecomment-1082916667.
After the `release/0.18.0` feature freeze branch is created then #575 should be merged.
### 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
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK a111d25476
Tree-SHA512: 18434dc95dbef47118a0d4fface908bdf920a7ffcef927b36bb740c15f8efcf11dea9198b364648f16f74aaec4aa18e92a3c5e925299b2f3b9d69e566f89e790
f2f0efc0b3 Never delete spent utxos from the database (Daniela Brozzoni)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
A `is_spent` field is added to LocalUtxo; when a txo is spent we set
this field to true instead of deleting the entire utxo from the
database.
This allows us to create txs double-spending txs already in blockchain.
Listunspent won't return spent in mempool utxos, effectively excluding them from the coin selection and balance calculation
Fixes#414
### 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 updated `CHANGELOG.md`
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
Re-ACK f2f0efc
Tree-SHA512: 7984b69c56b3b12746c2301072d637dea1346730aa132f1ea67c5bd8bf685bd557fc7cbdc2fc1185e63c71db07d36cd979514921b2c83c55737cd4b96102377e
bfd0d13779 [blockchain] Fix `sent` calculation in the RPC backend (Daniela Brozzoni)
128c37595c [tests] Pass tx inputs to the testutils macro (Daniela Brozzoni)
Pull request description:
<!-- 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 -->
### 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 -->
### 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
ACKs for top commit:
afilini:
ACK bfd0d13779
Tree-SHA512: 1c214819c5dc1f1c30b1c6ef18a44a3013d587651890b26aecfc5203d128173fd91497337186bbee6934f77d3cfe1686e67b83ca6fe6e47b4c1d4b1dbcc656ee
b6fe01c466 Implement XKeyUtils on InnerXKey (Gianluca Acerbis)
Pull request description:
Closes#395
<!-- 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 -->
### 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 -->
### 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
* [ ] I've updated `CHANGELOG.md`
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
ACK b6fe01c466 - the code looks good to me, I tested locally and all tests pass
notmandatory:
ACK b6fe01c466
Tree-SHA512: 00a3ed9532a0589ac4be55a7d0c6ac5251f03f716adb2086eb934d9a9b04bcb7fd95aaeba38b68c5c90876644ce53ac5e39a912a4096e789887342f8727ae434
07c270db03 [ci] Pin nightly docs workflow rust version to nightly-2022-01-25 (Steve Myers)
Pull request description:
### Description
Pin nightly docs workflow rust version to `nightly-2022-01-25` to fix#538 .
### Notes to the reviewers
The nightly docs should be changed to the `stable` version once it supports `feature(doc_cfg)`.
### 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)
#### Bugfixes:
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
afilini:
ACK 07c270db03
Tree-SHA512: a8e5de051e1963c10d686d5cb5d1785e20f6cd322b9bfcf2d2f5c381d9124a9b8518671dfbedb11550bdfa66c8c64ad505c7e1604e2bfe23600de39cb20467dd
fcf422752b Fix 0.16.0 changelog, include WIF fix (Steve Myers)
6fb42fdea1 Bump version to 0.16.1-dev (Steve Myers)
3f65e8c64b Bump version to 0.16.0 (Steve Myers)
3f0101d317 Bump version to 0.16.0-rc.1 (Steve Myers)
Pull request description:
### Description
Merge the 0.16.0 release branch back into the master branch.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
afilini:
ACK fcf422752b
Tree-SHA512: 7a1e9297f4e93284e5e57124c7afe37d1fbef1da48375b310aad7fe8556be6bba00c3263e9d15540209fc99ba5e928d3fc751d0df6592c2af871599f00941a7b
Alekos Filini
