protocols: Introduce error protocol message

[xdustinface]

Purpose:

This is meant to be a general error message for our peer API since we don't have such a thing yet. The message format is currently like below:

Im don't have really a use-case in mind for the Error.data from 6c9b8b3 and im not opposed to drop that commit if there are opinions about it but i thought it might be a good thing to add in case we want to add some extra data.

@streamable
@dataclass(frozen=True)
class Error(Streamable):
    code: int16  # Err
    message: str
    data: Optional[bytes] = None

While we have some specialised error responses for specific requests like reject_block which can come as response to request_block i think having this general error/Error message with the format above makes more sense since it anyway usually comes as response to a request (and if not we anyway don't handle it properly) so mapping the request parameter to the error output works without fully repeating the request parameter in the error message which is what the specialised messages seem to do mostly.

This imo also can be used to replace the none_response which was introduced in #13967 because it from my understanding was just added to handle cases like

chia-blockchain/chia/full_node/full_node_api.py

Lines 290 to 292 in fde6d6f

	if wp is None:
	self.log.error(f"failed creating weight proof for peak {request.tip}")
	return None

or other cases where we just log an error and return None but don't send a response, to send a message back in response instead of waiting for the timeout on the call site. So now with this new error mechanism introduced here we could in that any many other places instead just

if wp is None:
    raise ApiError(Err.SOME_ERROR_CODE, f"failed creating weight proof for peak {request.tip}")

which then also avoids waiting for the timeout while it also sends back error information to the API caller instead of just a None message.

Current Behavior:

We don't have any way to send general error messages in API methods.

New Behavior:

Raising ApiError leads to a Error message as response if the caller supports it: protocol_version >= 0.0.35

Based on:

• server: Introduce ApiProtocol #15466

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[coveralls-official]

Pull Request Test Coverage Report for Build 5533528242

• 121 of 121 (100.0%) changed or added relevant lines in 12 files are covered.
• 15 unchanged lines in 6 files lost coverage.
• Overall coverage increased (+0.07%) to 87.434%

---

Files with Coverage Reduction	New Missed Lines	%
chia/server/node_discovery.py	1	78.17%
chia/wallet/wallet_state_manager.py	1	95.52%
chia/farmer/farmer_api.py	3	69.74%
chia/full_node/full_node_api.py	3	77.3%
chia/timelord/timelord.py	3	73.2%
chia/full_node/full_node.py	4	85.55%

Totals
Change from base Build 5533491479:	0.07%
Covered Lines:	80091
Relevant Lines:	91488

---

💛 - Coveralls

[almogdepaz]

@xdustinface i didn't see code that considers the different protocol versions, what happens if a new node sends this error to an old node ?

[xdustinface]

@xdustinface i didn't see code that considers the different protocol versions, what happens if a new node sends this error to an old node ?

There is a protocol version check and it will not send it to an old node, see https://github.com/Chia-Network/chia-blockchain/pull/15493/files#diff-076ad87bb2203f6728105a8ac4aab4e126ba0113a38e4efff07884ed4934cabfR407-R413.

Here is also a test which tests a lower, the same and a higher version https://github.com/Chia-Network/chia-blockchain/pull/15493/files#diff-412caaa93bcfaabc889b96f6e66dc09326eb959295bf1006b0847f7e6aefb40cR112-R114.

[almogdepaz]

ok, looks pretty similar to the None response but this adds versioning for errors instead of using capabilities which we already have.

other then that the None response PR was similar but there where reservations about the timing of the merge from @emlowe and we commented the code out with the intent of uncommenting with the next new capability being added, the old None response pr was also already tested in testnet and fixes issues we had that prevent adding new capabilities

[xdustinface]

ok, looks pretty similar to the None response but this adds versioning for errors instead of using capabilities which we already have.

We also already have a protocol version so why not just use this? Is there any advantage you can think about when using capabilities for this?

other then that the None response PR was similar but there where reservations about the timing of the merge from @emlowe and we commented the code out with the intent of uncommenting with the next new capability being added, the old None response pr was also already tested in testnet and fixes issues we had that prevent adding new capabilities

Im not sure what those reservations are/were about and maybe its similar but there is a difference which is that the none response was from my understanding meant to just always send a empty message back to the caller without letting it know what went wrong if it was about an issue. But with this error message mechanism we can send actual error codes/messages to the caller while logging the issue on the server side by just raising appropriate exceptions in failure cases where we returned none and only logged the issue on the server side before.

[almogdepaz]

no reservation just seems a little redundant, i think if we merge this the pr probably needs to include removing of the commented out code since we will no longer need it and maybe we need to get rid of capabilities if we will use software version instead

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.