-
Notifications
You must be signed in to change notification settings - Fork 87
🦋 [Main] Naga Branch #857
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
base: master
Are you sure you want to change the base?
🦋 [Main] Naga Branch #857
Conversation
|
Packages published to prioritise usability of the naga-dev network: |
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This is the main naga branch that introduces a comprehensive set of changes across the Lit Protocol JavaScript SDK, including the creation of a new Artillery testing package, significant refactoring of access control conditions with the introduction of new schema-based validation, and removal of wrapped-keys testing files. The PR represents a major restructuring with the addition of comprehensive load testing capabilities and modernized access control condition handling.
Key changes:
- Added a new Artillery package for load testing with various test configurations
- Introduced access-control-conditions-schemas package with Zod-based validation
- Completely refactored access-control-conditions package with new validator, builder pattern, and improved type safety
- Removed all wrapped-keys related test files from local-tests
Reviewed Changes
Copilot reviewed 223 out of 1442 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/artillery/* | New Artillery load testing package with configurations and metadata |
| packages/access-control-conditions-schemas/* | New schema validation package using Zod for access control conditions |
| packages/access-control-conditions/src/lib/* | Complete refactor with new validation, builder pattern, and modernized utilities |
| local-tests/tests/wrapped-keys/* | Removal of all wrapped-keys test files |
| nx.json, package.json | Updated project configuration and dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| }); | ||
| } catch (e) { | ||
| console.log(`Failed to get decimals for ${acc.contractAddress}`); | ||
| logger.info(`Failed to get decimals for ${acc.contractAddress}`); // is this safe to fail and continue? |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inline comment raises a valid concern about error handling safety. Consider documenting the expected behavior when decimal retrieval fails or implementing proper error handling rather than just logging and continuing.
| logger.info(`Failed to get decimals for ${acc.contractAddress}`); // is this safe to fail and continue? | |
| logger.warn(`Failed to get decimals for ${acc.contractAddress}, using default of 18. Error: ${e}`); | |
| decimals = 18; // Default to 18 decimals if retrieval fails |
| ): Promise<string> => { | ||
| const hashed = await hashResourceId(resourceId); | ||
| return uint8arrayToString(new Uint8Array(hashed), 'base16'); | ||
| return Buffer.from(new Uint8Array(hashed)).toString('hex'); |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conversion from ArrayBuffer to Uint8Array to Buffer to hex string could be simplified to Buffer.from(hashed).toString('hex') since Buffer.from() can directly accept ArrayBuffer.
| return Buffer.from(new Uint8Array(hashed)).toString('hex'); | |
| return Buffer.from(hashed).toString('hex'); |
| path, | ||
| returnValueTest: { | ||
| key, | ||
| comparator: comparator || ('=' as any), |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using as any to bypass type checking should be avoided. Instead, define the proper type or use a type assertion that's more specific to the expected type.
| comparator: comparator || ('=' as any), | |
| comparator: comparator || '=', |
| method, | ||
| parameters, | ||
| returnValueTest: { | ||
| comparator: comparator || ('=' as any), |
Copilot
AI
Oct 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another instance of using as any to bypass type checking. This pattern appears multiple times and should be replaced with proper typing.
| comparator: comparator || ('=' as any), | |
| comparator: comparator || '=', |
- replace Bun setup in .github/workflows/lint.yml with Node + corepack-driven pnpm so linting aligns with the rest of the CI pipelines - run lint commands through pnpm to ensure they use the workspace lockfile - update packages/e2e/README.md installation section to point to pnpm add instead of bun install
…ce dependency management
…to-pnpm chore: switch lint workflow to pnpm and update e2e install docs
…g-add-signsessionkey-product-index-3-for feat(pricing): add `sign-session-key` pricing support and product-awa…
chore: release updates for naga-test
chore: update wasm-pack version to latest
chore: update release workflow to build packages before publishing
…0-2025 Feat/naga test upgrade 30 10 2025
chore(release): version packages
Signed-off-by: Michael Lodder <mike@litprotocol.com>
…ImportMeta for clarity and consistency
Detail updates
…ld-issue fix(contracts): guard module path resolution without import.meta.url
chore(release): version packages
WHAT
This is the main naga branch 🌶️
Getting started