-
Notifications
You must be signed in to change notification settings - Fork 181
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
Initial implementation of Tendermint #2185
base: main
Are you sure you want to change the base?
Conversation
244b3d9
to
c0b79c3
Compare
} | ||
|
||
algo.messages.addPrevote(val3Prevote) | ||
prevoteListener := listeners.PrecommitListener.(*senderAndReceiver[Precommit[felt.Felt, felt.Felt], value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we also need tests for line 36?
time.Sleep(time.Millisecond) | ||
algo.Stop() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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..
c0b79c3
to
2ea56e2
Compare
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.