Feature/lit 4238 clean unnecessary packages #802

FedericoAmura · 2025-02-26T16:51:51Z

Description

This PR makes the following changes

Removes misc package, mostly with zod schemas and function moving
Reimplements logger package using pino loggers
Removes uin8array package. Replaced by Buffer
Removes nacl package. Replaced with noble curves
Removes encryption package (just a wrapper over lit node client encrypt and decrypt methods)
Removes IEither pattern
Moves a lot of functions where they are actually used. Therefore also reduces packages public API
Fixes unit tests failing in some packages
Updates types and schemas
Unifies localstorage access behind misc-browser (to-be-replaced with storage param where needed)

Note: misc-browser is almost completely removed. It is merely a stricter wrapper of localstorage now. However, as several places rely on localstorage for caching it cannot be blindly removed. It has to be replaced with the storage options the base branch will implement

New package graph

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Compiling verifies a lot of the changes
New and existing unit tests in all packages pass

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

… moving and code refactors

…ommit

FedericoAmura · 2025-03-06T17:08:13Z

README.md

@@ -100,7 +96,7 @@ If you're a tech-savvy user and wish to utilize only specific submodules that ou

 ## Prerequisite

- node (v19.x or above)
+- node (v20.x or above)


v19 has gone out of maintenance a while ago
v20 is in maintenance and supports File (used in encryption)

https://nodejs.org/en/about/previous-releases

FedericoAmura · 2025-03-07T19:14:46Z

local-tests/setup/session-sigs/get-pkp-session-sigs.ts

@@ -43,7 +42,7 @@ export const getPkpAuthContext = (
    }),
  });

-  log('[getPkpAuthContext]: ', authContext);
+  console.log('[getPkpAuthContext]: ', authContext);


Testing logs are more clear when using console than using pino

FedericoAmura · 2025-03-07T19:19:51Z

package.json

+    "jest": "^29.2.2",
+    "jest-environment-jsdom": "^29.7.0",


ts-jest was complaining about jest old version and after v28 jest-environment-jsdom does not come automatically by default

FedericoAmura · 2025-03-07T19:33:14Z

packages/auth/src/lib/authenticators/metamask/eth.ts

-  localStorage.removeItem(storage.AUTH_SOL_SIGNATURE);
-  localStorage.removeItem(storage.AUTH_COSMOS_SIGNATURE);
-  localStorage.removeItem(storage.WEB3_PROVIDER);


None of these were used. We only had writes for some of them but no reads anywhere

FedericoAmura · 2025-03-07T19:34:25Z

packages/auth/src/lib/authenticators/utils.ts

- *
- * @returns {boolean} - True if provider is supported
- */
-export function isSocialLoginSupported(provider: string): boolean {


Not used anywhere and with the new architecture does not make a lot of sense. It is supported if there is a provider after all

FedericoAmura · 2025-03-07T19:43:48Z

packages/constants/project.json

@@ -15,12 +15,6 @@
        "updateBuildableProjectDepsInPackageJson": true
      }
    },
-    "copyJSONFilesToDist": {


FedericoAmura · 2025-03-07T19:44:30Z

packages/constants/src/lib/constants/constants.spec.ts

-  const MUST_HAVE_EVM_CHAINS: Array<string> = [
-    'ethereum',
-    'polygon',
-    'fantom',
-    'xdai',
-    'bsc',
-    'arbitrum',
-    'avalanche',
-    'fuji',
-    'harmony',
-    'kovan',
-    'mumbai',
-    'goerli',
-    'ropsten',
-    'rinkeby',
-    'cronos',
-    'optimism',
-    'celo',
-    'aurora',
-    'eluvio',
-    'alfajores',
-    'xdc',
-    'evmos',
-    'evmosTestnet',
-    'bscTestnet',
-    'baseGoerli',
-  ];
-
-  const MUST_HAVE_SOL_CHAINS = ['solana', 'solanaDevnet', 'solanaTestnet'];
-  const MUST_HAVE_COSMOS_CHAINS = [
-    'cosmos',
-    'kyve',
-    'evmosCosmos',
-    'evmosCosmosTestnet',
-  ];


Types enforce this now

Ansonhkg · 2025-03-11T14:50:03Z

nit: should we use standardise using # instead of _ for truly private fields for real runtime encapsulation in our codebase?

FedericoAmura · 2025-03-11T18:07:09Z

nit: should we use standardise using # instead of _ for truly private fields for real runtime encapsulation in our codebase?

I think we should. It has been supported since 2021 (node v16 and chrome 91) so I don't see any reason why not use it. However I would keep it in a followup to avoid making this bigger

Opinions @MaximusHaximus ?

MaximusHaximus · 2025-03-11T20:28:01Z

Just a reminder that tslib / typescript at multiple had issues with the # markup, so we had intentionally avoided using it in #526 -- are we sure it's safe at this point? :D

FedericoAmura · 2025-03-11T21:06:23Z

Just a reminder that tslib / typescript at multiple had issues with the # markup, so we had intentionally avoided using it in #526 -- are we sure it's safe at this point? :D

A rapid replace and build succeeded

As per gpt

Older versions of tslib (pre‑2.0) didn’t include the helper functions (like __classPrivateFieldGet and __classPrivateFieldSet) that TypeScript emits when down‑leveling the new ECMAScript # private fields and methods. This meant that if you were using # notation and targeting an older ECMAScript version with tslib 1.x, your compiled code would break at runtime.

The fix is to update tslib to version 2.0 or later. Alternatively, you can let TypeScript inline the helper code instead of relying on tslib, or target a modern ECMAScript version that supports private fields natively.

But we are using tslib 2.7 and 2.3. We can let consumers know that tslib requires v2.0.0 (which was released 5 years ago)

MaximusHaximus

One thing that is missing w/ the new logger changes is the ability to configure the SDK logger, which is a pretty fundamental function for the logger. Since we create instances of pino in multiple places in module scope, and don't expose those instances, they are all using defaults for transports, formatting, log levels, etc.

I think we still need a (much simpler than before!) SDK-wide logger package, where sdk consumers can do things like set log level and transports for logs being generated by the SDK.

For example, even though we (and our SDK consumers), might want to see trace level logs sometimes, for every-day purposes many (most) consumers will not want to see that degree of granularity in the output. Right now, since the loggers are not exposed in any way, there is no way for their level to be set.

This also has further repercussions -- one of the big benefits of using pino for logging is that its transports, serializers, and formatters can be configured for various purposes -- e.g. to ship logs to an aggregation service, pretty-print them, or output them as structured JSON. Right now there is no way for consumers to configure those transports.

The simplest way to handle this is, rather than creating new pino instances directly inside of module scopes, we want the logger package to expose two basic methods:
(1) setLoggerOptions(), which can mutate the options that are provided to an sdk-wide singleton pino logger instance.
(2)getLogger(), which returns the singleton instance of our SDK pino logger. Anyt time getLogger() is called, the logger singleton instance will be constructed using the options that are set at that time if it does not already exist, otherwise the existing instance will be returned.

The idea is that, at the top of my app that uses the LIT SDK, before I call any SDK methods, I can configure the SDK logger completely using any supported pino options:

import { setLoggerOptions } from '@lit-protocol/logger';

setLoggerOptions({level: 10, transports: [...] }); // Here's where I would set up e.g. pino-pretty or other log formatting/destination options.

We can also allow mutation of logging configuration even after it has been used. Any time setLoggerOptions() is run, we could reset any existing pino singleton, so that the next time code runs that calls getLogger(), a new one is created with the newly set options.

With this approach, when a consumer of the SDK wants to control the log verbosity, transports, serializers, etc, they can call our method setLoggerOptions() to mutate the options that we use when constructing the logger singleton, and then use the SDK as-normal.

The critical change in structure inside our code is that everywhere we are currently constructing a logger in module scope, then using it directly inside of our function calls, we must modify it to call getLogger() and use the instance returned by that call to call [logger.info](http://logger.info/), logger.error, etc. — on the sdk-wide logger instance.

If we want to provide helpers for consumers of the SDK, we could provide some examples of options combinations for e.g. pretty-printing logs, outputting them to files, or exporting them to an external log sink — all of these would be references to the existing Pino transport and options APIs!

…te and underscore syntax to avoid issues with tslib until we decide they are safe for consumers

FedericoAmura · 2025-03-12T18:20:42Z

One thing that is missing w/ the new logger changes is the ability to configure the SDK logger, which is a pretty fundamental function for the logger. Since we create instances of pino in multiple places in module scope, and don't expose those instances, they are all using defaults for transports, formatting, log levels, etc.

I think we still need a (much simpler than before!) SDK-wide logger package, where sdk consumers can do things like set log level and transports for logs being generated by the SDK.

For example, even though we (and our SDK consumers), might want to see trace level logs sometimes, for every-day purposes many (most) consumers will not want to see that degree of granularity in the output. Right now, since the loggers are not exposed in any way, there is no way for their level to be set.

This also has further repercussions -- one of the big benefits of using pino for logging is that its transports, serializers, and formatters can be configured for various purposes -- e.g. to ship logs to an aggregation service, pretty-print them, or output them as structured JSON. Right now there is no way for consumers to configure those transports.

The simplest way to handle this is, rather than creating new pino instances directly inside of module scopes, we want the logger package to expose two basic methods: (1) setLoggerOptions(), which can mutate the options that are provided to an sdk-wide singleton pino logger instance. (2)getLogger(), which returns the singleton instance of our SDK pino logger. Anyt time getLogger() is called, the logger singleton instance will be constructed using the options that are set at that time if it does not already exist, otherwise the existing instance will be returned.

The idea is that, at the top of my app that uses the LIT SDK, before I call any SDK methods, I can configure the SDK logger completely using any supported pino options:
import { setLoggerOptions } from '@lit-protocol/logger';

setLoggerOptions({level: 10, transports: [...] }); // Here's where I would set up e.g. pino-pretty or other log formatting/destination options.
We can also allow mutation of logging configuration even after it has been used. Any time setLoggerOptions() is run, we could reset any existing pino singleton, so that the next time code runs that calls getLogger(), a new one is created with the newly set options.

With this approach, when a consumer of the SDK wants to control the log verbosity, transports, serializers, etc, they can call our method setLoggerOptions() to mutate the options that we use when constructing the logger singleton, and then use the SDK as-normal.

The critical change in structure inside our code is that everywhere we are currently constructing a logger in module scope, then using it directly inside of our function calls, we must modify it to call getLogger() and use the instance returned by that call to call [logger.info](http://logger.info/), logger.error, etc. — on the sdk-wide logger instance.

If we want to provide helpers for consumers of the SDK, we could provide some examples of options combinations for e.g. pretty-printing logs, outputting them to files, or exporting them to an external log sink — all of these would be references to the existing Pino transport and options APIs!

Great feedback. Considering that I readded the logger pkg to be the centralization piece of loggers. I tested this locally with a script I have and turned out to be a great config place, for example to use pino-pretty when testing locally

…ons are extensible on machine construction and referenceable in machine definition

…is updated after import

…ecessary-packages

Ansonhkg · 2025-04-03T16:05:31Z

Merging this to avoid further divergence from the base PRs. Let’s create follow-up PRs if anything needs to be addressed.

feat: replace logger package and log in misc with pino

527e07a

FedericoAmura requested a review from MaximusHaximus February 26, 2025 16:51

FedericoAmura self-assigned this Feb 26, 2025

FedericoAmura added 10 commits March 3, 2025 16:04

feat: remove misc package with a lot of zod schemas, helper functions…

cfc3c34

… moving and code refactors

feat: changes in lit node client to remove misc, missed in previous c…

c99d1f0

…ommit

feat: logger fixes

21b33f8

fix: remove unused stuff and type fixes

3d2dc98

fix: localStorage.spec.ts

4e2d35c

fix: assemble-most-common-response when property types differ

fc17d03

feat: replace uint8arrays pkg with Buffer

a2ca33e

feat: replace nacl pkg with noble curves

2eec105

feat: remove IEither pattern

a6fd6b9

fix: linting

f1cc256

FedericoAmura commented Mar 6, 2025

View reviewed changes

FedericoAmura added 5 commits March 7, 2025 12:26

fix: run unit test for all packages

a45c133

feat: unify localstorage usage in misc-browser

bc87787

feat: lit errors

fc73d7f

feat: remove full erc20 abi

e9cf2db

feat: remove encryption package

e4d66c9

FedericoAmura commented Mar 7, 2025

View reviewed changes

FedericoAmura added the v8 | Naga label Mar 7, 2025

FedericoAmura marked this pull request as ready for review March 7, 2025 19:55

FedericoAmura requested a review from Ansonhkg as a code owner March 7, 2025 19:55

feat: errors in relay

a36466b

Ansonhkg approved these changes Mar 11, 2025

View reviewed changes

MaximusHaximus requested changes Mar 11, 2025

View reviewed changes

FedericoAmura added 3 commits March 12, 2025 18:16

feat: abstract pino sdk logger singleton in previous logger package

50b9857

feat: replace all javascript private properties with typescript priva…

2b63d1e

…te and underscore syntax to avoid issues with tslib until we decide they are safe for consumers

fix: remove removed tests references

eb930e8

FedericoAmura requested a review from MaximusHaximus March 12, 2025 18:20

FedericoAmura and others added 6 commits March 13, 2025 12:08

feat: missing logger migrations

27d1e9e

feat: make event listener use an action repository so action definiti…

cc83252

…ons are extensible on machine construction and referenceable in machine definition

fix: remove module level loggers that were not updated if sdk logger …

2016736

…is updated after import

feat: add custom actions doc for event listener state machines

14a0ede

Merge branch 'LIT-4211-auth-refactor' into feature/lit-4238-clean-unn…

9b63fd8

…ecessary-packages

chore: update yarn.lock

a3041a5

Ansonhkg merged commit f1d03ce into LIT-4211-auth-refactor Apr 3, 2025
1 check passed

Ansonhkg deleted the feature/lit-4238-clean-unnecessary-packages branch April 3, 2025 16:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/lit 4238 clean unnecessary packages #802

Feature/lit 4238 clean unnecessary packages #802

FedericoAmura commented Feb 26, 2025 •

edited

Loading

FedericoAmura Mar 6, 2025

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025

FedericoAmura Mar 7, 2025

Ansonhkg commented Mar 11, 2025

FedericoAmura commented Mar 11, 2025

MaximusHaximus commented Mar 11, 2025

FedericoAmura commented Mar 11, 2025

MaximusHaximus left a comment

FedericoAmura commented Mar 12, 2025

Ansonhkg commented Apr 3, 2025

Feature/lit 4238 clean unnecessary packages #802

Feature/lit 4238 clean unnecessary packages #802

Conversation

FedericoAmura commented Feb 26, 2025 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

FedericoAmura Mar 6, 2025

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025 • edited Loading

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025

Choose a reason for hiding this comment

FedericoAmura Mar 7, 2025

Choose a reason for hiding this comment

Ansonhkg commented Mar 11, 2025

FedericoAmura commented Mar 11, 2025

MaximusHaximus commented Mar 11, 2025

FedericoAmura commented Mar 11, 2025

MaximusHaximus left a comment

Choose a reason for hiding this comment

FedericoAmura commented Mar 12, 2025

Ansonhkg commented Apr 3, 2025

FedericoAmura commented Feb 26, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading

FedericoAmura Mar 7, 2025 •

edited

Loading