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

Feat/naga fix bls wasm cleanup #776

Open
wants to merge 50 commits into
base: feature/lit-3748-naga
Choose a base branch
from

Conversation

Ansonhkg
Copy link
Collaborator

@Ansonhkg Ansonhkg commented Jan 23, 2025

WHAT

  • Refactor bls.rs to use lit-bls-wasm as a dependency instead & added unit tests
  • Removed bls variant as it's handled automatically already by the lit-bls-wasm package.
  • Added threshold calculation, as consensus (minNodeCount) != threshold

Test

Command

E2E

NETWORK=custom DEBUG=true yarn test:local --filter=testUseEoaSessionSigsToPkpSign,testUseEoaSessionSigsToExecuteJsSigning,testUseEoaSessionSigsToEncryptDecryptString,testUsePkpSessionSigsToPkpSign --exclude=Parallel
image

BLS unit test

node 'node_modules/.bin/jest' './packages/wasm/src/lib/bls.spec.ts' -c './packages/wasm/jest.config.ts'
image

❗️NOTE

  • This is tested against (ordered by date as this page)

  • ❌ It's NOT compatible with the latest commit in the develop branch, specifically this commit/pr

  • ❌ This dose not work against naga-dev as we are unsure which commit was used to deploy.

  • ❌ authSig (ETH, SOL, Cosmos) encryption/decryption does not work.

Ansonhkg and others added 30 commits January 11, 2025 01:37
… usage

- Also send `nodeSet` to `sign_session_key` endpoint as it is required
- Fixed typo on name of wasm method for `ecdsaCombineAndVerify` (was previously unused)
…a-dev-support-with-rust-toolchain

add rust-toolchain.toml so we autoinstall some necessary rust stuff
…a-add-naga-dev-support

feat: add naga dev support
- **Added**: New file  defining:
  -  union type for cryptographic signature types.
  -  interface for standardized HTTP responses.
  - Partial  interface for raw ECDSA message data.
- **Updated**:  now exports all types from .

**Implications**:
- Enables consistent usage of HTTP response and signature types across the application.
- Improves maintainability and type safety for API client implementations.
…les and internal documentation (#1234)<details><summary>Difference analysis</summary>Changes made:- Removed the import of `PKPSignShare`- Removed the export of `SuccessNodePromises` from types- Removed commented example output blocks- Removed an internal console.log statementThe changes are primarily cleanup and do not alter functionality.</details>
- **Added**: Optional fields  and  to the  interface.
- **Updated**: Moved  to a new position and made it optional ().
- **Removed**: The original  line was refactored for better structure.

**Implications**:
- Provides flexibility by allowing certain fields to be undefined or null.
- Improves structure for handling edge cases or incomplete data in
@Ansonhkg Ansonhkg changed the base branch from feature/lit-4124-js-sdk-naga-fix-bls-wasm to feature/lit-3748-naga January 23, 2025 05:03
chore(ci): update `NAGA_COMMIT_HASH` for `rc-naga-2025-01-23` branch
Alright, so I'm trying to generate a commit message based on the provided diff file. Let me go through this step by step.

First, I look at the diff file. It shows changes made to the file `get-signatures.ts`. The modifications start at line 105 in that file. Comparing the before and after versions, I notice a few key things.

In the original code, there were lines where `publicKey` and `dataSigned` were being set using `mostCommonString`. These values were also using optional chaining with `?? ''`, which means if they were null or undefined, an empty string would be used as a fallback. But in the updated version, these assignments are moved to earlier lines where `_publicKey` and `_dataSigned` are declared, and now there's an additional check: if either is missing, an error is thrown.

So, the commit message needs to capture the essence of this change. The type here should be 'feat' because it adds a new error handling mechanism. The scope is clearly 'helpers' since that's where the file is located. For the summary, I need to succinctly explain what changed: ensuring both public key and data signed are present by throwing an error if either is missing.

Putting this all together, the message should be structured as `<type>(<scope>): <summary>`, which becomes 'feat(helpers): Ensure public key and data signed are present in combined signature.'
</think>

feat(helpers): Ensure public key and data signed are present in combined signature
Okay, I need to help the user generate a commit message based on the provided diff and history. Let me look at this step by step.

First, I'll examine the diff content. The file changed is `packages/wasm/rust/src/bls.rs`. Looking at the changes, it seems that a block of code has been removed between lines 11 and 16 of this file. Specifically, an enum called `BlsVariant` is being deleted.

The context provided includes prior commit messages. The last one was about removing a structure, so I should follow a similar pattern here. Since the task now involves removing an enum, that aligns well with using 'remove' as the type of change and indicating it's in the scope of the Rust package.

I'll form the message accordingly: '<type>' would be 'remove', the '<scope>' is 'rust', and the summary is what is being removed, which is the BlsVariant enum. Putting that together gives a clear and concise commit message that accurately reflects the change made.
</think>

The provided code diff shows that the `BlsVariant` enum has been removed from the file. This should be reflected in the commit message.

Step-by-step explanation:
1. Identify the type of change (addition, removal, modification).
2. Determine the scope or component affected.
3. Provide a clear and concise summary of the change.

Based on this:

remove(rust): BlsVariant enum
Alright, I'm faced with generating a commit message based on the provided diff. Let me go through this step by step.

First, I look at the files changed. The file in question is `index.ts` within the `packages/wasm/src/` directory. So, the scope of the change is likely 'wasm' since that's part of the path.

Next, I examine the specific changes in the diff. There are a few key things here:

1. **Imports Removed:** The import line for `BlsVariant` from `'./pkg/wasm-internal'` has been removed.
2. **Exports Adjusted:** Similarly, `BlsVariant` is no longer being exported from this file.
3. **Comments Deleted:** In two function definitions (`blsCombine`, `blsDecrypt`), the parameter comments for `{BlsVariant} variant` have been removed.

Putting this together, it seems that the `BlsVariant` type has been either moved elsewhere or is no longer needed in this context. The changes are about removing unnecessary elements from this file to clean up the codebase.

Considering commit message conventions, the general structure should be `<type>(<scope>): <summary>`. For this case, since it's a removal, `remove` would be an appropriate type. The scope is clearly 'wasm'.

So, the summary part should articulate that we're removing the BlsVariant type from index.ts.

Putting it all together: `remove(wasm): Remove BlsVariant type from index.ts`
</think>

The changes involve removing imports and exports of `BlsVariant` as well as its parameter comments in function definitions. This indicates removal or deprecation of the `BlsVariant` type from the current package. The scope is within 'wasm' based on the file path.

```text
remove(wasm): Remove BlsVariant type from index.ts
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants