ethereum / c-kzg-4844

A minimal implementation of the Polynomial Commitments API for EIP-4844 and EIP-7594, written in C.
Apache License 2.0
112 stars 105 forks source link

Make Rust `bindgen` build dependency optional #382

Closed DaniPopes closed 7 months ago

DaniPopes commented 10 months ago

Now that header definitions are stable across builds (#377), we can remove the Rust bindgen build dependency by gating it to an optional feature. This also transitively removes the dependency on libclang for Rust bindings.

DaniPopes commented 10 months ago

Looks like it will suppress diffs for GitHub PRs

Yep, and excludes from statistics like the language percentages


Just realized that this fails on Windows because the C_KZG_RET enum repr is i32 instead of u32: https://github.com/ethereum/c-kzg-4844/actions/runs/6839391407/job/18597486957?pr=382

I'd disable this for Windows because it's pretty minor, but let me know what I should do

jtraglia commented 10 months ago

Ah Windows... It would be nice if this worked for all platforms.

My first thought is to force the enum to be unsigned in C. It's not ideal, but this should work:

--- a/src/c_kzg_4844.h
+++ b/src/c_kzg_4844.h
@@ -94,9 +94,10 @@ typedef Bytes48 KZGProof;

 /**
  * The common return type for all routines in which something can go wrong.
+ * Note, values are large to force enum type to be an unsigned integer.
  */
 typedef enum {
-    C_KZG_OK = 0,  /**< Success! */
+    C_KZG_OK = 0x80000000,  /**< Success! */
     C_KZG_BADARGS, /**< The supplied data is invalid in some way. */
     C_KZG_ERROR,   /**< Internal error - this should never occur. */
     C_KZG_MALLOC,  /**< Could not allocate memory. */

Or this, which I think it pretty ugly but it's more clear what's going on:

/**
 * The common return type for all routines in which something can go wrong.
 */
typedef uint32_t C_KZG_RET;

/**
 * The possible return values for C_KZG_RET.
 */
enum {
    C_KZG_OK = (C_KZG_RET)0,
    C_KZG_BADARGS = (C_KZG_RET)1,
    C_KZG_ERROR = (C_KZG_RET)2,
    C_KZG_MALLOC = (C_KZG_RET)3,
};