Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fork or create new crate to replace rust-bitcoincore-rpc #1589

Open
notmandatory opened this issue Sep 3, 2024 · 6 comments
Open

Fork or create new crate to replace rust-bitcoincore-rpc #1589

notmandatory opened this issue Sep 3, 2024 · 6 comments
Labels
discussion There's still a discussion ongoing new feature New feature or request

Comments

@notmandatory
Copy link
Member

Describe the enhancement

Since rust-bitcoincore-rpc may not be updated as often as we need it was suggested on rust-bitcoin/rust-bitcoincore-rpc#365 that we may want to create our own BDK version of it so we can get changes merged sooner.

Use case

More frequent updates would help us get features we need sooner, like supporting pruned bitcoind nodes.

Additional context

rust-bitcoin/rust-bitcoincore-rpc#365 (comment)

@notmandatory notmandatory added new feature New feature or request discussion There's still a discussion ongoing labels Sep 3, 2024
@tnull
Copy link
Contributor

tnull commented Sep 4, 2024

I think it would make sense to build on the new rust-bitcoincore-json-rpc and, eventually, to have (at least basic) RPC/REST clients live in rust-bitcoincore-json-rpc itself (cf. this discussion here: rust-bitcoin/rust-bitcoin#3225 (reply in thread)).

@luisschwab
Copy link

It would make sense to me for rust-bitcoind-json-rpc to have it's own client, but since there doesn't seem to be much interest in maintaining that right now, we could add it as a new crate now, supporting what matters to BDK and maybe evolve that into a full-fledged client and port it to rust-bitcoind-json-rpc in the future.

@tnull
Copy link
Contributor

tnull commented Sep 5, 2024

It would make sense to me for rust-bitcoind-json-rpc to have it's own client

Well, it already has a blocking client, although not fully production-ready as this stage.

but since there doesn't seem to be much interest in maintaining that right now, we could add it as a new crate now, supporting what matters to BDK and maybe evolve that into a full-fledged client and port it to rust-bitcoind-json-rpc in the future.

Right, my main point is that we should build upon rust-bitcoind-json-rpc, not a fork of rust-bitcoincore-rpc going forward. That said, if someone already were to put more work into to making the client more production-ready, I'm sure the maintainers wouldn't completely disagree to have the changes upstreamed? (cc @tcharding ?)

@storopoli
Copy link
Contributor

I might have some changes to make upstream, I am working in a bitcoincore async client at @alpenlabs and had to do some improvements in the bitcoind-json-rpc-types

@tcharding
Copy link
Contributor

The thing is, no one in rust-bitcoin org (mainly me and Andrew) knows much about or cares about async. We are trying to provide rust-bitcoind-json-rpc as a minimum service to the community but we don't want the burden of maintaining async/sync + every feature every application wants. Steven basically does not maintain rust-bitcoincore-rpc so you guys are left, in my personal opinion, to either fork rust-bitcoincore-rpc or write your own client (and use types from bitcoind-json-rpc-types. (The sync client in bitcoind-json-rpc-client is, as @tnull says, explicitly for integration testing, this much I can and will maintain.)

If a bunch of you disagree with my "users should create their own production client" then you guys could build a client on top of bitcoind-json-rpc-types and release and maintain it. It can go in the rust-bitcoin org, hell it can even go in the bitcoind-json-rpc repo if someone commits to maintaining it and I don't have to do it. I am more than happy to share ownership (I specifically put the client code in client/src/client_sync incase someone wanted to add an async client).

@tnull
Copy link
Contributor

tnull commented Sep 6, 2024

If a bunch of you disagree with my "users should create their own production client" then you guys could build a client on top of bitcoind-json-rpc-types and release and maintain it. It can go in the rust-bitcoin org, hell it can even go in the bitcoind-json-rpc repo if someone commits to maintaining it and I don't have to do it. I am more than happy to share ownership (I specifically put the client code in client/src/client_sync incase someone wanted to add an async client).

IMO, we indeed should build and maintain such clients in a single place as otherwise everyone will need to build there own, which is just more total work. It would also hopefully result in having more eyes on less code, which is more generally a good idea. My preferred place for it would indeed be in the bitcoind-json-rpc repo since the foundation is already there. FWIW, given that we'll end up using/depending on these clients likely in multiple projects, I'd be happy to do some review on it from time to time, at least as long we're clear on the scope of it and we agree that we'd still want to keep it simple and that we won't accommodate every feature under the sun.

How we'd want to proceed implementing the async client is IMO not immediately clear as we previously concluded to drop reqwest from other similar projects (e.g., rust-esplora-client), but until someone finds the time to work on forking minreq/writing our own client it seems to be the only available dependency in the short term? Maybe the preferable alternative would be to start on a fork of the simple HTTP client we currently use in LDK (https://github.com/lightningdevkit/rust-lightning/blob/main/lightning-block-sync/src/http.rs) until the proper 'rewrite a sane HTTP client' project comes to fruition?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion There's still a discussion ongoing new feature New feature or request
Projects
Status: No status
Development

No branches or pull requests

5 participants