Upto date with reth upstream changes #10
Conversation
There was a problem hiding this comment.
Thanks a lot of the PR. Updating the reth dependency and removing the unnecessary type clutter would be great.
However we will have to keep the custom serialization of the u64 types for the reason given below, so I suggest removing those changes and limiting it only to what is necessary to keep compatibility with the updated reth dependency.
Also please run cargo fmt and cargo clippy to make the CI happy. ( We should probably document this in the README or a separate CONTRIBUTING.md - feel free to open a PR / Issue for any documentation that you feel is missing for a new contributor)
| pub gas_used: u64, | ||
| #[serde(deserialize_with = "as_u64")] | ||
| pub timestamp: u64, | ||
| pub prev_randao: B256, |
There was a problem hiding this comment.
The reason why we had the custom serialization above is because we are currently still aiming to be consistent with the current flashbots builder api. (See: #8)
In their api, numbers are encoded as decimal strings whereas the default deserialization of reth U64 type is hex. Therefore these changes would break the api.
I suggest removing these changes and limiting the PR to only what is necessary to make it compatible with the updated reth dependency.
There was a problem hiding this comment.
Note that this referes to the u64 fields only. The change from H256 to B256 is fine imo.
| ) -> eyre::Result<()> | ||
| where | ||
| Conf: RethRpcConfig, | ||
| Provider: BlockReaderIdExt |
There was a problem hiding this comment.
NIce, this does remove a lot of verbose type noise. Good stuff 👍
| @@ -11,6 +11,6 @@ clap = "4.4.5" | |||
| derivative = "2.2.0" | |||
| eyre = "0.6.8" | |||
| 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.git", rev = "1cccd09" } | |||
There was a problem hiding this comment.
In the other pr (#8) I also changed to using the reth repo directly, without fixing the commit hash.
Looking at the frequency of breaking api changes on the reth side, it might be better though to go with the fixed commit as you did here.
|
Thanks for the contribution Ankit 🙏 |
|
@ankitchiplunkar Thanks again for the contribution. |
No description provided.