e3a17f67d9 add try_finalize to SignOptions (KaFai Choi)
c2e4ba8cbd add remove_partial_sigs to SignOptions (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 2 keys(`try_finalize` and `remove_partial_sigs`) in `SignOptions`. See this issue for detail https://github.com/bitcoindevkit/bdk/issues/612
### Notes to the reviewers
~I found the negative naming of these 2 new keys `do_not_finalize` and `do_not_remove_partial_sigs` are a bit confusing(like most negative named paremeter/variable). Should we actually change it back to positive naming(`do_finalize` and `do_remove_partial_sigs`)?~
### 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:
notmandatory:
ReACK e3a17f67d9
Tree-SHA512: 781b31d3ecf0bcd605206c0481fd5de3125f1c8ff18a463dbf4c821e5557847f7d70a3fe8618e100fb89f4f6899655ac0efa3593f77f915ad5bcb7e558bb2a7a
2af678aa84 Get block hash by its height (Vladimir Fomene)
Pull request description:
### Description
This PR create a new trait `blockchain::GetBlockHash` with a `get_block_hash` method which returns a block hash given the block height. This has been implemented for all blockchain backends.
Fixes#603
### Notes to the reviewers
I haven't updated the `CHANGELOG.md` and docs. Am I suppose to update it for this 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:
* [x] 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:
notmandatory:
ACK 2af678aa84
Tree-SHA512: 9c084a6665ecbf27ee8170fdb06e0dc8373d6a901ce29e5f5a1bec111d1507cb3bee6b03a653a55fd20e0fabe7a5eada3353e24a1e21f3a11f01bb9881ae99e5
5ff8320e3b add private function ivcec_to_u32 in keyvalue (KaFai Choi)
e68d3b9e63 remove Database::flush (KaFai Choi)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
This PR is to remove Database::flush. See this issue for detail https://github.com/bitcoindevkit/bdk/issues/567
### Notes to the reviewers
The 2nd commit is a small refactoring of adding a new private ivec_to_u32 to avoid too much code duplication. Please let me know if it's ok to include this in this PR or I should make it into a separate PR
Currently existing test cases are shared across for all Databaes implementation so I am not sure if we should add specific test cases for keyvalue(Tree) for this auto-flush behaviour?(and I feel like it's more a implementation detail). Please let me know how should I proceed for test case in this PR
### 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`
#### 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:
re-ACK 5ff8320e3b
Tree-SHA512: eb37de8217efeb89d3ae346da36d0fb55aa67554d591b4759500f793bcf6aa7601c3d717fd473136c88e76aa72dbb6008ecf62b1d4ccf5ba3cbd1598f758522a
e1a1372bae rpc: use `importdescriptors` with Core >= 0.21 (Alekos Filini)
Pull request description:
### Description
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`.
This also makes us compatible with Core 23.0 and is thus a replacement for #613, which actually looking back at it was adding support for 23.0 but probably breaking older wallets by adding the extra argument to `createwallet`.
I believe #613 should now only focus on getting the tests to work against 23.0, which is still important but not such a high priority as being compatible with the latest version of Core.
Also fixes#598
### 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:
notmandatory:
ACK e1a1372bae
Tree-SHA512: 7891b8ab2fc900ea2a186f64a32aea970f28a50339063ed0e1a8d13248e5c038b8fff3d9e26b93cb7daafd0c873379e64a28836dbe4e4b82f1983577a88971ff
d9b9b3dc46 Fix InvalidColumnIndex error (Philipp Hoenisch)
Pull request description:
This query returns 7 rows, so last row is index 6
### 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:
danielabrozzoni:
tACK d9b9b3dc46
rajarshimaitra:
tACK d9b9b3dc46
Tree-SHA512: 8a3d8a291daa4af86a2a2eacc31f002972dd9cdb9bf300a4b09e2e015c4a967dc4fa7e925afbcce8b104a01e1d7f7c8cb0badda8e1ac5ade511681f490c719d5
35feb107ed [CI] Fix cont_integration test-blockchains to run all tests (Steve Myers)
2471908151 Update CHANGELOG with warning about sqlite-db deleted wallet data (Steve Myers)
0b1a399f4e Update sqlite schema with unique index for utxos, change insert_utxo to upsert (Steve Myers)
cea79872d7 Update database tests to verify set_utxo upserts (Steve Myers)
Pull request description:
### Description
This PR fixes#591 by:
1. Add sqlite `MIGRATIONS` statements to remove duplicate utxos and add unique utxos index on txid and vout.
2. Do an upsert (if insert fails update) instead of an insert in `set_utxo()`.
3. Update database::test::test_utxo to also verify `set_utxo()` doesn't insert duplicate utxos.
### Notes to the reviewers
I verified the updated `test_utxo` fails as expected before my fix and passes after the fix. I tested the new migrations using the below `bdk-cli` command and a manually updated sqlite db with duplicate utxos.
```shell
cargo run --no-default-features --features cli,sqlite-db,esplora-ureq -- wallet -w test1 --descriptor "wpkh(tpubEBr4i6yk5nf5DAaJpsi9N2pPYBeJ7fZ5Z9rmN4977iYLCGco1VyjB9tvvuvYtfZzjD5A8igzgw3HeWeeKFmanHYqksqZXYXGsw5zjnj7KM9/*)" sync
```
### 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
* [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:
utACK 35feb107ed - Code looks good, but I didn't do any local test to see if the db gets wiped
Tree-SHA512: 753c7a0cfd0e803b5e12f39181d9a718791c4ce229d5072e6498db75a7008e94d447b3d0b4b0c205e7a8f127f60102e12bac2d271b8bad3a3038856bfd54e99c
939a1156c6 unpinning dependency tokio to 1 (Richard Ulrich)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
unpinning dependency tokio to just 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`
#### 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 939a1156c6
Tree-SHA512: 7b33522411f6b63d844d12cc86c8992aa4105c29e91dbef67362a6c600f3ef39e802ff5893054bf8feeb7fa5bf383765025096cbc2ccb9a6b83bc5c919a5a43d
8795da4839 wallet: Move `wallet_name_from_descriptor` above the tests (Alekos Filini)
9c405e9c70 [blockchain] Add traits to reuse `Blockchain`s across multiple wallets (Alekos Filini)
2d83af4905 Move testutils macro module before the others (Alekos Filini)
Pull request description:
### Description
Add three 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.
- `StatefulBlockchain` is the opposite of `StatelessBlockchain`: it provides a method to "clone" a `Blockchain` with an updated internal state (a new wallet checksum and, optionally, a different number of blocks to skip from genesis). Potentially this also allows reusing the underlying connection on `Blockchain` types that support it.
- `MultiBlockchain` is a generalization of this concept: it's implemented automatically for every type that implements `StatefulBlockchain` and 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: MultiBlockchain` without having to deal with stateful and statless blockchains individually.
These new traits have been implemented for Electrum, Esplora and RPC (the first two being stateless and the latter stateful). 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.
### Notes to the reviewers
This is still a draft because:
- I'm still wondering if these traits should "inherit" from `Blockchain` instead of the less-restrictive `WalletSync` + `GetHeight` which is the bare minimum to sync a wallet
- I need to write tests, at least for rpc which is stateful
- I need to add examples
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
rajarshimaitra:
tACK 8795da4839
Tree-SHA512: 03f3b63d51199b26a20d58cf64929fd690e2530f873120291a7ffea14a6237334845ceb37bff20d6c5466fca961699460af42134d561935d77b830e2e131df9d
cca69481eb Bump MSRV to 1.56 (Alekos Filini)
Pull request description:
### Description
Following the discussion in #331, bump the MSRV to `1.56`. We already have other PRs bumping it to at least `1.51` (#593), but I'm felling like we are always lagging behind and our CI breaks regularly. As @LLFourn suggested, this PR makes a relatively large bump, hoping this buys us enough time to finish splitting up BDK, which will allow us to have a lower MSRV for the "core" crate.
### 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:
notmandatory:
ACK cca69481eb
Tree-SHA512: bc6572dc94e1c4cbb6d21f6e06a9730af5763fb4811311a61a6a6ec850b5a65664a21e4a7070a3ebcd702529fbba97b2e9a43c1277b9b9f092e194f16a39bc1a
9ed36875f1 export: Rename `WalletExport` to `FullyNodedExport` (Alekos Filini)
Pull request description:
### Description
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.
### 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:
bucko13:
Concept ACK 9ed3687
notmandatory:
ACK 9ed36875f1
Tree-SHA512: fe07307092d3040b30ae96901d5fc4afecee635412bca4a0d2e78d90f07990e7bbcd6b87ed923bf5804385356f419cacadbfefbaf97ebc9a622e34548438ee00
44758f9483 Simplify the `rpcwallet` example using the improved keys traits (Alekos Filini)
c350064dae Update changelog changes in `keys` mod (Alekos Filini)
92746440db [keys] Make `GenerateKey<K>` clonable if K is (Alekos Filini)
e4eb95fb9c [keys] Implement `DerivableKey<Ctx>` for `(GeneratedKey<Mnemonic, Ctx>, Option<String>)` (Alekos Filini)
Pull request description:
### Description
I recently looked at the `rpcwallet` example and immediately thought the key generation part was very confusing. I made a few little changes to our traits and now I believe it's much better.
I'm copying the next few paragraphs from the first commit in this PR which explains why it's better not to extract the key from the `GeneratedKey` wrapper:
> 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.
### Notes to the reviewers
I'm adding this to the `0.18` milestone even if the feature freeze is today. I don't think we should wait for this PR to start the release branch, but since it's just a minor addition to our traits I believe we could merge this afterwards to the release branch if we manage to get it reviewed and ready within the next week.
Otherwise it'll just wait for `0.19`, it's not a big deal.
### 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:
notmandatory:
tACK 44758f9483
Tree-SHA512: 31a8b5a3cd56c7c39688c4804026a8b6cee950c50ea880efcddc150ef523e2ddcc544831bf996a58b5cf3b7bc4a086c1008dc0728604e5134d331c68855c2d5b
adef166b22 Create vector of thread handles to spawn threads (nickfarrow)
Pull request description:
### Description
Speeds up esplora ureq syncing. Taken from https://github.com/bitcoindevkit/bdk/pull/560
The current sync just creates a map of scripts to joinhandles which doesn't yet spawn the sync threads due to lazy evaluation. In the following `handles.map()`, the thread handles are *sequentially* evaluated and joined. With the fix, the handles are collected so that the threads spawn in parallel, and then joined
### Notes to the reviewers
I had to add a `#[allow(clippy::needless_collect)]` so that it wouldn't complain about collecting and then iterating. (Perhaps clippy is partially responsible for this issue!)
Tested sync performance by doing a fresh sync on an existing [gun](https://gun.fun) wallet.
```
---- Before fix ---
real0m13.121s
real0m13.367s
real0m13.211s
---- After fix ----
real0m5.516s
real0m5.251s
real0m5.594s
```
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
ACKs for top commit:
notmandatory:
ACK adef166b22
Tree-SHA512: 47c617117afde9b4706bfa63759bf06d1ec60ff95d8a80931e5b7e40e3293c855d2f7dac0c681173d43aecf77201a842e739b82291da09ac81909cf526a51c8d
1b9014846c Update changelog (Alekos Filini)
86abd8698f [descriptor] Expose utilities to deal with derived descriptors (Alekos Filini)
0d9c2f76e0 [export] Use the new getters on `Wallet` to generate export JSONs (Alekos Filini)
63d5bcee93 [wallet] Add more getters (Alekos Filini)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Expose more getters and internal utilities for people who need to work with descriptors.
A good example of how this can be leveraged is in the second commit, which refactors the wallet export functionality to use the new public APIs rather than using members on `Wallet` directly.
### 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:
notmandatory:
ACK 8cd055090d
Tree-SHA512: 3e8833670ebc56316fce01fc572fcc9391d602ef85f0cde8edcb446295570a9012e18f6ba8af0984153e4688f66f7eea6803ef610ceb395867e464e05c01c137
2698fc0219 update AddressInfo struct (eunoia_1729)
Pull request description:
### Description
Resolves#541.
- Updates `AddressInfo` struct to include `keychainKind`
- Updates the related `get_address` functions to pass in this field
- Updates corresponding 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
Top commit has no ACKs.
Tree-SHA512: b1d22e5f322e1a35390fb06a5185457bd53af7c4a9c4e6e2fd5e93b967fa3f56af5848ac314e63e2b0becd60dbc8a4eeb761a2bdebfe8ba43ce49b13da4190b2
364ad95e85 Fix hierarchy of headers on docs landing page (thunderbiscuit)
Pull request description:
### Description
This PR fixes the hierarchy of headers on the lib.rs docs. I noticed it as I was reading through the docs. The new hierarchy is the following:
- About (h1)
- A tour of BDK (h1)
- Examples (h1)
- Sync the balance of a descriptor (h2)
- Generate a few addresses (h2)
- Create a transaction (h2)
- Sign a transaction (h2)
- Feature flags (h1)
- Internal features (h1)
### 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 https://github.com/bitcoindevkit/bdk/pull/554/commits/364ad95e85bd09a01cb4cbf3ceb9e9fcc074c8b4
Tree-SHA512: b4439a200c320a1815c572ac36b81c4d684209cb5cf1b90d3f761893ad672a270722be7d7b61d77434a7f98c4276d6fc9694d0c86cacf03c72dc35ca17b5e7d3
8a98e69e78 Add rpc wallet creation example (rajarshimaitra)
Pull request description:
### Description
As mentioned in https://github.com/bitcoindevkit/bitcoindevkit.org/issues/83#issuecomment-1005190174 it would be helpful
to have wallet creation example code in the `example` directory.
This is an attempt to create the most simplistic bdk wallet with a RPC backend and demonstrate receiving, creating, signing
and broadcasting transaction.
The code is a refinement of the RPC tutorial https://bitcoindevkit.org/blog/bitcoin-core-rpc-demo/ with more elaborate doc comments.
### Notes to the reviewers
To automate the background RPC process `electrsd` is used which is already a `dev-dependency` of 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:
notmandatory:
tACK 8a98e69e78
Tree-SHA512: f00c1ce57ef91c3d3ab2ce08af3571c41bbe98f18f705c61ba4217c2f8061196e5eaf6122742293cd6fa16939bbbfacd0f8506cd4151b55f19a05325fd94a74a
1803f5ea8a Update `electrsd` to v0.15 (Alekos Filini)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
PR RCasatta/electrsd#33 has been merged and the new changes released as part of `v0.15.0`: the PR allows setting an env variable (`ELECTRSD_EXE`) to use a custom electrsd binary, which can be useful when the one that is downloaded automatically doesn't work for whatever reason.
### 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:
LLFourn:
ACK 1803f5e
notmandatory:
ACK 1803f5ea8a
Tree-SHA512: 01d84763a5acf3aa1686b91ba7cb0c7ec3a0ca6c7b9bac29aa74a72f54ed31c476b7a8b0f8c6c8a8ccf530ad8a19e90e144ac4cacbe436699ef9e319b5732f1c
52bc63e48f Fix pre-segwit inputs with esplora (LLFourn)
Pull request description:
Unexpectedly pre-segwit inputs have an empty JSON witness field in esplora.
Fixes#570
* [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 52bc63e48f
afilini:
ACK 52bc63e48f
Tree-SHA512: 1da82aeb2739111e1a516d94c79fe7d7c7879526a8cd780dcd63ff5ae9ccb1bed4eb20e5c575a45e37b6d1818d63ce6d3812e7b9ae34ebb02bc190a47b9750f8
0cc4700bd6 Fix typo in CHANGELOG and doc in wallet/mod.rs (Steve Myers)
660faab1e2 Fix typo in CHANGELOG (LLFourn)
45767fcaf7 Remove max_addresses sync param (LLFourn)
fbb50ad1c8 apply doc suggestions from @notmandatory (Lloyd Fournier)
035307ef54 [rpc] Filter out unrelated transactions (LLFourn)
c0e75fc1a8 Split get_tx into its own trait (LLFourn)
dcd90f8b61 Restore but depreciate new_offline (LLFourn)
410a51355b Add SyncOptions as the second argument to Wallet::sync (LLFourn)
326bfe82a8 Remove Blockchain from wallet (LLFourn)
Pull request description:
While trying to finish #490 I thought that it'd be better to try the idea of getting rid of a lot of the async macros and just having tow different traits for sync and async stuff. While trying to do that I felt that this needed to be done first.
The goal of this change is to decouple the wallet from the blockchain trait. A wallet is something that keeps track of UTXOs and transactions (and can sign things). The only reason it should need to talk to the blockchain is if doing a `sync`. So we remove all superfluous calls to the blockchain and precisely define the requirements for the blockchain to be used to sync with two new traits: `WalletSync` and `GetHeight`.
1. Stop requesting height when wallet is created
2. `new_offline` is just `new` now.
3. a `WalletSync + GetHeight` is now the first argument to `sync`.
4. `SyncOptions` replaces the existing options to `sync` and allows for less friction when making breaking changes in the fuutre (no more noop_progress!).
5. We `Box` the `Progress` now to avoid type parameters
6. broadcast has been removed from `Wallet`. You just use the blockchain directly to broadcast now.
### Notes to the reviewers
This needs #502 before it can be merged but can reviewed now.
Be on the look up for stale documentation from this change.
Our doc build looks broken because of ureq or something.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* [x] I've updated `CHANGELOG.md`
Top commit has no ACKs.
Tree-SHA512: a8f3e21e45359be642b1f30d4ac1ba74785439e1b770dbeab0a5a4b8aab1eef4bb6b855aea4382e289257bc890fa713ca832a8b6c9655f7a59e96d412b4da3e6
edf2f0ce06 refactor wallet address caching into its own public method for offline wallet use (a5an0)
Pull request description:
### Description
Currently, the only way to ensure that a wallet's internal database has addresses loaded and cached is through `Wallet::sync`: that function generates and caches up to a number of addresses if they aren't already in the database, and then uses the wallet's blockchain client to sync those addresses. If you are using an offline wallet, there is no mechanism to ensure that the database has addresses loaded. This is a problem for usecases like an offline wallet being used as a multisig signer and wanting to validate change addresses as `Wallet::is_mine` will only work properly if the owned-address is loaded in the database.
This PR takes the address caching functionality out of `Wallet::sync` and puts it in a new public method that is available to offline wallets. `Wallet::sync` uses this method internally.
### Checklists
#### All Submissions:
* [X] I've signed all my commits
* [X] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [X] I ran `cargo fmt` and `cargo clippy` before committing
* [X] I've added docs for the new feature
* [X] I've updated `CHANGELOG.md`
Top commit has no ACKs.
Tree-SHA512: 8ddc58d71457163bb20ff663ac508feb4e77000688161b63841a94db30b3f29f60f35fa2467bd99546123148873e3aed11e2f13ae6cbceda6605b83c227d9079
022256c91a Fix comment on peek_address (Lloyd Fournier)
00f0901bac Add API for internal addresses (LLFourn)
Pull request description:
There are good reasons for applications to need to get internal
addresses too. For example creating a transactions that splits an output
into several smaller ones.
### 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:
notmandatory:
ACK 022256c91a
Tree-SHA512: 0ead6669c9974332708ae6cba1a5be69cd4b3b6c7a21896efdce178d4406265ba072e1cf02d1913178d0a551f3b276d99b75676c632833964f87484e262a61b1
1999d97aeb Remove `verify` flag from `TransactionDetails` via new MIGRATIONS (Steve Myers)
0195bc0636 Update CHANGELOG (rajarshimaitra)
1d7ea89d8a Refactor sync time verification (rajarshimaitra)
b05ee78c73 Remove verifcation flag from compact_filters (rajarshimaitra)
53c30b0479 Add verification tests in CI (rajarshimaitra)
6a09075d1a Remove verify flag from sqlite DB (rajarshimaitra)
61a95d0d15 Update changelog (rajarshimaitra)
08f312a82f Remove `verify` flag from `TransactionDetails` (rajarshimaitra)
acbf0ae08e Add sync verification for `esplora` (rajarshimaitra)
4761155707 Add sync verification in `electrum` (rajarshimaitra)
98a3b3282a Remove sync verification (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
As discussed in https://github.com/bitcoindevkit/bdk/issues/498 and also in team call,
- default verification from wallet sync is removed
- `verify_tx` refactored as an wallet API
- in `sync` verification added for electrum and esplora backends, gated by `verify` flag.
- `verify` flag is removed from `TransactionDetails`.
### Notes to the reviewers
I haven't looked into `comapct_filters` to see how verification can fit there, but that will probably be required in future.
### 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
#### Wallet API change:
* [x] I've updated `CHANGELOG.md`
Top commit has no ACKs.
Tree-SHA512: 72e307008a137468d96d5c2a6ec804b18fa52363606f3c978208ae5dc22973a7f0aa37488e9bb98dde88409a12d59cc5f00c675d2d408e57e661bf6210bee67b
b04bb590f3 Pin tokio version to ~1.14 (Steve Myers)
Pull request description:
### Description
The `tokio` project recently changed their MSRV to `1.49.0`. This PR will pin the `tokio` dependency version to `~1.14` which is prior to their MSRV increase.
### Notes to the reviewers
The LDK team took this approach for their `tokio` dev-dependency, see https://github.com/lightningdevkit/rust-lightning/pull/1315.
As long as `tokio` backports bug fixes to`1.14.x` releases this should be safe. If we need any new features we can revisit this decision.
### 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 updated `CHANGELOG.md`
ACKs for top commit:
LLFourn:
ACK b04bb590f3
Tree-SHA512: a44d61e0d644900837b5a99e0f2b9e5706cf9932f7430c3a2c8adbff37c82e4a6a545849fe031da748fcd8c8a70f5266d6974c5b6634b50af5eae148b9a26635
5ac51dfe74 fix and test is_dust (James Taylor)
a0c140bb29 add doc comment for IsDust trait (James Taylor)
bf5994b14a fixed fee in test, removed unnecessary comment (James Taylor)
ca682819b3 using dust value from rust-bitcoin (James Taylor)
Pull request description:
### Description
This PR aims to fix#472 . We can retrieve the dust value for a given ``bitcoin::blockdata::script::Script``, so I adjusted the ``is_dust`` function within the ``IsDust`` trait to receive such a ``&Script``. Thus, the ``is_dust`` function can make the proper comparison.
Let me know if you think that there could be a better interface than this.
Furthermore, because this new ``is_dust`` function provides a tighter upper bound on Bitcoin Core's ``GetDustThreshold()``, it actually invalidated a test. In the test, the drain output for a transaction was no longer considered dust and no longer included in the fee. Instead, the drain output was kicked back to the sender, invalidating the asserts in line 3436, 3437 and 3441 in ``src/wallet/mod.rs``. I increased the ``FeeRate`` in the test just enough that the drain output would be small enough to considered dust again and included in the total fee.
### 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 5ac51dfe74
notmandatory:
re ACK 5ac51dfe74
Tree-SHA512: addf38fe065de581ddfcd3b4e6db92cd35d5bfa8cac78bd08c01f7a01292724a203ef59b09f3f5cd8e0fa0bb6d89efe72afda36efc11ded0424fc8105326af3f
380a4f2588 Disable reqwest's default features (Thomas Eizinger)
Pull request description:
### Description
By default, reqwest uses openssl for TLS. Any consumer wanting to use
rustls will thus pull in unnecessary dependencies.
### 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`
#### 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 380a4f2588
Tree-SHA512: 17827fdd7656a1e97b4cc302bc3c4907a8493505c798fafd9b15fde12531a32cf60e7d63e878eb2001d6b3e95f7ae3da730e227eb85c73d9de55b56456cfb3a0
57a1185aef Only run clippy for the stable rust version (Steve Myers)
Pull request description:
### Description
It was decided during the team call today (2021-12-14) to only run clippy for the stable rust version.
### Notes to the reviewers
This is required to fix the below build issues when running clippy on rust version 1.46.0.
```shell
cargo clippy --all-targets --features async-interface --no-default-features -- -D warnings
```
```text
...
Checking bitcoincore-rpc v0.14.0
error: unknown clippy lint: clippy::no_effect_underscore_binding
--> src/blockchain/mod.rs:88:1
|
88 | #[maybe_async]
| ^^^^^^^^^^^^^^
|
= note: `-D clippy::unknown-clippy-lints` implied by `-D warnings`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
= note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)
error: unknown clippy lint: clippy::no_effect_underscore_binding
--> src/blockchain/mod.rs:220:1
|
220 | #[maybe_async]
| ^^^^^^^^^^^^^^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
= note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)
```
### 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
Top commit has no ACKs.
Tree-SHA512: 3fe0d2829415c7276d5339e217cefba1255c14d6d73ec0a5eff2b8072d189ffef56088623ef75f84e400d3d05e546f759b8048082b467a3738885796b3338323
f7f9bd2409 bump electrsd dep to 0.13 (Riccardo Casatta)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
bump electrsd dep to 0.13, close#491
ACKs for top commit:
rajarshimaitra:
ACK https://github.com/bitcoindevkit/bdk/pull/492/commits/f7f9bd2409d51354698aa1f24bccba8ca429f279
notmandatory:
utACK f7f9bd24
Tree-SHA512: 89a8094387896c9296e2f0120d7a2c7419e979049a12fc9a6ae7fe1810b75af43338db235887dd9054a6b52e400491b77f8e006f61451da54aca9635098ab342
084ec036a5 Use "Description" instead of "Descriptive Title" (rajarshimaitra)
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 -->
The `Descriptive Title` part is already in the issue title, and we can use `Description` in the issue body.
### 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)
ACKs for top commit:
thunderbiscuit:
ACK 084ec03.
notmandatory:
ACK 084ec036a5
Tree-SHA512: 2fc365066849033cce056a46bc9e8ab2d931aa45fd2799ebebe3e07bb789c458491ebf2c05e5868bdff63f60dec0657043f3374135dcfbc79a1f89a2562b1883
7f8103dd76 Fix typos in comments (thunderbiscuit)
Pull request description:
### Description
This PR fixes a bunch of small typos in comments. I'm getting acquainted with the codebase and found a few typos just by chance, and ended up going through it with an IDE searching for typos in all files.
### Notes to the reviewers
To be clear, this PR _only addresses typos that are within comments_.
### 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
ACKs for top commit:
notmandatory:
ACK 7f8103dd76
Tree-SHA512: eb3f8f21cbd05de06292affd9ef69c21b52022dfdf25c562c8f4d9c9c011f18175dff0c650cb7efcfb2b665f2af80d9a153be3d12327c47796b0d00bfd5d9803
fed4a59728 Bump version to 0.14.1-dev (Steve Myers)
c175dd2aae Bump version to 0.14.0 (Steve Myers)
6b1cbcc4b7 Bump version to 0.14.0-rc.1 (Steve Myers)
Pull request description:
### Description
Merge the 0.14.0 bdk release 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
Top commit has no ACKs.
Tree-SHA512: 0a897988c47f56e24b7e831cb45ff348b5637ca1a172555e9456539f5b75f263007421d63820b20737bcddb6f4c8077271471ea830e500ca6f88b902502f8186
b9fc06195b Fix typo in check_miniscript method declaration and use (thunderbiscuit)
Pull request description:
### Description
This PR renames the `check_minsicript()` method on the `CheckMiniscript` trait and its uses throughout the codebase to `check_miniscript()`.
### 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:
notmandatory:
ACK b9fc06195b
Tree-SHA512: cc4406c653cb86f9b15e60c6f87b95c300784d6b2992abc98b3f2db4b02ce252304cc0ab2c638f080b0caf3889e832885eca19e2d6582a3557c8709311b69644
8534cd3943 Update issue templates (Steve Myers)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Add bug reporting issue template and template for proposing a "Summer of Bitcoin" project.
### Notes to the reviewers
The SoB template is basically a copy of what Adi created but lightly formatted to work as github issue templates.
### 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)
Top commit has no ACKs.
Tree-SHA512: 956f7833b7cc1daf4880f74be3886fb17508719af37a247065c463d3f95b1f058bad785590714c609b0e069fe00784bc71cfb0812a79a70c18a2b5bdb22aca6b
2fc8114180 moving the function wallet_name_from_descriptor from blockchain/rpc.rs to wallet/mod.rs as it can be useful not only for rpc (Richard Ulrich)
Pull request description:
### Description
Moving the function wallet_name_from_descriptor from rpc.rs to mod.rs
Since the local cache for compact filters should be separate per wallet, this function can be useful not only for rpc.
### Notes to the reviewers
I thought about renaming it, but waited for opinions on that.
### 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:
notmandatory:
re-ACK 2fc8114
Tree-SHA512: d5732e74f7a54f54dde39fff77f94f12c611a419bed9683025ecf7be95cde330209f676dfc9346ebcd29194325589710eafdd1d533e8073d0662cb397577119f
9c5770831d Make stop_gap a parameter to EsploraBlockchainConfig::new (LLFourn)
0f0a01a742 s/vin/vout/ (LLFourn)
1a64fd9c95 Delete src/blockchain/utils.rs (LLFourn)
d3779fac73 Fix comments (LLFourn)
d39401162f Less intermediary data states in sync (LLFourn)
dfb63d389b s/observed_txs/finished_txs/g (LLFourn)
188d9a4a8b Make variable names consistent (LLFourn)
5eadf5ccf9 Add some logging to script_sync (LLFourn)
aaad560a91 Always get up to chunk_size heights to request headers for (LLFourn)
e7c13575c8 Don't request conftime during tx request (LLFourn)
808d7d8463 Update changelog (LLFourn)
732166fcb6 Fix feerate calculation for esplora (LLFourn)
3f5cb6997f Invert dependencies in electrum sync (LLFourn)
Pull request description:
## Description
This PR does dependency inversion on the previous sync logic for electrum and esplora captured in the trait `ElectrumLikeSync`. This means that the sync logic does not reference the blockchain at all. Instead the blockchain asks the sync logic (in `script_sync.rs`) what it needs to continue the sync and tries to retrieve it.
The initial purpose of doing this is to remove invocations of `maybe_await` in the abstract sync logic in preparation for completely removing `maybe_await` in the future. The other major benefit is it gives a lot more freedom for the esplora logic to use the rich data from the responses to complete the sync with less HTTP requests than it did previously.
## List of changes
- sync logic moved to `script_sync.rs` and `ElectrumLikeSync` is gone.
- esplora makes one http request per sync address. This means it makes half the number of http requests for a fully synced wallet and N*M less requests for a wallet which has N new transactions with M unique input transactions.
- electrum and esplora save less raw transactions in the database. Electrum still requests input transactions for each of its transactions to calculate the fee but it does not save them to the database anymore.
- The ureq and reqwest blockchain configuration is now unified into the same struct. This is the only API change. `read_timeout` and `write_timeout` have been removed in favor of a single `timeout` option which is set in both ureq and reqwest.
- ureq now does concurrent (parallel) requests using threads.
- An previously unnoticed bug has been fixed where by sending a lot of double spending transactions to the same address you could trick a bdk Esplora wallet into thinking it had a lot of unconfirmed coins. This is because esplora doesn't delete double spent transactions from its indexes immediately (not sure if this is a bug or a feature). A blockchain test is added for this.
- BONUS: The second commit in this PR fixes the feerate calculation for esplora and adds a test (the previous algorithm didn't work at all). I could have made a separate PR but since I was touching this file a lot I decided to fix it here.
## Notes to the reviewers
- The most important thing to review is the the logic in `script_sync.rs` is sound.
- Look at the two commits separately.
- I think CI is failing because of MSRV problems again!
- It would be cool to measure how much sync time is improved for your existing wallets/projects. For `gun` the speed improvements for modest but it is at least hammering the esplora server much less.
- I noticed the performance of reqwest in blocking is much worse in this patch than previously. This is because somehow reqwest is not re-using the connection for each request in this new code. I have no idea why. The plan is to get rid of the blocking reqwest implementation in a follow up PR.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
ACKs for top commit:
rajarshimaitra:
Retested ACK https://github.com/bitcoindevkit/bdk/pull/461/commits/a630685a0ab95604cadbb423f65d8358ba4bc4e4
Tree-SHA512: de74981e9d1f80758a9f20a3314ed7381c6b7c635f7ede80b177651fe2f9e9468064fae26bf80d4254098accfacfe50326ae0968e915186e13313f05bf77990b
Now works with latest released versions of rust-bitcoincore-rpc and
bitcoind. Once these crates are updated to support creating descriptor
wallets and add importdescriptors and bech32m support this test will
need to be updated.
Steve Myers