js gov: exceptions as errors (#2382)

python/ccf/proposal_generator.py

@@ -173,7 +173,7 @@ def build_proposal(
 
             for name, body in args.items():
                 vote_lines.append(f"  if (!('{name}' in args)) {{ return false }}")
-                vote_lines.append(f"  let expected = {json.dumps(body)}")
+                vote_lines.append(f"  var expected = {json.dumps(body)}")
                 vote_lines.append(
                     f"  if (args['{name}'] !== expected) {{ return false }}"
                 )

src/runtime_config/actions.js

@@ -5,16 +5,67 @@ class Action {
   }
 }
 
+function checkType(value, type, field) {
+  const optional = type.endsWith("?");
+  if (optional) {
+    if (value === null || value === undefined) {
+      return;
+    }
+    type = type.slice(0, -1);
+  }
+  if (type === "array") {
+    if (!Array.isArray(value)) {
+      throw new Error(`${field} must be an array`);
+    }
+  } else if (type === "integer") {
+    if (!Number.isInteger(value)) {
+      throw new Error(`${field} must be an integer`);
+    }
+  } else if (typeof value !== type) {
+    throw new Error(`${field} must be of type ${type} but is ${typeof value}`);
+  }
+}
+
+function checkEnum(value, members, field) {
+  if (!members.includes(value)) {
+    throw new Error(`${field} must be one of ${members}`);
+  }
+}
+
+function checkBounds(value, low, high, field) {
+  if (low !== null && value < low) {
+    throw new Error(`${field} must be greater than ${low}`);
+  }
+  if (high !== null && value > high) {
+    throw new Error(`${field} must be lower than ${high}`);
+  }
+}
+
+function checkLength(value, min, max, field) {
+  if (min !== null && value.length < min) {
+    throw new Error(`${field} must be an array of minimum ${min} elements`);
+  }
+  if (max !== null && value.length > max) {
+    throw new Error(`${field} must be an array of maximum ${max} elements`);
+  }
+}
+
+function checkX509CertChain(value, field) {
+  if (!ccf.isValidX509Chain(value)) {
+    throw new Error(
+      `${field} must be a valid X509 certificate (chain) in PEM format`
+    );
+  }
+}
+
 const actions = new Map([
   [
     "set_member_data",
     new Action(
       function (args) {
         // Check that member id is a valid entity id?
-        return (
-          typeof args.member_id == "string" &&
-          typeof args.member_data == "object"
-        );
+        checkType(args.member_id, "string", "member_id");
+        checkType(args.member_data, "object", "member_data");
       },
 
       function (args) {
@@ -22,26 +73,21 @@ const actions = new Map([
         let members_info = ccf.kv["public:ccf.gov.members.info"];
         let member_info = members_info.get(member_id);
         if (member_info === undefined) {
-          console.log(`Member ${args.member_id} does not exist`);
-          return false;
+          throw new Error(`Member ${args.member_id} does not exist`);
         }
         let mi = ccf.bufToJsonCompatible(member_info);
         mi.member_data = args.member_data;
         members_info.set(member_id, ccf.jsonCompatibleToBuf(mi));
-        return true;
       }
     ),
   ],
   [
     "rekey_ledger",
     new Action(
-      function (args) {
-        return true; // Check that args is null?
-      },
+      function (args) {},
 
       function (args) {
         ccf.node.rekeyLedger();
-        return true;
       }
     ),
   ],
@@ -49,12 +95,11 @@ const actions = new Map([
     "transition_service_to_open",
     new Action(
       function (args) {
-        return true; // Check that args is null?
+        // Check that args is null?
       },
 
       function (args) {
         ccf.node.transitionServiceToOpen();
-        return true;
       }
     ),
   ],
@@ -62,7 +107,7 @@ const actions = new Map([
     "set_user",
     new Action(
       function (args) {
-        return true; // Check that args is null?
+        // Check that args is null?
       },
 
       function (args) {
@@ -70,16 +115,15 @@ const actions = new Map([
         let raw_user_id = ccf.strToBuf(user_id);
 
         if (ccf.kv["ccf.gov.users.certs"].has(raw_user_id)) {
-          console.log(`User cert for ${user_id} already exists`);
-          return true; // Idempotent
+          return; // Idempotent
         }
 
         ccf.kv["ccf.gov.users.certs"].set(raw_user_id, ccf.strToBuf(args.cert));
 
         if (args.user_data != null) {
           if (ccf.kv["ccf.gov.users.info"].has(raw_user_id)) {
-            console.log(`User info for ${user_id} already exists`);
-            return false; // Internal error
+            throw new Error(`User info for ${user_id} already exists`);
+            // Internal error
           }
 
           ccf.kv["ccf.gov.users.info"].set(
@@ -87,8 +131,6 @@ const actions = new Map([
             ccf.jsonCompatibleToBuf(args.user_data)
           );
         }
-
-        return true;
       }
     ),
   ],
@@ -96,11 +138,8 @@ const actions = new Map([
     "set_recovery_threshold",
     new Action(
       function (args) {
-        return (
-          Number.isInteger(args.threshold) &&
-          args.threshold > 0 &&
-          args.threshold < 255
-        );
+        checkType(args.threshold, "integer", "threshold");
+        checkBounds(args.threshold, 1, 254, "threshold");
       },
       function (args) {}
     ),
@@ -108,72 +147,56 @@ const actions = new Map([
   [
     "always_accept_noop",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_reject_noop",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_accept_with_one_vote",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_reject_with_one_vote",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_accept_if_voted_by_operator",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_accept_if_proposed_by_operator",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_accept_with_two_votes",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
   [
     "always_reject_with_two_votes",
     new Action(
-      function (args) {
-        return true;
-      },
+      function (args) {},
       function (args) {}
     ),
   ],
@@ -181,7 +204,7 @@ const actions = new Map([
     "remove_user",
     new Action(
       function (args) {
-        return typeof args.user_id === "string";
+        checkType(args.user_id, "string", "user_id");
       },
       function (args) {
         const user_id = ccf.strToBuf(args.user_id);
@@ -194,7 +217,7 @@ const actions = new Map([
     "valid_pem",
     new Action(
       function (args) {
-        return ccf.isValidX509Chain(args.pem);
+        checkX509CertChain(args.pem, "pem");
       },
       function (args) {}
     ),

src/runtime_config/constitution.js

@@ -5,8 +5,12 @@ export function validate(input) {
   for (const action of proposal["actions"]) {
     const definition = actions.get(action.name);
     if (definition) {
-      if (!definition.validate(action.args)) {
-        errors.push(`${action.name} at position ${position} failed validation`);
+      try {
+        definition.validate(action.args);
+      } catch (e) {
+        errors.push(
+          `${action.name} at position ${position} failed validation: ${e}\n${e.stack}`
+        );
       }
     } else {
       errors.push(`${action.name}: no such action`);