From 9f5ea258a01315ebb9de9eb0b4be187f6b7dad1d Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:24:22 +0200 Subject: [PATCH 1/4] feat(config): add ReadArg config-local arg kind Add a first-class "read" config-call argument kind so a step argument can be the return value of a view/pure function call on an already-deployed contract (e.g. read token.decimals() and pass it to another step), instead of only a ref or a literal. - steps/types.ts: new ReadArg / ReadCallArg types (config-local, not re-exported from core); ConfigArg becomes RefArg | LiteralArg | ReadArg; ConfigArgExtended gains ReadArg alongside AddressRef. Nested reads are disallowed by construction (ReadCallArg = RefArg | LiteralArg). - steps/schema.ts: readArgSchema / readCallArgSchema (discriminated union without a "read" branch, so nested reads are rejected at the shape level) wired into configArgSchema and configArgExtendedSchema. - steps/validate.ts: MISSING_REF now also covers a read arg's own `contract` and any nested ref inside its `args`; documents that ABI/type-awareness is out of scope for this package (studio's job at authoring time). Issue #140. --- packages/config/src/index.ts | 5 + packages/config/src/steps/index.ts | 4 + packages/config/src/steps/schema.ts | 45 ++++++- packages/config/src/steps/types.ts | 176 +++++++++++++++++++++++--- packages/config/src/steps/validate.ts | 64 +++++++--- 5 files changed, 255 insertions(+), 39 deletions(-) diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index 50143e9..97bf35f 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -6,6 +6,8 @@ export type { LiteralArg, LiteralValue, ConfigArg, + ReadArg, + ReadCallArg, AddressRef, ConfigArgExtended, SetXStep, @@ -19,6 +21,8 @@ export type { export { LITERAL_MAX_DEPTH, configArgSchema, + readArgSchema, + readCallArgSchema, addressRefSchema, configArgExtendedSchema, setXStepSchema, @@ -45,5 +49,6 @@ export type { ApplyConfigResult, ConfigExecutor, ConfigCall, + ReadCall, ResolvedArg, } from "./execute/types.js"; diff --git a/packages/config/src/steps/index.ts b/packages/config/src/steps/index.ts index 550ebc3..75db419 100644 --- a/packages/config/src/steps/index.ts +++ b/packages/config/src/steps/index.ts @@ -8,6 +8,8 @@ export type { LiteralArg, LiteralValue, ConfigArg, + ReadArg, + ReadCallArg, AddressRef, ConfigArgExtended, SetXStep, @@ -20,6 +22,8 @@ export type { export { LITERAL_MAX_DEPTH, configArgSchema, + readArgSchema, + readCallArgSchema, addressRefSchema, configArgExtendedSchema, setXStepSchema, diff --git a/packages/config/src/steps/schema.ts b/packages/config/src/steps/schema.ts index 7404867..0782a0c 100644 --- a/packages/config/src/steps/schema.ts +++ b/packages/config/src/steps/schema.ts @@ -10,7 +10,15 @@ */ import { z } from "zod"; -import type { AddressRef, ConfigArg, ConfigArgExtended, ConfigSpec, ConfigStep, LiteralValue } from "./types.js"; +import type { + AddressRef, + ConfigArg, + ConfigArgExtended, + ConfigSpec, + ConfigStep, + LiteralValue, + ReadCallArg, +} from "./types.js"; // --------------------------------------------------------------------------- // Literal value (re-implemented here to avoid importing private internals from @@ -99,6 +107,34 @@ const literalArgSchema = z.object({ value: literalValueSchema, }); +/** + * ReadCallArg discriminated union — the positional args passed to a `read` + * arg's own view/pure function call. Restricted to `ref | literal`: there is + * intentionally no `read` branch in this union, so a nested `read` inside a + * read-call's `args` is rejected at the shape level (not merely by + * convention). Literal args here still go through `literalArgSchema` / + * `literalValueSchema`, so the LITERAL_MAX_DEPTH guard applies to read-call + * literal args exactly as it does everywhere else. + */ +export const readCallArgSchema: z.ZodType = z.discriminatedUnion("kind", [ + refArgSchema, + literalArgSchema, +]); + +/** + * `{ kind: "read", contract: "", function: "", args?: ReadCallArg[] }` + * + * `contract` is the deploy-id of the SOURCE contract to read FROM (resolved + * to an address like `RefArg.contract`); `function` is the view/pure + * function name to call; `args` are its (optional) positional arguments. + */ +export const readArgSchema = z.object({ + kind: z.literal("read"), + contract: z.string().min(1, { message: "read.contract must be a non-empty string" }), + function: z.string().min(1, { message: "read.function must be a non-empty string" }), + args: z.array(readCallArgSchema).optional(), +}); + /** * ConfigArg discriminated union. * Unknown `kind` values produce a clear parse error. @@ -106,6 +142,7 @@ const literalArgSchema = z.object({ export const configArgSchema: z.ZodType = z.discriminatedUnion("kind", [ refArgSchema, literalArgSchema, + readArgSchema, ]); // --------------------------------------------------------------------------- @@ -129,16 +166,18 @@ export const addressRefSchema: z.ZodType = z.object({ /** * Extended arg schema for studio-internal use — accepts `RefArg`, `LiteralArg`, - * and `AddressRef`. + * `ReadArg`, and `AddressRef`. * * NOTE: this schema is for validating studio-internal arg representations only. * It is NOT used by `configStepSchema` or the execution engine. Any `AddressRef` * parsed with this schema must be normalised to a `RefArg` before being placed - * in a `ConfigSpec` for validation or execution. + * in a `ConfigSpec` for validation or execution. `ReadArg` values need no + * normalisation — they are already part of the validated `ConfigArg` union. */ export const configArgExtendedSchema: z.ZodType = z.discriminatedUnion("kind", [ refArgSchema, literalArgSchema, + readArgSchema, z.object({ kind: z.literal("addressRef"), deployId: z.string().min(1, { message: "addressRef.deployId must be a non-empty string" }), diff --git a/packages/config/src/steps/types.ts b/packages/config/src/steps/types.ts index a0bb8c8..7e6f085 100644 --- a/packages/config/src/steps/types.ts +++ b/packages/config/src/steps/types.ts @@ -7,9 +7,45 @@ * * Argument types: this module reuses `RefArg`, `LiteralArg`, and `LiteralValue` * directly from `@redeploy/core` to avoid duplication and honour the declared - * dependency direction (core → config). The only config-local alias is - * `ConfigArg`, which is the same discriminated union but named to make step - * signatures self-documenting. + * dependency direction (core → config). `ReadArg` is a config-LOCAL addition + * (NOT re-exported from core — core has no notion of post-deployment reads). + * `ConfigArg` is the discriminated union of all three (`RefArg | LiteralArg | + * ReadArg`), named to make step signatures self-documenting. + * + * READ ARGS — reading a value from a deployed contract: + * + * A `ReadArg` (`{ kind: "read", contract, function, args? }`) lets a step + * argument be the return value of a view/pure function call on an already- + * deployed contract, instead of a literal or a plain address ref. Example: + * read `token.decimals()` and pass the result as an arg to another step. + * + * - `contract` is the deploy-id of the SOURCE contract to read FROM + * (resolved to an address exactly like `RefArg.contract`). + * - `function` is the view/pure function name (or canonical signature). + * - `args` are the (optional) positional arguments to the view call, and + * may themselves only be `ref` or `literal` — NOT a nested `read` (v1 + * deliberately disallows nested reads to keep resolution a single hop). + * + * ORDERING: the read's SOURCE contract is always already deployed by the + * time `applyConfig` runs configuration (all deploys precede all config). + * However, if the value being read depends on the EFFECT of a prior config + * step (e.g. a step that writes state the read later observes), the + * reading step must be placed in `orderedSteps` AFTER that configuring + * step — there is no separate dependency graph for config steps; ordering + * is expressed purely via list placement (`steps` vs. `orderedSteps` array + * order). See `ConfigSpec` below for the ordering model. + * + * EXECUTION-TIME SEMANTICS: reads are performed at STEP EXECUTION time (via + * `ConfigExecutor.read`, see execute/types.ts), never at validation time and + * never for a step that is skipped because it is already journaled — so a + * resumed run performs NO reads for already-completed steps. + * + * VALIDATION SCOPE: `@redeploy/config` has NO ABI awareness, so it cannot + * check that `function` is actually a view/pure function or that its + * argument/return types match the target contract. That is enforced by + * studio at authoring time (it has the manifest/ABI). At execution time a + * wrong function name or signature simply fails as a read-only `eth_call` + * that reverts cleanly — no on-chain state is mutated by a bad read. * * Ordered vs. unordered steps: * @@ -27,8 +63,10 @@ * Both lists share the same step-id namespace — duplicate ids across the two * lists are rejected by the validator. Steps in both lists reference deployed * contract addresses via `{ kind: "ref", contract: "" }` in arg - * fields; target / source / into fields are plain deploy-id strings resolved - * to addresses by the execution engine. + * fields, or read a value off a deployed contract via `{ kind: "read", + * contract: "", function: "" }`; target / source / into + * fields are plain deploy-id strings resolved to addresses by the execution + * engine. */ // Re-export the argument types from core so consumers of @redeploy/config do @@ -36,17 +74,81 @@ export type { RefArg, LiteralArg, LiteralValue } from "@redeploy/core"; import type { RefArg, LiteralArg } from "@redeploy/core"; +// --------------------------------------------------------------------------- +// ReadArg — value read from a deployed contract's view/pure function +// --------------------------------------------------------------------------- + +/** + * An argument to a `read`-call (the positional args passed to the view/pure + * function a `ReadArg` invokes). Restricted to `ref | literal` — nested + * `ReadArg`s are NOT allowed in v1, so a read's own resolution is always a + * single on-chain call away from its inputs. + */ +export type ReadCallArg = RefArg | LiteralArg; + /** - * A step argument: either a reference to a deployed contract (resolved to its - * address at execution time) or a literal JSON-serializable value. + * A config-LOCAL argument kind (NOT re-exported from `@redeploy/core`) whose + * value is read from a deployed contract's view/pure function at execution + * time, rather than supplied as a literal or a plain address reference. * - * Intentionally an alias for `ContractArg` from core — using a config-local - * name makes step typings self-documenting without introducing new semantics. + * See the module-level doc comment above ("READ ARGS") for the full + * ordering / execution-time / validation-scope semantics. + * + * @example Read `token.decimals()` and use it elsewhere: + * ```ts + * const arg: ReadArg = { kind: "read", contract: "token", function: "decimals" }; + * ``` + * + * @example Read `registry.lookup("minter")` (a view call with an argument): + * ```ts + * const arg: ReadArg = { + * kind: "read", + * contract: "registry", + * function: "lookup", + * args: [{ kind: "literal", value: "minter" }], + * }; + * ``` + */ +export interface ReadArg { + /** Discriminant — always `"read"`. */ + readonly kind: "read"; + /** + * Deploy-id of the SOURCE contract to read FROM. Resolved to an address at + * execution time exactly like `RefArg.contract`. Must resolve to a known + * deployed contract when a deployment is provided to `validateConfig`. + */ + readonly contract: string; + /** + * Name (or canonical signature) of the view/pure function to call on + * `contract`. Must be a non-empty string. + */ + readonly function: string; + /** + * Positional arguments to the view call. Each is a `ref` (resolved to an + * address) or a `literal` value — never a nested `read`. + */ + readonly args?: ReadCallArg[]; +} + +/** + * A step argument: a reference to a deployed contract (resolved to its + * address at execution time), a literal JSON-serializable value, or a value + * read from a deployed contract's view/pure function. + * + * `RefArg` and `LiteralArg` are re-exports from `@redeploy/core`'s + * `ContractArg` union; `ReadArg` is config-local (core has no notion of + * post-deployment reads). Using a config-local union name makes step typings + * self-documenting without introducing new semantics for the core-derived + * members. * * To pass a deployed contract's address as an argument, use a `RefArg`: * `{ kind: "ref", contract: "" }`. + * + * To pass a value read from a deployed contract, use a `ReadArg`: + * `{ kind: "read", contract: "", function: "" }`. */ -export type ConfigArg = RefArg | LiteralArg; +export type ConfigArg = RefArg | LiteralArg | ReadArg; + // --------------------------------------------------------------------------- // AddressRef — explicit address-of-a-deployed-contract reference @@ -73,7 +175,7 @@ export type ConfigArg = RefArg | LiteralArg; * if (arg.kind === "addressRef") { * return { kind: "ref", contract: arg.deployId }; * } - * return arg; // RefArg or LiteralArg pass through unchanged + * return arg; // RefArg, LiteralArg, or ReadArg pass through unchanged * } * ``` */ @@ -90,19 +192,21 @@ export interface AddressRef { /** * Studio-facing extended argument type that adds `AddressRef` to the - * validated `ConfigArg` union. + * validated `ConfigArg` union (`RefArg | LiteralArg | ReadArg`). * * IMPORTANT — `ConfigArgExtended` is a studio-internal type only. The * validated `ConfigSpec` / `ConfigStep` pipeline (schema, validator, executor) - * only accepts `ConfigArg` (`RefArg | LiteralArg`). Studio components that - * work with `ConfigArgExtended` internally MUST normalise any `AddressRef` - * value to a `RefArg` (`{ kind: "ref", contract: deployId }`) before producing - * a `ConfigSpec` for validation or execution. + * only accepts `ConfigArg` (`RefArg | LiteralArg | ReadArg`). Studio + * components that work with `ConfigArgExtended` internally MUST normalise any + * `AddressRef` value to a `RefArg` (`{ kind: "ref", contract: deployId }`) + * before producing a `ConfigSpec` for validation or execution. `ReadArg` + * values already pass straight through — they are part of the validated + * union and need no normalisation. * * Use `configArgExtendedSchema` to validate studio-internal arg values that * may contain `AddressRef` before that normalisation step. */ -export type ConfigArgExtended = RefArg | LiteralArg | AddressRef; +export type ConfigArgExtended = RefArg | LiteralArg | ReadArg | AddressRef; // --------------------------------------------------------------------------- // ConfigStep variants @@ -132,6 +236,17 @@ export type ConfigArgExtended = RefArg | LiteralArg | AddressRef; * args: [{ kind: "ref", contract: "token" }], * }; * ``` + * + * @example Passing a value read from a deployed contract via ReadArg: + * ```ts + * const step: SetXStep = { + * kind: "setX", + * id: "set-decimals-cache", + * target: "priceOracle", + * function: "setDecimals", + * args: [{ kind: "read", contract: "token", function: "decimals" }], + * }; + * ``` */ export interface SetXStep { /** Discriminant — always `"setX"`. */ @@ -152,12 +267,16 @@ export interface SetXStep { */ readonly function: string; /** - * Positional call arguments. Each is either a ref to a deployed contract - * (resolved to its address) or a literal value. + * Positional call arguments. Each is a ref to a deployed contract (resolved + * to its address), a literal value, or a value read from a deployed + * contract's view/pure function. * * To pass a deployed contract's address as an argument, use a `RefArg`: * `{ kind: "ref", contract: "" }`. * + * To pass a value read from a deployed contract, use a `ReadArg`: + * `{ kind: "read", contract: "", function: "" }`. + * * Studio tooling may use `AddressRef` (`{ kind: "addressRef", deployId }`) * internally; it must normalize to `RefArg` before producing a `ConfigSpec`. */ @@ -177,6 +296,22 @@ export interface SetXStep { * account: { kind: "ref", contract: "minterContract" }, * }; * ``` + * + * @example Granting a role to an address read from another deployed contract: + * ```ts + * const step: GrantRoleStep = { + * kind: "grantRole", + * id: "grant-minter", + * target: "token", + * role: "MINTER_ROLE", + * account: { + * kind: "read", + * contract: "registry", + * function: "lookup", + * args: [{ kind: "literal", value: "minter" }], + * }, + * }; + * ``` */ export interface GrantRoleStep { /** Discriminant — always `"grantRole"`. */ @@ -197,7 +332,8 @@ export interface GrantRoleStep { readonly role: string; /** * The account that receives the role. Can be a ref to a deployed contract - * (resolved to its address) or a literal address string. + * (resolved to its address), a literal address string, or a value read + * from a deployed contract's view/pure function. * * To reference a deployed contract's address, use a `RefArg`: * `{ kind: "ref", contract: "" }`. diff --git a/packages/config/src/steps/validate.ts b/packages/config/src/steps/validate.ts index 8d5b904..16c6b21 100644 --- a/packages/config/src/steps/validate.ts +++ b/packages/config/src/steps/validate.ts @@ -21,8 +21,19 @@ * Ref resolution against a deployment: * When a second argument (`deployment`) is passed, every ref-like field in * the spec (step `target`, `source`, `into`, and any `account` or `args` - * entry of kind `"ref"` or `"addressRef"`) is checked against the set of known - * deployed contract ids. Unknown refs produce a MISSING_REF error. + * entry of kind `"ref"`, `"read"`, or `"addressRef"`) is checked against + * the set of known deployed contract ids. Unknown refs produce a + * MISSING_REF error. For a `"read"` arg, both the read's own `contract` + * (the source being read FROM) and any nested `ref` entries in the read's + * `args` are checked (nested `read` entries are impossible — the schema + * already rejects them, see schema.ts's `readCallArgSchema`). + * + * NOTE on scope: this package has no ABI/manifest awareness, so it cannot + * verify that a `read` arg's `function` actually exists, is view/pure, or + * that its argument/return types match the target contract. That check is + * the responsibility of studio (which has the manifest) at authoring time. + * At execution time, an incorrect `function` name/signature simply fails as + * a read-only `eth_call` that reverts cleanly — see execute/execute.ts. * * Acceptable forms for `deployment`: * - A `DeploymentSpec` (imported from @redeploy/core) — ids are extracted @@ -117,13 +128,6 @@ function toDeployedIdSet(deployment: DeploymentInput): ReadonlySet { return new Set(spec.contracts.map((c) => c.id)); } -/** - * Collect all ref strings from a ConfigArg (returns the contract id if the arg - * is a ref, or nothing if it is a literal). - */ -function refFromArg(arg: ConfigArg): string | undefined { - return arg.kind === "ref" ? arg.contract : undefined; -} // --------------------------------------------------------------------------- // Cross-field validation @@ -218,13 +222,41 @@ function collectStepRefErrors( } /** - * Helper: check a single ConfigArg if it is a ref. + * Helper: check a single ConfigArg if it is a `ref` or a `read`. + * + * - `ref` — checks `arg.contract` at `${argBasePath}.contract`. + * - `read` — checks the read's own `arg.contract` (the source contract + * being read FROM) at the same `${argBasePath}.contract` path, then + * recurses into `arg.args` (each is `ref | literal` — schema-enforced, + * no nested `read` is possible) to check any nested `ref` entries at + * `${argBasePath}.args[k].contract`. + * - `literal` — no ref to check. + * + * `argBasePath` is the arg's own path WITHOUT a `.contract` suffix (e.g. + * `steps[0].args[0]` or `steps[0].account`) so both the top-level ref/read + * check and the nested read-args check can append the right suffix. */ - function checkArgRef(arg: ConfigArg, path: string, label: string): void { - const ref = refFromArg(arg); - if (ref !== undefined) { - checkRef(ref, path, label); + function checkArgRef(arg: ConfigArg, argBasePath: string, label: string): void { + if (arg.kind === "ref") { + checkRef(arg.contract, `${argBasePath}.contract`, label); + return; + } + if (arg.kind === "read") { + checkRef(arg.contract, `${argBasePath}.contract`, `${label} (read source)`); + if (arg.args) { + for (let k = 0; k < arg.args.length; k++) { + const nested = arg.args[k]; + if (nested.kind === "ref") { + checkRef( + nested.contract, + `${argBasePath}.args[${k}].contract`, + `${label} read args[${k}]`, + ); + } + } + } } + // arg.kind === "literal" — nothing to check. } switch (step.kind) { @@ -234,7 +266,7 @@ function collectStepRefErrors( for (let j = 0; j < step.args.length; j++) { checkArgRef( step.args[j], - `${basePath}.args[${j}].contract`, + `${basePath}.args[${j}]`, `setX step "${step.id}" args[${j}]`, ); } @@ -246,7 +278,7 @@ function collectStepRefErrors( checkRef(step.target, `${basePath}.target`, `grantRole step "${step.id}" target`); checkArgRef( step.account, - `${basePath}.account.contract`, + `${basePath}.account`, `grantRole step "${step.id}" account`, ); break; From a514e8f359ddbaf15a9c8e8cd3d947b842f030ae Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:24:28 +0200 Subject: [PATCH 2/4] feat(config): resolve read args via an optional executor.read() hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ConfigExecutor.read?(call: ReadCall): Promise — optional so existing executors (deploy-server, other callers) stay source-compatible. - execute/types.ts: new ReadCall type; ConfigExecutor.read is optional. - execute/errors.ts: new READ_UNSUPPORTED error code, thrown before any on-chain call is attempted when a step has a read arg but the injected executor has no read() method. - execute/execute.ts: resolveArg/buildConfigCall become async to await executor.read() for "read" args (resolving the read's own ref/literal args synchronously first). Reads happen only at step EXECUTION time, inside buildConfigCall, which executeStep calls only for non-skipped steps — so a step already recorded in the journal is skipped before any read is attempted, and a resumed run performs no reads for the steps it skips. Issue #140. --- packages/config/src/execute/errors.ts | 16 ++- packages/config/src/execute/execute.ts | 189 ++++++++++++++++++------- packages/config/src/execute/types.ts | 50 ++++++- 3 files changed, 198 insertions(+), 57 deletions(-) diff --git a/packages/config/src/execute/errors.ts b/packages/config/src/execute/errors.ts index 0500650..1cffd4d 100644 --- a/packages/config/src/execute/errors.ts +++ b/packages/config/src/execute/errors.ts @@ -18,12 +18,20 @@ export type ConfigExecErrorCode = */ | "INVALID_SPEC" /** - * A ref in a step could not be resolved to an address in deployedAddresses. - * This should not normally occur if the spec was validated against the full - * set of deployed ids (which applyConfig() does). It is provided as a typed - * defensive failure for the resolution path at runtime. + * A ref (or a `read` arg's source contract) could not be resolved to an + * address in deployedAddresses. This should not normally occur if the spec + * was validated against the full set of deployed ids (which applyConfig() + * does). It is provided as a typed defensive failure for the resolution + * path at runtime. */ | "UNKNOWN_REF" + /** + * A step's arg tree contains a `read` arg, but the injected `ConfigExecutor` + * does not implement the optional `read()` method. Thrown BEFORE any + * on-chain call is attempted for the step — `executor.execute()` is never + * invoked with unresolved data. + */ + | "READ_UNSUPPORTED" /** * The config-state journal file exists but could not be read or parsed. * The file may be corrupt or inaccessible. The stateDir path is included in diff --git a/packages/config/src/execute/execute.ts b/packages/config/src/execute/execute.ts index 6d82865..06a9b22 100644 --- a/packages/config/src/execute/execute.ts +++ b/packages/config/src/execute/execute.ts @@ -36,15 +36,21 @@ * 2. Validate the spec (fail fast — INVALID_SPEC if any ref is unknown). * 3. Read the journal to learn which steps are already complete (from a * previous run). - * 4. Iterate spec.steps IN ORDER. Steps already in the journal are SKIPPED. + * 4. Iterate spec.steps IN ORDER. Steps already in the journal are SKIPPED + * — the step's args are never resolved and NO read arg is ever invoked + * for a skipped step. * 5. Iterate spec.orderedSteps IN STRICT ARRAY ORDER. Steps already in the - * journal are SKIPPED. Each step waits for the previous to complete. - * 6. For each remaining step (in either list): + * journal are SKIPPED (same no-read guarantee as above). Each step + * waits for the previous to complete. + * 6. For each remaining (non-skipped) step (in either list): * a. Resolve all refs to addresses via the safe Map (throws UNKNOWN_REF * on unknown or prototype-key ids). - * b. Build a resolved ConfigCall. - * c. Call executor.execute(call) and await it. - * d. ONLY on success: append a completion record to the journal. + * b. Resolve any `read` args by calling `executor.read()` (throws + * READ_UNSUPPORTED if the executor has no `read` method) — this is + * the ONLY point at which a read (`eth_call`) is ever performed. + * c. Build a resolved ConfigCall. + * d. Call executor.execute(call) and await it. + * e. ONLY on success: append a completion record to the journal. * 7. If the executor throws, the error propagates immediately. The journal * retains only the steps that completed before the failure. Re-running * with the same stateDir resumes from the first un-journaled step. @@ -54,17 +60,29 @@ * next run. On-chain idempotency of re-execution is out of scope — callers * should design their contracts accordingly. * + * READ ARGS AND ORDERING + * ======================== + * + * The source contract for a `read` arg is always already deployed by the + * time `applyConfig` runs (all deploys precede all config). If the value + * being read depends on the EFFECT of an earlier config step, place the + * reading step in `orderedSteps` AFTER that step — there is no separate + * dependency graph for config steps; ordering is expressed purely by list + * placement (see ORDERED vs. UNORDERED STEPS above). + * * STEP-KIND → ConfigCall MAPPING * ================================ * * setX → target = safeAddresses.get(step.target) * function = step.function - * args = step.args (each ref resolved to its address) + * args = step.args (each ref resolved to its address, each + * read resolved via executor.read()) * * grantRole → target = safeAddresses.get(step.target) * function = "grantRole" * role = step.role - * args = [resolvedAccountAddress] + * args = [resolvedAccountAddress] (account may be a ref, + * literal, or read) * * wire → target = safeAddresses.get(step.into) * function = step.function @@ -78,12 +96,19 @@ import type { ApplyConfigOptions, ApplyConfigResult, ConfigCall, + ConfigExecutor, ResolvedArg, } from "./types.js"; -import type { ConfigSpec, ConfigStep, ConfigArg } from "../steps/types.js"; +import type { ConfigSpec, ConfigStep, ConfigArg, RefArg, LiteralArg } from "../steps/types.js"; // Re-export for convenient public API access from this module -export type { ApplyConfigOptions, ApplyConfigResult, ConfigCall, ResolvedArg } from "./types.js"; +export type { + ApplyConfigOptions, + ApplyConfigResult, + ConfigCall, + ReadCall, + ResolvedArg, +} from "./types.js"; export { ConfigExecError } from "./errors.js"; export type { ConfigExecErrorCode } from "./errors.js"; @@ -92,7 +117,32 @@ export type { ConfigExecErrorCode } from "./errors.js"; // --------------------------------------------------------------------------- /** - * Resolve a single ConfigArg to a ResolvedArg. + * Resolve a named contract id to its deployed address. + * Throws UNKNOWN_REF if not found. + * + * Uses `safeAddresses` (a Map built from own-enumerable entries of + * deployedAddresses) so prototype keys ("constructor", "__proto__", + * "toString", etc.) are never matched and always raise UNKNOWN_REF. + */ +function resolveContractId( + id: string, + safeAddresses: Map, + contextLabel: string, +): string { + const address = safeAddresses.get(id); + if (address === undefined) { + throw new ConfigExecError( + "UNKNOWN_REF", + `${contextLabel}: contract id "${id}" could not be resolved — not found in deployedAddresses`, + ); + } + return address; +} + +/** + * Resolve a `ref | literal` arg (i.e. a `ReadCallArg`, or the `ref`/`literal` + * branches of a `ConfigArg`) to a `ResolvedArg`. Synchronous — neither branch + * needs the executor. * * - Literal args pass through unchanged. * - Ref args are looked up in `safeAddresses` (a Map built from own-enumerable @@ -102,8 +152,8 @@ export type { ConfigExecErrorCode } from "./errors.js"; * "toString" are never present in the Map and therefore always throw * UNKNOWN_REF, even if they somehow bypassed upstream validation. */ -function resolveArg( - arg: ConfigArg, +function resolveSimpleArg( + arg: RefArg | LiteralArg, safeAddresses: Map, contextLabel: string, ): ResolvedArg { @@ -111,60 +161,84 @@ function resolveArg( return arg.value as ResolvedArg; } // arg.kind === "ref" - const address = safeAddresses.get(arg.contract); - if (address === undefined) { - throw new ConfigExecError( - "UNKNOWN_REF", - `${contextLabel}: ref "${arg.contract}" could not be resolved — not found in deployedAddresses`, - ); - } - return address; + return resolveContractId(arg.contract, safeAddresses, contextLabel); } /** - * Resolve a named contract id to its deployed address. - * Throws UNKNOWN_REF if not found. + * Resolve a single ConfigArg (`ref | literal | read`) to a ResolvedArg. * - * Uses `safeAddresses` (a Map built from own-enumerable entries of - * deployedAddresses) so prototype keys ("constructor", "__proto__", - * "toString", etc.) are never matched and always raise UNKNOWN_REF. + * - `ref` / `literal` — delegated to `resolveSimpleArg` (synchronous). + * - `read` — resolves the read's source `contract` to an address, resolves + * its own (ref/literal-only) `args`, then AWAITS `executor.read()`. Throws + * `ConfigExecError("READ_UNSUPPORTED", ...)` if the executor has no `read` + * method — this check happens BEFORE the read is attempted, so a + * read-unsupported executor's `execute()` is never reached with + * unresolved/partial data for this step. + * + * This function is async ONLY because of the `read` branch; `applyConfig` + * only calls it for steps that are actually being executed this run (never + * for steps skipped because they are already journaled), so a resumed run + * performs NO reads for skipped steps. */ -function resolveContractId( - id: string, +async function resolveArg( + arg: ConfigArg, safeAddresses: Map, contextLabel: string, -): string { - const address = safeAddresses.get(id); - if (address === undefined) { + executor: ConfigExecutor, +): Promise { + if (arg.kind !== "read") { + return resolveSimpleArg(arg, safeAddresses, contextLabel); + } + + // arg.kind === "read" + const target = resolveContractId( + arg.contract, + safeAddresses, + `${contextLabel} read source`, + ); + const readArgs: ResolvedArg[] = (arg.args ?? []).map((readArg, i) => + resolveSimpleArg(readArg, safeAddresses, `${contextLabel} read-args[${i}]`), + ); + + if (!executor.read) { throw new ConfigExecError( - "UNKNOWN_REF", - `${contextLabel}: contract id "${id}" could not be resolved — not found in deployedAddresses`, + "READ_UNSUPPORTED", + `${contextLabel}: read arg targets "${arg.contract}"."${arg.function}" but the injected executor does not implement read()`, ); } - return address; + + return executor.read({ target, function: arg.function, args: readArgs }); } /** * Build a resolved ConfigCall from a ConfigStep. * - * All refs in the step are resolved to addresses before this function returns. - * Throws ConfigExecError("UNKNOWN_REF") for any unresolvable ref. + * All refs in the step are resolved to addresses, and all `read` args are + * resolved via `executor.read()`, before this function's promise resolves. + * Throws ConfigExecError("UNKNOWN_REF") for any unresolvable ref, or + * ConfigExecError("READ_UNSUPPORTED") if a `read` arg is present but the + * executor has no `read` method. * * @param safeAddresses - A Map built exclusively from own-enumerable entries of * deployedAddresses (see applyConfig). Using a Map rather than the raw * Record prevents prototype-key mis-resolution. + * @param executor - The injected ConfigExecutor; only needed for its optional + * `read` method (invoked for any `read` arg encountered in the step). */ -function buildConfigCall( +async function buildConfigCall( step: ConfigStep, safeAddresses: Map, -): ConfigCall { + executor: ConfigExecutor, +): Promise { const stepLabel = `step "${step.id}" (${step.kind})`; switch (step.kind) { case "setX": { const target = resolveContractId(step.target, safeAddresses, stepLabel); - const args: ResolvedArg[] = (step.args ?? []).map((arg, i) => - resolveArg(arg, safeAddresses, `${stepLabel} args[${i}]`), + const args: ResolvedArg[] = await Promise.all( + (step.args ?? []).map((arg, i) => + resolveArg(arg, safeAddresses, `${stepLabel} args[${i}]`, executor), + ), ); return { stepId: step.id, @@ -177,10 +251,11 @@ function buildConfigCall( case "grantRole": { const target = resolveContractId(step.target, safeAddresses, stepLabel); - const accountAddress = resolveArg( + const accountAddress = await resolveArg( step.account, safeAddresses, `${stepLabel} account`, + executor, ); return { stepId: step.id, @@ -227,9 +302,12 @@ function buildConfigCall( * validateConfig() (checked against deployedAddresses keys). * @throws ConfigExecError with code "UNKNOWN_REF" if a ref cannot be * resolved during step execution (defensive; normally caught by INVALID_SPEC). + * @throws ConfigExecError with code "READ_UNSUPPORTED" if a step's args + * contain a `read` arg and the injected executor has no `read` method. * @throws ConfigExecError with code "JOURNAL_ERROR" if the journal file * exists but cannot be read. - * @throws any error thrown by executor.execute() — NOT wrapped. + * @throws any error thrown by executor.execute() or executor.read() — NOT + * wrapped. */ export async function applyConfig(options: ApplyConfigOptions): Promise { const { spec, deployedAddresses, executor, stateDir } = options; @@ -268,22 +346,31 @@ export async function applyConfig(options: ApplyConfigOptions): Promise { if (alreadyCompleted.has(step.id)) { - // Already journaled from a previous run — skip. + // Already journaled from a previous run — skip. No ref/read resolution + // and no executor call of any kind happens for a skipped step. skippedStepIds.push(step.id); return; } - // Resolve refs and build the ConfigCall. - // UNKNOWN_REF here is defensive (INVALID_SPEC above should catch missing - // refs), but we keep it for correctness when deployedAddresses diverges, - // and to guard against prototype-key refs that bypassed validation. - const call = buildConfigCall(step, safeAddresses); + // Resolve refs (and any `read` args, via executor.read()) and build the + // ConfigCall. UNKNOWN_REF here is defensive (INVALID_SPEC above should + // catch missing refs), but we keep it for correctness when + // deployedAddresses diverges, and to guard against prototype-key refs + // that bypassed validation. READ_UNSUPPORTED is thrown here if the step + // has a `read` arg but the executor has no `read` method. + const call = await buildConfigCall(step, safeAddresses, executor); // Execute the call. If it throws, the error propagates immediately. // We do NOT catch it — the journal keeps its current state so the next diff --git a/packages/config/src/execute/types.ts b/packages/config/src/execute/types.ts index c4a223b..f78972b 100644 --- a/packages/config/src/execute/types.ts +++ b/packages/config/src/execute/types.ts @@ -60,13 +60,38 @@ export interface ConfigCall { readonly role?: string; /** * Resolved positional arguments for the call. - * - setX: zero or more resolved values (literals + resolved ref addresses). - * - grantRole: exactly one element — the resolved account address. + * - setX: zero or more resolved values (literals + resolved ref addresses + + * resolved read results). + * - grantRole: exactly one element — the resolved account address (which + * may itself be the result of a `read` arg). * - wire: exactly one element — the resolved source contract address. */ readonly args: ResolvedArg[]; } +// --------------------------------------------------------------------------- +// ReadCall — the resolved, chain-agnostic description of a single read arg +// --------------------------------------------------------------------------- + +/** + * A resolved, chain-agnostic description of a single `read` arg (see + * `ReadArg` in ../steps/types.ts) — a view/pure function call whose result is + * used as an argument to a config call. + * + * All contract references have already been resolved to on-chain addresses + * and all positional `args` have already been resolved to `ResolvedArg` + * values (each is `ref | literal` by construction — `ReadArg.args` cannot + * itself contain a nested `read`). + */ +export interface ReadCall { + /** Resolved on-chain address of the contract to read FROM. */ + readonly target: string; + /** Name (or canonical signature) of the view/pure function to call. */ + readonly function: string; + /** Resolved positional arguments to the view call. */ + readonly args: ResolvedArg[]; +} + // --------------------------------------------------------------------------- // ConfigExecutor — injectable abstraction for on-chain calls // --------------------------------------------------------------------------- @@ -93,6 +118,27 @@ export interface ConfigExecutor { * and NOT record the step as complete. */ execute(call: ConfigCall): Promise; + + /** + * Perform a single resolved read-only call (a `read` arg) and return its + * result as a `ResolvedArg`. + * + * OPTIONAL — omitting `read` keeps existing executors (that predate `read` + * args) source-compatible. If a spec contains a `read` arg and the injected + * executor has no `read` method, `applyConfig` throws + * `ConfigExecError("READ_UNSUPPORTED", ...)` before ever attempting the + * call — the executor's `execute` is never invoked with unresolved data. + * + * Called at STEP EXECUTION time only — never during validation, and never + * for a step that is skipped because it is already journaled (resuming a + * completed run performs NO reads for the steps it skips). + * + * @param call - The resolved read description (target address, function + * name, resolved positional args). + * @throws any error if the read (`eth_call`) fails; propagates like + * `execute()` — the step is NOT recorded as complete. + */ + read?(call: ReadCall): Promise; } // --------------------------------------------------------------------------- From 274e5b268e25a3ba41a6bb82afac61c9e482b67e Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:24:37 +0200 Subject: [PATCH 3/4] =?UTF-8?q?test(config):=20cover=20read=20args=20?= =?UTF-8?q?=E2=80=94=20schema,=20resolution,=20resume,=20and=20e2e?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit tests: - schema/validate: valid read args parse; unknown source contract -> MISSING_REF; empty function/contract rejected; nested read inside a read-call's args rejected. - execute: a ReadFakeExecutor's read() result flows into the consuming call's args (both no-arg and ref/literal read-call args); an executor without read() throws READ_UNSUPPORTED cleanly before execute() is ever reached; a read-arg step already journaled is skipped on resume and read() is never invoked for it. E2E (Anvil, gated by isFoundryAvailable()): - ChainConfigExecutor gains a real read() (viem readContract) plus a Registry.register ABI fragment; RecordingExecutor forwards/records reads. - New scenario: register a minter address in Registry (a prior ordered step), then read registry.lookup(key) and use the result as a grantRole.account on Token — proving the on-chain read result (not a hardcoded literal) drives the grant. - New partial/resume scenario: interrupt before the reading step's on-chain tx, resume, and prove idempotency (the registering step never re-runs, and a fully-journaled third run performs zero reads). Issue #140. --- packages/config/test/e2e-anvil.test.ts | 193 +++++++++++++ packages/config/test/execute.test.ts | 265 ++++++++++++++++++ packages/config/test/helpers/chainExecutor.ts | 97 ++++++- packages/config/test/index.test.ts | 226 +++++++++++++++ 4 files changed, 779 insertions(+), 2 deletions(-) diff --git a/packages/config/test/e2e-anvil.test.ts b/packages/config/test/e2e-anvil.test.ts index 59efdef..ba56436 100644 --- a/packages/config/test/e2e-anvil.test.ts +++ b/packages/config/test/e2e-anvil.test.ts @@ -397,4 +397,197 @@ describe.skipIf(!foundryAvailable)("applyConfig — e2e against Anvil", () => { 30_000, ); }); + + // ------------------------------------------------------------------------- + // 4. read args — reading a value from a deployed contract into a later step + // ------------------------------------------------------------------------- + + describe("4. read args — reading a value written by a prior config step", () => { + it( + "reads registry.lookup(key) (set by a prior ordered config step) and uses the result as a grantRole.account", + async () => { + const fixtures = await freshFixtures("read"); + const stateDir = await makeTempDir("state-read"); + const executor = new ChainConfigExecutor(anvil.rpcUrl, ANVIL_DEV_PRIVATE_KEY_0); + + // The reading step ("grant-minter-via-read") depends on the EFFECT of + // "register-minter" (a prior config step), so — per the ordering rule + // documented in steps/types.ts — both live in `orderedSteps`, with the + // reading step placed strictly AFTER the step whose effect it reads. + const spec: ConfigSpec = { + version: 1, + steps: [], + orderedSteps: [ + { + kind: "setX", + id: "register-minter", + target: "registry", + function: "register", + args: [ + { kind: "literal", value: "minter" }, + { kind: "literal", value: ANVIL_DEV_ADDRESS_1 }, + ], + }, + { + kind: "grantRole", + id: "grant-minter-via-read", + target: "token", + role: "MINTER_ROLE", + account: { + kind: "read", + contract: "registry", + function: "lookup", + args: [{ kind: "literal", value: "minter" }], + }, + }, + ], + }; + + const result = await applyConfig({ + spec, + deployedAddresses: fixtures.addresses, + executor, + stateDir, + }); + + expect(result.success).toBe(true); + expect(result.executedStepIds).toEqual(["register-minter", "grant-minter-via-read"]); + + // The role was granted to the address READ from the registry — this + // only passes if the `read` arg's on-chain result (not a hardcoded + // literal) actually drove the grantRole call. + expect( + await reader.hasRole(fixtures.addresses.token, "MINTER_ROLE", ANVIL_DEV_ADDRESS_1), + ).toBe(true); + + expect(await readJournalStepIds(stateDir)).toEqual([ + "register-minter", + "grant-minter-via-read", + ]); + }, + 30_000, + ); + }); + + // ------------------------------------------------------------------------- + // 5. read args — partial + resume (idempotency) + // ------------------------------------------------------------------------- + + describe("5. read args — partial + resume", () => { + it( + "an interrupted run before the reading step's on-chain tx leaves it un-journaled; resuming executes it exactly once and applies the correct read result", + async () => { + const fixtures = await freshFixtures("read-resume"); + const stateDir = await makeTempDir("state-read-resume"); + const chainExecutor = new ChainConfigExecutor(anvil.rpcUrl, ANVIL_DEV_PRIVATE_KEY_0); + const spec: ConfigSpec = { + version: 1, + steps: [], + orderedSteps: [ + { + kind: "setX", + id: "register-minter", + target: "registry", + function: "register", + args: [ + { kind: "literal", value: "minter" }, + { kind: "literal", value: ANVIL_DEV_ADDRESS_1 }, + ], + }, + { + kind: "grantRole", + id: "grant-minter-via-read", + target: "token", + role: "MINTER_ROLE", + account: { + kind: "read", + contract: "registry", + function: "lookup", + args: [{ kind: "literal", value: "minter" }], + }, + }, + ], + }; + + // --- First run: interrupted before the 2nd on-chain tx (the reading + // step's grantRole call). "register-minter" lands on-chain and is + // journaled. The reading step's `read` IS attempted — a `read` arg + // resolves as part of building the ConfigCall, BEFORE executor.execute() + // is invoked (see execute/execute.ts) — but its on-chain tx never lands. + const executor1 = new RecordingExecutor(chainExecutor, 2); + await expect( + applyConfig({ + spec, + deployedAddresses: fixtures.addresses, + executor: executor1, + stateDir, + }), + ).rejects.toThrow("simulated interruption before call #2"); + + expect(executor1.calls.map((c) => c.stepId)).toEqual(["register-minter"]); + expect(executor1.reads).toHaveLength(1); + expect(executor1.reads[0]).toEqual({ + target: fixtures.addresses.registry, + function: "lookup", + args: ["minter"], + }); + + // The grant never landed on-chain. + expect( + await reader.hasRole(fixtures.addresses.token, "MINTER_ROLE", ANVIL_DEV_ADDRESS_1), + ).toBe(false); + expect(await readJournalStepIds(stateDir)).toEqual(["register-minter"]); + + // --- Second run: SAME spec, SAME stateDir — resumes and executes ONLY + // the reading step. "register-minter" is skipped, so Registry.register + // is never invoked a second time (proving idempotency for the + // already-completed step). + const executor2 = new RecordingExecutor(chainExecutor); + const result2 = await applyConfig({ + spec, + deployedAddresses: fixtures.addresses, + executor: executor2, + stateDir, + }); + + expect(result2.success).toBe(true); + expect(result2.skippedStepIds).toEqual(["register-minter"]); + expect(result2.executedStepIds).toEqual(["grant-minter-via-read"]); + expect(executor2.calls.map((c) => c.stepId)).toEqual(["grant-minter-via-read"]); + expect(executor2.reads).toHaveLength(1); + + // The role is now granted, using the (re-resolved) read result. + expect( + await reader.hasRole(fixtures.addresses.token, "MINTER_ROLE", ANVIL_DEV_ADDRESS_1), + ).toBe(true); + + // Idempotency: "register-minter" ran (landed on-chain) exactly once + // across both runs. + const allCallIds = [...executor1.calls, ...executor2.calls].map((c) => c.stepId); + expect(allCallIds.filter((id) => id === "register-minter")).toHaveLength(1); + + expect(await readJournalStepIds(stateDir)).toEqual([ + "register-minter", + "grant-minter-via-read", + ]); + + // --- Third run against the now fully-journaled stateDir: BOTH steps + // are skipped and NO reads are performed at all — proving a resumed, + // fully-complete run never re-invokes read() for steps it skips. + const executor3 = new RecordingExecutor(chainExecutor); + const result3 = await applyConfig({ + spec, + deployedAddresses: fixtures.addresses, + executor: executor3, + stateDir, + }); + expect(result3.success).toBe(true); + expect(result3.executedStepIds).toEqual([]); + expect(result3.skippedStepIds).toEqual(["register-minter", "grant-minter-via-read"]); + expect(executor3.calls).toHaveLength(0); + expect(executor3.reads).toHaveLength(0); + }, + 60_000, + ); + }); }); diff --git a/packages/config/test/execute.test.ts b/packages/config/test/execute.test.ts index b56396b..5da9e70 100644 --- a/packages/config/test/execute.test.ts +++ b/packages/config/test/execute.test.ts @@ -11,6 +11,8 @@ import type { ConfigCall, ConfigExecutor, ApplyConfigOptions, + ReadCall, + ResolvedArg, } from "../src/index.js"; // --------------------------------------------------------------------------- @@ -74,6 +76,31 @@ class FakeExecutor implements ConfigExecutor { } } +/** + * A fake ConfigExecutor that ALSO implements the optional `read()` method — + * used to test the `read` arg resolution path. Records every ReadCall it + * receives (so tests can assert a resumed/skipped run never invokes it) and + * returns a single configurable result for every read. + */ +class ReadFakeExecutor implements ConfigExecutor { + readonly calls: ConfigCall[] = []; + readonly reads: ReadCall[] = []; + private readonly readResult: ResolvedArg; + + constructor(readResult: ResolvedArg) { + this.readResult = readResult; + } + + async execute(call: ConfigCall): Promise { + this.calls.push(call); + } + + async read(call: ReadCall): Promise { + this.reads.push(call); + return this.readResult; + } +} + /** Build ApplyConfigOptions with given spec, executor, and stateDir. */ function makeOptions( spec: ConfigSpec | unknown, @@ -906,3 +933,241 @@ describe("applyConfig — journal malformed-line tolerance", () => { expect(({} as Record)["polluted"]).toBeUndefined(); }); }); + +// --------------------------------------------------------------------------- +// Test: `read` args — resolved via executor.read() +// --------------------------------------------------------------------------- + +describe("applyConfig — read args", () => { + it("resolves a no-arg read via executor.read() and passes the result into the consuming call's args", async () => { + const stateDir = await makeTempDir(); + const readResult = "0x9999999999999999999999999999999999999999"; + const executor = new ReadFakeExecutor(readResult); + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "setX", + id: "set-decimals", + target: "vault", + function: "setDecimalsCache", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }; + + await applyConfig({ + spec, + deployedAddresses: { vault: ADDRESSES.vault, token: ADDRESSES.token }, + executor, + stateDir, + }); + + // The read was performed against the resolved address of "token". + expect(executor.reads).toHaveLength(1); + expect(executor.reads[0]).toEqual({ + target: ADDRESSES.token, + function: "decimals", + args: [], + }); + + // The read's result flowed into the consuming call's args unchanged. + expect(executor.calls).toHaveLength(1); + expect(executor.calls[0].args[0]).toBe(readResult); + }); + + it("resolves a read with ref/literal args and forwards their resolved values to executor.read()", async () => { + const stateDir = await makeTempDir(); + const readResult = ADDRESSES.minterContract; + const executor = new ReadFakeExecutor(readResult); + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "grantRole", + id: "grant-minter", + target: "token", + role: "MINTER_ROLE", + account: { + kind: "read", + contract: "registry", + function: "lookup", + args: [ + { kind: "literal", value: "minter" }, + { kind: "ref", contract: "vault" }, + ], + }, + }, + ], + }; + + await applyConfig({ + spec, + deployedAddresses: { + token: ADDRESSES.token, + registry: ADDRESSES.feeController, + vault: ADDRESSES.vault, + }, + executor, + stateDir, + }); + + expect(executor.reads).toHaveLength(1); + expect(executor.reads[0]).toEqual({ + target: ADDRESSES.feeController, + function: "lookup", + args: ["minter", ADDRESSES.vault], + }); + + expect(executor.calls).toHaveLength(1); + expect(executor.calls[0].args[0]).toBe(readResult); + }); + + it("throws ConfigExecError(READ_UNSUPPORTED) when the executor has no read() and the spec has a read arg", async () => { + const stateDir = await makeTempDir(); + const executor = new FakeExecutor(); // no read() method + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "setX", + id: "set-decimals", + target: "vault", + function: "setDecimalsCache", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }; + + await expect( + applyConfig({ + spec, + deployedAddresses: { vault: ADDRESSES.vault, token: ADDRESSES.token }, + executor, + stateDir, + }), + ).rejects.toSatisfy( + (err: unknown) => err instanceof ConfigExecError && err.code === "READ_UNSUPPORTED", + ); + + // execute() must never have been reached with unresolved/partial data. + expect(executor.calls).toHaveLength(0); + }); + + it("throws ConfigExecError(READ_UNSUPPORTED) for a read arg inside grantRole.account", async () => { + const stateDir = await makeTempDir(); + const executor = new FakeExecutor(); // no read() method + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "grantRole", + id: "grant-minter", + target: "token", + role: "MINTER_ROLE", + account: { kind: "read", contract: "registry", function: "lookup" }, + }, + ], + }; + + await expect( + applyConfig({ + spec, + deployedAddresses: { token: ADDRESSES.token, registry: ADDRESSES.feeController }, + executor, + stateDir, + }), + ).rejects.toSatisfy( + (err: unknown) => err instanceof ConfigExecError && err.code === "READ_UNSUPPORTED", + ); + + expect(executor.calls).toHaveLength(0); + }); + + it("RESUME: a read-arg step already journaled is skipped and executor.read() is NOT invoked", async () => { + const stateDir = await makeTempDir(); + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "setX", + id: "set-decimals", + target: "vault", + function: "setDecimalsCache", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }; + const deployedAddresses = { vault: ADDRESSES.vault, token: ADDRESSES.token }; + + // --- First run: the step executes and performs exactly one read. ------- + const executor1 = new ReadFakeExecutor("18"); + const result1 = await applyConfig({ spec, deployedAddresses, executor: executor1, stateDir }); + expect(result1.executedStepIds).toEqual(["set-decimals"]); + expect(executor1.reads).toHaveLength(1); + + // --- Second run: SAME stateDir — the step is already journaled, so it - + // must be SKIPPED, and executor.read() must NEVER be called. + const executor2 = new ReadFakeExecutor("18"); + const result2 = await applyConfig({ spec, deployedAddresses, executor: executor2, stateDir }); + + expect(result2.executedStepIds).toEqual([]); + expect(result2.skippedStepIds).toEqual(["set-decimals"]); + expect(executor2.reads).toHaveLength(0); + expect(executor2.calls).toHaveLength(0); + }); + + it("RESUME: an interrupted run before a read-arg step does not invoke read() until the resumed run reaches it", async () => { + const stateDir = await makeTempDir(); + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "setX", + id: "set-fee", + target: "feeController", + function: "setFee", + args: [{ kind: "literal", value: 500 }], + }, + { + kind: "setX", + id: "set-decimals", + target: "vault", + function: "setDecimalsCache", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }; + const deployedAddresses = { + feeController: ADDRESSES.feeController, + vault: ADDRESSES.vault, + token: ADDRESSES.token, + }; + + // First run: crash right after "set-fee" (call #1), before "set-decimals" + // is ever reached — its read arg must not be resolved at all. + class CrashAfterFirstCall extends ReadFakeExecutor { + override async execute(call: ConfigCall): Promise { + if (call.stepId === "set-decimals") { + throw new Error("simulated crash before set-decimals"); + } + await super.execute(call); + } + } + const executor1 = new CrashAfterFirstCall("18"); + await expect( + applyConfig({ spec, deployedAddresses, executor: executor1, stateDir }), + ).rejects.toThrow(); + // The read WAS attempted for "set-decimals" (its args resolve before + // execute() runs) but the call itself failed, so nothing was journaled + // for it. + expect(executor1.reads).toHaveLength(1); + + // Resume: "set-fee" is skipped, "set-decimals" runs (and reads) again. + const executor2 = new ReadFakeExecutor("18"); + const result2 = await applyConfig({ spec, deployedAddresses, executor: executor2, stateDir }); + expect(result2.skippedStepIds).toEqual(["set-fee"]); + expect(result2.executedStepIds).toEqual(["set-decimals"]); + expect(executor2.reads).toHaveLength(1); + }); +}); diff --git a/packages/config/test/helpers/chainExecutor.ts b/packages/config/test/helpers/chainExecutor.ts index 4b37630..d73f256 100644 --- a/packages/config/test/helpers/chainExecutor.ts +++ b/packages/config/test/helpers/chainExecutor.ts @@ -7,7 +7,9 @@ * transaction for each `ConfigCall`, used only by this package's Anvil-backed * e2e tests. It is intentionally tiny: it knows how to encode exactly the * handful of setter functions the test fixtures expose (Vault.setFeeBps, - * Vault.setRegistry, Token.grantRole). + * Vault.setRegistry, Registry.register, Token.grantRole), plus the optional + * `read()` method (a real `eth_call` via viem's `readContract`) needed to + * resolve `read` args (e.g. Registry.lookup, Token.decimals). */ import { @@ -23,7 +25,7 @@ import { } from "viem"; import { privateKeyToAccount, type PrivateKeyAccount } from "viem/accounts"; import { foundry } from "viem/chains"; -import type { ConfigCall, ConfigExecutor } from "../../src/index.js"; +import type { ConfigCall, ConfigExecutor, ReadCall, ResolvedArg } from "../../src/index.js"; /** bytes32(0) — OpenZeppelin AccessControl's DEFAULT_ADMIN_ROLE. */ const DEFAULT_ADMIN_ROLE_HASH = `0x${"0".repeat(64)}` as const; @@ -51,6 +53,18 @@ const FUNCTION_ABIS: Record = { outputs: [], }, ], + register: [ + { + type: "function", + name: "register", + stateMutability: "nonpayable", + inputs: [ + { name: "key", type: "string" }, + { name: "addr", type: "address" }, + ], + outputs: [], + }, + ], grantRole: [ { type: "function", @@ -65,6 +79,35 @@ const FUNCTION_ABIS: Record = { ], }; +/** + * Minimal ABI fragments for the VIEW/PURE functions this e2e suite reads via + * `ChainConfigExecutor.read()` (a `read` arg's `function`). Mirrors the + * read-only helper ABIs in test/helpers/chainReader.ts — kept separate + * because chainReader.ts is a test-assertion helper (independent of the + * ConfigExecutor under test), while this map backs the actual `read` arg + * resolution path exercised by applyConfig(). + */ +const READ_FUNCTION_ABIS: Record = { + lookup: [ + { + type: "function", + name: "lookup", + stateMutability: "view", + inputs: [{ name: "key", type: "string" }], + outputs: [{ name: "", type: "address" }], + }, + ], + decimals: [ + { + type: "function", + name: "decimals", + stateMutability: "view", + inputs: [], + outputs: [{ name: "", type: "uint8" }], + }, + ], +}; + /** * Resolve a config-level role mnemonic (e.g. "MINTER_ROLE") to the bytes32 * value OpenZeppelin's AccessControl expects on-chain. Mirrors how the @@ -134,6 +177,36 @@ export class ChainConfigExecutor implements ConfigExecutor { ); } } + + /** + * Perform a real read-only `eth_call` for a resolved `ReadCall` (a `read` + * arg), via viem's `readContract`. Throws if the function has no + * registered read ABI fragment. + * + * bigint results (all Solidity integer types decode to `bigint` in viem) + * are converted to a `ResolvedArg`-compatible `number` when safe, else to + * a decimal string — `ResolvedArg` has no `bigint` member. + */ + async read(call: ReadCall): Promise { + const abi = READ_FUNCTION_ABIS[call.function]; + if (!abi) { + throw new Error( + `ChainConfigExecutor: no read ABI fragment registered for function "${call.function}"`, + ); + } + + const value = await this.publicClient.readContract({ + address: call.target as `0x${string}`, + abi, + functionName: call.function, + args: call.args, + }); + + if (typeof value === "bigint") { + return Number.isSafeInteger(Number(value)) ? Number(value) : value.toString(); + } + return value as ResolvedArg; + } } /** @@ -150,6 +223,8 @@ export class ChainConfigExecutor implements ConfigExecutor { */ export class RecordingExecutor implements ConfigExecutor { readonly calls: ConfigCall[] = []; + /** Every ReadCall forwarded to the delegate's `read()`, in the order received. */ + readonly reads: ReadCall[] = []; private readonly delegate: ConfigExecutor; private readonly throwOnCallNumber: number | undefined; @@ -168,4 +243,22 @@ export class RecordingExecutor implements ConfigExecutor { await this.delegate.execute(call); this.calls.push(call); } + + /** + * Forward a `read` arg's resolved call to the delegate (a real + * `ChainConfigExecutor`) and record it. Reads are never interrupted by + * `throwOnCallNumber` — that only guards `execute()` — so a `read` that + * happens while building the call for a step whose `execute()` is about to + * be interrupted still completes; only the on-chain WRITE is prevented. + * This mirrors applyConfig's own resolution order (reads resolve before + * `executor.execute()` is invoked — see execute/execute.ts). + */ + async read(call: ReadCall): Promise { + if (!this.delegate.read) { + throw new Error("RecordingExecutor: delegate does not implement read()"); + } + const result = await this.delegate.read(call); + this.reads.push(call); + return result; + } } diff --git a/packages/config/test/index.test.ts b/packages/config/test/index.test.ts index c5c1f3d..f1a8589 100644 --- a/packages/config/test/index.test.ts +++ b/packages/config/test/index.test.ts @@ -685,6 +685,232 @@ describe("validateConfig — failure modes", () => { }); }); +// --------------------------------------------------------------------------- +// `read` args +// --------------------------------------------------------------------------- + +describe("validateConfig — read args", () => { + it("accepts a valid no-arg read arg (setX)", () => { + const result = validateConfig( + { + version: 1, + steps: [ + { + kind: "setX", + id: "set-decimals", + target: "priceOracle", + function: "setDecimals", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }, + new Set(["priceOracle", "token"]), + ); + expect(result.ok).toBe(true); + }); + + it("accepts a valid read arg with ref/literal read-call args (grantRole.account)", () => { + const result = validateConfig( + { + version: 1, + steps: [ + { + kind: "grantRole", + id: "grant-minter", + target: "token", + role: "MINTER_ROLE", + account: { + kind: "read", + contract: "registry", + function: "lookup", + args: [ + { kind: "literal", value: "minter" }, + { kind: "ref", contract: "vault" }, + ], + }, + }, + ], + }, + new Set(["token", "registry", "vault"]), + ); + expect(result.ok).toBe(true); + }); + + it("skips ref-resolution for read args when no deployment is provided (shape-only)", () => { + const result = validateConfig({ + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "c", + function: "f", + args: [{ kind: "read", contract: "unknown-contract", function: "decimals" }], + }, + ], + }); + expect(result.ok).toBe(true); + }); + + it("emits MISSING_REF when a read arg's source contract is not in the deployment", () => { + const result = validateConfig( + { + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [{ kind: "read", contract: "noSuchToken", function: "decimals" }], + }, + ], + }, + new Set(["vault"]), + ); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("MISSING_REF"); + const refErr = (result as { ok: false; errors: ConfigError[] }).errors.find( + (e) => e.code === "MISSING_REF", + ); + expect(refErr?.path).toBe("steps[0].args[0].contract"); + }); + + it("emits MISSING_REF when a read arg's source contract is unknown in grantRole.account", () => { + const result = validateConfig( + { + version: 1, + steps: [ + { + kind: "grantRole", + id: "s1", + target: "token", + role: "MINTER", + account: { kind: "read", contract: "noSuchRegistry", function: "lookup" }, + }, + ], + }, + new Set(["token"]), + ); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("MISSING_REF"); + expect(errorPaths(result)).toContain("steps[0].account.contract"); + }); + + it("emits MISSING_REF for a nested ref inside a read arg's own args", () => { + const result = validateConfig( + { + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [ + { + kind: "read", + contract: "registry", + function: "lookup", + args: [{ kind: "ref", contract: "noSuchNestedRef" }], + }, + ], + }, + ], + }, + new Set(["vault", "registry"]), + ); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("MISSING_REF"); + const refErr = (result as { ok: false; errors: ConfigError[] }).errors.find( + (e) => e.code === "MISSING_REF", + ); + expect(refErr?.path).toBe("steps[0].args[0].args[0].contract"); + }); + + it("rejects a read arg with an empty-string function name", () => { + const result = validateConfig({ + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [{ kind: "read", contract: "token", function: "" }], + }, + ], + }); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("INVALID_SHAPE"); + }); + + it("rejects a read arg with an empty-string contract", () => { + const result = validateConfig({ + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [{ kind: "read", contract: "", function: "decimals" }], + }, + ], + }); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("INVALID_SHAPE"); + }); + + it("rejects a nested `read` inside a read arg's own args (no nested reads in v1)", () => { + const result = validateConfig({ + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [ + { + kind: "read", + contract: "registry", + function: "lookup", + args: [{ kind: "read", contract: "token", function: "decimals" }], + }, + ], + }, + ], + }); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("INVALID_SHAPE"); + }); + + it("rejects an unknown arg kind inside a read arg's own args", () => { + const result = validateConfig({ + version: 1, + steps: [ + { + kind: "setX", + id: "s1", + target: "vault", + function: "setDecimals", + args: [ + { + kind: "read", + contract: "registry", + function: "lookup", + args: [{ kind: "addressRef", deployId: "token" }], + }, + ], + }, + ], + }); + expect(result.ok).toBe(false); + expect(errorCodes(result)).toContain("INVALID_SHAPE"); + }); +}); + // --------------------------------------------------------------------------- // Schema exports // --------------------------------------------------------------------------- From 44b11e738333e2af7d66e83c64c9a0b775871c84 Mon Sep 17 00:00:00 2001 From: Roberto Cano <3525807+robercano@users.noreply.github.com> Date: Fri, 17 Jul 2026 11:37:24 +0200 Subject: [PATCH 4/4] fix(verify): handle ReadArg in config-drift arg resolution @redeploy/config's ConfigArg union grew a config-local `read` kind (issue #140), which broke resolveArg()'s fallthrough `return arg.value` (TS2339, since ReadArg has no `.value`). Config-drift verification does not execute on-chain reads to derive expected/actual values, so resolveArg() now switches exhaustively on `arg.kind` and throws a new ConfigVerifyError("UNSUPPORTED_ARG") with a precise message when a `read` arg is encountered, instead of silently mishandling it. Adds UNSUPPORTED_ARG to ConfigVerifyErrorCode and covers the new throw path for grantRole.account, setX read-descriptor expected/args, and wire read-descriptor args. --- packages/verify/src/verify/config-drift.ts | 45 ++++++-- packages/verify/src/verify/config-errors.ts | 9 +- packages/verify/test/config-drift.test.ts | 107 ++++++++++++++++++++ 3 files changed, 151 insertions(+), 10 deletions(-) diff --git a/packages/verify/src/verify/config-drift.ts b/packages/verify/src/verify/config-drift.ts index 54f083e..fc33f44 100644 --- a/packages/verify/src/verify/config-drift.ts +++ b/packages/verify/src/verify/config-drift.ts @@ -371,24 +371,51 @@ export function valuesEqual(expected: unknown, actual: unknown): boolean { * Resolve a ConfigArg to a plain JS value. * - RefArg → deployedAddresses[contract] * - LiteralArg → value + * - ReadArg → NOT supported (throws UNSUPPORTED_ARG); see below. + * + * `ReadArg` (`{ kind: "read", contract, function, args? }`) derives its value + * by calling a view/pure function on a deployed contract at execution time. + * Config-drift verification (this module) only compares *declared* expected + * values against live on-chain state read via `ChainReader`; it does not + * execute an additional on-chain read to resolve a `ReadArg` into a concrete + * value. Supporting this would require wiring a second on-chain call path + * into verify, which is out of scope for this iteration. Callers that used a + * `read` arg when producing/consuming a step must resolve it to a `literal` + * (or `ref`) before passing the spec to verifyConfig(). * * @throws ConfigVerifyError("UNKNOWN_REF") if the contract id is not in * deployedAddresses. + * @throws ConfigVerifyError("UNSUPPORTED_ARG") if `arg.kind === "read"`. */ function resolveArg(arg: ConfigArg, deployedAddresses: Record, context: string): unknown { - if (arg.kind === "ref") { - const address = deployedAddresses[arg.contract]; - if (address === undefined) { + switch (arg.kind) { + case "ref": { + const address = deployedAddresses[arg.contract]; + if (address === undefined) { + throw new ConfigVerifyError( + "UNKNOWN_REF", + `${context}: ref to unknown deployment id "${arg.contract}". ` + + `Known ids: ${Object.keys(deployedAddresses).join(", ") || "(none)"}`, + ); + } + return address; + } + case "read": + throw new ConfigVerifyError( + "UNSUPPORTED_ARG", + `${context}: 'read' args are not supported in config-drift verification yet (arg reads a value ` + + `from a deployed contract). Use a literal or ref, or resolve the read value before verification.`, + ); + case "literal": + return arg.value; + default: { + const exhaustive: never = arg; throw new ConfigVerifyError( - "UNKNOWN_REF", - `${context}: ref to unknown deployment id "${arg.contract}". ` + - `Known ids: ${Object.keys(deployedAddresses).join(", ") || "(none)"}`, + "MALFORMED_SPEC", + `${context}: unknown ConfigArg kind: ${JSON.stringify((exhaustive as ConfigArg).kind)}`, ); } - return address; } - // literal - return arg.value; } /** diff --git a/packages/verify/src/verify/config-errors.ts b/packages/verify/src/verify/config-errors.ts index 7212686..208445c 100644 --- a/packages/verify/src/verify/config-errors.ts +++ b/packages/verify/src/verify/config-errors.ts @@ -32,7 +32,14 @@ export type ConfigVerifyErrorCode = * The ConfigSpec itself is malformed: empty step id, missing required fields, * or an unknown step kind. */ - | "MALFORMED_SPEC"; + | "MALFORMED_SPEC" + /** + * A ConfigArg of a kind that config-drift verification does not (yet) + * support was encountered — currently this is `ReadArg` (`{ kind: "read" }`). + * Config-drift verification only resolves `ref` and `literal` args; it does + * not execute on-chain reads to derive expected/actual values. + */ + | "UNSUPPORTED_ARG"; /** * Thrown by verifyConfig() for setup and usage errors that prevent drift diff --git a/packages/verify/test/config-drift.test.ts b/packages/verify/test/config-drift.test.ts index 8fe5361..63e4fc9 100644 --- a/packages/verify/test/config-drift.test.ts +++ b/packages/verify/test/config-drift.test.ts @@ -860,6 +860,113 @@ describe("verifyConfig() — setup errors (ConfigVerifyError thrown)", () => { }); }); +// --------------------------------------------------------------------------- +// ReadArg — not supported in config-drift verification (UNSUPPORTED_ARG) +// --------------------------------------------------------------------------- + +describe("verifyConfig() — 'read' ConfigArg is not supported (UNSUPPORTED_ARG)", () => { + // A ReadArg derives its value by calling a view/pure function on a deployed + // contract. Config-drift verification only resolves `ref` and `literal` + // args, so any `read` arg encountered while resolving expected/args values + // must throw ConfigVerifyError("UNSUPPORTED_ARG") rather than being + // silently mishandled (e.g. returning `undefined` for a missing `.value`). + + it("throws UNSUPPORTED_ARG when a grantRole step's account is a read arg", async () => { + const spec: ConfigSpec = { + version: 1, + steps: [ + { + kind: "grantRole", + id: "grant-minter", + target: "token", + role: "MINTER_ROLE", + account: { kind: "read", contract: "minterContract", function: "owner" }, + }, + ], + }; + + await expect( + verifyConfig({ spec, deployedAddresses, reader: makeMockReader({}) }), + ).rejects.toSatisfy((err: unknown) => { + return ( + err instanceof ConfigVerifyError && + err.code === "UNSUPPORTED_ARG" && + /read.*not supported/i.test(err.message) + ); + }); + }); + + it("throws UNSUPPORTED_ARG when a setX read descriptor's 'expected' is a read arg", async () => { + const spec: ConfigSpec = { + version: 1, + steps: [{ kind: "setX", id: "set-fee", target: "feeController", function: "setFee" }], + }; + + await expect( + verifyConfig({ + spec, + deployedAddresses, + reader: makeMockReader({}), + reads: { + "set-fee": { + function: "getFee", + expected: { kind: "read", contract: "feeController", function: "defaultFee" }, + }, + }, + }), + ).rejects.toSatisfy((err: unknown) => { + return err instanceof ConfigVerifyError && err.code === "UNSUPPORTED_ARG"; + }); + }); + + it("throws UNSUPPORTED_ARG when a setX read descriptor's 'args' contains a read arg", async () => { + const spec: ConfigSpec = { + version: 1, + steps: [{ kind: "setX", id: "set-fee", target: "feeController", function: "setFee" }], + }; + + await expect( + verifyConfig({ + spec, + deployedAddresses, + reader: makeMockReader({}), + reads: { + "set-fee": { + function: "getFee", + args: [{ kind: "read", contract: "vault", function: "id" }], + expected: { kind: "literal", value: 500 }, + }, + }, + }), + ).rejects.toSatisfy((err: unknown) => { + return err instanceof ConfigVerifyError && err.code === "UNSUPPORTED_ARG"; + }); + }); + + it("throws UNSUPPORTED_ARG when a wire read descriptor's 'args' contains a read arg", async () => { + const spec: ConfigSpec = { + version: 1, + steps: [{ kind: "wire", id: "wire-token", source: "token", into: "vault", function: "setToken" }], + }; + + await expect( + verifyConfig({ + spec, + deployedAddresses, + reader: makeMockReader({}), + reads: { + "wire-token": { + function: "getToken", + args: [{ kind: "read", contract: "vault", function: "id" }], + }, + }, + }), + ).rejects.toSatisfy((err: unknown) => { + return err instanceof ConfigVerifyError && err.code === "UNSUPPORTED_ARG"; + }); + }); +}); + // --------------------------------------------------------------------------- // grantRole — detailed verification // ---------------------------------------------------------------------------