Initial implementation of Tendermint #2185
Conversation
IronGauntlets
force-pushed
the
IronGauntlets/tendermint
branch
from
244b3d9
to
c0b79c3
Compare
IronGauntlets
changed the title
| } | ||
|
|
||
| algo.messages.addPrevote(val3Prevote) | ||
| prevoteListener := listeners.PrecommitListener.(*senderAndReceiver[Precommit[felt.Felt, felt.Felt], value, |
There was a problem hiding this comment.
typo. Also, do you think we could collapse the above three tests into a single test? It seems most of the code is repeated
There was a problem hiding this comment.
I was trying to make the tests independent. If we start sharing variables, it becomes difficult to debug because of side effects. I have variables that don't change at the top of the test.
| t.scheduledTms = append(t.scheduledTms, tm) | ||
| } | ||
|
|
||
| func (t *Tendermint[_, H, A]) OnTimeoutPropose(h, r uint) { |
There was a problem hiding this comment.
So imagine that we start up a node, and it never recieves messages from other nodes. We start round 0, schedule the propose timeout, which eventually gets hit, and we end up here. Then we cast a nil prevote, set step to prevote and continue. We will end up back at startRound(0) again, reset everything, and the process will repeat indefinitely, right? We would effectively be casting nil prevotes with round=0 forever? (I would have thought the round would increase..)
| t.Run("Line 22: receive a new proposal before timeout expiry", func(t *testing.T) { | ||
| }) | ||
| // | ||
| //t.Run("Line 28: receive an old proposal before timeout expiry", func(t *testing.T) { |
There was a problem hiding this comment.
I guess we also need tests for line 36?
| time.Sleep(time.Millisecond) | ||
| algo.Stop() |
There was a problem hiding this comment.
Removing these two lines seems to have no effect. Which is weird, I would have thought there were needed, and don't understand how algo.Stop() is triggered..
All of the Tendermint algorithm has been implemented.
Outstanding tasks are described in various "todos".
Once sufficient tests have been added I will split the PR into multiple commits and it will be ready to merge.