Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Implement all JSON-RPC endpoints #26

Closed
36 of 42 tasks
carver opened this issue May 30, 2018 · 26 comments
Closed
36 of 42 tasks

Implement all JSON-RPC endpoints #26

carver opened this issue May 30, 2018 · 26 comments

Comments

@carver
Copy link
Contributor

carver commented May 30, 2018

Checklist of endpoints to support, and which are supported in master or pull requests

  • support all the web_* methods
  • support all the net_* methods
  • support all the eth_* write methods
    • make list of methods
  • support all the eth_* read-only methods
    • eth_protocolVersion
    • eth_syncing
    • eth_coinbase
    • eth_mining
    • eth_hashrate
    • eth_gasPrice
    • eth_accounts
    • eth_blockNumber
    • eth_getBalance
    • eth_getStorageAt
    • eth_getTransactionCount
    • eth_getBlockTransactionCountByHash
    • eth_getBlockTransactionCountByNumber
    • eth_getUncleCountByBlockHash
    • eth_getUncleCountByBlockNumber block int
    • eth_getUncleCountByBlockNumber block string
    • eth_getCode
    • eth_call
    • eth_estimateGas
    • eth_getBlockByHash
    • eth_getBlockByNumber block int
    • eth_getBlockByNumber block string
    • eth_getTransactionByHash
    • eth_getTransactionByBlockHashAndIndex
    • eth_getTransactionByBlockNumberAndIndex
    • eth_getTransactionReceipt
    • eth_getUncleByBlockHashAndIndex
    • eth_getUncleByBlockNumberAndIndex block int
    • eth_getUncleByBlockNumberAndIndex block string
    • eth_getCompilers (deprecated)
    • eth_getLogs
    • eth_getWork
@MysticRyuujin
Copy link

eth_syncing would be good so we can get a node up on ethstats.net 👍

@pipermerriam
Copy link
Member

@MysticRyuujin yes, we should be able to get that one done quite easily. Will try to get a release out today with that endpoint enabled.

@cburgdorf
Copy link
Contributor

cburgdorf commented Jun 5, 2018

Quick question:

@pipermerriam I think you mentioned that you don't want the JSON-RPC exposed via HTTP directly because there's some tooling developed that exposes a standard HTTP JSON API Server that can connect to any of the popular nodes. Do you have a pointer for that?

And as a follow up question, do I understand it correctly that as of today, the only way for people to connect to the JSON-RPC API is to write this sort of boilerplace code themselves?

https://github.com/ethereum/py-evm/blob/f66d296074c0603eefa4e4c718421956d70d183e/tests/trinity/core/json-rpc/test_ipc.py#L79-L89

If yes, would it be reasonable to at least implement some sort of CLI command to give uses something to test out the JSON-RPC API without implementing such boiler plate code on their end?

@voith
Copy link
Contributor

voith commented Jun 5, 2018

do I understand it correctly that as of today, the only wait for people to connect to the JSON-RPC API is to write this sort of boilerplace code themselves?

@cburgdorf I think users can use web3.py to connect to the JSON-RPC API.

from web3 import Web3
w3 = Web3(Web3.IPCProvider(jsonrpc_ipc_pipe_path))
# then use the w3 instance to call any API of your choice. eg
w3.eth.accounts

@cburgdorf
Copy link
Contributor

@voith Yes, that's true and in fact, I guess that is the preferred way for most use cases. However, that only exposes you to an interface of web3 which then talks to the JSON-RPC API itself. So, if the underlying JSON-RPC API exposes an API that web3 does not support there's no simple way to run requests against the underlying JSON-API directly I assume.

However, I realize that web3 basically seems to expose the entire underlying API so that may just be the way to go 👍

cburgdorf referenced this issue in cburgdorf/py-evm Jun 5, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 5, 2018
@voith
Copy link
Contributor

voith commented Jun 5, 2018

