Skip to content

hashsig-glue: replace Box allocs with Zig-owned placement-init pattern - #935

Open
zclawz wants to merge 3 commits into
mainfrom
feat/ffi-zero-alloc-placement-main
Open

hashsig-glue: replace Box allocs with Zig-owned placement-init pattern#935
zclawz wants to merge 3 commits into
mainfrom
feat/ffi-zero-alloc-placement-main

Conversation

@zclawz

@zclawz zclawz commented May 26, 2026

Copy link
Copy Markdown
Contributor

Addresses the review comment #918 (comment) (ref PR #918) requesting that key-generation and signing be refactored to avoid additional Rust heap allocations, consistent with the caller-supplies-buffer pattern already used in multisig-glue.

Problem

hashsig-glue was Box-allocating every KeyPair, Signature, and PublicKey on the Rust heap and handing raw pointers to Zig. Zig then had to call back into Rust (hashsig_keypair_free etc.) to free them. This created a split-ownership model where allocation and deallocation crossed the FFI boundary in opposite directions, complicating lifetime tracking and adding unnecessary alloc/free pairs.

Solution: placement-init pattern

Rust exports layout queries so Zig can pre-allocate correctly-sized storage, then passes that storage to Rust for in-place initialisation. Cleanup runs Rust Drop in-place, then Zig frees its own buffer. The Rust heap is never involved for the outer struct.

rust/hashsig-glue/src/lib.rs

New layout queries:

  • hashsig_sizeof_keypair / _signature / _public_key — Zig queries at runtime to know how much to allocate
  • hashsig_alignof_keypair / _signature / _public_key — required alignment (C malloc always satisfies this for #[repr(C)] types)

New placement-init (replaces Box-returning functions):

  • hashsig_keypair_generate_into(out: *mut KeyPair, ...) — uses std::ptr::write, returns i32 (0 / -1)
  • hashsig_keypair_from_ssz_into, hashsig_sign_into, hashsig_signature_from_ssz_into, hashsig_public_key_from_ssz_into — same pattern

New drop-in-place (replaces Box::from_raw frees):

  • hashsig_keypair_deinit / hashsig_signature_deinit / hashsig_public_key_deinit — call std::ptr::drop_in_place, do not free the buffer (caller owns it)

Removed (all were Box-allocating):
hashsig_keypair_generate, hashsig_keypair_from_ssz, hashsig_keypair_free, hashsig_sign, hashsig_signature_free, hashsig_signature_from_ssz, hashsig_public_key_from_ssz, hashsig_public_key_free

pkgs/xmss/src/hashsig.zig

  • KeyPair, Signature, PublicKey wrappers now own their storage via cAlloc/cFree (thin wrappers over std.c.malloc/std.c.free)
  • C malloc always returns memory aligned to max_align_t (>=8 bytes on LP64), satisfying the alignment of any Rust #[repr(C)] type used here
  • Public API is unchanged — same generate, fromSsz, sign, verify, toBytes, deinit methods; no changes needed in aggregation.zig or any other caller
  • PublicKeyCache slot semantics unchanged; deinit correctly reconstructs _buf + handle from the stored integer pointer

What this does NOT eliminate

  • Inner leansig Vec<u8> data inside the XMSS key/signature types — these are owned by the leansig library and freed correctly when drop_in_place runs Drop
  • The intermediate Vec<u8> inside the to_bytes() serialisation helpers — a leansig API constraint; short-lived temporaries that never escape the FFI call

Verification

  • cargo check -p hashsig-glue clean
  • zig ast-check on hashsig.zig, aggregation.zig, lib.zig all pass
  • No remaining callers of any removed Box-based FFI symbol in the Zig tree

zclawz added a commit that referenced this pull request May 26, 2026
PublicKey gained a _buf field in the placement-init refactor (PR #935)
but the PublicKeyCache::deinit in lib.zig still used the old single-field
struct init. Match the already-corrected pattern from hashsig.zig:
reconstruct both _buf and handle from the stored atomic integer
(they both point to the start of the same C-heap allocation).

Fixes build/test CI failures on macOS.
@zclawz

zclawz commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

CI fix pushed — commit 4aced8e

Root cause: pkgs/xmss/src/lib.zig has its own copy of PublicKeyCache (separate from the one in hashsig.zig). When PublicKey gained the _buf field in this PR, hashsig.zig's deinit was correctly updated to supply both fields:

const raw: [*]u8 = @ptrFromInt(ptr_int);
var pk = PublicKey{ ._buf = raw, .handle = @ptrCast(raw) };

But lib.zig's deinit was left with the old single-field init:

var pk = PublicKey{ .handle = @ptrFromInt(ptr_int) }; // missing _buf

This caused error: missing struct field: _buf on all three macOS checks (build / test / Dummy prove).

Fix: port the same corrected pattern to lib.zig's deinit. Both _buf and handle point to the same C-heap allocation start, so the stored atomic usize (which is @intFromPtr(handle)) is sufficient to reconstruct both fields.

🤖 zclawz

zclawz added 3 commits May 26, 2026 16:08
Rust no longer heap-allocates the outer KeyPair / Signature / PublicKey
wrapper structs.  Instead, each init function accepts a caller-supplied
buffer and writes into it with std::ptr::write; the matching _deinit runs
std::ptr::drop_in_place (Rust Drop) without freeing the buffer.

Zig side allocates the buffers via C malloc (always malloc-aligned, safe
for all Rust #[repr(C)] types) and frees them after calling _deinit.

New Rust exports
  hashsig_sizeof_keypair / _signature / _public_key   — allocation size
  hashsig_alignof_keypair / _signature / _public_key  — required alignment
  hashsig_keypair_generate_into  (was: hashsig_keypair_generate → *mut Box)
  hashsig_keypair_from_ssz_into  (was: hashsig_keypair_from_ssz → *mut Box)
  hashsig_keypair_deinit         (was: hashsig_keypair_free → Box::from_raw)
  hashsig_sign_into              (was: hashsig_sign → *mut Box)
  hashsig_signature_deinit       (was: hashsig_signature_free)
  hashsig_signature_from_ssz_into
  hashsig_public_key_from_ssz_into
  hashsig_public_key_deinit

Removed Rust exports (were Box-allocating):
  hashsig_keypair_generate, hashsig_keypair_from_ssz, hashsig_keypair_free
  hashsig_sign, hashsig_signature_free, hashsig_signature_from_ssz
  hashsig_public_key_from_ssz, hashsig_public_key_free

Note: the inner XMSS types (leansig XmssPublicKey / XmssSignature etc.)
still carry their own heap-allocated data (Vec<u8> inside leansig);
those are inherent to the leansig API and are freed correctly when
_deinit runs Drop in-place.  The intermediate Vec<u8> in to_bytes()
serialisation helpers is similarly a leansig API constraint; those are
short-lived temporaries that do not escape the FFI call.

Addresses: #918 (comment)
PublicKey gained a _buf field in the placement-init refactor (PR #935)
but the PublicKeyCache::deinit in lib.zig still used the old single-field
struct init. Match the already-corrected pattern from hashsig.zig:
reconstruct both _buf and handle from the stored atomic integer
(they both point to the start of the same C-heap allocation).

Fixes build/test CI failures on macOS.
@zclawz
zclawz force-pushed the feat/ffi-zero-alloc-placement-main branch from cb8401e to 2efa790 Compare May 26, 2026 16:08
@zclawz

zclawz commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

CI fix: Rebased onto main to resolve build failure.

The CI was failing because main renamed initTestThreadPoolsetupTestPrimitives in forkchoice.zig and testing.zig. The PR branch was forked before that rename, so the merge commit had an undeclared identifier at line 3967.

Fix: clean rebase onto main (no conflicts, PR changes to xmss/hashsig-glue preserved). Force-pushed to re-trigger CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant