Improve API ergonomics

[thomaseizinger]

While working with the library, I noticed a slight inconvenience regarding the error handling: All methods on the Secp256k1 struct which deal with signing return a Result because the context might not have been initialized for signing.

However, if the code invoking the sign-methods created the instance of Secp256k1 itself, it can be sure that the context will always be correct. It still has to handle the error although it will never occur, which is inconvenient.

The PR proposes a different approach regarding the capabilities of the underlying context of the Secp256k1 struct. Through the use of generics, the check of "can this instance of Secp256k1 sign a message" is covered by the compiler. This does not only resolve the above stated inconvenience but actually makes it possible to 'request' an instance of Secp256k1 with certain capabilities from the caller of your code.

The downside of this PR is that it introduces a few breaking changes:

• Secp256k1#sign does no longer return a Result
• Secp256k1#sign_recoverable does no longer return a Result
• Secp256k1#generate_keypair does no longer return a Result
• PublicKey#from_secret_key does no longer return a Result
• The enum ContextFlag has been removed (and all methods using it)
• The variant IncapableContext of the Error enum has been removed.

Let me know what you think about the approach :)

[apoelstra]

Concept ACK. @TheBlueMatt can I get a concept ACK on this before I do a detailed review?

It'll break the API in a ton of ways but moves runtime checks into compile-time checks which is awesome.

[TheBlueMatt]

Oh this is awesome, definitely Concept ACK, but can we make sure to flush all changes to crates.io in a minor version bump before we merge this (with a major version bump?).

[apoelstra]

Yep for sure. Will make sure all current changes are on crates before merging any part of this.

[apoelstra]

Pushed current master to crates, along with tag 0.9.1.

[thomaseizinger]

Awesome news!
Looking forward to the review :)

[apoelstra]

Can you squash the first three commits together, and move

    /// Creates a new Secp256k1 context with no capabilities (just de/serialization)
    pub fn without_caps() -> Secp256k1<None> {
        Secp256k1 { ctx: unsafe { ffi::secp256k1_context_create(ffi::SECP256K1_START_NONE) }, phantom: PhantomData }
    }    

out of impl<C> Secp256k1<C> and into its own impl Secp256k1<None>? I think this will get one commit that compiles (except for unit tests). Currently when you try to use without_caps it does not compile, saying "cannot infer type for C", the reason being that you've got this function defined inside a impl block that doesn't restrict it in any way.

[thomaseizinger]

I squashed the first commits. This one now compiles. The second commit makes the tests pass.

I also fixed the warnings about missing documentation by adding documentation to the public items I added.

[apoelstra]

This looks great. ACK, except that there's one remaining warning (in the capabilities unit test we no longer use the no-capabilities context none, so we should remove it).

Let me know if you want to clean up the history at all. It looks like the unit tests pass for every commit except the first, which is fine by me.

[thomaseizinger]

Oh I must have overlooked that one! Will fix asap!

I will take another look at the history. The commits are indeed probably a bit too fine grained.

[thomaseizinger]

Let me know if the new history is better now :)

[apoelstra]

Much better, thanks :)