-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(net):supplement disconnect reasons #5392
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(net):supplement disconnect reasons #5392
Conversation
@@ -601,6 +601,7 @@ enum ReasonCode { | |||
TOO_MANY_PEERS_WITH_SAME_IP = 0x22; | |||
LIGHT_NODE_SYNC_FAIL = 0x23; | |||
BELOW_THAN_ME = 0X24; | |||
NOT_WITNESS = 0x25; |
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.
Is it compatible with older versions?
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.
All of the newly added unrecognized ReasonCode will be converted to ReasonCode UNKNOWN, and no exception will be thrown.
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.
- It is recommended to make it more perfect, whether each of these merged cases defines a reason, and there are some unknown scenarios
case NO_SUCH_MESSAGE:
case MESSAGE_WITH_WRONG_LENGTH:
case BAD_MESSAGE:
code = Protocol.ReasonCode.BAD_PROTOCOL;
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.
@wubin01 there is no MESSAGE_WITH_WRONG_LENGTH really. But there is NO_SUCH_MESSAGE. Do you means add NO_SUCH_MESSAGE in Protocol.ReasonCode?
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 have added new ReasonCode NO_SUCH_MESSAGE in proto.
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.
Everyone have any idea for more updates.
What does this PR do?
Why are these changes required?
This PR has been tested by:
Follow up
Extra details