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.

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

Research how to simplify the connection between JS and C++ code (considering both ios and android) #37

Open
CMCDragonkai opened this issue Jun 26, 2022 · 3 comments
Labels
development Standard development r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms research Requires research

Comments

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jun 26, 2022

Specification

The work from #19 introduced rocksdb, and inherited the architecture from leveldown and rocksdb.

This means that we have roughly 3 layers of abstraction:

  1. The JS side has classes like DB.ts, DBTransaction.ts and DBIterator.ts - this is what is used by the end user of this library
  2. The C++ NAPI binding - this makes use of napi-macros, but doesn't use the node-addon-api framework. This creates a flat list of functions exposed from index.cpp, and these functions manipulate classes like Database, Transaction, these classes embed logic involving the napi system in nodejs, the code here is not portable to the non-node environment
  3. The RocksDB C++ code, this is what actually does all the database work, it just lives in its own environment and exposes all of its functionality over C headers. We compile this from a submodule using node-gyp, and all of its objects are encapsulated in classes in our C++ NAPI binding

There's a couple things here that aren't as robust as they can be:

  1. Instead of using the haphazard napi-macros, let's just use node-addon-api, it seems like a much more comprehensive framework for integrating into nodejs
  2. Lots of places where an asynchronous error/exception may occur is not being checked for, and would result in invisible segfaults or undefined behaviour
  3. Memory management and garbage collection seems duplicated, there's logic on the JS side to manage their objects, and similar objects on the C++ NAPI side to do the same again, this seems repeated
  4. The lack of C++ exceptions makes some interactions awkward, I'd like to use C++ exceptions and connect them to JS exceptions
  5. The ability to use some JS constructs in our NAPI code would be useful, but this may impact portability of the C++ NAPI code, it's already not portable to environments where node wouldn't be available like react native bindings, but we need to be aware of what can be done in android & ios

I have some ideas to make the C++ classes behave more like JS classes, and to use the napi bindings to connect them together. Then instead of exposing raw functions, we just extend the native class constructors with the JS classes. However at the same time, keeping the interface simple might simplify the expectations the JS side has of the C++ side, so that when android/ios work is involved, and napi conveniences are not available, then we have to consider if there's an additional bridge layer like Java or Swift/Obj-C that sits in-between the JS and C++ code.

Additional context

Tasks

  1. ...
  2. ...
  3. ...
@CMCDragonkai CMCDragonkai added the development Standard development label Jun 26, 2022
@CMCDragonkai CMCDragonkai changed the title Simplify how the JS and C++ code is connected together Research how to simplify the connection between JS and C++ code (considering both ios and android) Jun 26, 2022
@CMCDragonkai CMCDragonkai added the research Requires research label Jun 26, 2022
@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Jul 14, 2022

Should investigate the usage of node-addon-api before tackling #5 or #1 as they would involve more C/C++ code. In fact #5 would be prime candidate for migrating to using node-addon-api.

And also the integration of ccache #44 as it would be quite slow to compile.

@teebirdy teebirdy added r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management and removed r&d:polykey:core activity 1 Secret Vault Sharing and Secret History Management labels Jul 24, 2022
@CMCDragonkai CMCDragonkai added the r&d:polykey:supporting activity Supporting core activity label Jul 10, 2023
@CMCDragonkai CMCDragonkai self-assigned this Jul 10, 2023
@CMCDragonkai
Copy link
Member Author

CMCDragonkai commented Jul 10, 2023

The rust way of doing things is a far simpler. And NAPI-RS does alot of good things. If we switch to using the rust's rocksdb, we might get around some of the problems.

Of course one has to be aware of the block encryption requirement. Right now no mention of that API support. I created an upstream issue rust-rocksdb/rust-rocksdb#802

Also see:
https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20171220_encryption_at_rest.md

@CMCDragonkai CMCDragonkai added r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms and removed r&d:polykey:supporting activity Supporting core activity labels Jul 10, 2023
@CMCDragonkai
Copy link
Member Author

I have a feeling that delegating the lifecycle and resource management to JS side is the right thing, and instead the C++ code shouldn't be using classes at all, but instead exposing underlying state possibly through opaque pointers that can be managed manually be JS. This does require a bit of rearchitecture.

@CMCDragonkai CMCDragonkai removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms research Requires research
Development

No branches or pull requests

2 participants