format: evaluate a scope's define in dependency order

CHANGELOG.md

@@ -55,6 +55,20 @@ The sections do not signal obligations; the prefixes do.
 
 ### Changed
 
+- The members of a `define` no longer depend on their order, since JSON
+  objects are unordered. A member may use any other member of the same
+  `define`. Inside its own expression, its own name means the variable of the
+  enclosing scope. A cycle of members makes the pointer invalid ([#408]).
+  - Schemas: **ethdebug/format/pointer/collection/scope**
+  - Producers: required: a name of a later member of the same `define` now
+    means that member, not the enclosing scope's variable with that name. Data
+    that used such a name for the outer variable must put the member in a
+    nested scope. Data that uses only earlier members, a member's own name, or
+    other outer names resolves as before.
+  - Consumers: required: "A consumer evaluates each member after the members
+    it uses. If members depend on each other in a cycle, the pointer is
+    invalid."
+
 - Variables now have a stated sort. A list's `each` variable is an integer.
   A `define` variable has the sort of its expression, so a bytes variable
   keeps its width. A template's `expect`ed variable takes the sort of the
@@ -748,3 +762,4 @@ First published version of the specification.
 [#325]: https://github.com/ethdebug/format/pull/325
 [#402]: https://github.com/ethdebug/format/pull/402
 [#404]: https://github.com/ethdebug/format/pull/404
+[#408]: https://github.com/ethdebug/format/pull/408

packages/pointers/CHANGELOG.md

@@ -6,6 +6,13 @@ itself are tracked in the root [`CHANGELOG.md`](../../CHANGELOG.md).
 
 ## Unreleased
 
+### Changed
+
+- `dereference` evaluates the members of a scope's `define` in dependency
+  order, not member order. A member sees the other members by name, and its
+  own name means the variable of the enclosing scope. A cycle of members throws
+  "Circular reference detected" ([#408]).
+
 ### Fixed
 
 - The published type declarations no longer reference Node's `util` types.
@@ -147,3 +154,4 @@ First publication.
 [#317]: https://github.com/ethdebug/format/pull/317
 [#323]: https://github.com/ethdebug/format/pull/323
 [#325]: https://github.com/ethdebug/format/pull/325
+[#408]: https://github.com/ethdebug/format/pull/408

packages/pointers/src/dereference/index.test.ts

@@ -1,5 +1,5 @@
 import { vitest, expect, describe, it, beforeEach } from "vitest";
-import { Pointer } from "@ethdebug/format";
+import { Pointer, schemas } from "@ethdebug/format";
 import { Machine } from "#machine";
 import { Data } from "#data";
 import { dereference } from "./index.js";
@@ -336,6 +336,83 @@ describe("dereference", () => {
     expect(regions[1]).toMatchObject({ slot: Data.fromNumber(2) });
   });
 
+  describe("define member order", () => {
+    // the region of a scope whose `in` is at storage slot "result"
+    const regionOf = async (
+      define: Pointer.Collection.Scope["define"],
+      outer?: Pointer.Collection.Scope["define"],
+    ) => {
+      const scope: Pointer = {
+        define,
+        in: { location: "storage", slot: "result" },
+      };
+      const pointer: Pointer = outer ? { define: outer, in: scope } : scope;
+      const { regions } = await (await dereference(pointer)).view(state);
+      return regions[0];
+    };
+
+    it("evaluates members after the members they use", async () => {
+      expect(
+        await regionOf({ result: { "~sum": ["a", 1] }, a: 5 }),
+      ).toMatchObject({ slot: Data.fromNumber(6) });
+    });
+
+    it("evaluates a longer chain in any order", async () => {
+      expect(
+        await regionOf({
+          result: { "~sum": ["c", 1] },
+          c: { "~product": ["b", { "~sum": ["a", 1] }] },
+          b: { "~sum": ["a", 2] },
+          a: 1,
+        }),
+      ).toMatchObject({ slot: Data.fromNumber(7) });
+    });
+
+    it("binds a member's own name to the enclosing scope", async () => {
+      expect(
+        await regionOf({ result: { "~sum": ["result", 1] } }, { result: 4 }),
+      ).toMatchObject({ slot: Data.fromNumber(5) });
+    });
+
+    it("hides an outer variable from the other members", async () => {
+      expect(
+        await regionOf({ result: { "~sum": ["a", 1] }, a: 5 }, { a: 100 }),
+      ).toMatchObject({ slot: Data.fromNumber(6) });
+    });
+
+    it("resolves the scope schema's out-of-order example", async () => {
+      const { examples } = schemas[
+        "schema:ethdebug/format/pointer/collection/scope"
+      ] as { examples: Pointer[] };
+      const example = examples.find((pointer) =>
+        JSON.stringify(pointer).includes('"end"'),
+      )!;
+      const { regions } = await (await dereference(example)).view(state);
+      expect(regions[0]).toMatchObject({
+        offset: Data.fromNumber(96),
+        length: Data.fromNumber(32),
+      });
+    });
+
+    it("throws on a cycle between members", async () => {
+      await expect(regionOf({ result: "a", a: "b", b: "a" })).rejects.toThrow(
+        "Circular reference detected",
+      );
+    });
+
+    it("throws on a cycle even when outer variables match", async () => {
+      await expect(
+        regionOf({ result: "a", a: "b", b: "a" }, { a: 1, b: 2 }),
+      ).rejects.toThrow("Circular reference detected");
+    });
+
+    it("still throws on an unknown variable", async () => {
+      await expect(regionOf({ result: "a", b: 1 })).rejects.toThrow(
+        "Unknown variable with identifier a",
+      );
+    });
+  });
+
   it("scopes variables defined inside referenced templates", async () => {
     const pointer: Pointer = {
       define: { slot: "0x00" },

packages/pointers/src/dereference/process.ts

@@ -138,19 +138,51 @@ async function* processScope(
 ): Process {
   const { define: variableExpressions, in: in_ } = collection;
 
-  const allVariables: Record<string, Value> = Object.assign(
-    Object.create(null),
-    options.variables,
-  );
-  const newVariables: { [identifier: string]: Value } = {};
-  for (const [identifier, expression] of Object.entries(variableExpressions)) {
-    const value = await evaluate(expression, {
-      ...options,
-      variables: allVariables,
+  const newVariables: { [identifier: string]: Value } = Object.create(null);
+  const isMember = (name: PropertyKey): name is string =>
+    Object.prototype.hasOwnProperty.call(variableExpressions, name);
+
+  // the variables one member sees: a name of another member means that
+  // member, so it must be evaluated first; any other name, the member's
+  // own included, means a variable of the enclosing scope
+  const variablesFor = (identifier: string) =>
+    new Proxy(options.variables, {
+      get(target, name) {
+        if (name === identifier || !isMember(name)) {
+          return target[name as string];
+        }
+        if (name in newVariables) {
+          return newVariables[name];
+        }
+        throw new Error(`Variable not evaluated yet: ${name}`);
+      },
     });
 
-    allVariables[identifier] = value;
-    newVariables[identifier] = value;
+  // evaluate members in dependency order, independent of member order.
+  // retry a member that uses one not evaluated yet; when every remaining
+  // member has failed since the last success, they form a cycle
+  const queue = Object.keys(variableExpressions);
+  let failures = 0;
+  while (queue.length > 0) {
+    const identifier = queue.shift()!;
+    try {
+      newVariables[identifier] = await evaluate(
+        variableExpressions[identifier],
+        { ...options, variables: variablesFor(identifier) },
+      );
+      failures = 0;
+    } catch (error) {
+      if (
+        !(error instanceof Error) ||
+        !error.message.startsWith("Variable not evaluated yet: ")
+      ) {
+        throw error;
+      }
+      if (++failures > queue.length) {
+        throw new Error(`Circular reference detected: ${identifier}`);
+      }
+      queue.push(identifier);
+    }
   }
 
   // variables defined here are visible only within `in`

packages/web/docs/core-schemas/pointers/collections.mdx

@@ -84,10 +84,13 @@ evaluates to a non-zero value.
 
 ## Scope
 
-A **scope** defines variables that can be used in a nested pointer. Variables
-are evaluated in order, so later variables can reference earlier ones. They are
-visible only within `in`: pointers outside the scope, such as later members of
-an enclosing group, do not see them.
+A **scope** defines variables that can be used in a nested pointer. A variable
+can use the other variables of the same `define`, in any order; member order
+has no effect. Inside its own expression, a variable's own name means the outer
+variable with that name, so a variable never depends on itself. A cycle of
+variables makes the pointer invalid. The variables are visible only within
+`in`: pointers outside the scope, such as later members of an enclosing group,
+do not see them.
 
 Scopes help break complex pointer definitions into readable steps. For examples
 combining scopes with keccak256 for storage slot computation, see the

packages/web/docs/implementation-guides/pointers/dereference-logic/generating-regions.mdx

@@ -227,10 +227,12 @@ Finally, the last kind of collection defined by this schema is for defining
 a scope of variables by identifier by specifying the expression values for
 each of those variables.
 
-Since this schema takes advantage of how JSON objects are ordered lists of
-key/value pairs, variables specified later may reference variables specified
-earlier. The only trickiness in implementing `processScope` is ensuring that
-variable values are available immediately.
+JSON objects are unordered, so the members of a `define` may use each other
+in any order. `processScope` evaluates each member after the members it uses.
+Like the fields of a region that refer to each other with `~this`, it queues
+the members and retries one that uses a member not evaluated yet. Each member
+sees the other members by name, but its own name means the variable of the
+enclosing scope. If every remaining member fails in turn, they form a cycle.
 
 <CodeListing
   packageName="@ethdebug/pointers"

schemas/pointer.schema.yaml

@@ -211,6 +211,15 @@ examples:
                 length: ~wordsize
 
               - define:
+                  "total-slots":
+                    # account for both zero and nonzero slot remainders by
+                    # adding ~wordsize-1 to the length before dividing. this
+                    # uses "string-length", defined below: the members of a
+                    # define may appear in any order
+                    ~quotient:
+                      - ~sum: ["string-length", { ~difference: [~wordsize, 1] }]
+                      - ~wordsize
+
                   "string-length":
                     ~quotient:
                       - ~difference:
@@ -221,13 +230,6 @@ examples:
                   "start-slot":
                     ~keccak256:
                       - ~wordsized: "string-storage-contract-variable-slot"
-
-                  "total-slots":
-                    # account for both zero and nonzero slot remainders by adding
-                    # ~wordsize-1 to the length before dividing
-                    ~quotient:
-                      - ~sum: ["string-length", { ~difference: [~wordsize, 1] }]
-                      - ~wordsize
                 in:
                   list:
                     count: "total-slots"

schemas/pointer/collection/scope.schema.yaml

@@ -7,8 +7,18 @@ description: |
   as expressions.
 
   Variables are specified by the `define` field as an object mapping of
-  expression by identifier. Variables are specified **in order**, so that
-  later appearing variables may reference earlier ones in the same object.
+  expression by identifier. The members of one `define` can use each other
+  in any order. Member order has no effect.
+
+  - A name in a variable's expression that matches **another** member of
+    the same `define` means that member.
+  - A name that matches the variable **itself** means the variable with
+    that name in the enclosing scope. So a variable can never depend on
+    itself.
+  - Any other name means a variable of the enclosing scope.
+
+  A consumer evaluates each member after the members it uses. If members
+  depend on each other in a cycle, the pointer is invalid.
 
   Each variable has the sort of the expression that defines it (see
   **ethdebug/format/pointer/expression**). A variable defined by an integer
@@ -19,7 +29,8 @@ description: |
   The variables are visible only within `in`. Pointers outside this scope
   do not see them: later members of an enclosing group, for example, see
   only the variables of their own enclosing scopes. A variable defined here
-  with the same identifier as an outer variable shadows it within `in`.
+  with the same identifier as an outer variable shadows it within `in` and
+  within the expressions of the other members.
 
 type: object
 
@@ -52,6 +63,23 @@ examples:
       offset: example-offset
       length: example-length
 
+  - # example: `end` uses `offset`, a member that appears after it. in its
+    # own expression, `offset` means the outer `offset` (64), so the inner
+    # `offset` is 96 and `end` is 128: the region is 32 bytes at 96.
+    define:
+      offset: 64
+    in:
+      define:
+        end:
+          ~sum: [offset, ~wordsize]
+        offset:
+          ~sum: [offset, 32]
+      in:
+        location: memory
+        offset: offset
+        length:
+          ~difference: [end, offset]
+
   - # example: a variable with a bytes value keeps its width, so it can be
     # hashed directly. this is the storage slot of `m["abc"]` for a Solidity
     # `mapping(string => uint256) m` at slot 0: the key's raw bytes followed