From feccc666b3d7a6c07c98e1272d57345435d2dbd3 Mon Sep 17 00:00:00 2001 From: halaprix Date: Thu, 23 Jul 2026 20:41:30 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20pinBlock=20=E2=80=94=20run-start=20?= =?UTF-8?q?block=20resolution=20with=20onPin=20(F8)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fill the two seams T9 reserved in src/core/internal.ts: - validatePinCapability(options, executor): pre-consumption. Throws when pinBlock is requested against an executor lacking getBlockNumber (always, regardless of block shape — a predictable capability contract), and when block.blockTag is 'pending' (no stable number). - resolvePinnedBlock(options, executor): post-consumption, pre-execute. No-op when pinBlock is false. When true: resolves an absent block or a latest/safe/finalized tag via one executor.getBlockNumber() call (+1 RT) to a concrete blockNumber used for every step; an explicit blockNumber/blockHash is used as-is with no extra RPC. Reports the resolved PinnedBlock to onPin synchronously, exactly once, before any multicall batch is dispatched; a throwing onPin rejects the run with tasks left consumed. Adds StepExecutor.getBlockNumber (optional, additive) and its Eip1193Executor implementation (eth_getBlockByNumber; blockNumber short-circuits with no RPC; blockHash throws — pinning never calls it for hashes). Adds BatchOptions.pinBlock/onPin and the PinnedBlock type, exported from index.ts. Both runners now thread the resolved block through their executeBatch policy instead of options.block directly. Docs: new "Atomicity" section in docs/api-reference.md (within-batch vs cross-step consistency, the resolution/no-op mapping table, and a pinBlock + Presets.throughput snippet); API Surface entries updated for the new fields/method/type. README bundle-size badge refreshed for the measured gzip delta. --- README.md | 2 +- docs/api-reference.md | 37 ++- src/__tests__/pinBlock.test.ts | 479 +++++++++++++++++++++++++++++++++ src/core/internal.ts | 154 +++++++++-- src/core/presets.ts | 5 +- src/core/runMultistepTasks.ts | 70 ++++- src/core/runSettled.ts | 17 +- src/core/types.ts | 32 +++ src/engine/eip1193.ts | 36 +++ src/index.ts | 1 + 10 files changed, 797 insertions(+), 36 deletions(-) create mode 100644 src/__tests__/pinBlock.test.ts diff --git a/README.md b/README.md index 811f16d..8b994d6 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ _| _| _| _| _| _| _| _| _| _| _| _| [![CI](https://github.com/halaprix/domino/actions/workflows/ci.yml/badge.svg)](https://github.com/halaprix/domino/actions/workflows/ci.yml) [![npm version](https://img.shields.io/npm/v/@halaprix/domino)](https://www.npmjs.com/package/@halaprix/domino) -[![bundle size](https://img.shields.io/badge/gzip-13.0KB-brightgreen)](https://www.npmjs.com/package/@halaprix/domino) +[![bundle size](https://img.shields.io/badge/gzip-13.9KB-brightgreen)](https://www.npmjs.com/package/@halaprix/domino) [![TypeScript](https://img.shields.io/badge/TypeScript-5.5-blue)](https://www.typescriptlang.org/) [![MIT License](https://img.shields.io/badge/License-MIT-yellow.svg)](LICENSE) diff --git a/docs/api-reference.md b/docs/api-reference.md index 754c216..0876cd0 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -39,6 +39,37 @@ The FSM runs through all tasks step-by-step: Both `defineTask` output and hand-written `MultistepTask` objects are ordinary `MultistepTask`s to the FSM — they can be freely mixed in the same `tasks` array and batched together. +## Atomicity + +One physical multicall batch (one `eth_call` against Multicall3, or its deployless fallback) is atomic: every call inside it reads the same EVM state, because the node evaluates the whole batch against a single block. Across separate batches — later steps of the same task, or physical batches split by `batchSize`/`maxConcurrentBatches` within one step — there is **no such guarantee** by default: the chain can advance between round-trips. + +Set `pinBlock: true` to remove that gap. The run resolves ONE concrete block at the very start and reuses it for every step and every physical batch for the rest of that `run`/`runSettled` call: + +| `block` input | Resolved to | Extra RPC | +|---|---|---| +| absent, or `{ blockTag: 'latest' \| 'safe' \| 'finalized' }` | `executor.getBlockNumber(block)` → `{ blockNumber }`, used for every step | +1 round-trip | +| `{ blockTag: 'pending' }` | throws — `pending` has no stable block number | — | +| `{ blockNumber }` | used as-is | none | +| `{ blockHash, requireCanonical? }` | used as-is, `requireCanonical` untouched | none | + +```typescript +import { Eip1193Executor, MulticallResolver, Presets } from "@halaprix/domino" +import type { MultistepTask } from "@halaprix/domino" + +declare const executor: Eip1193Executor +declare const tasks: MultistepTask[] + +const resolver = new MulticallResolver(executor) + +await resolver.run(tasks, { + ...Presets.throughput, + pinBlock: true, + onPin: (block) => console.log("pinned to", block), +}) +``` + +`onPin` — when provided — runs synchronously exactly once per run, only when `pinBlock: true`, with the resolved `PinnedBlock`. If it throws, the run rejects with that error; tasks are already marked consumed by that point, and no multicall batch is ever dispatched. `pinBlock: true` against a `StepExecutor` without `getBlockNumber(block?)` throws immediately, before anything is consumed — a custom `StepExecutor` opts into pinning by implementing that one optional method. + ## `defineTask` — the recommended way to describe a task `defineTask(build)` runs `build` exactly once, synchronously, and returns a compiled `MultistepTask` — pass it straight to `runMultistepTasks`, `runSettled`, or `resolver.run()`/`resolver.runSettled()`, batched with any other task (including hand-written `MultistepTask`s, see [below](#legacy-hand-written-multisteptask-definetasks-compilation-target)) exactly like before. @@ -444,7 +475,8 @@ Import the following directly from `@halaprix/domino` (this list is exhaustive **Core — FSM executor:** - `runMultistepTasks(executor: StepExecutor, tasks: MultistepTask[], options?: BatchOptions): Promise` -- `BatchOptions` — `{ batchSize?: number; block?: BlockParam }` +- `BatchOptions` — `{ batchSize?: number; maxConcurrentBatches?: number; maxBatchAttempts?: number; adaptiveBatching?: boolean; dedupe?: boolean; block?: BlockParam; pinBlock?: boolean; onPin?: (block: PinnedBlock) => void }` +- `PinnedBlock` — the resolved block reported to `onPin`: an explicit `blockNumber`, or `blockHash` + optional `requireCanonical` — see [Atomicity](#atomicity) **Per-task settlement (`runSettled`):** - `runSettled(executor: StepExecutor, tasks: MultistepTask[], options?: BatchOptions): Promise[]>` @@ -468,6 +500,7 @@ Import the following directly from `@halaprix/domino` (this list is exhaustive **Eip1193Executor class:** - `constructor(provider: Eip1193Provider)` - `executeMulticall(calls: StepCall[], block?: BlockParam): Promise` +- `getBlockNumber(block?: BlockParam): Promise` — resolves a `BlockParam` to a concrete block number; used by `pinBlock` (see [Atomicity](#atomicity)) - `refreshChainId(): Promise` **`MulticallResolver` class** (implements `ResolverEngine`; signatures below use the default `TAddr = Address`): @@ -507,7 +540,7 @@ Import the following directly from `@halaprix/domino` (this list is exhaustive - `StepCall` — a single call in a multicall batch - `StepResult` — result of a call routed back to its task - `MultistepTask` — stateful task pipeline (`maxStep`, `buildStepCalls`, `consumeStepResults`, `finalize`) -- `StepExecutor` — executor interface (one method: `executeMulticall`) +- `StepExecutor` — executor interface (`executeMulticall`; an optional `getBlockNumber` powers `pinBlock` — see [Atomicity](#atomicity)) - `RawResult` — raw return value before routing - `Address` — hex string type alias (`0x${string}`) - `BlockParam` — block identifier (blockNumber, blockTag, or blockHash) diff --git a/src/__tests__/pinBlock.test.ts b/src/__tests__/pinBlock.test.ts new file mode 100644 index 0000000..9a0d094 --- /dev/null +++ b/src/__tests__/pinBlock.test.ts @@ -0,0 +1,479 @@ +import { describe, it, expect, vi } from 'vitest' +import { parseAbi, encodeFunctionResult, encodeAbiParameters } from 'viem' +import { defineTask } from '../core/defineTask' +import { runMultistepTasks } from '../core/runMultistepTasks' +import { runSettled } from '../core/runSettled' +import { Presets } from '../core/presets' +import { Eip1193Executor } from '../engine/eip1193' +import { DominoTaskReuseError } from '../core/errors' +import type { Address, MultistepTask, StepCall, StepExecutor, RawResult, BlockParam, PinnedBlock } from '../core/types' + +/** + * F8 — `pinBlock`. Fills the two seams T9 created in `src/core/internal.ts` + * (`validatePinCapability`/`resolvePinnedBlock`), whose pipeline positions + * are contractual: + * + * validateOptions -> rejectDuplicateInstances -> validatePinCapability + * -> markTasksConsumed -> resolvePinnedBlock -> executeSteps + * + * So: a capability/tag rejection (`validatePinCapability`) always happens + * BEFORE consumption (task reusable afterward); a resolution/`onPin` + * failure (`resolvePinnedBlock`) always happens AFTER consumption (task + * stays consumed) but strictly BEFORE any `executeMulticall` dispatch. + */ + +const ADDR = '0xA0b86991C6218B36C1D19D4a2e9EB004c35d5Cc4' as Address + +const testAbi = [ + { + type: 'function', + name: 'getNum', + stateMutability: 'view', + inputs: [], + outputs: [{ type: 'uint256' }], + }, +] as const + +/** Fresh single-call `defineTask` task — branded (single-use). */ +function brandedTask(): MultistepTask { + return defineTask((t) => t.call({ target: ADDR, abi: testAbi, functionName: 'getNum' })) +} + +/** Legacy (unbranded) two-step task — one call per step, so a 2-step run + * dispatches two separate physical `executeMulticall` invocations. */ +function twoStepLegacyTask(): MultistepTask<{ a?: bigint; b?: bigint }> { + const ctx: { a?: bigint; b?: bigint } = {} + return { + maxStep: 2, + buildStepCalls(step) { + if (step === 1) return [{ key: 'a', target: ADDR, abi: testAbi, functionName: 'getNum' }] + if (step === 2) return [{ key: 'b', target: ADDR, abi: testAbi, functionName: 'getNum' }] + return [] + }, + consumeStepResults(step, results) { + for (const r of results) { + if (r.status !== 'success') continue + if (step === 1) ctx.a = r.value as bigint + if (step === 2) ctx.b = r.value as bigint + } + }, + finalize() { + return { + ...(ctx.a !== undefined ? { a: ctx.a } : {}), + ...(ctx.b !== undefined ? { b: ctx.b } : {}), + } + }, + } +} + +/** One-step legacy task — for tests that don't care about step count. */ +function oneStepLegacyTask(): MultistepTask { + let value: bigint | undefined + return { + maxStep: 1, + buildStepCalls(step) { + if (step !== 1) return [] + return [{ key: 'v', target: ADDR, abi: testAbi, functionName: 'getNum' }] + }, + consumeStepResults(_step, results) { + for (const r of results) if (r.status === 'success') value = r.value as bigint + }, + finalize() { + return value + }, + } +} + +/** Records every `executeMulticall` invocation (calls + block param) and lets + * the caller push labelled events (e.g. from `onPin`) into the SAME ordered + * log, so relative ordering between `onPin` and the first dispatched batch + * can be asserted directly. */ +interface TrackingExecutor extends StepExecutor { + invocations: { calls: StepCall[]; block?: BlockParam }[] + events: string[] +} + +function makeExecutor(opts: { + getBlockNumber?: (block?: BlockParam) => Promise +} = {}): TrackingExecutor { + const invocations: { calls: StepCall[]; block?: BlockParam }[] = [] + const events: string[] = [] + const executor: TrackingExecutor = { + invocations, + events, + async executeMulticall(calls: StepCall[], block?: BlockParam): Promise { + events.push('executeMulticall') + invocations.push({ calls, ...(block !== undefined ? { block } : {}) }) + return calls.map((): RawResult => ({ status: 'success', value: 1n })) + }, + } + if (opts.getBlockNumber) { + executor.getBlockNumber = vi.fn(opts.getBlockNumber) + } + return executor +} + +// ─── 1/2. Tag resolution happens exactly once, at the real Eip1193Executor level ─── + +const totalSupplyAbi = parseAbi(['function getNum() view returns (uint256)']) + +function encodeAggregate3Result(value: bigint): string { + const returnData = encodeFunctionResult({ + abi: totalSupplyAbi, + functionName: 'getNum', + result: value, + }) + return encodeAbiParameters( + [{ type: 'tuple[]', components: [{ name: 'success', type: 'bool' }, { name: 'returnData', type: 'bytes' }] }], + [[{ success: true, returnData }]], + ) +} + +describe('F8 pinBlock — tag resolution at the Eip1193Executor level', () => { + it('block: { blockTag: "safe" } — eth_getBlockByNumber called exactly once for a 2-step task; every eth_call carries the resolved hex block number (+1 RT)', async () => { + // Post-Multicall3-deployment on mainnet (14,353,601) so this exercises + // the deployed path, not the deployless CREATE-wrapper fallback. + const RESOLVED_HEX = `0x${(20_000_000n).toString(16)}` + const mockResult = encodeAggregate3Result(42n) + + const provider = { + request: vi.fn(async ({ method, params }: { method: string; params?: readonly unknown[] }) => { + if (method === 'eth_chainId') return '0x1' + if (method === 'eth_getBlockByNumber') { + expect(params?.[0]).toBe('safe') + return { number: RESOLVED_HEX } + } + if (method === 'eth_call') return mockResult + throw new Error(`unexpected method ${method}`) + }), + } + + const executor = new Eip1193Executor(provider) + const task = twoStepLegacyTask() + + await runMultistepTasks(executor, [task], { pinBlock: true, block: { blockTag: 'safe' } }) + + const calls = provider.request.mock.calls as [{ method: string; params?: readonly unknown[] }][] + const blockByNumberCalls = calls.filter((c) => c[0].method === 'eth_getBlockByNumber') + const ethCallCalls = calls.filter((c) => c[0].method === 'eth_call') + + expect(blockByNumberCalls).toHaveLength(1) + expect(ethCallCalls).toHaveLength(2) // one per step + + for (const call of ethCallCalls) { + const blockParam = call[0].params?.[1] + expect(blockParam).toBe(RESOLVED_HEX) + } + + // Total round-trips: 1 chainId + 1 getBlockNumber (+1 RT) + 2 eth_call. + expect(calls).toHaveLength(4) + }) + + it('default block (absent) resolves "latest" once; all steps pinned to it', async () => { + const RESOLVED_HEX = `0x${(21_000_000n).toString(16)}` + const mockResult = encodeAggregate3Result(7n) + + const provider = { + request: vi.fn(async ({ method, params }: { method: string; params?: readonly unknown[] }) => { + if (method === 'eth_chainId') return '0x1' + if (method === 'eth_getBlockByNumber') { + expect(params?.[0]).toBe('latest') + return { number: RESOLVED_HEX } + } + if (method === 'eth_call') return mockResult + throw new Error(`unexpected method ${method}`) + }), + } + + const executor = new Eip1193Executor(provider) + const task = twoStepLegacyTask() + + await runMultistepTasks(executor, [task], { pinBlock: true }) + + const calls = provider.request.mock.calls as [{ method: string; params?: readonly unknown[] }][] + const blockByNumberCalls = calls.filter((c) => c[0].method === 'eth_getBlockByNumber') + const ethCallCalls = calls.filter((c) => c[0].method === 'eth_call') + + expect(blockByNumberCalls).toHaveLength(1) + expect(ethCallCalls).toHaveLength(2) + for (const call of ethCallCalls) { + expect(call[0].params?.[1]).toBe(RESOLVED_HEX) + } + }) +}) + +// ─── 3. `pending` + pinBlock -> throws pre-consumption ─── + +describe('F8 pinBlock — validatePinCapability (pre-consumption)', () => { + it('blockTag "pending" + pinBlock: true throws before consumption; the branded task is reusable afterward', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 1n }) + const task = brandedTask() + + await expect( + runMultistepTasks(executor, [task], { pinBlock: true, block: { blockTag: 'pending' } }), + ).rejects.toThrow(/pending/i) + + // getBlockNumber must never have been called — the pending check throws + // before resolution is ever attempted. + expect(executor.getBlockNumber).not.toHaveBeenCalled() + expect(executor.invocations).toHaveLength(0) + + // Not consumed: a later, ordinary run with the SAME instance still works. + const [result] = await runMultistepTasks(executor, [task]) + expect(result).toBe(1n) + }) + + it('runSettled: blockTag "pending" + pinBlock: true also rejects the whole call (not a settled rejection), pre-consumption', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 1n }) + const task = brandedTask() + + await expect( + runSettled(executor, [task], { pinBlock: true, block: { blockTag: 'pending' } }), + ).rejects.toThrow(/pending/i) + + const [settled] = await runSettled(executor, [task]) + expect(settled).toEqual({ status: 'fulfilled', value: 1n, diagnostics: { optionalFailures: [] } }) + }) + + it('executor without getBlockNumber + pinBlock: true throws before consumption; same executor WITHOUT pinBlock works (additive)', async () => { + const executor = makeExecutor() // no getBlockNumber at all + const task = brandedTask() + + await expect(runMultistepTasks(executor, [task], { pinBlock: true })).rejects.toThrow( + /getBlockNumber/, + ) + expect(executor.invocations).toHaveLength(0) + + // Not consumed — and the SAME executor works fine without pinBlock. + const [result] = await runMultistepTasks(executor, [task]) + expect(result).toBe(1n) + }) + + it('capability check fires even for an explicit blockNumber/blockHash block (predictable contract, not RPC-need-dependent)', async () => { + const executorNoCap = makeExecutor() + + const taskA = brandedTask() + await expect( + runMultistepTasks(executorNoCap, [taskA], { pinBlock: true, block: { blockNumber: 5n } }), + ).rejects.toThrow(/getBlockNumber/) + // Not consumed. + const [resultA] = await runMultistepTasks(executorNoCap, [taskA]) + expect(resultA).toBe(1n) + + const taskB = brandedTask() + await expect( + runMultistepTasks(executorNoCap, [taskB], { + pinBlock: true, + block: { blockHash: '0xaaaa000000000000000000000000000000000000000000000000000000000a' }, + }), + ).rejects.toThrow(/getBlockNumber/) + const [resultB] = await runMultistepTasks(executorNoCap, [taskB]) + expect(resultB).toBe(1n) + }) +}) + +// ─── 5/6. Explicit blockNumber / blockHash -> no-op, no RPC ─── + +describe('F8 pinBlock — resolvePinnedBlock no-op paths (explicit blockNumber/blockHash)', () => { + it('explicit blockNumber: getBlockNumber never invoked; onPin receives { blockNumber }; executeMulticall gets the same block', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 999n }) + const task = brandedTask() + const pins: PinnedBlock[] = [] + + await runMultistepTasks(executor, [task], { + pinBlock: true, + block: { blockNumber: 42n }, + onPin: (b) => pins.push(b), + }) + + expect(executor.getBlockNumber).not.toHaveBeenCalled() + expect(pins).toEqual([{ blockNumber: 42n }]) + expect(executor.invocations).toHaveLength(1) + expect(executor.invocations[0]!.block).toEqual({ blockNumber: 42n }) + }) + + const HASH = '0xbbbb000000000000000000000000000000000000000000000000000000000b' as const + + it('explicit blockHash (with requireCanonical): no RPC; executeMulticall gets the ORIGINAL param untouched; onPin receives { blockHash, requireCanonical }', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 999n }) + const task = brandedTask() + const pins: PinnedBlock[] = [] + + await runMultistepTasks(executor, [task], { + pinBlock: true, + block: { blockHash: HASH, requireCanonical: true }, + onPin: (b) => pins.push(b), + }) + + expect(executor.getBlockNumber).not.toHaveBeenCalled() + expect(pins).toEqual([{ blockHash: HASH, requireCanonical: true }]) + expect(executor.invocations[0]!.block).toEqual({ blockHash: HASH, requireCanonical: true }) + }) + + it('explicit blockHash (requireCanonical absent): onPin\'s PinnedBlock has NO requireCanonical key at all (Object.hasOwn false)', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 999n }) + const task = brandedTask() + let pinned: PinnedBlock | undefined + + await runMultistepTasks(executor, [task], { + pinBlock: true, + block: { blockHash: HASH }, + onPin: (b) => { + pinned = b + }, + }) + + expect(executor.invocations[0]!.block).toEqual({ blockHash: HASH }) + expect(pinned).toEqual({ blockHash: HASH }) + expect(Object.hasOwn(pinned!, 'requireCanonical')).toBe(false) + }) +}) + +// ─── 7. onPin: timing, gating, and throw-propagation semantics ─── + +describe('F8 pinBlock — onPin timing and throw semantics', () => { + it('called exactly once, synchronously, strictly before the first executeMulticall', async () => { + const executor = makeExecutor({ + getBlockNumber: async () => { + executor.events.push('getBlockNumber') + return 100n + }, + }) + const task = twoStepLegacyTask() + let onPinCalls = 0 + + await runMultistepTasks(executor, [task], { + pinBlock: true, + onPin: () => { + onPinCalls += 1 + executor.events.push('onPin') + }, + }) + + expect(onPinCalls).toBe(1) + // getBlockNumber -> onPin -> executeMulticall (step 1) -> executeMulticall (step 2) + expect(executor.events).toEqual(['getBlockNumber', 'onPin', 'executeMulticall', 'executeMulticall']) + }) + + it('provided but pinBlock is NOT set: onPin is never invoked', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 100n }) + const task = oneStepLegacyTask() + const onPin = vi.fn() + + await runMultistepTasks(executor, [task], { onPin }) + + expect(onPin).not.toHaveBeenCalled() + expect(executor.getBlockNumber).not.toHaveBeenCalled() + }) + + it('onPin throws -> run() rejects with that error; task remains consumed; executeMulticall is never called', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 100n }) + const task = brandedTask() + const boom = new Error('onPin exploded') + + await expect( + runMultistepTasks(executor, [task], { + pinBlock: true, + onPin: () => { + throw boom + }, + }), + ).rejects.toBe(boom) + + expect(executor.invocations).toHaveLength(0) // no multicall batch ever dispatched + + // Tasks remain consumed — a second submission of the SAME instance sees + // the reuse error, not another onPin throw. + await expect(runMultistepTasks(executor, [task])).rejects.toThrow(DominoTaskReuseError) + }) + + it('onPin throws -> runSettled() rejects the whole call (not a settled rejection); task remains consumed; executeMulticall never called', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 100n }) + const task = brandedTask() + const boom = new Error('onPin exploded (settled)') + + await expect( + runSettled(executor, [task], { + pinBlock: true, + onPin: () => { + throw boom + }, + }), + ).rejects.toBe(boom) + + expect(executor.invocations).toHaveLength(0) + await expect(runSettled(executor, [task])).rejects.toThrow(DominoTaskReuseError) + }) +}) + +// ─── 8. runSettled parity for tag-resolution-once / explicit-blockNumber / onPin-ordering ─── + +describe('F8 pinBlock — runSettled parity', () => { + it('tag resolves exactly once (parity with run())', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 100n }) + const task = twoStepLegacyTask() + + const [settled] = await runSettled(executor, [task], { pinBlock: true, block: { blockTag: 'finalized' } }) + + expect(settled!.status).toBe('fulfilled') + expect(executor.getBlockNumber).toHaveBeenCalledTimes(1) + expect(executor.invocations).toHaveLength(2) + for (const inv of executor.invocations) expect(inv.block).toEqual({ blockNumber: 100n }) + }) + + it('explicit blockNumber: no getBlockNumber invocation; onPin receives { blockNumber }', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 999n }) + const task = brandedTask() + const pins: PinnedBlock[] = [] + + const [settled] = await runSettled(executor, [task], { + pinBlock: true, + block: { blockNumber: 7n }, + onPin: (b) => pins.push(b), + }) + + expect(settled!.status).toBe('fulfilled') + expect(executor.getBlockNumber).not.toHaveBeenCalled() + expect(pins).toEqual([{ blockNumber: 7n }]) + }) + + it('onPin runs exactly once, before the first executeMulticall (parity with run())', async () => { + const executor = makeExecutor({ + getBlockNumber: async () => { + executor.events.push('getBlockNumber') + return 100n + }, + }) + const task = twoStepLegacyTask() + let onPinCalls = 0 + + await runSettled(executor, [task], { + pinBlock: true, + onPin: () => { + onPinCalls += 1 + executor.events.push('onPin') + }, + }) + + expect(onPinCalls).toBe(1) + expect(executor.events).toEqual(['getBlockNumber', 'onPin', 'executeMulticall', 'executeMulticall']) + }) +}) + +// ─── 9. Presets.throughput composes with pinBlock ─── + +describe('F8 pinBlock — composes with Presets.throughput', () => { + it('{ ...Presets.throughput, pinBlock: true } resolves the block once and runs to completion', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 555n }) + const task = brandedTask() + + const [result] = await runMultistepTasks(executor, [task], { + ...Presets.throughput, + pinBlock: true, + }) + + expect(result).toBe(1n) + expect(executor.getBlockNumber).toHaveBeenCalledTimes(1) + expect(executor.invocations[0]!.block).toEqual({ blockNumber: 555n }) + }) +}) diff --git a/src/core/internal.ts b/src/core/internal.ts index 5ad8627..bee9fe8 100644 --- a/src/core/internal.ts +++ b/src/core/internal.ts @@ -38,7 +38,7 @@ * so future code should prefer descriptive names. */ -import type { MultistepTask } from './types' +import type { MultistepTask, StepExecutor, BlockParam, PinnedBlock } from './types' import { DominoTaskReuseError } from './errors' /** @@ -109,6 +109,19 @@ export interface NumericOptionsInput { dedupe?: boolean } +/** + * Options `validatePinCapability`/`resolvePinnedBlock` read (F8) — a + * structural subset of `BatchOptions`. Declared here rather than imported + * from `runMultistepTasks.ts` for the same reason `NumericOptionsInput` is: + * `runMultistepTasks.ts` already imports FROM this module, so importing + * `BatchOptions` back would be circular. + */ +export interface PinOptionsInput { + pinBlock?: boolean + block?: BlockParam + onPin?: (block: PinnedBlock) => void +} + /** * Shared positive-safe-integer check for all three F6a numeric fields. * `Number.isSafeInteger` (not `Number.isInteger`) — a value like `2**53` @@ -195,16 +208,49 @@ export function rejectDuplicateInstances(ts: MultistepTask[]): void { } /** - * Pipeline step 3 — named no-op seam. F8/T17 (pinBlock capability) fills - * this in: it will check whether the executor supports the block-pinning - * capability the run requires and throw if not. Must run — and, once - * implemented, must throw — BEFORE `markTasksConsumed`: a capability - * rejection must never leave the rejected submission's tasks consumed. - * Takes no arguments today; F8/T17 adds whatever it needs (tasks/options) - * when it fills this in. + * Pipeline step 3 (F8) — checks whether this run's block-pinning request is + * satisfiable, and throws before anything is consumed if not. A no-op + * whenever `pinBlock` isn't set (the overwhelmingly common case — this + * function never even looks at `executor` or `block` then). + * + * Runs BEFORE `markTasksConsumed`: a capability rejection must never leave + * the rejected submission's tasks consumed — this is exactly the ordering + * `prepareRun` already documents, now filled in. + * + * Two checks, both spec-literal: + * + * (a) **Capability**: `pinBlock: true` against an executor that doesn't + * implement `getBlockNumber` throws — ALWAYS, even when the caller's + * `block` is an explicit `blockNumber`/`blockHash` that would never + * actually need to CALL `getBlockNumber` (see `resolvePinnedBlock` + * below). This is deliberate: `pinBlock` is a predictable capability + * contract on the executor, not a "throw only if we happen to need the + * RPC this time" heuristic — a caller who later switches their `block` + * to a tag shouldn't have that switch silently start throwing on an + * executor they'd already been using with `pinBlock: true`. + * (b) **Tag support**: `block: { blockTag: 'pending' }` throws — `pending` + * has no stable block number, so pinning it is a contradiction in + * terms, not something resolvable by picking a number. Every other tag + * (`latest`/`safe`/`finalized`), an explicit `blockNumber`, an explicit + * `blockHash`, and an absent `block` (implicit `'latest'`) are all fine + * here — `resolvePinnedBlock` is what actually branches on which. */ -export function validatePinCapability(): void { - // Intentionally empty until F8/T17. +export function validatePinCapability(options: PinOptionsInput | undefined, executor: StepExecutor): void { + if (!options?.pinBlock) return + + if (typeof executor.getBlockNumber !== 'function') { + throw new Error( + 'pinBlock: true requires a StepExecutor implementing getBlockNumber(block?) — this executor ' + + 'does not support block pinning (Eip1193Executor implements it; a custom StepExecutor must ' + + 'add it to opt in)', + ) + } + + if (options.block && 'blockTag' in options.block && options.block.blockTag === 'pending') { + throw new Error( + "pinBlock: true does not support blockTag 'pending' — pending has no stable block number", + ) + } } /** @@ -233,16 +279,74 @@ export function markTasksConsumed(ts: MultistepTask[]): void { } /** - * Pipeline step 5 — named async no-op seam. F8/T17 (pinBlock resolution) - * fills this in with an `await getBlockNumber()`-style call. Deliberately - * runs AFTER `markTasksConsumed`: a failure here means execution has already - * begun (calls are about to be dispatched), so by design the tasks are left - * consumed — the same rule `executeSteps` itself already follows for any - * later failure. Takes no arguments today; F8/T17 adds whatever it needs - * (tasks/options) when it fills this in with an `await getBlockNumber()`. + * Pipeline step 5 (F8) — resolves the ONE effective block every step of this + * run will use, and (when `pinBlock` is set) synchronously reports it via + * `onPin`. Deliberately runs AFTER `markTasksConsumed`: a failure here means + * execution has already begun (calls are about to be dispatched), so by + * design the tasks are left consumed — the same rule the step loop itself + * already follows for any later failure (see the executor-rejection-after- + * consumption test in `singleUse.test.ts`). + * + * `pinBlock` false/absent (the default): a pure no-op — returns + * `options?.block` completely untouched, so callers see byte-identical + * behavior to every pre-F8 release (including "block omitted" staying + * `undefined`, never defaulted here). + * + * `pinBlock: true` — by this point `validatePinCapability` has already + * guaranteed `executor.getBlockNumber` exists and `block.blockTag` isn't + * `'pending'`: + * - `block` absent, or `{ blockTag: 'latest' | 'safe' | 'finalized' }`: + * `await executor.getBlockNumber(block)` (the ONE extra round-trip + * pinning costs) resolves a concrete number; the effective block for + * every step becomes `{ blockNumber: resolved }`, and that's also the + * `PinnedBlock` reported to `onPin`. + * - explicit `{ blockNumber }`: no RPC — used as-is, reported as-is. + * - explicit `{ blockHash, requireCanonical? }`: no RPC — used as-is + * (`requireCanonical` untouched), reported as + * `{ blockHash, requireCanonical }` with the key omitted entirely when + * the caller didn't supply it (`exactOptionalPropertyTypes`-safe: an + * omitted key must be genuinely absent, not present-with-`undefined`). + * + * `onPin`, when provided, is called exactly once, SYNCHRONOUSLY (a direct + * call, no microtask hop introduced here beyond whatever `await + * getBlockNumber()` already caused) — and only ever reached when + * `pinBlock` is true; a caller who passes `onPin` without `pinBlock` gets it + * silently never invoked (see `BatchOptions.onPin`'s doc comment). If it + * throws, that throw is this function's rejection: the caller (`run`/ + * `runSettled`) sees it as `resolvePinnedBlock`'s own failure, tasks stay + * consumed (already marked, above), and — because this function is always + * `await`ed strictly before the step loop starts — no `executeMulticall` + * call has been dispatched yet. */ -export async function resolvePinnedBlock(): Promise { - // Intentionally empty until F8/T17. +export async function resolvePinnedBlock( + options: PinOptionsInput | undefined, + executor: StepExecutor, +): Promise { + if (!options?.pinBlock) return options?.block + + const requested = options.block + let effectiveBlock: BlockParam + let pinned: PinnedBlock + + if (requested && 'blockNumber' in requested) { + effectiveBlock = requested + pinned = { blockNumber: requested.blockNumber } + } else if (requested && 'blockHash' in requested) { + effectiveBlock = requested + pinned = + requested.requireCanonical !== undefined + ? { blockHash: requested.blockHash, requireCanonical: requested.requireCanonical } + : { blockHash: requested.blockHash } + } else { + // Absent, or an explicit blockTag — 'pending' is already impossible here + // (validatePinCapability rejected it pre-consumption). + const resolved = await executor.getBlockNumber!(requested) + effectiveBlock = { blockNumber: resolved } + pinned = { blockNumber: resolved } + } + + options.onPin?.(pinned) + return effectiveBlock } /** @@ -251,15 +355,23 @@ export async function resolvePinnedBlock(): Promise { * mark branded tasks consumed. Returns the validated options bundle so each * runner can drop this straight in place of its old inline validation. * + * Takes `executor` (F8) purely to hand it to `validatePinCapability` — this + * function still runs entirely synchronously (the capability check is sync; + * only `resolvePinnedBlock`, next, needs `await`). + * * Deliberately stops here (not shared with `resolvePinnedBlock`/the step * loop): `run()`/`runSettled()` diverge in failure handling once execution * actually starts, so those stay in each runner's own body (see * `src/core/engine.ts`'s `runSteps` for the part that IS now shared). */ -export function prepareRun(ts: MultistepTask[], o: NumericOptionsInput | undefined): ValidatedRunOptions { +export function prepareRun( + ts: MultistepTask[], + o: (NumericOptionsInput & PinOptionsInput) | undefined, + executor: StepExecutor, +): ValidatedRunOptions { const validated = validateOptions(o) rejectDuplicateInstances(ts) - validatePinCapability() + validatePinCapability(o, executor) markTasksConsumed(ts) return validated } diff --git a/src/core/presets.ts b/src/core/presets.ts index bff4289..c42000c 100644 --- a/src/core/presets.ts +++ b/src/core/presets.ts @@ -15,9 +15,8 @@ * await resolver.run(tasks, { ...Presets.throughput, batchSize: 200 }) * ``` * - * (A future `pinBlock` capability will compose the same way once it ships — - * `{ ...Presets.throughput, pinBlock: true }` — but that option does not - * exist yet as of this release.) + * `pinBlock` (F8) composes the same way — spread the preset, then add it: + * `{ ...Presets.throughput, pinBlock: true }`. * * `dedupe: true` here can never change legacy-task semantics: dedup only * ever merges calls stamped dedup-ELIGIBLE (a compiled `TypedCallSpec` call, diff --git a/src/core/runMultistepTasks.ts b/src/core/runMultistepTasks.ts index 26a97c3..35116e6 100644 --- a/src/core/runMultistepTasks.ts +++ b/src/core/runMultistepTasks.ts @@ -13,7 +13,7 @@ * (vs O(N) sequential calls for naive approach) */ -import type { MultistepTask, StepCall, StepResult, StepExecutor, RawResult, BlockParam } from './types' +import type { MultistepTask, StepCall, StepResult, StepExecutor, RawResult, BlockParam, PinnedBlock } from './types' import { prepareRun, resolvePinnedBlock } from './internal' import { runSteps, type StepEnginePolicy } from './engine' @@ -95,9 +95,60 @@ export interface BatchOptions { */ adaptiveBatching?: boolean - /** Block to query at (defaults to 'latest'). Same block used for ALL steps. */ + /** + * Block to query at (defaults to 'latest'). Same block PARAMETER used for + * every step's `executeMulticall` call — but without `pinBlock: true`, + * that does not mean every step reads the same STATE: a tag like + * `'latest'`/`'safe'`/`'finalized'` is re-resolved by the node on every + * separate `eth_call`, so the chain can advance between steps. See + * `pinBlock` below (and the "Atomicity" section in `docs/api-reference.md`) + * to remove that gap. + */ block?: BlockParam + /** + * Enable block pinning (F8): resolve ONE concrete block at the very start + * of the run and reuse it for every step's `executeMulticall` call, + * closing the "chain advanced between steps" gap `block` alone leaves + * open (see its doc comment above). Default: `false`. + * + * Resolution (see `resolvePinnedBlock` in `src/core/internal.ts` for the + * full contract): + * - `block` absent, or `{ blockTag: 'latest' | 'safe' | 'finalized' }` — + * resolved via `executor.getBlockNumber(block)` (one extra round-trip); + * every step then queries `{ blockNumber: }`. + * - `block: { blockTag: 'pending' }` — throws before any task is consumed. + * `pending` has no stable block number; pinning it is unsupported. + * - explicit `block: { blockNumber }` — no-op, no extra RPC: already a + * single concrete block. + * - explicit `block: { blockHash, requireCanonical? }` — no-op, no extra + * RPC, `requireCanonical` preserved untouched. + * + * Requires a `StepExecutor` implementing `getBlockNumber` — `true` against + * an executor that lacks it throws immediately (before anything is + * consumed), regardless of which `block` shape was passed (a predictable + * capability contract, not a "only if we happen to need the RPC" check). + * `Eip1193Executor` implements it; a custom `StepExecutor` opts in the + * same way. + */ + pinBlock?: boolean + + /** + * Synchronous callback reporting the block a `pinBlock: true` run resolved + * (F8) — invoked exactly once per run, with the resolved `PinnedBlock`. + * Mapping: a resolved tag or an explicit `blockNumber` reports + * `{ blockNumber }`; an explicit `blockHash` reports + * `{ blockHash, requireCanonical }` (no extra RPC is ever made just to + * learn its number). **Only invoked when `pinBlock` is `true`** — supplying + * `onPin` without `pinBlock` is accepted but it is simply never called. + * + * If `onPin` throws, the run rejects with that error: tasks are already + * marked consumed by this point (block resolution happens after the + * consumption pipeline — see `src/core/internal.ts`), and no multicall + * batch has been dispatched yet. + */ + onPin?: (block: PinnedBlock) => void + /** * Enable within-step, cross-task call dedup (F7): before the wire list for * a step reaches batching/bisection, calls that are dedup-ELIGIBLE and @@ -177,8 +228,17 @@ export async function runMultistepTasks( // -> mark-consumed), see `src/core/internal.ts`. Only branded tasks // (`defineTask`/`buildErc20Task`/`buildErc4626Task` output) are affected — // legacy `MultistepTask`s pass through every step as a no-op. - const { batchSize, maxConcurrentBatches, adaptiveBatching, maxBatchAttempts, dedupe } = prepareRun(ts, options) - await resolvePinnedBlock() + const { batchSize, maxConcurrentBatches, adaptiveBatching, maxBatchAttempts, dedupe } = prepareRun( + ts, + options, + executor, + ) + // F8: the ONE effective block every step's executeMulticall uses — either + // `options?.block` untouched (pinBlock off/absent, the default) or the + // resolved pin (see `resolvePinnedBlock`'s doc comment in + // `src/core/internal.ts`). Replaces the old direct `options?.block` read + // in `executeBatch` below. + const effectiveBlock = await resolvePinnedBlock(options, executor) // `run`'s fail-fast policy (F6a/F6b) — see `src/core/engine.ts`'s doc // comment for the full hook contract. `buildStepCalls`/`consumeStepResults` @@ -202,7 +262,7 @@ export async function runMultistepTasks( }, executeBatch(batch) { - return executor.executeMulticall(batch, options?.block) + return executor.executeMulticall(batch, effectiveBlock) }, consumeStepResults(taskIndex, step, results) { diff --git a/src/core/runSettled.ts b/src/core/runSettled.ts index 058bbe4..a29b864 100644 --- a/src/core/runSettled.ts +++ b/src/core/runSettled.ts @@ -156,11 +156,20 @@ export async function runSettled( // `runSettled(executor, [], { batchSize: 0 })` rejects rather than // silently resolving to `[]` (unchanged 1.0 ordering — contrast with // `runMultistepTasks`, which checks the empty-tasks shortcut first). - const { batchSize, maxConcurrentBatches, adaptiveBatching, maxBatchAttempts, dedupe } = prepareRun(ts, options) + const { batchSize, maxConcurrentBatches, adaptiveBatching, maxBatchAttempts, dedupe } = prepareRun( + ts, + options, + executor, + ) if (ts.length === 0) return [] - await resolvePinnedBlock() + // F8: the ONE effective block every step's executeMulticall uses — either + // `options?.block` untouched (pinBlock off/absent, the default) or the + // resolved pin (see `resolvePinnedBlock`'s doc comment in + // `src/core/internal.ts`). Replaces the direct `options?.block` reads + // below. + const effectiveBlock = await resolvePinnedBlock(options, executor) // Dead-task bookkeeping: once true, no further buildStepCalls/consumeStepResults // calls happen for that task index, and finalize() is skipped entirely. @@ -198,7 +207,7 @@ export async function runSettled( // itself now — see `src/core/pool.ts` — so it applies uniformly to // this call whether the pool handed it a whole batch or a bisected // sub-batch. - return executor.executeMulticall(batch, options?.block) + return executor.executeMulticall(batch, effectiveBlock) } // Adaptive OFF (default) — byte-for-byte T14/F5 behavior: catch a @@ -211,7 +220,7 @@ export async function runSettled( // now the pool's job — see `executeBatch` above and `pool.ts` — so // this branch, like the adaptive one, is just the raw executor call.) try { - return await executor.executeMulticall(batch, options?.block) + return await executor.executeMulticall(batch, effectiveBlock) } catch (transportError) { return synthesizeBatchFailures(batch, transportError) } diff --git a/src/core/types.ts b/src/core/types.ts index 490da75..9bd15da 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -23,6 +23,20 @@ export type BlockParam = /** Default block: 'latest' */ export const DEFAULT_BLOCK: BlockParam = { blockTag: 'latest' } +/** + * The resolved, single-block identity a `pinBlock: true` run pins every step + * to (F8) — exposed synchronously via `BatchOptions.onPin`. A subset of + * `BlockParam`'s union deliberately missing `blockTag`: by the time a + * `PinnedBlock` exists, any tag has already been resolved to a concrete + * `blockNumber` (or the input was already an explicit `blockNumber`/ + * `blockHash`, which need no resolution). See `resolvePinnedBlock` in + * `src/core/internal.ts` for the tag -> concrete-block resolution and the + * exact mapping table. + */ +export type PinnedBlock = + | { blockNumber: bigint } + | { blockHash: `0x${string}`; requireCanonical?: boolean } + /** * Minimal EIP-1193 provider interface. * Works with viem PublicClient, ethers providers, window.ethereum. @@ -78,6 +92,24 @@ export interface StepExecutor { * @param block — optional block identifier (defaults to 'latest') */ executeMulticall(calls: StepCall[], block?: BlockParam): Promise + + /** + * Resolve a `BlockParam` to a concrete block number (F8, `pinBlock`). + * Optional — additive: existing custom `StepExecutor` implementations are + * unaffected. An executor that omits this method can still be used + * normally; it simply can't be used with `pinBlock: true` + * (`validatePinCapability` in `src/core/internal.ts` throws up front, + * before anything is consumed, when `pinBlock` is requested against an + * executor lacking this method). + * + * @param block — block to resolve; absent means 'latest'. Implementations + * are never called with a `blockHash` block by domino's own pin pipeline + * (an explicit `blockNumber`/`blockHash` needs no resolution — see + * `resolvePinnedBlock`), but a direct caller invoking this method itself + * with a `blockHash` should get a clear rejection rather than silently + * wrong behavior — `Eip1193Executor`'s implementation throws for that case. + */ + getBlockNumber?(block?: BlockParam): Promise } /** diff --git a/src/engine/eip1193.ts b/src/engine/eip1193.ts index e319a85..241a0fc 100644 --- a/src/engine/eip1193.ts +++ b/src/engine/eip1193.ts @@ -61,6 +61,42 @@ export class Eip1193Executor implements StepExecutor { return this.#detectChainId() } + /** + * Resolve a `BlockParam` to a concrete block number (F8, `pinBlock`'s + * capability method — see `StepExecutor.getBlockNumber`'s doc comment). + * + * - `block` absent, or `{ blockTag }` — one `eth_getBlockByNumber` request + * (tag defaults to `'latest'` when `block` is absent), returning the + * block's `number` field parsed from hex to `bigint`. + * - explicit `{ blockNumber }` — returned as-is, no RPC. + * - `{ blockHash, ... }` — NOT resolved by this method (deliberately + * minimal: domino's own `pinBlock` pipeline never calls this for a + * blockHash block — an explicit hash needs no resolution to begin with, + * see `resolvePinnedBlock` in `src/core/internal.ts`). A direct caller + * invoking this method with a blockHash block gets a clear, immediate + * rejection instead of a silently wrong/absent block number. + */ + async getBlockNumber(block?: BlockParam): Promise { + if (block && 'blockHash' in block) { + throw new Error('getBlockNumber does not resolve block hashes') + } + if (block && 'blockNumber' in block) { + return block.blockNumber + } + + const tag = block && 'blockTag' in block ? block.blockTag : 'latest' + const result = (await this.#provider.request({ + method: 'eth_getBlockByNumber', + params: [tag, false], + })) as { number: `0x${string}` | null } | null + + const numberHex = result?.number + if (numberHex == null) { + throw new Error(`getBlockNumber: no block found for tag "${tag}"`) + } + return BigInt(numberHex) + } + /** * Execute one batch of calls. */ diff --git a/src/index.ts b/src/index.ts index 2b7dafe..0636314 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ export type { BlockParam, BlockTag, Eip1193Provider, + PinnedBlock, } from './core/types' export type { BatchOptions } from './core/runMultistepTasks' From a0f2d4a2fef7b29d3e8a16df6a9cc1d6db1cdb89 Mon Sep 17 00:00:00 2001 From: halaprix Date: Thu, 23 Jul 2026 20:52:03 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20F8=20pinBlock=20=E2=80=94=20onPin=20?= =?UTF-8?q?fires=20on=20empty=20runs,=20doc=20precision=20(external=20revi?= =?UTF-8?q?ew)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P2 findings from external review, same branch: 1. runMultistepTasks's empty-tasks shortcut used to bypass F8 entirely, so onPin never fired for a zero-task pinBlock:true run; runSettled had the opposite bug — it always ran prepareRun (and validatePinCapability) for an empty submission but returned [] before ever reaching resolvePinnedBlock, so onPin never fired there either. Both fixed: onPin's "exactly once per run when pinBlock: true" contract now holds for zero-task runs in both runners. runMultistepTasks's 1.0-pinned quirk (invalid batchSize with zero tasks silently resolves to []) stays intact — only pinBlock's own two seams are exempted from that shortcut, nothing else. 2. Added the missing consumed-state test: executor.getBlockNumber rejecting (the post-consumption seam) rejects the run with that exact error, dispatches zero multicall batches, and leaves the task consumed — proven by a resubmission throwing DominoTaskReuseError instead of another transport failure. Covers both runners. 3. Docs: the getBlockNumber entry in api-reference.md now states its exact contract (absent/tag resolves via eth_getBlockByNumber; explicit blockNumber returns as-is with no RPC; blockHash rejects). Atomicity table and the matching source doc comments now correctly note that 'earliest' resolves the same way as 'latest'/'safe'/'finalized' (any stable tag does) — only 'pending' throws. Adds 7 tests to src/__tests__/pinBlock.test.ts (24 total in that file). --- docs/api-reference.md | 4 +- src/__tests__/pinBlock.test.ts | 96 ++++++++++++++++++++++++++++++++++ src/core/internal.ts | 13 +++-- src/core/runMultistepTasks.ts | 36 ++++++++++--- src/core/runSettled.ts | 14 ++++- 5 files changed, 146 insertions(+), 17 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 0876cd0..b832bbc 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -47,7 +47,7 @@ Set `pinBlock: true` to remove that gap. The run resolves ONE concrete block at | `block` input | Resolved to | Extra RPC | |---|---|---| -| absent, or `{ blockTag: 'latest' \| 'safe' \| 'finalized' }` | `executor.getBlockNumber(block)` → `{ blockNumber }`, used for every step | +1 round-trip | +| absent, or any stable tag (`{ blockTag: 'latest' \| 'earliest' \| 'safe' \| 'finalized' }`) | `executor.getBlockNumber(block)` → `{ blockNumber }`, used for every step | +1 round-trip | | `{ blockTag: 'pending' }` | throws — `pending` has no stable block number | — | | `{ blockNumber }` | used as-is | none | | `{ blockHash, requireCanonical? }` | used as-is, `requireCanonical` untouched | none | @@ -500,7 +500,7 @@ Import the following directly from `@halaprix/domino` (this list is exhaustive **Eip1193Executor class:** - `constructor(provider: Eip1193Provider)` - `executeMulticall(calls: StepCall[], block?: BlockParam): Promise` -- `getBlockNumber(block?: BlockParam): Promise` — resolves a `BlockParam` to a concrete block number; used by `pinBlock` (see [Atomicity](#atomicity)) +- `getBlockNumber(block?: BlockParam): Promise` — resolves a `BlockParam` to a concrete block number. Exact contract: `block` absent or carrying a `blockTag` resolves via one `eth_getBlockByNumber` round-trip; an explicit `blockNumber` is returned as-is with no RPC at all; a `blockHash` block is rejected (throws `"getBlockNumber does not resolve block hashes"` — domino's own `pinBlock` pipeline never calls it that way, since an explicit hash needs no resolution to begin with). Used by `pinBlock` (see [Atomicity](#atomicity)) - `refreshChainId(): Promise` **`MulticallResolver` class** (implements `ResolverEngine`; signatures below use the default `TAddr = Address`): diff --git a/src/__tests__/pinBlock.test.ts b/src/__tests__/pinBlock.test.ts index 9a0d094..3ab59e9 100644 --- a/src/__tests__/pinBlock.test.ts +++ b/src/__tests__/pinBlock.test.ts @@ -272,6 +272,102 @@ describe('F8 pinBlock — validatePinCapability (pre-consumption)', () => { }) }) +// ─── Empty-tasks ordering (external review, P2) ─── +// +// `runMultistepTasks`'s empty-tasks shortcut historically ran BEFORE the F2 +// pipeline entirely (a 1.0-pinned quirk: an invalid `batchSize` with zero +// tasks silently resolves to `[]`). That shortcut used to also skip F8's own +// seams outright — meaning `onPin` never fired for a zero-task `pinBlock: +// true` run. `runSettled` had the opposite bug: it always ran `prepareRun` +// (and so `validatePinCapability`) for an empty submission, but returned `[]` +// BEFORE ever reaching `resolvePinnedBlock`, so `onPin` never fired there +// either. Both are fixed: `onPin`'s contract ("exactly once per run whenever +// `pinBlock: true`") now holds for zero-task runs in both runners, while the +// legacy `batchSize`-with-zero-tasks quirk stays intact for `run()`. + +describe('F8 pinBlock — empty-tasks ordering (external review, P2)', () => { + it('run(): empty tasks + pinBlock + onPin -> onPin fires once with the resolved PinnedBlock; zero executeMulticall', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 777n }) + const pins: PinnedBlock[] = [] + + const result = await runMultistepTasks(executor, [], { pinBlock: true, onPin: (b) => pins.push(b) }) + + expect(result).toEqual([]) + expect(executor.getBlockNumber).toHaveBeenCalledTimes(1) + expect(pins).toEqual([{ blockNumber: 777n }]) + expect(executor.invocations).toHaveLength(0) + }) + + it('runSettled(): empty tasks + pinBlock + onPin -> onPin fires once with the resolved PinnedBlock; zero executeMulticall', async () => { + const executor = makeExecutor({ getBlockNumber: async () => 888n }) + const pins: PinnedBlock[] = [] + + const result = await runSettled(executor, [], { pinBlock: true, onPin: (b) => pins.push(b) }) + + expect(result).toEqual([]) + expect(executor.getBlockNumber).toHaveBeenCalledTimes(1) + expect(pins).toEqual([{ blockNumber: 888n }]) + expect(executor.invocations).toHaveLength(0) + }) + + it('run(): empty tasks + pinBlock + missing getBlockNumber capability -> throws', async () => { + const executor = makeExecutor() // no getBlockNumber at all + await expect(runMultistepTasks(executor, [], { pinBlock: true })).rejects.toThrow(/getBlockNumber/) + expect(executor.invocations).toHaveLength(0) + }) + + it('runSettled(): empty tasks + pinBlock + missing getBlockNumber capability -> throws', async () => { + const executor = makeExecutor() + await expect(runSettled(executor, [], { pinBlock: true })).rejects.toThrow(/getBlockNumber/) + expect(executor.invocations).toHaveLength(0) + }) + + it('run(): empty tasks + pinBlock: false + invalid batchSize -> still resolves [] (1.0-pinned legacy quirk, unaffected by F8)', async () => { + const executor = makeExecutor() + const result = await runMultistepTasks(executor, [], { batchSize: 0 }) + expect(result).toEqual([]) + expect(executor.invocations).toHaveLength(0) + }) +}) + +// ─── getBlockNumber rejection: resolution stays strictly AFTER markTasksConsumed (external review, P2) ─── + +describe('F8 pinBlock — getBlockNumber rejection leaves tasks consumed', () => { + it('run(): executor.getBlockNumber rejects -> run rejects with that error; zero executeMulticall; resubmission throws DominoTaskReuseError', async () => { + const boom = new Error('getBlockNumber transport failure') + const executor = makeExecutor({ + getBlockNumber: async () => { + throw boom + }, + }) + const task = brandedTask() + + await expect(runMultistepTasks(executor, [task], { pinBlock: true })).rejects.toBe(boom) + expect(executor.invocations).toHaveLength(0) // zero multicall dispatches + + // Proves resolution ran AFTER markTasksConsumed: the task is already + // consumed, so a resubmission (even WITHOUT pinBlock, so getBlockNumber + // is never called again) sees the reuse error, not another transport + // failure. + await expect(runMultistepTasks(executor, [task])).rejects.toThrow(DominoTaskReuseError) + }) + + it('runSettled(): executor.getBlockNumber rejects -> whole call rejects with that error (not a settled rejection); zero executeMulticall; resubmission throws DominoTaskReuseError', async () => { + const boom = new Error('getBlockNumber transport failure (settled)') + const executor = makeExecutor({ + getBlockNumber: async () => { + throw boom + }, + }) + const task = brandedTask() + + await expect(runSettled(executor, [task], { pinBlock: true })).rejects.toBe(boom) + expect(executor.invocations).toHaveLength(0) + + await expect(runSettled(executor, [task])).rejects.toThrow(DominoTaskReuseError) + }) +}) + // ─── 5/6. Explicit blockNumber / blockHash -> no-op, no RPC ─── describe('F8 pinBlock — resolvePinnedBlock no-op paths (explicit blockNumber/blockHash)', () => { diff --git a/src/core/internal.ts b/src/core/internal.ts index bee9fe8..f28ab0c 100644 --- a/src/core/internal.ts +++ b/src/core/internal.ts @@ -230,10 +230,11 @@ export function rejectDuplicateInstances(ts: MultistepTask[]): void { * executor they'd already been using with `pinBlock: true`. * (b) **Tag support**: `block: { blockTag: 'pending' }` throws — `pending` * has no stable block number, so pinning it is a contradiction in - * terms, not something resolvable by picking a number. Every other tag - * (`latest`/`safe`/`finalized`), an explicit `blockNumber`, an explicit - * `blockHash`, and an absent `block` (implicit `'latest'`) are all fine - * here — `resolvePinnedBlock` is what actually branches on which. + * terms, not something resolvable by picking a number. Every other, + * STABLE tag (`latest`/`earliest`/`safe`/`finalized`), an explicit + * `blockNumber`, an explicit `blockHash`, and an absent `block` + * (implicit `'latest'`) are all fine here — `resolvePinnedBlock` is + * what actually branches on which. */ export function validatePinCapability(options: PinOptionsInput | undefined, executor: StepExecutor): void { if (!options?.pinBlock) return @@ -295,7 +296,9 @@ export function markTasksConsumed(ts: MultistepTask[]): void { * `pinBlock: true` — by this point `validatePinCapability` has already * guaranteed `executor.getBlockNumber` exists and `block.blockTag` isn't * `'pending'`: - * - `block` absent, or `{ blockTag: 'latest' | 'safe' | 'finalized' }`: + * - `block` absent, or carrying any STABLE `blockTag` + * (`'latest' | 'earliest' | 'safe' | 'finalized'` — anything but + * `'pending'`, already rejected by `validatePinCapability`): * `await executor.getBlockNumber(block)` (the ONE extra round-trip * pinning costs) resolves a concrete number; the effective block for * every step becomes `{ blockNumber: resolved }`, and that's also the diff --git a/src/core/runMultistepTasks.ts b/src/core/runMultistepTasks.ts index 35116e6..0416b9c 100644 --- a/src/core/runMultistepTasks.ts +++ b/src/core/runMultistepTasks.ts @@ -14,7 +14,7 @@ */ import type { MultistepTask, StepCall, StepResult, StepExecutor, RawResult, BlockParam, PinnedBlock } from './types' -import { prepareRun, resolvePinnedBlock } from './internal' +import { prepareRun, resolvePinnedBlock, validatePinCapability } from './internal' import { runSteps, type StepEnginePolicy } from './engine' /** @@ -98,11 +98,11 @@ export interface BatchOptions { /** * Block to query at (defaults to 'latest'). Same block PARAMETER used for * every step's `executeMulticall` call — but without `pinBlock: true`, - * that does not mean every step reads the same STATE: a tag like - * `'latest'`/`'safe'`/`'finalized'` is re-resolved by the node on every - * separate `eth_call`, so the chain can advance between steps. See - * `pinBlock` below (and the "Atomicity" section in `docs/api-reference.md`) - * to remove that gap. + * that does not mean every step reads the same STATE: a stable tag like + * `'latest'`/`'earliest'`/`'safe'`/`'finalized'` is re-resolved by the + * node on every separate `eth_call`, so the chain can advance between + * steps. See `pinBlock` below (and the "Atomicity" section in + * `docs/api-reference.md`) to remove that gap. */ block?: BlockParam @@ -114,7 +114,8 @@ export interface BatchOptions { * * Resolution (see `resolvePinnedBlock` in `src/core/internal.ts` for the * full contract): - * - `block` absent, or `{ blockTag: 'latest' | 'safe' | 'finalized' }` — + * - `block` absent, or carrying any STABLE `blockTag` + * (`'latest' | 'earliest' | 'safe' | 'finalized'`) — * resolved via `executor.getBlockNumber(block)` (one extra round-trip); * every step then queries `{ blockNumber: }`. * - `block: { blockTag: 'pending' }` — throws before any task is consumed. @@ -211,7 +212,26 @@ export async function runMultistepTasks( // 1.0 behavior (an invalid `batchSize` with zero tasks silently resolves // to `[]` rather than throwing) and must not change; see `runSettled`'s // deliberately different ordering (it validates first) for contrast. - if (tasks.length === 0) return [] + // + // F8 ordering nuance (external review, P2): that 1.0 quirk is preserved + // EXACTLY — `validateOptions` (batchSize/etc.) still never runs for an + // empty submission, `pinBlock` or not. But `onPin`'s own contract is + // "invoked exactly once per run whenever `pinBlock: true`" — a run with + // zero tasks is still a run, and there is no task for that guarantee to + // hide behind. So when `pinBlock` is set, this shortcut does NOT bypass + // F8's two seams: it runs `validatePinCapability` (still vacuously + // "pre-consumption" — there is nothing to consume) and + // `resolvePinnedBlock` (which invokes `onPin`) before returning `[]`, + // with zero tasks ever touching `executeMulticall`. `pinBlock: false` + // (the default) takes neither branch below and is byte-identical to + // every pre-F8 release. + if (tasks.length === 0) { + if (options?.pinBlock) { + validatePinCapability(options, executor) + await resolvePinnedBlock(options, executor) + } + return [] + } // TOCTOU fix (external review, P1): snapshot the caller-owned array BEFORE // the consumption pipeline runs, and read ONLY this snapshot (`ts`) for diff --git a/src/core/runSettled.ts b/src/core/runSettled.ts index a29b864..8f99df0 100644 --- a/src/core/runSettled.ts +++ b/src/core/runSettled.ts @@ -162,15 +162,25 @@ export async function runSettled( executor, ) - if (ts.length === 0) return [] - // F8: the ONE effective block every step's executeMulticall uses — either // `options?.block` untouched (pinBlock off/absent, the default) or the // resolved pin (see `resolvePinnedBlock`'s doc comment in // `src/core/internal.ts`). Replaces the direct `options?.block` reads // below. + // + // F8 ordering nuance (external review, P2): called BEFORE the + // empty-tasks check below — `prepareRun` above already runs + // unconditionally regardless of `ts.length` (see its own comment), so + // `validatePinCapability` has already gated a bad `pinBlock` request + // either way; the only fix needed here is to not shortcut `resolvePinnedBlock` + // (and therefore `onPin`) away for an empty submission. `onPin`'s contract + // is "exactly once per run whenever `pinBlock: true`" — a zero-task run is + // still a run. `pinBlock: false` (the default) makes this call a pure + // no-op regardless of `ts.length`, so ordering here is unobservable then. const effectiveBlock = await resolvePinnedBlock(options, executor) + if (ts.length === 0) return [] + // Dead-task bookkeeping: once true, no further buildStepCalls/consumeStepResults // calls happen for that task index, and finalize() is skipped entirely. // Lives here (not in the shared engine) — it's `runSettled`-specific state