Align with api format of flashbots builder #8
Conversation
…t as flashbots api
ckoopmann
force-pushed
the
testing-and-aligning-api-format
branch
from
f108cdc to
f49c10a
Compare
Cargo.toml
Outdated
| jsonrpsee = "0.20.1" | ||
| reth = { git = "https://github.com/ultrasoundmoney/reth-block-validator", branch = "changes-to-enable-validation-api-extension" } | ||
| reth = { git = "https://github.com/paradigmxyz/reth" } |
There was a problem hiding this comment.
As you can see I just import the latest commit from reth. This could lead to it breaking when they push a breaking change. Should I instead pin a commit ? (Then we miss continous updates though unless we manually update).
Unfortunately there seems to be no releases on crates.io yet so we can't do any more advanced version patterns. (i think)
There was a problem hiding this comment.
Yes, let's pin a commit. Having the code we depend on change between builds is a no-go. Maybe a tag?
Let's try to summon @gakonst. What is the thinking here for projects to depend on Reth? Are published version numbers coming on crates, should we use tags?
There was a problem hiding this comment.
Niclas pointed out the lockfile may keep things pinned already.
From the cargo book
Once a git dependency has been added, Cargo will lock that dependency to the latest commit at the time. New commits will not be pulled down automatically once the lock is in place. However, they can be pulled down manually with cargo update.
So not as necessary, still think it would be good to be explicit and not have to browse lockfiles to see what code we depend on.
There was a problem hiding this comment.
Note that I opened an issue in reth asking for more documentation on this:
paradigmxyz/reth#4898
There was a problem hiding this comment.
I explicitly fixed the commit hash to the one that I extracted from the Cargo.lock. (also started tracking the previously git-ignored lockfile)
Cargo.toml
Outdated
| jsonrpsee = "0.20.1" | ||
| reth = { git = "https://github.com/ultrasoundmoney/reth-block-validator", branch = "changes-to-enable-validation-api-extension" } | ||
| reth = { git = "https://github.com/paradigmxyz/reth" } |
There was a problem hiding this comment.
Yes, let's pin a commit. Having the code we depend on change between builds is a no-go. Maybe a tag?
Let's try to summon @gakonst. What is the thinking here for projects to depend on Reth? Are published version numbers coming on crates, should we use tags?
| @@ -0,0 +1,12 @@ | |||
| use std::sync::Arc; | |||
|
|
|||
| pub mod cli_ext; | |||
There was a problem hiding this comment.
You wrote a CLI for this? Neat.
There was a problem hiding this comment.
Well it is really just an extension of the reth cli. I mostly just followed their example. But yes it is quite neat.
| actual: T, | ||
| ) -> RpcResult<()> { | ||
| if expected != actual { | ||
| Err(internal_rpc_err(format!( |
There was a problem hiding this comment.
What does this mean for the return type? Is flashbots current code returning internal errors for block validation issues? I'd say the obvious return here is 400 not 500, but if their code does 500 let's stick with 500 for now.
There was a problem hiding this comment.
On the http level those should still be successfull (200) requests afaik. However they will contain an error json rpc message with a detailed error code and reason. (which is different from http error codes).
See this SO discussion for reference. I think this is the same behaviour as on flashbots although the error reasons / messages might be different.
Does that answer your question ?
There was a problem hiding this comment.
You can see an example of this by running the test script against an instance of the validator. (either new or flashbots)
| "params":[], | ||
| "id":1 | ||
| }' |\ | ||
| jq --slurpfile payload crates/rpc/rpc-types/test_data/validation/execution_payload.json '.params += $payload' |
There was a problem hiding this comment.
Hm. Interesting use of jq. AI? 👀
There was a problem hiding this comment.
No AI, just good old stackoverflow + youtube tutorial + documentation😅
There was a problem hiding this comment.
I think i tried using ai in between to debug my jq usage actually but had to revert to good old trial and error.
These changes should adjust the api format to be consistent with the one on the flashbots builder, thereby enabling drop in replacement / testing.
flashbots_validateBuilderSubmissionV2ValidationRequestBodywhich includes themessage,execution_payloadandsignaturefields. (same as on flashbots endpoint.block_hashparent_hashbetweenmessageand the block generated fromexecution_payload. (analogous to flashbots logic here).Error messages will not necessarily be the same since they are bubbled up from lower in the stack (geth vs. reth implementation).
This branch is not yet functionally complete / equivalent with flashbots. In particular the verification of the proposer payment is still missing. (see #7)
Additionally this PR: