0010ecd94aae49f389093cd3e8983c405049dbf5 Add badge to README (wszdexdrf)
690411722e2131f69a605a6f3b5e97911778e9ed Change configs for source-base code coverage (wszdexdrf)
Pull request description:
### Description
This also changes the code coverage front end to coveralls instead of codecov, which had some issues with other changes in the PR. This will provide better and more accurate code coverage reports.
### Notes to the reviewers
The tests run before generating the report are not exhaustive (not exhaustive earlier too, but I added as many as I could), and hence the report won't be 100% accurate.
### 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:
danielabrozzoni:
ACK 0010ecd94aae49f389093cd3e8983c405049dbf5
Tree-SHA512: 04a21b7481b80287cf8a31276238b0a5958871310664363f54d53779bb2dda6f49198baaf3b5471667fa6131443b022aabd86f2e3b1cfcf5d4aacadf137a166e
d7163c3a976daf1b6a38884d77546a745805bd3c Update electrum-client to 0.11.0 (Alekos Filini)
Pull request description:
### Description
Update electrum-client to 0.11.0
### Changelog notice
- Updated `electrum-client` to `0.11.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
#### 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:
danielabrozzoni:
utACK d7163c3a976daf1b6a38884d77546a745805bd3c
Tree-SHA512: 3be61935e2bebfc5e4b530d17b9900e3c1bd2ef377fdd18df4303de713b124d51555f48f7aff5f4c2579105e86f52480e6b3f2af9c7494cbdb1dd2a2c5b4e0da
3451d1c12e4271b6067ff246d095eb9fb1ca3bd5 Fix docs.rs features (Alekos Filini)
Pull request description:
### Description
Fix docs.rs features
### 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`
ACKs for top commit:
danielabrozzoni:
utACK 3451d1c12e4271b6067ff246d095eb9fb1ca3bd5
notmandatory:
ACK 3451d1c12e4271b6067ff246d095eb9fb1ca3bd5
Tree-SHA512: 96dc4f816b21cf20fc2828dcfd56865f3f9add8e4aa643205879c810d09e6f2e73d6643061bf3ca98145b37e726907dedd31a145f92d6646c172a43ae2285aa8
7c57965999ca4a3d640d1dc11d7e10401abcabbf Bump version before making release branch, separate patch_release template (Steve Myers)
3d69f1c291b8d595399bfcb3ce202dca77a3f474 Update DEVELOPMENT_CYCLE.md to work with [patch.crates-io] (Steve Myers)
Pull request description:
### Description
Update DEVELOPMENT_CYCLE and release instructions to make [overriding dependencies] possible for downstream projects with unreleased `bdk` versions for development and testing. Also simplifies the release process by capturing changelog information in the `pull_request_template` and recording release changelog information in the release tag message instead of in a `CHANGELOG.md` file which causes too many merge conflicts and complicates the release process.
Fixes#536Fixes#496
### Notes to the reviewers
The primary changes to our current release process are:
1. Don't add `-dev` or `-rc.x` to unreleased `bdk` cargo versions because those extensions do not work with [overriding dependencies].
2. Increment the `master` branch version as soon as a `release/MAJOR.MINOR` branch is created, the next release `release/MAJOR.MINOR` branch version with be **MAJOR.MINOR.PATCH**, and the `master` branch development version will be **MAJOR.MINOR+1.0**; either version can be used with [overriding dependencies].
4. Remove the `bdk` version from the `src/lib.rs` file so that it doesn't need to be changed on every release, because it isn't needed in the rust docs for most developers and removing it will help simplify the release process.
5. The new release process is now documented as a checklist in a new `release.md` github issue template.
6. Putting changelog information in the release tag message is how the tokio project does it. ~~After this PR is merged I will replace old tags with new ones containing changelog information and then do a new PR to remove the CHANGELOG.md file.~~ After this PR is merged I don't think we need to update old tags, only rename the CHANGELOG.md file to CHANGELOG-OLD.md with a note to check tags for future change log info.
[overriding dependencies]: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html
### 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 7c57965999ca4a3d640d1dc11d7e10401abcabbf
Tree-SHA512: 818e2f9bc7a629cbbb190a83b9743e8f4de49a4093beae83ed0b9c506f33e6f96b2c1e376f788536d58c46908d278bde08140f43a625515401ea1f9efdb9153f
bfd7b2f65debe57edc5d563c071f0bfce592a8d2 Allow creating transactions with dust outputs (Liam)
Pull request description:
We needed this for testing our wallet with dust, does this look like a reasonable feature? If so, I'll go ahead and add a test, update the changelog, etc.
ACKs for top commit:
danielabrozzoni:
tACK bfd7b2f65debe57edc5d563c071f0bfce592a8d2
Tree-SHA512: b467b365d8a68f5a868cc5cc88387677533e8fb0bf543bf4c7a5b984f8b28972281029a3be8d2c92cee7d6ee05c243d12af0841e7a7e1d652745567557f2bede
138acc3b7d137788d0518182e2167504e58ebc48 Change `populate_test_db` to not return empty input (wszdexdrf)
d6e1dd104063075f49b617786d82d29c1f9c6a0a Change CI to add test using ledger emulator (wszdexdrf)
76034772cba4d3d6fa1bdcb08977c2b9d7a157c2 Add a custom signer for hardware wallets (wszdexdrf)
Pull request description:
Also adds a new test in CI for building and testing on a virtual
hardware wallet.
### Description
This PR would enable BDK users to sign transactions using a hardware wallet. It is just the beginning hence there are no complex features, but I hope not for long.
I have added a test in CI for building a ledger emulator and running the new test on it. The test is similar to the one on bitcoindevkit/rust-hwi.
### Notes to the reviewers
The PR is incomplete (and wouldn't work, as the rust-hwi in `cargo.toml` is pointing to a local crate, temporarily) as a small change is required in rust-hwi (https://github.com/bitcoindevkit/rust-hwi/pull/42).
### 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:
afilini:
ACK 138acc3b7d137788d0518182e2167504e58ebc48
Tree-SHA512: 54337f06247829242b4dc60f733346173d957de8e9f8b80beb91304d679cfb4e0e4db722c967469265a5b6ede2bd641ba5c089760391c671995dc30de37897de
08668ac46247d527cc53af5b6f359b1fa4e3b6aa Set the db sync height (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
Fixes#719
Previously we weren't setting the db sync height in populate_test_db
macro even when current height is provided.. This creates a bug that
get_funded_wallet will return 0 balance.
This PR fixes the populate_test_db macro and updates tests which were
previously dependent on the unsynced wallet behavior.
### 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)
* [x] I ran `cargo fmt` and `cargo clippy` before committing
#### Bugfixes:
* [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:
afilini:
ACK 08668ac46247d527cc53af5b6f359b1fa4e3b6aa
Tree-SHA512: 1dcc968e4b3551e916b450c5ff2fab6636083f104cc982eb3f7602c624382434e0170d9f0c0a356e6c9c5f834eebe5cb1365b37ef73d7b4ef15d652a364dc2ab
de358f8cdc97bf685ba46cf6751443d007b9f58e Implement conversion for Lightning fee rate (Vladimir Fomene)
Pull request description:
This PR fixes#608.
### Description
Lightning denotes transaction fee rate sats / 1000 weight units and sats / 1000 vbytes.
Here we add support for creating BDK FeeRate from lightning fee rate. We also move all FeeRate tests to
types.rs and rename as_sat_vb to as_sat_per_vb.
### Notes to the reviewers
Matt was concerned that we might round down value in fee calculation in such a way that a transaction may not be relayed because it is below Bitcoin Core's min relay fee (1 sat/vbyte). I don't think we need to worry about that because we [round up(ceil)](https://github.com/bitcoindevkit/bdk/blob/master/src/types.rs#L91) during fee calculation, we don't round down. I will love to hear what you think. Is there something I'm missing? @johncantrell97, I will appreciate your review on this one.
### 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
* [x] I'm linking the issue being fixed by this PR
ACKs for top commit:
danielabrozzoni:
ACK de358f8cdc97bf685ba46cf6751443d007b9f58e
Tree-SHA512: aaa7da8284b668d15ad9c92168c149c4b3ee0f8faee9b7eb159745d23e38835189eaf5c336da14ba9272ee07cd366718eefb8365da9ddf53014e122b6393a087
Lightning denotes transaction fee rate
sats / 1000 weight units and sats / 1000 vbytes.
Here we add support for creating BDK fee rate from
lightning fee rate. We also move all FeeRate test to
types.rs and rename as_sat_vb to as_sat_per_vb.
Previously we weren't setting the db sync height in populate_test_db
macro even when current height is provided.. This creates a bug that
get_funded_wallet will return 0 balance.
This PR fixes the populate_test_db macro and updates tests which were
previously dependent on the unsynced wallet behavior.
7b1ad1b62914a26d6f445364ace4e784bb2901c2 Verify signatures after signing (Scott Robinson)
Pull request description:
### Description
Verify signatures after signing
As per [BIP-340, footnote 14][fn]:
> Verifying the signature before leaving the signer prevents random or
> attacker provoked computation errors. This prevents publishing invalid
> signatures which may leak information about the secret key. It is
> recommended, but can be omitted if the computation cost is prohibitive.
[fn]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki#cite_note-14
### Notes to the reviewers
How do we test this?
### Checklists
#### All Submissions:
* [ ] 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:
re-ACK 7b1ad1b62914a26d6f445364ace4e784bb2901c2
Tree-SHA512: 7319db1f8cec2fcfe4ac443ab5728893f9fb6133b33331b35ec6910662c45de8a7cdcf80ac1f3bb435815e914ccf639682a5c07ff0baef42605bf044a34a8232
baf7eaace66edcae8bf3252a962b9417b704ba26 Implement Deref<Target=UrlClient> for EsploraBlockchain (Vladimir Fomene)
Pull request description:
### Description
There is currently no way to access the client from the EsploraBlockchain. This makes it difficult for users to extend it's functionality. This PR exposes both the reqwest and ureq clients. This PR is related to PR #705.
### 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 baf7eaace66edcae8bf3252a962b9417b704ba26
Tree-SHA512: e2f530058c88e06fc2972edfcd2df1b534d43b0214d710b62e4d5200ac0e38dad6a9f8db1e0c7a7ed19892e59411dcc07f3f6dc8ad58afae9d677169ca98bb38
c5952dd09a61b1cd2185c273e0b8bcb3fd6ed2dd Implement `Deref<Target=Client>` for `ElectrumBlockchain` (Alekos Filini)
Pull request description:
### Description
As pointed out in https://github.com/bitcoindevkit/rust-electrum-client/pull/58#issuecomment-1207890096 there was no way to keep using the client once it was given to 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:
rajarshimaitra:
ACK c5952dd09a61b1cd2185c273e0b8bcb3fd6ed2dd
Tree-SHA512: fbfbada51c9426266c8960da5508ee07b196808f0d670a09a51962bd6eda9ccf585e209f5b99b5ab78a3d17af774bdb3e33ef36ac4f4d1ce7f2c3398ae4f6d0c
a8f9f6c43adb90c9ceced21da6b6bafaa90d7af9 RpcBlockchain derefs to the underlying RPC Client (rajarshimaitra)
Pull request description:
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
For the same reason as #705 and #722 ..
### 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 a8f9f6c43adb90c9ceced21da6b6bafaa90d7af9
Tree-SHA512: 81e596fe451c275ca0ce27ee7ac9cf7e88433775603021c2dd1cd26a26558531cf74f81ef05d0ae9d5d0e59e91196e3ac6d38c0f4853b1889ddf822d8e63e178
As per [BIP-340, footnote 14][fn]:
> Verifying the signature before leaving the signer prevents random or
> attacker provoked computation errors. This prevents publishing invalid
> signatures which may leak information about the secret key. It is
> recommended, but can be omitted if the computation cost is prohibitive.
[fn]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki#cite_note-14
cd078903a7108f86fdd9557b206f25c9611d07d3 Fix P2WPKH_SATISFACTION_SIZE in CS tests (Daniela Brozzoni)
Pull request description:
Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8f99fc0897753ce57d48ea24adf495a633 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.
### Description
### Notes to the reviewers
### 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:
csralvall:
utACK cd078903a7108f86fdd9557b206f25c9611d07d3
afilini:
ACK cd078903a7108f86fdd9557b206f25c9611d07d3
Tree-SHA512: 3e39735505e411392cf01885b5920443d23fa21d9c20cc7c8fdeaa2698df8bc2da86241b6c20f5e3f5941fe1a0aebe8f957d8145d4f9e7ad3f213e4658d6ea68
Our costant for the P2WPKH satisfaction size was wrong: in
7ac87b8f99fc0897753ce57d48ea24adf495a633 we added 1 WU for the script
sig len - but actually, that's 4WU! This resulted in
P2WPKH_SATISFACTION_SIZE being equal to 109 instead of 112.
This also adds a comment for better readability.
8026bd9476fc5a89baa86ea2a8707a9d341d3743 Bump version to 0.21.1-dev (Alekos Filini)
e2bd96012a08137e4f6776c9192d5247ae20df0b Bump version to 0.21.0 (Alekos Filini)
2c01b6118f3291c2918a60cac635f26df1780dac Bump version to 0.21.0-rc.1 (Alekos Filini)
Pull request description:
### Description
Merge the release branch back into master
### 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:
danielabrozzoni:
ACK 8026bd9476fc5a89baa86ea2a8707a9d341d3743
Tree-SHA512: a2a924a60d551a823de035b609d4d51652a165a0695212af76dea87706919c8929dba977bb297f4787708470bf075d14dd0a37657bd3a76e7d44a746fb5439df
There is currently no way to access the client
from the EsploraBlockchain. This makes it difficult
for users to extend it's functionality. This PR exposes
both the reqwest and ureq clients. This PR is related to
PR #705.
134b19a9cb127989402fe331f48a1e37eb3cdcad Fix minor typos in docs (thunderbiscuit)
Pull request description:
### Description
This PR fixes:
1. The use of "i.e." in docs, sometimes spelled as "ie."
2. A small typo in the sentence "Note that this methods only operate on the internal database..."
3. A small typo in the sentence "Finish the building the transaction"
I came across these while building docs for bdk-kotlin.
### Notes to the reviewers
### Checklists
#### All Submissions:
* [x] I've signed all my commits
* [x] I ran `cargo fmt` and `cargo clippy` before committing
ACKs for top commit:
danielabrozzoni:
ACK 134b19a9cb127989402fe331f48a1e37eb3cdcad
Tree-SHA512: 67296999eba8ffe1fe64756fa023d85064774cb9d4c26e99054d467b5024baea4138f11d602d04e695412c61625ee4f5b4687b75f177cfec2604a6c61a5a6216
74e2c477f124489a2357921ca879cc82e24da5fd Replace `rpc::CoreTxIter` with `list_transactions` fn. (志宇)
Pull request description:
### Description
This fixes a bug where `CoreTxIter` attempts to call `listtransactions` immediately after a tx result is filtered (instead of being returned), when in fact, the correct logic will be to pop another tx result.
The new logic also ensures that tx results are returned in chonological order. The test `test_list_transactions` verifies this. We also now ensure that `page_size` is between the range `[0 to 1000]` otherwise an error is returned.
Some needless cloning is removed from `from_config` as well as logging improvements.
### Notes to the reviewers
This is an oversight by me (sorry) for PR #683
### 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:
afilini:
ACK 74e2c477f124489a2357921ca879cc82e24da5fd
Tree-SHA512: f32314a9947067673d19d95da8cde36b350c0bb0ebe0924405ad50602c14590f7ccb09a3e03cdfdd227f938dccd0f556f3a2b4dd7fdd6eba1591c0f8d3e65182
This fixes a bug where `CoreTxIter` attempts to call `listtransactions`
immediately after a tx result is filtered (instead of being returned),
when in fact, the correct logic will be to pop another tx result.
The new logic also ensures that tx results are returned in chonological
order. The test `test_list_transactions` verifies this. We also now
ensure that `page_size` is between the range `[0 to 1000]` otherwise an
error is returned.
Some needless cloning is removed from `from_config` as well as logging
improvements.
0f03831274d3aa69da6e89729c65d66530bbd752 Change get_balance to return in categories. (wszdexdrf)
Pull request description:
### Description
This changes `get_balance()` function so that it returns balance separated in 4 categories:
- available
- trusted-pending
- untrusted-pending
- immature
Fixes#238
### Notes to the reviewers
Based on #614
### 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 tests for the new feature
* [x] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
ACKs for top commit:
afilini:
ACK 0f03831274d3aa69da6e89729c65d66530bbd752
Tree-SHA512: 39f02c22c61b6c73dd8e6d27b1775a72e64ab773ee67c0ad00e817e555c52cdf648f482ca8be5fcc2f3d62134c35b720b1e61b311cb6debb3ad651e79c829b93
5eeba6cced9a6fa0ad8ee4f64d04e1948620eac8 Various `RpcBlockchain` improvements (志宇)
5eb74af41494b7ec4894d7da3015da2981639228 Rpc: Manually add immature coinbase utxos (志宇)
ac19c19f21fce43a99ecf0c4f95ae818b620558c New `RpcBlockchain` implementation with various fixes (志宇)
Pull request description:
Fixes#677
### Description
Unfortunately to fix all the problems, I had to do a complete re-implementation of `RpcBlockchain`.
**The new implementation fixes the following:**
* We can track more than 100 scriptPubKeys
* We can obtain more than 1000 transactions per sync
* Transaction "metadata" for already-syned transactions are updated when we introduce new scriptPubKeys
**`RpcConfig` changes:**
* Introduce `RpcSyncParams`.
* Remove `RpcConfig::skip_blocks` (this is replaced by `RpcSyncParams::start_time`).
### Notes to the reviewers
* The `RpcConfig` structure is changed. It will be good to confirm whether this is an okay 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:
~* [ ] I've added tests for the new feature~
* [x] I've added docs for the new feature
* [x] I've updated `CHANGELOG.md`
#### Bugfixes:
* [x] 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:
afilini:
ACK 5eeba6cced9a6fa0ad8ee4f64d04e1948620eac8
Tree-SHA512: 7e0c9cfc4ef10fb07e4ac7f6fbf30cf28ca6395495c0237fa5bfa9a2fcbbd4d8ff980ffcf71ddd10bc052e4c07bc2c27f093dd3cd1c69cb29141455c693f2386
These are as suggested by @danielabrozzoni and @afilini
Also introduced `RpcSyncParams::force_start_time` for users who
prioritise reliability above all else.
Also improved logging.
Before this commit, the rpc backend would not notice immature utxos
(`listunspent` does not return them), making the rpc balance different
to other blockchain implementations.
Co-authored-by: Daniela Brozzoni <danielabrozzoni@protonmail.com>
The new implementation fixes the following:
* We can track more than 100 scriptPubKeys
* We can obtain more than 1000 transactions per sync
* `TransactionDetails` for already-synced transactions are updated when
new scriptPubKeys are introduced (fixing the missing balance/coins
issue of supposedly tracked scriptPubKeys)
`RpcConfig` changes:
* Introduce `RpcSyncParams`.
* Remove `RpcConfig::skip_blocks` (this is replaced by
`RpcSyncParams::start_time`).
9d85c9667f7d12902afef3ba08ea7231f6868a78 Fix the early InsufficientFunds error in the branch and bound (Alekos Filini)
Pull request description:
### Description
We were wrongly considering the sum of "effective value" (i.e. value -
fee cost) when reporting an early "insufficient funds" error in the
branch and bound coin selection.
This commit fixes essentially two issues:
- Very high fee rates could cause a panic during the i64 -> u64
conversion because we assumed the sum of effective values would never
be negative
- Since we were comparing the sum of effective values of *all* the UTXOs
(even the optional UTXOs with negative effective value) with the target
we'd like to reach, we could in some cases error and tell the user we
don't have enough funds, while in fact we do! Since we are not required
to spend any of the optional UTXOs, so we could just ignore the ones
that *cost us* money to spend and excluding them could potentially
allow us to reach the target.
There's a third issue that was present before and remains even with this
fix: when we report the "available" funds in the error, we are ignoring
UTXOs with negative effective value, so it may look like there are less
funds in the wallet than there actually are.
I don't know how to convey the right message the user: if we actually
consider them we just make the "needed" value larger and larger (which
may be confusing, because if the user asks BDK to send 10k satoshis, why
do we say that we actually need 100k?), while if we don't we could report
an incorrect "available" value.
### Notes to the reviewers
I'm opening this as a draft before adding tests because I want to gather some feedback on the available vs needed error reporting. I personally think reporting a reasonable "needed" value is more important than the "available", because in a wallet app I would expect this is the value that would be shown to the user.
### 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:
danielabrozzoni:
utACK 9d85c9667f7d12902afef3ba08ea7231f6868a78
Tree-SHA512: 9a06758cba61ade73198f35b08070987d5eb065e01750ce62409f86b37cd0b0894640e9f75c8b2c26543c0da04e3f77bd397fab540e789f221661aae828db224
