Skip to content

chore(cbind): nim-ffi migration [4/9] — streams & custom protocols (+ echo example)#2775

Merged
gmelodie merged 4 commits into
masterfrom
chore/cbind/ffi/04-streams
Jul 13, 2026
Merged

chore(cbind): nim-ffi migration [4/9] — streams & custom protocols (+ echo example)#2775
gmelodie merged 4 commits into
masterfrom
chore/cbind/ffi/04-streams

Conversation

@gmelodie

@gmelodie gmelodie commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

nim-ffi cbind migration — PR train

  1. chore(cbind): nim-ffi migration [1/9] — deps + pinning scaffold #2772 — deps + pinning
  2. chore(cbind): nim-ffi migration [2/9] — new-library scaffold + CI #2773 — scaffold + CI
  3. chore(cbind): nim-ffi migration [3/9] — connectivity & identity #2774 — connectivity
  4. chore(cbind): nim-ffi migration [4/9] — streams & custom protocols (+ echo example) #2775 — streams (+ echo) ← this PR
  5. chore(cbind): nim-ffi migration [5/9] — pubsub / gossipsub (+ gossipsub example) #2776 — pubsub (+ gossipsub)
  6. chore(cbind): nim-ffi migration [6/9] — kademlia + CID + crypto #2777 — kad + CID + crypto
  7. chore(cbind): nim-ffi migration [7/9] — service discovery + extended peer records #2778 — service discovery
  8. chore(cbind): nim-ffi migration [8/9] — relay, peerstore, metrics (API parity) #2779 — relay / peerstore / metrics
  9. chore(cbind): nim-ffi migration [9/9] — flip to libp2p.nim, delete legacy cbind #2780 — flip + delete legacy

Part 4 of a 9-PR train that replaces the hand-rolled cbind/ C bindings with the generated nim-ffi codegen, landing it domain by domain so each PR stays reviewable and CI-green (build the FFI lib + generate bindings + build/run examples).

Targets chore/cbind/ffi/03-connectivity (stacked).

This PR adds

  • Stream ops over streamId handles: readExactly, readLp, write, writeLp, close, closeWithEof, release.
  • libp2pMountProtocol + the onIncomingStream {.ffiEvent.}, plus the StreamRegistry release-waiter logic that keeps custom-protocol handlers alive across the FFI boundary until the host releases the stream.
  • validateReadLength hardening — rejects negative lengths and anything past the MaxReadBytes (64 MiB) DoS ceiling before it sizes an allocation.
  • The echo C example plus its build harness: common.h, CMakeLists.txt, README.md, updated Makefile, and the examples nimble task.

Lifecycle procs now return bool

libp2pStart, libp2pStop, libp2pConnect, and libp2pDisconnect return Result[bool, string] instead of Result[void, string]. This is the first PR that compiles libp2p_ffi.nim in the always-on c-bindings job, and nim-ffi's async dispatch can't marshal a void success — its generated handler lambda unwraps the result value, which doesn't exist for void. A trivial bool gives every lifecycle call a uniform on_bool reply, which the echo example already consumes.

Also

  • Bumps pinned nim-ffi to 7ef58f4 (latest master): picks up compile-time --nimMainPrefix validation (Fix/misc #116), the loud scalar-fast-path binding drop (Less confusing init patterns #120), and the shorter genbindings incantation (Disable new fragmentation tests #119). All are satisfied by the current build flags (--nimMainPrefix:liblibp2p) and the default CBOR ABI, so no binding source change was needed.
  • Points buildffi/genbindings/examples at the installed ffi and cbor_serialization packages via --path. The c-bindings job runs under nimble's project-local deps mode, and those two packages aren't cbind requires (not in the registry), so nimble setup leaves them off nimble.paths. Their transitive deps are libp2p deps already on the inherited root paths.

Review artifact: the generated header diff shows the stream/protocol entry points and the incoming-stream event.

@codecov-commenter

codecov-commenter commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.06%. Comparing base (de4cbb1) to head (b05e2b0).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2775      +/-   ##
==========================================
- Coverage   81.07%   81.06%   -0.02%     
==========================================
  Files         171      171              
  Lines       31135    31135              
  Branches       12       13       +1     
==========================================
- Hits        25244    25240       -4     
- Misses       5891     5895       +4     

see 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmelodie gmelodie mentioned this pull request Jul 7, 2026
2 tasks
@github-project-automation github-project-automation Bot moved this from new to In Progress in nim-libp2p Jul 7, 2026
@gmelodie
gmelodie force-pushed the chore/cbind/ffi/03-connectivity branch 3 times, most recently from 543a063 to 021033a Compare July 8, 2026 20:35
@gmelodie
gmelodie force-pushed the chore/cbind/ffi/04-streams branch 3 times, most recently from f5eeca5 to 5e8e73c Compare July 10, 2026 19:16
Base automatically changed from chore/cbind/ffi/03-connectivity to master July 10, 2026 23:34
@gmelodie
gmelodie force-pushed the chore/cbind/ffi/04-streams branch 5 times, most recently from eba4827 to 4e4d51f Compare July 13, 2026 16:15
@gmelodie
gmelodie force-pushed the chore/cbind/ffi/04-streams branch from 4e4d51f to a8a2b57 Compare July 13, 2026 16:40
Comment thread cbind/examples/echo.c
Comment on lines -172 to -173

static void event_handler(int callerRet, const char *msg, size_t len,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These and lines below got moved to common.h

@gmelodie
gmelodie marked this pull request as ready for review July 13, 2026 16:43
@gmelodie
gmelodie requested review from a team and richard-ramos July 13, 2026 16:43
@gmelodie
gmelodie enabled auto-merge July 13, 2026 17:54
Comment thread cbind/libp2p_ffi.nim
streams.release(streamId)
let releaseWaiter =
Future[void].Raising([CancelledError]).init("cbind custom protocol release")
streams.releaseWaiters[streamId] = releaseWaiter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do a stream_release on all releaseWaiters in libp2pStop / destroy?

@gmelodie
gmelodie added this pull request to the merge queue Jul 13, 2026
Merged via the queue into master with commit 3a59e0e Jul 13, 2026
37 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to done in nim-libp2p Jul 13, 2026
@gmelodie
gmelodie deleted the chore/cbind/ffi/04-streams branch July 13, 2026 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: done

Development

Successfully merging this pull request may close these issues.

4 participants