fix(builder): depends must be specified explicitly when getInput er…

…ror occurs (#866) Co-authored-by: Daniel Beal <git@dbeal.dev> Co-authored-by: dbeal <git@danb.email>
usecannon · Apr 4, 2024 · 5f4a966 · 5f4a966
1 parent af7fdf7
commit 5f4a966
Show file tree

Hide file tree

Showing 18 changed files with 120 additions and 84 deletions.
diff --git a/packages/builder/src/access-recorder.ts b/packages/builder/src/access-recorder.ts
@@ -1,9 +1,6 @@
-import Debug from 'debug';
 import _ from 'lodash';
 import { CannonHelperContext } from './types';
 
-const debug = Debug('cannon:builder:access-recorder');
-
 class ExtendableProxy {
   readonly accessed = new Map<string, AccessRecorder>();
 
@@ -41,10 +38,11 @@ export class AccessRecorder extends ExtendableProxy {
     return acc;
   }
 }
+export type AccessComputationResult = { accesses: string[]; unableToCompute: boolean };
 
-export function computeTemplateAccesses(str?: string) {
+export function computeTemplateAccesses(str?: string): AccessComputationResult {
   if (!str) {
-    return [];
+    return { accesses: [], unableToCompute: false };
   }
   //const recorder = new AccessRecorder();
   const recorders: { [k: string]: AccessRecorder } = {
@@ -64,13 +62,11 @@ export function computeTemplateAccesses(str?: string) {
     },
   });
 
+  let unableToCompute = false;
   try {
     baseTemplate();
   } catch (err) {
-    debug('encountered error while parsing template for dependencies:', err);
-    debug(
-      'usually this happens because the access recorder is getting passed into a function that expects certain types. If you are finding some of your dependencies are not being resolved, please add them manually to `depends`.'
-    );
+    unableToCompute = true;
   }
 
   const accesses: string[] = [];
@@ -79,5 +75,12 @@ export function computeTemplateAccesses(str?: string) {
     accesses.push(...Array.from(recorders[recorder].accessed.keys()).map((a: string) => `${recorder}.${a}`));
   }
 
-  return accesses;
+  return { accesses, unableToCompute };
+}
+
+export function mergeTemplateAccesses(r1: AccessComputationResult, r2: AccessComputationResult): AccessComputationResult {
+  return {
+    accesses: [...r1.accesses, ...r2.accesses],
+    unableToCompute: r1.unableToCompute || r2.unableToCompute,
+  };
 }
diff --git a/packages/builder/src/actions.ts b/packages/builder/src/actions.ts
@@ -10,6 +10,7 @@ import pullSpec from './steps/pull';
 import routerSpec from './steps/router';
 import varSpec from './steps/var';
 import { ChainArtifacts, ChainBuilderContext, ChainBuilderContextWithHelpers, PackageState } from './types';
+import { AccessComputationResult } from './access-recorder';
 
 export interface CannonAction {
   label: string;
@@ -26,7 +27,7 @@ export interface CannonAction {
   /**
    * Returns a list of state keys that this step consumes (used for dependency inference)
    */
-  getInputs?: (config: any, packageState: PackageState) => string[];
+  getInputs?: (config: any, packageState: PackageState) => AccessComputationResult;
 
   /**
    * Returns a list of state keys this step produces (used for dependency inference)

diff --git a/packages/builder/src/builder.test.ts b/packages/builder/src/builder.test.ts
@@ -172,7 +172,7 @@ describe('builder.ts', () => {
       },
     },
   });
-  jest.mocked(deployStep.getInputs).mockReturnValue([]);
+  jest.mocked(deployStep.getInputs).mockReturnValue({ accesses: [], unableToCompute: false });
   jest.mocked(deployStep.getOutputs).mockReturnValue([]);
 
   jest.mocked(invokeStep.getState).mockResolvedValue([{}] as any);
@@ -181,7 +181,7 @@ describe('builder.ts', () => {
       smartFunc: { hash: '0x56785678', events: {}, deployedOn: 'invoke.smartFunc', gasCost: '0', gasUsed: 0, signer: '' },
     },
   });
-  jest.mocked(invokeStep.getInputs).mockReturnValue([]);
+  jest.mocked(invokeStep.getInputs).mockReturnValue({ accesses: [], unableToCompute: false });
   jest.mocked(invokeStep.getOutputs).mockReturnValue([]);
 
   describe('build()', () => {

diff --git a/packages/builder/src/builder.ts b/packages/builder/src/builder.ts
@@ -438,11 +438,18 @@ export function addOutputsToContext(ctx: ChainBuilderContext, outputs: ChainArti
     ctx.settings[n] = outputs.settings[n];
   }
 
+  if (!ctx.extras) {
+    ctx.extras = {};
+  }
+
+  for (const n in outputs.extras) {
+    ctx.extras[n] = outputs.extras[n];
+  }
+
   for (const override in ctx.overrideSettings) {
     ctx.settings[override] = ctx.overrideSettings[override];
   }
 
-  ctx.extras = {};
   for (const n in ctx.settings) {
     ctx.extras[n] = ctx.settings[n];
   }

diff --git a/packages/builder/src/definition.ts b/packages/builder/src/definition.ts
@@ -48,6 +48,7 @@ export function validatePackageVersion(v: string) {
 
 export class ChainDefinition {
   private raw: RawChainDefinition;
+  private sensitiveDependencies: boolean;
 
   readonly allActionNames: string[];
 
@@ -63,9 +64,10 @@ export class ChainDefinition {
   readonly dependencyFor = new Map<string, string>();
   readonly resolvedDependencies = new Map<string, string[]>();
 
-  constructor(def: RawChainDefinition) {
+  constructor(def: RawChainDefinition, sensitiveDependencies = false) {
     debug('begin chain def init');
     this.raw = def;
+    this.sensitiveDependencies = sensitiveDependencies;
 
     const actions = [];
 
@@ -303,8 +305,8 @@ export class ChainDefinition {
       });
 
       throw new Error(`invalid dependency: ${node}. Available "${stepName}" steps:
-        ${stepList.map((dep) => `\n - ${stepName}.${dep}`).join('')}
-      `);
+          ${stepList.map((dep) => `\n - ${stepName}.${dep}`).join('')}
+        `);
     }
 
     const deps = (_.get(this.raw, node)!.depends || []) as string[];
@@ -316,7 +318,22 @@ export class ChainDefinition {
     }
 
     if (ActionKinds[n].getInputs) {
-      for (const input of ActionKinds[n].getInputs!(_.get(this.raw, node), { name: '', version: '', currentLabel: node })) {
+      const accessComputationResults = ActionKinds[n].getInputs!(_.get(this.raw, node), {
+        name: '',
+        version: '',
+        currentLabel: node,
+      });
+
+      // Only throw this error if the user hasn't explicitly defined dependencies
+      if (this.sensitiveDependencies && accessComputationResults.unableToCompute && !_.get(this.raw, node).depends) {
+        throw new Error(
+          `Unable to compute dependencies for [${node}] because of advanced logic in template strings. Specify dependencies manually, like "depends = ['${_.uniq(
+            _.uniq(accessComputationResults.accesses).map((a) => `${this.dependencyFor.get(a)}`)
+          ).join("', '")}']"`
+        );
+      }
+
+      for (const input of accessComputationResults.accesses) {
         debug(`deps: ${node} consumes ${input}`);
         if (this.dependencyFor.has(input)) {
           deps.push(this.dependencyFor.get(input)!);

diff --git a/packages/builder/src/index.ts b/packages/builder/src/index.ts
@@ -1,5 +1,5 @@
 export { createInitialContext, build, getArtifacts, getOutputs } from './builder';
-export { computeTemplateAccesses } from './access-recorder';
+export { computeTemplateAccesses, mergeTemplateAccesses } from './access-recorder';
 export { registerAction } from './actions';
 export type { CannonAction } from './actions';
 export type { RawChainDefinition } from './actions';

diff --git a/packages/builder/src/steps/clone.test.ts b/packages/builder/src/steps/clone.test.ts
@@ -18,7 +18,7 @@ describe('steps/clone.ts', () => {
     jest.mocked(fakeRuntime.derive).mockReturnThis();
 
     jest.mocked(deployAction.getOutputs).mockReturnValue([]);
-    jest.mocked(deployAction.getInputs).mockReturnValue([]);
+    jest.mocked(deployAction.getInputs).mockReturnValue({ accesses: [], unableToCompute: false });
 
     jest.mocked(deployAction.exec).mockResolvedValue({
       contracts: {

diff --git a/packages/builder/src/steps/clone.ts b/packages/builder/src/steps/clone.ts
@@ -2,7 +2,7 @@ import { yellow } from 'chalk';
 import Debug from 'debug';
 import _ from 'lodash';
 import { z } from 'zod';
-import { computeTemplateAccesses } from '../access-recorder';
+import { computeTemplateAccesses, mergeTemplateAccesses } from '../access-recorder';
 import { build, createInitialContext, getOutputs } from '../builder';
 import { CANNON_CHAIN_ID } from '../constants';
 import { ChainDefinition } from '../definition';
@@ -75,18 +75,16 @@ const cloneSpec = {
   },
 
   getInputs(config: Config) {
-    const accesses: string[] = [];
-
-    accesses.push(...computeTemplateAccesses(config.source));
-    accesses.push(...computeTemplateAccesses(config.sourcePreset));
-    accesses.push(...computeTemplateAccesses(config.targetPreset));
+    let accesses = computeTemplateAccesses(config.source);
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.sourcePreset));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.targetPreset));
 
     if (config.options) {
-      _.forEach(config.options, (a) => accesses.push(...computeTemplateAccesses(a)));
+      _.forEach(config.options, (a) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(a))));
     }
 
     if (config.tags) {
-      _.forEach(config.tags, (a) => accesses.push(...computeTemplateAccesses(a)));
+      _.forEach(config.tags, (a) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(a))));
     }
 
     return accesses;

diff --git a/packages/builder/src/steps/deploy.test.ts b/packages/builder/src/steps/deploy.test.ts
@@ -135,7 +135,7 @@ describe('steps/contract.ts', () => {
             args: ['<%= contracts.h %>', '<%= contracts.i %>'],
             salt: '<%= contracts.j %>',
           })
-          .sort()
+          .accesses.sort()
       ).toEqual([
         'contracts.a',
         'contracts.b',

diff --git a/packages/builder/src/steps/deploy.ts b/packages/builder/src/steps/deploy.ts
@@ -2,7 +2,7 @@ import Debug from 'debug';
 import _ from 'lodash';
 import * as viem from 'viem';
 import { z } from 'zod';
-import { computeTemplateAccesses } from '../access-recorder';
+import { computeTemplateAccesses, mergeTemplateAccesses } from '../access-recorder';
 import { deploySchema } from '../schemas';
 import { ensureArachnidCreate2Exists, makeArachnidCreate2Txn, ARACHNID_DEFAULT_DEPLOY_ADDR } from '../create2';
 import {
@@ -191,29 +191,30 @@ const deploySpec = {
   },
 
   getInputs(config: Config) {
-    const accesses: string[] = [];
-
-    accesses.push(...computeTemplateAccesses(config.from));
-    accesses.push(...computeTemplateAccesses(config.nonce));
-    accesses.push(...computeTemplateAccesses(config.artifact));
-    accesses.push(...computeTemplateAccesses(config.value));
-    accesses.push(...computeTemplateAccesses(config.abi));
-    accesses.push(...computeTemplateAccesses(config.salt));
+    let accesses = computeTemplateAccesses(config.from);
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.nonce));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.artifact));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.value));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.abi));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.salt));
 
     if (config.abiOf) {
-      config.abiOf.forEach((v) => accesses.push(...computeTemplateAccesses(v)));
+      _.forEach(config.abiOf, (v) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(v))));
     }
 
     if (config.args) {
-      _.forEach(config.args, (a) => accesses.push(...computeTemplateAccesses(JSON.stringify(a))));
+      _.forEach(
+        config.args,
+        (v) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(JSON.stringify(v))))
+      );
     }
 
     if (config.libraries) {
-      _.forEach(config.libraries, (a) => accesses.push(...computeTemplateAccesses(a)));
+      _.forEach(config.libraries, (v) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(v))));
     }
 
     if (config?.overrides?.gasLimit) {
-      accesses.push(...computeTemplateAccesses(config.overrides.gasLimit));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.overrides.gasLimit));
     }
 
     return accesses;

diff --git a/packages/builder/src/steps/invoke.test.ts b/packages/builder/src/steps/invoke.test.ts
@@ -134,7 +134,7 @@ describe('steps/invoke.ts', () => {
             args: ['<%= contracts.h %>', '<%= contracts.i %>'],
             overrides: { gasLimit: '<%= contracts.j %>' },
           })
-          .sort()
+          .accesses.sort()
       ).toEqual([
         'contracts.a',
         'contracts.b',
@@ -162,7 +162,7 @@ describe('steps/invoke.ts', () => {
           },
           { name: '', version: '', currentLabel: 'invoke.Hello' }
         )
-      ).toEqual(['txns.Hello', 'contracts.something', 'settings.else']);
+      ).toEqual(['txns.Hello', 'contracts.something', 'settings.else', 'extras.else']);
     });
   });
 

diff --git a/packages/builder/src/steps/invoke.ts b/packages/builder/src/steps/invoke.ts
@@ -3,7 +3,7 @@ import * as viem from 'viem';
 import { AbiFunction } from 'viem';
 import _ from 'lodash';
 import { z } from 'zod';
-import { computeTemplateAccesses } from '../access-recorder';
+import { computeTemplateAccesses, mergeTemplateAccesses } from '../access-recorder';
 import { invokeSchema } from '../schemas';
 import {
   CannonSigner,
@@ -401,48 +401,54 @@ const invokeSpec = {
   },
 
   getInputs(config: Config) {
-    const accesses: string[] = [];
+    let accesses = computeTemplateAccesses(config.abi);
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.func));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.from));
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.value));
 
     for (const target of config.target) {
       if (!viem.isAddress(target)) {
         if (target.includes('.')) {
-          accesses.push(`imports.${target.split('.')[0]}`);
+          accesses.accesses.push(`imports.${target.split('.')[0]}`);
         } else {
-          accesses.push(`contracts.${target}`);
+          accesses.accesses.push(`contracts.${target}`);
         }
       }
     }
 
-    accesses.push(...computeTemplateAccesses(config.abi));
-    accesses.push(...computeTemplateAccesses(config.func));
-    accesses.push(...computeTemplateAccesses(config.from));
-    accesses.push(...computeTemplateAccesses(config.value));
-
     if (config.args) {
-      _.forEach(config.args, (a) => accesses.push(...computeTemplateAccesses(JSON.stringify(a))));
+      _.forEach(
+        config.args,
+        (a) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(JSON.stringify(a))))
+      );
     }
 
     if (config.fromCall) {
-      accesses.push(...computeTemplateAccesses(config.fromCall.func));
-      _.forEach(config.fromCall.args, (a) => accesses.push(...computeTemplateAccesses(JSON.stringify(a))));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.fromCall.func));
+
+      _.forEach(
+        config.fromCall.args,
+        (a) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(JSON.stringify(a))))
+      );
     }
 
     if (config?.overrides) {
-      accesses.push(...computeTemplateAccesses(config.overrides.gasLimit));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.overrides.gasLimit));
     }
 
     for (const name in config.factory) {
       const f = config.factory[name];
 
-      accesses.push(...computeTemplateAccesses(f.event));
-      accesses.push(...computeTemplateAccesses(f.artifact));
-      _.forEach(f.abiOf, (a) => accesses.push(...computeTemplateAccesses(a)));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(f.event));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(f.artifact));
+
+      _.forEach(f.abiOf, (a) => (accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(a))));
     }
 
     const varsConfig = config.var || config.extra;
     for (const name in varsConfig) {
       const f = varsConfig[name];
-      accesses.push(...computeTemplateAccesses(f.event));
+      accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(f.event));
     }
 
     return accesses;

diff --git a/packages/builder/src/steps/pull.ts b/packages/builder/src/steps/pull.ts
@@ -1,7 +1,7 @@
 import Debug from 'debug';
 import _ from 'lodash';
 import { z } from 'zod';
-import { computeTemplateAccesses } from '../access-recorder';
+import { computeTemplateAccesses, mergeTemplateAccesses } from '../access-recorder';
 import { getOutputs } from '../builder';
 import { ChainDefinition } from '../definition';
 import { PackageReference } from '../package';
@@ -58,10 +58,8 @@ const pullSpec = {
   },
 
   getInputs(config: Config) {
-    const accesses: string[] = [];
-
-    accesses.push(...computeTemplateAccesses(config.source));
-    accesses.push(...computeTemplateAccesses(config.preset));
+    let accesses = computeTemplateAccesses(config.source);
+    accesses = mergeTemplateAccesses(accesses, computeTemplateAccesses(config.preset));
 
     return accesses;
   },