fix: Functions that return `Result<KeyPair, OperationError>` do not pass the Document Test

[Hyunggilwoo]

Document test for fn key_encrypt does not pass the assertion. (The functions that return Result<KeyPair, OperationError> would be expected to have the same error identified below)

The return type for fn key_encrypt is Result<KeyPair, OperationError>, but the key_encrypt and key_decrypt methods give me the following error:

no field pub_key on type Result<KeyPair, OperationError>, despite the documents on both key_encrypt and key_decrypt use KeyPair struct, which has the fields, pub_key and priv_key.

Basically, there are many places where the library would panic, and now, we return an error instead.

[Hyunggilwoo]

FYI in that the example code written in Doc Test need to be tested, so that correct code actually passes the assert()

[Dustin-Ray]

check this out

Ive added a quick fix that shows how to correctly use the symmetric function now. The rest of the ops can be documented in a similar way. Notice the use of the .map function which gets an error type and then lets you decide what to do with it. If you move through the other ops and document them in this way, it should be idiomatic enough.

// Verify operation success using map
msg.op_result.as_ref().map(|_| {
    assert!(msg.op_result.is_ok(), "Decryption failed");
}).expect("SHA3 decryption encountered an error");

breaking this down:

msg.op_result: this stores the result of the decryption, either being successful or having failed

.as_ref(): this is something unique to rust. When you grab op_result, you are borrowing it from the message. .as_ref means that you just want to borrow op_result for a second so you can look at it, without changing anything about it. this is an example of memory safety in rust. the ref cannot change the underlying value in any way, and the ref is dropped as soon as you are done with it.

.map(|_|) this is somewhat advanced syntax and is an example of a closure. the |_| is actually a anonymous function which is why you see brackets { immediately after the |_|. Inside of this function we are deciding what to do with the borrowed op_result reference.

assert!: this is a macro in rust. macros are super easy to understand, they simply let you type assert! instead of a huge block of code that actually powers the assertion. Rust can be very verbose and this helps keep the code clean. this is an assertion that works the same way as any other language. it is fine for a usage example or a test but not in production code generally because it was crash the program with a panic. but here, we are just saying we expect the op_result to be ok, and if it is not, panic with the string "Decryption failed", and if anything else went wrong, panic with "SHA3 decryption encountered an error

[Hyunggilwoo]

check this out

Ive added a quick fix that shows how to correctly use the symmetric function now. The rest of the ops can be documented in a similar way. Notice the use of the .map function which gets an error type and then lets you decide what to do with it. If you move through the other ops and document them in this way, it should be idiomatic enough.

// Verify operation success using map
msg.op_result.as_ref().map(|_| {
    assert!(msg.op_result.is_ok(), "Decryption failed");
}).expect("SHA3 decryption encountered an error");

breaking this down:

msg.op_result: this stores the result of the decryption, either being successful or having failed

.as_ref(): this is something unique to rust. When you grab op_result, you are borrowing it from the message. .as_ref means that you just want to borrow op_result for a second so you can look at it, without changing anything about it. this is an example of memory safety in rust. the ref cannot change the underlying value in any way, and the ref is dropped as soon as you are done with it.

.map(|_|) this is somewhat advanced syntax and is an example of a closure. the |_| is actually a anonymous function which is why you see brackets { immediately after the |_|. Inside of this function we are deciding what to do with the borrowed op_result reference.

assert!: this is a macro in rust. macros are super easy to understand, they simply let you type assert! instead of a huge block of code that actually powers the assertion. Rust can be very verbose and this helps keep the code clean. this is an assertion that works the same way as any other language. it is fine for a usage example or a test but not in production code generally because it was crash the program with a panic. but here, we are just saying we expect the op_result to be ok, and if it is not, panic with the string "Decryption failed", and if anything else went wrong, panic with "SHA3 decryption encountered an error

Thank you. I will revise the remaining functions in ops.rs as how you revised it from above.

[Hyunggilwoo]

Just noting that completing a single doc test took 0.40 seconds. I don't know if this could lead to an error later.

[Hyunggilwoo]

Just noting that completing a single doc test took 0.40 seconds. I don't know if this could lead to an error later.

A major reason that these operations take a long time to encrypt/decrypt is because the message is a random bytes of 5MB.

[Dustin-Ray]

those run times are actually only relevant in the testing environment, if you want to get a sense of the actual performance, you can run cargo bench to see real performance numbers. everything looks good there!

[Dustin-Ray]

this PR is merged to main and so closing this issue, nicely done and thanks for all of the help!