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

encoding issue with btcd.rpcclient #5

Open
10 tasks
tiger5226 opened this issue Apr 23, 2018 · 6 comments
Open
10 tasks

encoding issue with btcd.rpcclient #5

tiger5226 opened this issue Apr 23, 2018 · 6 comments
Labels
hacktoberfest Welcome to Hacktoberfest help wanted Extra attention is needed level: 3 Significant knowledge of the existing code is recommended

Comments

@tiger5226
Copy link
Collaborator

tiger5226 commented Apr 23, 2018

The btcd rpcclient has an issue handling certain encoding. This needs to be looked into further as the problem is not obvious. Below is the reproduction so we do not lose track of the problem.

Reproduction:

Added the following code to the top of daemon.DoYourThing()

claims, err := lbrycrd.GetClaimsForName(`¶`)
    spew.Dump(claims, err)
    return

then in a separate window, run sudo tcpdump -i lo port 9245 -A

this lets you see the data sent to and from lbrycrd daemon

when you run ./dev.sh, you get this:

POST / HTTP/1.1
Host: localhost:9245
User-Agent: Go-http-client/1.1
Connection: close
Content-Length: 68
Authorization: Basic bGJyeTpGU0p1cFV2WXBlNzhzbzltcUNRM3RiVGlsYlljU1JSYg==
Content-Type: application/json
Accept-Encoding: gzip

{"jsonrpc":"1.0","method":"getclaimsforname","params":[".."],"id":3}

Looking at the params above .. is not
here is lbrycrd-cli

POST / HTTP/1.1
Host: 127.0.0.1
Connection: close
Authorization: Basic bGJyeTpGU0p1cFV2WXBlNzhzbzltcUNRM3RiVGlsYlljU1JSYg==
Content-Length: 63

{"method":"getclaimsforname","params":["\u00c2\u00b6"],"id":1}

Not sure where the encoding problem is. Using a different RPC client does not cause the same issue.

Acceptance Criteria

Definition of Done

  • Tested against acceptance criteria
  • Tested against the assumptions of the user story
  • The project builds without errors
  • Unit tests are written and passing
  • Tests on devices/browsers listed in the issue have passed
  • QA performed & issues resolved
  • Refactoring completed
  • Any configuration or build changes documented
  • Documentation updated
  • Peer Code Review performed
@tiger5226
Copy link
Collaborator Author

tiger5226 commented Apr 24, 2018

in the RawRequestAsync function in btcd.rpcclient package, adding the following line after the marshalling produced the listed output

println("vendor1",string(marshalledJSON))

output:{"jsonrpc":"1.0","method":"getclaimsforname","params":["¶"],"id":3}

So most likely this is related to something further into the rpcclient library.

EDIT: I went all the way to before it goes back to the httpclient.

@tiger5226 tiger5226 added help wanted Extra attention is needed level: 3 Significant knowledge of the existing code is recommended labels May 13, 2018
@lyoshenka
Copy link
Contributor

@roylee17 is talking about adding LBRY support to btcd. I think this will be solved once he does that, but maybe he can shed some light on this as he's digging in the code.

@tiger5226
Copy link
Collaborator Author

@roylee17 Any update here or idea of what this could be?

@alyssaoc alyssaoc added low prioity priority: low Work should be done but can stay in the backlog for now and removed low prioity labels Aug 29, 2018
@tiger5226
Copy link
Collaborator Author

@lyoshenka What are the next steps here. If we just want to ignore these errors, can we close this issue? The last time I investigated this issue, I did not make much progress. I know the old rpc client worked great.

@lyoshenka
Copy link
Contributor

this is low priority but i don't want to give up on this yet. we need to understand if there's a general problem with jsonrpc, or if we're doing something wrong. i think forgetting about this will come back to bite us later.

@lyoshenka
Copy link
Contributor

the next step is creating the simplest reproduction of this bug that we can. use as little total code/pieces as possible. post code here when its available, or link to a repo

@lyoshenka lyoshenka removed their assignment Sep 26, 2018
@alyssaoc alyssaoc added hacktoberfest Welcome to Hacktoberfest and removed priority: low Work should be done but can stay in the backlog for now labels Oct 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Welcome to Hacktoberfest help wanted Extra attention is needed level: 3 Significant knowledge of the existing code is recommended
Projects
None yet
Development

No branches or pull requests

4 participants