feat: implement clientVersionV1 in engine API

[guha-rahul]

fixes #7668 and based on #7708 which I closed because of branch problems
Added ClientVersionV1to engine.rs and engine_api.rs
Need to add ClientVersionV1 in crates/node/builder/src/launch/mod.rs.
Adding reth-node-core.workspace =true in rpc/rpc-engine-api gives me a cyclic dependency issue.

[guha-rahul]

After some debugging i think i have solved those issues but I am getting this error now:-

   --> crates/node/builder/src/launch/mod.rs:430:35
    |
430 |             code: CLIENTVERSIONV1.code,
    |                                   ^^^^ private field

error[E0616]: field `name` of struct `reth_node_core::version::ClientVersionV1` is private
   --> crates/node/builder/src/launch/mod.rs:431:35
    |
431 |             name: CLIENTVERSIONV1.name,
    |                                   ^^^^ private field

error[E0616]: field `version` of struct `reth_node_core::version::ClientVersionV1` is private
   --> crates/node/builder/src/launch/mod.rs:432:38
    |
432 |             version: CLIENTVERSIONV1.version,
    |                                      ^^^^^^^ private field

error[E0616]: field `commit` of struct `reth_node_core::version::ClientVersionV1` is private
   --> crates/node/builder/src/launch/mod.rs:433:37
    |
433 |             commit: CLIENTVERSIONV1.commit,
    |                                     ^^^^^^ private field

any ideas on how to solve this?

[onbjerg]

Yes, you need to mark the fields as public. Please see https://doc.rust-lang.org/rust-by-example/mod/struct_visibility.html

[guha-rahul]

Hey , when I try to run cargo nextest run without adding reth-node-core.workspace =true in my cargo file in crates/rpc/rpc-engine-api, it runs successfully but fails at make pr with the error

use reth_node_core::CLIENTVERSIONV1;
 |         ^^^^^^^^^^^^^^ use of undeclared crate or module `reth_node_core`

If I remove it, then it shows me a cyclic package error

error: cyclic package dependency: package `reth-node-core v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/node-core)` depends on itself. Cycle:
package `reth-node-core v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/node-core)`
 ... which satisfies path dependency `reth-node-core` (locked to 0.2.0-beta.6) of package `reth-rpc-engine-api v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/rpc/rpc-engine-api)`
 ... which satisfies path dependency `reth-rpc-engine-api` (locked to 0.2.0-beta.6) of package `reth-rpc v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/rpc/rpc)`
 ... which satisfies path dependency `reth-rpc` (locked to 0.2.0-beta.6) of package `reth-node-core v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/node-core)`
 ... which satisfies path dependency `reth-node-core` (locked to 0.2.0-beta.6) of package `reth-exex v0.2.0-beta.6 (/home/rahul/Reth/reth/crates/exex)`
 ... which satisfies path dependency `reth-exex` (locked to 0.2.0-beta.6) of package `reth v0.2.0-beta.6 (/home/rahul/Reth/reth/bin/reth)`
 ... which satisfies path dependency `reth` (locked to 0.2.0-beta.6) of package `beacon-api-sse v0.0.0 (/home/rahul/Reth/reth/examples/beacon-api-sse)`
 ```

Any pointers on how I can get rid of this?

[onbjerg]

@guha-rahul Yes, don't use CLIENTVERSIONV1 in the tests for the method, just fill it with dummy fields, which is fine for testing. This should get rid of the cyclical dependency

[guha-rahul]

@onbjerg Resolved all the errors, should i change it from draft to normal?

[Rjected]

@guha-rahul could you solve merge conflicts here?

bump @onbjerg for review

[Rjected]

one suggestion, the implementation looks good! otherwise, I think putting this is reth rpc-types is fine for now given the only added dep is for node-builder, which imports a lot already. I do wonder whether or not it fits better in another crate, but that should not be a blocker for this pr.

[guha-rahul]

@Rjected made the changes and i commented out the CLIENTVERSIONV1 struct from version.rs since it was causing many problems and defined it where the engine-api is initialised. I think we can make do with it being in the rpc-types but do let me know if you want to put it in another crate.

[mattsse]

ty

pending @Rjected

[Rjected]

just one thing, otherwise lgtm

[Rjected]

lgtm, thanks!

resolved