Is Result<bool, E> idiomatic or recommend against?

[alxiong]

Which one of the following API designs is more idiomatic and less error-prone?

// return Ok(true) when proof is valid, Ok(false) when proof is invalid
// return Err(E) when proof is malformed or other intermediate step failures
fn verify(proof) -> Result<bool, E>;

// return Ok(()) when proof is verified, Err(E) otherwise.
// E::InvalidProof is just a case under the error type: `enum E`
fn verify(proof) -> Result<(), E>; 

cc @bascule

[RedDocMD]

The first one is idiomatic when it valid to have a false result.
The second one is idiomatic when it is invalid to have a false result.

[scottmcm]

It's a modelling question, so without more domain information we can't really say.

If you're parsing a bool, then -> Result<bool, E> is absolutely correct, since false being success is correct.

If you're validating an SSL certificate, then -> Result<(), E> is probably better so if people .unwrap() it they're safe -- it would be really easy to mistakenly think that Ok(_) means it passed validation, and making people remember to check it's not Ok(false) is error-prone.

But really, Parse, don’t validate. A better signature is UncheckedFoo -> Result<Foo, SomeErrorIncludingTheUncheckedFoo>, rather than bool or ().

[H2CO3]

How exactly is an "invalid" proof different from "malformed" or "intermediate step failure"?

[zirconium-n]

Not OP, my understanding:

• Valid: couldd be parsed and contains no logic error.
• Invalid: could be parsed but contains logic error.
• Malformed: failed to parse.

[kornel]

Pragmatically, #[must_use] won't work on Result<bool, E>, so you're risking users calling verify()?; and proceeding without checking the boolean. So use Result<(), E>, which makes such mistake less likely to go unnoticed.

Or even remove verify method entirely, and have:

fn get_proof() -> Result<Proof, VerificationFailed>

instead, so that it's impossible to have an invalid proof in the first place.

[bascule]

I'd agree that using bool for a proof verification failure risks bugs that would otherwise be caught by #[must_use] on Result.

FWIW, the signature crate uses Result exclusively for similar purposes, so as to ensure signature verification errors are always handled.

[JeremyRubin]

A third option is to use newtype wrappers and to make your API something like:

parse(s: *[u8]) -> Result<ParsedProof, ParseError>

validate(p: ParsedProof) -> Result<ValidProof, ValidationError>

it's not always possible to do this, but if you can split out the steps it can lead to much better API designs since you can pass around proofs as validatedproofs to APIs that have invariants requiring a proof be correct.