refactor!: Sans-io Client + bitreq feature#27
Merged
Conversation
Collaborator
|
The separation of concerns sounds excellent. |
ValuedMammal
force-pushed
the
sans_io_impl
branch
from
6d385d8 to
8be8113
Compare
ValuedMammal
changed the title
Client and async example using bitreq
ValuedMammal
force-pushed
the
sans_io_impl
branch
2 times, most recently
from
55abb04 to
6a0f8ff
Compare
ValuedMammal
force-pushed
the
sans_io_impl
branch
2 times, most recently
from
25a61fc to
e44188c
Compare
tvpeter
reviewed
May 12, 2026
tvpeter
left a comment
tvpeter
left a comment
Collaborator
There was a problem hiding this comment.
tACK e44188c
Thank you @ValuedMammal, for introducing sans-io architecture into the library.
I have left a few suggestions.
Thank you.
Previously, enabling `--all-features` would cause the lowest version feature to take precedence, which is counter-intuitive and incorrect for CI. Make the features additive so that `--all-features` implies the latest version feature. Each version feature explicitly enables the previous one. This means `--all-features` now correctly activates the highest version. Feature cfgs are updated throughout the library and integration tests to match the new behavior. `default` is now the minimum supported version (`28_0`), so that additional features can be given without conflict.
The previous Client owned a `jsonrpc::Client<BitreqHttpTransport>` directly, coupling all users to the bitreq HTTP transport. Introduce a layered architecture: `crate::Client`: transport-agnostic type that manages request building and ID tracking. Callers supply a `send_fn` closure per call. `bitreq::Client`: batteries-included HTTP client behind the `bitreq` feature flag (included in default). Owns a `Box<dyn Transport>` and exposes all RPC methods. Auth and cookie-file parsing live here. `with_auth` now accepts an explicit `timeout: Duration` and is renamed to `with_auth_timeout`. `corepc-types` is now an optional dependency pulled in by `bitreq`. Error variants only reachable through bitreq code are gated behind `bitreq` feature.
ValuedMammal
force-pushed
the
sans_io_impl
branch
from
e44188c to
f85b4d2
Compare
Collaborator
Author
|
tvpeter
approved these changes
May 14, 2026
ValuedMammal
commented
May 18, 2026
| @@ -1,15 +1,17 @@ | |||
| // SPDX-License-Identifier: MIT OR Apache-2.0 | |||
|
|
|||
| //! [`Client`] methods for Bitcoin Core v28.0 and earlier. | |||
Collaborator
Author
There was a problem hiding this comment.
I don't think "and earlier" belongs here.
Suggested change
| //! [`Client`] methods for Bitcoin Core v28.0 and earlier. | |
| //! [`Client`] methods for Bitcoin Core v28.0. |
|
|
||
| use core::fmt; | ||
| use core::num::TryFromIntError; | ||
| #[cfg(feature = "bitreq")] |
Collaborator
Author
There was a problem hiding this comment.
There's a lot of #[cfg(feature = "bitreq"] in the error module which is annoying, don't know if it'd be worth implementing a separate bitreq::Error 🤔
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Refactors the library to a layered, sans-io architecture. The previous
Clientowned ajsonrpc::Client<BitreqHttpTransport>directly, coupling all users to thebitreqHTTP transport. This PR separates concerns:Client: A transport-agnostic type that manages JSON-RPC request building and ID tracking. Callers supply the transport at each call site via asend_fn: Fn(Request) -> Result<Response, E>closure.bitreq::Client(behind thebitreqfeature): A batteries-included HTTP client backed byjsonrpc'sbitreq_httptransport. Owns aBox<dyn Transport>and exposes all Bitcoin Core RPC methods.Auth, cookie-file parsing, andwith_auth_timeoutlive here.Rpcenum: Strongly-typed RPC method names whoseDisplayimpl produces the exact lowercase method string expected by Bitcoin Core.Additional cleanup:
Errorvariants that are only reachable throughbitreqcode are now gated behind#[cfg(feature = "bitreq")].corepc-typesis now an optional dependency, only pulled in bybitreq.with_authnow accepts an explicittimeout: Durationparameter instead of a hardcoded 60-second value.Notes to the reviewers
The
bitreqfeature is included indefault, so existing users who don't configure features explicitly will see no change in the available API surface (thebitreq::Clientreplaces the old top-levelClient1:1 for the common case). The new sans-ioClientis an additive API available to users who need a custom transport.Changelog notice
Added
Rpcenum for strongly-typed RPC method names.Changed
crate::Clientis now a transport-agnostic core; callers supply the transport per call via asend_fnclosure.bitreq::Clientreplaces the oldClientas the HTTP client (requiresbitreqfeature, enabled by default).Client::with_authrenamed tobitreq::Client::with_auth_timeoutand now takes an explicittimeout: core::time::Durationparameter.corepc-typesis now an optional dependency (required bybitreqfeature).