Although there is *some* code to handle multipath keys inside bdk,
it's all untested, and from a few quick tests it
seems that it's pretty easy to find buggy edge cases.
Better to deny multipath descs for now, and revisit the
decision once we work on supporting multidescriptor wallets.
This PR changes the `LocalChain` implementation to have blocks stored as a linked-list. This allows the data-src thread to hold a shared ref to a single checkpoint and have access to the whole history of checkpoints without cloning or keeping a lock on `LocalChain`.
The APIs of `bdk::Wallet`, `esplora` and `electrum` are also updated to reflect these changes. Note that the `esplora` crate is rewritten to anchor txs in the confirmation block (using the esplora API's tx status block_hash). This guarantees 100% consistency between anchor blocks and their transactions (instead of anchoring txs to the latest tip). `ExploraExt` now has separate methods for updating the `TxGraph` and `LocalChain`.
A new method `TxGraph::missing_blocks` is introduced for finding "floating anchors" of a `TxGraph` update (given a chain).
Additional changes:
* `test_local_chain.rs` is refactored to make test cases easier to write. Additional tests are also added.
* Examples are updated.
* Exclude example-cli `*.db` files in `.gitignore`.
* Rm duplicate `bdk_tmp_plan` module.
### Notes to the reviewers
This is the smallest possible division of #1002 without resulting in PRs that do not compile. Since we have changed the API of `LocalChain`, we also need to change `esplora`, `electrum` crates and examples alongside `bdk::Wallet`.
### Changelog notice
* Implement linked-list `LocalChain`. This allows the data-src thread to hold a shared ref to a single checkpoint and have access to the whole history of checkpoints without cloning or keeping a lock on `LocalChain`.
* Rewrote `esplora` chain-src crate to anchor txs to their confirmation blocks (using esplora API's tx-status `block_hash`).
### 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
Update CONTRIBUTING.md to specify that new commits should use 'Conventional Commits 1.0' style messages. See https://www.conventionalcommits.org/en/v1.0.0/. I also fixed our current MSRV in the doc and fixed a spelling error.
### Notes to the reviewers
I'm open to suggestions if anyone has another well documented style for commit messages.
### Changelog notice
None.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
thunderbiscuit:
ACK e17f03e7555b7b6f5348b9854f3d4ef559e84d75. I've been using conventional commits for Padawan for over 2 years and think it's got good value, particularly on projects with bigger teams and number of contributors.
danielabrozzoni:
ACK e17f03e7555b7b6f5348b9854f3d4ef559e84d75 - I was hesitant at first, but I think this would make the commit messages cleaner (and in most cases even shorter). Going forward we should consider adding a CI check enforcing this.
* Rename `LocalUpdate::keychain` to `LocalUpdate::last_active_indices`.
* Change docs for `CheckPoint` to be more descriptive.
* Fix incorrect logic in `update_local_chain` for `EsploraExt` and
`EsploraAsyncExt`.
* Improve/fix `LocalChain` documentation
* Refactor `TxGraph::missing_blocks` to make it more explicit that
`last_block` has state.
* `update_local_chain` method of `EsploraExt` and `EsploraAsyncExt` now
returns a `local_chain::Update` instead of just a `CheckPoint`.
Implement linked-list `LocalChain` and update chain-src crates/examples
This commit changes the `LocalChain` implementation to have blocks
stored as a linked-list. This allows the data-src thread to hold a
shared ref to a single checkpoint and have access to the whole history
of checkpoints without cloning or keeping a lock on `LocalChain`.
The APIs of `bdk::Wallet`, `esplora` and `electrum` are also updated to
reflect these changes. Note that the `esplora` crate is rewritten to
anchor txs in the confirmation block (using the esplora API's tx status
block_hash). This guarantees 100% consistency between anchor blocks and
their transactions (instead of anchoring txs to the latest tip).
`ExploraExt` now has separate methods for updating the `TxGraph` and
`LocalChain`.
A new method `TxGraph::missing_blocks` is introduced for finding
"floating anchors" of a `TxGraph` update (given a chain).
Additional changes:
* `test_local_chain.rs` is refactored to make test cases easier to
write. Additional tests are also added.
* Examples are updated.
* Fix `tempfile` dev dependency of `bdk_file_store` to work with MSRV
* [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
Adding it fails with "The repository'http://apt.llvm.org/focal
llvm-toolchain-focal Release' does not have a Release file.", and for
some reason it's not needed anymore.
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
Steve Myers [Wed, 19 Jul 2023 21:15:35 +0000 (16:15 -0500)]
docs: add required style for commit messages
Update CONTRIBUTING.md to specify that new commits should use 'Conventional Commits 1.0'
style messages. See https://www.conventionalcommits.org/en/v1.0.0/.
Carrying over relevant maintenance changes from release 0.27.2 to bdk `master`.
- Use `default-features = false` for `miniscript` and `bitcoin`
- Introduce `std` features for `bdk`, `bdk_chain` and `bdk_esplora`
### Notes to the reviewers
The `default-features = false`for `bitcoin` and `miniscript` is to let `bdk` be unbiased for things like no-std.
### Changelog notice
- Set default-features = false for rust-bitcoin and rust-miniscript #894
- Introduce `std` features for `bdk`, `bdk_chain` and `bdk_esplora`
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
This PR solves #836. This PR adds a P2TR
descriptor template and a BIP86 taproot
descriptor template. With this, users
can now create a taproot descriptor with 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)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
#### Bugfixes:
* [ ] 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
* [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
* Update `Wallet` to use redesigned structures.
* Update `bdk_electrum::ElectrumExt` to produce updates for redesigned structures.
* Update `bdk_esplora::EsploraExt` and `bdk_esplora::EsploraAsyncExt` to produce updates for redesigned structures.
* Added `example-crates/example_cli` library for implementing examples with redesigned structures.
* Added `example-crate/example_electrum` which is an electrum CLI wallet using the redesigned structures.
* Updated `example-crate/{wallet_electrum|wallet_esplora|wallet_esplora_async}` examples to use redesigned structures.
* Remove all old structures.
### Notes to the reviewers
~These changes bump our `all-features` MSRV to `1.60.0` because of the introduction of `bdk_esplora`. As long as the `bdk_chain` and `bdk_wallet` crates hit a MSRV of `1.48.0`, it will be fine (this work is done in #987).~ No longer needed due to #993
~I had to comment out the examples that use `Wallet` with our chain sources. Once we update the helper-packages for those chain sources, we can also update the examples.~
Possible future improvements for `ElectrumExt`:
* Remove requirement to retry obtaining ALL data after reorg is detected. Transactions can be anchored to a lower block (not block tip), and an `assume_final_depth` value can be used.
* The logic to finalize an update with confirmation time can be improved during reorgs to not require returning an error.
* Use the subscription model of electrum, as intended by the API.
### 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
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
志宇 [Wed, 24 May 2023 03:37:26 +0000 (11:37 +0800)]
[chain_redesign] Remove old structures
Other changes:
* The `async-https` feature of `bdk_esplora` is no longer default.
* Rename `ObservedAs` to `ChainPosition`.
* Set temporary MSRV to 1.60.0 to compile all workspace members will all
features.
志宇 [Thu, 18 May 2023 02:02:23 +0000 (10:02 +0800)]
`bdk_electrum` API improvements and simplifications
* `ElectrumUpdate::missing_full_txs` now returns a `Vec<Txid>` so we
don't keep a reference to the passed-in `graph`.
* `ElectrumUpdate::finalize*` methods now takes in `missing` txids
instead of `full_txs`. `Client::batch_transaction_get` is called
within the methods.
Other changes:
* `wallet::ChangeSet` is now made public externally. This is required as
a wallet db should implement `PersistBackend<wallet::ChangeSet>`.
There are a number of improvements that can be done, but it is in a
decent state to be usable.
Possible improvements:
* Remove requirement to retry obtaining ALL data after reorg is
detected. Transactions can be anchored to a lower block (not block
tip), and an `assume_final_depth` value can be used.
* The logic to finalize an update with confirmation time can be improved
during reorgs to not require returning an error.
* [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
base64 lowered the MSRV to 1.57.0 in version 0.21.2
### 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
Vladimir Fomene [Tue, 14 Feb 2023 22:16:02 +0000 (01:16 +0300)]
Create taproot descriptor template
This PR solves #836. This PR adds a P2TR
descriptor template and a BIP86 taproot
descriptor template. With this, users
can now create a taproot descriptor with templates.
* [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 noted in https://github.com/bitcoindevkit/bdk/issues/971#issuecomment-1542408941.
Instead of relying on a `OwnedIndexer` trait to filter for relevant txouts, we move the listing and balance methods from `IndexedTxGraph` to `TxGraph` and add an additional input (list of relevant outpoints) to these methods.
This provides a simpler implementation and a more flexible API.
Previously, regardless of whether writing to persistence backend is successful or not, the logic always cleared `self.staged`. This is changed to only clear `self.staged` after successful write.
Additionally, the written changeset (if any) is returned, and `PersistBackend::write_changes` will not be called if `self.staged` is empty.
### Notes to the reviewers
Yes, slightly more boilerplate to do the same things, but less code to maintain and a much more flexible API. Very worth it IMO.
### 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
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
志宇 [Thu, 11 May 2023 03:49:33 +0000 (11:49 +0800)]
Fix and improve `Persist::commit` method
Previously, regardless of whether writing to persistence backend is
successful or not, the logic always cleared `self.staged`. This is
changed to only clear `self.staged` after successful write.
Additionally, the written changeset (if any) is returned, and
`PersistBackend::write_changes` will not be called if `self.staged` is
empty.
志宇 [Wed, 10 May 2023 06:14:29 +0000 (14:14 +0800)]
Improve `txout` listing and balance APIs for redesigned structures
Instead of relying on a `OwnedIndexer` trait to filter for relevant
txouts, we move the listing and balance methods from `IndexedTxGraph` to
`TxGraph` and add an additional input (list of relevant outpoints) to
these methods.
This provides a simpler implementation and a more flexible API.
* Introduce a more generic version of the `keychain::persist::*` structures that only needs a single generic for the changeset type.
Additional changes:
* The `Append` trait has a new method `is_empty`.
* Introduce `Store` structure for `bdk_file_store` (which implements `PersistBackend`).
### 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
#### New Features:
* [x] I've added tests for the new feature
* [x] I've added docs for the new feature
[chain_redesign] Various `LocalChain` improvements
* Introduce `LocalChain::inner` method to get the inner map of block
height to hash.
* Replace `LocalChain::get_block` (which outputted `BlockId`, thus able
to return invalid representation) with `get_blockhash` that just
returns a `BlockHash`.
* Remove `TODO` comments that should be github tickets.
[chain_redesign] `BlockId` should not implement `Anchor`
If `BlockId` implements `Anchor`, the meaning is ambiguous. We cannot
tell whether it means the tx is anchors at the block, or whether it also
means the tx is confirmed at that block.
Instead, `ConfirmationHeightAnchor` and `ConfirmationTimeAnchor` structs
are introduced as non-ambiguous `Anchor` implementations.
Additionally, `TxGraph::relevant_heights` is removed because it is also
ambiguous. What heights are deemed relevant? A simpler and more flexible
method `TxGraph::all_anchors` is introduced instead.
[chain_redesign] Change behavior of `try_get_chain_position`
`TxGraph::try_get_chain_position` used to always exclude unconfirmed
transactions with last_seen value of 0. However, what is the point of
including a transaction in the graph if it cannot be part of the chain
history? Additionally, maybe sometimes we don't wish to use the
last_seen field at all.
The new behavior will consider unconfirmed transactions with last_seen
of 0.
The initial `bdk_chain` structures allowed updating to be done without blocking IO (alongside many other benefits). However, the requirement to have transactions "perfectly positioned" in our `SparseChain` increased complexity of the syncing API. Updates needed to be meticulously crafted to properly connect with the original `SparseChain`. Additionally, it forced us to keep a local copy of the "best chain" data (which may not always be needed depending on the chain source).
The redesigned structures, as introduced by this PR, fixes these shortcomings.
1. Instead of `SparseChain`, we introduce the ability to `Anchor` a transaction to multiple blocks that may or may not be in the same chain history. We expand `TxGraph` to records these anchors (while still maintaining the *monotone* nature of `TxGraph`). When updating our new `TxGraph`, we don't need to complicated *are-we-connected* checks that we need for `SparseChain`.
2. The chain source, in combination with our new`TxGraph` is all that we need to determine the "chain position" of a transaction. The chain source only needs to implement a single trait `ChainOracle`. This typically does not need to be updated (as it is remote), although we can have a special `ChainOracle` implementation that is stored locally. This only needs block height and hash information, reducing the scope of *non-monotine* structures that need to be updated.
**What is done:**
* `TxGraph` includes anchors (ids of blocks that the tx is seen in) and last-seem unix timestamp (for determining which non-confirmed tx we should consider as part of "best chain" if there are conflicts). This structure continues to be fully "monotone"; we can introduce data in any manner and not risk resulting in an inconsistent state.
* `LocalChain` includes the "checkpoint" logic of `SparseChain` but removes the `txid` data. `LocalChain` implements the `ChainOracle` trait. Any blockchain-source can also implement the `ChainOracle` trait.
* `IndexedTxGraph` is introduced and contains two fields; an internal `TxGraph` struct and a `TxIndex` implementation. These two fields will be updated atomically and can replace the functionality of `keychain::Tracker`.
**What is in-progress:**
* ~@evanlinjin: The `TxIndex` trait should not have `is_{}_relevant` methods as we cannot guarantee this across all transaction indexes. We should introduce extension traits for these (https://github.com/bitcoindevkit/bdk/pull/926#discussion_r1159286393).~
* ~@evanlinjin: `BlockAnchor` should be defined as "if this block is in chain, then this tx must be in chain". Therefore, the anchor does not provide us with the exact confirmation height of the tx. We need to introduce an extension trait `ExactConfirmationHeightAnchor` for certain operations (https://github.com/bitcoindevkit/bdk/pull/926#discussion_r1155480352).~
**What will be done external to this PR:**
* @rajarshimaitra: Persisting `indexed_tx_graph::Additions` (#937).
* @notmandatory: Update examples to use redesigned structures.
* Update `bdk::Wallet` to use the redesigned structures.
### Changelog notice
* Initial implementation of the `bdk_chain` redesign (as mentioned in #895). Currently, only the `bdk_chain` core structures are implemented. Persistence and updates to the current examples and `bdk::Wallet` will be done in separate PRs.
### 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
ACKs for top commit:
rajarshimaitra:
ACK https://github.com/bitcoindevkit/bdk/pull/926/commits/b799a5728b7e08a18059ffee7fe7ee6078354977
danielabrozzoni:
Partial ACK b799a5728b7e08a18059ffee7fe7ee6078354977 - good job! I haven't looked at the tests or at the methods implementations in depth, but I have reviewed the architecture and it looks good. I have a few non-blocking questions.
* `IndexedTxGraph::try_balance` should include "confirmed and spendable"
into `confirmed` balance.
* `TxGraph::try_list_chain_unspents` filter logic should be reversed.
* `Additions` now implements `Append` and uses `Append` to implement
`append()`.
* `append()` logic enforces that `last_seen` values should only
increase.
* Test written for `append()` with `last_seen` behaviour.
[bdk_chain_redesign] Make `insert_relevant_txs` topologically-agnostic
The `insert_relevant_txs` test has also been changed to used
`KeychainTxOutIndex` so that index additions can be checked
(`SpkTxOutIndex` has no additions).
Additionally, generic bounds of some `IndexedTxGraph` list methods have
been fixed.
The `ConfirmationHeight` trait has been removed in favour of a second
method on the `Anchor` trait: `confirmation_height_upper_bound()`.
Methods `try_balance_at()` and `balance_at()` of `IndexedTxGraph` have
been removed as they do not provide additional functionality.
`IndexedTxGraph::insert_relevant_txs` now uses two loops, the first loop
indexes all transactions first. This is done as some indexes require
ancestor transactions to be indexed first and we cannot guarantee that
the input transactions are in topological order.
projects / bdk / log