chia-blockchain
chia-blockchain copied to clipboard
server: Drop `ChiaServer.validate_broadcast_message_type`
Purpose:
I might miss the point but this validation there seems strange to me and even more strange that we disconnect/ban from the peers because we tried to initiate sending the "wrong" messages. For me it looks like even sending a message which requires a response doesn't lead to actual issues, does it?
If there is a point in keeping the message_requires_reply
checks i might just put it directly into send_to_all
but seems to me like just dropping it is fine?
Current Behavior:
The method validates static messages to peers and disconnects from the peers if the code tries to send a message which expects a response.
New Behavior:
It just sends the messages.
Pull Request Test Coverage Report for Build 6045745687
- 0 of 0 changed or added relevant lines in 0 files are covered.
- 32 unchanged lines in 6 files lost coverage.
- Overall coverage decreased (-0.02%) to 89.158%
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
chia/wallet/wallet_node.py | 2 | 87.34% |
chia/rpc/rpc_server.py | 3 | 87.11% |
chia/server/node_discovery.py | 3 | 78.4% |
chia/timelord/timelord.py | 3 | 71.9% |
chia/wallet/wallet_state_manager.py | 4 | 95.89% |
chia/full_node/full_node.py | 17 | 85.79% |
<!-- | Total: | 32 |
Totals | |
---|---|
Change from base Build 6044229157: | -0.02% |
Covered Lines: | 85995 |
Relevant Lines: | 96376 |
💛 - Coveralls
did you run into a problem with this check?
as far as I can tell, this patch removes the check entirely. I agree that it seems reasonable to just drop the message that we attempt to send erroneously, rather than dropping both the peer connection and the message. However, since this would only happen if we have a bug, dropping the peer connection (and the associated co-routines) might make it more likely we chop off the damaged part of the program.
I also would think we really want an error message in the log if this happens.
This pull request has conflicts, please resolve those before we can evaluate the pull request.
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.
Conflicts have been resolved. A maintainer will review the pull request shortly.
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.