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

feat: replace store_update_{time,height} by metadata update #973

Closed
Tracked by #554
mina86 opened this issue Nov 20, 2023 · 0 comments · Fixed by #972
Closed
Tracked by #554

feat: replace store_update_{time,height} by metadata update #973

mina86 opened this issue Nov 20, 2023 · 0 comments · Fixed by #972
Labels
A: breaking Admin: breaking change that may impact operators O: optimization Objective: aims to optimize performance, allocations and computations

Comments

@mina86
Copy link
Contributor

mina86 commented Nov 20, 2023

store_update_time and store_update_height methods are always called
together. In fact, they are also usually called together with
store_consensus_state.

Sadly, since they are separate calls, the implementations need to
treat them as separate pieces of data. For example, if an
implementation stores update times and heights in a map, it must
perform two separate lookups to update each value.

Change the interface such that store_consensus_state in addition takes
host timestamp and height. This makes it possible for implementations
to perform optimisations where timestamp and height are stored in
a single location. This reduces number of lookups and possibly also
improves memory footprint of the implementation.

In fact, they may be stared together with the consensus state making
the optimisation potential even stronger.

Alas, updating of the timestamp and height is at times called without
updating the consensus state. To accommodate that use case, introduce
store_consensus_metadata call which only takes the timestamp and
height arguments.

@Farhad-Shabani Farhad-Shabani added O: optimization Objective: aims to optimize performance, allocations and computations A: breaking Admin: breaking change that may impact operators labels Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: breaking Admin: breaking change that may impact operators O: optimization Objective: aims to optimize performance, allocations and computations
Projects
Status: Done
2 participants