Conversation
Collaborator
Pull Request Test Coverage Report for Build 14117682437Details
💛 - Coveralls |
Well-known error codes are used on both sides of the protocol. Having a strong type will let us serialize an error Reply across a foreign language boundary. We were returning a non-spec sender-params-error which amounts to a rejection of the original PSBT for unparseable feerate. Instead, I decided to return the well-known original-psbt-rejected error code and to include the parse error in the debug message in order to comply more closely with existing spec.
This concrete simple struct can more easily cross the foreign language boundaries for bindings than the errors with complex internal variants that implement the JsonError trait.
Hand rolling strings is simple, but serde_json is already a dependency and is more robust.
This simple type can more easily cross foreign language boundaries. Fix payjoin#605
spacebear21
reviewed
Mar 27, 2025
Comment on lines
376
to
384
| impl WellKnownError { | ||
| pub fn error_code(&self) -> &str { | ||
| pub fn error_code(&self) -> ErrorCode { | ||
| match self { | ||
| WellKnownError::Unavailable(_) => UNAVAILABLE, | ||
| WellKnownError::NotEnoughMoney(_) => NOT_ENOUGH_MONEY, | ||
| WellKnownError::VersionUnsupported { .. } => VERSION_UNSUPPORTED, | ||
| WellKnownError::OriginalPsbtRejected(_) => ORIGINAL_PSBT_REJECTED, | ||
| WellKnownError::Unavailable(_) => ErrorCode::Unavailable, | ||
| WellKnownError::NotEnoughMoney(_) => ErrorCode::NotEnoughMoney, | ||
| WellKnownError::VersionUnsupported { .. } => ErrorCode::VersionUnsupported, | ||
| WellKnownError::OriginalPsbtRejected(_) => ErrorCode::OriginalPsbtRejected, | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
This makes me wonder if WellKnownError should be something like pub struct WellKnownError(ErrorCode) or a struct with error_code: ErrorCode and message: String?
Contributor
Author
There was a problem hiding this comment.
I was able to net delete some lines in the following commit. Note it also prunes down the pub interface of WellKnownError, which really should only be exposing its Debug and Display implementations for downstream consumption. No end user/implementor should need to construct this error or pull get the code or message out, Display is the proper interface.
The types are directly related and the send/receive separation is no longer part of our feature distinctions so it makes sense to combine them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #605