Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: Libwaku tweaks #3233

Merged
merged 5 commits into from
Jan 8, 2025
Merged

chore: Libwaku tweaks #3233

merged 5 commits into from
Jan 8, 2025

Conversation

Ivansete-status
Copy link
Collaborator

@Ivansete-status Ivansete-status commented Jan 8, 2025 •
edited
Loading

Description

  • Avoid the use of string type in libwaku. In fact, we should avoid any GC'ed type (string, ref, closure, seq) when performing ffi operations. Instead, we can only allocate/deallocate memory manually
  • Initialize default cstring params with nil instead of an empty string ("")
  • library/alloc.nim: reserve an empty cstring when nil is passed to alloc proc
  • lightpush_request.nim, self_req_handler.nim: return a msg_hash when successful publish
  • library/waku_thread/inter_thread_communication/requests/protocols/store_request.nim: rm extra deallocShared

Issue

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 2025 •
edited
Loading

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3233

Built from aab7480

@Ivansete-status Ivansete-status marked this pull request as ready for review January 8, 2025 17:38
@gabrielmer gabrielmer self-requested a review January 8, 2025 17:45
gabrielmer
gabrielmer approved these changes Jan 8, 2025
View reviewed changes
Copy link
Contributor

@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 amazing! Thanks so much! 🔥

richard-ramos
richard-ramos approved these changes Jan 8, 2025
View reviewed changes
Copy link
Member

@richard-ramos richard-ramos 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 Ivansete-status merged commit 625c8ee into master Jan 8, 2025
10 of 11 checks passed
@Ivansete-status Ivansete-status deleted the libwaku-tweaks branch January 8, 2025 19:52
@Ivansete-status Ivansete-status mentioned this pull request Jan 10, 2025
Open
Ivansete-status added a commit that referenced this pull request Jan 24, 2025
@Ivansete-status
chore: libwaku tweaks (#3233)
2ae5595 
* make lightpush return msg hash after successful publish
* libwaku avoid the use of string
* library alloc.nim allocate memory when nil cstring is passed
* libwaku store_request remove extra destroyShared(self)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richard-ramos richard-ramos richard-ramos approved these changes

@gabrielmer gabrielmer gabrielmer approved these changes

Assignees

@Ivansete-status Ivansete-status

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ivansete-status @richard-ramos @gabrielmer