-
Notifications
You must be signed in to change notification settings - Fork 113
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
feat(grandpa): implement neighbor tracker and message handling #4230
Conversation
6293cf3
to
0c0cbb8
Compare
b51687a
to
7c17435
Compare
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.
LGTM, just a small comment over the usage of a constant.
7c17435
to
38ae1e9
Compare
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.
Just a few nits/questions.
370e30c
to
8104f27
Compare
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.
There should be more tests regarding running the NeighborTracker
and passing in messages via channel. NeighborTracker.BroadcastNeighborMsg
should be tested.
I think one of the integration tests are broken as well based on the changes.
8104f27
to
9aee52b
Compare
@timwu20 Responded to feedback, fixed broken integration tests, and added tests for 1) start/stop 2) updating peer state via channel. I did not add testing for the finalization channel because no state is actually updated based on it so I didnt know what to assert. Since this channel is used/tested in other parts of the code I thought this is okay, but let me know if/how this should be updated. |
4f39d8f
to
fa4d931
Compare
fa4d931
to
6625430
Compare
6625430
to
3dbb1db
Compare
Changes
This PR adds the neighbor messaging protocol. This includes the ability to handle received neighbor messages, the ability to correctly send neighbor messages, as well as tracking the state of our neighbors (that we use when deciding who to send messages too),
Tests
go test -tags integration github.com/ChainSafe/gossamer
Issues
#2931