Stabilize external error types

[spacebear21]

Currently we use opaque type externally because we aren't sure which variants will stay. Eventually these should be transparent and we can remove the Internal variants.

[DanGould]

• [x] Fix #498

[DanGould]

There are a few remaining receiver Error variants that can probably be handled as ReplyableError depending on the receiver's preferences.

SelectionError

SelectionError arises from receiver coin selection failing, when attempting to apply a certain algorithm like avoid_uih. This error should be preserved, in case a downstream implementation wants to do some more manual coin selection than we currently offer, but ultimately if it fails it must be convertable into an unavailable ReplyableError and potentially broadcast the original PSBT automatically.

The current implementation of payjoin-cli just proceeds without providing inputs if one of these errors arises, which seems like the only correct option if not converted to an Implementation Error variant and returning Unavailable.

OutputSubstitutionError

A downstream implementation should be able to try to move on with a simple consolidation to payjoin, and otherwise convert this error (or the one from failing to consolidate) into an unavailable from this variant

Right now this arises when we force new address substitution for demo purposes in the V1 payjoin-cli and return it as an Implementation error, which actually seems correct, since that produces an Unavailable error.

InputContributionError

This panics in payjoin-cli, though it must definitely be wrapped in an Implementation error and returned as Unavailable if it's really unavailable. Must not panic. I can see an iterative implementation where a receiver keeps trying to add inputs until this variant is not triggered, and then replying, and otherwise coercing the error into Implementation and replying as a ReplyableError.

[spacebear21]

While we're doing this, I'd like to see SelectionError renamed to CoinSelectionError for max clarity.

[spacebear21]

Until we're sure we can remove the Internal types we may want to consider making those pub(crate) so that testers can check for specific error types.

[DanGould]

• [ ] When we make error variants public, remember to reflect the change in tests by matching variants instead of messages.

[DanGould]

From Tobin @ rust-payjoin

Hey mate, in rust-bitcoin we took the stance that we basically can't commit to any error types for the initial 1.0 releases so we used that same InternalError pattern. In hex-conservative 1.0 we committed to a couple of pub enums but the variants use structs with all private fields. As far as where to draw the line I'd draw it as close to 'commit to nothing' as possible, at least at first. Hope that helps Be careful not to add From<PrivateError> for PublicError either, because that puts PrivateError in the public API

me: do you just then convert all InternalErrors into PublicErrors with .map_err ?

Yep, use map_err.

[arminsabouri]

Acceptance Criteria: audit all the error types. If we need to make internal error changes would we break semver? Once we have the 1.0 RC lets audit and ensure the answer to the above is no