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

Prune Expired Tendermint Consensus States #52

Closed
AdityaSripal opened this issue Jul 8, 2020 · 8 comments · Fixed by #2800
Closed

Prune Expired Tendermint Consensus States #52

AdityaSripal opened this issue Jul 8, 2020 · 8 comments · Fixed by #2800

Comments

@AdityaSripal
Copy link
Member

Each IBC tendermint client must be kept up to date by submitting tendermint headers to the chain. These headers then get stored as ConsensusStates. As IBC clients are long-lived, they will store many consensus states. Each consensus state stores the validator set (which may contain ~100 validators), thus they take up a nontrivial amount of space.

However, the only consensus states that need to be kept by the IBC client are the ones that are within the current unbonding period. Every other consensus state that has been stored since client state initialization can be pruned.

Since well-connected chains like hubs maybe connected to many clients, each of which may be submitting many headers, I think it is worth pruning the consensus states so that the space required by IBC module does not explode with time.

Not sure if this is absolutely necessary for IBC 1.0, but definitely seems like a good feature to have, and relatively simple to implement.


Open to other proposals but a simple fix is to prune all expired consensus states upon receiving a new one. This would just require that clients also have a field that includes the earliest consensus height that is still in state EarliestHeight uint64.

The UpdateClient function would then, upon receiving a valid update, check the earliest height to see if it is expired given unbonding period. If it is, then delete it from store and keep checking and deleting the next earliest heights until we find one that isn't expired. Reset EarliestHeight in client and return.

This should keep the space taken by each client bounded by the average number of blocks produced within an unbonding period.

cc: @cwgoes @fedekunze @colin-axner

@cwgoes
Copy link
Contributor

cwgoes commented Jul 9, 2020

This makes sense, and I think your proposed solution

Open to other proposals but a simple fix is to prune all expired consensus states upon receiving a new one. This would just require that clients also have a field that includes the earliest consensus height that is still in state EarliestHeight uint64.

should work fine. One minor note is that the timestamp & height may both need to be checked to see whether a consensus state is within the unbonding period.

@fedekunze
Copy link
Contributor

I like this proposal a lot. One thing to consider is to be careful when pruning consensus states from a previous version, i.e after upgrading. I propose to use the abstract height from cosmos/ibc#447 instead of a uint 👍

@colin-axner colin-axner transferred this issue from cosmos/cosmos-sdk Mar 5, 2021
@colin-axner colin-axner added this to the 1.0.0 milestone Apr 7, 2021
@colin-axner
Copy link
Contributor

#125 addresses basic pruning of tendermint consensus state. Each update can potentially prune the oldest consensus state

Future improvements should support:

  • batch pruning
  • pruning consensus states for expired clients

@tac0turtle
Copy link
Member

is there an update to this issue, state growth on networks using IBC seems to grow faster than others

@AdityaSripal
Copy link
Member Author

We already prune a single consensus state for each new one. But batch pruning has not been implemented and is not on the immediate roadmap

@tac0turtle
Copy link
Member

oh okay, if we prune as the new one comes in, then why would batch be needed?

@AdityaSripal
Copy link
Member Author

because when the new one comes in, there may be multiple expired consensus states in store. we only prune the oldest as of now.

We could prune all, but it introduces variable gas costs into UpdateClient. maybe something worth implementing in a separate msg

@colin-axner
Copy link
Contributor

Migrating from stargate to ibc-go will prune all expired consensus states

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

Successfully merging a pull request may close this issue.

5 participants