Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Light-client feedback #2429

Open
dapplion opened this issue May 21, 2021 · 1 comment
Open

Light-client feedback #2429

dapplion opened this issue May 21, 2021 · 1 comment

Comments

@dapplion
Copy link
Collaborator

After implementing a rough POC web-based light-client consumer in Lodestar, I would like to share some feedback about it.

  1. In validate_light_client_update this condition invalidates updates that are technically valid.
if update_period == snapshot_period:
        sync_committee = snapshot.current_sync_committee
        assert update.next_sync_committee_branch == [Bytes32() for _ in range(floorlog2(NEXT_SYNC_COMMITTEE_INDEX))]

The update provider (server) does not know in advance if the update_period is > snapshot_period. It could just check before if update.next_sync_committee is zeroed and verify it otherwise

  1. An update provider may want to send the light-client consumer an update for the same slot but with higher consensus. This happens frequently for finalized sync where sync committees will be producing valid updates for the same update.header.slot during an epoch but they may have more bits than the previous. This condition invalidates useful updates and I believe it can be omitted.
assert update.header.slot > snapshot.header.slot
  1. In LightClientStore the purpose of valid_updates is to potentially use one of those if there is an update timeout. Instead, LightClientStore could just keep a single best_update and run the logic the pick the best on every new update. To comply with SSZ it can be zeroed initially to represent a no value.

  2. The UX for finalized sync is not great, the distance from the header you have in the snapshot vs current chain head is way to big. I'm writing this to signal the importance of getting a safe sound protocol for a re-org capable light-client close to Altair release.

  3. Sharing the API we are using currently as a potential proposal for a REST API or to eventually translate it to a ReqResp protocol

GET /eth/v1/lightclient/best_updates/:periods

Return updates in batch to a range of periods. Must return exactly 1 update per period requested.
The definition of "best-update" is vague so nodes have the freedom to optimize what to store.
For example, to persist the latest LightclientUpdate that has the most bits within a period.

GET /eth/v1/lightclient/latest_update_finalized/

Request the latest finalized update seen by the node

GET /eth/v1/lightclient/latest_update_nonfinalized/

Request the latest non finalized update seen by the node

CC: @wemeetagain

@Besiktas19033
Copy link

After implementing a rough POC web-based light-client consumer in Lodestar, I would like to share some feedback about it.

  1. In validate_light_client_update this condition invalidates updates that are technically valid.
if update_period == snapshot_period:
        sync_committee = snapshot.current_sync_committee
        assert update.next_sync_committee_branch == [Bytes32() for _ in range(floorlog2(NEXT_SYNC_COMMITTEE_INDEX))]

The update provider (server) does not know in advance if the update_period is > snapshot_period. It could just check before if update.next_sync_committee is zeroed and verify it otherwise

  1. An update provider may want to send the light-client consumer an update for the same slot but with higher consensus. This happens frequently for finalized sync where sync committees will be producing valid updates for the same update.header.slot during an epoch but they may have more bits than the previous. This condition invalidates useful updates and I believe it can be omitted.
assert update.header.slot > snapshot.header.slot
  1. In LightClientStore the purpose of valid_updates is to potentially use one of those if there is an update timeout. Instead, LightClientStore could just keep a single best_update and run the logic the pick the best on every new update. To comply with SSZ it can be zeroed initially to represent a no value.

  2. The UX for finalized sync is not great, the distance from the header you have in the snapshot vs current chain head is way to big. I'm writing this to signal the importance of getting a safe sound protocol for a re-org capable light-client close to Altair release.

  3. Sharing the API we are using currently as a potential proposal for a REST API or to eventually translate it to a ReqResp protocol

GET /eth/v1/lightclient/best_updates/:periods

Return updates in batch to a range of periods. Must return exactly 1 update per period requested.
The definition of "best-update" is vague so nodes have the freedom to optimize what to store.
For example, to persist the latest LightclientUpdate that has the most bits within a period.

GET /eth/v1/lightclient/latest_update_finalized/

Request the latest finalized update seen by the node

GET /eth/v1/lightclient/latest_update_nonfinalized/

Request the latest non finalized update seen by the node

CC: @wemeetagain

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

No branches or pull requests

5 participants
@hwwhww @dapplion @Besiktas19033 and others