feat: implement clientVersionV1 in engine API

[guha-rahul]

for #7668 ,
The implementation for the get_client_version_v1 logic in impl<Provider, EngineT> EngineApi<Provider, EngineT> is needed to be done.

[mattsse]

great start

we want to give the client info as argument to the new function:

reth/crates/rpc/rpc-engine-api/src/engine_api.rs

Line 60 in 7cda594

pub fn new(

and we create a default instance using the constants that are defined here:

reth/crates/node-core/src/version.rs

Line 4 in 7cda594

[guha-rahul]

sorry i accidentally closed this issue because I synced the commits.

[guha-rahul]

hey should i reopen the pr with a different branch than main?

[onbjerg]

Some nits

If you open your PRs from a branch that isn't main next time, then we can take most of the smaller nits ourselves. We are not allowed to do that on your main branch unfortunately :(

[onbjerg]

also this should be a struct

[onbjerg]

this should be a struct, the spec expects us to send a json object over the wire that looks like

{
  code: "RH",
  name: "Reth",
  version: "v0.2.0-beta.5",
  commit: "defa64b2"
}

[guha-rahul]

hey , when i am trying to compile I am getting two errors environment variable NAME_CLIENT not defined at compile time and environment variable CLIENT_CODE not defined at compile time . I have also added a struct and using that as CLIENTVERSIONV1 type. Do you think you can help me out a bit?

[onbjerg]

hey! that's because those two are not environment variables, they are constants. feel free to push your most recent changes with the struct and i'll take a look:)

[guha-rahul]

I have pushed the changes.

[guha-rahul]

hey @onbjerg , i tried debugging though those errors and after hardcoding some values the errors were solved except for a EngineApi instance in crates/node-builder/src/launch/mod.rs. Can you give me some pointers on so that i can use NAME_CLIENT and CLIENT_CODE as env in compile time.

[onbjerg]

i'm not keen on having those as environment variables as it complicates building and testing

[guha-rahul]

i'm not keen on having those as environment variables as it complicates building and testing

Hey, so I already changed those and in the engine_api.rs I imported the values from the reth_node_core crate but when I try to add to cargo.toml it's showing me a cyclic dependency error (it works without error if I don't add it to cargo.toml). Also when I try the same thing in crates/node-builder/src/launch/mod.rs , its showing me no CLIENTVERSIONV1 in the root. @onbjerg
And should I close this draft and create a new one from a different branch cuz its hindering me from contributing to other issues.

[onbjerg]

sure, open a new pr from a diff branch, this will also allow me to push to it

let's pick up the discussion in the new pr