志宇 [Wed, 25 Oct 2023 22:20:37 +0000 (06:20 +0800)]
feat(bdk)!: have separate methods for creating and loading `Wallet`
`Wallet::new` now creates a new wallet. `Wallet::load` loads an existing
wallet. The network type is now recoverable from persistence. Error
types have been simplified.
Fixes #1118.
Adds `dependabot.yml` to `.github/` to check for `"github-action"`
updates on a `"weekly"` basis.
This does not touch Rust code or Cargo workflows.
It will create PRs and we would need to approve them
(they would be subject to the same merge policy)
to instantiate the proposed dependabots into `master`.
Fixes #1144.
Coinbase transactions cannot exist in the mempool and be unconfirmed. `TxGraph::try_get_chain_position` should always return `None` for coinbase transactions not anchored in best chain.
### 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
Closes #1187.
An `Anchor` implementation that records both height and time should have both attributes included in the name.
### 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 would previously sign with whatever x_only_pubkey we had in hand, without first checking if it was the right key or not. This effectively meant that adding multiple taproot PrivateKey signers would produce unbroadcastable transactions.
### Changelog notice
- Fix a bug related to taproot signing with internal keys. We would previously sign with the first private key we had, without checking if it was the correct internal key or not.
### 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
This may or may not fix #1125.
Fixed what appeared to be a logic error in `construct_update_tip` in `electrum_ext.rs` that caused the local chain tip to always be a block behind the newest confirmed block.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [ ] This pull request breaks the existing API
* [x] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
ACK 1010efd8d68e886cc46f0ac2f016630b670ea73c - although I've been able to reproduce the issue in #1125, I'm convinced that this PR fixes at least a bug, as demonstrated in #1171 (yet to be reviewed and merged).
Wei Chen [Tue, 3 Oct 2023 10:06:53 +0000 (18:06 +0800)]
fix(electrum): fixed chain sync issue
Fixed a logic error in `construct_update_tip` in `electrum_ext.rs` that caused
the local chain tip to always be a block behind the newest confirmed block.
Wei Chen [Thu, 9 Nov 2023 21:34:08 +0000 (05:34 +0800)]
fix(chain): filter coinbase tx not in best chain
Coinbase transactions cannot exist in the mempool and be unconfirmed.
`TxGraph::try_get_chain_position` should always return `None` for coinbase
transactions not anchored in best chain.
fix(bdk): Check if we're using the correct...
...internal key before signing
Fixes #1142
We would previously sign with whatever x_only_pubkey we had in hand,
without first checking if it was the right key or not. This effectively
meant that adding multiple taproot PrivateKey signers would produce
unbroadcastable transactions.
Fixes #1102. If a conflicting tx has the same `last_seen`, then we check lexicographical sorting of txids.
### Notes to the reviewers
The tests for this fix exist in the `TxTemplate` structure in #1064 which may need to be merged first.
### 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
- rename `TxGraph::direct_conflicts_of_tx` to `TxGraph::direct_conflicts`
### 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 builds on top of #1034 and adds the `bitcoind_rpc` chain-src module and example.
### Notes to the reviewers
Don't merge this until #1034 is in!
### Changelog notice
* Add `bitcoind_rpc` chain-source module.
* Add `example_bitcoind_rpc` example module.
* Add `AnchorFromBlockPosition` trait which are for anchors that can be constructed from a given block, height and position in block.
* Add helper methods to `IndexedTxGraph` and `TxGraph` for batch operations and applying blocks directly.
* Add helper methods to `CheckPoint` for easier construction from a block `Header`.
### Checklists
* [x] Add test: we should detect when an initially-confirmed transaction is "unconfirmed" during a reorg.
* [x] Improve `example_bitcoind_rpc`: add `live` command.
* [x] Improve docs.
* [x] Reintroduce `CheckPoint`.
#### 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
When wallet_esplora_* was used to sync a wallet containing a transaction confirmed some time ago (more than 10-15 blocks ago), the transaction would be stuck in an "unconfirmed" state forever.
At the first scan time, `update_local_chain` would just fetch the last 10 to 15 blocks (depending on the server used), and `tx_graph.missing_heights` wouldn't return the tx's confirmation block as it was called on the original, non-updated tx_graph.
So, after the first scan, we would have a transaction in memory with an anchor that doesn't exist in our local_chain, and try_get_chain_position would return unconfirmed.
When scanning again, missing_heights would find the missing anchor, but `update_local_chain` wouldn't include it as it's older than ASSUME_FINAL_DEPTH.
The missing block would be downloaded every time, but never included in the local_chain, and the transaction would remain unconfirmed forever.
Here we call missing_heights on the updated graph, so that it can correctly return the anchor height, and `update_local_chain` can fetch it and include it in the chain.
### Notes to the reviewers
I'm not sure if this is the right approach, so I'm opening this PR to gather feedback. I still need to add tests, I'll do so once we decide if this is the right way to go or not.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [x] This pull request breaks the existing API
* [ ] I've added tests to reproduce the issue which are now passing
* [x] I'm linking the issue being fixed by this PR
* avoid holding mutex lock over io
* document `CHANNEL_BOUND` const
* use the `relevant` variant of `batch_insert_unconfirmed`
* print elapsed time in stdout for various updates
bitcoind_rpc!: bring back `CheckPoint`s to `Emitter`
* `bdk_chain` dependency is added. In the future, we will introduce a
separate `bdk_core` crate to contain shared types.
* replace `Emitter::new` with `from_height` and `from_checkpoint`
* `from_height` emits from the given start height
* `from_checkpoint` uses the provided cp to find agreement point
* introduce logic that ensures emitted blocks can connect with
receiver's `LocalChain`
* in our rpc example, we can now `expect()` chain updates to always
since we are using checkpoints and receiving blocks in order
bitcoind_rpc: rm `BlockHash` from `Emitter::last_mempool_tip`
Instead of comparing the blockhash against the emitted_blocks map
to see whether the block is part of the emitter's best chain, we
reduce the `last_mempool_tip` height to the last agreement height
during the polling logic.
The benefits of this is we have tighter bounds for avoiding re-
emission. Also, it will be easier to replace `emitted_blocks` to
a `CheckPoint` (since we no longer rely on map lookup).
chain: split `IndexedTxGraph::insert_tx` into 3 methods
Instead of inserting anchors and seen_at timestamp in the same method,
we have three separate methods. This makes the API easier to understand
and makes `IndexedTxGraph` more consistent with the `TxGraph` API.
chain: improvements to `IndexedTxGraph` and `TxGraph` APIs
For `IndexedTxGraph`:
- Remove `InsertTxItem` type (this is too complex).
- `batch_insert_relevant` now uses a simple tuple `(&tx, anchors)`.
- `batch_insert` is now also removed, as the same functionality can be
done elsewhere.
- Add internal helper method `index_tx_graph_changeset` so we don't need
to create a seprate `TxGraph` update in each method.
- `batch_insert_<relevant>_unconfirmed` no longer takes in an option of
last_seen.
- `batch_insert_unconfirmed` no longer takes a reference of a
transaction (since we apply all transactions anyway, so there is no
need to clone).
For `TxGraph`:
- Add `batch_insert_unconfirmed` method.
* `CheckPoint::from_header` allows us to construct a checkpoint from
block header.
* `CheckPoint::into_update` transforms the cp into a
`local_chain::Update`.
Fixes #1151.
When wallet_esplora_* was used to sync a wallet containing a transaction
confirmed some time ago (more than 10-15 blocks ago), the transaction would
be stuck in an "unconfirmed" state forever.
At the first scan time, `update_local_chain` would just fetch the last 10 to
15 blocks (depending on the server used), and `tx_graph.missing_heights`
wouldn't return the tx's confirmation block as it was called on the
original, non-updated tx_graph.
So, after the first scan, we would have a transaction in memory with an
anchor that doesn't exist in our local_chain, and try_get_chain_position
would return unconfirmed.
When scanning again, missing_heights would find the missing anchor, but
`update_local_chain` wouldn't include it as it's older than
ASSUME_FINAL_DEPTH.
The missing block would be downloaded every time, but never included in
the local_chain, and the transaction would remain unconfirmed forever.
Here we call missing_heights on the updated graph, so that it can
correctly return the anchor height, and `update_local_chain` can
fetch it and include it in the chain.
* [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
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Fixes #1063.
This PR introduces a new `TxTemplate` struct to test different transaction conflict scenarios in `TxGraph`.
The following transaction conflict scenarios are tested:
- 2 unconfirmed txs with different last_seens conflict. The most recent tx should be the only tx that appears in the list methods.
- 3 unconfirmed txs with different last_seens conflict. The most recent tx should be the only tx that appears in the list methods.
- An unconfirmed tx U conflicts with a tx anchored in orphaned block O. O has higher last_seen. O should be the only tx that appears in the list methods.
- An unconfirmed tx U conflicts with a tx anchored in orphaned block O. U has higher last_seen. U should be the only tx that appears in the list methods.
- Multiple unconfirmed txs conflict with a confirmed tx. None of the unconfirmed txs should appear in the list methods.
- B and B' conflict. C spends B. B' is anchored in best chain. B and C should not appear in the list methods.
- B and B' conflict. C spends B. B is anchored in best chain. B' should not appear in the list methods.
- B and B' conflict. C spends both B and B'. C is impossible.
- B and B' conflict. C spends both B and B'. C is impossible. B' is confirmed.
- B and B' conflict. C spends both B and B'. C is impossible. D spends C.
These tests revealed that `TxGraph::walk_conflicts` was not checking ancestors of the root tx for conflicts. `TxGraph::walk_conflicts` has been refactored to check for conflicting ancestor transactions by using a new `TxAncestors` iterator in `TxGraph`.
### Changelog notice
- Introduced `tx_template` module
- Introduced `TxGraph::TxAncestors` iterator
- Refactored `TxGraph::walk_conflicts` to use `TxGraph::TxAncestors`
- Added `walk_ancestors` to `TxGraph`
### 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
In try_get_chain_pos, when we notice that a transaction is not included
in the best chain, we check the transactions in mempool to find
conflicting ones, and decide based on that if our transaction is still
in mempool or has been dropped.
This commit adds a check for transactions conflicting with the
unconfirmed ancestors of our tx.
* [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
The tests could use some cleanup but get the job done for this PR.
### 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
#### 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
This pr helps solve this issue: https://github.com/bitcoindevkit/bdk/issues/856
I added documentation to the fee_rate() method to describe the units as either satoshis/vbyte (sats/vbyte) or satoshis/kwu (sats/kwu), depending on the FeeRate type.
I also added documentation to the fee_absolute() method to clarify that the fee is determined by whichever method (fee_rate or fee_absolute) was called last, as the FeePolicy is an enum, and FeeRate/FeeAmount are mutually exclusive.
### Notes to the reviewers
I thought it would be helpful to provide documentation to alleviate confusion over the fee_rate method and the fee_absolute method.
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
#### Bugfixes:
* [x] I'm linking the issue being fixed by this PR
I did a global spell check and found and fixed a few spelling errors.
### Notes to the reviewers
This is low priority but want to make sure it's done before we do a beta release.
### 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
- Move WalletUpdate to the wallet module
- Remove ForEachTxout trait completely
- Refactor ElectrumExt to not use WalletUpdate.
### 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
#### New Features:
* [ ] I've added tests for the new feature
* [ ] I've added docs for the new feature
ACKs for top commit:
realeinherjar:
ACK 2090021
danielabrozzoni:
ACK 20900218ceba728c8051a3c017a6341d74cbaee3 - code looks good, `example_esplora` and `wallet_esplora_blocking` work just fine.
志宇 [Sat, 26 Aug 2023 12:29:46 +0000 (20:29 +0800)]
feat(electrum)!: change signature of `ElectrumExt`
We remove `ElectrumUpdate` and return tuples instead for `ElectrumExt`
methods. We introduce the `IncompleteTxGraph` structure to specifically
hodl the incomplete `TxGraph`.
When you pass a non-wildcard descriptor in `new_with_range`, we make
sure that the range length is at most 1; if that's not the case, we
shorten it.
We would previously use `new_with_range` without this check and with a
non-wildcard descriptor in `spks_of_all_keychains`, this meant creating
a spkiterator that would go on producing the same spks over and over
again, causing some issues with syncing on electrum/esplora.
To reproduce the bug, run in `example-crates/example_electrum`:
```
cargo run "sh(wsh(or_d(c:pk_k(cPGudvRLDSgeV4hH9NUofLvYxYBSRjju3cpiXmBg9K8G9k1ikCMp),c:pk_k(cSBSBHRrzqSXFmrBhLkZMzQB9q4P9MnAq92v8d9a5UveBc9sLX32))))#zp9pcfs9" scan
```
### Changelog notice
- Fixed a bug where `KeychainTxOutIndex::spks_of_all_keychains`/`KeychainTxOutIndex::spks_of_keychain` would return an iterator yielding infinite spks even for non-wildcard 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
* [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
When you pass a non-wildcard descriptor in `new_with_range`, we make
sure that the range length is at most 1; if that's not the case, we
shorten it.
We would previously use `new_with_range` without this check and with a
non-wildcard descriptor in `spks_of_all_keychains`, this meant creating
a spkiterator that would go on producing the same spks over and over
again, causing some issues with syncing on electrum/esplora.
To reproduce the bug, run in `example-crates/example_electrum`:
```
cargo run "sh(wsh(or_d(c:pk_k(cPGudvRLDSgeV4hH9NUofLvYxYBSRjju3cpiXmBg9K8G9k1ikCMp),c:pk_k(cSBSBHRrzqSXFmrBhLkZMzQB9q4P9MnAq92v8d9a5UveBc9sLX32))))#zp9pcfs9" scan
```
projects / bdk / log