Skip to content
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

multi: Remove reject wire protocol support. #3254

Merged

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented May 10, 2024

This completes the deprecation and code removal for the reject message as previously detailed by #2546.

In particular, it consists of a series of commits that:

  • Bumps the minimum required server protocol version to the version that no longer supports the reject message (v9)
  • Bumps the minimum required peer protocol version to the version that no longer supports the reject message (v9)
  • Removes peer code for handling the no longer supported reject message
  • Updates the mock connection used in the peer tests to close the underlying readers and writers when the connection is closed to more closely match the behavior of real connections when they are closed
  • Removes wire code for accepting reject messages which are no longer supported by the network

Closes #2546.

@davecgh davecgh added the wire protocol change Discussion and pull requests regarding items that require changes to the wire protocol. label May 10, 2024
@davecgh davecgh added this to the 1.9.0 milestone May 10, 2024
@davecgh davecgh force-pushed the multi_remove_reject_wire_protocol_support branch from 2816bf1 to 5c616aa Compare May 10, 2024 04:26
This bumps the minimum required protocol version to the version that no
longer supports the reject message now that a new consensus change has
taken place and all older nodes that might use it are no longer able to
follow the chain anyway.
This removes code for handling deprecated reject messages which are no
longer supported by the network.

Since removing the handler definition itself would be a major module
version bump, it is not removed at this time and instead the deprecated
comment is updated to clearly communicate that the underlying message is
no longer valid and thus the handler will no longer ever be invoked.
This updates the mock connection used in the peer tests to close the
underlying readers and writers when the connection is closed to more
closely match the behavior of real connections when they are closed.
This bumps the minimum required protocol version for peers to the
version that no longer supports the reject message now that a new
consensus change has taken place and all older nodes that might use it
are no longer able to follow the chain anyway.

It also adds a test to ensure peers with older protocol versions are
disconnected as intended.
This removes code for accepting reject messages which are no longer
supported by the network.

Since removing the type definition itself would be a major module
version bump, it is not removed at this time and instead the ability to
recognize the message is removed so that any attempts to read the
message will result in a protocol error.

The type and associated test code can be removed on a major module
version bump.
@davecgh davecgh force-pushed the multi_remove_reject_wire_protocol_support branch from 5c616aa to ffa391d Compare May 11, 2024 03:52
@davecgh davecgh merged commit ffa391d into decred:master May 11, 2024
2 checks passed
@davecgh davecgh deleted the multi_remove_reject_wire_protocol_support branch May 11, 2024 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wire protocol change Discussion and pull requests regarding items that require changes to the wire protocol.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal to deprecate and eventually remove the reject wire protocol message.
2 participants