secp256k1_context_static should be a const variable [edited]

[purpleKarrot]

Some language bindings have issues with exported variables. The static context is currently exported as a variable:

SECP256K1_API const secp256k1_context *secp256k1_context_static;

which is implemented as:

const secp256k1_context *secp256k1_context_static = &secp256k1_context_static_;

A more portable way would be to provide it through a function:

SECP256K1_API const secp256k1_context *secp256k1_context_static(void);

with implementation:

const secp256k1_context *secp256k1_context_static(void)
{
    return &secp256k1_context_static_;
}

PS: It is clear that changing the symbol into a function is a breaking change and therefore off the table. However, please consider adding a new symbol with the proposed implementation.

[real-or-random]

Some language bindings have issues with exported variables.

Can you elaborate a bit? Which bindings and what are the issues?

[purpleKarrot]

In Swift, when secp256k1_context_static is passed to a function, the compiler complains:

Reference to var 'secp256k1_context_static' is not concurrency-safe because it involves shared mutable state

This is definitely a false positive, because the instance is const. The workaround is to define a custom function in the ffi wrapper: https://github.com/swift-bitcoin/swift-bitcoin/blob/develop/src/ecc-helper/ecdsa.c#L6-L8

I remember having had issues with exported variables in the past with other languages. I don't remember the exact details. It may have been with Javascript, Go, Lua, or Matlab. It is less of an issue when the variable is of a builtin type, which is the case here with a pointer.

While it is easily possible to find work-arounds for any ffi, providing functions only is just more intuitive. Consider this Python:

>>> from ctypes import CDLL
>>> lib = CDLL("libsecp256k1.5.dylib")
>>> lib
<CDLL 'libsecp256k1.5.dylib', handle 6d672be0 at 0x102b3a430>
>>> lib.secp256k1_selftest
<_FuncPtr object at 0x102b83340>
>>> lib.secp256k1_selftest()
-1486072369
>>> lib.secp256k1_context_static
<_FuncPtr object at 0x102b83a00>   # <--- orly?
>>> lib.secp256k1_context_static()
fish: Job 1, 'python3' terminated by signal SIGBUS (Misaligned address error)

[real-or-random]

In Swift, when secp256k1_context_static is passed to a function, the compiler complains:

Reference to var 'secp256k1_context_static' is not concurrency-safe because it involves shared mutable state

This is definitely a false positive, because the instance is const.

Not sure. A pointer to const is a promise not to mutate through the pointer, but that doesn't mean that the object pointed to cannot be mutated at all (e.g., there could exist a pointer to non-const). In this case, secp256k1_context_static_ is indeed static const, but I assume this is (in general) not obvious if you just see the shared library. I mean, the identifier secp256k1_context_static_ doesn't even make sense at this level of abstraction.

Anyway, I agree that if Swift's FFI is happy with a function that returns a pointer to const, then it should also be happy with a variable of the same type...

---

I don't think it hurts to add something like this:

/** Return secp256k1_get_context_static. */
const secp256k1_context* secp256k1_get_context_static(void);

const secp256k1_context* secp256k1_get_context_static(void) {
    return secp256k1_context_static;
}

But then we'd need to do the same for all the other exported variables (and add a rule to CONTRIBUTING.md). I'm not entirely convinced that the best place to do this in libsecp256k1. Shouldn't it be the job of bindings to provide a wrapper if necessary?

[sipa]

The Swift FFI is right, actually.

The variable secp256k1_context_static is a mutable global variable (it points to a constant object, but the variable itself isn't constant).

I think we should change the definition from

const secp256k1_context *secp256k1_context_static = &secp256k1_context_static_;

to:

const secp256k1_context * const secp256k1_context_static = &secp256k1_context_static_;

(and similarly in the .h file)

[real-or-random]

Oh, indeed.

@purpleKarrot Can you check if this solves your problem?

[purpleKarrot]

Yes. I confirm this fixes the problem: 5f6bacc

[real-or-random]

Great, are you willing to open a PR?

[real-or-random]

I took the liberty to edit the title of the issue.