Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Implement eth_getproof JSON RPC API #1824

Merged
merged 24 commits into from
Aug 14, 2019

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Aug 6, 2019

PR description

Implement the eth_getproof JSON RPC API according to the official specification.

  • Request
{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "eth_getProof",
  "params": [
    "0x8f92424781fa8f07bc17e88d201c6c2602857687",
    [  "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "0x0000000000000000000000000000000000000000000000000000000000000001"],
    "900000"
  ]
}
  • Response
{
  "jsonrpc": "2.0",
  "id": 1,
  "result": {
    "accountProof": [
      "0xf90211a08b1d6cc2a2b67f0bb82158b4aa87249db84433a5e2fee423ccdeb....................................................9fe00cb4ed881569ad1939ad0d8076e19fbdf7c18846e36d52c2e80",
      "0xf90211a0ffe4943d011a7974ae842f5bf4e0bbbc82ce6f7e12ca05ed87658....................................................33f750be353ea046431908bca7a3b286214969f8efbe933fcab7380",
      "0xf90211a0a7a2fab4baba244c36703d7481399eb4343c76a06ae787f72ec39....................................................741c31984bfaa821b628bd9c3f16f1cf53b29329d13165d24efae80",
      "0xf8b180a0a70031c295dd22acb08b29a2072152aa5ad5c8ef7eb329f9746d2....................................................14d5b28ce1625e8f8224f6d5a8d8316341706f2bb648e2480808080",
      "0xf8709f2068bbb111b44f1f3fd2e398c686ac81aec360c79054fca4a853153....................................................cda6721fd976b633d3d4612b5a3af701c4efe13a117fac561a1a725"
    ],
    "address": "0x8f92424781fa8f07bc17e88d201c6c2602857687",
    "balance": "0xe429160e9da713c0",
    "codeHash": "0xb9cbe1d72cda6721fd976b633d3d4612b5a3af701c4efe13a117fac561a1a725",
    "nonce": "0x0",
    "storageHash": "0x9f19e92db08d3b4c3de946c4410e0f39c73d3eafb3a64e11aa4cb8c2c9f2f5d3",
    "storageProof": [
      {
        "key": "0x0000000000000000000000000000000000000000000000000000000000000001",
        "value": "0x2",
        "proof": [
          "0xf8d180808080a0f9e25201c207895e14aa7f8aa0bdf8d0781b70a29d5....................................................b17ec695041156a7625418ed0f1dc98217bfbcf3146e1b4d7383680",
          "0xe2a0310e2d527612073b26eecdfd717e6a320cf44b4afac2b0732d9fcbe2b7fa0cf602"
        ]
      },
      {
        "key": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
        "value": "0x0",
        "proof": [
          "0xf8d180808080a0f9e25201c207895e14aa7f8aa0bdf8d0781b70a29d5....................................................b17ec695041156a7625418ed0f1dc98217bfbcf3146e1b4d7383680"
        ]
      }
    ]
  }
}

…feature/pan-2967-eth_getproof

# Conflicts:
#	ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Account.java
…feature/pan-2967-eth_getproof

# Conflicts:
#	ethereum/core/src/main/java/tech/pegasys/pantheon/ethereum/core/Account.java
Copy link
Contributor

@mbaxter mbaxter left a comment

Choose a reason for hiding this comment

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

Great progress on this! Left a few small comments and would like to see some more tests written for the new functionality.

@matkt
Copy link
Contributor Author

matkt commented Aug 11, 2019

Great progress on this! Left a few small comments and would like to see some more tests written for the new functionality.

I made all the changes you requested. I just have a comment. I did not use the class AbstractBlockParameterMethod because it forced me to put the field blockNumber in the first place (in the specification the field is in the third place). If we do not put any field on the json rpc command we have an error Missing required json rpc parameter at index 2 whereas we should have Missing required json rpc parameter at index 0. The second reason is because I have not found a way to return error (with this class it seems to me that we can return only success). Should I use it anyway?

@mbaxter
Copy link
Contributor

mbaxter commented Aug 12, 2019

it forced me to put the field blockNumber in the first place (in the specification the field is in the third place)

You can override the method blockParameter to return the param from a specific index. For example: return getParameters().required(request.getParams(), 2, BlockParameter.class);

I have not found a way to return error

If you override resultByBlockNumber, you can return any JsonRpcSuccessResponse or JsonRpcErrorResponse. Check out EthCall for an example.

@matkt
Copy link
Contributor Author

matkt commented Aug 12, 2019

it forced me to put the field blockNumber in the first place (in the specification the field is in the third place)

You can override the method blockParameter to return the param from a specific index. For example: return getParameters().required(request.getParams(), 2, BlockParameter.class);

I have not found a way to return error

If you override resultByBlockNumber, you can return any JsonRpcSuccessResponse or JsonRpcErrorResponse. Check out EthCall for an example.

Done

@mbaxter
Copy link
Contributor

mbaxter commented Aug 13, 2019

Hey @matkt - this is looking good to me! I pulled down your branch, and did some testing locally. Pushed a few fixes here based on my testing.

If that all looks good to you, I think this should be ready to merge once those changes go in and tests pass. 🎉

[PAN-2967] EthGetProof Review Touch Ups
@matkt
Copy link
Contributor Author

matkt commented Aug 14, 2019

Hey @matkt - this is looking good to me! I pulled down your branch, and did some testing locally. Pushed a few fixes here based on my testing.

If that all looks good to you, I think this should be ready to merge once those changes go in and tests pass.

Merged. thank you for your comments and your patience. I hope all your advice will improve the quality of my future contributions.

@mbaxter mbaxter merged commit e24e7e7 into PegaSysEng:master Aug 14, 2019
@mbaxter
Copy link
Contributor

mbaxter commented Aug 14, 2019

Thanks for contributing!! 💯

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants