Merge bitcoindevkit/bdk#1308: feat(esplora): include previous `TxOut`s for fee calculation
552f11cb5f66a9366dbc05c801fcae043ebe5b82 feat(esplora): include previous `TxOut`s for fee calculation The previous `TxOut` for transactions received from an external wallet are added as floating `TxOut`s to `TxGraph` to allow for fee calculation. (Wei Chen)
Pull request description:
### Description
Partially implements #1265.
The previous `TxOut` for transactions received from an external wallet are added as floating `TxOut`s to `TxGraph` to allow for fee calculation.
### Notes to the reviewers
Currently only the `esplora` portion of #1265 has been implemented.
The `electrum` portion will potentially be done in a new PR, as discussed on the 1/30/24 Lib call.
### Checklists
#### To Do:
* [ ] Implement `electrum` portion of #1265.
#### 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
Wei Chen [Wed, 31 Jan 2024 15:41:42 +0000 (23:41 +0800)]
feat(esplora): include previous `TxOut`s for fee calculation
The previous `TxOut` for transactions received from an external
wallet are added as floating `TxOut`s to `TxGraph` to allow for
fee calculation.
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
There are several instances in the code where we allow clippy lints that would otherwise be flagged during regular checks. It would be preferable to minimize the number of "clippy allow" attributes either by fixing the affected areas or setting a specific configuration in `clippy.toml`. In cases where we have to allow a particular lint, it should be documented why the lint doesn't apply.
For context see https://github.com/bitcoindevkit/bdk/issues/1127#issuecomment-1784256647 as well as the commit message details.
One area I'm unsure of is whether `Box`ing a large error in 4fc2216 is the right approach. Logically it makes sense to avoid allocating a needlessly heavy `Result`, but I haven't studied the implications or tradeoffs of such a 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
bdk_chain to 0.9.0
bdk_bitcoind_rpc to 0.4.0
bdk_electrum to 0.7.0
bdk_esplora to 0.7.0
bdk_file_store to 0.5.0
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* Changed to implement `ElectrumExt` for all that implements `ElectrumApi`.
### 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
After a second look at the `update_local_chain` implementations, it was clear that they were over complicated. This PR simplifies the `EsploraExt::update_local_chain` method(s) of the `bdk_esplora` crate and adds a whole bunch of tests.
### Notes to the reviewers
The description of #1199 is very brief, however @danielabrozzoni mentioned about potentially-problematic logic with `ASSUME_FINAL_DEPTH`. The logic was indeed convoluted and it did not need to be that way. This PR removes the need for `ASSUME_FINAL_DEPTH`.
* [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~ (there are now lots of tests though)
* [x] I'm linking the issue being fixed by this PR
We now filter out duplicate coins before calling `CoinSelectionAlgorithm::coin_select`. If a UTXO exists in both `required_utxos` and `optional_utxos`, only the copy in `required_utxos` will be kept.
Test helper methods are also updated to not create duplicate UTXOs.
### Changelog notice
Fixed
* Filter out duplicate UTXOs before calling `CoinSelectionAlgorithm::coin_select`. If a UTXO exists in both `required_utxos` and `optional_utxos`, only the copy in `required_utxos` will be kept.
### 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
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* [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
We forgot to include the `last_seen` when converting from `ChainPosition` to `ConfirmationTime`.
### Notes to the reviewers
No brainer.
### Changelog notice
* Fix `ConfirmationTime` conversion from `ChainPosition` to include the `last_seen`.
### 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
`EntryIter` performance is improved by reducing syscalls. The underlying file reader is wrapped with `BufReader` (to reduce calls to `read` and `seek`).
Two new tests are introduced. One ensures correct behavior when the last changeset write is too short. The other ensures the next write position is correct after a short read.
### Notes to the reviewers
This is extracted from #1172 as suggested by https://github.com/bitcoindevkit/bdk/pull/1172#pullrequestreview-1817465627.
### Changelog notice
Changed
* `EntryIter` performance is improved by reducing syscalls.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
This method is useful, I'm not sure why it is not public.
I've also updated the docs and tests.
### Changelog notice
Added
* Expose `SpkIterator::new_with_range`
### 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
志宇 [Tue, 23 Jan 2024 04:32:13 +0000 (12:32 +0800)]
fix(file_store): recover file offset after read
Because we use wrap the file with `BufReader` with the `EntryIter`, we
need to sync the `BufReader`'s position with the file's offset when we
drop the `EntryIter`. Therefore we have a custom drop impl for
`EntryIter`.
This PR removes the routines `get_checksum` and `get_checksum_bytes` (in the bdk crate, descriptor/checksum.rs), which have been deprecated in 0.24.0. Consequently, the routine `calc_checksum_bytes_internal` is consolidated into `calc_checksum_bytes`, as the boolean parameter `exclude_hash` is not needed anymore. See also the two TODOs in the touched file.
### Notes to the reviewers
In the second commit, the signature of the function `calc_checksum_bytes` slightly changes, as the `desc` parameter now is declared as `mut`, in order to change the local variable within the function. My rust experience is rather limited, so I'm not sure if this is a problem for users. IIUC, this is comparable to changing a pass-by-value parameter in C++ from `const std::string desc` to `std::string desc`, which is relevant only for the function implementation, but doesn't change the interface.
### Changelog notice
- Remove deprecated `get_checksum` and `get_checksum_bytes` routines
### 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
Some janitor cleaning in the README to update the `example-crates/`
### Notes to the reviewers
None.
### Changelog notice
- fix(readme): update example crates
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
Note also, the bit referring to Electrum's API sounds more like a developer note, so I made it a regular code comment.
### 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
* Corrects an awkward use of the word 'an'
* Adds missing backticks, brackets in a few places
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
Thank you @thunderbiscuit for raising this issue. I noticed this method borrows the language from `SpkTxOutIndex::sent_and_received`, which this PR doesn't touch. I think in general it's desirable that methods in `bdk_chain` crate would refer to some of the core structures 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
Remove gratuitous use of lifetimes in the main persistence struct `Store`. Having lifetimes on this means that you have to keep the magic bytes alive longer than the database which is particularly offensive if you have to send the database to another thread. On top of that the bytes aren't even read.
Introduce block-by-block API for `bdk::Wallet`. A `wallet_rpc` example is added to demonstrate syncing `bdk::Wallet` with the `bdk_bitcoind_rpc` chain-source crate.
The API of `bdk_bitcoind_rpc::Emitter` is changed so the receiver knows how to connect to the block emitted.
### Notes to the reviewers
### Changelog notice
Added
* `Wallet` methods to apply full blocks (`apply_block` and `apply_block_connected_to`) and a method to apply a batch of unconfirmed transactions (`apply_unconfirmed_txs`).
* `CheckPoint::from_block_ids` convenience method.
* `LocalChain` methods to apply a block header (`apply_header` and `apply_header_connected_to`).
* Test to show that `LocalChain` can apply updates that are shorter than original. This will happen during reorgs if we sync wallet with `bdk_bitcoind_rpc::Emitter`.
Fixed
* `InsertTxError` now implements `std::error::Error`.
#### 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
LLFourn [Fri, 19 Jan 2024 00:23:46 +0000 (11:23 +1100)]
feat(persist): Add stage_and_commit to Persist
In the example_cli we were not always committing (seemingly by mistake).
This then caused all the examples to have to compensate by manually
committing.
bdk_chain to 0.8.0
bdk_bitcoind_rpc to 0.3.0
bdk_electrum to 0.6.0
bdk_esplora to 0.6.0
bdk_file_store to 0.4.0
Fixes #1281.
### Notes to the reviewers
Since the version of bdk_chain changed all crates that depend on it also need to have their versions bumped.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
This will help avoid confusion with mainnet descriptors.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
Previously `SpkTxOutIndex` methods can be called from `KeychainTxOutIndex` due to the `DeRef` implementation. However, the internal `SpkTxOut` will also contain lookahead spks resulting in an error-prone API.
`SpkTxOutIndex` methods are now not directly callable from `KeychainTxOutIndex`. Methods of `KeychainTxOutIndex` are renamed for clarity. I.e. methods that return an unbounded spk iter are prefixed with `unbounded`.
In addition to this, I also optimized the peek-address logic of `bdk::Wallet` using the optimized `<SpkIterator as Iterator>::nth` implementation.
### Notes to the reviewers
This is mostly refactoring, but can also be considered a bug-fix (as the API before was very problematic).
### Changelog notice
Changed
* Wallet's peek-address logic is optimized by making use of `<SpkIterator as Iterator>::nth`.
* `KeychainTxOutIndex` API is refactored to better differentiate between methods that return unbounded vs stored spks.
* `KeychainTxOutIndex` no longer directly exposes `SpkTxOutIndex` methods via `DeRef`. This was problematic because `SpkTxOutIndex` also contains lookahead spks which we want to hide.
Added
* `SpkIterator::descriptor` method which gets a reference to the internal descriptor.
### 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
#### Bugfixes:
* [x] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
志宇 [Sat, 13 Jan 2024 12:04:49 +0000 (20:04 +0800)]
refactor(chain)!: revamp `KeychainTxOutIndex` API
Previously `SpkTxOutIndex` methods can be called from
`KeychainTxOutIndex` due to the `DeRef` implementation. However, the
internal `SpkTxOut` will also contain lookahead spks resulting in an
error-prone API.
`SpkTxOutIndex` methods are now not directly callable from
`KeychainTxOutIndex`. Methods of `KeychainTxOutIndex` are renamed for
clarity. I.e. methods that return an unbounded spk iter are prefixed
with `unbounded`.
feat(chain)!: make `IndexedTxGraph::apply_block_relevant` more efficient
Previously, `apply_block_relevant` used `batch_insert_relevant` which
allows inserting non-topologically-ordered transactions. However,
transactions from blocks are always ordered, so we can avoid looping
through block transactions twice (as done in `batch_insert_relevant`).
Additionally, `apply_block_relevant` now takes in a reference to a
`Block` instead of consuming the `Block`. This makes sense as typically
very few of the transactions in the block are inserted.
志宇 [Sat, 30 Dec 2023 12:48:20 +0000 (20:48 +0800)]
feat(bitcoind_rpc)!: emissions include checkpoint and connected_to data
Previously, emissions are purely blocks + the block height. This means
emitted blocks can only connect to previous-adjacent blocks. Hence, sync
must start from genesis and include every block.
Add a method for disconnecting a chain of blocks starting from the given `BlockId`.
### Notes to the reviewers
I want to have this for https://github.com/utreexo/utreexod/pull/110
### Changelog notice
Added
* `LocalChain::disconnect_from` method to evict a chain of blocks starting from a given `BlockId`.
### 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
志宇 [Sat, 13 Jan 2024 09:47:13 +0000 (17:47 +0800)]
test(file_store): `last_write_is_short`
This test simulates a situation where the last write to the db is short.
Aggregating the changeset after reopening the file should return an
error (which includes a partially-aggregated changeset) containing an
aggregation of changesets that were fully written.
At this point, the test re-writes the final changeset (and this time it
successfully writes in full).
The file should be recoverable with all changesets, including the last
one.
feat(file_store)!: optimize `EntryIter` by reducing syscalls
* Wrap file reader with `BufReader`. This reduces calls to `read`.
* Wrap file reader with `CountingReader`. This counts the bytes read by
the underlying reader. We can rewind without seeking first.
<!-- 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 -->
This should fix #1252
### 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
As a part of #1203, I found myself having to modify `reveal_to_target`, but had some troubles understanding exactly what the method was doing.
This PR refactors `reveal_to_target` to be, in my opinion, a bit clearer. We now exist prematurely if `next_reveal_index` < `target_index`; this way, we don't need to keep track of `next_reveal_index`, as it would always be equal to `Some(target_index)`.
As a part of trying to understand `reveal_to_target` I had to read through `next_store_index`, I slightly improved it for clarity reasons as well.
### Changelog notice
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
These crates need to have their versions bumped because the version of bdk_chain was bumped to 0.7.0 with the 1.0.0-alpha.3 release.
bdk_esplora to 0.5.0
bdk_file_store to 0.3.0
After this PR is merged I'll publish new versions of these crates to crates.io.
### Notes to the reviewers
I should have done this as part of the 1.0.0-alpha.3 release process. This problem was found when @thunderbiscuit started updating the bdk-ffi project to alpha.3 and we got type mismatch errors for bdk_chain types used in bdk_esplora.
Simplify the `reveal_to_target` algorithm by exiting prematurely if
the `target_index` is already revealed.
Since the `reveal_to_index` variable was different from
`Some(target_index)` only if the target was already revealed, we can
getrid of the variable altogether.
This commit moves the `hardwaresigner` outside of bdk and inside `bdk_hwi`
### Notes to the reviewers
There are currently two issues with the code:
- `TransactionSigner` dictates that `sign_transaction` must return a `SignerError` - but being `SignerError` defined inside of bdk, we can't modify it to include an hwi specific error! I don't know how we could fix this (other than getting rid of the trait altogether :)); for now I just added a `SignerError::Generic` variant, lmk if you have better ideas!
- The hwi tests used the bdk utils to get a funded wallet for testing, which aren't available in `bdk_hwi`, which made me realize - maybe we should expose them so that we can use them across our crates, and also our users can use them to test their code?
For now, I just left the test commented.
### Changelog notice
- The old `hardwaresigner` module has been moved out of `bdk` and inside `bdk_hwi`.
### 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
- Bump bdk version to 1.0.0-alpha.3
- Bump bdk_chain to 0.7.0
- Bump bdk_bitcoin_rpc to 0.2.0
- Bump bdk_electrum to 0.5.0
### Notes to the reviewers
### Changelog notice
See #1254
### 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
We previously bumped the `bip39` version to 2.0 [in the 0.2X release branch](https://github.com/bitcoindevkit/bdk/pull/875). Back then, bumping it in `master` was [assumed unnecessary](https://github.com/bitcoindevkit/bdk/pull/875#issuecomment-1483990088). It seems this was erroneous, as the current `1.0.1` dependency is present since https://github.com/bitcoindevkit/bdk/pull/793, which was merged before the bump in `release/0.27`.
Here, we therefore bump the crate version in `master` after all.
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
projects / bdk / log