Add import_waiter for RpcBlockchain.
#5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As pointed out in bitcoindevkit/rust-electrum-client#58 (comment) there was no way to keep using the client once it was given to BDK.
…com:bitcoindevkit/bdk into release/0.21.0
afilini
force-pushed
the
fix/rpc-retry-on-error
branch
from
9aa08ed to
ccb951a
Compare
evanlinjin
force-pushed
the
fix/rpc-retry-on-error
branch
from
204465d to
305f4ea
Compare
9 tasks
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 bitcoindevkit#705.
8026bd9 Bump version to 0.21.1-dev (Alekos Filini) e2bd960 Bump version to 0.21.0 (Alekos Filini) 2c01b61 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 8026bd9 Tree-SHA512: a2a924a60d551a823de035b609d4d51652a165a0695212af76dea87706919c8929dba977bb297f4787708470bf075d14dd0a37657bd3a76e7d44a746fb5439df
Our costant for the P2WPKH satisfaction size was wrong: in 7ac87b8 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.
cd07890 Fix P2WPKH_SATISFACTION_SIZE in CS tests (Daniela Brozzoni) Pull request description: Our costant for the P2WPKH satisfaction size was wrong: in 7ac87b8 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 cd07890 afilini: ACK cd07890 Tree-SHA512: 3e39735505e411392cf01885b5920443d23fa21d9c20cc7c8fdeaa2698df8bc2da86241b6c20f5e3f5941fe1a0aebe8f957d8145d4f9e7ad3f213e4658d6ea68
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
…hain a8f9f6c 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 bitcoindevkit#705 and bitcoindevkit#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 a8f9f6c Tree-SHA512: 81e596fe451c275ca0ce27ee7ac9cf7e88433775603021c2dd1cd26a26558531cf74f81ef05d0ae9d5d0e59e91196e3ac6d38c0f4853b1889ddf822d8e63e178
…umBlockchain` c5952dd Implement `Deref<Target=Client>` for `ElectrumBlockchain` (Alekos Filini) Pull request description: ### Description As pointed out in bitcoindevkit/rust-electrum-client#58 (comment) 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 c5952dd Tree-SHA512: fbfbada51c9426266c8960da5508ee07b196808f0d670a09a51962bd6eda9ccf585e209f5b99b5ab78a3d17af774bdb3e33ef36ac4f4d1ce7f2c3398ae4f6d0c
…aBlockchain baf7eaa 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 bitcoindevkit#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 baf7eaa Tree-SHA512: e2f530058c88e06fc2972edfcd2df1b534d43b0214d710b62e4d5200ac0e38dad6a9f8db1e0c7a7ed19892e59411dcc07f3f6dc8ad58afae9d677169ca98bb38
7b1ad1b 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 7b1ad1b Tree-SHA512: 7319db1f8cec2fcfe4ac443ab5728893f9fb6133b33331b35ec6910662c45de8a7cdcf80ac1f3bb435815e914ccf639682a5c07ff0baef42605bf044a34a8232
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.
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.
de358f8 Implement conversion for Lightning fee rate (Vladimir Fomene) Pull request description: This PR fixes bitcoindevkit#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 de358f8 Tree-SHA512: aaa7da8284b668d15ad9c92168c149c4b3ee0f8faee9b7eb159745d23e38835189eaf5c336da14ba9272ee07cd366718eefb8365da9ddf53014e122b6393a087
Also add function to get funded wallet with coinbase
08668ac 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 bitcoindevkit#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 08668ac Tree-SHA512: 1dcc968e4b3551e916b450c5ff2fab6636083f104cc982eb3f7602c624382434e0170d9f0c0a356e6c9c5f834eebe5cb1365b37ef73d7b4ef15d652a364dc2ab
138acc3 Change `populate_test_db` to not return empty input (wszdexdrf) d6e1dd1 Change CI to add test using ledger emulator (wszdexdrf) 7603477 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 (bitcoindevkit/rust-hwi#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 138acc3 Tree-SHA512: 54337f06247829242b4dc60f733346173d957de8e9f8b80beb91304d679cfb4e0e4db722c967469265a5b6ede2bd641ba5c089760391c671995dc30de37897de
Add TxBuilder::allow_dust() that skips checking the dust limit
bfd7b2f 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 bfd7b2f Tree-SHA512: b467b365d8a68f5a868cc5cc88387677533e8fb0bf543bf4c7a5b984f8b28972281029a3be8d2c92cee7d6ee05c243d12af0841e7a7e1d652745567557f2bede
…tch.crates-io] 7c57965 Bump version before making release branch, separate patch_release template (Steve Myers) 3d69f1c 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 bitcoindevkit#536 Fixes bitcoindevkit#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 7c57965 Tree-SHA512: 818e2f9bc7a629cbbb190a83b9743e8f4de49a4093beae83ed0b9c506f33e6f96b2c1e376f788536d58c46908d278bde08140f43a625515401ea1f9efdb9153f
3451d1c 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 3451d1c notmandatory: ACK 3451d1c Tree-SHA512: 96dc4f816b21cf20fc2828dcfd56865f3f9add8e4aa643205879c810d09e6f2e73d6643061bf3ca98145b37e726907dedd31a145f92d6646c172a43ae2285aa8
`RpcImportParams` keeps track of the scriptPubKey derivation index to start from for the next call to `importdescripts`/`importmulti`, thus avoiding re-importing into Bitcoin Core.
* Add `RpcSyncParams::page_size` that restricts req/resp array count for various RPC calls. * Add `pagenated_import` function.
Whenever the transport errors with "WouldBlock" we wait a bit and retry the call. Related to bitcoindevkit#650, should at least fix the errors with RPC
evanlinjin
force-pushed
the
fix/rpc-retry-on-error
branch
5 times, most recently
from
cafd917 to
9d8cc0c
Compare
Bitcoin Core returns code -4 during scriptPubKey/descriptor import when it is still rescanning the wallet. This PR waits until the rescan completes before continuing sync as if it was a successful import. The caveat of this approach is that a sync cycle may miss out on importing some scriptPubKeys. Also, re-import logic is not ideal as network interruptions or parallel requests will result in strange behavior. TODO: This may not be this best solution.
evanlinjin
force-pushed
the
fix/rpc-retry-on-error
branch
from
9d8cc0c to
987c125
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bitcoin Core rejects importing scriptPubKeys/descriptors when it is still rescanning the wallet. We should wait until rescan completes before importing again.