Internal "unsoundness" in sign_ecdsa_with_noncedata_pointer

[Kixunil]

While sign_ecdsa_with_noncedata_pointer is private so consumers can't cause UB in safe code it's still considered wrong to not mark the function unsafe because it's confusing. From my understanding such issue caused horrible flamewar at actix-web crate.

Given that the function is called either with &[u8; 32] or null pointer I propose to change it to accept Option<&[u8; 32]> instead. Internally, it does the same thing thanks to layout optimizations but is safer. We supply null pointer if there is None and the pointer obtained from reference if there is Some.

[apoelstra]

I think it'd be simpler to just mark it unsafe. If we accept a Option<&[u8; 32]> then we have to convert that to a pointer inside the function before calling FFI (I understand the Option optimization but don't believe we can depend on that when crossing a FFI boundary).

Alternately: making the function unsafe and removing the internal unsafe {} saves two lines of code. Adding a conversion requires an extra line of code :).

[Kixunil]

don't believe we can depend on that when crossing a FFI boundary

Actually, we can! rustc would complain if we couldn't but it doesn't complain. Yes this is a documented feature, guaranteed to be correct.

Edit: I believe even if this wasn't true converting option would've been better.

[apoelstra]

Ok, nice. Concept ACK doing this. I can PR if you'd like, though if you're up for it that might be faster :)

[Kixunil]

Got a bunch of things to do, so I probably can't sooner than in 24 hours.

[apoelstra]

So, we can't actually change the FFI signature to take a Option<&[u8; 32]> because the signature on the C side is a void* not a whatever_u8_32_would_be_in_c* -- and in fact, this is not, in general, a pointer to 32 bytes of data. (It just happens to be with the rfc6979 nonce function that we're calling here.)

I can certainly clean up the unsafety but I don't see a way to use the nullable pointer trick.

[Kixunil]

Ah, good point. match is still better and it'll almost certainly get optimized out anyway.