feat: add c binding #1322

2-towns · 2025-09-09T18:36:21Z

Just opening a draft PR to get early reviews

gmega · 2025-09-09T19:29:25Z

library/codex_context.nim

+proc sendRequestToCodexThread*(
+    ctx: ptr CodexContext,
+    reqType: RequestType,
+    reqContent: pointer,


Hm... how will this work when we have to upload data to Codex? I suppose reqContent is meant to be a string or byte array (and not, say, a file descriptor), so we will probably have to constrain loading to happen from files and pass a local path here? Also wondering what'll be the best approach for a download API...

So, I didn't think deeply to that yet, but maybe we could have some kind of steam mode where the reqContent is a Stream or Reader, so the worker will read small chunks of the files until it is fully uploaded.
And for the download, opposite direction with Writer.

I am not sure of how to do it with Nim + C binding, but in Javascript I would try to consume the data in that way.

reqContent is more for describing the codex-local operation (eg start node).
userData I believe is what should be used for passing data. It is a pointer,
which could point to a file path or a seq of bytes (assuming it is not copied).

reqContent is more for describing the codex-local operation (eg start node). userData I believe is what should be used for passing data. It is a pointer, which could point to a file path or a seq of bytes (assuming it is not copied).

If you look at sendRequestToCodexThread, the third parameter is the reqContent, which is in the case of Lifecycle request :

NodeLifecycleRequest.createShared( NodeLifecycleMsgType.CREATE_NODE, configJson ),

It creates a request shared object that contains the json config provided by the Go application. So I think the reqContent contains the actual data passed by the caller.

The userData is a bit confusing. It looks like a context that will be used when the Go app receives the callback. It is not intended to be used by the Nim code, but by the Go code when the callback is invoked.
You can have a global userData context plus a userData context per call. I’m not sure how useful this is for Codex right now.

gmega · 2025-09-09T19:32:59Z

library/codex_context.nim

+  # between threads assumes that there aren't concurrent requests.
+  # Rearchitecting the signaling + migrating to a MP Channel will allow us to receive
+  # requests concurrently and spare us the need of locks
+  lock: Lock


Not sure this is a big deal as requests run async and the actual sending of the request to the Codex thread should be relatively fast, plus this shouldn't be a very "hot" API anyway?

Yes. The only issue arises if you have multiple application threads, meaning multiple calls to sendRequestToCodexThread. In that case, the request channel may receive several requests, but currently the worker thread only processes one request per wakeup and sends an ACK to the first application thread:

# Pop a request from the channel let recvOk = ctx.reqChannel.tryRecv(request) if not recvOk: error "codex thread could not receive a request" continue

So we could be in a situation where the second application thread could end up waiting indefinitely for an ACK from the Codex thread.

gmega · 2025-09-09T19:44:44Z

library/codex_thread_requests/codex_thread_request.nim

+    of LIFECYCLE:
+      cast[ptr NodeLifecycleRequest](request[].reqContent).process(codex)
+
+  handleRes(await retFut, request)


Hmmm... does this mean we are calling back the C code directly from the Chronos thread?

To summarize the discussion during the meeting:

The callbacks are currently executed on the Chronos thread (= working thread). This can be an issue if a callback is slow, because the Chronos thread will block and other requests cannot be processed.

One solution would be to introduce a dispatcher thread with a queue for events and a dedicated thread that dequeues them and runs the callbacks.

Another potential approach would be to move to a Multi Producer channel instead of the current Single Producer channel. This would allow multiple threads to enqueue requests concurrently, so even if one thread is blocked by a callback, others can continue processing requests.

gmega · 2025-09-09T19:46:03Z

library/codex_thread_requests/codex_thread_request.nim

+    deallocShared(request)
+
+  if res.isErr():
+    foreignThreadGc:


Why do you need this? Isn't handleRes being called from the Chronos thread?

Well, I’m not sure that’s always the case, especially when a Go routine is involved.

I mean from what I understood, Go routines will run threads that are considered as foreign by Nim so that my be an issue with the GC.

emizzle

Definitely looks like it's going in the right direction! I tried it out locally and it worked well!

Just a few suggestions/small comments even though I know this is a draft. I can imagine you were saving exception handling for "clean up". Best for last right 😂

codex/contracts/clock.nim

codex/conf.nim

codex/codex.nim

codex/conf.nim

emizzle · 2025-09-10T04:59:10Z

codex/conf.nim

-  let count = parseInt(input)
-  if count != 0 and count < 2:
-    warn "Invalid number of threads", input = input
+proc parse*(T: type ThreadCount, p: string): Result[ThreadCount, string] =


I'd probably pull all these out into a parser module, so that you can import the parser module from conf and egnode_lifecycle_request. This will prevent the need to import conf from node_lifecycle_request.

you will still need to import CodexConf from conf. But yeah I can import only CodexConf and move the parsing method into a parser.nim as you suggested.

emizzle · 2025-09-10T05:11:21Z

codex.nim

+  try:
+    updateLogLevel(config.logLevel)
+  except ValueError as err:
+    try:
+      stderr.write "Invalid value for --log-level. " & err.msg & "\n"
+    except IOError:
+      echo "Invalid value for --log-level. " & err.msg
+    quit QuitFailure


Isn't this already called in setupLogging?

I removed it from the setupLogging because the compiler was complaining about gcsafe issue.

build.nims

library/libcodex.nim

emizzle · 2025-09-10T05:41:29Z

library/codex_context.nim

+      cast[CodexCallback](ctx[].eventCallback)(
+        RET_OK, unsafeAddr event[0], cast[csize_t](len(event)), ctx[].eventUserData
+      )
+    except Exception, CatchableError:


I'm assuming that Exception had to be caught to get this to compile? Could be that there is no way to avoid Defects being raised in the call path.

Well, I am not sure this template is not called yet :). But I will remove the Exception for now and add it back if it is needed.

