rust-bitcoin/rust-secp256k1

`secp256k1`: Remove `secp256k1_sys` types from the public API

tcharding opened this issue · 4 comments

If we want to be able to stabalize this crate and release a v1.0 we either have to stabalize secp256k1_sys or remove all the ffi types from the public API.

Since the secp256k1_sys crate may never stabalize we should proactively wrap/hide/remove the secp256k1_sys types from the public API.

Yeah, I guess you're correct.

I wonder if we should still export secp256k1-sys itself. Maybe under a new name that is clearly marked _unstable?

Clearly marked _unstable is good, I've seen most, if not all, crates to also put it behind a feature flag.

To clarify, is this referring to the NonceFn, EcdhHashFn, SchnorrNonceFn and EllswiftEcdhHashFn?

WDYT about re-exporting from a new feature gated unstable module instead of the _unstable suffix? Something like

#[cfg(feature = "unstable")]
/// pub re-export of ffi types which may never stabilise
pub mod unstable {
    pub type NonceFn = super::NonceFn;
    pub type EcdhHashFn = super::EcdhHashFn;
    pub type SchnorrNonceFn  = super::SchnorrNonceFn;
    pub type EllswiftEcdhHashFn = super::EllswiftEcdhHashFn;
}

To clarify, is this referring to the NonceFn, EcdhHashFn, SchnorrNonceFn and EllswiftEcdhHashFn?

It's more than that -- most types have to_ffi or from_ffi or similar methods which convert between the rust-secp types and the secp-sys types. So we would need to also put those types in the corresponding module, and rename the functions to have unstable in the name.

I'm not a huge fan of having a feature-gate, because it increases the test matrix, it brings up bad memories of the old unstable flag that we used to feature-gate nightly-compiler-only stuff, and because I think that naming the module/functions unstable already provides enough of a "don't use these" cue.

But if other people feel we should have the feature gate I will go along with it.

We have a similar problem in bitcoin-primitives.