chore: address PR reviews

Bilb · Bilb · commit 15151d763599 · 2024-11-20T10:39:01.000+11:00
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -17,8 +17,6 @@ jobs:
 
     strategy:
       fail-fast: false
-
-
       matrix:
         os: [windows-2022, macos-12, ubuntu-20.04]
     env:
@@ -68,7 +66,6 @@ jobs:
           sed -i "s/target_compile_options(oxen-logging-warnings INTERFACE/#target_compile_options(oxen-logging-warnings INTERFACE/" libsession-util/external/oxen-libquic/external/oxen-logging/CMakeLists.txt
           cat libsession-util/external/oxen-libquic/external/oxen-logging/CMakeLists.txt
 
-
       - name: build libsession-util-nodejs
         shell: bash
-        run:  yarn install --frozen-lockfile
+        run: yarn install --frozen-lockfile
diff --git a/src/base_config.hpp b/src/base_config.hpp
@@ -106,19 +106,6 @@ class ConfigBaseImpl {
 
             Napi::Env env = info.Env();
 
-            // config->logger = [env, class_name](session::config::LogLevel, std::string_view x) {
-            //     std::string toLog =
-            //             "libsession-util:" + std::string(class_name) + ": " + std::string(x) +
-            //             "\n";
-
-            //     Napi::Function consoleLog = env.Global()
-            //                                         .Get("console")
-            //                                         .As<Napi::Object>()
-            //                                         .Get("log")
-            //                                         .As<Napi::Function>();
-            //     consoleLog.Call({Napi::String::New(env, toLog)});
-            // };
-
             return config;
         });
     }
