Skip to content

Fix/remove unused case 218 for bls encrypt #682

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Fix/remove unused case 218 for bls encrypt #682

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9b0ca1
doc(bls-encrypt): better comment on G2/G1 points usage
Ansonhkg Oct 10, 2024
407682a
feat(bls): remove case 218 as we don't use it
Ansonhkg Oct 10, 2024
6d2f502
fmt
Ansonhkg Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 29 additions & 6 deletions packages/crypto/src/lib/crypto.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export const unloadModules = () => {

/**
* Encrypt data with a BLS public key.
* We are using G1 for encryption and G2 for signatures
*
* @param publicKey hex-encoded string of the BLS public key to encrypt with
* @param data Uint8Array of the data to encrypt
Expand All @@ -114,12 +115,34 @@ export const encrypt = (
publicKey: string,
data: Uint8Array,
identity: Uint8Array
): string => {
return blsSdk.encrypt(
publicKey,
uint8arrayToString(data, 'base64'),
uint8arrayToString(identity, 'base64')
);
): Promise<string> => {

const publicKey = Buffer.from(publicKeyHex, 'hex');

/**
* Our system uses BLS12-381 on the G1 curve for encryption.
* However, on the SDK side (this function), we expect the public key
* to use the G2 curve for signature purposes, hence the switch on public key length.
*
* The G2 curve, `Bls12381G2`, is typically associated with signature generation/verification,
* while G1 is associated with encryption. Here, the length of the public key determines how
* we handle the encryption and the format of the returned encrypted message.
*/

if (publicKeyHex.replace('0x', '').length !== 96) {
throw new InvalidParamType(
{
info: {
publicKeyHex,
},
},
`Invalid public key length. Expecting 96 characters, got ${publicKeyHex.replace('0x', '').length} instead.`
);
}
return Buffer.from(
await blsEncrypt('Bls12381G2', publicKey, message, identity)
Copy link

@graphite-app graphite-app bot Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parameter message in the blsEncrypt function call should be replaced with data to correctly use the function parameter defined in the method signature. This ensures consistency with the input parameters and prevents potential errors. Consider updating the line to:

await blsEncrypt('Bls12381G2', publicKey, data, identity)

This change aligns the function call with the method's declared parameters.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

Copy link

@graphite-app graphite-app bot Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blsEncrypt function is called but appears to be undefined in this scope. Please ensure it's properly imported from the appropriate module or defined within this file. If it's a custom implementation, consider adding a comment explaining its origin or providing a reference to its definition.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

).toString('base64');

};

/**
Expand Down
Loading