Merge branch 'main' into fix/sigsegv

.cargo/config.toml

@@ -0,0 +1,7 @@
+[env]
+# Pin the macOS deployment target so vendored C/C++ builds (SQLCipher, OpenSSL)
+# compile with the same minimum version as the Nim linker expects.
+# Without this, the host SDK version is used (currently 15.5), causing
+# "was built for newer macOS version" linker warnings.
+# This is caused by nimble and cargo defaulting to different targets.
+MACOSX_DEPLOYMENT_TARGET = { value = "15.0", force = false }

.github/workflows/ci.yml

@@ -2,11 +2,11 @@ name: CI
 
 on:
   push:
-    branches: [ main ]
+    branches: [main]
   pull_request:
-    branches: [ main ]
+    branches: [main]
 
-env: 
+env:
   CARGO_TERM_COLOR: always
 
 jobs:
@@ -36,3 +36,21 @@ jobs:
       - run: rustup update stable && rustup default stable
       - run: rustup component add rustfmt
       - run: cargo fmt --all -- --check
+
+  nim-bindings-test:
+    name: Nim Bindings Test
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest]
+    steps:
+      - uses: actions/checkout@v4
+      - run: rustup update stable && rustup default stable
+      - name: Install Nim via choosenim
+        run: |
+          curl https://nim-lang.org/choosenim/init.sh -sSf | sh -s -- -y
+          echo "$HOME/.nimble/bin" >> $GITHUB_PATH
+      - run: nimble install -dy
+        working-directory: nim-bindings
+      - run: nimble pingpong
+        working-directory: nim-bindings

conversations/src/api.rs

@@ -55,9 +55,12 @@ pub fn create_context(name: repr_c::String) -> repr_c::Box<ContextHandle> {
 }
 
 /// Returns the friendly name of the contexts installation.
+///
+/// # ABI note
+/// The result is written through `out` (Nim's calling convention for large struct returns).
 #[ffi_export]
-pub fn installation_name(ctx: &ContextHandle) -> repr_c::String {
-    ctx.0.installation_name().to_string().into()
+pub fn installation_name(ctx: &ContextHandle, out: &mut repr_c::String) {
+    *out = ctx.0.installation_name().to_string().into();
 }
 
 /// Destroys a conversation store and frees its memory
@@ -74,11 +77,13 @@ pub fn destroy_context(ctx: repr_c::Box<ContextHandle>) {
 /// Creates an intro bundle for sharing with other users
 ///
 /// # Returns
-/// Returns the number of bytes written to bundle_out
 /// Check error_code field: 0 means success, negative values indicate errors (see ErrorCode).
+///
+/// # ABI note
+/// The result is written through `out` (Nim's calling convention for large struct returns).
 #[ffi_export]
-pub fn create_intro_bundle(ctx: &mut ContextHandle) -> CreateIntroResult {
-    match ctx.0.create_intro_bundle() {
+pub fn create_intro_bundle(ctx: &mut ContextHandle, out: &mut CreateIntroResult) {
+    *out = match ctx.0.create_intro_bundle() {
         Ok(v) => CreateIntroResult {
             error_code: ErrorCode::None as i32,
             intro_bytes: v.into(),
@@ -87,7 +92,7 @@ pub fn create_intro_bundle(ctx: &mut ContextHandle) -> CreateIntroResult {
             error_code: ErrorCode::UnknownError as i32,
             intro_bytes: repr_c::Vec::EMPTY,
         },
-    }
+    };
 }
 
 /// Creates a new private conversation
@@ -95,19 +100,24 @@ pub fn create_intro_bundle(ctx: &mut ContextHandle) -> CreateIntroResult {
 /// # Returns
 /// Returns a struct with payloads that must be sent, the conversation_id that was created.
 /// The NewConvoResult must be freed.
+///
+/// # ABI note
+/// The result is written through `out` (Nim's calling convention for large struct returns).
 #[ffi_export]
 pub fn create_new_private_convo(
     ctx: &mut ContextHandle,
     bundle: c_slice::Ref<'_, u8>,
     content: c_slice::Ref<'_, u8>,
-) -> NewConvoResult {
+    out: &mut NewConvoResult,
+) {
     // Convert input bundle to Introduction
     let Ok(intro) = Introduction::try_from(bundle.as_slice()) else {
-        return NewConvoResult {
+        *out = NewConvoResult {
             error_code: ErrorCode::BadIntro as i32,
             convo_id: "".into(),
             payloads: Vec::new().into(),
         };
+        return;
     };
 
     // Create conversation
@@ -122,11 +132,11 @@ pub fn create_new_private_convo(
         })
         .collect();
 
-    NewConvoResult {
+    *out = NewConvoResult {
         error_code: 0,
         convo_id: convo_id.to_string().into(),
         payloads: ffi_payloads.into(),
-    }
+    };
 }
 
 /// Sends content to an existing conversation
@@ -134,19 +144,24 @@ pub fn create_new_private_convo(
 /// # Returns
 /// Returns a PayloadResult with payloads that must be delivered to participants.
 /// Check error_code field: 0 means success, negative values indicate errors (see ErrorCode).
+///
+/// # ABI note
+/// The result is written through `out` (Nim's calling convention for large struct returns).
 #[ffi_export]
 pub fn send_content(
     ctx: &mut ContextHandle,
     convo_id: repr_c::String,
     content: c_slice::Ref<'_, u8>,
-) -> SendContentResult {
+    out: &mut SendContentResult,
+) {
     let payloads = match ctx.0.send_content(&convo_id, &content) {
         Ok(p) => p,
         Err(_) => {
-            return SendContentResult {
+            *out = SendContentResult {
                 error_code: ErrorCode::UnknownError as i32,
                 payloads: safer_ffi::Vec::EMPTY,
             };
+            return;
         }
     };
 
@@ -158,10 +173,10 @@ pub fn send_content(
         })
         .collect();
 
-    SendContentResult {
+    *out = SendContentResult {
         error_code: 0,
         payloads: ffi_payloads.into(),
-    }
+    };
 }
 
 /// Handles an incoming payload
@@ -170,15 +185,19 @@ pub fn send_content(
 /// Returns HandlePayloadResult
 /// This call does not always generate content. If data is zero bytes long then there
 /// is no data, and the converation_id should be ignored.
+///
+/// # ABI note
+/// The result is written through `out` (Nim's calling convention for large struct returns).
 #[ffi_export]
 pub fn handle_payload(
     ctx: &mut ContextHandle,
     payload: c_slice::Ref<'_, u8>,
-) -> HandlePayloadResult {
-    match ctx.0.handle_payload(&payload) {
+    out: &mut HandlePayloadResult,
+) {
+    *out = match ctx.0.handle_payload(&payload) {
         Ok(o) => o.into(),
         Err(e) => e.into(),
-    }
+    };
 }
 
 // ------------------------------------------
@@ -195,9 +214,13 @@ pub struct CreateIntroResult {
 }
 
 /// Free the result from create_intro_bundle
+///
+/// # ABI note
+/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
+/// accepting the struct by value would be an ABI mismatch on the caller side.
 #[ffi_export]
-pub fn destroy_intro_result(result: CreateIntroResult) {
-    drop(result);
+pub fn destroy_intro_result(result: &mut CreateIntroResult) {
+    unsafe { std::ptr::drop_in_place(result) }
 }
 
 /// Payload structure for FFI
@@ -219,9 +242,13 @@ pub struct SendContentResult {
 }
 
 /// Free the result from send_content
+///
+/// # ABI note
+/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
+/// accepting the struct by value would be an ABI mismatch on the caller side.
 #[ffi_export]
-pub fn destroy_send_content_result(result: SendContentResult) {
-    drop(result);
+pub fn destroy_send_content_result(result: &mut SendContentResult) {
+    unsafe { std::ptr::drop_in_place(result) }
 }
 
 /// Result structure for handle_payload
@@ -237,9 +264,13 @@ pub struct HandlePayloadResult {
 }
 
 /// Free the result from handle_payload
+///
+/// # ABI note
+/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
+/// accepting the struct by value would be an ABI mismatch on the caller side.
 #[ffi_export]
-pub fn destroy_handle_payload_result(result: HandlePayloadResult) {
-    drop(result);
+pub fn destroy_handle_payload_result(result: &mut HandlePayloadResult) {
+    unsafe { std::ptr::drop_in_place(result) }
 }
 
 impl From<ContentData> for HandlePayloadResult {
@@ -291,7 +322,11 @@ pub struct NewConvoResult {
 }
 
 /// Free the result from create_new_private_convo
+///
+/// # ABI note
+/// Takes `&mut` instead of ownership because Nim always passes large structs as a pointer;
+/// accepting the struct by value would be an ABI mismatch on the caller side.
 #[ffi_export]
-pub fn destroy_convo_result(result: NewConvoResult) {
-    drop(result);
+pub fn destroy_convo_result(result: &mut NewConvoResult) {
+    unsafe { std::ptr::drop_in_place(result) }
 }

conversations/src/lib.rs

@@ -25,7 +25,11 @@ mod tests {
         let mut raya = create_context("raya".into());
 
         // Raya Creates Bundle and Sends to Saro
-        let intro_result = create_intro_bundle(&mut raya);
+        let mut intro_result = CreateIntroResult {
+            error_code: -99,
+            intro_bytes: safer_ffi::Vec::EMPTY,
+        };
+        create_intro_bundle(&mut raya, &mut intro_result);
         assert!(is_ok(intro_result.error_code));
 
         let raya_bundle = intro_result.intro_bytes.as_ref();
@@ -33,13 +37,24 @@ mod tests {
         // Saro creates a new conversation with Raya
         let content: &[u8] = "hello".as_bytes();
 
-        let convo_result = create_new_private_convo(&mut saro, raya_bundle, content.into());
+        let mut convo_result = NewConvoResult {
+            error_code: -99,
+            convo_id: "".into(),
+            payloads: safer_ffi::Vec::EMPTY,
+        };
+        create_new_private_convo(&mut saro, raya_bundle, content.into(), &mut convo_result);
         assert!(is_ok(convo_result.error_code));
 
         // Raya recieves initial message
         let payload = convo_result.payloads.first().unwrap();
 
-        let handle_result = handle_payload(&mut raya, payload.data.as_ref());
+        let mut handle_result: HandlePayloadResult = HandlePayloadResult {
+            error_code: -99,
+            convo_id: "".into(),
+            content: safer_ffi::Vec::EMPTY,
+            is_new_convo: false,
+        };
+        handle_payload(&mut raya, payload.data.as_ref(), &mut handle_result);
         assert!(is_ok(handle_result.error_code));
 
         // Check that the Content sent was the content received

nim-bindings/conversations_example.nimble

@@ -13,9 +13,14 @@ bin           = @["libchat"]
 requires "nim >= 2.2.4"
 requires "results"
 
-# Build Rust library before compiling Nim
-before build:
+proc buildRust() =
   exec "cargo build --release --manifest-path ../Cargo.toml"
 
+
+# Build Rust library before compiling Nim
+before build:
+  buildRust()
+
 task pingpong, "Run pingpong example":
-  exec "nim c -r --path:src examples/pingpong.nim"
+  buildRust()
+  exec "nim c -r --path:src --passL:../target/release/liblibchat.a --passL:-lm examples/pingpong.nim"

nim-bindings/src/bindings.nim

@@ -1,29 +1,5 @@
 # Nim FFI bindings for libchat conversations library
 
-import std/[os]
-
-# Dynamic library path resolution
-# Can be overridden at compile time with -d:CONVERSATIONS_LIB:"path/to/lib"
-# Or at runtime via LIBCHAT_LIB environment variable
-when defined(macosx):
-  const DEFAULT_LIB_NAME = "liblibchat.dylib"
-elif defined(linux):
-  const DEFAULT_LIB_NAME = "liblibchat.so"
-elif defined(windows):
-  const DEFAULT_LIB_NAME = "libchat.dll"
-else:
-  const DEFAULT_LIB_NAME = "libchat"
-
-# Try to find the library relative to the source file location at compile time
-const
-  thisDir = currentSourcePath().parentDir()
-  projectRoot = thisDir.parentDir().parentDir()
-  releaseLibPath = projectRoot / "target" / "release" / DEFAULT_LIB_NAME
-  debugLibPath = projectRoot / "target" / "debug" / DEFAULT_LIB_NAME
-
-# Default to release path, can be overridden with -d:CONVERSATIONS_LIB:"..."
-const CONVERSATIONS_LIB* {.strdefine.} = releaseLibPath
-
 # Error codes (must match Rust ErrorCode enum)
 const
   ErrNone* = 0'i32
@@ -97,23 +73,23 @@ type
 
 ## Creates a new libchat Context
 ## Returns: Opaque handle to the context. Must be freed with destroy_context()
-proc create_context*(name: ReprCString): ContextHandle {.importc, dynlib: CONVERSATIONS_LIB.}
+proc create_context*(name: ReprCString): ContextHandle {.importc.}
 
 ## Returns the friendly name of the context's identity
 ## The result must be freed by the caller (repr_c::String ownership transfers)
-proc installation_name*(ctx: ContextHandle): ReprCString {.importc, dynlib: CONVERSATIONS_LIB.}
+proc installation_name*(ctx: ContextHandle): ReprCString {.importc.}
 
 ## Destroys a context and frees its memory
 ## - handle must be a valid pointer from create_context()
 ## - handle must not be used after this call
-proc destroy_context*(ctx: ContextHandle) {.importc, dynlib: CONVERSATIONS_LIB.}
+proc destroy_context*(ctx: ContextHandle) {.importc.}
 
 ## Creates an intro bundle for sharing with other users
 ## Returns: CreateIntroResult struct - check error_code field (0 = success, negative = error)
 ## The result must be freed with destroy_intro_result()
 proc create_intro_bundle*(
   ctx: ContextHandle,
-): CreateIntroResult {.importc, dynlib: CONVERSATIONS_LIB.}
+): CreateIntroResult {.importc.}
 
 ## Creates a new private conversation
 ## Returns: NewConvoResult struct - check error_code field (0 = success, negative = error)
@@ -122,7 +98,7 @@ proc create_new_private_convo*(
   ctx: ContextHandle,
   bundle: SliceUint8,
   content: SliceUint8,
-): NewConvoResult {.importc, dynlib: CONVERSATIONS_LIB.}
+): NewConvoResult {.importc.}
 
 ## Sends content to an existing conversation
 ## Returns: SendContentResult struct - check error_code field (0 = success, negative = error)
@@ -131,7 +107,7 @@ proc send_content*(
   ctx: ContextHandle,
   convo_id: ReprCString,
   content: SliceUint8,
-): SendContentResult {.importc, dynlib: CONVERSATIONS_LIB.}
+): SendContentResult {.importc.}
 
 ## Handles an incoming payload
 ## Returns: HandlePayloadResult struct - check error_code field (0 = success, negative = error)
@@ -141,19 +117,19 @@ proc send_content*(
 proc handle_payload*(
   ctx: ContextHandle,
   payload: SliceUint8,
-): HandlePayloadResult {.importc, dynlib: CONVERSATIONS_LIB.}
+): HandlePayloadResult {.importc.}
 
 ## Free the result from create_intro_bundle
-proc destroy_intro_result*(result: CreateIntroResult) {.importc, dynlib: CONVERSATIONS_LIB.}
+proc destroy_intro_result*(result: CreateIntroResult) {.importc.}
 
 ## Free the result from create_new_private_convo
-proc destroy_convo_result*(result: NewConvoResult) {.importc, dynlib: CONVERSATIONS_LIB.}
+proc destroy_convo_result*(result: NewConvoResult) {.importc.}
 
 ## Free the result from send_content
-proc destroy_send_content_result*(result: SendContentResult) {.importc, dynlib: CONVERSATIONS_LIB.}
+proc destroy_send_content_result*(result: SendContentResult) {.importc.}
 
 ## Free the result from handle_payload
-proc destroy_handle_payload_result*(result: HandlePayloadResult) {.importc, dynlib: CONVERSATIONS_LIB.}
+proc destroy_handle_payload_result*(result: HandlePayloadResult) {.importc.}
 
 # ============================================================================
 # Helper functions

nim-bindings/src/libchat.nim

@@ -44,11 +44,10 @@ proc createIntroductionBundle*(ctx: LibChat): Result[seq[byte], string] =
     return err("Context handle is nil")
 
   let res = create_intro_bundle(ctx.handle)
+  defer: destroy_intro_result(res)
 
   if res.error_code != ErrNone:
-    result = err("Failed to create private convo: " & $res.error_code)
-    destroy_intro_result(res)
-    return
+    return err("Failed to create intro bundle: " & $res.error_code)
 
   return ok(res.intro_bytes.toSeq())
 
@@ -62,32 +61,18 @@ proc createNewPrivateConvo*(ctx: LibChat, bundle: seq[byte], content: seq[byte])
   if content.len == 0:
     return err("content is zero length")
 
-  let res = bindings.create_new_private_convo(
-    ctx.handle,
-    bundle.toSlice(),
-    content.toSlice()
-  )
+  let res = bindings.create_new_private_convo(ctx.handle, bundle.toSlice(), content.toSlice())
+  defer: destroy_convo_result(res)
 
   if res.error_code != 0:
-    result = err("Failed to create private convo: " & $res.error_code)
-    destroy_convo_result(res)
-    return
+    return err("Failed to create private convo: " & $res.error_code)
 
-  # Convert payloads to Nim types
   var payloads = newSeq[PayloadResult](res.payloads.len)
   for i in 0 ..< res.payloads.len:
     let p = res.payloads[int(i)]
-    payloads[int(i)] = PayloadResult(
-      address: $p.address,
-      data: p.data.toSeq()
-    )
+    payloads[int(i)] = PayloadResult(address: $p.address, data: p.data.toSeq())
 
-  let convoId = $res.convo_id
-
-  # Free the result
-  destroy_convo_result(res)
-
-  return ok((convoId, payloads))
+  return ok(($res.convo_id, payloads))
 
 ## Send content to an existing conversation
 proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq[PayloadResult], string] =
@@ -97,24 +82,13 @@ proc sendContent*(ctx: LibChat, convoId: string, content: seq[byte]): Result[seq
   if content.len == 0:
     return err("content is zero length")
 
-  let res = bindings.send_content(
-    ctx.handle,
-    convoId.toReprCString,
-    content.toSlice()
-  )
+  let res = bindings.send_content(ctx.handle, convoId.toReprCString, content.toSlice())
+  defer: destroy_send_content_result(res)
 
   if res.error_code != 0:
-    result = err("Failed to send content: " & $res.error_code)
-    destroy_send_content_result(res)
-    return
+    return err("Failed to send content: " & $res.error_code)
 
-
-  let payloads = res.payloads.toSeq().mapIt(PayloadResult(
-    address: $it.address,
-    data: it.data.toSeq()
-  ))
-
-  destroy_send_content_result(res)
+  let payloads = res.payloads.toSeq().mapIt(PayloadResult(address: $it.address, data: it.data.toSeq()))
   return ok(payloads)
 
 type
@@ -131,14 +105,8 @@ proc handlePayload*(ctx: LibChat, payload: seq[byte]): Result[Option[ContentResu
   if payload.len == 0:
     return err("payload is zero length")
 
-  var conversationIdBuf = newSeq[byte](ctx.buffer_size)
-  var contentBuf = newSeq[byte](ctx.buffer_size)
-  var conversationIdLen: uint32 = 0
-
-  let res = bindings.handle_payload(
-    ctx.handle,
-    payload.toSlice(),
-  )
+  let res = bindings.handle_payload(ctx.handle, payload.toSlice())
+  defer: destroy_handle_payload_result(res)
 
   if res.error_code != ErrNone:
     return err("Failed to handle payload: " & $res.error_code)