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

Migrate Sv2 Roles to use rust-bitcoincore-rpc #1123

Closed
average-gary opened this issue Aug 20, 2024 · 8 comments
Closed

Migrate Sv2 Roles to use rust-bitcoincore-rpc #1123

average-gary opened this issue Aug 20, 2024 · 8 comments

Comments

@average-gary
Copy link
Contributor

https://github.com/rust-bitcoin/rust-bitcoincore-rpc

#1095 (comment)
#1095 (review)

There's probably a checklist to be made here for follow on issues/PRs.

@Fi3
Copy link
Collaborator

Fi3 commented Aug 21, 2024

we removed that dependency why do you want re add it? @lorbax

@jbesraa
Copy link
Contributor

jbesraa commented Aug 22, 2024

we removed that dependency why do you want re add it? @lorbax

why was it removed?

@marathon-gary do you mind please explaining why you would prefer rust-bitcoincore-rpc over mini-rpc ?

@average-gary
Copy link
Contributor Author

we removed that dependency why do you want re add it?

I was unaware of this history, I'll dig through issues/PRs to find the reasoning.

do you mind please explaining why you would prefer rust-bitcoincore-rpc over mini-rpc ?

Reducing the amount of code that needs to be maintained is the first thing that comes to mind.
Aside from that, I don't have a technical argument, as I assume mini-rpc works fine.

@average-gary
Copy link
Contributor Author

#761 (comment)
Found this reasoning but didn't dig into what the import problem is. Would be fine to leave as is since it works.

@Fi3
Copy link
Collaborator

Fi3 commented Aug 22, 2024

if I remeber correctly was to have somthing with an async api but not sure maybe @lorbax remeber better.
Btw that remove a dependecy that per se is not bad

@lorbax
Copy link
Collaborator

lorbax commented Aug 23, 2024

json-rpc uses serde for serializing and deserializing. The way it is imported is non compatible with the way it is imported in the libraries. To fix this error requires likely much more work than writing it ex novo.

@jbesraa
Copy link
Contributor

jbesraa commented Aug 23, 2024

Is there a documentation of the actual error?

@lorbax
Copy link
Collaborator

lorbax commented Aug 26, 2024

Is there a documentation of the actual error?

no, but it is easy to try reproduce it, you have to use bitcoincore-rpc and as I remember you get an error by just importing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants