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

feat(rpc): add rpc call to query output at block #102

Merged
merged 9 commits into from
Apr 24, 2023

Conversation

shrimalmadhur
Copy link
Contributor

@shrimalmadhur shrimalmadhur commented Apr 20, 2023

  • Added RPC to get latest output root at block

TODO

  • RPC Server
  • Return actual output root
  • Clarify from address and inputs to get_proof
  • How to run and test the endpoint

Sample request/response

Request

{
    "jsonrpc": "2.0",
    "method": "optimism_outputAtBlock",
    "params": [4074155],
    "id": 1
}

Response

{
    "jsonrpc": "2.0",
    "result": {
        "outputRoot": "0xd644fd2ec6bd24785902a47f16b243af1a08db9c8fd8d148278b7177712858e6",
        "version": "0x0000000000000000000000000000000000000000000000000000000000000000",
        "stateRoot": "0x762473ad22b64cd68f4704b2af32b88ca32b81b324713ef6773556ff8bb4674e",
        "withdrawalStorageRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"
    },
    "id": 1
}

Error handling, unit tests coming in subsequent PR.

Fixes #86

@shrimalmadhur
Copy link
Contributor Author

@ncitron so here's my first ever Rust PR. From what I understand this is what I got. Would love to get some feedback on general rust practices and of course if this is totally NOT what you expected, let me know.

Also how do I test it? I am still syncing the base-goerli node, can I just kill the docker and build it again and restart?

@ncitron
Copy link
Contributor

ncitron commented Apr 20, 2023

This is a good start. Structurally things look about right.

One thing that I think got mixed up is that you are attempting to make a request to some endpoint in this code, but we want to actually host the endpoint. We want to host the optimism_outputAtBlock endpoint. Here is a good example of how to use jsonrpsee to create new endpoints. The logic for the endpoint should then use the ethers crate to create a provider for the L2 like we do here to fetch the necessary data for computing the result. You will need to fetch the L2 block to get the state root and block hash, and use getProof to get the storage root that you need.

To test it out you can just start syncing to base or optimism. You don't even need to sync to the head, just fetch the result for a block number you have already passed and we should be able to compute the output root for it.

@shrimalmadhur
Copy link
Contributor Author

This is a good start. Structurally things look about right.

One thing that I think got mixed up is that you are attempting to make a request to some endpoint in this code, but we want to actually host the endpoint. We want to host the optimism_outputAtBlock endpoint. Here is a good example of how to use jsonrpsee to create new endpoints. The logic for the endpoint should then use the ethers crate to create a provider for the L2 like we do here to fetch the necessary data for computing the result. You will need to fetch the L2 block to get the state root and block hash, and use getProof to get the storage root that you need.

To test it out you can just start syncing to base or optimism. You don't even need to sync to the head, just fetch the result for a block number you have already passed and we should be able to compute the output root for it.

whooops. I see. that makes sense. thanks for the detailed explanation. Let me get back in a bit with changes

@shrimalmadhur
Copy link
Contributor Author

This is a good start. Structurally things look about right.

One thing that I think got mixed up is that you are attempting to make a request to some endpoint in this code, but we want to actually host the endpoint. We want to host the optimism_outputAtBlock endpoint. Here is a good example of how to use jsonrpsee to create new endpoints. The logic for the endpoint should then use the ethers crate to create a provider for the L2 like we do here to fetch the necessary data for computing the result. You will need to fetch the L2 block to get the state root and block hash, and use getProof to get the storage root that you need.

To test it out you can just start syncing to base or optimism. You don't even need to sync to the head, just fetch the result for a block number you have already passed and we should be able to compute the output root for it.

@ncitron Magi doesn't have any RPC server right now right, I am assuming I will create a new one on a new port? Is there any specific port you would like for that?

@ncitron
Copy link
Contributor

ncitron commented Apr 20, 2023

Yep. Let's use port 9545 which is what I've seen lots of op-node deploys do for now. We can always make it configurable in a follow up PR

@shrimalmadhur
Copy link
Contributor Author

Hey @ncitron , I got the rpc server working. Take a look. I tested it by running it locally. works as expected

Now I am trying to construction the output root. I see the method is like this

async fn get_proof<T: Into<NameOrAddress> + Send + Sync>(
        &self,
        from: T,
        locations: Vec<H256>,
        block: Option<BlockId>,
    )
  • locations here is I am assuming is the state_root
  • block is block_hash

I am not sure what from would be. According to op-node code, it seems to be a pre-deployed address. Where do have the reference from here. I saw the config but couldn't really find which one to use.

@ncitron
Copy link
Contributor

ncitron commented Apr 21, 2023

For fetching the withdrawal_storage_root value, we need to use getProof and provide 0xEF2ec5A5465f075E010BE70966a8667c94BCe15a as the address for optimism Goerli (the message passer contract). We should add this to ChainConfig. I'll track down the right address for base too. Locations can actually be an empty array. The result should have a storage_hash field somewhere in it which is what we need.

For finding the state_root value, we can use the get block method for the given block. Every block has a state_root field.

@shrimalmadhur
Copy link
Contributor Author

For fetching the withdrawal_storage_root value, we need to use getProof and provide 0xEF2ec5A5465f075E010BE70966a8667c94BCe15a as the address for optimism Goerli (the message passer contract). We should add this to ChainConfig. I'll track down the right address for base too. Locations can actually be an empty array. The result should have a storage_hash field somewhere in it which is what we need.

For finding the state_root value, we can use the get block method for the given block. Every block has a state_root field.

Yea I think I already have other fields coded in. I'll add the message passer contract - at least for OP goerli.

@shrimalmadhur shrimalmadhur force-pushed the madhur/add-output-atblock_rpc branch 3 times, most recently from 7a08312 to 26ad89a Compare April 22, 2023 19:35
@shrimalmadhur
Copy link
Contributor Author

For fetching the withdrawal_storage_root value, we need to use getProof and provide 0xEF2ec5A5465f075E010BE70966a8667c94BCe15a as the address for optimism Goerli (the message passer contract). We should add this to ChainConfig. I'll track down the right address for base too. Locations can actually be an empty array. The result should have a storage_hash field somewhere in it which is what we need.

For finding the state_root value, we can use the get block method for the given block. Every block has a state_root field.

using this address for optimism-goerli I am getting this error when I call get_proof

thread 'tokio-runtime-worker' panicked at 'called `Result::unwrap()` on an `Err` value: JsonRpcClientError(JsonRpcError(JsonRpcError { code: -32000, message: "missing trie node a644a44b847d28bf4a4791220762cea7995ff60d45063cbbc926fa6eb2e83149 (path ) <nil>", data: None }))', src/rpc/mod.rs:47:14

I would assume this error would come when the node is pruned or something. any suggestions on how to debug this @ncitron ?

@ncitron
Copy link
Contributor

ncitron commented Apr 24, 2023

Yeah this error usually happens when you are trying to get a proof from an old block number. For now, I think it is fine if we only support getting the output root near the head (I think geth full nodes can keep track of 128 blocks behind iirc). To test, you just want to get a output root near the head of where op-geth is.

@shrimalmadhur shrimalmadhur marked this pull request as ready for review April 24, 2023 18:12
@shrimalmadhur
Copy link
Contributor Author

@ncitron this is ready to review. You can see the request/response in the description. There are few more tasks (rpc port from config, some error handling etc) - I am thinking of tackling them in subsequent PR. Let me know what you think

@shrimalmadhur shrimalmadhur changed the title add rpc call to query output at block feat(rpc) add rpc call to query output at block Apr 24, 2023
@shrimalmadhur shrimalmadhur changed the title feat(rpc) add rpc call to query output at block feat(rpc): add rpc call to query output at block Apr 24, 2023
@shrimalmadhur
Copy link
Contributor Author

shrimalmadhur commented Apr 24, 2023

@ncitron this is ready to review. You can see the request/response in the description. There are few more tasks (rpc port from config, some error handling etc) - I am thinking of tackling them in subsequent PR. Let me know what you think

sorry I pushed one more commit. Added the correct address for Base L2toL1MessagePasser using https://docs.base.org/network-information

Copy link
Contributor

@ncitron ncitron left a comment

Choose a reason for hiding this comment

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

This is looking really good! Just have a couple of things I'd like to see cleaned up before we merge this.

Cargo.toml Outdated Show resolved Hide resolved
src/config/mod.rs Outdated Show resolved Hide resolved
src/driver/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
src/rpc/mod.rs Outdated Show resolved Hide resolved
@ncitron
Copy link
Contributor

ncitron commented Apr 24, 2023

Looks like you have a formatting error. You can run cargo fmt on your code to resolve.

@shrimalmadhur
Copy link
Contributor Author

Looks like you have a formatting error. You can run cargo fmt on your code to resolve.

whoops. Done. thanks!

Copy link
Contributor

@ncitron ncitron left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks for the contribution! This will allow us to interact with services such as clabby/op-challenger to automatically detect and challenge fraud which is a huge win for safety of the entire OP Stack.

@ncitron ncitron merged commit 7884956 into a16z:master Apr 24, 2023
@shrimalmadhur
Copy link
Contributor Author

LGTM!

Thanks for the contribution! This will allow us to interact with services such as clabby/op-challenger to automatically detect and challenge fraud which is a huge win for safety of the entire OP Stack.

Really appreciate all the help with my first Rust change (thanks for being patient). Will be happy to contribute more.

@shrimalmadhur shrimalmadhur deleted the madhur/add-output-atblock_rpc branch April 24, 2023 23:20
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.

feat: add output root rpc method
2 participants