Skip to content

Conversation

@chirag-parmar
Copy link
Contributor

@chirag-parmar chirag-parmar commented Oct 21, 2025

meta: #3101

Description:

This PR ports the existing light client within nimbus-eth2 for use in the verified proxy with light client updates downloaded purely via the REST API. This is beneficial for using the proxy in a browser. In doing so, the PR modifies the behaviour of the light client slightly as noted below.

Notes:

  1. Syncing: The original light client starts with a bootstrap and syncs blocks from bootstrap to head. It then follows the chain optimistically. here and here. The verified proxy version of the light client uses the bootstrap update and subsequent sync committee updates only for establishing the (current and next) sync committee. Once established, however, it starts following the chain directly from the current head. From the perspective of trust, optimistically following the chain from the current head is the same as syncing the chain and then following it optimistically (since the validation is only dependent on the current sync committee which doesn't change within the current sync committee period) here

  2. Peers and Endpoints: The original light client uses multiple peers concurrently to download a particular update. The verified proxy version uses the same code but abstracts out rotating/randomizing peers (in the case of NVP it is API endpoints) to the backend implementation through the use of reqIds. The modified light client uses unique request ids for all concurrent requests which can then be used to rotate/randomize and score API endpoints. The parallelization parameter for the light client is hardcoded to 2.

  3. Downloading Updates: The original light client used a mix of light client updates and p2p gossip to maintain the canonical chain. The verified proxy version purely uses light client updates to maintain the canonical chain. Hence, it runs the manager loop on a per slot frequency rather than the per sync committee period frequency of the original client(not fully true, it is quite arbitrary but can be considered the average case scenario)

Other Changes:

  • remove deprecated warnings
  • remove extra file (c_frontend)
  • fix nits (FullVersionStr)

@chirag-parmar chirag-parmar changed the title Proxy rest lc proxy: rest based light client Oct 21, 2025
@chirag-parmar chirag-parmar marked this pull request as ready for review October 22, 2025 04:50
Comment on lines 70 to 75
try:
await peer.restClient.getLightClientBootstrap(
blockRoot, client.cfg, client.forkDigests
)
except CatchableError as e:
raise newException(CancelledError, e.msg)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the deal exactly with these CancelledErrors being raised on CatchableError (I only noticed now there are already cases of this being done in NVP code base)?

Is this intentional? If so, how is this handled? seems very risky to use in such way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional. The API used for light client updates (defined here beacon_chain/spec/eth2_apis/rest_light_client_calls.nim) doesn't use proper error specifications. Therefore we catch CatchableError and propagate a cancelled error in place. Refer this discussion: #3709 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same for everywhere else in verified proxy

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because there is precedence in other modules doesn't make it more correct. CancelledError is used in Chronos when you cancel a future. It indicates that the entire sync process should get cancelled at the next opportunity. By transforming random errors to a CancelledError, you change the meaning from "there was some issue with one particular request" to "please cancel the entire sync process and propagate cancellation".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What etan said basically.

But regarding that discussion you link, I think what @arnetheduck meant there is that catching CatchableError will make you also catch the CancelledError, which would stop the flow of raising a cancellation. So instead you want to catch all but the CancelledError.

What is done here now however is transforming all exceptions into a CancelledError.

So I think what you want instead is something like:

try:
  <async code>
except CancelledError as e:
  raise e
except CatchableError as e:
  return err(e.msg)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot more sense. Apologies, for misunderstanding the instruction. I'll rectify this.

Copy link
Contributor

@etan-status etan-status left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall flow seems correct, but quite a bit of duplication is introduced across nimbus-eth2/nimbus-eth1. It would be cleaner to adapt nimbus-eth2 modules to support the pluggable backend, then plug the REST backend from here, and the libp2p backend from there.

Another effect is that this removes the libp2p backend. This probably makes sense as all the other APIs also require REST based backends, but may warrant some communication to users upgrading to new version (they now have to pass lightclientdata.org or whatever backend they want).

else:
default(ForkedLightClientHeader)

proc new*(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is heavily inspired by createLightClient in nimbus-eth2, maybe better to integrate lc_backend there, and make it so that Eth2Node is no longer required (but instead, any backend(s) can be passed). Avoids code duplication across repos with one copy of them guaranteed to become stale over time


continue

# check and download sync committee updates
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nextLightClientSyncTask no good?

await sleepAsync(chronos.seconds(2))
continue

# check for updates every slot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this leads to rate-limiting, the events endpoint could be useful as an alternative. There's nextLcSyncTaskDelay to compute a delay that allows following the sync committee in time while being subscribed to events. That's the strategy I'm using on eth-light.xyz

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a great strategy but then it would make rotating between endpoints more complex. Also, if messages are initiated from within the proxy we can reserve the control over bandwidth and privacy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants