[internal] Infra for errors and warnings. (#3590)

* Infra for errors and warnings.

* Most allowlist/denylist consistency errors are warnings only.

* Adapt danger to errors+wranings.

Co-authored-by: Catenocrypt <catenocrypt@users.noreply.github.com>

dangerfile.ts

@@ -1,9 +1,10 @@
-import { fail, markdown } from "danger";
+import { fail, warn, markdown } from "danger";
 import { sanityCheckAll } from "./script/action/update-all";
 
-sanityCheckAll().then(errors => {
-    if (errors.length > 0) {
-        errors.forEach(err => fail(err));
-        markdown("Please fix the errors above.  Files can be replaced/renamed in this pull request (using command-line, or GitHub Desktop).  Alternatively, you may close this pull request and open a new one.");
+sanityCheckAll().then(([errors, warnings]) => {
+    errors.forEach(err => fail(err));
+    warnings.forEach(err => warn(err));
+    if (errors.length > 0 || warnings.length > 0 {
+        markdown("Please fix the errors/warnings above.  Files can be replaced/renamed in this pull request (using command-line, or GitHub Desktop).  Alternatively, you may close this pull request and open a new one.");
     }
 });

pricing/coinmarketcap/check.ts

@@ -12,9 +12,9 @@ export function getSanityChecks(): CheckStepInterface[] {
             getName: () => { return "Must have items";},
             check: async () => {
                 if (cmcMap.length == 0) {
-                    return `CMC map must have items`;
+                    return ["CMC map must have items", ""];
                 }
-                return "";
+                return ["", ""];
             }
         },
         {
@@ -30,7 +30,7 @@ export function getSanityChecks(): CheckStepInterface[] {
                         }
                     }
                 });
-                return error;
+                return [error, ""];
             }
         },
         {
@@ -55,7 +55,7 @@ export function getSanityChecks(): CheckStepInterface[] {
     
                     }
                 });
-                return error;
+                return [error, ""];
             }
         },
         {
@@ -72,7 +72,7 @@ export function getSanityChecks(): CheckStepInterface[] {
                         }
                     })
                 });
-                return error;
+                return [error, ""];
             }
         },
         {
@@ -98,7 +98,7 @@ export function getSanityChecks(): CheckStepInterface[] {
                         }
                     }
                 });
-                return error;
+                return [error, ""];
             }
         },
         {
@@ -134,7 +134,7 @@ export function getSanityChecks(): CheckStepInterface[] {
                             break;
                     }
                 });
-                return error;
+                return [error, ""];
             }
         },
         {
@@ -158,7 +158,7 @@ export function getSanityChecks(): CheckStepInterface[] {
                         }
                     }
                 });
-                return error;
+                return [error, ""];
             }
         },
     ];

script/action/allowlists.ts

