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

Mock implementation of all RPC methods under the eth namespace #10

Merged
merged 6 commits into from
Nov 21, 2023

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Nov 1, 2023

Work towards:

Description

Creates an RPC server, registers a service for the eth namespace, and serves JSON-RPC on a TCP listener.

The BlockChainAPI service which is registered on the eth namespace, implements all the RPC methods described in the above issues. Their implementations either return empty / mock data, or a not implemented/support error according to the description of each issue.

Running this locally, results in:

➜  flow-evm-grpc git:(go-ethereum-json-rpc-example) go run .                                    
Request:  {"jsonrpc":"2.0","id":1,"method":"eth_blockNumber","params": []}
Response:  {"jsonrpc":"2.0","id":1,"result":"0x3ecc3d0"}

Request:  {"jsonrpc":"2.0","id":2,"method":"eth_chainId","params": []}
Response:  {"jsonrpc":"2.0","id":2,"result":"0x309"}

Request:  {"jsonrpc":"2.0","id":3,"method":"eth_syncing","params": []}
Response:  {"jsonrpc":"2.0","id":3,"result":false}

Request:  {"jsonrpc":"2.0","id":4,"method":"eth_getBalance","params": ["0x407d73d8a49eeb85d32cf465507dd71d507100c1"]}
Response:  {"jsonrpc":"2.0","id":4,"result":"0x65"}

Request:  {"jsonrpc":"2.0","id":5,"method":"eth_getBlockTransactionCountByNumber","params": ["0x4E4ee"]}
Response:  {"jsonrpc":"2.0","id":5,"result":"0x20a"}

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@m-Peter m-Peter self-assigned this Nov 1, 2023
@m-Peter m-Peter marked this pull request as draft November 1, 2023 13:40
@devbugging
Copy link
Contributor

Sorry for late reply, let me take a look.

@devbugging
Copy link
Contributor

devbugging commented Nov 9, 2023

@m-Peter I would suggest a different approach. I would first focus on defining the schema for the data you would get from the Flow network related to EVM blocks and transaction results. I would create a mock version and then focus on the indexer of that data. Then I would generate endpoitns based on the ethereum rpc spec. Abstracting away the implementation details of how you get the data from Flow network about the blocks and transactions will allow you to adapt it as the project evolves. The schema might stay more or less similar. Having mocks in place for getting data you can also then implement the proofs you need.

@ramtinms
Copy link
Contributor

ramtinms commented Nov 9, 2023

Even though its a draft, still looks good to me, would review again when it becomes something ready to be merged

@m-Peter m-Peter changed the title Example of utilising the JSON-RPC server implementation of go-ethereum and wiring with flow-go-sdk Mock implementation of all the RPC methods under the eth namespace Nov 17, 2023
@m-Peter m-Peter marked this pull request as ready for review November 17, 2023 13:29
@m-Peter m-Peter changed the title Mock implementation of all the RPC methods under the eth namespace Mock implementation of all RPC methods under the eth namespace Nov 17, 2023
@m-Peter m-Peter requested a review from ramtinms November 17, 2023 13:51
Copy link
Contributor

@peterargue peterargue left a comment

Choose a reason for hiding this comment

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

Looking over the issues, all of the endpoints are accounted for here.

I think it's probably worthwhile to start structuring the project a bit. Rather than putting everything within a main.go, how about a structure like:

/
├─ api/
│  ├─ api.go
│  ├─ server.go
│  ├─ models.go
├─ cmd/
│  ├─ server/
│  │  ├─ main.go

I'm definitely not attached to any specific structure, and this can be done in a separate PR.

With that said, I think this is a fine foundation if you want to merge it in and start iterating in more PRs

main.go Outdated
ChainID *hexutil.Big `json:"chainId,omitempty"`
}

type BlockChainAPI struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would say calling this grpcServer would be more appropriate, because in the future there might be multiple implementation of the "ethereum API".

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be useful to define an interface for this API so you can mock out for testing (maybe too early for this tho)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would say calling this grpcServer would be more appropriate, because in the future there might be multiple implementation of the "ethereum API".

This is not the actual JSON-RPC server, but an object that has to adhere to the Ethereum Specification. E.g.

server := rpc.NewServer()
server.RegisterName("eth", &BlockChainAPI{})

Each exported method of this object (for example BlockNumber) is wired to an RPC method -> eth_blockNumber

It would also be useful to define an interface for this API so you can mock out for testing (maybe too early for this tho)

That's indeed a good idea 👍 And I will also break it down to more types (TransactionsAPI, AccountsAPI, FiltersAPI).

main.go Outdated
// returned, regardless of the current head block. We used to return an error when the chain
// wasn't synced up to a block where EIP-155 is enabled, but this behavior caused issues
// in CL clients.
func (api *BlockChainAPI) ChainId() *hexutil.Big {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there maybe a protobuff you can use to generate the methods needed for eth rpc API?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately there is no such protobuf in the go-ethereum repository. Most like these methods are hand-written.

main.go Outdated
// accessListResult returns an optional accesslist
// It's the result of the `debug_createAccessList` RPC call.
// It contains an error if the transaction itself failed.
type accessListResult struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would put this and some other types under a model package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point 👍 .
I have added a basic project structure, as pointed out by Peter 62a9b84.
For now I have added them in a models.go file, under the api package.

main.go Outdated
}

// eth_feeHistory (transaction fee history)
// FeeHistory returns the fee market history.
Copy link
Contributor

Choose a reason for hiding this comment

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

Explain all the arguments

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added in ed4fe06
I will go over all the methods and get the argument explanation from https://ethereum.github.io/execution-apis/api-documentation/

main.go Outdated
func (s *BlockChainAPI) CreateAccessList(
ctx context.Context,
args TransactionArgs,
blockNrOrHash *rpc.BlockNumberOrHash,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
blockNrOrHash *rpc.BlockNumberOrHash,
BlockNumberOrHash *rpc.BlockNumberOrHash,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 62a9b84

main.go Outdated
func (s *BlockChainAPI) GetStorageAt(
ctx context.Context,
address common.Address,
hexKey string, blockNrOrHash rpc.BlockNumberOrHash,
Copy link
Contributor

Choose a reason for hiding this comment

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

What is hexKey, maybe publicKey? also might be better to not put hex in the name, since you will validate the input anyway and you can detect the encoding. You should put that in the comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point 👍 In the specification it is written:
Returns the value from a storage position at a given address. So hexKey is actually the storage position.
And the accepted value is a hex encoded 256 bit unsigned integer. Most of the naming comes from the go-ethereum` repository, but I will rename the arguments accordingly, to make them more readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in 71ef6d1

main.go Outdated
// only the transaction hash is returned.
func (s *BlockChainAPI) GetBlockByNumber(
ctx context.Context,
number rpc.BlockNumber,
Copy link
Contributor

Choose a reason for hiding this comment

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

I would define enums for above casses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually the go-ethereum repository already defines the following constants:

type BlockNumber int64

const (
	SafeBlockNumber      = BlockNumber(-4)
	FinalizedBlockNumber = BlockNumber(-3)
	LatestBlockNumber    = BlockNumber(-2)
	PendingBlockNumber   = BlockNumber(-1)
	EarliestBlockNumber  = BlockNumber(0)
)

So we can use these.

main.go Outdated
number rpc.BlockNumber,
fullTx bool,
) (map[string]interface{}, error) {
return map[string]interface{}{}, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why returning map[string]any? wouldn't it be better to define a type for it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am assuming that's because the method has to return nil. At least that's what I read on the specification page, and the actual implementation:

block, err := s.b.BlockByNumber(ctx, number)
if block != nil && err == nil {
	response, err := s.rpcMarshalBlock(ctx, block, true, fullTx)
	if err == nil && number == rpc.PendingBlockNumber {
		// Pending blocks need to nil out a few fields
		for _, field := range []string{"hash", "nonce", "miner"} {
			response[field] = nil
		}
	}
	return response, err
}
return nil, err

And there's a special handling for pending blocks, where they have to set 3 fields to nil value.

@devbugging
Copy link
Contributor

I left some comments, I agree with @peterargue some of the structure you should already put in place, you don't have to overdo it at this early state, but also having everything in main will make more changes at later point to refactor. With the same thinking be careful about exposing implementation details in names etc (left some comments on that).

@m-Peter m-Peter force-pushed the go-ethereum-json-rpc-example branch from e1cc0ea to 62a9b84 Compare November 20, 2023 16:57
@m-Peter m-Peter requested a review from devbugging November 21, 2023 13:00
@m-Peter
Copy link
Collaborator Author

m-Peter commented Nov 21, 2023

@sideninja I have addressed / answered all of the comments. Could you take another look? 🙏

@m-Peter m-Peter merged commit 7156cee into onflow:main Nov 21, 2023
@m-Peter m-Peter deleted the go-ethereum-json-rpc-example branch November 21, 2023 19:07
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.

5 participants