From c557db4a78115373f8b0fe7c8fd847a9964bce9d Mon Sep 17 00:00:00 2001 From: dapplion Date: Mon, 30 Nov 2020 00:20:52 +0000 Subject: [PATCH] Make errors consistent --- src/blst/publicKey.ts | 6 +++++- src/blst/signature.ts | 18 +++++++++++------- src/errors.ts | 34 ++++++++++++++++++++++++++++------ src/functional.ts | 8 ++++---- src/herumi/context.ts | 4 ++-- src/herumi/privateKey.ts | 4 ++-- src/herumi/publicKey.ts | 6 +++--- src/herumi/signature.ts | 7 ++++--- test/spec/aggregate.test.ts | 3 ++- 9 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/blst/publicKey.ts b/src/blst/publicKey.ts index 187e191..39c3684 100644 --- a/src/blst/publicKey.ts +++ b/src/blst/publicKey.ts @@ -1,5 +1,5 @@ import * as blst from "@chainsafe/blst"; -import {ZeroPublicKeyError} from "../errors"; +import {EmptyAggregateError, ZeroPublicKeyError} from "../errors"; import {bytesToHex, hexToBytes} from "../helpers"; import {IPublicKey} from "../interface"; @@ -27,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); diff --git a/src/blst/signature.ts b/src/blst/signature.ts index de4379a..3a476fd 100644 --- a/src/blst/signature.ts +++ b/src/blst/signature.ts @@ -2,7 +2,7 @@ import * as blst from "@chainsafe/blst"; import {bytesToHex, hexToBytes} from "../helpers"; import {ISignature} from "../interface"; import {PublicKey} from "./publicKey"; -import {ZeroSignatureError} from "../errors"; +import {EmptyAggregateError, ZeroSignatureError} from "../errors"; export class Signature implements ISignature { readonly affine: blst.Signature; @@ -12,12 +12,7 @@ export class Signature implements ISignature { } static fromBytes(bytes: Uint8Array): Signature { - const affine = blst.Signature.fromBytes(bytes); - if (affine.value.is_inf()) { - throw new ZeroSignatureError(); - } - - return new Signature(affine); + return new Signature(blst.Signature.fromBytes(bytes)); } static fromHex(hex: string): Signature { @@ -25,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]); } diff --git a/src/errors.ts b/src/errors.ts index 80ff3fc..58ac7b2 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,23 +1,45 @@ /** - * Indicate that this error is expected and should not be ignored - * by the functional interface try / catch blocks + * This error should not be ignored by the functional interface + * 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("PRIVATE_KEY_IS_ZERO"); + super("ZERO_PRIVATE_KEY"); } } export class ZeroPublicKeyError extends Error { constructor() { - super("PUBLIC_KEY_IS_ZERO"); + super("ZERO_PUBLIC_KEY"); } } export class ZeroSignatureError extends Error { constructor() { - super("SIGNATURE_IS_ZERO"); + 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`); } } diff --git a/src/functional.ts b/src/functional.ts index da7ad41..a731a7d 100644 --- a/src/functional.ts +++ b/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; } } diff --git a/src/herumi/context.ts b/src/herumi/context.ts index b4da52e..7780283 100644 --- a/src/herumi/context.ts +++ b/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; } diff --git a/src/herumi/privateKey.ts b/src/herumi/privateKey.ts index cfa5438..7eb9df6 100644 --- a/src/herumi/privateKey.ts +++ b/src/herumi/privateKey.ts @@ -6,7 +6,7 @@ import {PublicKey} from "./publicKey"; import {Signature} from "./signature"; import {bytesToHex, hexToBytes} from "../helpers"; import {IPrivateKey} from "../interface"; -import {ZeroPrivateKeyError} from "../errors"; +import {InvalidLengthError, ZeroPrivateKeyError} from "../errors"; export class PrivateKey implements IPrivateKey { readonly value: SecretKeyType; @@ -21,7 +21,7 @@ export class PrivateKey implements IPrivateKey { 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(); diff --git a/src/herumi/publicKey.ts b/src/herumi/publicKey.ts index 9fdf543..34c8338 100644 --- a/src/herumi/publicKey.ts +++ b/src/herumi/publicKey.ts @@ -3,7 +3,7 @@ import {getContext} from "./context"; import {PUBLIC_KEY_LENGTH} from "../constants"; import {bytesToHex, hexToBytes, isZeroUint8Array} from "../helpers"; import {IPublicKey} from "../interface"; -import {ZeroPublicKeyError} from "../errors"; +import {EmptyAggregateError, InvalidLengthError, ZeroPublicKeyError} from "../errors"; export class PublicKey implements IPublicKey { readonly value: PublicKeyType; @@ -18,7 +18,7 @@ export class PublicKey implements IPublicKey { 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(); @@ -35,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()); diff --git a/src/herumi/signature.ts b/src/herumi/signature.ts index cf4fa32..ad0df9f 100644 --- a/src/herumi/signature.ts +++ b/src/herumi/signature.ts @@ -4,13 +4,14 @@ import {getContext} from "./context"; import {PublicKey} from "./publicKey"; 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,7 +19,7 @@ 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(); @@ -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(); diff --git a/test/spec/aggregate.test.ts b/test/spec/aggregate.test.ts index a825e74..f7b6277 100644 --- a/test/spec/aggregate.test.ts +++ b/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, ZeroSignatureError} 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; } },