diff --git a/src/blinding/blinding.hpp b/src/blinding/blinding.hpp
@@ -19,7 +19,7 @@ class BlindingWrapper : public Napi::ObjectWrap<BlindingWrapper> {
   public:
     BlindingWrapper(const Napi::CallbackInfo& info) : Napi::ObjectWrap<BlindingWrapper>{info} {
         throw std::invalid_argument(
-                "BlindingWrapper is all static and don't need to be constructed");
+                "BlindingWrapper is static and doesn't need to be constructed");
     }
 
     static void Init(Napi::Env env, Napi::Object exports) {
diff --git a/src/groups/meta_group_wrapper.cpp b/src/groups/meta_group_wrapper.cpp
@@ -20,7 +20,6 @@ void MetaGroupWrapper::Init(Napi::Env env, Napi::Object exports) {
             "MetaGroupWrapperNode",
             {
                     // shared exposed functions
-
                     InstanceMethod("needsPush", &MetaGroupWrapper::needsPush),
                     InstanceMethod("push", &MetaGroupWrapper::push),
                     InstanceMethod("needsDump", &MetaGroupWrapper::needsDump),
@@ -39,7 +38,6 @@ void MetaGroupWrapper::Init(Napi::Env env, Napi::Object exports) {
                     InstanceMethod("memberGetOrConstruct", &MetaGroupWrapper::memberGetOrConstruct),
                     InstanceMethod(
                             "memberConstructAndSet", &MetaGroupWrapper::memberConstructAndSet),
-
                     InstanceMethod("memberGetAll", &MetaGroupWrapper::memberGetAll),
                     InstanceMethod(
                             "memberGetAllPendingRemovals",
@@ -65,15 +63,12 @@ void MetaGroupWrapper::Init(Napi::Env env, Napi::Object exports) {
                     InstanceMethod("memberEraseAndRekey", &MetaGroupWrapper::memberEraseAndRekey),
 
                     // keys exposed functions
-
                     InstanceMethod("keysNeedsRekey", &MetaGroupWrapper::keysNeedsRekey),
                     InstanceMethod("keyRekey", &MetaGroupWrapper::keyRekey),
                     InstanceMethod("keyGetAll", &MetaGroupWrapper::keyGetAll),
-
                     InstanceMethod("currentHashes", &MetaGroupWrapper::currentHashes),
                     InstanceMethod("loadKeyMessage", &MetaGroupWrapper::loadKeyMessage),
                     InstanceMethod("keyGetCurrentGen", &MetaGroupWrapper::keyGetCurrentGen),
-
                     InstanceMethod("encryptMessages", &MetaGroupWrapper::encryptMessages),
                     InstanceMethod("decryptMessage", &MetaGroupWrapper::decryptMessage),
                     InstanceMethod("makeSwarmSubAccount", &MetaGroupWrapper::makeSwarmSubAccount),
@@ -82,7 +77,6 @@ void MetaGroupWrapper::Init(Napi::Env env, Napi::Object exports) {
                             "swarmVerifySubAccount", &MetaGroupWrapper::swarmVerifySubAccount),
                     InstanceMethod("loadAdminKeys", &MetaGroupWrapper::loadAdminKeys),
                     InstanceMethod("keysAdmin", &MetaGroupWrapper::keysAdmin),
-
                     InstanceMethod("swarmSubaccountSign", &MetaGroupWrapper::swarmSubaccountSign),
                     InstanceMethod(
                             "generateSupplementKeys", &MetaGroupWrapper::generateSupplementKeys),
@@ -538,7 +532,6 @@ void MetaGroupWrapper::memberSetPromotionAccepted(const Napi::CallbackInfo& info
 }
 
 void MetaGroupWrapper::memberSetProfilePicture(const Napi::CallbackInfo& info) {
-
     wrapExceptions(info, [&] {
         assertInfoLength(info, 2);
         assertIsString(info[0]);
diff --git a/src/groups/meta_group_wrapper.hpp b/src/groups/meta_group_wrapper.hpp
@@ -120,7 +120,6 @@ class MetaGroupWrapper : public Napi::ObjectWrap<MetaGroupWrapper> {
     void membersMarkPendingRemoval(const Napi::CallbackInfo& info);
 
     /** Keys Actions */
-
     Napi::Value keysNeedsRekey(const Napi::CallbackInfo& info);
     Napi::Value keyRekey(const Napi::CallbackInfo& info);
     Napi::Value keyGetAll(const Napi::CallbackInfo& info);
diff --git a/src/meta/meta_base_wrapper.hpp b/src/meta/meta_base_wrapper.hpp
@@ -68,7 +68,6 @@ class MetaBaseWrapper {
             std::optional<ustring_view> dumped_keys;
 
             if (dumped_meta) {
-
                 auto dumped_meta_str = from_unsigned_sv(*dumped_meta);
 
                 oxenc::bt_dict_consumer combined{dumped_meta_str};
diff --git a/src/multi_encrypt/multi_encrypt.hpp b/src/multi_encrypt/multi_encrypt.hpp
@@ -4,20 +4,19 @@
 
 #include <algorithm>
 
+#include "../utilities.hpp"
 #include "session/config/user_profile.hpp"
 #include "session/multi_encrypt.hpp"
 #include "session/random.hpp"
-#include "../utilities.hpp"
 
 namespace session::nodeapi {
 
 class MultiEncryptWrapper : public Napi::ObjectWrap<MultiEncryptWrapper> {
-
   public:
     MultiEncryptWrapper(const Napi::CallbackInfo& info) :
             Napi::ObjectWrap<MultiEncryptWrapper>{info} {
         throw std::invalid_argument(
-                "MultiEncryptWrapper is all static and don't need to be constructed");
+                "MultiEncryptWrapper is static and doesn't need to be constructed");
     }
 
     static void Init(Napi::Env env, Napi::Object exports) {
@@ -85,7 +84,7 @@ class MultiEncryptWrapper : public Napi::ObjectWrap<MultiEncryptWrapper> {
             std::vector<ustring_view> messages_sv(messages.begin(), messages.end());
             std::vector<ustring_view> recipients_sv(recipients.begin(), recipients.end());
 
-            // Note: this function needs the first 2 args to be vector of sv explicitely
+            // Note: this function needs the first 2 args to be vector of sv explicitly
             return session::encrypt_for_multiple_simple(
                     messages_sv, recipients_sv, ed25519SecretKey, domain, random_nonce);
         });
diff --git a/src/user_config.cpp b/src/user_config.cpp
@@ -9,7 +9,6 @@
 
 namespace session::nodeapi {
 
-// using config::LogLevel;
 using config::UserProfile;
 
 void UserConfigWrapper::Init(Napi::Env env, Napi::Object exports) {
diff --git a/src/utilities.hpp b/src/utilities.hpp
@@ -149,7 +149,6 @@ struct toJs_impl<std::unordered_set<T>> {
         auto arr = Napi::Array::New(env, as_array.size());
         for (size_t i = 0; i < as_array.size(); i++)
             arr[i] = toJs(env, as_array[i]);
-
         return arr;
     }
 };
diff --git a/types/groups/groupmembers.d.ts b/types/groups/groupmembers.d.ts
@@ -55,7 +55,7 @@ declare module 'libsession_util_nodejs' {
     // setters
     memberSetNameTruncated: (pubkeyHex: PubkeyType, newName: string) => void;
 
-    /** A member invite states defaults to invite-not-sent. Use this function to mark that you've sent one, or at least tried (failed: boolean)*/
+    /** A member's invite state defaults to invite-not-sent. Use this function to mark that you've sent one, or at least tried (failed: boolean)*/
     memberSetInvited: (pubkeyHex: PubkeyType, failed: boolean) => void;
     /** User has accepted an invitation and is now a regular member of the group */
     memberSetAccepted: (pubkeyHex: PubkeyType) => void;
diff --git a/types/groups/index.d.ts b/types/groups/index.d.ts
@@ -1,2 +1 @@
-/// <reference path="./groupinfo.d.ts" />
-/// <reference path="./metagroup.d.ts" />
+/// <reference path="./metagroup.d.ts" />
diff --git a/types/groups/metagroup.d.ts b/types/groups/metagroup.d.ts
@@ -7,9 +7,22 @@ declare module 'libsession_util_nodejs' {
   export type ConfirmKeysPush = [data: Uint8Array, hash: string, timestampMs: number];
 
   export type GroupWrapperConstructor = {
-    userEd25519Secretkey: Uint8Array; // (len 64)
-    groupEd25519Pubkey: Uint8Array; // the ed25519 pubkey without the 03 prefix  (len 32)
-    groupEd25519Secretkey: Uint8Array | null; // the ed25519 privkey if we have it (len 64) (comes from usergroup wrapper if we have it)
+    /**
+     * The user's ed25519 secret key, length 64.
+     */
+    userEd25519Secretkey: Uint8Array;
+    /**
+     *  The group ed25519 pubkey without the 03 prefix, length 32.
+     */
+    groupEd25519Pubkey: Uint8Array;
+    /**
+     * The group ed25519 priv key if we have it (len 64). Having this means we have admin rights in the group.
+     * This usually comes from the user group wrapper if we have it.
+     */
+    groupEd25519Secretkey: Uint8Array | null;
+    /**
+     * The unified dumps (as saved in the db) for this group. i.e. Keys, Members and Info concatenated, see MetaGroupWrapper::metaDump for details.
+     */
     metaDumped: Uint8Array | null;
   };
 
diff --git a/types/shared.d.ts b/types/shared.d.ts
@@ -6,7 +6,7 @@ declare module 'libsession_util_nodejs' {
   /**
    * Allow a single type to be Nullable. i.e. string => string | null
    */
-  type Nullable<T> = T | null;
+  export type Nullable<T> = T | null;
 
   /**
    * Allow all the fields of a type to be -themselves- nullable.
diff --git a/types/user/usergroups.d.ts b/types/user/usergroups.d.ts
@@ -36,17 +36,38 @@ declare module 'libsession_util_nodejs' {
     name: string; // human-readable; this should normally always be set, but in theory could be set to an empty string.
     encPubkey: Uint8ArrayLen32; // bytes (32 or empty)
     encSeckey: Uint8Array; // bytes (32 or empty)
-    // disappearingTimerSeconds: number; // in seconds, 0 == disabled.
     members: Array<LegacyGroupMemberInfo>;
   };
 
   type UserGroupsGet = BaseUserGroup & {
-    pubkeyHex: GroupPubkeyType; // The group "session id" (33 bytes), starting with 03.
-    secretKey: Uint8ArrayLen64 | null; // len 64
-    authData: Uint8ArrayLen100 | null; // len 100
+    /**
+     * The group "session id" (33 bytes), starting with 03.
+     */
+    pubkeyHex: GroupPubkeyType;
+    /**
+     * The group admin secret key if we have it, length 64.
+     */
+    secretKey: Uint8ArrayLen64 | null;
+    /**
+     * The group auth data we were given if we have it, length 100.
+     */
+    authData: Uint8ArrayLen100 | null;
+    /**
+     * The group name.
+     */
     name: string | null;
-    invitePending: boolean; // tracks `db.approved`. libsession allows this field for all groups (including communities, but we don't need it for more)
-    kicked: boolean; // Note: if the group was `destroyed` this will be false, but `destroyed` will be true
+    /**
+     * True if the invite is pending, i.e. we've received it but haven't approved or auto-approved it yet.
+     */
+    invitePending: boolean;
+    /**
+     * If we were kicked from that group, this will be true.
+     * Note: if the group was `destroyed` this will be false, but `destroyed` will be true
+     */
+    kicked: boolean;
+    /**
+     * If the group was destroyed, this will be true
+     */
     destroyed: boolean;
   };
 

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ class BlindingWrapper : public Napi::ObjectWrap<BlindingWrapper> {`
`19`	`19`	`public:`
`20`	`20`	`BlindingWrapper(const Napi::CallbackInfo& info) : Napi::ObjectWrap<BlindingWrapper>{info} {`
`21`	`21`	`throw std::invalid_argument(`
`22`		`- "BlindingWrapper is all static and don't need to be constructed");`
	`22`	`+ "BlindingWrapper is static and doesn't need to be constructed");`
`23`	`23`	`}`
`24`	`24`
`25`	`25`	`static void Init(Napi::Env env, Napi::Object exports) {`
Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,6 @@ struct toJs_impl<std::unordered_set<T>> {`
`149`	`149`	`auto arr = Napi::Array::New(env, as_array.size());`
`150`	`150`	`for (size_t i = 0; i < as_array.size(); i++)`
`151`	`151`	`arr[i] = toJs(env, as_array[i]);`
`152`		`-`
`153`	`152`	`return arr;`
`154`	`153`	`}`
`155`	`154`	`};`