diff --git a/.github/workflows/review-candidate.yml b/.github/workflows/review-candidate.yml new file mode 100644 index 0000000..1171e70 --- /dev/null +++ b/.github/workflows/review-candidate.yml @@ -0,0 +1,40 @@ +name: Official revision evidence + +on: + pull_request: + push: + branches: [main] + workflow_dispatch: + +permissions: + contents: read + +jobs: + candidate: + name: Pinned official candidate and negative control + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + - uses: foundry-rs/foundry-toolchain@v1 + with: + version: v1.7.1 + - name: Install forge-std + working-directory: contracts + run: forge install foundry-rs/forge-std@v1.16.2 --no-git + - name: Check gate failure modes + run: node --test scripts/review-candidate.test.mjs + - name: Test exact upstream candidate bytes + run: node scripts/review-candidate.mjs + - uses: actions/upload-artifact@v4 + if: always() + with: + name: erc8350-upstream-candidate-${{ github.sha }} + path: | + artifacts/review-candidate/*.json + artifacts/review-candidate/*.txt + artifacts/review-candidate/erc-8350.md + artifacts/review-candidate/*/erc-8350/*.sol + if-no-files-found: error diff --git a/.gitignore b/.gitignore index 6c9518f..80563e9 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ artifacts/conformance/ artifacts/external-reproductions/ artifacts/independent-client*/ artifacts/account-conformance/ +artifacts/review-candidate/ account-conformance/out/ account-conformance/cache/ account-conformance/lib/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 90d0deb..015c219 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable implementation and protocol changes are recorded here. ### Added +- Pinned official authorization-candidate gate and CI: verifies proposed upstream + bytes, runs 38 authorization/signature tests, and requires two dirty-padding + failures against the historical official source as a negative control. Review + packet links merged scoped-client, signature and account evidence without + replacing historical official baselines or granting G4 acceptance. + - Separate Prague real-account conformance harness with pinned Safe 1.4.1, 2-of-2 signatures and real SignMessageLib approval; actual local Anvil type-4 transactions verify delegation nonce/stale-authority behavior. Registry bytecode diff --git a/docs/interop/account-conformance.md b/docs/interop/account-conformance.md index fd5ff83..297858c 100644 --- a/docs/interop/account-conformance.md +++ b/docs/interop/account-conformance.md @@ -73,5 +73,6 @@ EVM implementations can adapt the `_deploy` helper in the common cases. Non-EVM clients should reproduce the same inputs, decisions and before/after state, not copy reference-specific error selectors. Signature validation needs the specified chain/block wallet state; offline hash replay alone cannot verify historical 1271 -policy. Portable static signature-vector publication is a separate follow-up; -this package does not change or rename v1/v2 or the v1 wire format. +policy. Portable static data is now published as `test-vectors/signature-vectors-v1.json` +with 46 cases and core/minimal TS plus Solidity adapters (PR #37). It does not +replace this executable account suite or change v1/v2 or the v1 wire format. diff --git a/docs/review/draft-to-review.md b/docs/review/draft-to-review.md index dbede8d..387a936 100644 --- a/docs/review/draft-to-review.md +++ b/docs/review/draft-to-review.md @@ -1,7 +1,9 @@ # Draft-to-Review preparation -Date: 2026-11-01. Status: preparation and open technical questions, not a submitted -status transition. The official ERC remains Draft. +Updated: 2026-11-06. Status: technical revision submitted in +[ethereum/ERCs#2147](https://github.com/ethereum/ERCs/pull/2147), not a submitted +status transition. The official ERC remains Draft. The proposed revision is +`397426c046049e1a0c3436e543f9731408708fd3`; it is not a new accepted baseline. ## Review packet @@ -11,9 +13,12 @@ status transition. The official ERC remains Draft. - [Authorization decision table and proposed wording](../spec-baseline/AUTHORIZATION.md). - [AQ-03 runtime/audit delta](../security/aq03-runtime-delta.md). - [Real-account test command and boundaries](../interop/account-conformance.md). -- [External replay evidence](../interop/external-reproduction.md), with the latest - upstream correction submitted separately in project PR #33. +- [Official-candidate reproduction and acceptance checklist](upstream-authorization.md). +- [External replay evidence](../interop/external-reproduction.md): project PR #33 + merged; issues #29/#31 closed on 2026-11-01 with preserved failure records. - [Independent-client acceptance package](../interop/independent-client-handoff.md). +- [Scoped Python results](../interop/independent-client-results.md) and the + [merged PR #37 main acceptance](https://github.com/AwareLiquid/AgentMemoryState/pull/37#issuecomment-6679669756). - Public external report intake in `audits/g4/external/submissions/`; acceptance remains pending under issues #11/#22, not granted by this packet. @@ -30,13 +35,13 @@ status transition. The official ERC remains Draft. 5. Are immutable enforcing logic, scoped transition references and the separation between hash compatibility, state enforcement and actual memory truth clear? -## Proposed upstream work, in order +## Remaining work, in order -Technical: obtain review of the above semantic clarifications, then submit a -focused official specification/CC0-asset PR. Preserve the assigned number, -authors, status, signing domain and wire format; link the discussion and tests. -After upstream merge, refresh the mirror and record a new official baseline. -Do not replace the pinned official mirror with unapproved project prose. +Technical: address technical/editor feedback on official PR #2147. The wording +and reference-code correction are separate commits. Number, authors, Draft +status, signing domain and wire format are preserved. After upstream merge, +refresh the official mirror and record a new baseline with the actual merged +commit. Until then, spec-baseline-2 and the original official mirror stay pinned. Operations: publish concise discussion updates with exact commits/results, track dissent and external implementation responses, and link reviewer-owned audit @@ -51,13 +56,17 @@ An audit does not automatically produce Review or Final. ## Suggested discussion text -> We have pinned the official Draft and published an explicit authorization -> decision table, together with a proposed malformed-return fix. We are seeking -> feedback on empty/custom ERC-1271 signatures, nonempty self-calls, and ordered -> ECDSA fallback for delegated accounts. Our tests and replay evidence state -> their exact scope; full independent-client work and audit acceptance remain -> open. Please identify any ambiguous acceptance rule before building against -> the proposal. We will incorporate feedback in a focused upstream revision -> before requesting a status transition. +> The focused authorization revision is now proposed in +> https://github.com/ethereum/ERCs/pull/2147 (candidate 397426c0; status remains +> Draft). We seek feedback on three specific points: ERC-1271 receives original +> empty/custom-length bytes before the ECDSA-only length guard; nonempty self-call +> signatures are validated; malformed return words fail wallet validation and +> continue to the same-byte ECDSA fallback. That fallback intentionally preserves +> an EIP-7702 account's key path even when its delegate rejects. Our pinned runner +> tests the exact candidate and reproduces the old dirty-padding failure. +> Independently maintained Python hash/state/EOA coverage is accepted only in its +> reported scope; independent ERC-1271/Safe/7702 coverage and current-source audit +> acceptance remain open. Which acceptance rule, if any, is still ambiguous for +> your implementation? This is technical feedback, not a status-change request. This text is a draft for community discussion, not a claim that it was posted. diff --git a/docs/review/upstream-authorization.md b/docs/review/upstream-authorization.md new file mode 100644 index 0000000..87771f0 --- /dev/null +++ b/docs/review/upstream-authorization.md @@ -0,0 +1,92 @@ +# Official authorization revision evidence + +Date: 2026-11-06. Technical proposal, not upstream acceptance or a release. + +## Exact inputs + +- Official PR: [ethereum/ERCs#2147](https://github.com/ethereum/ERCs/pull/2147). +- Candidate: `397426c046049e1a0c3436e543f9731408708fd3` in `XiaoHai67890/ERCs`. +- Wording commit: `6420c36e`; runtime commit: `397426c0`. +- Accepted official text remains `b9651bc84714e87e6361f4f6c0c8ff39cc265d0a`. +- [Candidate manifest](upstream-candidate.json) pins the proposal text, all three + CC0 source files, and the old-source negative control by commit and SHA-256. +- Harness originates from merged project main `5770a9090700cc6df925b7a399e53014eac16667`; + each generated record identifies the actual runner commit and dirty status. + +The candidate's three assets match the current local CC0 assets byte-for-byte. +Neither `erc/erc-8350.md` nor either historical baseline manifest is replaced. +The normative wording is a proposal resolving conflicting readings, not an +editorial-only change or already accepted rule. + +## Reproduce + +Requirements: Node 22, Foundry 1.7.1, solc 0.8.24, GitHub/solc download access. +From a clean checkout of this PR's head: + +```bash +cd contracts +forge install foundry-rs/forge-std@v1.16.2 --no-git +cd .. +node --test scripts/review-candidate.test.mjs +node scripts/review-candidate.mjs +``` + +The runner downloads pinned source bytes, verifies their digests, checks the +candidate against local CC0 assets, and explicitly remaps the test import to +the fetched directory. Candidate/control compiler outputs are isolated. It does +not execute scripts from the external repository. Its JSON/logs are saved under +`artifacts/review-candidate/` and uploaded by `Official revision evidence` CI. +The initial local run passed; the PR's clean-checkout CI is the durable run record. + +Expected results: + +| Layer | Required result | Scope | +| --- | --- | --- | +| Runner adversarial tests | 9 pass, zero skips | Hash/HTTP failure, missing suites, false approval, missing assets, reduced fuzz counts | +| Candidate authorization | 36 pass | 18 tests per Registry copy, three operations, atomic rejection; 256 padding fuzz runs per copy | +| Candidate signature vectors | 2 pass | Each test executes all 46 externally exported ECDSA cases | +| Old official negative control | Exactly 2 specified tests fail | Dirty-padding valid-key fallback, deterministic and fuzz case, using old official source | + +An empty suite, skip, fetch failure, malformed JSON or compiler failure cannot +satisfy the expected negative control. The expected failures are not unresolved +failures of the candidate. This gate proves the documented test outcomes, not +the completeness of the specification or an independent security assessment. + +## Evidence already merged + +[PR #36](https://github.com/AwareLiquid/AgentMemoryState/pull/36) and +[PR #37](https://github.com/AwareLiquid/AgentMemoryState/pull/37) are merged. +The [main acceptance record](https://github.com/AwareLiquid/AgentMemoryState/pull/37#issuecomment-6679669756) +binds clean commit `5770a909` to full workspace/Golden Vector checks, 155 TypeScript +and 120 Solidity tests, 58 real-account tests with local type-4 receipts, scoped +Python reproduction and both live replay suites. The new runner supplements, +not replaces, those gates. Static vectors cannot demonstrate wallet execution. + +## Concrete handoff and acceptance + +| Task | Responsibility | Output | Acceptance | +| --- | --- | --- | --- | +| Official technical review | Technical | PR #2147; answers to AQ-01/AQ-02/AQ-03 feedback | Review comments addressed; upstream merge observed, not inferred from CI | +| Community discussion | Operations | Targeted questions in the existing Magicians thread | Published URL and substantive responses/dissent recorded; draft prose is not a posted comment | +| Post-merge baseline | Technical | New numbered manifest and byte-identical official mirror | Actual merged SHA/digests verified; baselines 1/2 retained; notify implementers | +| Independent wallet coverage | Operations | Optional scope agreement in issue #21 | Contributor explicitly accepts a bounded task; no assumed maintenance commitment | +| Account conformance handoff | Technical | Existing 58-case harness, 46 vectors and rule table | External account results bind wallet/version/state, commands, outcomes and rejection snapshots; unsupported rows declared | +| Audit addendum | Operations | Reviewer-owned supplement via issues #11/#22 | Exact new candidate and runtime delta, methodology, findings, risk mapping and attestation; genuine acceptance before canonical record | +| Draft-to-Review submission | Technical | Separate status-only official PR after technical revision acceptance | Author requests peer review; no false claim that audit/independent tests guarantee editor merge | + +Full independent implementation and G4 are project quality/release gates, not +universal EIP-1 requirements to request Review. ZK profiles, markets, new SDK +features and deployments are not prerequisites for this focused revision. + +## Audit and deployment boundary + +The October reviewer-controlled report is archived as a provisional submission; +it is not absent, but canonical acceptance is still pending. The frozen target +`af5a75fb2db532fd5603554083d8895a825c2de2` is unchanged and does not cover AQ-03. +The [runtime delta](../security/aq03-runtime-delta.md) is the supplement input. +`pnpm security:rc` remains a failing release gate until actual current-scope +evidence and acceptance exist. CI scope checks do not mean security-ready. + +No deployment is changed by this work. An immutable existing address cannot be +patched by merging source. A future deployment needs separate bytecode/build, +Sourcify Exact Match and reachable-upgrade-path evidence for the actual network. diff --git a/docs/review/upstream-candidate.json b/docs/review/upstream-candidate.json new file mode 100644 index 0000000..e6a8af6 --- /dev/null +++ b/docs/review/upstream-candidate.json @@ -0,0 +1,17 @@ +{ + "schema": "erc8350/upstream-candidate/v1", + "status": "proposed-not-upstream-approved", + "repository": "XiaoHai67890/ERCs", + "commit": "397426c046049e1a0c3436e543f9731408708fd3", + "specSha256": "295ab952357cefe141f3f1df5ceaaed2fe40826939fd22c683519c747a0fc072", + "assets": { + "AgentMemoryStateRegistry.sol": "94c7f39920205ae581e6537828a80a3dcfeb0a283c833ed2b50bc10927a292c3", + "ECDSA.sol": "74d0a828f70af9384457e2cf5165638dcc44a7b9749b8dc80cb7c9bd2ba2ccf0", + "IAgentMemoryState.sol": "274bfee5de979ea164abcf30af6b9dace1fccdb42bee4cf59bdeb0a9621be93a" + }, + "negativeControl": { + "repository": "ethereum/ERCs", + "commit": "b9651bc84714e87e6361f4f6c0c8ff39cc265d0a", + "registrySha256": "6d99fc690abd776c53f607be9da1e25c2a48a80037088a3f3fb029f54943685b" + } +} diff --git a/docs/security/aq03-runtime-delta.md b/docs/security/aq03-runtime-delta.md index d49aa0d..20e02e7 100644 --- a/docs/security/aq03-runtime-delta.md +++ b/docs/security/aq03-runtime-delta.md @@ -48,3 +48,10 @@ upgrade or reuse of the old audit claim. The official Draft text remains pinned; empty/custom-signature clarifications are still proposals. The local CC0 asset fix must receive a separate upstream review before claiming ethereum/ERCs contains it. No production readiness is asserted. + +Update 2026-11-06: [official PR #2147](https://github.com/ethereum/ERCs/pull/2147) +proposes the exact local CC0 bytes at `397426c046049e1a0c3436e543f9731408708fd3`. +The [candidate gate](../review/upstream-authorization.md) verifies fetched source +digests, runs all 38 authorization/signature tests and requires two expected +dirty-padding failures against the old official source. Upstream acceptance and +the reviewer-owned current-source supplement remain pending. diff --git a/docs/spec-baseline/AUTHORIZATION.md b/docs/spec-baseline/AUTHORIZATION.md index 93ec143..63df7e3 100644 --- a/docs/spec-baseline/AUTHORIZATION.md +++ b/docs/spec-baseline/AUTHORIZATION.md @@ -57,10 +57,11 @@ with invalid non-empty bytes does not fall back to the direct-empty shortcut. ## Proposed clarification text -The following is proposed replacement wording for the signature-validation -subsection, not a replacement for the pinned official file. Submit it separately -upstream after reviewing AQ-01 through AQ-03. In particular, invalid-ABI handling -must be settled before this text is claimed to describe all existing behavior. +The following records the original project direction, not a replacement for the +pinned official file. The complete wording, including canonical return-word and +state-validation constraints, is now submitted in +[ethereum/ERCs#2147](https://github.com/ethereum/ERCs/pull/2147) at `397426c0`. +It remains a proposal until upstream review/merge, not a new official baseline. > The required account is the controller for registration, the current controller > for an authorization update, and the current authorizer for a transition. @@ -109,8 +110,9 @@ and rejection atomicity. Existing `ExternalReviewBoundary.t.sol`, encoding, callback, replay and state-linkage evidence. The separate [account conformance suite](../interop/account-conformance.md) adds pinned Safe 1.4.1 and real Prague delegation, including local mined type-4 receipts. Portable -signature data and external language adapters remain follow-ups; a mock's name -does not establish those capabilities. +signature data is now published as `test-vectors/signature-vectors-v1.json`, with TS and +Solidity adapters merged in PR #37. External wallet execution remains a follow-up; +a mock's name does not establish those capabilities. ## Gas and policy boundary diff --git a/docs/spec-baseline/OPEN_QUESTIONS.md b/docs/spec-baseline/OPEN_QUESTIONS.md index e4eb68f..b91128d 100644 --- a/docs/spec-baseline/OPEN_QUESTIONS.md +++ b/docs/spec-baseline/OPEN_QUESTIONS.md @@ -4,6 +4,11 @@ Current implementation baseline: **spec-baseline-2** (AQ-03 runtime fix). The official text is unchanged. Historical spec-baseline-1 remains byte-identical. See [AQ-03 change and audit delta](../security/aq03-runtime-delta.md). +Update 2026-11-06: AQ-01/AQ-02 wording and the AQ-03 CC0 asset correction are +submitted in [ethereum/ERCs#2147](https://github.com/ethereum/ERCs/pull/2147), +candidate `397426c046049e1a0c3436e543f9731408708fd3`. Submission is not acceptance; +the pinned official mirror is unchanged. See the [review evidence](../review/upstream-authorization.md). + Baseline: `spec-baseline-1`. Status: P1a inputs recorded; semantic clarifications are proposed, not approved upstream. Technical and operations are responsibility categories, both currently coordinated by `@XiaoHai67890`. @@ -17,7 +22,7 @@ categories, both currently coordinated by `@XiaoHai67890`. | AQ-03 | Baseline 2 fixes dirty-padding abort using a strict full-word comparison; regression tests cover both copies and all three operations | External audit and official upstream asset coverage remain pending | Technical | Review the runtime delta, obtain audit addendum and upstream review; do not inherit the old frozen report | | AQ-04 | Gas exhaustion, large returndata/resource limits and relayer underfunding | Non-blocking for implementation start; no liveness or production-readiness claim | Technical | Adequate/insufficient gas tests, rejection atomicity, RPC/client guidance; do not mandate an arbitrary universal gas allowance | | AQ-05 | Portable rejection semantics versus v2 reference error selectors/order | Reference-exact certification, not valid hash interoperability | Technical | Label each case by normative rule or reference choice; do not require unstandardized error selectors of other languages | -| AQ-06 | Real Prague delegation harness and actual local type-4 transactions now implemented; portable static signature data remains separate | Completion of portable P1b authorization evidence | Technical | Run `pnpm test:accounts`; publish CI artifacts, then add language-neutral signature vectors/adapters without changing v1/v2 | +| AQ-06 | Real Prague/type-4 harness and 46 portable ECDSA vectors/adapters merged in PRs #35/#37 | Independent external wallet execution remains separate in #21 | Technical | Maintainer account/signature gates passed on main 5770a909; retain zero-skip runs and explicit external coverage boundaries | | AQ-07 | Safe 1.4.1 source/handler/message encoding pinned and executable 2-of-2 / signed-message cases added | No claim about every Safe version or production deployment | Technical | Run `pnpm test:accounts`; inspect approved/unapproved/wrong-message and approveHash negative controls | | AQ-08 | Immutable enforcing logic and deployed-code identity | New release/deployment evidence, not a prerequisite to start an offline implementation | Technical | Reproduce bytecode; record Sourcify Exact Match, actual libraries/immutables and deployment transaction; inspect reachable upgrade paths and chain-specific SELFDESTRUCT semantics | @@ -47,23 +52,15 @@ Historical failures and prior successful runs describe their pinned versions and block ranges; neither should be erased. Upstream PR #2 merged on 2026-10-29. [Project PR #33](https://github.com/AwareLiquid/AgentMemoryState/pull/33) repins that reviewed revision, records a fresh two-suite PASS and adds trusted regression -notifications. Issues #29/#31 track merge closure, not waiting for upstream now. - -- Technical: preserve header, MPT, second-RPC and nonzero-failure checks; document - the root cause against execution specifications and publish fresh run identities. -- Operations: seek independent review. Choose a follow-up date, not a deadline - after which a maintainer fork becomes independent evidence automatically. -- Technical: keep the full fork commit/digest pinned until an acceptable upstream - revision exists. This need not block unrelated baseline/implementation work. -- Operations: close #29 only after its criteria are met, or explicitly transfer - every outstanding independent-verification criterion to a named open issue. - #31's repin-only description does not automatically cover those criteria. -- Technical: scheduled CI already exists. Add deduplicated failure/recovery issue - notifications in trusted scheduled/manual runs, with write permissions isolated - from untrusted PR code. Default GitHub email routing is not a team alert policy. - -The P0 actions above are implemented separately in PR #33. Its recorded result -and issue comments distinguish implementation/testing from merge/issue closure. +notifications. The merged upstream commit/digest replaces the temporary fork pin. +Header, MPT, second-RPC and nonzero-failure checks are retained. Notification write +permissions are isolated from untrusted PR code; default GitHub email routing is +not used as the team alert policy. + +These P0 actions were merged in PR #33. Issues #29/#31 were closed on +2026-11-01 with upstream confirmation and preserved records. Fresh main acceptance +at `5770a909` also passed both live replay suites. These are no longer pending +merge/closure tasks; issue #21 separately tracks independent full-client coverage. The notification job has not been tested by deliberately failing production CI; deduplication/recovery logic is locally checked with a mocked GitHub client. diff --git a/docs/spec-baseline/README.md b/docs/spec-baseline/README.md index c0bd278..10dfcbe 100644 --- a/docs/spec-baseline/README.md +++ b/docs/spec-baseline/README.md @@ -69,7 +69,15 @@ Full checkout validation remains `pnpm conformance:clean`. The characterization tests exercise the existing implementation. They are not a new externally maintained implementation, a complete portable authorization suite, a real Safe integration, or a real 7702 authorization-transaction test. -Those P1b deliverables remain explicitly tracked. +Separate P1b deliverables, rather than these characterization tests alone, +provide that additional coverage. + +Update 2026-11-06: separate P1b work has since merged real Safe/7702 account +scenarios, portable 46-case signature adapters, and scoped independent Python +reproduction. Those results do not expand the baseline characterization tests' +scope. [Official PR #2147](https://github.com/ethereum/ERCs/pull/2147) now proposes +AQ-01/AQ-02 wording and the AQ-03 asset fix. The [review packet](../review/draft-to-review.md) +records completed evidence and remaining upstream/external acceptance work. ## Confirmation and notification diff --git a/scripts/review-candidate.mjs b/scripts/review-candidate.mjs new file mode 100644 index 0000000..87cd395 --- /dev/null +++ b/scripts/review-candidate.mjs @@ -0,0 +1,135 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; + +const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const names = ["AgentMemoryStateRegistry.sol", "ECDSA.sol", "IAgentMemoryState.sol"]; +const suites = { + "test/unit/SpecBaselineAuthorization.t.sol:SpecBaselineOfficialAuthorizationTest": 18, + "test/unit/SpecBaselineAuthorization.t.sol:SpecBaselineProductAuthorizationTest": 18, + "test/unit/SignatureVectors.t.sol:OfficialSignatureVectorTest": 1, + "test/unit/SignatureVectors.t.sol:ProductSignatureVectorTest": 1, +}; +const regressions = [ + "test_DirtyPaddingDoesNotVetoValidECDSAFallback()", + "testFuzz_DirtyPaddingFallsBack(uint224)", +]; +export const sha256 = (bytes) => createHash("sha256").update(bytes).digest("hex"); + +export function validateManifest(m) { + assert.equal(m.schema, "erc8350/upstream-candidate/v1"); + assert.equal(m.status, "proposed-not-upstream-approved"); + assert.equal(m.repository, "XiaoHai67890/ERCs"); + assert.match(m.commit, /^[a-f0-9]{40}$/u); + assert.match(m.specSha256, /^[a-f0-9]{64}$/u); + assert.deepEqual(Object.keys(m.assets).sort(), [...names].sort()); + for (const hash of Object.values(m.assets)) assert.match(hash, /^[a-f0-9]{64}$/u); + assert.deepEqual(m.negativeControl, { + repository: "ethereum/ERCs", + commit: "b9651bc84714e87e6361f4f6c0c8ff39cc265d0a", + registrySha256: "6d99fc690abd776c53f607be9da1e25c2a48a80037088a3f3fb029f54943685b", + }); +} + +export async function fetchPinned(url, hash, fetcher = fetch) { + const response = await fetcher(url, { signal: AbortSignal.timeout(30000) }); + assert.equal(response.ok, true, `HTTP ${response.status}: ${url}`); + const bytes = Buffer.from(await response.arrayBuffer()); + assert.equal(sha256(bytes), hash, `Source digest mismatch: ${url}`); + return bytes; +} + +export function checkResults(value, negative = false) { + const expected = negative ? { [Object.keys(suites)[0]]: 2 } : suites; + assert.deepEqual(Object.keys(value).sort(), Object.keys(expected).sort(), "Missing/unexpected suite"); + let count = 0; + for (const [name, size] of Object.entries(expected)) { + const results = value[name].test_results; + assert.equal(Object.keys(results).length, size, `Wrong test count: ${name}`); + if (negative) assert.deepEqual(Object.keys(results).sort(), [...regressions].sort()); + for (const [test, result] of Object.entries(results)) { + assert.equal(result.status, negative ? "Failure" : "Success", `Unexpected result: ${test}`); + if (!negative && test.startsWith("testFuzz_")) { + assert.equal(result.kind.Fuzz.runs, 256, "Expected 256 fuzz runs"); + } + count++; + } + } + return count; +} + +async function main() { + assert.equal(process.argv.length, 2, "Usage: node scripts/review-candidate.mjs"); + const output = resolve(root, "artifacts/review-candidate"); + mkdirSync(output, { recursive: true }); + const report = { schema: "erc8350/upstream-candidate-result/v1", result: "fail", commands: [] }; + function run(command, args, allowFailure = false, env = {}) { + const result = spawnSync(command, args, { + cwd: root, encoding: "utf8", maxBuffer: 32 * 1024 * 1024, timeout: 300000, + env: { ...process.env, ...env }, + }); + report.commands.push({ command, args, env, exitCode: result.status }); + assert.ifError(result.error); + if (!allowFailure) assert.equal(result.status, 0, result.stderr || result.stdout); + return result; + } + try { + report.harnessCommit = run("git", ["rev-parse", "HEAD"]).stdout.trim(); + report.dirtyCheckout = run("git", ["status", "--porcelain"]).stdout.trim() !== ""; + report.foundry = run("forge", ["--version"]).stdout.trim(); + const manifest = JSON.parse(readFileSync(resolve(root, "docs/review/upstream-candidate.json"))); + validateManifest(manifest); + report.inputs = manifest; + const raw = (repo, commit, path) => `https://raw.githubusercontent.com/${repo}/${commit}/${path}`; + const spec = await fetchPinned(raw(manifest.repository, manifest.commit, "ERCS/erc-8350.md"), manifest.specSha256); + assert.match(spec.toString(), /^status: Draft$/mu); + writeFileSync(resolve(output, "erc-8350.md"), spec); + for (const label of ["candidate", "negative-control"]) { + const negative = label === "negative-control"; + const source = negative ? manifest.negativeControl : manifest; + const assetDir = resolve(output, label, "erc-8350"); + mkdirSync(assetDir, { recursive: true }); + for (const name of names) { + const hash = negative && name === names[0] ? source.registrySha256 : manifest.assets[name]; + const bytes = await fetchPinned(raw(source.repository, source.commit, `assets/erc-8350/${name}`), hash); + if (!negative) assert.deepEqual(bytes, readFileSync(resolve(root, "erc/assets/erc-8350", name)), + `Candidate differs from tested project asset: ${name}`); + writeFileSync(resolve(assetDir, name), bytes); + } + // Isolated outputs and an explicit import remapping bind tests to fetched bytes. + const args = ["test", "--root", "contracts", "--json", "--no-cache", + "--out", resolve(output, label, "out"), + "--remappings", `erc8350-assets/=${assetDir}/`, + "--fuzz-runs", "256", "--fuzz-seed", "0x8350", + "--match-contract", negative ? "^SpecBaselineOfficialAuthorizationTest$" + : "^(SpecBaseline.*AuthorizationTest|OfficialSignatureVectorTest|ProductSignatureVectorTest)$"]; + if (negative) args.push("--match-test", "^(test_DirtyPaddingDoesNotVetoValidECDSAFallback|testFuzz_DirtyPaddingFallsBack)\\("); + const result = run("forge", args, true, { + FOUNDRY_FUZZ_FAILURE_PERSIST_DIR: resolve(output, label, "fuzz-failures"), + FOUNDRY_TEST_FAILURES_FILE: resolve(output, label, "test-failures"), + }); + writeFileSync(resolve(output, `${label}.json`), result.stdout); + writeFileSync(resolve(output, `${label}.stderr.txt`), result.stderr); + assert.equal(result.status, negative ? 1 : 0, `Unexpected forge exit: ${label}`); + const count = checkResults(JSON.parse(result.stdout), negative); + report[label] = { result: "pass", tests: count, expectedStatus: negative ? "Failure" : "Success", + outputSha256: sha256(result.stdout) }; + console.log(`${label}: ${count} expected outcomes, zero skips`); + } + report.result = "pass"; + } catch (error) { + report.error = error.message; + process.exitCode = 1; + console.error(error.message); + } finally { + report.generatedAt = new Date().toISOString(); + writeFileSync(resolve(output, "report.json"), `${JSON.stringify(report, null, 2)}\n`); + } +} + +if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) { + await main(); +} diff --git a/scripts/review-candidate.test.mjs b/scripts/review-candidate.test.mjs new file mode 100644 index 0000000..194d290 --- /dev/null +++ b/scripts/review-candidate.test.mjs @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; +import { checkResults, fetchPinned, sha256, validateManifest } from "./review-candidate.mjs"; + +const manifest = JSON.parse(readFileSync(new URL("../docs/review/upstream-candidate.json", import.meta.url))); +test("candidate is pinned, separate from official acceptance", () => validateManifest(manifest)); +for (const [name, mutate] of [ + ["moving commit", (m) => { m.commit = "main"; }], + ["false approval", (m) => { m.status = "approved"; }], + ["missing asset", (m) => { delete m.assets["ECDSA.sol"]; }], + ["path traversal", (m) => { m.assets["../outside.sol"] = "a".repeat(64); }], + ["wrong control", (m) => { m.negativeControl.commit = m.commit; }], +]) { + test(`rejects ${name}`, () => { + const changed = structuredClone(manifest); + mutate(changed); + assert.throws(() => validateManifest(changed)); + }); +} +test("source verification fails closed on HTTP, network and digest errors", async () => { + const bytes = Buffer.from("exact bytes"); + const fetcher = async () => new Response(bytes); + assert.deepEqual(await fetchPinned("https://example.invalid/source", sha256(bytes), fetcher), bytes); + await assert.rejects(fetchPinned("u", "0".repeat(64), fetcher), /digest mismatch/); + await assert.rejects(fetchPinned("u", sha256(bytes), async () => new Response("", { status: 404 })), /HTTP 404/); + await assert.rejects(fetchPinned("u", sha256(bytes), async () => { throw new Error("offline"); }), /offline/); +}); +const control = () => ({ "test/unit/SpecBaselineAuthorization.t.sol:SpecBaselineOfficialAuthorizationTest": { + test_results: Object.fromEntries([ + "test_DirtyPaddingDoesNotVetoValidECDSAFallback()", "testFuzz_DirtyPaddingFallsBack(uint224)", + ].map((name) => [name, { status: "Failure" }])), +} }); +test("requires exactly the two old-source regression failures", () => { + assert.equal(checkResults(control(), true), 2); + assert.throws(() => checkResults({}, true), /suite/); + assert.throws(() => checkResults(control()), /suite/); + for (const status of ["Success", "Skipped"]) { + const value = control(); + Object.values(Object.values(value)[0].test_results)[0].status = status; + assert.throws(() => checkResults(value, true), /Unexpected result/); + } +}); +test("candidate needs all four suites and rejects skipped tests or reduced fuzzing", () => { + const result = {}; + for (const [name, count] of [ + ["test/unit/SpecBaselineAuthorization.t.sol:SpecBaselineOfficialAuthorizationTest", 18], + ["test/unit/SpecBaselineAuthorization.t.sol:SpecBaselineProductAuthorizationTest", 18], + ["test/unit/SignatureVectors.t.sol:OfficialSignatureVectorTest", 1], + ["test/unit/SignatureVectors.t.sol:ProductSignatureVectorTest", 1], + ]) result[name] = { test_results: Object.fromEntries(Array.from({ length: count }, (_, i) => + [i === 0 ? "testFuzz_example()" : `test_${i}()`, { status: "Success", kind: { Fuzz: { runs: 256 } } }])) }; + assert.equal(checkResults(result), 38); + const first = Object.values(Object.values(result)[0].test_results)[0]; + first.kind.Fuzz.runs = 1; + assert.throws(() => checkResults(result), /256 fuzz/); + first.kind.Fuzz.runs = 256; + first.status = "Skipped"; + assert.throws(() => checkResults(result), /Unexpected result/); +});