Simplify error handling and cleanup API

[nskinkel]

This pull request drastically simplifies error handling across all modules and makes the API semantically nicer to use (also us to remove the directives turning off compiler warnings).

Simplified Error Handling

Return Values

Previously, every function returned a Result<T, SomeModuleErr::SomeErr>. However, a number of core tweetnacl functions (the majority) actually cannot fail (barring some catastrophic internal error). These functions, such as crypto_XXX_keypair(), now just directly return a &[u8] or Vec<u8> as appropriate and ditch the Result entirely.

Other functions, such as crypto_verify_XXX() now return a bool.

The only functions that return a Result are those that are expected to either return data or an error, such as the crypto_XXX_open() family of functions.

The unreachable!() macro is used extensively, and the number of module-level error enums is now greatly reduced.

Naming

Previously, error enums were named something like CryptoSignErr::CryptoSignOpen. Now, since they have proper namespacing, all error enums are simply Error (and would be accessed like crypto_sign::Error::Verify).

Constants

Previously, we had defined crate constants in ffi.rs. Constants are now defined in the .rs file in which they are used. For instance, crypto_secretbox_KEYBYTES is defined in crypto_secretbox.rs as KEYBYTES, and would be used like crypto_secretbox::KEYBYTES.

FFI Cleanup

Functions in ffi.rs were cleaned up a bit to make them easier to read.

Issues

This pull request closes #13 and #14

[dwtj]

In general, I think that this is a fantastic cleanup. The more appropriate use of namespacing cleans things up throughout the code. More judicious use of Result seems much more natural.

Here's one minor concern: I think it's a bit strange that there are multiple Error::Verify values, defined in each of the different modules. I understand that the semantics of a Verify error might differ from function to function, but that doesn't mean that we need a whole different type for these cases.

I propose that we use a single verify error value called crypto::verify::Error::Verify.

[nskinkel]

I see what you're saying with this, and it does seem repetitive.

What I was sort of going for was forcing users to be explicit about errors. For instance, consider this code:

// yes this is a sorta contrived example
fn verify_signature_and_decrypt(&sm: [u8; 96]) -> Result<[u8; blah], Error> {

    let result1 = crypto_sign_open(&sm, &pk, &sk);
    if result1 == Error::Verify {
        return result1;
    }
    let result2 = crypto_secretbox_open(&c, &n, &sk);
    if result2 == Error::Verify {
        return result2;
    }
    // do other stuff
}

How does the caller know whether a failure was from signature verification or ciphertext forgery? Semantically the results are different: bad signature or pk/sk pair vs. a forged ciphertext or bad nonce etc. The verification errors here aren't the same.

So...I would prefer keeping the error namespacing to force something like:

let result1 = crypto_sign_open(&sm, &sk, &pk);
assert_eq!(result1, sign::Error::Verify);
let result2 = crypto_secretbox_open(&c, &n, &sk);
assert_eq!(result2, secretbox::Error::Verify);

assert!(!(result1 == result2));

But maybe there's a way to do this without having to repeat the exact same enum definition all over the place? Can we make a top-level Error::Verify enum and then just do whatever the Rust equivalent is of subclassing in different modules to avoid code duplication but still get namespacing?