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

Manually trigger LightClientStore force updates #2947

Merged
merged 2 commits into from
Jul 22, 2022
Merged

Manually trigger LightClientStore force updates #2947

merged 2 commits into from
Jul 22, 2022

Conversation

etan-status
Copy link
Contributor

Replaces process_slot_for_light_client_store which force updates the
LightClientStore automatically based on finalized_header age with
try_light_client_store_force_update which may be manually called based
on use case dependent heuristics if light client sync appears stuck.
Not all use cases share the same risk profile.

Replaces `process_slot_for_light_client_store` which force updates the
`LightClientStore` automatically based on `finalized_header` age with
`try_light_client_store_force_update` which may be manually called based
on use case dependent heuristics if light client sync appears stuck.
Not all use cases share the same risk profile.
specs/altair/light-client/sync-protocol.md Outdated Show resolved Hide resolved
apply_light_client_update(store, store.best_valid_update)
store.best_valid_update = None
```
- `try_light_client_store_force_update` MAY be called based on use case dependent heuristics if light client sync appears stuck.
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems a big change to light client behavior.

/cc @wemeetagain @dapplion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previous behaviour can be simulated by calling this on every slot, same as before. But new behaviour allows for use cases such as historic data sync where it is not appropriate to force-apply every single update just because it is historic.

Copy link
Contributor

Choose a reason for hiding this comment

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

@etan-status I see. So it is backward compatible. 👍

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

2 participants