Mixmix/is valid config (#322)

* config schema - initial tests, json schema

* isValidConfig: tests + passing

* plug isValidConfig into assertConfig

* plug assertConfig into config.init

* fix tests: config.set

* split config tests into files

* config migration 5 - fix selectedAccount

* rm test.only

* fix config.set test post-migration

* add globstar to support recurssive test search

* fix migration 5 (code + tests)

* new account setting selected account incorrectly, now fixed

---------

Co-authored-by: Nayyir Jutha <[email protected]>

dev/bin/test-ts.sh

@@ -1,11 +1,14 @@
 #! /usr/bin/env bash
 
 # HACK: normally we could just go:
-#   tape tests/*.test.ts
+#   tape tests/**/*.test.ts
 #
 # but here we are fighting TS ... this works well enough
 
-ONLY_FILES=`grep 'test.only' tests/*.test.ts -l`
+shopt -s globstar
+# required for "globalstar" (**)in bash
+
+ONLY_FILES=`grep 'test.only' tests/**/*.test.ts -l`
 
 if [ $ONLY_FILES ]; then
   # If there are files with test.only, run only those files
@@ -17,7 +20,7 @@ if [ $ONLY_FILES ]; then
 else
   # Otherwise run all tests
   set -e;
-  for t in tests/*.test.ts; do
+  for t in tests/**/*.test.ts; do
     npx tsx $t;
   done
 fi

package.json

@@ -49,6 +49,7 @@
   "homepage": "https://github.com/entropyxyz/cli#readme",
   "dependencies": {
     "@entropyxyz/sdk": "0.4.0",
+    "ajv": "^8.17.1",
     "commander": "^12.1.0",
     "env-paths": "^3.0.0",
     "inquirer": "8.0.0",

src/account/utils.ts

@@ -24,7 +24,7 @@ export async function selectAndPersistNewAccount (configPath: string, newAccount
   accounts.push(newAccount)
   await config.set(configPath, {
     ...storedConfig,
-    selectedAccount: newAccount.address
+    selectedAccount: newAccount.name
   })
 }
 

src/config/index.ts

@@ -2,10 +2,12 @@ import { readFile, writeFile, rm } from 'node:fs/promises'
 import { mkdirp } from 'mkdirp'
 import { join, dirname } from 'path'
 import envPaths from 'env-paths'
+import AJV from 'ajv'
 
 import allMigrations from './migrations'
 import { serialize, deserialize } from './encoding'
 import { EntropyConfig, EntropyConfigAccount } from './types'
+import { configSchema } from './schema'
 
 const paths = envPaths('entropy-cryptography', { suffix: '' })
 const OLD_CONFIG_PATH = join(process.env.HOME, '.entropy-cli.config')
@@ -51,8 +53,14 @@ export async function init (configPath: string, oldConfigPath = OLD_CONFIG_PATH)
   const newConfig = migrateData(allMigrations, currentConfig)
 
   if (newConfig[VERSION] !== currentConfig[VERSION]) {
+    // a migration happened, write updated config
+    // "set" checks the format of the config for us
     await set(configPath, newConfig)
   }
+  else {
+    // make sure the config the app is about to run on is safe
+    assertConfig(newConfig)
+  }
 }
 
 export async function get (configPath) {
@@ -84,38 +92,87 @@ export async function setSelectedAccount (configPath: string, account: EntropyCo
 
 /* util */
 function noop () {}
-function assertConfig (config: any) {
-  if (
-    !config ||
-    typeof config !== 'object'
-  ) {
-    throw Error('Config#set: config must be an object')
-  }
 
-  if (!Array.isArray(config.accounts)) {
-    throw Error('Config#set: config must have "accounts"')
-  }
+export function assertConfig (config: any) {
+  if (isValidConfig(config)) return
 
-  if (!config.endpoints) {
-    throw Error('Config#set: config must have "endpoints"')
-  }
+  // @ts-expect-error this is valid Node...
+  throw new Error('Invalid config', {
+    cause: isValidConfig.errors
+      .map(err => {
+        return err.instancePath
+          ? `config${err.instancePath}: ${err.message}`
+          : err.message
+      })
+      .join("; ")
+  })
 
-  if (typeof config.selectedAccount !== 'string') {
-    throw Error('Config#set: config must have "selectedAccount"')
-  }
-
-  if (typeof config['migration-version'] !== 'number') {
-    throw Error('Config#set: config must have "migration-version"')
-  }
 }
+
 function assertConfigPath (configPath: string) {
   if (!configPath.endsWith('.json')) {
     throw Error(`configPath must be of form *.json, got ${configPath}`)
   }
 }
+
 export function isDangerousReadError (err: any) {
   // file not found:
   if (err.code === 'ENOENT') return false
 
   return true
 }
+
+const ajv = new AJV({
+  allErrors: true,
+})
+
+let validator
+export const isValidConfig: ValidatorFunction = function (input: any) {
+  if (!validator) validator = ajv.compile(configSchema)
+  // lazy compile once, it's slowish (~20ms)
+
+  const generalResult = validator(input)
+  const selectedAccountResult = isValidSelectedAccount(input)
+
+  const isValid = generalResult && selectedAccountResult
+
+  isValidConfig.errors = isValid
+    ? null
+    : [
+      ...(validator.errors || []),
+      ...(isValidSelectedAccount.errors || [])
+    ]
+
+  return isValid
+}
+
+const isValidSelectedAccount: ValidatorFunction = function (input: any) {
+  if (input?.selectedAccount === null) {
+    isValidSelectedAccount.errors = null
+    return true
+  }
+
+  if (!input?.selectedAccount || !Array.isArray(input?.accounts)) {
+    isValidSelectedAccount.errors = [{
+      message: 'unable to check "selectedAccount" validity'
+    }]
+    return false
+  }
+
+  const isValid = input.accounts.find(acct => acct.name === input.selectedAccount)
+
+  isValidSelectedAccount.errors = isValid
+    ? null
+    : [{ message: `config/selectedAccount: "${input.selectedAccount}" "no account had a "name" matching "selectedAccount": ` }]
+
+  return isValid
+}
+
+type ValidatorFunction = {
+  errors?: null|ValidatorErrorObject[]
+  (input: any): boolean
+}
+interface ValidatorErrorObject {
+  instancePath?: string
+  message: string
+}

src/config/migrations/05.ts

@@ -0,0 +1,39 @@
+// The purpose of this migration is to:
+// 1. change encoding of empty selectedAccount from "" => null
+// 2. ensure selectedAccount present is an account.name
+
+export const version = 5
+
+export function migrate (data) {
+  try {
+    const newData = { ...data }
+
+    if (newData.selectedAccount === null) {
+      // nothing to do
+    }
+    else if (newData.selectedAccount === "") {
+      newData.selectedAccount = null
+    }
+    else {
+      const target = newData.selectedAccount
+      const accountMatchingName = newData.accounts.find(a => a.name === target)
+      const accountMatchingAddress = newData.accounts.find(a => a.address === target)
+
+      if (accountMatchingName) {
+        // nothing to do
+      }
+      else if (accountMatchingAddress) {
+        // change the refference to be the account.name
+        newData.selectedAccount = accountMatchingAddress.name
+      }
+      else {
+        throw Error(`unable to correct selectedAccount - no account found which matches "${target}"`)
+      }
+    }
+    return newData
+  } catch (err) {
+    // @ts-expect-error : ts stupid
+    throw Error('Migration 5 failed', { cause: err })
+  }
+
+}

src/config/migrations/index.ts

@@ -3,13 +3,15 @@ import * as migration01 from './01'
 import * as migration02 from './02'
 import * as migration03 from './03'
 import * as migration04 from './04'
+import * as migration05 from './05'
 
 export const migrations = [
   migration00,
   migration01,
   migration02,
   migration03,
-  migration04
+  migration04,
+  migration05,
 ]
 
 export default migrations

src/config/schema.ts

@@ -0,0 +1,69 @@
+import migrations from './migrations'
+
+const currentVersion = migrations.at(-1).version
+
+const accountSchema = {
+  type: "object",
+  properties: {
+    name: {
+      type: "string"
+    },
+    address: {
+      type: "string",
+      pattern: [
+        "^[",
+        "a-k", /* l, */ "m-z",
+        "A-H", /* I, */ "J-N", /* O, */ "P-Z",
+        /* 0 */ "1-9",
+        "]{48,48}$"
+      ].join("")
+      // base58: https://en.wikipedia.org/wiki/Binary-to-text_encoding#Encoding_standards
+      //
+      // Similar to Base64, but modified to avoid both non-alphanumeric characters (+ and /) and letters
+      // that might look ambiguous when printed (0 – zero, I – capital i, O – capital o and l – lower-case L).
+    },
+    data: {
+      type: "object",
+      properties: {
+        admin: { type: "object" },
+        registration: { type: "object" }
+      },
+      required: ["admin", "registration"]
+    }
+  },
+  required: ["name", "address", "data"]
+}
+
+export const configSchema = {
+  type: "object",
+  properties: {
+    accounts: {
+      type: "array",
+      items: accountSchema,
+      uniqueItems: true
+    },
+    selectedAccount: {
+      oneOf: [
+        { type: "null" },
+        { type: "string" }
+      ]
+    },
+    endpoints: {
+      type: "object",
+      patternProperties: {
+        "^\\w+$": {
+          type: "string",
+          pattern: "^wss?://"
+        },
+      },
+      required: ["test-net"]
+    },
+    "migration-version": {
+      type: "integer",
+      minimum: currentVersion,
+      maximum: currentVersion
+    }
+  },
+  required: ["accounts", "selectedAccount", "endpoints", "migration-version"]
+}
+

src/config/types.ts

@@ -6,7 +6,7 @@ export interface EntropyConfig {
   }
   // selectedAccount is account.name (alias) for the account
   selectedAccount: string
-  'migration-version': string
+  'migration-version': number
 }
 
 export interface EntropyConfigAccount {