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

Relocating jsonrpc endpoints #1273

Closed
njgheorghita opened this issue Mar 12, 2019 · 3 comments
Closed

Relocating jsonrpc endpoints #1273

njgheorghita opened this issue Mar 12, 2019 · 3 comments

Comments

@njgheorghita
Copy link
Contributor

njgheorghita commented Mar 12, 2019

What was wrong?

Web3's api needs updating to match EIP 1474, and the Parity / Geth supported jsonrpc endpoints.

Configuration.

Only endpoints listed in EIP 1474 should be available under the web3 namespace

  • eg. eth_signTransaction -> web3.eth.signTransaction

Endpoints supported by Geth but not defined in EIP 1474 should be available under the web3.geth namespace

  • eg. txpool_status -> web3.geth.txpool.status

Endpoints supported by Parity but not defined in EIP 1474 should be available under the web3.parity namespace

  • eg. shh_post -> web3.parity.shh.post

EIP 1474 defined endpoints currently missing in Web3

  • eth_signTransaction
  • eth_signTypedData
  • eth_submitHashrate
  • eth_submitWork

What needs moving in Web3?

  • web3.shh -> web3.parity.shh and web3.geth.shh
  • web3.version.ethereum -> web3.eth.protocolVersion
  • web3.version.node -> ??? (jsonrpc method = web3_clientVersion)
  • deprecate web3.version api entirely?
  • web3.txpool -> web3.geth.txpool
  • web3.miner -> web3.geth.miner
  • web3.admin -> web3.geth.admin

@kclowes

How can it be fixed?

Fill this section in if you know how this could or should be fixed.

@njgheorghita
Copy link
Contributor Author

njgheorghita commented Mar 18, 2019

@kclowes From what I can tell, we're very close to wrapping this up. One concern I have is that the reference I've been using for the Geth-implemented endpoints might be outdated (last updated 2017). Have you come across any better resources?

Somewhat related to the above point, but from what I can tell, neither Geth nor Parity have implemented eth_signTypedData which is defined in EIP 1474. Should we still implement it in eth.py?

As of now, web3.version only has one endpoint left in it, web3.version.api which returns the web3 api version number. IMO, it's worth considering moving this endpoint to the web3 namespace and deprecating the web3.version namespace entirely. Thoughts?

Another question about the documentation layout. A lot of the recently shuffled around modules (admin, miner, etc...) have their own documentation page. While these pages explain how to use their respective endpoints properly, it might be confusing to users since we no longer have those namespaces web3.admin, web3.miner, etc.. Any thoughts on a better doc layout? Maybe moving all the documentation for non-standard (EIP 1474) endpoints to the documentation page for clients that support them (i.e. geth.rst will contain docs for GethAdmin, GethMiner ....)

Lastly, this has been discussed in various places, but adding typehints, doctest, and autodoc are all good targets to hit in V5 and probably deserve their own issue (though as @pipermerriam said, they aren't breaking changes so it won't block the V5 release)

@kclowes
Copy link
Collaborator

kclowes commented Mar 18, 2019

Have you come across any better resources?

This is the link I've been using. Looks like it was updated a little more recently than 2017 :)

neither Geth nor Parity have implemented eth_signTypedData which is defined in EIP 1474. Should we still implement it in eth.py?

I think they have both implemented and merged (see this issue but maybe there are no docs yet. And maybe it hasn't been released yet? I'm not sure how geth/parity releases work. @Bhargavasomu is implementing in the eth-account repo. See this WIP PR: #1241. And note that EIP 1474 is wrong here.

IMO, it's worth considering moving this endpoint to the web3 namespace and deprecating the web3.version namespace entirely. Thoughts?

Yeah, I think that makes sense.

Maybe moving all the documentation for non-standard (EIP 1474) endpoints to the documentation page for clients that support them (i.e. geth.rst will contain docs for GethAdmin, GethMiner ....)

Yeah, moving them to a geth.rst and a parity.rst was my first thought too. I think that makes sense.

adding typehints, doctest, and autodoc are all good targets to hit in V5 and probably deserve their own issue

yes! I made an issue for increasing doctest coverage, but I don't see one for typehints or autodoc. Do you mind adding those in?

Thanks @njgheorghita!

@kclowes
Copy link
Collaborator

kclowes commented Jun 13, 2019

@njgheorghita can this be closed?

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

2 participants