ef9fd6bf50 Bump version to 0.19.0 and add CHANGELOG.md (Steve Myers)
Pull request description:
Besides bumping the version I've also added a simple changelog that list the PRs in this (0.19.0) and the prior (0.18.0) release. The main reason for this release is the bump of the rust-bitcoin version to 0.31.0 which is needed to upgrade dependent projects like bdk to also use rust-bitcoin 0.31.0.
ACKs for top commit:
evanlinjin:
ACK ef9fd6bf50
Tree-SHA512: c18a0312915adfad40ef900ac4f1ee10454863f7a29882f1fa8f967f4c13df34b4e3929703614d0c3e6b8a189338f3a4d15b8d4598f68b87948454184fb13957
d8554fb550 enforce timeout on initial socks5 proxy connection (conduition)
Pull request description:
This PR fixes a bug which excepted initial SOCKS5 proxy connection attempts from the punctual enforcement of timeouts.
Before this change, invoking `Socks5Stream::connect` (or `Socks5Stream::connect_with_password`) could block for much longer than the configured timeout. In practice, this manifested as `Client::from_config` apparently failing to respect the timeout specified in the `Config` passed to it. AFAICT this only applied to SOCKS proxy connections.
## Example
To demonstrate, here is a simple example program which attempts to connect to an unreachable electrum server with a 10 second timeout.
```rust
use electrum_client::{Client, ConfigBuilder, Socks5Config};
fn main() {
let proxy = Socks5Config::new("127.0.0.1:9050");
let config = ConfigBuilder::new()
.socks5(Some(proxy))
.timeout(Some(10))
.build();
let start = std::time::SystemTime::now();
let result = Client::from_config(
"tcp://bejqtnc64qttdempkczylydg7l3ordwugbdar7yqbndck53ukx7wnwad.onion:50001",
config,
);
match result {
Ok(_) => {
println!("Successfully connected")
}
Err(e) => {
println!(
"failed to connect after {:.2}s: {e}",
start.elapsed().unwrap().as_secs_f64()
);
}
};
}
```
You'd expect the connection attempt to always fail at around 10 seconds, but in fact most attempts take considerably longer.
```
$ for i in {1..10} ; do cargo run -q ; done
failed to connect after 7.65s: host unreachable
failed to connect after 47.78s: host unreachable
failed to connect after 18.17s: host unreachable
failed to connect after 29.24s: host unreachable
failed to connect after 16.15s: host unreachable
failed to connect after 14.40s: host unreachable
failed to connect after 16.89s: host unreachable
failed to connect after 9.93s: host unreachable
failed to connect after 8.81s: host unreachable
failed to connect after 17.80s: host unreachable
```
## Cause and Fix
This was happening because the private method `Socks5Stream::connect_raw` [only respected the `timeout` parameter for _the initial connection to the proxy address_](5ecb26fd7d/src/socks/v5.rs (L200-L205)).
Once that TCP socket is established, the SOCKS5 client code must exchange a couple of messages with the proxy itself: One request/response cycle to authenticate, and then another request/response cycle to configure the forward proxy to the ultimate destination address. The latter of these two request/response cycles could block for long periods of time, in the case where the proxy was responsive but the ultimate destination was unresponsive.
Since no timeout was set on the socket at this stage, the `Socks5Stream` code would wait for an indefinite amount of time for a reply from the proxy, usually only once the proxy itself times out and finally sends a reply.
My suggested fix in this PR is to set the read/write timeouts immediately on the socket connecting to the proxy, so that if the proxy doesn't reply in time, we return an error to the caller promptly.
ACKs for top commit:
RCasatta:
utACK d8554fb550
Tree-SHA512: 4bc0ca203465c0d9722680de7251ee49dbea6d5a2b2a833a1ed42e792342a7ac977ad72649603caacd3e58d233b1a516af1ad40c6935addb8165b720d823cf42
dd3c171a7a Upgrade bitcoin (Tobin C. Harding)
Pull request description:
Upgrade bitcoin dependency to `rust-bitcoin v0.31.0-rc1`:
Allows us to remove the dependency on `bitcoin-private` because the `hex` stuff is exposed by `bitcoin` now.
ACKs for top commit:
notmandatory:
ACK dd3c171a7a
Tree-SHA512: 9082d3c2136445230bb23669f83fed58c90d1baf28d35527fc3dea1d40c9d3bdebeddbc44b3bdbc8e79c9b701e09cb453c018af0bc4ac8bcd1c4a14d11c90e39
Upgrade bitcoin dependency to `rust-bitcoin v0.31.0`:
Allows us to remove the dependency on `bitcoin-private` because the
`hex` stuff is exposed by `bitcoin` now.
54fd52d898 Add test coverage for `validate_merkle_proof` (Elias Rohrer)
fe33e19bda Add utility for validating a Merkle inclusion proof (Elias Rohrer)
dd872d6714 Make response types `Clone` (Elias Rohrer)
Pull request description:
I recently needed to validate a Merkle inclusion proof as retrieved via `transaction_get_merkle`.
As I figured it might be useful to other people, too, we add it here as a simple utility method.
ACKs for top commit:
notmandatory:
ACK 54fd52d898
Tree-SHA512: aac12160d5b91a011988f45013eb92924c2dfb244c1720e73dc5bcb731e69065c38e022502c756100d8ee6c9af06efa0de9bbfbb2b9e3c2e34d3223539206e1c
a331ae8059 Remove `webpki` and bump `webpki-roots` to `v0.25` (Yuki Kishimoto)
Pull request description:
I noticed that `webpki` dependency is no longer maintained and that has a high severity vulnerability.
This PR remove the `webpki` dependency and bump `webpki-roots` to v0.25
ACKs for top commit:
notmandatory:
ACK a331ae8059
Tree-SHA512: 63e9498dc0d56a07e7dd09dd43ca9a924d7e9ebb09934f2c762e64c9ce163cd58edb4d1563db4eba18a0fdf22642cb7d801940baeb97b6ce5473970b739278d4
adb0c7444e Add `Batch::raw` and improve docs (志宇)
Pull request description:
Being able to add raw requests to `Batch` makes sense from an API standpoint because we already allow raw non-batched requests. This is also useful when the electrum server API gets an updated version and our client is unable to keep up.
Additional to this, I have improved the documentation and made `Call` private (since `Call` is never used externally).
ACKs for top commit:
notmandatory:
ACK adb0c7444e
Tree-SHA512: 808dccf1152b750881e45a9709fb4127835ecff3da5ecccffcb9f03e62171192c58154860195db7d3d3467ae8e3e450bba845ff4e8d4dffb302c3d8d6eb837fd
3f6dd0c796 Bump to version 0.18 (Daniela Brozzoni)
Pull request description:
ACKs for top commit:
RCasatta:
ACK 3f6dd0c796
Tree-SHA512: f42f2008d79b9a0334c375652842fb2745d2481422a24f27dd4e350bcc17e47f0da2888545a344e1438ede288ed2ad0de8b359b87004fbe059f310aad886c60a
2d44350b44 Revert "errors if expecing headers notification but not subscribed" (Riccardo Casatta)
Pull request description:
This reverts commit b86f2bb22c.
Some errors started to happen in downstream tests after this commit
#114
ACKs for top commit:
danielabrozzoni:
utACK 2d44350b44
Tree-SHA512: d31174055f4245cc9d99f336b166a44271067b8daecaf2bb55d507ecfa4eb557b9802d576742fba59fd4dfda3f45fc76f02d7896af31353f19fc3a38698ac5a2
the mtgox address used got too many txs, resulting in a server error:
"too many txs"
The address is changed with the peter todd sha256 breaking challenge, which
should be very difficult to spend :) and as of now has just a few txs
f5a438cd0a Bump version to 0.16.0 (Tobin C. Harding)
9a7cc14c94 Update rustls dependency to version 0.21 (Tobin C. Harding)
Pull request description:
Update the `rustls` dependency to version 0.21 and bump the version of this crate to 0.16 so the change can be released.
I don't know what stage in your release cycle you guys are up to so I put the version bump as a separate patch - can drop it if not needed.
Done to help with https://github.com/bitcoindevkit/rust-esplora-client/pull/51
ACKs for top commit:
danielabrozzoni:
utACK f5a438cd0a
Tree-SHA512: 106cbe57651a6e1365f87836598cbec9b1e837f0376ddc1c56cb75e5615543659f5c05cea7d5eb8da8b6cf278fcbe6ef866a019b9829db40cc8055798eb0f541
b86f2bb22c errors if expecing headers notification but not subscribed (Riccardo Casatta)
Pull request description:
the opposite, erroring when subscribing multiple times, is not handled because clients could call multiple times to trigger the server to reply with the tip
ACKs for top commit:
danielabrozzoni:
utACK b86f2bb22c - changes look good to me, didn't test locally
Tree-SHA512: e1a21223448e708cc054271eb1ac5285dc98bdadf7497a856fc4a19ff51655879352ca6ef9f3fb5ec5f9071dd7ee50b2e44d4c65e70694e5d9fa53d280179dd2
the opposite, erroring when subscribing multiple times, is not handled because
clients could call multiple times to trigger the server to reply with the tip
0f356ef94d Bump to v0.15.1 (Alekos Filini)
Pull request description:
Release as a bug fix with #107 merged
ACKs for top commit:
danielabrozzoni:
ACK 0f356ef94d
Tree-SHA512: 3da6a4701707549d62cfa936f5c185eec0967766797dfbacce1fb68b716b90f7e7e163c797dfb7792435cad2769a43e1b23247b0ec86f9a098a79ea4589ee281
3a61037733 Retry client should return AlreadySubscribed error (Riccardo Casatta)
Pull request description:
Users may leverage the Error to avoid remembering client-side the subscription status. eg. Always subscribing and calling script_pop in case AlreadySubscribed is returned.
Backport for version 0.14.0
ACKs for top commit:
afilini:
ACK 3a61037733
Tree-SHA512: d6542227ca03b9f3755cec05115f86de54789cfc49fc87b603b653e0458f6b03ae9fea57fa110221395a4e33cf62a5b2bd7639c4269ae5291e4c92c99ef41eba
Users may leverage the Error to avoid remembering client-side the subscription
status. eg. Always subscribing and calling script_pop in case AlreadySubscribed
is returned.
c925a9179e upgrade bitcoin dep to 0.30.0 (Riccardo Casatta)
Pull request description:
ACKs for top commit:
danielabrozzoni:
ACK c925a9179e
Tree-SHA512: 2b9b7756639af4d931213046866dc36ebee2a5adcbd01c906e27990696b07f1bcf87c2ee009a959f96b17407637801b856e8d4fb9a933fab148f22a619a0d586
80b2adeb4b Make ScriptStatus Serializable (Riccardo Casatta)
Pull request description:
it's common to cache this value client side, so making it Serializable simplify things downstream
ACKs for top commit:
afilini:
ACK 80b2adeb4b
Tree-SHA512: 9467b4f2ee24ee2a684931df31607135f44c624f65d7d3ee64b3991a06de73a07475e4d1a4b8062093500e6a7cb47c1b2ebf094596fca33bf4cc54b26851b910
40cecd5010 Use Borrow for all items in IntoIterator (Riccardo Casatta)
bf2f8ef2e5 script_subscribe IntoIterator accepts Borrow<&Script> (Riccardo Casatta)
156e6fc839 Implement batch call for script subscribe (Riccardo Casatta)
Pull request description:
script_subscribe call hasn't the batch counterpart which may be convenient for multiple subsequent subscribe.
ACKs for top commit:
afilini:
ACK 40cecd5010
Tree-SHA512: 26a3ebb9d1267a224c85b765cf793523771bf7e74990dd804a35693bfebc549f7524214725092eac2276f22cd24eafc72270986549ac02f52641db83fcb71324
Being able to add raw requests to `Batch` makes sense from an API
standpoint because we already allow raw non-batched requests. This is
also useful when the electrum server API gets an updated version and our
client is unable to keep up.
Additional to this, I have improved the documentation and made `Call`
private (since `Call` is never used externally).
561db261d3 `timeout()` and `socks()` don't return Result (Riccardo Casatta)
Pull request description:
The `Result` was necessary in the past because they couldn't both be set,
this is not true anymore after 7493630ed8
ACKs for top commit:
afilini:
ACK 561db261d3
Tree-SHA512: 7654cfbbc7beac6acec786060e373bbc046e1dae0faf2b2ca1df33121e9e92a8f4950c7f8775e44226a62483aeae41c76162cf037ce38137c44d2fedd38560c8
97e60bad4b Bump version to 0.12.0 (Alekos Filini)
Pull request description:
ACKs for top commit:
danielabrozzoni:
ACK 97e60bad4b
Tree-SHA512: e71c2093229552b5317f9309008924a6b2e8f6a48564535d2cd77e6830f342359574518836cfa579460b642c6aa8a2f36b81637b2ee8a48a68f695425333f1a2
