5dc4bb6360 ci: bump clippy to 1.90.0 (valued mammal)
e31cf4773b feat!: Change ConfigBuilder::timeout to accept Option<Duration> (valued mammal)
Pull request description:
Change `ConfigBuilder::timeout` to accept `Option<Duration>`. This makes the API more explicit and less error prone, as the caller no longer needs to assume the units of the given duration.
Also updated the code to run clippy check in CI using rust 1.90.0.
BREAKING:
The `timeout` method on `ConfigBuilder` is changed to accept a `timeout: Option<Duration>`. Previously it was `Option<u8>`.
fix#151fix#175
Supersedes #179
ACKs for top commit:
oleonardolima:
ACK 5dc4bb6360
Tree-SHA512: cd6ca6fdd91dcf6f193327ca40f6d37ea8b1b548102f080d0b72a0df509f6455aa50ee43148e4746ca6b295a30a78cb7d786fb0e90c050448e96e122a5b8c92d
c2656995fc chore: Bump version to 0.24.1 (valued mammal)
Pull request description:
Bump version to `0.24.1`.
Update CHANGELOG.md.
ACKs for top commit:
oleonardolima:
ACK c2656995fc
Tree-SHA512: 1e73dc9268f0c09224df758cd84620d591f0b0b82e114dfda07507ede5c58b0c43d99d1935b301db46a162e71de35bde56c9f8d36b4d9451f90f56890b48cc07
80bf744a70 test: don't assert the value returned by `relay_fee` (valued mammal)
0e28021b3e Check in CI that we compile if all `rustls` features are set (Elias Rohrer)
980fa4afd6 Default to `ring` if multiple `rustls` features are set (Elias Rohrer)
Pull request description:
Fixes#181
We default to use `ring` if multiple features are set, ensuring the features are additive. To that end, we also add a check to CI that asserts we succeed to build with both features set.
Would be cool to get a patch release for this, as it's currently breaking our docs.rs builds in `lightning-liquidity`.
ACKs for top commit:
ValuedMammal:
ACK 80bf744a70.
oleonardolima:
cACK 80bf744a70
Tree-SHA512: 0b1bacb0f3a57fd8c666e1ece14b9a733f9d5dfdec7efd6461b400d58ef0e302c286597531381e417b5fdf1b97659ed36266d2cf89f8a0c4cab5e4d9b3fdeed7
Changed `test_relay_fee` to no longer assert the value of min
relay fee, and instead assert that the value is non-zero.
This fixes a test failure likely due to some nodes now having a
smaller default min relay fee (100sat).
24d07e2cac chore(release): update `CHANGELOG.md` (Leonardo Lima)
fbffdfe65e chore(release): bump version to `0.24.0` (Leonardo Lima)
Pull request description:
fixes#173
- bump the crate version to `0.24.0`.
- updates the `CHANGELOG.md` with relevant changes: https://github.com/bitcoindevkit/rust-electrum-client/compare/0.24.0...HEAD
ACKs for top commit:
ValuedMammal:
ACK 24d07e2cac
Tree-SHA512: 23060238562788fbc8d4ecdb2273db25329c4f645700d4d515a8d21a98b04bb421d0c8532d1b38ff566257d4df9095299d5b539261b88d090507350f80141d86
74ac97cfe4 feat: add `batch_transaction_get_merkle` (Leonardo Lima)
Pull request description:
partially addresses https://github.com/bitcoindevkit/bdk/issues/1987
- adds the new batch method for `blockchain.transaction.get_merkle`.
- adds a new test for `batch_transaction_get_merkle` with 3 different
txids and block_heights.
ACKs for top commit:
ValuedMammal:
ACK 74ac97cfe4
Tree-SHA512: 430e07d4cb3f0dc812a389b9b093b55f934ac72d32de5eebe5e072780dc4bd82c15796ba70ff3ae9354e822fac45e479618ff76e0360bd8d887edd7b62dd1327
f24458248a Install `rustls`'s `CryptoProvider` based on features (Elias Rohrer)
Pull request description:
Previously, we'd already assume `use-rustls` to use the default `aws-lc-rs` provider and `use-rustls-ring` to use the `ring` `CryptoProvider`, e.g., for `NoCertificateVerification`.
However, we **wouldn't** actually install the respective provider based on the features, leading to a **reachable** panic at runtime when user tried to access `ssl://` Electrum servers.
Here, we fix this omission and install the default provider according to the configured features.
(cc @oleonardolima @thunderbiscuit)
ACKs for top commit:
ValuedMammal:
ACK f24458248a
oleonardolima:
ACK f24458248a
Tree-SHA512: b502b97e4162c0dd46e17ccf1c0a0a8461158dbec06833d7d8715072fa4feeb87beb3ee2dd93c594689ef0c2ecd84ed52a4f9309d826bebb95f3c9e57dd933fb
Previously, we'd already assume `use-rustls` to use the default
`aws-lc-rs` provider and `use-rustls-ring` to use the `ring`
`CryptoProvider`, e.g., for `NoCertificateVerification`.
However, we **wouldn't** actually install the respective provider based
on the features, leading to a **reachable** panic at runtime when user
tried to access `ssl://` Electrum servers.
Here, we fix this omission and install the default provider according to
the configured features.
- adds the new batch method for `blockchain.transaction.get_merkle`.
- adds a new test for `batch_transaction_get_merkle` with 3 different
txids and block_heights.
ed9784ee79 chore(release): bump version to `0.23.1` (Leonardo Lima)
e14f66b2f1 ci: use `ubuntu-latest` instead of `ubuntu-20.04` (Leonardo Lima)
Pull request description:
fixes#168
- updates the CI to run on `ubuntu-latest` GitHub runner, as the previously used `ubuntu-20.04` has been officially deprecated as of 2025-04-15, see: https://github.com/actions/runner-images/issues/11101.
- updates the project version to `0.23.1`.
- updates the `CHANGELOG.md` with changes to latest version.
ACKs for top commit:
ValuedMammal:
ACK ed9784ee79
Tree-SHA512: d448fc6fa2f9626dc4344ea3c8e7d1a948587f388803d923c755b454c73e4d6db860b70cba103a1092147c20656f7837e535e35eeefbead3dcb2f4260b25d839
a3488f4ff9 ci: bump actions/cache to v4 (valued mammal)
0ee0a6ad7a test: add batch response ordering test (marshallyale)
6721929f22 fix: fix batch ordering issue (marshallyale)
Pull request description:
Fixes issue #75
This is my first open source pull request so I apologize for any formatting issues. Additionally, I don't know the repository as well as others so there may be a better way to implement the fix.
I believe I found the root cause of this. I added a pull request to fix, but I'm going to copy/paste what I believe is causing the error.
The main issue in the code is inside raw_client.rs inside the `recv` method implementation (snippet below):
805ea0af30/src/raw_client.rs (L671-L685)
When this is first called, the `self._reader_thread` will run. Inside the `self._reader_thread`, if the request id matches the response id, everything works fine. However, if the request id does not match the response id, we run the following code:
805ea0af30/src/raw_client.rs (L602-L612)
The channel that the response is sent back into is not unique, but rather all the channels share the same sender.clone() and receiver. The only validation that is done is to check that the request id is still being searched for inside `self.waiting_map`. This means that the receiver channel receives whatever the next response is into the channel without any validation that it matches the request id which happens here `match receiver.recv()?`.
This is fixed by implementing unique channels for every request id. This fix can be verified with the code johnzweng used to show the issue
If you run this with the initial code, it will error out after 1-10 cycles normally. However, after the fix this runs indefinitely.
ACKs for top commit:
ValuedMammal:
reACK a3488f4ff9
Tree-SHA512: c56d572c0d9e709352fde0c0438103fe4c0338e4b591d5290468b1658d6d73dbc818044e1b7ea6307e449a8d4380d9deba6adf2b89eb1dcbc119cec277fd721c
Fixes issue #75
Raw client waiting map was using the same channel
for every request/response. When items were put
back into the channel inside of _reader_thread
the waiting receiver in recv would just take the
next response without validating it on request id
request.
This fixes this by using unique channels for each
request response inside of the waiting map.
0e459b6bd6 chore: bump version to 0.23.0 (valued mammal)
Pull request description:
Bump crate version to 0.23.0 and update `CHANGELOG.md`
ACKs for top commit:
notmandatory:
ACK 0e459b6bd6
oleonardolima:
ACK 0e459b6bd6
Tree-SHA512: b50296b65b7533114674b90991ead8446f35625931bf153f09c149f4fcd3430c412a338ff4a1aa8a9616d4f0af6706a632a5268261431dbd257e6c15929fc7dd
3dc4daceaf ci(clippy): bump to `1.84` (Leonardo Lima)
5a476fdfd9 deps(rustls): bump to `0.23.21` (Leonardo Lima)
ec90685324 ci(msrv): bump rust MSRV to `1.75` (Leonardo Lima)
Pull request description:
partially fixes https://github.com/bitcoindevkit/bdk/issues/1750
It bumps the project MSRV to `1.75.0` in order to follow along with LDK changes, and to remove the pinned version of `rustls`, which now is under `1.71` MSRV, bumping it to latest `0.23.21`.
It also updates the `clippy` step on CI to use rust `1.84`, fixing the found clippy issues.
ACKs for top commit:
tnull:
> ACK [3dc4dac](3dc4daceaf)
ValuedMammal:
ACK 3dc4dac
notmandatory:
ACK 3dc4daceaf
Tree-SHA512: 3b9c2feffc8cc32cb9751a36406e7f0d5ed4692afa1af186621bef93da924def3e23c5132e004deef955e40bd7c3242a2d07c33e3843b1b76b613be2f54afefe
- bumps the MSRV on CI to 1.75.
- updates the `Cargo.toml` rust-version to 1.75.
- removes the guideline to pin `rustls` version, as previously required
by old 1.63 MSRV.
- update `README.md` badges to `1.75` MSRV.
- update `clippy.toml` to `1.75` MSRV.
ed0cdae329 chore: bump version to 0.22.1, update CHANGELOG (Steve Myers)
7ef3ff6873 chore: Enforce min `rustls` version to 0.23.19 (志宇)
Pull request description:
`rustls` versions 0.23.18 and 0.23.19 contains fix for vulnerability RUSTSEC-2024-0399. However, 0.23.18 bumps MSRV to 1.71. 0.23.19 reverts MSRV back to 1.63.
We enforce min `rustls` version to 0.23.19 to make it easier to compile on MSRV and ensure we include the RUSTSEC-2024-0399 fix.
Note that in CI, I decided to pin `rustls` dependency to 0.23.19 explicitly. This is because in future versions of `rustls`, the MSRV will be changed to 1.71.
Context: https://github.com/rustls/rustls/pull/2244
ACKs for top commit:
notmandatory:
ACK ed0cdae329
Tree-SHA512: 9b6319d520a1002b3404743f051019a5ff63f53a9fe37ed080ed8e761f3db3edd446d014d2145e9c098279d7b3e6e017e82dd633baeef202a904afb37c49c4d9
`rustls` versions 0.23.18 and 0.23.19 contains fix for vulnerability
RUSTSEC-2024-0399. However, 0.23.18 bumps MSRV to 1.71. 0.23.19 reverts
MSRV back to 1.63.
We enforce min `rustls` version to 0.23.19 to make it easier to compile
on MSRV and ensure we include the RUSTSEC-2024-0399 fix.
Note that in CI, I decided to pin `rustls` dependency to 0.23.19
explicitly. This is because in future versions of `rustls`, the MSRV
will be changed to 1.71.
b0a1bfb0cb ci: pin msrv dep version for rustls (Steve Myers)
Pull request description:
I'm not thrilled about having to pin `rustls` to meet our MSRV, but we need to make a new release and can't count on rustls/rustls#2239 being accepted by the `rustls` team.
ACKs for top commit:
luisschwab:
utACK b0a1bfb0cb
ValuedMammal:
utACK b0a1bfb0cb
oleonardolima:
ACK b0a1bfb0cb
Tree-SHA512: 1f37b423f3e10a7e3fa4e2072d5ad38b96341a91f78c1bcffc011dedc08071bd176b556646e40e635c3b558e6b771a4a7baa85b10d14da64d2cd385a69342ee4
d917f891e8 Update URLs in `Cargo.toml` (Elias Rohrer)
Pull request description:
.. while it's still magical, the URLs seem to be outdated ..
ACKs for top commit:
ValuedMammal:
ACK d917f891e8
Tree-SHA512: cf652d6949d8be67f5884457f388e8fb5bc68a54d826e387dcc3542c00c2d71d62bad1dd9cb78a95f55b4f19d1beec2379540b7fea2e610a71817b8cb743f90d
a871b084ec feat: add `id_from_pos` support (Wei Chen)
Pull request description:
This PR introduces the `blockchain.transaction.id_from_pos` feature from Electrum. This functionality is essential for an ongoing implementation in `bdk_electrum`, which requires retrieving the coinbase transaction from a block at a specified `height`. Currently, there is no straightforward method to achieve this. By implementing the `id_from_pos` feature, we will effectively address this gap.
ACKs for top commit:
oleonardolima:
utACK a871b084ec
ValuedMammal:
ACK a871b084ec
evanlinjin:
ACK a871b084ec
Tree-SHA512: e8f1ab44473f67030b4f19e766fcee472b7cfc1dac6df8c9f0e08d966e0ed48289de03768771623b9fc7e7ef15dacdb7e3b26c04f9b0de7566d67cc7585a7fa6
05771a81d7 fix: `NoCertificateVerification` implementation (Leonardo Lima)
Pull request description:
fixes#149https://github.com/bitcoindevkit/bdk/issues/1598
<!-- You can erase any parts of this template not applicable to your Pull Request. -->
### Description
<!-- Describe the purpose of this PR, what's being adding and/or fixed -->
It has been noticed some issues by both users and developers, as reported in #149, https://github.com/bitcoindevkit/bdk/issues/1598 and https://github.com/wizardsardine/liana/issues/1300, when using the library with `use-rustls-{ring}` feature to connect to electrum servers that use self-signed certificates, there are even some issues when trying to connect to `ssl://electrum.blockstream.info:50002` server.
To connect in an insecure manner either with `rustls` or `openssl` features, the user can set the `validate_domain` field in the `Config` to false, this will either set the `SslVerifyMode::NONE` when using `openssl`, or use the custom `NoCertificateVerification` for the
`rustls::client::danger::ServerCertVerifier` trait when using `rustls`, that said it should ignore the certificate verification when used.
At the current library state, it's failing because we didn't set up the supported `rustls::SignatureScheme` properly, returning an empty vector at the moment. This PR focuses on fixing this issue by relying on the `CryptoProvider` in usage to get the correct and supported signature schemes.
As part of the research to understand the problem, I've noticed that ideally, we should still use both the `rustls::webpki::verify_tls12_signature` and `rustls::webpki::verify_tls12_signature` and only rely on `rustls::client::danger::ServerCertVerified::assertion()` to ignore the certificate verification, however, it would still fail in scenarios such as https://github.com/bitcoindevkit/bdk/issues/1598 which uses X.509 certificates with any version other than 3 (it uses version 1), see [here](1a0d1646d0/src/cert.rs (L202-L218)).
I kept the current behavior to also ignore the TLS signature, but I still would like to bring this to the discussion, should we validate it properly and update the documentation to mention the `webpki` limitation instead ?
### Notes to the reviewers
I kept the current behavior to also ignore the TLS signature, but I still would like to bring this to the discussion, should we validate it properly and update the documentation to mention the `webpki` limitation instead ?
<!-- 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 -->
### Changelog notice
- Updates the `NoCertificateVerification` implementation for the
`rustls::client::danger::ServerCertVerifier` to use the `rustls::SignatureScheme` from `CryptoProvider` in use.
<!-- Notice the release manager should include in the release tag message changelog -->
<!-- See https://keepachangelog.com/en/1.0.0/ for examples -->
### 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
* [ ] 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:
LLFourn:
ACK 05771a81d7
ValuedMammal:
ACK 05771a81d7
notmandatory:
ACK 05771a81d7
Tree-SHA512: f74dedf458853fb19cd21dedb5b92158acd865ee0ab0fd6bbb2b3e267bac22edc7cf004d2dc0068f66d2665d87e6dd419231710a02317e3b2bfaa9f408b30759
It updates the `NoCertificateVerification` implementation of
`rustls::client::danger::ServerCertVerifier` trait, it keeps the usage
of both `ServerCertVerified::assertion()` and
`HandshakeSignatureValid::assertion()` usage, but now instead of having
an empty vector vector of supported `SignatureScheme`, it uses the ones
supported by the used `CryptoProvider`.
4dd7e2135e Bump version to 0.21.0 and update CHANGELOG.md (Steve Myers)
Pull request description:
Bumped crate version to 0.21.0 and added below to changelog:
## 0.21.0
- Add use-rustls-ring feature #135
- refactor: make validate_merkle_proof more efficient #134
- chore: set rust edition to 2021, fix clippy, add ci fmt and clippy checks #139
## 0.20.0
- Upgrade rustls to 0.23 #132
- chore(deps): upgrade rust-bitcoin to 0.32.0 #133
- ci: add test with MSRV 1.63.0 #128
ACKs for top commit:
oleonardolima:
ACK 4dd7e2135e
ValuedMammal:
ACK 4dd7e2135e
Tree-SHA512: 3fcec2fb437733eac235bccb1b9c8f6b706e7a713c71de85016adc93f7db128ca6eadb5e9d1d44df27f1b49cce139b222aa9c21343afcf25befdf80a47442e51
0dd51408a3 chore(ci): add fmt and clippy checks (Steve Myers)
e7af332684 chore: fix clippy errors and set msrv to 1.63 (Steve Myers)
84f4f609b9 chore: set rust edition to 2021 (Steve Myers)
Pull request description:
A little house keeping to avoid the warning about missing the edition. After setting the rust edition to 2021 I had to fix clippy warnings. I also added CI fmt and clippy checks.
ACKs for top commit:
ValuedMammal:
ACK 0dd51408a3
oleonardolima:
ACK 0dd51408a3
Tree-SHA512: ee37c3ef06e217f76dcb622cdbb53e9b5c94163b789ca584037d5a8bb4bdef13c6cf251b555ac0e0ede309d7436295f1ede4e2ac6c0dc141c9bf2e3d6ecf9238
