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

Epochs should be kept for longer in storage #1857

Closed
Tracked by #2023 ...
sug0 opened this issue Aug 29, 2023 · 11 comments · Fixed by #1944
Closed
Tracked by #2023 ...

Epochs should be kept for longer in storage #1857

sug0 opened this issue Aug 29, 2023 · 11 comments · Fixed by #1944
Assignees
Labels
bug Something isn't working ethereum-bridge has-a-pr The issue has been solved by a PR that has yet to be merged ledger PoS prio:high

Comments

@sug0
Copy link
Collaborator

sug0 commented Aug 29, 2023

Currently, Namada is only storing up to 2 epochs worth of validator data. This is problematic, as plenty more validator sets are required for validator set updates in the Ethereum bridge.

@sug0 sug0 changed the title Epochs should get kept for longer in storage Epochs should be kept for longer in storage Aug 29, 2023
@brentstone
Copy link
Collaborator

Can you quantify some approximate upper bound on the number of epochs we should be storing?

@sug0
Copy link
Collaborator Author

sug0 commented Sep 6, 2023

@brentstone I think we can store every epoch, but in a more space efficient manner. This is the solution I proposed to Tomas:

We keep the last two epochs cached in storage (i.e. the current implementation) for fast access to their validator sets. For epochs older than the last two, we store a chain of operations that allows us to compute the consensus validator sets of said epochs.

For instance, if you start epoch 0 with the set of validators $S_A = (A,B,C)$, you would issue $\text{insert}(A,B,C)$ as the first operation, and store the index 0 of the epoch. Then, if at epoch 1 validator $B$ was removed from the consensus set, you would store $\text{remove}(B)$ and the index 1. This would be more efficient than storing $(A,B,C)$ and $(A,C)$, and of course it would scale with larger validator sets.

The benefit of this approach is that, assuming validator sets are more or less static, not a whole lot of space is required to store the sets. Space could be reduced even further by implementing rollback mechanisms of sorts, that would allow you to reset to the validator set of a given epoch, so you would only need to store 1 command (the rollback) vs. 2 or more commands to reach the same state.

Old consensus sets can be cached in the Shell for fast access, as well.

@sug0 sug0 added the PoS label Sep 6, 2023
@brentstone
Copy link
Collaborator

@sug0 ok so for this purpose, only the epoch and the validator address (with some operation like $\text{insert}$) are needed? No other info such as stake, etc?

@sug0
Copy link
Collaborator Author

sug0 commented Sep 6, 2023

@brentstone we need validator and stake pairs for the consensus set. anything else can be removed from storage, in principle

btw, idk if this was clear in my previous comment, but you don't need to store actions for every epoch. if the validator set doesn't change in, say, 4 epochs, you don't need to store anything at the end of those 4 epochs.

@brentstone
Copy link
Collaborator

@sug0 Ok so but now do we need to record every time a consensus validator's stake changes even if they remain in the consensus set after change?

@sug0
Copy link
Collaborator Author

sug0 commented Sep 7, 2023

@brentstone oh, good point. I guess that's one of the actions that must be included :\

maybe it's possible to consult a validator's stake by reading from some storage key at some height when the epoch begins? so this way you only need to store consensus validator addresses, and you can retrieve their stake from somewhere else

@brentstone
Copy link
Collaborator

@Fraccaman What is the maximum time in number of epochs from the creation of a gov proposal to its execution (if any)? After chatting with @sug0, thinking this may place a bound on the number of epochs for which we need the consensus validator set information for eth-bridge

@Fraccaman
Copy link
Member

we need max_proposal_period epochs in storage for governance. For eth bridge there is not really an upper bound I think?

@brentstone
Copy link
Collaborator

brentstone commented Sep 20, 2023

@tzemanovic @sug0

Coming back to this, it seems like we need only the consensus validator set with stakes for at least max_proposal_period epochs.

For this issue, do we need any kind of ordering? The validator sets are currently a nested map of token::Amount -> Position -> Address. Is the Position still needed? I.e. can we store the past sets as maps Address -> token::Amount for storage efficiency (though maybe the improvement here is negligible)?

I think the storage of operations that @sug0 described in #1857 (comment) may not be super useful since an operation would be needed even if a consensus validator's stake changes but keeps it in the consensus set. After we implement #939, inflationary rewards will likely be applied eagerly to the validator deltas and stakes in the validator sets, which means that every entry in the consensus validator set will change every epoch at pipeline length.

@brentstone brentstone self-assigned this Sep 20, 2023
@sug0
Copy link
Collaborator Author

sug0 commented Sep 21, 2023

@brentstone max_proposal_period being the maximum length of a governance proposal? I think this would be fine, if we added a couple more (say, 2) buffer epochs, to give time to relay validator sets. this could be an extra parameter, as well (albeit not really that necessary).

the set of validators doesn't need to be ordered. we have our own logic to sort the set of validators in descending order, by their stake.

wrt the last point, yeah, agreed, we can ignore my proposal. this solution seems simple to implement, it literally just requires changing a parameter that we pass to the logic aleks wrote.

@sug0
Copy link
Collaborator Author

sug0 commented Sep 26, 2023

related to #1665

@sug0 sug0 added the has-a-pr The issue has been solved by a PR that has yet to be merged label Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ethereum-bridge has-a-pr The issue has been solved by a PR that has yet to be merged ledger PoS prio:high
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants