Improvements to error handling

[sanket1729]

Here is one strategy that I think we should do to improve the ergonomics of errors. Broadly speaking, there are two ways users can interact with secp API.

1. Users who just bubble up the error.
2. Users who match on errors on variants and deal with them in some way.

Right now, our API is not the most convenient for users (2). For example, Signature::from_der returns only one error variant but the user is forced to match against all variants.

Proposal

Have each API return a separate error type that does not have any variants that are unreachable. Provide a top level common error type to unify all error types for users who just want to bubble up errors.

pub Error{
	InvalidSig(InvalidSigError),
	PubkeyParseError(PubKeyParseError),
}
impl Signature{
	pub fn from_der() -> Result<.., InvalidSigError>;
}
impl FromStr for PublicKey {
	pub fn from_str() -> Result<..., PubkeyParseError>;
}

impl From<InvalidSigError> for Error {}
impl From<PubkeyParseError> for Error {}

[apoelstra]

concept ACK. We will probably wind up with multiple layers of nested errors and we should make sure that we have From implemented from each error type to all of its containing error types. Otherwise users need to type .map_err(Error1::from).map_err(Error2::from) etc.

[tcharding]

I like the idea, any reason you posted this in rust-secp256k1 and not in rust-bitcoin? Perhaps this should be an org wide idea - or was the idea to test run it here in secp first to see how it goes? We have an error epic on rust-bitcoin crate also: rust-bitcoin/rust-bitcoin#820

[sanket1729]

Otherwise users need to type .map_err(Error1::from).map_err(Error2::from) etc.

Yes, we will add impls from all individual types to top level error. Note that users can call .into() which does transitively do the correct thing.

I like the idea, any reason you posted this in rust-secp256k1 and not in rust-bitcoin? Perhaps this should be an org wide idea - or was the idea to test run it here in secp first to see how it goes?

No special reason. I wanted to try a PR in this repo first because there is small code surface and APIs can be easily separated.

[tcharding]

Just read this, I already fixed up rust-bitcoin/rust-bitcoin#1950 to try out your idea.

[Kixunil]

FYI this is exactly what I wanted for a while, so I'm happy with this.