Skip to content

Add nim-ffi suggestion#325

Merged
fryorcraken merged 5 commits intomasterfrom
suggest-new-nim-ffi
Aug 4, 2025
Merged

Add nim-ffi suggestion#325
fryorcraken merged 5 commits intomasterfrom
suggest-new-nim-ffi

Conversation

@Ivansete-status
Copy link
Contributor

This PR adds a new deliverable: Create nim-ffi

The main purpose of this deliverable is to have a single source of truth for ffi-nim-based work and we aim to create a general-purpose repository that will help other Nim developers to expose their projects to other environments

Copy link

@gabrielmer gabrielmer left a comment

Choose a reason for hiding this comment

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

Looks good :)

Copy link

@NagyZoltanPeter NagyZoltanPeter left a comment

Choose a reason for hiding this comment

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

I have the feeling that Streamline FFI API... belongs to this new point nim-ffi.
WDYT?
Why No FURPS? Yet? Shall we add?

- [ ] Docs: links to README.md or docs.waku.org (TBD)


### Create new nim-ffi repository that will allow exposing any Nim library in an easy way

Choose a reason for hiding this comment

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

Can't we merge the previous point ### Streamline FFI API Creation by using Protobuf types instead of JSON PoC with this nim-ffi?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave it to you. Wouldhte first version of nim-ffi be a copy paste of json usage? or do you want to go directly for protobuf?

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, I have noticed that JSON generator is not used because we used default of WakuNodeConf. I intend to correct that,see logos-messaging/logos-delivery#3490

Copy link
Contributor

Choose a reason for hiding this comment

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

I think fine to keep it separate for now. even if both end up in same code base.

@fryorcraken
Copy link
Contributor

Why No FURPS? Yet? Shall we add?

Yes FURPS please.

- [ ] Docs: links to README.md or docs.waku.org (TBD)


### Create new nim-ffi repository that will allow exposing any Nim library in an easy way
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll leave it to you. Wouldhte first version of nim-ffi be a copy paste of json usage? or do you want to go directly for protobuf?

- [ ] Docs: links to README.md or docs.waku.org (TBD)


### Create new nim-ffi repository that will allow exposing any Nim library in an easy way
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, I have noticed that JSON generator is not used because we used default of WakuNodeConf. I intend to correct that,see logos-messaging/logos-delivery#3490

Comment on lines +84 to +87
- [Waku SDK](/FURPS/core/waku_sdk.md)
- [RLN SDK](/FURPS/core/rln_sdk.md)
- [Chat SDK](/FURPS/application/chat_sdk.md)
- [SDS](/FURPS/application/sds.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to create a new feature here which would be Nim FFI and write FURPS for it.

In the + section of the FURPS, you can add that it is used by Chat, RLN, etc.

- [Chat SDK](/FURPS/application/chat_sdk.md)
- [SDS](/FURPS/application/sds.md)

**No FURPS**:
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes FURPS

@Ivansete-status
Copy link
Contributor Author

Answering this comment #325 (comment)
Yes, the initial idea is to extract the current libwaku strategy to a more generic repo and from there, in upcoming iterations we can add protobuf support

Copy link
Contributor

@fryorcraken fryorcraken left a comment

Choose a reason for hiding this comment

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

@Ivansete-status I cleaned up the FURPS. looks good?

- [ ] Docs: links to README.md or docs.waku.org (TBD)


### Create new nim-ffi repository that will allow exposing any Nim library in an easy way
Copy link
Contributor

Choose a reason for hiding this comment

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

I think fine to keep it separate for now. even if both end up in same code base.

@fryorcraken
Copy link
Contributor

@Ivansete-status would that impact any of our other commitments?

@Ivansete-status
Copy link
Contributor Author

@Ivansete-status would that impact any of our other commitments?

@fryorcraken - this deliverable represents a big effort but it will be very profitable in the future because all the ffi maintenance will happen in one single repository.

I'll double-check the furps tomorrow

@Ivansete-status
Copy link
Contributor Author

@Ivansete-status I cleaned up the FURPS. looks good?

Thanks for it!
I just pushed some changes now

Comment on lines +6 to +8
2. The exposed C library can be used in Golang.
3. The exposed C library can be used in Rust.
4. The exposed C library can be used in Python.
Copy link
Contributor

Choose a reason for hiding this comment

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

These are Supportability items

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Addressed in 3005c54


## Reliability

1. The exposed C library does not leak memory.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really under nim-ffi control of hte underneath nim library leaks memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question! The underneath library may eventually leak memory but we need to make sure the nim-ffi doesn't leak either because in there, we will handle memory manually, i.e., we cannot use GC'ed types when interfacing between two threads and due to that there's a risk of leaking memory in that process.

Comment on lines +27 to +28
2. The interaction with the exposed C library is through JSON.
3. The interaction with the exposed C library is through protobuf.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are Usability, because it allows the dev to use Json or proto serializer libs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Addressed in 3005c54


## Supportability

1. Any Nim project can use it and can be installed using Nimble,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I would put th egolang/rust support.
The nimble support is more of a usability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Addressed in 3005c54

@fryorcraken fryorcraken merged commit 2fcbc2a into master Aug 4, 2025
@fryorcraken
Copy link
Contributor

New deliverable, please create it cc @chair28980

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.

4 participants