if the underlying JSON-RPC API exposes an API that web3 does not support there's no simple way to run requests against the underlying JSON-API directly I assume.

IMO, web3 should implement all the API's mentioned in the JSON-RPC spec. Also, web3 has node specifc modules eg. parity. If trinity has any extra API's we could add a trinity module to web3.

@cburgdorf
Copy link
Contributor

IMO, web3 should implement all the API's mentioned in the JSON-RPC spec

Yes, I guess from a users perspective web3 seems the go to solution to query the API. 👍

I'd still be interested to learn deeper about the generic JSON RPC HTTP API that @pipermerriam mentioned at some point. I think connecting to a local node via HTTP would be quite common (e.g. building some sort of website interface that sits on top of a local node) but if I recall correctly Piper mentioned he doesn't want to include such a server in Trinity directly because there's a generic solution being developed that would work with any node (geth, trinity, parity etc) making such functionality directly in Trinity redundant.

cburgdorf referenced this issue in cburgdorf/py-evm Jun 5, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 5, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 5, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 6, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 6, 2018
cburgdorf referenced this issue in cburgdorf/py-evm Jun 6, 2018
@pipermerriam
Copy link
Member

@cburgdorf I just did a quick search and can't seem to find it. I'll let you know when I do. Regardless of whether that tool is being actively developed or looks promising, I still want to hold out on the HTTP API to reduce scope. Since we can get at all of these things using web3.py I think this should be sufficient but let me know if I'm missing something.

cburgdorf referenced this issue in cburgdorf/py-evm Jun 7, 2018
cburgdorf referenced this issue in ethereum/py-evm Jun 7, 2018
@MysticRyuujin
Copy link

I don't know if it's possible to use Trinity with MEW or MyCrypto w/o HTTP API? Just a thought.

@carver
Copy link
Contributor Author

carver commented Jun 7, 2018

The IPC -> HTTP daemon: https://github.com/ethereum/cpp-ethereum/blob/develop/scripts/jsonrpcproxy.py

@MysticRyuujin
Copy link

I just noticed that admin.nodeInfo doesn't work and it's not on this list. Will that be supported?

@carver
Copy link
Contributor Author

carver commented Jun 26, 2018

@MysticRyuujin That method is non-standard across geth/parity, also not in the closest thing we have to a json-rpc standard. I expect us to implement something like it, maybe at a different endpoint name. Either way, the method is not part of this particular issue, which is focused on the endpoints from that "spec".

@veox
Copy link
Contributor

veox commented Oct 9, 2018

@carver (and posterity): there's now work on standartising Ethereum's JSON-RPC in an EIP. (The link is to a WIP branch - i.e., a draft of an EIP in Draft status.)

@pipermerriam
Copy link
Member

!!!!!!!!!!!!!!!!!!!!!!!!! YES !!!!!!!!!!!!!!!!!!!!!!!!! So glad someone is EIP-ing this.

@Bhargavasomu
Copy link
Contributor

Bhargavasomu commented Dec 18, 2018

@pipermerriam @carver all the JSON RPC modules above mentioned have been implemented except for the following.

  • eth_coinbase
  • eth_hashrate
  • eth_gasPrice
  • eth_getTransactionByHash
  • eth_getTransactionReceipt
  • eth_getCompilers (As per specs, it is deprecated. Hence need confirmation for implementation.)
  • eth_getLogs
  • eth_getWork

The ticks have to be updated likewise I guess.

@Bhargavasomu
Copy link
Contributor

Bhargavasomu commented Dec 18, 2018

Also I want to kind of complete these, so that the API would be in a better state. I want to start off with the first 3 modules and I was thinking of the following approach.

eth_coinbase

We could follow the same approach for the eth_blockNumber.
https://github.com/ethereum/py-evm/blob/eb6e9c61fe4c949c4d2d130084752aa80cad818a/trinity/rpc/modules/eth.py#L147-L149
Except we return the coinbase instead of the block number.

eth_gasPrice

  • Get the canonical block.
  • return the Average of the gas_price of all of the transactions of that block.

eth_hashrate

I don't know how to get the hashrate from the current API. I was thinking that maybe we could make a spoof mining operation with the current difficulty, and note the time taken for it to complete. But to calculate the hashrate from that, I need the overall number of hashing operations that took place. So any suggestions please?

Also could you please give me a brief of how they are tested?

@carver
Copy link
Contributor Author

carver commented Dec 18, 2018

eth_gasPrice

  • Get the canonical block.
  • return the Average of the gas_price of all of the transactions of that block.

Getting a useful estimate of gas price is notoriously difficult to do well. Check out some other attempts here: https://github.com/ethereum/web3.py/blob/master/web3/gas_strategies/time_based.py

I think it's okay to do something simple, but I don't think the mean is the best simple thing we can do. Maybe the median or 90th percentile? I think it's okay to "overprice" when the user is lazy about choosing a price, so lean toward the 90th percentile.

--

  • eth_coinbase looks fine.
  • eth_hashrate can definitely be punted. No one should be doing proof of work mining using trinity.

@carver
Copy link
Contributor Author

carver commented Dec 18, 2018

Also could you please give me a brief of how they are tested?

One place to start is the existing tests of some other endpoints, here: https://github.com/ethereum/py-evm/blob/master/tests/trinity/core/json-rpc/test_ipc.py

@Bhargavasomu
Copy link
Contributor

Bhargavasomu commented Dec 19, 2018

@pipermerriam @carver I just noticed the coinbase address which would be present in a block, would be the coinbase address of the miner who mined it, and not the address of the client who is actually using it. So we should actually be returning the address of the client who is using trinity (if at all set to some address, else ZERO_ADDRESS is the default). Is there any configuration settings in trintiy which would allow the get/set of the coinbase address? (Need the address of the first account in other words.)

@pipermerriam
Copy link
Member

Since we don't support mining my opinion is:

  • eth_coinbase can always return the 0x0 address for now.
  • eth_hashrate can always return 0

@pipermerriam
Copy link
Member

pipermerriam commented Dec 19, 2018

RE: eth_gasPrice

Average mechanism seems fine though it will return 0 when a block has no transactions. @carver comments about using a percentile seem like they'd approach better numbers but frankly, I think anything we do is going to have a flaw.

Which suggests this should eventually be pluggable which makes me think that initial implementation should be dead simple since we'll very likely replace it with something that a user can control (probably via a plugin).

@pipermerriam
Copy link
Member

  • eth_getCompilers can be left out (not implemented)
  • eth_getLogs is going to be very complex since currently, we don't store any kind of lookup table for logs.
  • eth_getWork also kind of awkward but we should be able to return valid data if we pull the canonical HEAD and generate an empty block on top of it and then extract the three parameters.

@Bhargavasomu
Copy link
Contributor

Average mechanism seems fine though it will return 0 when a block has no transactions

@pipermerriam we could probably cache the most recent gasPrice that was calculated, and if the current block has no transactions, then we could return the most recently calculated non-zero gas price.

@Bhargavasomu
Copy link
Contributor

Bhargavasomu commented Jan 9, 2019

@pipermerriam as part of implementing eth_getTransactionReceipt, I would like to add a new function def get_transaction_receipt(self, header: BlockHeader, transaction_index: int, receipt_class: Type[Receipt]) -> Receipt to ChainDB. I feel that this function would be good for the API. Please confirm. Thankyou.

EDIT : I would also like to add this method to the FrontierBlock

@cburgdorf
Copy link
Contributor

I believe we can close this for the same reason that we closed #20 (they seemed fairly overlapping) but feel free to reopen in case I misunderstood.

@carver
Copy link
Contributor Author

carver commented Apr 3, 2020

Yeah, I think the "write" methods were the main difference, and since trinity won't host keys for the foreseeable future, I'm good with closing 👍

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

9 participants