From d2e891b5e3eb138ff4f227e7d22f67caf7214508 Mon Sep 17 00:00:00 2001 From: Evan Sangaline Date: Thu, 22 Feb 2024 11:44:41 -0700 Subject: [PATCH] Improve Sindri Manifest lint output by discriminating circuit type When there are Sindri Manifest schema issues, the linting output was previously not as helpful as it could be because issues would be logged across all circuit types. This happened because the validator wasn't able to accurately discriminate the union of different subschemas. This PR manually discriminates the circuit type and modifies the schema that we're validating against to be specifically for the matching subschema. The code is a little verbose/hacky, but the error messages are significantly more helpful so I think the tradeoff is well worth it. Merges #74 --- src/cli/lint.ts | 66 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/cli/lint.ts b/src/cli/lint.ts index bfcdea3..1ff7bf4 100644 --- a/src/cli/lint.ts +++ b/src/cli/lint.ts @@ -1,3 +1,4 @@ +import assert from "assert"; import { existsSync, readFileSync } from "fs"; import path from "path"; import process from "process"; @@ -76,6 +77,71 @@ export const lintCommand = new Command() return process.exit(1); } + // Determine the circuit type and manually discriminate the subschema type to narrow down the + // schema so that the user gets more relevant validation errors. + assert(Array.isArray(sindriManifestJsonSchema.anyOf)); + let subSchema: Schema | undefined; + if (!("circuitType" in sindriJson) || !sindriJson.circuitType) { + subSchema = undefined; + } else if (sindriJson.circuitType === "circom") { + subSchema = sindriManifestJsonSchema.anyOf.find((option: Schema) => + /circom/i.test(option["$ref"] ?? ""), + ); + } else if (sindriJson.circuitType === "gnark") { + subSchema = sindriManifestJsonSchema.anyOf.find((option: Schema) => + /gnark/i.test(option["$ref"] ?? ""), + ); + } else if (sindriJson.circuitType === "halo2") { + if ( + "halo2Version" in sindriJson && + sindriJson.halo2Version === "axiom-v0.2.2" + ) { + subSchema = sindriManifestJsonSchema.anyOf.find((option: Schema) => + /halo2axiomv022/i.test(option["$ref"] ?? ""), + ); + } else if ( + "halo2Version" in sindriJson && + sindriJson.halo2Version === "axiom-v0.3.0" + ) { + subSchema = sindriManifestJsonSchema.anyOf.find((option: Schema) => + /halo2axiomv022/i.test(option["$ref"] ?? ""), + ); + } else { + // We can't discriminate the different halo2 manifests if there's not a valid `halo2Version` + // so we'll just filter down the `anyOf` to the halo2 manifests instead of picking one. + subSchema = { + anyOf: sindriManifestJsonSchema.anyOf.filter((option: Schema) => + /halo2/i.test(option["$ref"] ?? ""), + ), + }; + } + } else if (sindriJson.circuitType === "noir") { + subSchema = sindriManifestJsonSchema.anyOf.find((option: Schema) => + /noir/i.test(option["$ref"] ?? ""), + ); + } + if (subSchema) { + delete sindriManifestJsonSchema.anyOf; + sindriManifestJsonSchema = { ...sindriManifestJsonSchema, ...subSchema }; + } else { + sindri.logger.warn( + `Circuit type is not configured in "${sindriJsonPath}" so some linting steps will be ` + + "skipped and the manifest linting output will be very noisy. Please correct " + + '"circuiType" in "sindri.json" and rerun "sindri lint" to get better linting.', + ); + } + const circuitType: "circom" | "gnark" | "halo2" | "noir" | null = + "circuitType" in sindriJson && + typeof sindriJson.circuitType === "string" && + ["circom", "gnark", "halo2", "noir"].includes(sindriJson.circuitType) + ? (sindriJson.circuitType as "circom" | "gnark" | "halo2" | "noir") + : null; + if (circuitType) { + sindri.logger.debug(`Detected circuit type "${circuitType}".`); + } else { + sindri.logger.debug("No circuit type detected!"); + } + // Validate `sindri.json`. const manifestValidator = new JsonValidator(); const validationStatus = manifestValidator.validate(