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

Fix json-rpc version string #65

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

GitGab19
Copy link

@GitGab19 GitGab19 commented Jun 5, 2024

After running into issues while testing public-pool using the latest main commit of bitcoin-core, I finally found that the root cause of my pain was the json-rpc version field which is used by this library.

According to json-rpc spec, the json-rpc version HAS to be a string. But here it's created as a number, in the body used for rpc request:

const body = { method, params, jsonrpc: 1.0, id: "rpc-bitcoin" };

This leads to an error from bitcoin-core, since in this PR that has been merged a check was introduced. The line where the error is generated is here: https://github.com/bitcoin/bitcoin/blob/ff7d2054c4f1d7ff98078b9695e7c36e79a476c6/src/rpc/request.cpp#L197

This PR fixes this incompatibility, by using a string instead of a number there.

Copy link

@0xBEEFCAF3 0xBEEFCAF3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also running into this. tACK eb0c5da

@remcoros
Copy link

remcoros commented Oct 2, 2024

tACK eb0c5da

@Sjors
Copy link

Sjors commented Oct 3, 2024

Afaik the "jsonrpc" field doesn't exist in the 1.0 spec: http://www.simple-is-better.org/rpc/#differences-between-1-0-and-2-0

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

Successfully merging this pull request may close these issues.

4 participants