Merge pull request #43 from ChainSafe/fix-tests

Fix v1.0.0 spec tests and error conditions

.gitignore

@@ -65,3 +65,5 @@ typings/
 dist/
 lib/
 benchmark-reports
+
+.vscode/

src/blst/privateKey.ts

@@ -1,9 +1,10 @@
 import * as blst from "@chainsafe/blst";
-import {bytesToHex, hexToBytes, randomBytes} from "../helpers";
+import {bytesToHex, hexToBytes, isZeroUint8Array, randomBytes} from "../helpers";
 import {SECRET_KEY_LENGTH} from "../constants";
 import {IPrivateKey} from "../interface";
 import {PublicKey} from "./publicKey";
 import {Signature} from "./signature";
+import {ZeroPrivateKeyError} from "../errors";
 
 export class PrivateKey implements IPrivateKey {
   readonly value: blst.SecretKey;
@@ -13,6 +14,11 @@ export class PrivateKey implements IPrivateKey {
   }
 
   static fromBytes(bytes: Uint8Array): PrivateKey {
+    // draft-irtf-cfrg-bls-signature-04 does not allow SK == 0
+    if (isZeroUint8Array(bytes)) {
+      throw new ZeroPrivateKeyError();
+    }
+
     const sk = blst.SecretKey.fromBytes(bytes);
     return new PrivateKey(sk);
   }

src/blst/publicKey.ts

@@ -1,4 +1,5 @@
 import * as blst from "@chainsafe/blst";
+import {EmptyAggregateError, ZeroPublicKeyError} from "../errors";
 import {bytesToHex, hexToBytes} from "../helpers";
 import {IPublicKey} from "../interface";
 
@@ -13,6 +14,10 @@ export class PublicKey implements IPublicKey {
 
   static fromBytes(bytes: Uint8Array): PublicKey {
     const affine = blst.PublicKey.fromBytes(bytes);
+    if (affine.value.is_inf()) {
+      throw new ZeroPublicKeyError();
+    }
+
     const jacobian = blst.AggregatePublicKey.fromPublicKey(affine);
     return new PublicKey(affine, jacobian);
   }
@@ -22,6 +27,10 @@ export class PublicKey implements IPublicKey {
   }
 
   static aggregate(pubkeys: PublicKey[]): PublicKey {
+    if (pubkeys.length === 0) {
+      throw new EmptyAggregateError();
+    }
+
     const jacobian = blst.aggregatePubkeys(pubkeys.map((pk) => pk.jacobian));
     const affine = jacobian.toPublicKey();
     return new PublicKey(affine, jacobian);

src/blst/signature.ts

@@ -2,6 +2,7 @@ import * as blst from "@chainsafe/blst";
 import {bytesToHex, hexToBytes} from "../helpers";
 import {ISignature} from "../interface";
 import {PublicKey} from "./publicKey";
+import {EmptyAggregateError, ZeroSignatureError} from "../errors";
 
 export class Signature implements ISignature {
   readonly affine: blst.Signature;
@@ -19,11 +20,20 @@ export class Signature implements ISignature {
   }
 
   static aggregate(signatures: Signature[]): Signature {
+    if (signatures.length === 0) {
+      throw new EmptyAggregateError();
+    }
+
     const agg = blst.AggregateSignature.fromSignatures(signatures.map((sig) => sig.affine));
     return new Signature(agg.toSignature());
   }
 
   verify(publicKey: PublicKey, message: Uint8Array): boolean {
+    // Individual infinity signatures are NOT okay. Aggregated signatures MAY be infinity
+    if (this.affine.value.is_inf()) {
+      throw new ZeroSignatureError();
+    }
+
     return this.aggregateVerify([message], [publicKey.affine]);
   }
 

src/constants.ts

@@ -3,5 +3,3 @@ export const SIGNATURE_LENGTH = 96;
 export const FP_POINT_LENGTH = 48;
 export const PUBLIC_KEY_LENGTH = FP_POINT_LENGTH;
 export const G2_HASH_PADDING = 16;
-export const EMPTY_PUBLIC_KEY = new Uint8Array(PUBLIC_KEY_LENGTH);
-export const EMPTY_SIGNATURE = new Uint8Array(SIGNATURE_LENGTH);

src/errors.ts

@@ -1,5 +1,45 @@
 /**
- * Indicate that this error is expected and should not be ignored
+ * This error should not be ignored by the functional interface
- * by the functional interface try / catch blocks
+ * try / catch blocks, to prevent false negatives
  */
-export class ExpectedError extends Error {}
+export class NotInitializedError extends Error {
+  constructor(implementation: string) {
+    super(`NOT_INITIALIZED: ${implementation}`);
+  }
+}
+
+export class ZeroPrivateKeyError extends Error {
+  constructor() {
+    super("ZERO_PRIVATE_KEY");
+  }
+}
+
+export class ZeroPublicKeyError extends Error {
+  constructor() {
+    super("ZERO_PUBLIC_KEY");
+  }
+}
+
+export class ZeroSignatureError extends Error {
+  constructor() {
+    super("ZERO_SIGNATURE");
+  }
+}
+
+export class EmptyAggregateError extends Error {
+  constructor() {
+    super("EMPTY_AGGREGATE_ARRAY");
+  }
+}
+
+export class InvalidOrderError extends Error {
+  constructor() {
+    super("INVALID_ORDER");
+  }
+}
+
+export class InvalidLengthError extends Error {
+  constructor(arg: string, length: number) {
+    super(`INVALID_LENGTH: ${arg} must have ${length} bytes`);
+  }
+}

src/functional.ts

@@ -1,6 +1,6 @@
 import {IBls} from "./interface";
 import {validateBytes} from "./helpers";
-import {ExpectedError} from "./errors";
+import {NotInitializedError} from "./errors";
 
 // Returned type is enforced at each implementation's index
 // eslint-disable-next-line @typescript-eslint/explicit-function-return-type
@@ -54,7 +54,7 @@ export function functionalInterfaceFactory({
     try {
       return Signature.fromBytes(signature).verify(PublicKey.fromBytes(publicKey), message);
     } catch (e) {
-      if (e instanceof ExpectedError) throw e;
+      if (e instanceof NotInitializedError) throw e;
       return false;
     }
   }
@@ -76,7 +76,7 @@ export function functionalInterfaceFactory({
         message
       );
     } catch (e) {
-      if (e instanceof ExpectedError) throw e;
+      if (e instanceof NotInitializedError) throw e;
       return false;
     }
   }
@@ -102,7 +102,7 @@ export function functionalInterfaceFactory({
         messages.map((msg) => msg)
       );
     } catch (e) {
-      if (e instanceof ExpectedError) throw e;
+      if (e instanceof NotInitializedError) throw e;
       return false;
     }
   }

src/helpers/utils.ts

@@ -3,14 +3,6 @@ import randomBytes from "randombytes";
 // Single import to ease changing this lib if necessary
 export {randomBytes};
 
-export function isEqualBytes(a: Buffer | Uint8Array, b: Buffer | Uint8Array): boolean {
-  return toBuffer(a).equals(toBuffer(b));
-}
-
-export function toBuffer(input: Uint8Array): Buffer {
-  return Buffer.from(input.buffer, input.byteOffset, input.length);
-}
-
 /**
  * Validate bytes to prevent confusing WASM errors downstream if bytes is null
  */
@@ -24,3 +16,7 @@ export function validateBytes(
     }
   }
 }
+
+export function isZeroUint8Array(bytes: Uint8Array): boolean {
+  return bytes.every((byte) => byte === 0);
+}

src/herumi/context.ts

@@ -1,6 +1,6 @@
 /* eslint-disable require-atomic-updates */
 import bls from "bls-eth-wasm";
-import {ExpectedError} from "../errors";
+import {NotInitializedError} from "../errors";
 
 type Bls = typeof bls;
 let blsGlobal: Bls | null = null;
@@ -28,7 +28,7 @@ export function destroy(): void {
 
 export function getContext(): Bls {
   if (!blsGlobal) {
-    throw new ExpectedError("BLS not initialized");
+    throw new NotInitializedError("herumi");
   }
   return blsGlobal;
 }

src/herumi/privateKey.ts

@@ -6,17 +6,22 @@ import {PublicKey} from "./publicKey";
 import {Signature} from "./signature";
 import {bytesToHex, hexToBytes} from "../helpers";
 import {IPrivateKey} from "../interface";
+import {InvalidLengthError, ZeroPrivateKeyError} from "../errors";
 
 export class PrivateKey implements IPrivateKey {
   readonly value: SecretKeyType;
 
   constructor(value: SecretKeyType) {
+    if (value.isZero()) {
+      throw new ZeroPrivateKeyError();
+    }
+
     this.value = value;
   }
 
   static fromBytes(bytes: Uint8Array): PrivateKey {
     if (bytes.length !== SECRET_KEY_LENGTH) {
-      throw Error(`Private key should have ${SECRET_KEY_LENGTH} bytes`);
+      throw new InvalidLengthError("PrivateKey", SECRET_KEY_LENGTH);
     }
 
     const context = getContext();

src/herumi/publicKey.ts

@@ -1,24 +1,29 @@
 import {PublicKeyType} from "bls-eth-wasm";
 import {getContext} from "./context";
-import {EMPTY_PUBLIC_KEY, PUBLIC_KEY_LENGTH} from "../constants";
+import {PUBLIC_KEY_LENGTH} from "../constants";
-import {bytesToHex, hexToBytes, isEqualBytes} from "../helpers";
+import {bytesToHex, hexToBytes, isZeroUint8Array} from "../helpers";
 import {IPublicKey} from "../interface";
+import {EmptyAggregateError, InvalidLengthError, ZeroPublicKeyError} from "../errors";
 
 export class PublicKey implements IPublicKey {
   readonly value: PublicKeyType;
 
   constructor(value: PublicKeyType) {
+    if (value.isZero()) {
+      throw new ZeroPublicKeyError();
+    }
+
     this.value = value;
   }
 
   static fromBytes(bytes: Uint8Array): PublicKey {
     if (bytes.length !== PUBLIC_KEY_LENGTH) {
-      throw Error(`Public key must have ${PUBLIC_KEY_LENGTH} bytes`);
+      throw new InvalidLengthError("PublicKey", PUBLIC_KEY_LENGTH);
     }
 
     const context = getContext();
     const publicKey = new context.PublicKey();
-    if (!isEqualBytes(EMPTY_PUBLIC_KEY, bytes)) {
+    if (!isZeroUint8Array(bytes)) {
       publicKey.deserialize(bytes);
     }
     return new PublicKey(publicKey);
@@ -30,7 +35,7 @@ export class PublicKey implements IPublicKey {
 
   static aggregate(pubkeys: PublicKey[]): PublicKey {
     if (pubkeys.length === 0) {
-      throw Error("EMPTY_AGGREGATE_ARRAY");
+      throw new EmptyAggregateError();
     }
 
     const agg = new PublicKey(pubkeys[0].value.clone());

src/herumi/signature.ts

@@ -1,16 +1,17 @@
-import {SIGNATURE_LENGTH, EMPTY_SIGNATURE} from "../constants";
+import {SIGNATURE_LENGTH} from "../constants";
 import {SignatureType} from "bls-eth-wasm";
 import {getContext} from "./context";
 import {PublicKey} from "./publicKey";
-import {bytesToHex, hexToBytes, isEqualBytes} from "../helpers";
+import {bytesToHex, hexToBytes, isZeroUint8Array} from "../helpers";
 import {ISignature} from "../interface";
+import {EmptyAggregateError, InvalidLengthError, InvalidOrderError} from "../errors";
 
 export class Signature implements ISignature {
   readonly value: SignatureType;
 
   constructor(value: SignatureType) {
     if (!value.isValidOrder()) {
-      throw Error("Signature is not in valid order");
+      throw new InvalidOrderError();
     }
 
     this.value = value;
@@ -18,12 +19,12 @@ export class Signature implements ISignature {
 
   static fromBytes(bytes: Uint8Array): Signature {
     if (bytes.length !== SIGNATURE_LENGTH) {
-      throw Error(`Signature must have ${SIGNATURE_LENGTH} bytes`);
+      throw new InvalidLengthError("Signature", SIGNATURE_LENGTH);
     }
 
     const context = getContext();
     const signature = new context.Signature();
-    if (!isEqualBytes(EMPTY_SIGNATURE, bytes)) {
+    if (!isZeroUint8Array(bytes)) {
       signature.deserialize(bytes);
     }
     return new Signature(signature);
@@ -35,7 +36,7 @@ export class Signature implements ISignature {
 
   static aggregate(signatures: Signature[]): Signature {
     if (signatures.length === 0) {
-      throw Error("EMPTY_AGGREGATE_ARRAY");
+      throw new EmptyAggregateError();
     }
 
     const context = getContext();

test/benchmark/index.ts

@@ -3,9 +3,7 @@ import {runForAllImplementations} from "../switch";
 import {IPublicKey, ISignature} from "../../src/interface";
 import {randomBytes} from "../../src/helpers";
 
-runForAllImplementations(async (bls, implementation) => {
+runForAllImplementations((bls, implementation) => {
-  await bls.init();
-
   const aggCount = 30;
 
   // verify

test/spec/aggregate.test.ts

@@ -3,6 +3,7 @@ import {describeDirectorySpecTest, InputType} from "@chainsafe/lodestar-spec-tes
 import {bytesToHex, hexToBytes} from "../../src/helpers";
 import {SPEC_TESTS_DIR} from "../params";
 import {describeForAllImplementations} from "../switch";
+import {EmptyAggregateError} from "../../src/errors";
 
 interface IAggregateSigsTestCase {
   data: {
@@ -21,7 +22,7 @@ describeForAllImplementations((bls) => {
         const agg = bls.aggregateSignatures(signatures.map(hexToBytes));
         return bytesToHex(agg);
       } catch (e) {
-        if (e.message === "EMPTY_AGGREGATE_ARRAY") return null;
+        if (e instanceof EmptyAggregateError) return null;
         throw e;
       }
     },

test/spec/sign.test.ts

@@ -3,6 +3,7 @@ import {describeDirectorySpecTest, InputType} from "@chainsafe/lodestar-spec-tes
 import {bytesToHex, hexToBytes} from "../../src/helpers";
 import {SPEC_TESTS_DIR} from "../params";
 import {describeForAllImplementations} from "../switch";
+import {ZeroPrivateKeyError} from "../../src/errors";
 
 interface ISignMessageTestCase {
   data: {
@@ -19,9 +20,14 @@ describeForAllImplementations((bls) => {
     "bls/sign/small",
     path.join(SPEC_TESTS_DIR, "general/phase0/bls/sign/small"),
     (testCase) => {
+      try {
         const {privkey, message} = testCase.data.input;
         const signature = bls.sign(hexToBytes(privkey), hexToBytes(message));
         return bytesToHex(signature);
+      } catch (e) {
+        if (e instanceof ZeroPrivateKeyError) return null;
+        else throw e;
+      }
     },
     {
       inputTypes: {data: InputType.YAML},

test/switch.ts

@@ -15,11 +15,12 @@ export function getBls(implementation: Implementation): IBls {
 }
 
 export async function runForAllImplementations(
-  callback: (bls: IBls, implementation: Implementation) => Promise<void> | void
+  callback: (bls: IBls, implementation: Implementation) => void
 ): Promise<void> {
   for (const implementation of ["blst", "herumi"] as Implementation[]) {
     const bls = getBls(implementation);
-    await callback(bls, implementation);
+    await bls.init();
+    callback(bls, implementation);
   }
 }
 

test/unit/helpers/bytes.test.ts

@@ -0,0 +1,29 @@
+import {expect} from "chai";
+import {isZeroUint8Array} from "../../../src/helpers/utils";
+
+describe("helpers / bytes", () => {
+  describe("isZeroUint8Array", () => {
+    const testCases: {isZero: boolean; hex: string}[] = [
+      {hex: "0x00", isZero: true},
+      {hex: "0x" + "00".repeat(32), isZero: true},
+      {hex: "0x" + "00".repeat(96), isZero: true},
+      {hex: "0x" + "00".repeat(31) + "01", isZero: false},
+      {
+        hex: "0xb6f21199594b56d77670564bf422cb331d5281ca2c1f9a45588a56881d8287ef8619efa6456d6cd2ef61306aa5b21311",
+        isZero: false,
+      },
+    ];
+
+    for (const {hex, isZero} of testCases) {
+      it(`${hex} isZero = ${isZero}`, () => {
+        const bytes = hexToBytesNode(hex);
+        console.log(bytes);
+        expect(isZeroUint8Array(bytes)).to.equal(isZero);
+      });
+    }
+  });
+});
+
+function hexToBytesNode(hex: string): Buffer {
+  return Buffer.from(hex.replace("0x", ""), "hex");
+}