consensus: do we really need initialHeight in the consensus reactor?
#4491
Labels
Comments
3 tasks
|
I think it makes sense. It seems that you can get that from the state and as you mentioned it doesn't change. |
|
I agree with @andynog ; we need initialHeight. A great example was saving Juno. After the attack that shattered it into 1 chain per validator (btw still don't know how this happened, all vals saw their block as valid (kinda know how it happened, I think something with using system time) we used that feature to get the chain up and running again. Note: just reread issue and noted the title -- as for the feature I would vote to keep it. And for where - I noticed you mentioned the consensus reactor -- was there thought to moving it elsewhere? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Originally posted by @cason in #4436 (comment)
This value is copied from the consensus state (the actual algorithm implementation) in order to validate received messages of a single type, which is a control message, where a node reports its current state:
I really think that once we have started a node, the
state.InitialHeightnever changes. It is setup in the genesis, loaded at startup, but it cannot be changed by the anyone. This code is super old, introduced by tendermint/tendermint#5191.Nonetheless, with more or less efficient methods we are pulling this integer every time we receive such a message from any peer (several times per consensus round). This is really inefficient and we should probably get rid of this logic.
The text was updated successfully, but these errors were encountered: