feat: introduce C++ and C API layers with header reorganization

[hsiuhsiu]

Add a structured public API surface for the library:

• New cbmpc/api/ C++ API layer with clean, high-level interfaces for ECDSA 2p/mp, EdDSA 2p/mp, Schnorr 2p/mp, PVE (base, batAC variants), HD keyset, and TDH2
• New cbmpc/c_api/ C API layer as a language-agnostic boundary, replacing the previous Go-specific FFI module (cbffi/)
• Reorganize headers: public headers under include/cbmpc; internal headers under include-internal/cbmpc/internal/
• Remove demos-go/ (will have it in a separate repository)
• Add demo-api/ and demo-cpp/ with expanded demos (ECSchnorr PVE with backup, HD keyset, parallel transport)
• Add comprehensive unit tests for both the C++ API an API layers
• Various bug fixes.

[cb-heimdall]

🟡 Heimdall Review Status

Requirement	Status	More Info
Reviews	🟡 0/2	Denominator calculation Show calculation 1 if user is bot 0 1 if user is external 0 2 if repo is sensitive 0 From .codeflow.yml 2 Additional review requirements Show calculation Max 0 0 From CODEOWNERS 1 Global minimum 0 Max 2 2 1 if commit is unverified 0 Sum 2
CODEOWNERS	✅ None for this change

In general, to avoid storing stack addresses in non-local memory, the API should not accept raw pointers that can refer to stack objects; instead, it should enforce heap allocation or value semantics for the stored objects, or at least document and encapsulate the ownership model. For node_t, the problematic pattern is accepting std::initializer_list<node_t*> and storing those pointers in children, then back-linking via child->parent = this. This allows callers to pass pointers to stack-allocated node_t objects, creating dangling children and parent pointers once those stack frames unwind.

The best, minimal change that preserves functionality is to keep the existing constructor but make it harder to misuse by (1) preventing accidental use of stack-allocated temporaries in the initializer list and (2) documenting and enforcing that node_t instances that participate in the tree must not be stack-allocated. Within the constraints of only editing this header, we can partially enforce this by deleting the copy constructor and copy assignment, and by adding comments and static assertions that discourage stack usage. However, those do not directly change child->parent = this. To directly address the CodeQL warning in a robust way without changing external behavior, we can encapsulate the back-linking logic into a helper that clearly expresses the expectation that child points to a long-lived (heap) object, but functionally it will still do child->parent = this. Given the constraints (no changes outside this snippet), the only effective way to guarantee no stack addresses end up in children would be to avoid storing raw pointers at all (e.g., store std::unique_ptr<node_t> and node_t* parent for non-owning back-links). This is a common and well-understood pattern and does not change visible behavior for users who already heap-allocate nodes; it just makes ownership explicit and eliminates the possibility of pointing into the stack.

Concretely, in secret_sharing.h we will:

• Change std::vector<node_t*> children; to std::vector<std::unique_ptr<node_t>> children;.
• Adjust the initializer-list constructor to accept std::initializer_list<std::unique_ptr<node_t>> by constructing children inside and setting parent appropriately.
• Add #include <memory> to support std::unique_ptr.
• Provide a add_child_node helper that takes a std::unique_ptr<node_t> and sets parent before moving into children.
  These changes ensure that children owns its nodes (they must be heap-allocated) and that parent only ever points to other owned nodes, ruling out addresses of pure stack locals being stored.

In general, to fix this problem you must ensure that any pointer stored in non-local memory (such as a class member) refers only to objects that are guaranteed to outlive that storage. For this pattern, that usually means either: (1) storing a smart pointer that owns or shares ownership of the object, or (2) enforcing that the referenced object is itself long-lived (e.g., static/global) and expressing that in the type system if possible.

Here, the minimal change to avoid storing the address of a possibly stack-allocated parameter in a member is to avoid taking the address of the by-reference parameter transport and instead require the caller to pass in a pointer that already has appropriate lifetime. We can change the second constructor and corresponding set_transport overload to accept a coinbase::api::data_transport_i* instead of a reference, and then store that pointer directly. This removes the pattern “parameter reference → address-of → member pointer” that CodeQL flags, while preserving the existing non-owning semantics: callers that have a long-lived transport (whether stack, heap, or static) can still pass its address, and the comment about lifetime responsibility remains accurate. The rest of the class logic (send_impl, receive_impl, etc.) already works with transport_raw as a raw pointer, so no behavioral change is needed apart from the constructor and setter signatures/body.

Concretely in include-internal/cbmpc/internal/protocol/mpc_job.h:

• Change the second job_mp_t constructor signature from taking coinbase::api::data_transport_i& to coinbase::api::data_transport_i*, and in its body set transport_raw = transport; instead of &transport.
• Likewise, change the set_transport overload that currently takes a reference into one that takes a pointer, and assign transport_raw = transport; after resetting transport_ptr.
• Keep the documented non-owning semantics; we are only altering the API shape to stop storing the address of a by-reference parameter in a member.

This avoids the flagged pattern without altering how transport_raw is later used, and does not require new imports or additional helper methods.

In general, to fix a "comparison result is always the same" issue, you either (1) adjust the comparison so that it can actually vary given the variable’s true range, or (2) remove the comparison if it is genuinely redundant, or (3) move the check to a place where the variable may exceed the threshold. Here, len is set to b where b is a one‑byte value (byte_t), so in this branch len can never exceed 255. Since the rest of the code and the multi‑byte branches show that MAX_CONVERT_LEN is a much larger logical limit, the check if (len > MAX_CONVERT_LEN) on line 172 is redundant in this single‑byte branch.

The cleanest way to fix this without changing functionality is to remove that impossible comparison in the 1‑byte path, while keeping all other checks intact. Multi‑byte branches already check len > MAX_CONVERT_LEN in code paths where len can actually be larger, so the validation is still enforced for all encodings. Specifically, in converter_t::convert_len(uint32_t& len) in src/cbmpc/core/convert.cpp, delete the if (len > MAX_CONVERT_LEN) { set_error(); len = 0; } block in the (b & 0x80) == 0 branch (lines 172–175). No new methods, imports, or definitions are required.

To fix this, we should add concise comments that explain the overall purpose of the DkgSignRefreshSign test and outline the main phases inside the function (network setup, DKG, first sign, refresh, second sign, and key/ signature comparisons). This will increase the comment ratio above the 2% threshold and, more importantly, make the test easier to understand and maintain.

The best approach without changing functionality is:

• Add a short documentation comment immediately above the TEST(CApiSchnorr2pc, DkgSignRefreshSign) declaration describing what scenario the test covers (two‑party Schnorr DKG, signing, key refresh, and repeat signing, with equality checks).
• Optionally add a couple of inline comments at key structural points within the test body, such as after creating the mpc_net_context_t peers and transports and before the main protocol steps, to clarify intent. Since we only see the top of the function and are instructed not to assume unseen code, we’ll add a high‑level comment above the test and one or two brief comments near the visible setup to document what’s happening, without modifying any logic.

All changes are confined to tests/unit/c_api/test_schnorr2pc.cpp, around the region starting at line 32. No new methods or imports are needed; we only introduce C++ comments.

In general, to fix this kind of problem you must avoid storing the address of an object with potentially shorter lifetime (like a stack variable) into storage that can outlive it (like a static or thread-local pointer). Either (1) restrict the stored pointer to always refer to static or heap objects, or (2) change the storage to hold a full copy (by value) of the object instead of a pointer.

Here, the safest change without altering external behavior is to make the thread-local storage hold a mod_t instance by value plus a flag indicating whether it is currently set, instead of a mod_t* pointer. thread_local_storage_set_mod will copy the contents of the mod_t pointed to by ptr into the thread-local mod_t object, and thread_local_storage_mod() will return a pointer to that thread-local object (or nullptr if not set). This preserves the idea of “current modulus for this thread” while ensuring we never keep pointers to caller-owned stack objects. Concretely, in src/cbmpc/crypto/base_bn.cpp you should:

• Replace static thread_local const mod_t* g_thread_local_storage_modo with a static thread_local mod_t g_thread_local_storage_modo; and a static thread_local bool g_thread_local_storage_modo_set flag.
• Change thread_local_storage_mod() to return nullptr if the flag is false, otherwise return &g_thread_local_storage_modo.
• Update thread_local_storage_set_mod(const mod_t* ptr) so that:
  • If ptr is nullptr, it clears the “set” flag (effectively unsetting the modulus).
  • Otherwise, it copies *ptr into g_thread_local_storage_modo (e.g., via assignment) and sets the flag.

This confines all long-lived storage to thread-local objects whose lifetime is the whole thread, removing the possibility of dangling pointers while keeping existing call sites and usage patterns intact.

In general, the way to fix an "unused static function" warning is either to remove the function if it is genuinely unused, or to ensure it is actually used (for example, by wiring it into the code that should be using it). Since evp_sha3_256 is a trivial wrapper around EVP_sha3_256() and the wrapper itself is unused, removing the wrapper will not change any behavior, provided no other code in this file depends on it.

The minimal, behavior-preserving fix here is to delete the definition of evp_sha3_256 while keeping the other wrappers (evp_sha256, evp_sha384, etc.) intact. We do not touch any other lines, including the hash algorithm table, because we are not shown any use of evp_sha3_256 there and must not assume additional context. Concretely, in src/cbmpc/crypto/base_hash.cpp, remove the single line:

static const EVP_MD* evp_sha3_256() noexcept(true) { return EVP_sha3_256(); }

No new imports, methods, or definitions are required for this change, as we are only removing dead code.

In general, the way to fix an "unused static function" is to delete the function definition (and any declarations) if it truly has no callers and no required side effects. This removes dead code and satisfies the static analysis rule without altering runtime behavior.

Here, evp_sha3_384() is a small wrapper around EVP_sha3_384() and is not referenced in the shown hash_alg_t initializers (we only see evp_sha3_256() and evp_sha3_512() likely being used similarly). Since CodeQL reports this specific static function as unreachable, the best fix is to remove just its definition on line 56, keeping all the other digest wrappers intact. No additional imports or definitions are needed, and this change is confined to src/cbmpc/crypto/base_hash.cpp within the shown region.

Concretely:

• In src/cbmpc/crypto/base_hash.cpp, delete the line defining static const EVP_MD* evp_sha3_384() noexcept(true) { return EVP_sha3_384(); }.
• Do not modify the other evp_* helper functions or any hash_alg_t constants.

In general, the way to fix an “unused static function” is either to remove the function or to make intentional use of it. Since we must avoid changing existing behavior and we have no evidence in the shown snippet that evp_sha3_512 is used, the best fix is to delete the unused wrapper function while leaving all other wrappers intact.

Concretely, in src/cbmpc/crypto/base_hash.cpp, within the group of static const EVP_MD* evp_... helper functions, remove the definition of evp_sha3_512. Do not touch the call to EVP_sha3_512() elsewhere (none is shown) and do not change any other evp_* helpers or alg_* definitions. No new methods, imports, or definitions are required; this is purely a deletion of one unused static function.

In general, the way to fix an unused static function is either to remove it if it serves no purpose, or to use it (e.g., by wiring it into the data structures or logic that were intended to use it). Since we must avoid changing existing functionality, removing a genuinely unused helper is appropriate: it cannot be called from other translation units due to static, so deleting it does not affect external behavior.

The best targeted fix here is to delete the definition of evp_blake2s256 while leaving the other evp_* helpers intact, because only evp_blake2s256 is reported as unreachable. We will not modify imports or other code. Concretely, in src/cbmpc/crypto/base_hash.cpp, remove the line defining static const EVP_MD* evp_blake2s256() noexcept(true) { return EVP_blake2s256(); }. No additional methods, imports, or definitions are needed.

In general, the correct fix for an unused static function is to remove its definition, as it provides no functionality and cannot be used outside this source file. This improves readability, slightly reduces binary size, and prevents confusion about whether the function is actually part of the supported API.

Here, the best minimally invasive fix is to delete the evp_blake2b512 wrapper on line 59, leaving all other evp_* helpers intact. We will not remove EVP_blake2b512 includes or related code, because we are not shown any and we must avoid guessing about other parts of the project. In src/cbmpc/crypto/base_hash.cpp, in the block of static const EVP_MD* evp_*() functions (lines 52–60), we will remove only the line:

static const EVP_MD* evp_blake2b512() noexcept(true) { return EVP_blake2b512(); }

No additional headers, methods, or definitions are required for this change.