@@ -2,8 +2,7 @@ import { chainsWithDenylist } from "../common/blockchains";
 import {
     getChainAssetsList,
     getChainAllowlistPath,
-    getChainDenylistPath,
-    getChainPath
+    getChainDenylistPath
 } from "../common/repo-structure";
 import { readFileSync, writeFileSync } from "../common/filesystem";
 import {
@@ -17,8 +16,9 @@ import { formatSortJson } from "../common/json";
 import * as bluebird from "bluebird";
 import { copyFile } from "fs";
 
-async function checkUpdateAllowDenyList(chain: string, checkOnly: boolean ): Promise<[boolean, string]> {
-    let wrongMsg = "";
+async function checkUpdateAllowDenyList(chain: string, checkOnly: boolean ): Promise<[boolean, string, string]> {
+    let errorMsg = "";
+    let warningMsg = "";
     const assets = getChainAssetsList(chain);
 
     const allowlistPath = getChainAllowlistPath(chain);
@@ -31,11 +31,12 @@ async function checkUpdateAllowDenyList(chain: string, checkOnly: boolean ): Pro
 
     const commonElementsOrDuplicates = findCommonElementsOrDuplicates(currentAllowlist, currentDenylist);
     if (commonElementsOrDuplicates && commonElementsOrDuplicates.length > 0) {
-        wrongMsg += `Denylist and allowlist for chain ${chain} should have no common elements or duplicates, found ${commonElementsOrDuplicates.length} ${commonElementsOrDuplicates[0]}\n`;
+        errorMsg += `Denylist and allowlist for chain ${chain} should have no common elements or duplicates, found ${commonElementsOrDuplicates.length} ${commonElementsOrDuplicates[0]}\n`;
     }
     const allowlistOrphan = arrayDiff(currentAllowlist, assets);
     if (allowlistOrphan && allowlistOrphan.length > 0) {
-        wrongMsg += `Allowlist for chain ${chain} contains non-exitent assets, found ${allowlistOrphan.length}, ${allowlistOrphan[0]}\n`;
+        // warning only
+        warningMsg += `Allowlist for chain ${chain} contains non-exitent assets, found ${allowlistOrphan.length}, ${allowlistOrphan[0]}\n`;
     }
 
     const newDeny = makeUnique(currentDenylist.concat(allowlistOrphan));
@@ -45,33 +46,35 @@ async function checkUpdateAllowDenyList(chain: string, checkOnly: boolean ): Pro
 
     const wDiff1 = arrayDiffNocase(newAllow, currentAllowlist);
     if (wDiff1.length > 0) {
-        wrongMsg += `Some elements are missing from allowlist for chain ${chain}: ${wDiff1.length} ${wDiff1[0]}\n`;
+        // warning only
+        warningMsg += `Some elements are missing from allowlist for chain ${chain}: ${wDiff1.length} ${wDiff1[0]}\n`;
     }
     const wDiff2 = arrayDiffNocase(currentAllowlist, newAllow);
     if (wDiff2.length > 0) {
-        wrongMsg += `Some elements should be removed from allowlist for chain ${chain}: ${wDiff2.length} ${wDiff2[0]}\n`;
+        // warning only
+        warningMsg += `Some elements should be removed from allowlist for chain ${chain}: ${wDiff2.length} ${wDiff2[0]}\n`;
     }
 
     const bDiff1 = arrayDiffNocase(newDeny, currentDenylist);
     if (bDiff1.length > 0) {
-        wrongMsg += `Some elements are missing from denylist for chain ${chain}: ${bDiff1.length} ${bDiff1[0]}\n`;
+        warningMsg += `Some elements are missing from denylist for chain ${chain}: ${bDiff1.length} ${bDiff1[0]}\n`;
     }
     const bDiff2 = arrayDiffNocase(currentDenylist, newDeny);
     if (bDiff2.length > 0) {
-        wrongMsg += `Some elements should be removed from denylist for chain ${chain}: ${bDiff2.length} ${bDiff2[0]}\n`;
+        warningMsg += `Some elements should be removed from denylist for chain ${chain}: ${bDiff2.length} ${bDiff2[0]}\n`;
     }
 
     // additionally check for nice formatting, sorting:
     const newAllowText = formatSortJson(newAllow);
     const newDenyText = formatSortJson(newDeny);
     if (newAllowText !== currentAllowlistText) {
-        wrongMsg += `Allowlist for chain ${chain}: not formatted nicely \n`;
+        warningMsg += `Allowlist for chain ${chain}: not formatted nicely \n`;
     }
     if (newDenyText !== currentDenylistText) {
-        wrongMsg += `Denylist for chain ${chain}: not formatted nicely \n`;
+        warningMsg += `Denylist for chain ${chain}: not formatted nicely \n`;
     }
 
-    if (wrongMsg.length > 0) {
+    if (errorMsg.length > 0 || warningMsg.length > 0) {
         // sg wrong, may need to fix
         if (!checkOnly) {
             // update
@@ -80,7 +83,7 @@ async function checkUpdateAllowDenyList(chain: string, checkOnly: boolean ): Pro
             console.log(`Updated allow and denylists for chain ${chain}`);
         }
     }
-    return [(wrongMsg.length == 0), wrongMsg];
+    return [(errorMsg.length == 0 && warningMsg.length == 0), errorMsg, warningMsg];
 }
 
 export class Allowlist implements ActionInterface {
@@ -95,11 +98,11 @@ export class Allowlist implements ActionInterface {
                 {
                     getName: () => { return `Allowlist and denylist for ${chain} should be consistent with assets`},
                     check: async () => {
-                        const [isOK, msg] = await checkUpdateAllowDenyList(chain, true);
+                        const [isOK, errorMsg, warningMsg] = await checkUpdateAllowDenyList(chain, true);
                         if (!isOK) {
-                            return msg;
+                            return [errorMsg, warningMsg];
                         }
-                        return "";
+                        return ["", ""];
                     }
                 }
             );

script/action/binance.ts

@@ -115,7 +115,7 @@ export class BinanceAction implements ActionInterface {
                         }
                     });
                     console.log(`     ${assets.length} assets checked.`);
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/cosmos.ts

@@ -26,7 +26,7 @@ export class CosmosAction implements ActionInterface {
                             error += `Address ${addr} should be in lowercase\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/eth-forks.ts

@@ -97,7 +97,7 @@ export class EthForks implements ActionInterface {
                                 error += infoMsg + "\n";
                             }
                         });
-                        return error;
+                        return [error, ""];
                     }    
                 }
             );

script/action/folders-and-files.ts

@@ -34,7 +34,7 @@ export class FoldersFiles implements ActionInterface {
                             error += `File "${file}" should not be in root or added to predifined list\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
             {
@@ -51,7 +51,7 @@ export class FoldersFiles implements ActionInterface {
                             }
                         });
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
             {
@@ -68,7 +68,7 @@ export class FoldersFiles implements ActionInterface {
                             error += error1 + "\n";
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
             {
@@ -88,7 +88,7 @@ export class FoldersFiles implements ActionInterface {
                             }) ;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/interface.ts

@@ -1,8 +1,8 @@
 // A single check step
 export interface CheckStepInterface {
     getName(): string;
-    // return error or null/empty on success
-    check(): Promise<string>;
+    // return [error, warning], null/"" on success
+    check(): Promise<[string, string]>;
 }
 
 // An action for a check, fix, or update, or a combination.

script/action/json.ts

@@ -23,7 +23,7 @@ export class JsonAction implements ActionInterface {
                             error += `${file} path contains invalid JSON\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/kava.ts

@@ -26,7 +26,7 @@ export class KavaAction implements ActionInterface {
                             error += `Address ${addr} should be in lowercase\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/logo-size.ts

@@ -83,9 +83,9 @@ export class LogoSize implements ActionInterface {
                     const foundChains = readDirSync(chainsPath);
                     var largePath = await checkDownsize(foundChains, true);
                     if (largePath.length > 0) {
-                        return `Found at least one logo that is too large: ${largePath}`;
+                        return [`Found at least one logo that is too large: ${largePath}`, ""];
                     }
-                    return "";
+                    return ["", ""];
                 }
             },
         ];

script/action/terra.ts

@@ -26,7 +26,7 @@ export class TerraAction implements ActionInterface {
                             error += `Address ${addr} should be in lowercase\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/tezos.ts

@@ -90,7 +90,7 @@ export class TezosAction implements ActionInterface {
                             error += `Address ${addr} must be valid Tezos address'\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/action/tron.ts

@@ -37,7 +37,7 @@ export class TronAction implements ActionInterface {
                             error += `Missing file at path '${assetsLogoPath}'\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
             {
@@ -50,7 +50,7 @@ export class TronAction implements ActionInterface {
                             error += `Address ${addr} should be TRC20 address'\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }                
             }
         ];

script/action/update-all.ts

@@ -34,16 +34,22 @@ const actionList: ActionInterface[] = [
     new Coinmarketcap()
 ];
 
-async function checkStepList(steps: CheckStepInterface[]): Promise<string[]> {
+async function checkStepList(steps: CheckStepInterface[]): Promise<[string[], string[]]> {
     var errors: string[] = [];
+    var warnings: string[] = [];
     await bluebird.each(steps, async (step) => {
         try {
             //console.log(`     Running check step '${step.getName()}'...`);
-            const error = await step.check();
+            const [error, warning] = await step.check();
             if (error && error.length > 0) {
                 console.log(`-  ${chalk.red('X')} '${step.getName()}': '${error}'`);
                 errors.push(`${step.getName()}: ${error}`);
-            } else {
+            }
+            if (warning && warning.length > 0) {
+                console.log(`-  ${chalk.yellow('!')} '${step.getName()}': '${warning}'`);
+                warnings.push(`${step.getName()}: ${warning}`);
+            }
+            if (error.length == 0 && warning.length == 0) {
                 console.log(`-  ${chalk.green('✓')} '${step.getName()}' OK`);
             }
         } catch (error) {
@@ -51,22 +57,27 @@ async function checkStepList(steps: CheckStepInterface[]): Promise<string[]> {
             errors.push(`${step.getName()}: Exception: ${error.message}`);
         }
     });
-    return errors;
+    return [errors, warnings];
 }
 
-async function sanityCheckByActionList(actions: ActionInterface[]): Promise<string[]> {
+async function sanityCheckByActionList(actions: ActionInterface[]): Promise<[string[], string[]]> {
     console.log("Running sanity checks...");
     var errors: string[] = [];
+    var warnings: string[] = [];
     await bluebird.each(actions, async (action) => {
         try {
             if (action.getSanityChecks) {
                 const steps = action.getSanityChecks();
                 if (steps && steps.length > 0) {
                     console.log(`   Action '${action.getName()}' has ${steps.length} check steps`);
-                    const errors1 = await checkStepList(steps);
+                    const [errors1, warnings1] = await checkStepList(steps);
                     if (errors1.length > 0) {
                         errors1.forEach(e => errors.push(e));
-                    } else {
+                    }
+                    if (warnings1.length > 0) {
+                        warnings1.forEach(w => warnings.push(w));
+                    }
+                    if (errors1.length == 0 && warnings1.length == 0) {
                         console.log(`- ${chalk.green('✓')} Action '${action.getName()}' OK, all ${steps.length} steps`);
                     }
                 }
@@ -76,23 +87,28 @@ async function sanityCheckByActionList(actions: ActionInterface[]): Promise<stri
             errors.push(`${action.getName()}: Exception: ${error.message}`);
         }
     });
-    console.log(`All sanity checks done, found ${errors.length} errors`);
-    return errors;
+    console.log(`All sanity checks done, found ${errors.length} errors, ${warnings.length} warnings`);
+    return [errors, warnings];
 }
 
-async function consistencyCheckByActionList(actions: ActionInterface[]): Promise<string[]> {
+async function consistencyCheckByActionList(actions: ActionInterface[]): Promise<[string[], string[]]> {
     console.log("Running consistency checks...");
     var errors: string[] = [];
+    var warnings: string[] = [];
     await bluebird.each(actions, async (action) => {
         try {
             if (action.getConsistencyChecks) {
                 const steps = action.getConsistencyChecks();
                 if (steps && steps.length > 0) {
                     console.log(`   Action '${action.getName()}' has ${steps.length} check steps`);
-                    const errors1 = await checkStepList(steps);
+                    const [errors1, warnings1] = await checkStepList(steps);
                     if (errors1.length > 0) {
                         errors1.forEach(e => errors.push(e));
-                    } else {
+                    }
+                    if (warnings1.length > 0) {
+                        warnings1.forEach(w => warnings.push(w));
+                    }
+                    if (errors1.length == 0 && warnings1.length == 0) {
                         console.log(`- ${chalk.green('✓')} Action '${action.getName()}' OK, all ${steps.length} steps`);
                     }
                 }
@@ -102,8 +118,8 @@ async function consistencyCheckByActionList(actions: ActionInterface[]): Promise
             errors.push(`${action.getName()}: Exception: ${error.message}`);
         }
     });
-    console.log(`All consistency checks done, found ${errors.length} errors`);
-    return errors;
+    console.log(`All consistency checks done, found ${errors.length} errors, ${warnings.length} warnings`);
+    return [errors, warnings];
 }
 
 async function sanityFixByList(actions: ActionInterface[]) {
@@ -151,11 +167,11 @@ async function updateByList(actions: ActionInterface[]) {
     console.log("All updates done.");
 }
 
-export async function sanityCheckAll(): Promise<string[]> {
+export async function sanityCheckAll(): Promise<[string[], string[]]> {
     return await sanityCheckByActionList(actionList);
 }
 
-export async function consistencyCheckAll(): Promise<string[]> {
+export async function consistencyCheckAll(): Promise<[string[], string[]]> {
     return await consistencyCheckByActionList(actionList);
 }
 

script/action/validators.ts

@@ -34,14 +34,14 @@ export class Validators implements ActionInterface {
     getName(): string { return "Validators"; }
 
     getSanityChecks(): CheckStepInterface[] {
-        var steps = [
+        var steps: CheckStepInterface[] = [
             {
                 getName: () => { return "Make sure tests added for new staking chain"},
-                check: async () => {
+                check: async (): Promise<[string, string]> => {
                     if (stakingChains.length != 7) {
-                        return `Wrong number of staking chains ${stakingChains.length}`;
+                        return [`Wrong number of staking chains ${stakingChains.length}`, ""];
                     }
-                    return "";
+                    return ["", ""];
                 }
             },
         ];
@@ -52,7 +52,7 @@ export class Validators implements ActionInterface {
                     check: async () => {
                         const validatorsListPath = getChainValidatorsListPath(chain);
                         if (!isValidJSON(validatorsListPath)) {
-                            return `Not valid Json file at path ${validatorsListPath}`;
+                            return [`Not valid Json file at path ${validatorsListPath}`, ""];
                         }
 
                         var error: string = "";
@@ -86,7 +86,7 @@ export class Validators implements ActionInterface {
                             }
                         });
 
-                        return error;
+                        return [error, ""];
                     }
                 }
             );

script/action/waves.ts

@@ -25,7 +25,7 @@ export class WavesAction implements ActionInterface {
                             error += `Address ${addr} should be a Waves address'\n`;
                         }
                     });
-                    return error;
+                    return [error, ""];
                 }
             },
         ];

script/main/check-sanity.ts

@@ -2,7 +2,8 @@ import { sanityCheckAll } from "../action/update-all";
 
 export async function main() {
     try {
-        const errors = await sanityCheckAll();
+        const [errors, warnings] = await sanityCheckAll();
+        // warnings ignored
         process.exit(errors.length);
     } catch(err) {
         console.error(err);

script/main/check.ts

@@ -4,7 +4,8 @@ export async function main() {
     var returnCode: number = 0;
 
     try {
-        const errors1 = await sanityCheckAll();
+        const [errors1, warnings1] = await sanityCheckAll();
+        // warnings ignored
         if (errors1.length > 0) {
             returnCode = errors1.length;
         }
@@ -14,7 +15,8 @@ export async function main() {
     }
 
     try {
-        const errors1 = await consistencyCheckAll();
+        const [errors1, warnings1] = await consistencyCheckAll();
+        // warnings ignored
         if (errors1.length > 0) {
             returnCode = errors1.length;
         }