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.

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

Fix error when hexes of length 64 are used as signer secrets #28

Merged
merged 1 commit into from
Mar 11, 2022
Merged

Fix error when hexes of length 64 are used as signer secrets #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 25 additions & 25 deletions src/mcl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const mcl = require("mcl-wasm");
import * as mcl from "mcl-wasm";
import { BigNumber } from "ethers";
import { hashToField } from "./hashToField";
import { arrayify, hexlify, randomBytes, isHexString } from "ethers/lib/utils";
Expand All @@ -15,14 +15,10 @@ export const FIELD_ORDER = BigNumber.from(
"0x30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47"
);

export type mclG2 = any;
export type mclG1 = any;
export type mclFP = any;
export type mclFR = any;
export type SecretKey = mclFR;
export type MessagePoint = mclG1;
export type Signature = mclG1;
export type PublicKey = mclG2;
export type SecretKey = mcl.Fr;
export type MessagePoint = mcl.G1;
export type Signature = mcl.G1;
export type PublicKey = mcl.G2;

export type solG1 = [string, string];
export type solG2 = [string, string, string, string];
Expand All @@ -39,6 +35,10 @@ export async function init() {
mcl.setMapToMode(mcl.BN254);
}

export function validateHex(hex: string) {
if (!isHexString(hex)) throw new BadHex(`Expect hex but got ${hex}`);
}

export function validateDomain(domain: Domain) {
if (domain.length != 32)
throw new BadDomain(`Expect 32 bytes but got ${domain.length}`);
Expand All @@ -57,48 +57,48 @@ export function hashToPoint(msg: string, domain: Domain): MessagePoint {
return p;
}

export function mapToPoint(e0: BigNumber): mclG1 {
export function mapToPoint(e0: BigNumber): mcl.G1 {
let e1 = new mcl.Fp();
e1.setStr(e0.mod(FIELD_ORDER).toString());
return e1.mapToG1();
}

export function toBigEndian(p: mclFP): Uint8Array {
export function toBigEndian(p: mcl.Fp | mcl.Fp2): Uint8Array {
// serialize() gets a little-endian output of Uint8Array
// reverse() turns it into big-endian, which Solidity likes
return p.serialize().reverse();
}

export function g1(): mclG1 {
export function g1(): mcl.G1 {
const g1 = new mcl.G1();
g1.setStr("1 0x01 0x02", 16);
return g1;
}

export function g2(): mclG2 {
export function g2(): mcl.G2 {
const g2 = new mcl.G2();
g2.setStr(
"1 0x1800deef121f1e76426a00665e5c4479674322d4f75edadd46debd5cd992f6ed 0x198e9393920d483a7260bfb731fb5d25f1aa493335a9e71297e485b7aef312c2 0x12c85ea5db8c6deb4aab71808dcb408fe3d1e7690c43d37b4ce6cc0166fa7daa 0x090689d0585ff075ec9e99ad690c3395bc4b313370b38ef355acdadcd122975b"
);
return g2;
}

export function negativeG2(): mclG2 {
export function negativeG2(): mcl.G2 {
const g2 = new mcl.G2();
g2.setStr(
"1 0x1800deef121f1e76426a00665e5c4479674322d4f75edadd46debd5cd992f6ed 0x198e9393920d483a7260bfb731fb5d25f1aa493335a9e71297e485b7aef312c2 0x1d9befcd05a5323e6da4d435f3b617cdb3af83285c2df711ef39c01571827f9d 0x275dc4a288d1afb3cbb1ac09187524c7db36395df7be3b99e673b13a075a65ec"
);
return g2;
}

export function g1ToHex(p: mclG1): solG1 {
export function g1ToHex(p: mcl.G1): solG1 {
p.normalize();
const x = hexlify(toBigEndian(p.getX()));
const y = hexlify(toBigEndian(p.getY()));
return [x, y];
}

export function g2ToHex(p: mclG2): solG2 {
export function g2ToHex(p: mcl.G2): solG2 {
p.normalize();
const x = toBigEndian(p.getX());
const x0 = hexlify(x.slice(32));
Expand Down Expand Up @@ -183,20 +183,20 @@ export function aggregateRaw(signatures: Signature[]): Signature {
return aggregated;
}

export function randFr(): mclFR {
export function randFr(): mcl.Fr {
const r = hexlify(randomBytes(12));
let fr = new mcl.Fr();
fr.setHashOf(r);
return fr;
}

export function randMclG1(): mclG1 {
export function randMclG1(): mcl.G1 {
const p = mcl.mul(g1(), randFr());
p.normalize();
return p;
}

export function randMclG2(): mclG2 {
export function randMclG2(): mcl.G2 {
const p = mcl.mul(g2(), randFr());
p.normalize();
return p;
Expand All @@ -211,38 +211,38 @@ export function randG2(): solG2 {
}

export function parseFr(hex: string) {
if (!isHexString(hex)) throw new BadHex(`Expect hex but got ${hex}`);
validateHex(hex);
const fr = new mcl.Fr();
fr.setStr(hex);
return fr;
}

export function setHashFr(hex: string) {
if (!isHexString(hex)) throw new BadHex(`Expect hex but got ${hex}`);
validateHex(hex);
const fr = new mcl.Fr();
fr.setHashOf(hex);
return fr;
}

export function parseG1(solG1: solG1): mclG1 {
export function parseG1(solG1: solG1): mcl.G1 {
const g1 = new mcl.G1();
const [x, y] = solG1;
g1.setStr(`1 ${x} ${y}`, 16);
return g1;
}

export function parseG2(solG2: solG2): mclG2 {
export function parseG2(solG2: solG2): mcl.G2 {
const g2 = new mcl.G2();
const [x0, x1, y0, y1] = solG2;
g2.setStr(`1 ${x0} ${x1} ${y0} ${y1}`);
return g2;
}

export function dumpFr(fr: mclFR): string {
export function dumpFr(fr: mcl.Fr): string {
return `0x${fr.serializeToHexStr()}`;
}

export function loadFr(hex: string): mclFR {
export function loadFr(hex: string): mcl.Fr {
const fr = new mcl.Fr();
fr.deserializeHexStr(hex.slice(2));
return fr;
Expand Down
21 changes: 16 additions & 5 deletions src/signer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,24 @@ export class BlsSignerFactory {
private constructor() {}

public getSigner(domain: Domain, secretHex?: string) {
const secret = secretHex
? secretHex.length == 66
? parseFr(secretHex)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After the [email protected] update, parseFr no longer works with hexes of length 64 (string length 66). It does work on hex values of length 1 (i.e. 0x1) up to length 63, one less than what keccak256 produces.

@ChihChengLiang Do you know why this might have changed between mcl-wasm versions, and if there are any implications to switching to just use setHashFr?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know the changes in mcl-wasm. Maybe #22 is related?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was unable to determine a way to validate that the hex is in range. So I ended up setting up the secret parsing for the signer as such:

  • If the secret is missing/empty, generate a random one.
  • Attempt to directly set the hex value. If this fails, fall back to hashing the secret.

I think this should cover the range of use cases and expected outputs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like a good solution. @philsippl Would you like to provide a test case example value to make sure this change doesn't break your use case?

: setHashFr(secretHex)
: randFr();
const secret = this.getSecret(secretHex);
return new BlsSigner(domain, secret);
}

private getSecret(secretHex?: string) {
if (!secretHex) {
// Generate a random secret
return randFr();
}

try {
// Attempt to directly parse the hex
return parseFr(secretHex);
} catch {
// If that fails, hash it
return setHashFr(secretHex);
}
}
}

class BlsSigner extends BlsVerifier implements BlsSignerInterface {
Expand Down
35 changes: 35 additions & 0 deletions test/signer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { assert } from "chai";
import { arrayify, formatBytes32String, keccak256 } from "ethers/lib/utils";
import { aggregate, BlsSignerFactory } from "../src/signer";
import { BadHex } from "../src/exceptions";

describe("BLS Signer", async () => {
// Domain is a data that signer and verifier must agree on
Expand Down Expand Up @@ -60,4 +61,38 @@ describe("BLS Signer", async () => {
assert.isTrue(signer.verify(signature, signer.pubkey, message));
}
}).timeout(20000);

describe("getSigner", () => {
const prettyHex = (hex: string): string => {
if (hex.length < 11) {
return hex;
}
return `${hex.slice(0, 6)}...${hex.slice(-4)}`;
};
let factory: BlsSignerFactory;

before(async () => {
factory = await BlsSignerFactory.new();
});

it("fails if secret is not a hex value", () => {
assert.throws(() => factory.getSigner(DOMAIN, "abc"), BadHex);
});

[
"",
jacque006 marked this conversation as resolved.
Show resolved Hide resolved
"0x0",
"0x01",
"0x3ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1c",
"0x2ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1cb",
"0x3ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1cb",
"0x3ac225168df54212a25c1c01fd35bebfea408fdac2e31ddd6f80a4bbf9a5f1cbb5553de315e0edf504d9150af82dafa5c4667fa618ed0a6f19c69b41166c5510",
].forEach((secret) => {
it(`gets a signer for secret "${prettyHex(secret)}" (length ${
secret.length
})`, () => {
assert.isDefined(factory.getSigner(DOMAIN, secret));
});
});
});
});