emizzle · 2025-09-10T09:40:49Z

library/codex_thread_requests/requests/node_lifecycle_request.nim

+proc readValue*(r: var JsonReader, val: var MultiAddress) =
+  val = MultiAddress.init(r.readValue(string)).get()
+
+proc readValue*(r: var JsonReader, val: var NatConfig) =


It feels like these deserialisations are actually quite generic and should maybe live elsewhere. In ethers, we redirect all de/serializations to nim-serde instead:

import pkg/serde/json ## Generic conversions to use nim-serde instead of nim-json-serialization for ## json rpc serialization purposes ## writeValue => `%` ## readValue => fromJson proc writeValue*[T: not JsonNode]( writer: var JsonWriter[JrpcConv], value: T) {.raises:[IOError].} = writer.writeValue(%value) proc readValue*[T: not JsonNode]( r: var JsonReader[JrpcConv], result: var T) {.raises: [SerializationError, IOError].} = var json = r.readValue(JsonNode) result = T.fromJson(json).getOrRaise(SerializationError)

library/libcodex.nim

emizzle · 2025-09-11T01:51:52Z

library/codex_context.nim

+proc sendRequestToCodexThread*(
+    ctx: ptr CodexContext,
+    reqType: RequestType,
+    reqContent: pointer,


reqContent is more for describing the codex-local operation (eg start node).
userData I believe is what should be used for passing data. It is a pointer,
which could point to a file path or a seq of bytes (assuming it is not copied).

library/codex_thread_requests/codex_thread_request.nim

library/libcodex.nim

emizzle · 2025-09-11T03:15:16Z

library/codex_context.nim

+  ## process proc. See the 'codex_thread_request.nim' module for more details.
+  ok()
+
+proc runCodex(ctx: ptr CodexContext) {.async.} =


Probably should also not raise errors inside this proc otherwise it may silently
crash the thread?

…he Go code

emizzle · 2025-09-22T07:55:23Z

library/libcodex.nim

+  return callback.success("", userData)
+
+proc codex_upload_init(
+    ctx: ptr CodexContext, filepath: cstring, callback: CodexCallback, userData: pointer


What happens with the filepath if you're uploading via chunks? Is it not used?

Well, the filepath can be obviously the absolute file path when you upload a file.
But when you upload with chunks, it is used as filename. Then the node.store will use it for the filename and the mimetype.

…ze and the last chunk

…e logs

…egfault

…am running

2-towns added 5 commits September 9, 2025 20:30

Init library

dba938d

Define pragma raises

1ea8e0b

Move the updateLogLevel in the main file in order to avoid gcsafe error

11c868c

Catch exception when shutting down taskpool

c6b2578

Refactor the config in order to share parse functions with json parse

869759b

2-towns changed the title ~~feature: add c binding~~ feat: add c binding Sep 9, 2025

2-towns changed the title ~~feat: add c binding~~ feature: add c binding Sep 9, 2025

2-towns requested review from gmega and emizzle September 9, 2025 18:38

gmega reviewed Sep 9, 2025

View reviewed changes

emizzle reviewed Sep 10, 2025

View reviewed changes

emizzle reviewed Sep 11, 2025

View reviewed changes

2-towns added 5 commits September 11, 2025 08:22

Set dynamic build as default

75803af

Log fatal when command line argument cannot be parsed

e5fd789

Define raises pragma for the whole file

3bb2bd0

Avoid raising errors in the spawn threads

fcdb0df

Simplify a bit by removing handleRequest func

f28f6a3

2-towns mentioned this pull request Sep 11, 2025

Implement a Codex library so that clients can use Codex directly from their apps #1220

Open

2-towns added 9 commits September 11, 2025 14:55

Add limitation doc

6f9557b

Use await keyword as soon as possible to yield to the event event loop

820d535

Replace version git command to get the last tag

e1f6977

Return the Codex version

c9670e8

Add codex revision

fc5d069

Provide a better API with ACK and OK differenciation

8fe29c6

Add repo and fix pointer freeing memory

aa2128d

Remove ACK and CODEX_CALL in order to return the first RET value in t…

e9da685

…he Go code

Add async method with callabck for Codex start

2d578f9

emizzle reviewed Sep 22, 2025

View reviewed changes

2-towns added 9 commits September 22, 2025 15:33

Add progress callback for upload

378dbfe

Cap the percent to 100 because of the difference between the block si…

c01c913

…ze and the last chunk

Use cancelSoon instead of cancel

5d56a45

Add async version for file upload

d9cb745

Use faststream, provide better cleanup and comments

1cd6a94

Provide better comments

4f40be4

Add more comment

3333e91

Provide better api for progress, fix memory pointer issues and improv…

9a86c48

…e logs

Provide better logs (hopefully)

e6330e2

2-towns linked an issue Sep 25, 2025 that may be closed by this pull request

Implement a Codex library so that clients can use Codex directly from their apps #1220

Open

2-towns added 6 commits September 29, 2025 15:17

Rename OnBlockStored

3535b21

Remove dead code

56cf2d4

Refactore upload for better memory management with thread and avoid s…

792ba0e

…egfault

Refactor to avoid memory issues and add download local

56b01d5

Add download streaming mode

8bee7ec

Add comments

421c88b

2-towns changed the title ~~feature: add c binding~~ feat: add c binding Sep 30, 2025

2-towns added 5 commits October 1, 2025 06:10

Add manifest

f883bba

Add storage list

f9aa69a

Add node storage fetch (network download to local node)

e4d1d22

Add node space

007f92c

Add storage delete (block or dataset delete)

74e7f08

github-actions bot added the feat label Oct 1, 2025

2-towns added 6 commits October 2, 2025 08:28

Define raises for async pragma for waitUntil

ffae046

Cancel the download in the example to avoid to keep a future and stre…

1d24d1a

…am running

Fix log message

eb562bc

Fix mock clock

db5109a

Make rest api server optional

40cc019

Reraise cancelled error

fe7e9b3

feat: add c binding #1322

Are you sure you want to change the base?

feat: add c binding #1322

Uh oh!

Conversation

2-towns commented Sep 9, 2025

Uh oh!

gmega Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

2-towns Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmega Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmega Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emizzle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gmega Sep 9, 2025 •

edited

Loading

2-towns Sep 11, 2025 •

edited

Loading

gmega Sep 9, 2025 •

edited

Loading

gmega Sep 9, 2025 •

edited

Loading