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.
df74b23f315007cc36d3a12904380f861326fb32 Pin base64 to 0.21.0 to keep the MSRV to 1.57.0 (Daniela Brozzoni)
Pull request description:
Fixes#986
### Changelog notice
- Pin base64 to 0.21.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
ACKs for top commit:
notmandatory:
ACK df74b23f315007cc36d3a12904380f861326fb32
Tree-SHA512: fb11db0771542b074b7605e4af9ec655c22106b1d3451e27103d2f81686270efb44d8687f8312b5b7ab724f03e8f24c349a9f4f3b6e0c89ed9550b121f526ce7
c7a045fa54a8183899d90730249c06c3f538b027 Fixed typos in CONTRIBUTING.md (Jon Marrs)
Pull request description:
### Description
Fixed typos in CONTRIBUTING.md
### 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 c7a045fa54a8183899d90730249c06c3f538b027.
Tree-SHA512: de66f4be7c7fd608f5a74e51c5666f4fe6b962c5a8345c08c7b1da2551f81b9adfb28c58f0991ac6c985fa5c31f3eb7d17f99edf136b04945cefa24b28eaee19
ed89de752cf20cb80deefda3ddf65a137b669288 Improve txout filter/listing method docs for `TxGraph` (志宇)
fb75aa94a9f47d53ce7246c14d739fc53dc38197 Clarify `TxGraph::try_filter_chain_unspents` logic (志宇)
96b10751325dc1bced09f9e864438216400e4ba3 Fix and improve `Persist::commit` method (志宇)
e01d17d59bd6177bf5e457232219669782b727b7 Improve `txout` listing and balance APIs for redesigned structures (志宇)
Pull request description:
### Description
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.
#### Other Fixes
The `Persist::commit` method is fixed in 96b10751325dc1bced09f9e864438216400e4ba3.
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
ACKs for top commit:
LLFourn:
ACK ed89de752cf20cb80deefda3ddf65a137b669288
Tree-SHA512: efae18c13ee74eff801febca61cb16bf4d8f3203ced96172b9ef555d8d2a749a382f87d024e8c92aacbab7eb4e50a8337de798e10524291ad80c6b35c4dc0161
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.
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.
4963240599364c6520d0a6ecae97db77cc7b8ba8 Add more `impl`s for `Append` and docs for file store `magic` (志宇)
2aa08a5898545f670df9ed9c4804231f321d811a [persist_redesign] Introduce redesigned `persist` types (志宇)
Pull request description:
### Description
This is part of #895 and #971
* 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
Top commit has no ACKs.
Tree-SHA512: 0211fbe7d7e27805d3ed3a80b42f184cdff1cebb32fd559aa9838e4a7f7c7e47b6c366b6ef68e299f876bafed549b8d1d8b8cc0366bf5b61db079504a565b9b4
This is a more generic version of `keychain::persist::*` structures.
Additional changes:
* The `Append` trait has a new method `is_empty`.
* Introduce `Store` structure for `bdk_file_store`.
* 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.
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.
`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.
SpkIterator was created with its own nth() and next() implementations
and its own new() and new_with_range() constructors.
Co-authored-by: 志宇 <hello@evanlinjin.me>
b799a5728b7e08a18059ffee7fe7ee6078354977 [bdk_chain_redesign] Add tests for `IndexedTxGraph` with `LocalChain` (rajarshimaitra)
8cd0328eec0b55f10dd085597a435b689b37444a [bdk_chain_redesign] Implement `OwnedIndexer` for indexers (rajarshimaitra)
911af34f509ad0b4c86c0be819d79b202679e3e5 [bdk_chain_redesign] Fix calculation bugs. (rajarshimaitra)
e536307e5c5532fc0cac0657a26609cfd26115bf [bdk_chain_redesign] Fix `tx_graph::Additions::append` logic (志宇)
f101dde09b6067c17b0b64ea3e3efd358a32a820 [bdk_chain_redesign] Fix `tx_graph::Additions::append` logic (志宇)
1b152647c557d2ff492cd241999c5320f7c4f6c5 [bdk_chain_redesign] Change `insert_relevant_txs` method (志宇)
ecc74ce4cd1ab0c51c2e9bcaa4f7d53390dd4d9b [bdk_chain_redesign] Docs for `is_mature` and `is_confirmed_and_spendable` (志宇)
ac336aa32f485cb253ac7ea5cae3b4bcc78cf507 [bdk_chain_redesign] Make `insert_relevant_txs` topologically-agnostic (志宇)
165b874dfedb2e05a7ea5d56e6f80577ada48d73 [bdk_chain_redesign] Add test for `insert_relevant_txs` (志宇)
f3e7b67bf1195eadf48a8f26f960b3f39d6134f7 [bdk_chain_redesign] Various tweaks and fixes (志宇)
03c128311a687249238fb109f804cde25ffddad2 [bdk_chain_redesign] Revert changes to `SparseChain` (志宇)
34a7bf5afe2d78db47b149f4958f0a0365b017d0 [bdk_chain_redesign] Rm unnecessary code and premature optimisation (志宇)
6c495707423cc7ce26c3027893dc812281b73053 [bdk_chain_redesign] Rm `HashSet` from `TxGraph::relevant_heights` (志宇)
1003fe2ee6167e110b0195e2431560b5b222e2f1 [bdk_chain_redesign] Test `LocalChain` (志宇)
7175a82c04b0a3f0c5716f4a37e122da70b8ceac [bdk_chain_redesign] Add tests for `TxGraph::relevant_heights` (志宇)
8e36a2e5f6d0fe2813dca9a13005e0b1be7e94c3 [bdk_chain_redesign] Remove incomplete logic (志宇)
81436fcd72a3c45bb10a098be28de0116322d22d [bdk_chain_redesign] Fix `Anchor` definition + docs (志宇)
001efdd1cb658f3a2929ffcf9047e6b07e9bd15c Include tests for new updates of TxGraph (rajarshimaitra)
10ab77c549e597d0d8157d94ae6fa3b4d39fd5dc [bdk_chain_redesign] MOVE `TxIndex` into `indexed_chain_graph.rs` (志宇)
7d92337b932fdcfec7008da8ed81f2b4b6e7a069 [bdk_chain_redesign] Remove `IndexedTxGraph::last_height` (志宇)
a7fbe0ac672cde6c308737fc98020f6693071a5f [bdk_chain_redesign] Documentation improvements (志宇)
ee1060f2ff168e6aaffa41882be2b319729f7de8 [bdk_chain_redesign] Simplify `LocalChain` (志宇)
611d2e3ea2ed9249ddf04e0f9089642160e5c901 [bdk_chain_redesign] Consistent `ChainOracle` (志宇)
bff80ec378fab29556099f9830bcb42911658710 [bdk_chain_redesign] Improve `BlockAnchor` docs (志宇)
24cd8c5cc7f3a6bd0db2bd45642f08a28ea5337a [bdk_chain_redesign] More tweaks and renamings (志宇)
ddd5e951f5ec77070034c7390a635d8d5bd7cb85 [bdk_chain_redesign] Modify signature of `TxIndex` (志宇)
da4cef044d4a3ad0f44ff1e33936c93c38c2f774 [bdk_chain_redesign] Introduce `Append` trait for additions (志宇)
89cfa4d78e059f9fe2544b690bbbf90e92b3efee [bdk_chain_redesign] Better names, comments and generic bounds (志宇)
6e59dce10b66212d7180cadabba887cc4d20fc32 [bdk_chain_redesign] `chain_oracle::Cache` (志宇)
a7eaebbb77f8794c5ff3717aaf0cf73dd5a77480 [bdk_chain_redesign] Add serde support for `IndexedAdditions` (志宇)
c09cd2afce4e649caa2797628edaffae08a60628 [bdk_chain_redesign] Added methods to `LocalChain` (志宇)
7810059ed0f23cae7dee61fe587a1c8f3f49480a [bdk_chain_redesign] `TxGraph` tweaks (志宇)
a63ffe97397cd14bc0a13ea5e96ceddf8b63a4f0 [bdk_chain_redesign] Simplify `TxIndex` (志宇)
a1172def7df478c61076b0d99d5d0f5f9cd99da6 [bdk_chain_redesign] Revert some API changes (志宇)
8c906170c96919cd8c65e306b8351fe01e139fd4 [bdk_chain_redesign] Make default anchor for `TxGraph` as `()` (志宇)
468701a1295c90761749e1bb46cc201cc7f95613 [bdk_chain_redesign] Initial work on `LocalChain`. (志宇)
34d0277e44fd054c8d463dfa756d8531ccda3ca9 [bdk_chain_redesign] Rm anchor type param for structs that don't use it (志宇)
3440a057110fbbcc653f2f8c7d58175472299bae [bdk_chain_redesign] Add docs (志宇)
236c50fa7bace29a0373dd16416ecebbb6dc1ae8 [bdk_chain_redesign] `IndexedTxGraph` keeps track of the last synced height (志宇)
e902c10295ba430bf0522b92ffab68cd60bd1666 [bdk_chain_redesign] Fix `apply_additions` logic for `IndexedTxGraph`. (志宇)
313965d8c84f5de43cafa58c7bd9250aea93b22c [bdk_chain_redesign] `mut_index` should be `index_mut` (志宇)
db7883d813e97229340c32a8fa82a9a13bac7361 [bdk_chain_redesign] Add balance methods to `IndexedTxGraph` (志宇)
d0a2aa83befce5dda26f8b3ae05449f6967df25a [bdk_chain_redesign] Add `apply_additions` to `IndexedTxGraph` (志宇)
6cbb18d409d84ea0c399d9b3ecb0cdb49cc0b32e [bdk_chain_redesign] MOVE: `IndexedTxGraph` into submodule (志宇)
784cd34e3db727659dbb26c428ed9096927286c1 [bdk_chain_redesign] List chain data methods can be try/non-try (志宇)
43b648fee02291858dfcab9b639c55a0bc3fad81 [bdk_chain_redesign] Add `..in_chain` methods (志宇)
61a8606fbcaec933f915c4f0600cd6f5e35636e8 [bdk_chain_redesign] Introduce `ChainOracle` and `TxIndex` traits (志宇)
5ae5fe30ebd53d72fe567509506ae0cda7a3a244 [bdk_chain_redesign] Introduce `BlockAnchor` trait (志宇)
Pull request description:
### Description
This is part of #895
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 b799a5728b
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.
Tree-SHA512: 8c386354cbd02f0701b5134991b65d206a54d19a2e78ab204e6ff1fa78a18f16299051bc0bf4ff4d2f5a0adab9b15658fa53cd0de2ca16969f4bf6a485225082
* `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.
Instead of forcing all transactions inserted to use the same anchors, we
change the API to have unique anchors per transaction.
This allows for more flexibility in general. For example, use `Anchor`
implementations that contain the position in a block of a transaction.
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.
* Remove `chain_oracle::CacheBackend` for now as it is not used.
* `SparseChain` does not need to implement `ChainOracle`.
* Remove filter predicate for `list..` methods of `TxGraph` and
`IndexedTxGraph` as this is premature optimisation.
* `Append` can be implemented for all `BTreeMap`s and `BTreeSet`s,
instead of only `local_chain::ChangeSet`.
The `HashSet` was used for iterating without duplicate items. However,
since `anchors` is a `BTreeSet`, heights are in order. So a single
variable tracking last height will be sufficient.
This is mostly copying over the relevant tests from `SparseChain`.
Changes are made to `local_chain::ChangeSet` to re-add the ability to
remove blocks.
Previously, I have misunderstood the definition of anchor. If a tx is
anchored in a block, it does not necessarily mean it is confirmed in
that block. The tx can be confirmed in an ancestor block of the anchor
block.
With this new definition, we need a new trait `ConfirmationHeight` that
has one method `confirmation_height`. This trait can be used to extend
`Anchor` for those implementations that can give us the exact
conirmation height of a tx (which is useful in most cases).
Another change is to add another variant to the `ObservedAs` enum;
`ObservedAs::ConfirmedImplicit(A)`. If a tx does not have an anchor, but
another tx that spends it has an anchor that in in the best chain, we
can assume that tx is also in the best chain. The logic of
`TxGraph::try_get_chain_position` is also changed to reflect this.
Some methods from `IndexedTxGraph` have been moved to `TxGraph` as they
do not require the `Indexer`. Some `TxGraph` methods have been renamed
for clarity and consistency.
Also more docs are added.
ebd6103e6597e0a66470541d2fcd25709ea77866 Fix policy condition calculation (Alekos Filini)
Pull request description:
### Description
When constructing the `Condition` struct we recursively call `get_condition` on all the items in a threshold and short-circuit if there's an error somewhere (for example, because the policy-path hasn't been provided for a specific threshold).
This can cause issues when the user doesn't care about a subtree, because we still try to call `get_condition` on all the items and fail if something is missing, even if the specific subtree isn't selected and won't be used later on.
This commit changes the logic so that we first filter only the `selected` items, and then unwrap the error using the question mark. If errors happened somewhere else they will be ignored, as they should.
### Notes to the reviewers
I think it makes sense to backport this to `0.27`: even though it's not a critical issue (and there's a workaround[^1] for the bug) it may be a while before the new `1.0` is released. I wouldn't do a release just for this, but I would just leave it there and maybe in a few weeks if there are other fixes to be backported to pre-1.0 they could all be released.
### Changelog notice
- Fixed a bug in the policy condition calculation
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [ ] I'm linking the issue being fixed by this PR
[^1]: The workaround is to simply set the items in the policy tree even if they won't be used. For example, if the item causing troubles is a `thresh(1, ...)` just set `[0]` in the policy path for that id.
ACKs for top commit:
notmandatory:
ACK ebd6103e6597e0a66470541d2fcd25709ea77866
Tree-SHA512: 1e0c6140e47ead3a518656cab660932b40f9e67425ce144f072267c30fd5cae5923ba943d9682af050c8f314756c522260abb68b104b9eeaae597319612984db
