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

[davecgh]

I would like to propose that the reject wire protocol message be deprecated in v1.7.0 and then removed entirely in the release after that. This is a holdover from the original btcsuite codebase as it was required on that network, but it really is not a useful message and has several downsides.

• Nodes on the network must be trustless, which means anything relying on such a message is setting itself up for failure because nodes are not obligated to send it at all nor be truthful as to the reason
• It can be harmful to privacy as it allows additional node fingerprinting
• It can lead to security issues for implementations that don't handle it with proper sanitization practices
• It can easily give software implementations the 100% incorrect impression that it can be relied on for determining if transactions and blocks are valid
• The only way it is actually used currently is to show a debug log message, however, all of that information is already available via the peer and/or wire logging anyway
• It carries a non-trivial amount of development overhead to continue to support it when nothing actually uses it

Additional Info About Correctness

As mentioned above, software already realistically MUST NOT rely on p2p reject messages for correctness anyway. In particular, the proper way to handle the two main cases (both today and after these proposed changes) is:

• Anything that creates and sends transactions (such as wallets) are responsible for ensuring that transactions are seen on the network by listening for announcements because the absence of receiving a p2p reject message is NOT an indication of success
• Testing if a block is valid for any external software producing them is to make use of the submitblock RPC

Proposed Roadmap

If we decide to move forward with this, I would expect the v1.7.0 release to:

• Stop sending reject messages and ignore any received reject message (in practice this means stop logging them)
• Bump the wire protocol version and make it an error with associated protocol violation handling to send the message for all peers that have negotiated to the new protocol version
• However, any peers negotiated to an older version would not be punished for sending them, newer versions simply won't send them and will ignore them from the older version peer

Then, in the following release:

• Make the new protocol version introduced in v1.7.0 the minimum required protocol version to participate in the network
• Remove the reject protocol message altogether

[jholdstock]

Given everything you have written above, it seems only sensible to remove the reject messages.

It carries a non-trivial amount of development overhead to continue to support

I'm curious what this is? Presumably theres a request/response type and some handling code, but if the handling code is basically just a log line, where does the dev complexity come from?

Make the new protocol version introduced in v1.7.0 the minimum required protocol version to participate in the network

If I understand correctly, this is going to be backwards incompatible. Is there a good reason to do this? If clients on the new protocol version simply ignore the reject messages, why force an upgrade for clients who otherwise might not need to upgrade?

There is a good chance that 1.8 will include other changes which genuinely are backwards incompatible and will force and upgrade, but I don't think this needs to be one of them.

[davecgh]

I'm curious what this is? Presumably theres a request/response type and some handling code, but if the handling code is basically just a log line, where does the dev complexity come from?

There is a bunch of code dedicated to converting internal errors into the appropriate reject codes and it constantly has to be updated as new internal errors are added.

If I understand correctly, this is going to be backwards incompatible. Is there a good reason to do this? If clients on the new protocol version simply ignore the reject messages, why force an upgrade for clients who otherwise might not need to upgrade?

There is a good chance that 1.8 will include other changes which genuinely are backwards incompatible and will force and upgrade, but I don't think this needs to be one of them.

Yes, but 1.7.0 will almost positively have a consensus vote which means that everyone will already be required to be on the 1.7.0 code base (which would support the new protocol version) anyway. That is actually the primary reason I'm suggesting this approach. If it turns out that is not the case, we'd just punt to the next version after one that does.

The goal is to be able to remove all of the wire protocol code which can't be done as long as you have to negotiate to old versions that support it, even if you yourself are not sending / ignoring it, you still have to parse and understand it at the protocol level.

[matheusd]

I double checked and indeed the only other place unrelated to tx or block submission that uses it is as a reply to a version message when the connected peer can't provide full node services. I guess that can be easily handled as a simple disconnect instead of a reject msg.

Updating the minimum required protocol version in the release following a hard fork upgrade event seems reasonable to me.

[davecgh]

I double checked and indeed the only other place unrelated to tx or block submission that uses it is as a reply to a version message when the connected peer can't provide full node services. I guess that can be easily handled as a simple disconnect instead of a reject msg.

Yep, that was exactly my thought on it. Right now, it sends a reject and then disconnects. It would just log and disconnect. Anyone dealing with software related to the implementation of protocol that are trying to figure out why can then just enable debug logging on dcrd to see the reason.

[davecgh]

The first part of this slated for v1.7.0 has been completed via PRs #2586 and #3017.

Since the second part has to wait until v1.9.0 (or whatever version comes after 1.8.0), I've marked this issue as a milestone for v1.9.0 and will leave it open until the second part is completed.

[davecgh]

The final part of this has been completed via PR #3254.