Client: eth_chainID and eth_syncing RPC implementations

[gabrocheleau]

This PR implements the following JSON-RPC methods (required to synchronize the ETH2 teku client and the ETH1 client):

• eth_chainID
• eth_syncing

These methods now allow us to further connect the ETH2 and the ETH1 clients for the merge, getting rid of the
Eth1 service down for xxx s, retrying error message

To implement the eth_syncing method, I added a synchronized boolean property to the Client class. This property is initialized as false and gets set to true when the synchronized event is emitted.

I also added a startingBlock property to the Synchronizer class. It is initialized as 0 and gets set to the current local head block number once the open method is called.

One thing worth mentioning is that we don't, at the moment, handle the case where the client would go from the synchronized state to the not synchronized state (e.g. when falling behind other peers).

[codecov]

Codecov Report

Merging #1314 (9562b5e) into master (9c5d6ea) will decrease coverage by 1.35%.
The diff coverage is 85.71%.

Flag	Coverage Δ
block	85.57% <ø> (ø)
blockchain	83.46% <ø> (+0.06%)	⬆️
client	84.23% <85.71%> (+0.23%)	⬆️
common	93.75% <ø> (-0.27%)	⬇️
devp2p	84.00% <ø> (-0.22%)	⬇️
ethash	82.83% <ø> (ø)
tx	88.24% <ø> (-0.12%)	⬇️
vm	79.23% <ø> (-3.41%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

[holgerd77]

Really nice. 😄

[gabrocheleau]

Ready for review! A couple things I'm not 100% confident on:

• The eth/syncing.spec.ts test was "hanging", likely because the client was still running (tap doesn't end if a process is still ongoing, see The execution never ends tape-testing/tape#216). To fix the issue, I added tape.onFinish(() => process.exit(0)) at the end of the test file, but I'd prefer a neater solution (is there a way to somehow force the closing of the client?). Edit: actually this does not fix the issue when ran in the the GitHub actions, will need to figure out an actual fix.
• The implementation of startingBlock and synchronized are a bit naïve, and would likely not behave as expected in the event of synchronization errors, synchronization lagging behind, or synchronization restarts.
• I implemented the latest method on the LightSynchronizer in the same way as on the FullSynchronizer to make the API surface more consistent. Was there a reason is was missing?

Thanks!

[ryanio]

PR and all the code looks great!

I added a commit to help fix the hanging client issue by using the createClient() mock, and adding some properties to the synchronizer property to help achieve the intended results.

The implementation of startingBlock and synchronized are a bit naïve, and would likely not behave as expected in the event of synchronization errors, synchronization lagging behind, or synchronization restarts.

I think this can be a topic for a future PR, perhaps you can open an issue to summarize it, since we are still working on staying on the tip of the chain (#1132)

I implemented the latest method on the LightSynchronizer in the same way as on the FullSynchronizer to make the API surface more consistent. Was there a reason is was missing?

Sounds good to me, I don't see any reason why it couldn't also be implemented there.

[gabrocheleau]

PR and all the code looks great!

I added a commit to help fix the hanging client issue by using the createClient() mock, and adding some properties to the synchronizer property to help achieve the intended results.

The implementation of startingBlock and synchronized are a bit naïve, and would likely not behave as expected in the event of synchronization errors, synchronization lagging behind, or synchronization restarts.

I think this can be a topic for a future PR, perhaps you can open an issue to summarize it, since we are still working on staying on the tip of the chain (#1132)

I implemented the latest method on the LightSynchronizer in the same way as on the FullSynchronizer to make the API surface more consistent. Was there a reason is was missing?

Sounds good to me, I don't see any reason why it couldn't also be implemented there.

Great! Thanks a lot for the review and the help with fixing the client mocking! Will merge now!