reth icon indicating copy to clipboard operation
reth copied to clipboard

feat: implement clientVersionV1 in engine API

Open guha-rahul opened this issue 9 months ago • 5 comments

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 avatar May 01 '24 08:05 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?

guha-rahul avatar May 01 '24 12:05 guha-rahul

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

onbjerg avatar May 01 '24 14:05 onbjerg

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?

guha-rahul avatar May 01 '24 16:05 guha-rahul

@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

onbjerg avatar May 01 '24 18:05 onbjerg

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

guha-rahul avatar May 01 '24 19:05 guha-rahul

@guha-rahul could you solve merge conflicts here?

bump @onbjerg for review

Rjected avatar May 22 '24 20:05 Rjected

@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.

guha-rahul avatar May 23 '24 23:05 guha-rahul