diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f809e6b24..519bf11bde 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/packages/pointers/CHANGELOG.md b/packages/pointers/CHANGELOG.md index f741117115..bf9623c77c 100644 --- a/packages/pointers/CHANGELOG.md +++ b/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 diff --git a/packages/pointers/src/dereference/index.test.ts b/packages/pointers/src/dereference/index.test.ts index ff0e8f3cd4..b6feb1c83d 100644 --- a/packages/pointers/src/dereference/index.test.ts +++ b/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" }, diff --git a/packages/pointers/src/dereference/process.ts b/packages/pointers/src/dereference/process.ts index 7590e5f83f..fc9d6b7bd7 100644 --- a/packages/pointers/src/dereference/process.ts +++ b/packages/pointers/src/dereference/process.ts @@ -138,19 +138,51 @@ async function* processScope( ): Process { const { define: variableExpressions, in: in_ } = collection; - const allVariables: Record = 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` diff --git a/packages/web/docs/core-schemas/pointers/collections.mdx b/packages/web/docs/core-schemas/pointers/collections.mdx index f68688a9fa..24bc9abb12 100644 --- a/packages/web/docs/core-schemas/pointers/collections.mdx +++ b/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 diff --git a/packages/web/docs/implementation-guides/pointers/dereference-logic/generating-regions.mdx b/packages/web/docs/implementation-guides/pointers/dereference-logic/generating-regions.mdx index 428afb4c29..37cb80559a 100644 --- a/packages/web/docs/implementation-guides/pointers/dereference-logic/generating-regions.mdx +++ b/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. uint256) m` at slot 0: the key's raw bytes followed