Changes to enable validation api extension #4783
Conversation
ckoopmann
requested review from
mattsse,
Rjected,
rakita,
joshieDo,
onbjerg and
gakonst
as code owners
rkrasiuk
added
C-debt
Codecov Report
... and 11 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Thanks for the suggestion, Good point. So the alternative would be to reimplement / rewrite that function if I don't add the AccountReader interface ?🤔 |
|
i see, fair enough. @ckoopmann pls fix the failing docs check |
Should be fixed now (passing locally). Please re-approve ci run🙏 |
ckoopmann
force-pushed
the
changes-to-enable-validation-api-extension
branch
from
0f3147b to
01789de
Compare
crates/rpc/rpc/src/result.rs
Outdated
| @@ -105,7 +106,7 @@ impl_to_rpc_result!(reth_interfaces::RethError); | |||
| impl_to_rpc_result!(reth_network_api::NetworkError); | |||
|
|
|||
| /// An extension to used to apply error conversions to various result types | |||
| pub(crate) trait ToRpcResultExt { | |||
| pub trait ToRpcResultExt { | |||
There was a problem hiding this comment.
I'm okay with making this pub
There was a problem hiding this comment.
I'm not using this trait anymore in my specific use case. Instead I made the (more useful) ToRpcResult trait public:
f672f19
Therefore I removed the above change and kept the ...Ext trait crate internal.
crates/rpc/rpc/src/result.rs
Outdated
| impl ToRpcResultExt for RethResult<()> { | ||
| type Ok = (); | ||
|
|
||
| fn map_ok_or_rpc_err(self) -> RpcResult<<Self as ToRpcResultExt>::Ok> { | ||
| match self { | ||
| Ok(()) => Ok(()), | ||
| Err(err) => Err(internal_rpc_err(err.to_string())), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think this is very useful
There was a problem hiding this comment.
Removed in favour of ToRpcResult implementation:
f0247ad
crates/rpc/rpc/src/result.rs
Outdated
| impl ToRpcResultExt for Result<SealedBlock, PayloadError> { | ||
| type Ok = SealedBlock; | ||
|
|
||
| fn map_ok_or_rpc_err(self) -> RpcResult<<Self as ToRpcResultExt>::Ok> { | ||
| match self { | ||
| Ok(block) => Ok(block), | ||
| Err(err) => Err(internal_rpc_err(err.to_string())), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
same, we probably want
impl_to_rpc_result!(PayloadError);
here
|
@ckoopmann please resolve conflicts, unable to push manually to this branch |
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
ckoopmann
force-pushed
the
changes-to-enable-validation-api-extension
branch
from
4f352f9 to
6e2a642
Compare
Resolved conflicts in rebase onto latest main. |
These are some changes that I did to enable a specific rpc extension for validating builder payloads, see:
ultrasoundmoney/reth-payload-validator#1
as well as:
ultrasoundmoney#5
The changes I had to do where:
Re-export
reth-rpc-types-compatto get access totry_into_sealed_blocksee hereRe-export
reth-consensus-commonto get access tofull_validationsee hereExport
ExecutionPayloadV1, ExecutionPayloadV2to implement a method that converts from my struct class into theExecutionPayloadtype (necessary because of some different formatting in our api). see hereRe-export
reth_rpc::resultto convert a RethResult to an RPC Result and add aditional implementations ofmap_ok_or_rpc_error. see hereAdd
AccountReadertrait in a few places sincefull_validationrequires it.I am also more than open to alternatives to using the above mentioned methods / datatypes in case those were intentionally not include din the library api.