Freeze the 9 EFS schemas + upgradeable resolvers + CREATE3/Safe deploy + SystemAccount#24
Open
JamesCarnley wants to merge 79 commits into
Open
Freeze the 9 EFS schemas + upgradeable resolvers + CREATE3/Safe deploy + SystemAccount#24JamesCarnley wants to merge 79 commits into
JamesCarnley wants to merge 79 commits into
Conversation
Freeze ANCHOR/PROPERTY/DATA/PIN/TAG/MIRROR/LIST/LIST_ENTRY. Drop BLOB (redundant w/ DATA+MIRROR, unvalidated) and NAMING (tooling); defer SORT_INFO. All deferrals are additive (no orphaning). Resolvers move to guarded-initializer proxies deployed via CREATE2 so the frozen UID embeds the proxy (not impl) address; register only after James signs the frozen-UID table. Simple single-admin upgrade key for Sepolia. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…/burn ADR-0049: freeze DATA as-is (bytes32 contentHash, uint64 size); contentHash=0 is first-class "no inline hash" (zero-download remote pins); durable hash story moves to self-describing multihash/CID PROPERTYs. No field change, no migration. ADR-0048 r2 (after 91+38-agent review): all 8 schemas validated solid-as-is; register-LAST + atomic CREATE3 deploy+init; ERC-7201 + _disableInitializers; burn-to-immutable gated by full invariant suite + 14-day soak; supersedes ADR-0030 no-proxy clause (proxy iff key burned before first mainnet attest). Write-time pass found 2 resolver-logic bugs (MIRROR uri allowlist, ANCHOR name encoding) — not freeze blockers. Freeze table + pre-burn checklist updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ADR-0049 r2: DATA becomes empty "" (pure identity), revocable=false. contentHash+size move to trust-scoped reserved-key PROPERTYs (never-trust- client; lets you pin a 10GB IPFS file with zero download). Real Tier-1 reshape (new DATA UID, drops dataByContentKey) — safe now, nothing frozen on Sepolia yet. find-by-hash = general on-chain property index (resolver logic, upgradeable, frozen at burn — does not block freeze). ADR-0050: REDIRECT schema (9th) "bytes32 target, uint8 kind" revocable=true, AliasResolver. Covers canonical/sameAs/supersededBy/symlink. Only uint8 kind frozen; kind taxonomy is upgradeable logic. Write-time guards (no self-loop, typed); multi-hop cycle resolution is a Durable read-time spec (cycle -> lowest-UID-in-SCC) with conformance vectors before durable seeding. Hardlinks already native. Freeze table -> 9 schemas. Permanence-tier: Etched Refs: ADR-0048, ADR-0049, ADR-0050 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
kind is an open relationship vocabulary (discriminator, not a counter), so ADR-0047's rule says "narrow" — but widening is free (uint8 and uint16 both pad to one 32-byte ABI word; identical payload/gas), and the field string is frozen forever. On an irreversible surface where the safe choice costs nothing, take the headroom: uint16 (65536 kinds) removes any ceiling while still reading as a bounded discriminator (uint256 would misrepresent it as a counter). LIST.targetType stays uint8 — closed 3-mode set, not an open vocab. Permanence-tier: Etched Refs: ADR-0050, ADR-0047 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…chor) Capture why kind stays uint16 enum: open anchor-pointer vocabulary is unenforceable by the resolver (defeating the reason REDIRECT is a first-class schema) and makes every kind permissionlessly followable; open descriptive relationships are TAG's job. Decision landed; rationale preserved. Permanence-tier: Etched Refs: ADR-0050 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Comprehensive, reviewable build plan: EFSUpgradeableResolver base, per-resolver initializer+ERC-7201 refactor, empty-DATA reshape, REDIRECT/AliasResolver, MIRROR/ANCHOR resolver-logic fixes, CREATE3 register-last deploy pipeline, conventions + Sepolia freeze + round-trip. Test-first, with verify gates and a self-review section for AI/human critique. One doc by design (holistic review). Refs: ADR-0048, ADR-0049, ADR-0050, docs/SEPOLIA_FREEZE_TABLE.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Architecture holds; plan needs a revision pass. Clusters 66 verified findings into 10 themes (split into 3 PRs; on-chain ListEntry self-UID verify; empty-DATA decode/event/deploy-string; DATA ripple ownership; CREATE3 spike; storage-layout+upgrade-with-state tests; SORT_INFO wiring; getEAS guard; anchor-encoding ordering; rollback+redirect-following). Records 11 refuted over-statements incl. the '_eas-under-delegatecall unsound' claim. Refs: docs/plans/2026-06-02-schema-freeze-build-plan.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Single PR with staged commits (DATA reshape -> proxy -> REDIRECT), per James. Critical fixes: on-chain ListEntry self-UID verify (+getter); empty-DATA abi.decode deletion + DataCreated event migration + deploy-string change; DATA-ripple consumer tasks + production-client cross-repo flag; CreateX spike-first (BLOCKING); validateUpgrade + upgrade-with-state tests scheduled; getEAS guard; SORT_INFO wiring removed; rollback procedure; anchor-encoding sequenced before freeze. Refs: docs/plans/2026-06-02-schema-freeze-plan-critique.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Headline: irreversible surface ready (0 real freeze-blockers; the 5 tagged were over-tags — known-coupling/deferred-component/build-task; schema->resolver binding confirmed sound to freeze). Real risk = the dev layer: every persona only partially succeeded. Top pre-hackathon themes (mostly additive/cheap): missing Solidity SDK library; no PIN/TAG indexer events; getter-naming sprawl; pagination inconsistency; missing query helpers/reverse-lookups; docs gaps. 26 mainnet proposals (decompose EFSIndexer, typed EVENT edge, property index, redirect-following, signatures, ERC-165). Sequencing decision surfaced. Refs: docs/plans/2026-06-02-schema-freeze-build-plan.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ta-permanent Capstone of the contract+API analysis. Verdict: foundation is mainnet-forward (all forward-compat lenses safe-with-fixes; CREATE3 makes Sepolia structure == mainnet structure -> data persists, no rewrite). EIP-170 resolved EMPIRICALLY: EFSIndexer = 14912 B (61% of 24576 limit, ~9.6KB headroom) -> no forced split; decomposition stays optional/post-freeze behind the proxy. Documents the 9- contract set, the frozen-vs-contained-vs-redeployable surface, and confirms the pre-Sepolia gates are all already in build-plan r2 (+ a cheap CI size gate). Refs: 2026-06-02-schema-freeze-build-plan.md, ADR-0048/0049/0050 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Per ADR-0049, DATA becomes an empty schema ("", revocable=false): a DATA
attestation now asserts only "a file identity exists". Its EAS UID is the
file's identity; MIRRORs, PINs, and PROPERTYs reference that UID. contentHash
and size move OUT of DATA into lens-scoped reserved-key PROPERTYs bound to the
DATA UID, because a content hash is client-supplied and unverifiable on-chain
and must not be welded into identity. This is safe to do now (pre-launch,
nothing frozen on Sepolia; localhost data is throwaway).
EFSIndexer.onAttest DATA branch: deleted the
abi.decode(data, (bytes32,uint64)) (it reverts on zero-length payload) and the
dataByContentKey write. DATA now only validates standalone (refUID==0) +
non-revocable, then indexes the bare UID. The dataByContentKey mapping is kept
DECLARED (storage-order preserved) but is now unused/advisory.
ABI change (downstream indexers must re-bind): DataCreated dropped its
contentHash param. New signature:
event DataCreated(bytes32 indexed dataUID, address indexed attester)
Also dropped BLOB and NAMING schema registrations and the SchemaNameIndex
deployment from deploy/01_indexer.ts; removed EFSIndexer's BLOB_SCHEMA_UID
immutable + constructor param + index()/indexBatch() skip branches; updated
the constructor call (now 4 args) and the nonce offset (8 -> 6).
Specs updated in the same change: specs/02 §3 (DATA = empty, contentHash/size
as reserved-key PROPERTYs), overview.md (schema table, three-layer model,
dedup paragraph, upload-flow steps 2/4). Status lines of ADR-0002/0004/0005
annotated "superseded in part by ADR-0049".
Out-of-scope consumers that deeply depend on DATA fields / dataByContentKey
are left with // AGENT-NOTE: A2 ripple markers for the follow-up A2 task
(EFSFileView.sol, the simulate-*/seed-impl scripts, nextjs CreateItemModal)
and are NOT refactored here.
Tests: test/EFSIndexer.test.ts reshaped to register empty DATA and attest
zero-length payloads; added a TDD test asserting empty DATA is accepted and
indexed. 37 passing. The 9 other test files that construct EFSIndexer with the
old 5-arg constructor / create DATA-with-fields are A2 ripple, deferred.
Permanence-tier: Etched
Refs: ADR-0049
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The DATA-empty reshape (ADR-0049, commit 1eb8a64) left 9 hardhat test files red and several other consumers binding to removed fields/symbols. This fixes the fallout so the suite is green and nothing depends on the removed DATA fields. Contracts: - EFSFileView: stop decoding DATA inline fields in the listing path (the old abi.decode (bytes32,uint64) reverted on real empty DATA); the FileSystemItem.contentHash view field is kept ABI-stable but always 0. getCanonicalData is now a pure no-op returning bytes32(0) and the unused dataByContentKey interface entry was dropped. contentHash/size are reserved-key PROPERTYs now (ADR-0049); reverse-lookup is future on-chain property-index work. Tests (9 files): switch EFSIndexer ctor to 4 args (drop blobSchemaUID), register DATA as the empty schema (""), fix nonce arithmetic where the dropped BLOB registration shifted predicted addresses, mint zero-length DATA ("0x"), and update/remove contentHash/size/dedup sub-assertions to the new reality. Scripts: seed-impl, simulate-file-browser, simulate-sort-overlay, simulate-transports, simulate-lists now mint empty DATA and drop the content-dedup (dataByContentKey) assertions. nextjs debug UI (minimal): drop BLOB_SCHEMA_UID reads + BLOB forms/decoders, stop decoding DATA fields, and remove the dataByContentKey dedup read in the upload flow (DATA create now sends empty data). useSchemaRegistry no longer exposes BLOB. Verified: yarn hardhat compile clean; yarn hardhat test 362 passing / 0 failing; yarn next:check-types exit 0. Every prior "AGENT-NOTE: A2 ripple" is resolved or replaced with a forward-looking note pointing at the future PROPERTY/REDIRECT (ADR-0050) work. Note: deploy/01_indexer.ts carries a pre-existing tsc error from the reshape commit (untouched here); the three gates above are green regardless. deployedContracts.ts intentionally NOT regenerated (needs a fork RPC — deploy phase). Permanence-tier: Durable Refs: ADR-0049 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Removing BLOB/NAMING removed the only schema entries carrying noResolver, so `if (schema.noResolver)` became a tsc error (TS2339) and dead code — all 5 remaining schemas resolve to the indexer or the edge resolver. Fixes the red type-check the Group A reviewer flagged. Permanence-tier: Durable Refs: ADR-0049 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Proxy-refactor deps (plan r2 Phase 0.1, Tier-2, pre-approved). Pinned contracts-upgradeable to EXACTLY 5.0.2 to match @openzeppelin/contracts 5.0.2 — the caret range pulled 5.6.1, and the upgradeable variants must match the base contracts version or risk import/behavior skew. hardhat-upgrades@4.0.1 provides validateUpgrade for storage-layout gating. Permanence-tier: Durable Refs: ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Shared base every EFS schema resolver will inherit to be deployed behind an upgradeable proxy. The logic iterates during dev; the proxy ADDRESS (baked into each schema UID) stays stable, so the upgrade key is burned before mainnet. Why _eas stays a constructor immutable (not proxy storage): EAS is a per-chain constant, immutables live in implementation bytecode and resolve correctly under the proxy's delegatecall, and EAS invokes resolvers via CALL (not delegatecall) so onlyEAS (msg.sender == _eas) holds. The accepted rule: every implementation upgrade MUST be deployed with the same EAS address; the deploy/verify gate asserts getEAS() == expectedEAS. The constructor calls _disableInitializers() so the implementation itself can never be initialized (Parity/Wormhole class). All EFS-specific per-deployment state (schema UIDs, partner refs) is deliberately deferred to each subclass's ERC-7201 namespaced storage, set in a guarded initialize() — NOT here. The base is intentionally minimal. Two toolchain deviations from the task spec, logged in decisions.md: - Did NOT wire `import "@openzeppelin/hardhat-upgrades"` into hardhat.config.ts: the pinned 4.0.1 targets Hardhat 3 and crashes config load under this repo's Hardhat 2.22. Needs an @^3 re-pin (human/Tier 2). - Added a public getEAS() to the base: installed EAS SchemaResolver keeps _eas internal with no getter, but the verify-gate and base NatSpec need one. It's an immutable read, no proxy storage. Tests (test/Upgradeability.test.ts) deploy the proxy via a manual ERC1967Proxy (spec-sanctioned alternative, no plugin dependency): impl initializer is locked, proxy init works once then locks, getEAS() reads through the proxy. RED confirmed by removing _disableInitializers first. Full suite 365 passing / 0 failing. Permanence-tier: Etched Refs: ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… plugin B0 mistakenly installed hardhat-upgrades@4.0.1, which targets Hardhat 3 and crashes config load on this repo's Hardhat 2.22.19. Re-pinned to ^3 (3.9.1, the HH2 line: peers hardhat-ethers 3 + ethers 6, both satisfied) and added `import "@openzeppelin/hardhat-upgrades"` to hardhat.config.ts. Config now loads and compiles clean; the upgrades/validateUpgrade namespace is available for the storage-layout gate + upgrade-with-state test (B4). Permanence-tier: Durable Refs: ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
EFSIndexer now inherits EFSUpgradeableResolver + OwnableUpgradeable and is
deployed behind an ERC1967 proxy (ADR-0048). Per-deployment config that was
constructor immutables — ANCHOR/PROPERTY/DATA schema UIDs — migrates into
ERC-7201 namespaced storage (efs.indexer.config, slot
0x8236c748e6a502fa91232b6ce96a08db6ef6eb51d97817035831708b310c8900) written
once in a guarded initialize(anchor, property, data, owner_). Immutables live
in impl bytecode and would read construction-time values under the proxy's
delegatecall, so they can't carry per-proxy config.
The public getter NAMES (ANCHOR_SCHEMA_UID() etc.) are preserved as view
functions over the config struct so the external ABI is unchanged. Internal
call sites read the struct directly to avoid extra SLOADs.
wireContracts() and setSortsAnchor() swap from DEPLOYER-gating to onlyOwner
(OwnableUpgradeable, set in initialize); their one-shot edgeResolver/
sortsAnchorUID guards are kept (proxy storage, survive upgrades). The former
DEPLOYER immutable is removed, but DEPLOYER() is kept as a thin owner()-backed
alias because EFSRouter (default-lens fallback) and the nextjs explorer bind to
that getter — refactoring those is a later task.
getEAS() reconciliation: the duplicate is removed; EFSIndexer inherits the
base's getEAS() (constructor-immutable EAS, correct under delegatecall).
Storage layout: existing mappings keep slots 0-31 byte-identical (the migrated
immutables occupied no storage slots; OZ 5 Ownable/Initializable are
ERC-7201-namespaced), verified via the compiled storageLayout.
Tests deploy EFSIndexer behind a proxy via a new
test/helpers/deployIndexerProxy.ts (manual TestERC1967Proxy + init-calldata,
matching test/Upgradeability.test.ts); schema UIDs register against the proxy
address. New TDD coverage proves initialize-once / revert-on-second,
getEAS()/config through the proxy, identical ANCHOR indexing through the proxy,
and onlyOwner gating. 369 passing / 0 failing.
deploy/01_indexer.ts is made transitional (impl + TestERC1967Proxy + initialize
at the resolver-predicted nonce) with an AGENT-NOTE that full CREATE3 +
register-last is Phase D. No test runs the deploy scripts, so this does not
affect the suite; downstream scripts (02-09) still getContract("Indexer")
expecting the EFSIndexer ABI while "Indexer" is now the proxy artifact — flagged
for Phase-D resequencing.
Permanence-tier: Etched
Refs: ADR-0048
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…den MIRROR schemes) Make MirrorResolver and ListResolver upgradeable behind ERC1967 proxies, following the proven EFSIndexer pattern (commit b1909dc): each inherits the shared EFSUpgradeableResolver base (EAS `_eas` constructor-immutable + OZ Initializable + impl-locking `_disableInitializers()` + public getEAS()). MirrorResolver: - Inherits EFSUpgradeableResolver + OwnableUpgradeable. - Former `indexer` immutable moves into an ERC-7201 namespaced struct under its own namespace `efs.mirror.config` (slot 0x3f72924bb6e35e0588c2971086d9c6a0d31d2c8b5992f0e713fd93fab2407a00), set once in initialize(indexer_, owner_) which calls __Ownable_init(owner_). - Former `_deployer` immutable + `msg.sender == _deployer` guard on setTransportsAnchor() replaced by `onlyOwner`; the one-shot `require(transportsAnchorUID == EMPTY_UID)` guard is kept. - `transportsAnchorUID` storage var keeps its slot (verified slot 0, offset 0 via compiled storageLayout); Initializable/Ownable state lives in ERC-7201 namespaced storage, so sequential layout is unchanged. ListResolver: - Stateless pure validation: `contract ListResolver is EFSUpgradeableResolver` with an empty `initialize()` (no config/owner). Only the impl's `_disableInitializers()` is load-bearing. Widen MIRROR URI scheme allowlist (planned MIRROR change per ADR-0048): add ftp://, s3://, gs://, dat://, rsync://, bittorrent:// to the existing {web3,ipfs,ar,https,magnet} set. Scheme safety is a client-render concern (the router never executes URIs); javascript:/data: stay rejected to block XSS. Supersedes ADR-0023 in part (Status note appended; XSS-rejection intent preserved) and reflected in specs/02 §Mirror. Tests deploy each refactored resolver behind a manual TestERC1967Proxy + initialize() via a new reusable helper test/helpers/deployResolverProxy.ts (impl, then proxy, two deployer txs). The proxy ADDRESS is what's baked into the MIRROR / LIST schema UIDs, so nonce predictions in EFSTransports, EFSRouter, EFSDataModel.e2e, Lists.unit, and Lists.conformance shift accordingly. Added TDD coverage: s3://+ftp:// mirror acceptance, javascript: still rejected, initialize-once-then-revert and getEAS()-through-proxy for both resolvers, and onlyOwner gating + one-shot guard on setTransportsAnchor. Deploy scripts (deploy/05_mirrors.ts, deploy/09_lists.ts) intentionally untouched — proxy-ification of the deploy pipeline is deferred to Phase D. Suite: 379 passing / 0 failing (was 369; +10 new tests). Permanence-tier: Etched Refs: ADR-0048, ADR-0023 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
EdgeResolver now runs behind an ERC1967 proxy (ADR-0048), matching the reviewed-SAFE pattern on EFSIndexer and MirrorResolver/ListResolver. Immutable -> storage migration: the four former constructor immutables (PIN_SCHEMA_UID, TAG_SCHEMA_UID, indexer, schemaRegistry) move into an ERC-7201 namespaced struct under its own unique namespace `efs.edge.config` (slot 0xa7006bf19fd32b664fa0a26984d3f000dbb27df471e5d0cacd76f128f9d5e600). Immutables live in impl bytecode and would read the impl's construction-time values, not the proxy's, under delegatecall; the namespaced slot sits high, away from the sequential mappings. The public getter NAMES are preserved as views over the struct so the ABI is unchanged. Invariants moved to initialize: the three constructor requires (pinSchemaUID != 0, tagSchemaUID != 0, pinSchemaUID != tagSchemaUID) now live in `initialize(bytes32,bytes32,IEFSIndexerForEdges,ISchemaRegistry)` guarded by OZ `initializer`. EdgeResolver has NO deployer/owner-gated functions, so it does NOT inherit OwnableUpgradeable (kept minimal); config is write-once via initialize and never mutated after. Storage-layout confirmation: dumped the compiled storageLayout before vs after. Every consensus-critical PIN/TAG mapping keeps its exact slot (ADR-0009): _activeEdge=0, _activeCount=1, _edgeDefinitions=2, _hasEdgeDef=3, _targetsByDef=4, _hasTargetForDef=5, _childrenWithEdge=6, _isChildWithEdge=7, _activeTotalByDefAndAttester=8, _activeBySlot=9, _activeByAAS=10, _activeByAASIndex=11 — identical before and after. No mapping reordered, retyped, inserted, or removed. Tests deploy EdgeResolver behind a TestERC1967Proxy via the deployResolverProxy helper; nonce predictions shifted by +1 (the proxy, not a single constructor deploy, is the address baked into the PIN/TAG schema UIDs) across EdgeResolverPin/Tag/Contains, EFSRouter, EFSDataModel.e2e, EFSTransports, EFSFileView. Added proxy-specific coverage: initialize-once-then-revert, impl initializer locked, the three invariant reverts, getEAS() through the proxy, and the four former-immutable getters reading namespaced config. PIN cardinality-1 supersession and TAG swap-and-pop now exercise the proxy path. Full suite: 386 passing / 0 failing (was 379; +7 proxy tests). No deploy script touched. Permanence-tier: Etched Refs: ADR-0048, ADR-0041, ADR-0009 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…tialize)
ListEntryResolver now extends EFSUpgradeableResolver and runs behind an
ERC1967 proxy (ADR-0048), like EFSIndexer/EdgeResolver/Mirror/List.
The critical bug this fixes: the contract derived its own LIST_ENTRY
schema UID in the CONSTRUCTOR as
keccak256(LIST_ENTRY_DEFINITION, address(this), true). Under a proxy the
constructor runs in the IMPLEMENTATION's context, so address(this) is the
impl address — but the LIST_ENTRY schema is registered in EAS against the
PROXY address. The stored UID and the registered UID would diverge, and
onAttest/onRevoke (which revert WrongSchema when a.schema != stored UID)
would reject EVERY genuine LIST_ENTRY forever. The self-UID derivation now
lives in initialize(), where the proxy's delegatecall makes
address(this) == the proxy, matching the registered resolver.
Changes:
- constructor(IEAS eas) EFSUpgradeableResolver(eas) {} (base disables
initializers on the impl); no OwnableUpgradeable (no owner-gated fns).
- initialize(bytes32 listSchemaUID): require != 0, set LIST schema UID, and
self-derive the LIST_ENTRY UID against the proxy.
- LIST_SCHEMA_UID and the self-UID migrated from constructor immutables into
an ERC-7201 namespaced struct, namespace efs.listentry.config, slot
0xc8fe6f31aa81c74d03ea5598de5ffa4582f1edd708ad9c21f6b1fb3027cf0a00.
- New on-chain getter listEntrySchemaUID() (deploy verify-gate + golden
vector read it); LIST_SCHEMA_UID() getter name preserved.
- onAttest/onRevoke read the struct via _cfg().
Storage-layout discipline (ADR-0009): the six consensus-critical mappings
(_decl, _entries, _entryCount, _entryPosPlusOne, _listAttesters,
_isListAttester) keep slots 0–5 byte-identical before/after — only the two
former immutables moved, into the high namespaced slot, which cannot
collide. Verified via compiled storageLayout dumps.
Tests: deploy ListEntryResolver behind TestERC1967Proxy via the helper,
register LIST_ENTRY against the PROXY, and prove end-to-end that the
on-chain listEntrySchemaUID() == proxy-derived UID == registered EAS UID,
that a real LIST_ENTRY attestation through EAS does NOT revert WrongSchema,
and that revoke works. False-green guard verified (flipping == to != fails
the assertion). ListReader wiring repointed at the proxy; nonce predictions
updated for the two-tx-per-proxy deploy. 392 passing (was 386), 0 failing.
No deploy script touched (Phase D handles deploy proxy-ification).
Permanence-tier: Etched
Refs: ADR-0048, ADR-0046, ADR-0009
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
EFSIndexer._isValidAnchorName previously rejected common name bytes
(space, ':', '&', '?', '#', '%', '/', ...) and there was no defined
canonical encoding. Different clients would therefore encode the same
human name differently (e.g. "Q&A: Episode 5") and resolve it to
DIFFERENT anchors, silently breaking the Schelling-point guarantee that
everyone independently resolves the same name to the same anchor. This
lands pre-freeze because it fixes the canonical form of every
folder/file name.
Define the canonical anchor-name encoding as Unicode NFC normalization
+ percent-encoding of a reserved byte set, with a clean client/resolver
split of responsibility:
- Client-side (documented, not enforceable on-chain): NFC-normalize the
human name, then percent-encode the reserved bytes as %XX (UPPERCASE
hex). NFC tables are far too large to run in Solidity, so the resolver
cannot and does not verify normalization -- that is the client's job.
- Resolver-side (enforced, cheap single byte-pass): accept only the
unreserved set + high-bit UTF-8 bytes + canonical %XX escapes. Reject
bare reserved bytes (must be percent-encoded), malformed/truncated
escapes (%, %2, %ZZ), and lowercase-hex escapes (%2f) so each byte has
exactly ONE valid encoding (%2f vs %2F can't both exist). The
empty/"."/".." rejections from ADR-0025 are preserved.
Reserved set = C0 controls (0x00-0x1F) + DEL (0x7F) + space + '%' +
the URI/path-special bytes " # & / : = ? @ [ \ ] ^ ` { | }. So
"Q&A: Episode 5" -> Q%26A%3A%20Episode%205.
Logic + convention change only: the ANCHOR schema field string
"string name, bytes32 schemaUID" is unchanged (frozen shape). Safe
behind the EFSIndexer proxy. No seed/test re-encoding needed -- all
existing anchor names are reserved-char-free (persona names with spaces
are PROPERTY values, not anchor names). Spec 02 sec 1 documents the
encoding; ADR-0025 status superseded-in-part; decisions.md records the
call. Suite: 400 passing / 0 failing (was 392, +8 tests).
Permanence-tier: Durable
Refs: ADR-0048, ADR-0025, specs/02
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
50-year storage-corruption guard for the upgradeable resolvers (ADR-0048). All five EFS resolvers now run behind proxies with ERC-7201 namespaced config + initialize(), with the consensus-critical append-only index mappings preserved at their sequential slots. Before mainnet we must prove a future implementation upgrade cannot silently corrupt those indices. Runtime guard (test/UpgradeWithState.test.ts): for the three most stateful resolvers — EFSIndexer (anchors/path), EdgeResolver (PIN + TAG active-edge), ListEntryResolver (LIST entries) — deploy V1 behind a proxy + initialize, seed representative state through REAL EAS attestations, snapshot the key reads + every config getter + getEAS(), upgrade the proxy V1->V2, then assert every read is byte-identical, the new V2 appended var works, and config/getEAS() are unchanged. Each test carries a mutation tripwire that confirms the byte-identical assertion fails against a wrong expected value (RED then GREEN) so the check is known to bite. V2 mocks (contracts/test/Mock*V2.sol) model a realistic, layout-safe change: each APPENDS a fresh ERC-7201 namespaced struct + getter/setter, touching no inherited sequential slot — proving additive storage is safe. Upgrade path: TransparentUpgradeableProxy + ProxyAdmin.upgradeAndCall (ERC1967). The resolvers carry no UUPS hook (no upgradeToAndCall in their bytecode), and the existing TestERC1967Proxy is a plain non-upgradeable ERC1967Proxy, so the upgrade logic must live in the proxy (Transparent), not the impl. Added a test-only TestTransparentProxy + deployUpgradeableProxy helper for this. Static gate (test/StorageLayout.gate.test.ts): committed storage-layout snapshots (test/__snapshots__/storage-layout/*.json), NOT upgrades.validateUpgrade. The OZ hardhat-upgrades plugin (3.9.1) fights this repo's pattern — it tries to instantiate the impl factory during validation and throws an ethers-v6 MISSING_ARGUMENT (the SchemaResolver(eas) immutable constructor arg) before it ever compares layouts, even with unsafeAllow: ['constructor','state-variable-immutable']. The snapshot gate reads the same compiler storageLayout output, has no plugin dependency, and asserts every existing sequential slot is preserved (moved/retyped/removed slots fail; appends pass). Negative proof: the comparator is shown to reject a retyped mapping, a slot reorder, and a removed slot, and to allow an append. Regenerate after an intentional append-only change with scripts/snapshot-storage-layout.ts. Tests only + test-only mocks + committed snapshot. No production contract changes, no deploy script touched. Suite: 400 -> 411 passing, 0 failing. Permanence-tier: Durable Refs: ADR-0048, ADR-0009 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Freeze the 9th EAS schema, REDIRECT = "bytes32 target, uint16 kind" (revocable true), plus its resolver AliasResolver, per ADR-0050. REDIRECT is the trust-scoped "this points at that" primitive: canonical/dedup (sameAs), version supersession (supersededBy), and path symlinks. refUID is the source; target is the destination DATA/Anchor UID. Kind is a uint16 discriminator (not uint8 — open-ended relationship vocabulary, widening is free as both pad to one ABI word). Only the field string is frozen; the kind taxonomy (0=sameAs, 1=supersededBy, 2=symlink, 3+=reserved) is resolver logic + client convention, NOT part of the UID. AliasResolver enforces WRITE-TIME guards only: schema==self-derived redirectSchemaUID (rejects foreign schemas), target!=0, target!=source (no trivial self-loop), and per-kind typing (sameAs/supersededBy require source+target both DATA; symlink requires source ANCHOR, target ANCHOR or DATA; kind>=3 recorded without typing). Read-time multi-hop resolution — cycle handling (lowest-UID-in-SCC), chain following, depth caps, lens precedence — is deferred to the client/router + a later Durable resolution spec, NOT the resolver. Self-UID lesson (ADR-0048, same as ListEntryResolver): the REDIRECT schema UID is self-derived in initialize() (address(this)==proxy under delegatecall), where it matches the proxy-registered EAS schema UID; deriving it in the constructor would store the impl address and reject every genuine REDIRECT with WrongSchema. Config lives in ERC-7201 namespaced storage (efs.alias.config). Reverse fan-in is left to the off-chain indexer (advisory on-chain index is future upgradeable logic). Specs: add REDIRECT to 02-Data-Models and overview's schema table; mark SORT_INFO deferred / not in the Sepolia freeze set (frozen set is the 9: ANCHOR, PROPERTY, DATA, PIN, TAG, MIRROR, LIST, LIST_ENTRY, REDIRECT). Tests: 18 new (self-UID == registered EAS UID, per-kind guard cases via real EAS attestations, kind>=3 untyped, revoke, initialize-once, getEAS). False-green verified: breaking the self-UID derivation turns 15 tests RED with WrongSchema. Suite 429 passing / 0 failing. Permanence-tier: Etched Refs: ADR-0050, ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…oken) The header comment claimed hardhat-upgrades 4.x couldn't load under HH2; it was re-pinned to 3.9.1 (HH2 line) and wired into hardhat.config in 5d42eb5. The real reason we deploy proxies manually is that validateUpgrade fights our immutable- constructor + _disableInitializers pattern, so we gate storage layout via a committed snapshot. Comment-only; no behavior change. Permanence-tier: Ephemeral Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
End-to-end deploy/freeze/upgradability runbook: CREATE3 Transparent proxies, register-LAST, the human freeze-gate (James signs FREEZE_LEDGER before any registration), live smoke, pin. Answers the Safe question: deploy from a funded EOA (JamesCarnley.eth or a dedicated key), TRANSFER ProxyAdmin + resolver ownership to the EFS.eth Safe at the end — Safe is the upgrade authority (and the eventual renounceOwnership burn), the EOA retains nothing. Cross-chain parity via same-deployer + CREATE3 salts. Includes James-runnable command sequence + the build-vs-TODO status (Phase-D script is next). Permanence-tier: Durable Refs: ADR-0048, ADR-0049, ADR-0050, ADR-0037, docs/SEPOLIA_FREEZE_TABLE.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ore) Replace the nonce-prediction + transitional TestERC1967Proxy deploy path with a single orchestrated CREATE3 ceremony per docs/DEPLOYMENT.md §3, fork-rehearsable on the pinned Sepolia fork with no private key. What problem existed: the deploy was spread across 01/04/05/09 using EAS nonce prediction and a transitional proxy; resolvers were refactored to (IEAS) constructor + initialize(), so those scripts' constructor args no longer matched and the resolver address was deploy-order-dependent. Approach (the deploy core): - deploy/lib/schemas.ts — single source of truth for the 9 frozen schemas (field strings byte-match the contract constants; golden vectors). - deploy/lib/create3.ts — TransparentUpgradeableProxy (OZ v5) deployed at a CREATE3-deterministic address via CreateX (0xba5Ed099633D3B313e4D5F7bdc1305d3c28ba5Ed). Uses permissioned salts (leading 20 bytes = deployer, flag byte 0x00) so the address is keyed on (deployer, salt) and is Sepolia<->mainnet identical for the same EOA; guarded salt = keccak256(deployer ++ rawSalt). deployCreate3(bytes32,bytes) with the resolver initialize() baked into the proxy constructor (atomic deploy+init). Salts are committed constants (frozen for parity). - deploy/lib/verify.ts — abort-on-failure gate: realized==predicted per proxy; initialize() locked (2nd proxy call + impl direct call revert); ListEntry/Alias on-chain self-UID getters == computed == to-be-registered; getEAS()==EAS; golden-vector field strings -> UIDs. - deploy/lib/orchestrate.ts + deploy/00_efs_core.ts — predict all 6 proxy addresses, compute all 9 UIDs off-chain, deploy+init each proxy, run the verify gate, wireContracts (SORT_INFO deferred -> zero), register the 9 schemas LAST against the proxies (assert getSchema(uid).resolver==proxy), transfer every ProxyAdmin + resolver Ownable owner to the Safe, then a per-schema smoke (1 attestation through each of the 9 schemas). Freeze-gate split: EFS_DEPLOY_MODE=until-freeze-gate deploys+verifies+wires and STOPS before any registration (the human signs the FREEZE_LEDGER); after-freeze-gate re-binds the existing proxies (no redeploy) and registers + transfers. full runs straight through (fork rehearsal). Safe-owner transfer: EFS_SAFE_ADDRESS (a second signer on the fork); asserts owner()==Safe and the deployer holds nothing. Verified: test/Deploy.fork.test.ts on MAINNET_FORKING_ENABLED=true --network hardhat — 6 proxies @predicted CREATE3 addresses, verify gate green, 9 schemas registered against the proxies, ownership==test Safe, 9/9 per-schema smokes pass. Both freeze-gate modes exercised end-to-end on the fork. yarn hardhat compile clean; full unit suite 429 passing / 1 pending (the fork test self-skips when not forking) — the unit suite deploys contracts directly and never touches deploy scripts, confirmed unaffected. Neutralized (superseded, single-source): 01_indexer / 04_sortoverlay / 05_mirrors / 09_lists short-circuit via deploy/lib/superseded.ts wherever CreateX is present. Deferred to D2: rebinding the downstream consumer scripts (02/03/06/07/08, which getContract("Indexer") etc.) to the proxies, and the full create/read round-trip e2e. Permanence-tier: Durable Refs: ADR-0048, docs/DEPLOYMENT.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…d, Safe-addr fail-fast, golden vectors)
I-1 (golden vectors): verify.ts step 5 recomputed UIDs from schemas.ts
against itself — circular for the 7 schemas whose field strings live only
in NatSpec comments (ANCHOR/PROPERTY/DATA/PIN/TAG/MIRROR/LIST), so a typo in
schemas.ts couldn't be caught. Added test/SchemaGoldenVectors.test.ts: a
frozen table pinning each of the 9 schema names to its field string,
revocable flag, and computed UID at a fixed mock resolver (address(0xEF5)).
The literals are cross-checked against docs/SEPOLIA_FREEZE_TABLE.md. It does
NOT fork (imports schemas.ts, recomputes off-chain) so it runs in the normal
suite — any future character change to a frozen field string now fails CI.
I-2 (runbook command): docs/DEPLOYMENT.md §4 told the operator to run
`yarn deploy:efs --network sepolia --until-freeze-gate/--after-freeze-gate`,
but no such script/task existed and the flags were parsed nowhere (the real
switch was the EFS_DEPLOY_MODE env var). Added a real `deploy:efs` hardhat
task (packages/hardhat/tasks/deployEfs.ts, wired into hardhat.config.ts) that
maps the flags onto EFS_DEPLOY_MODE and runs ONLY the EFSCore tag
(00_efs_core.ts) — never the downstream scripts. Added the `deploy:efs`
package script (routes through the encrypted-key runner via
HARDHAT_DEPLOY_TASK). The §4 commands now work verbatim; verified
`yarn deploy:efs --network hardhat` runs the full fork ceremony end-to-end
and exits 0 (generateTsAbis no longer throws / clobbers the pin on the
in-memory hardhat network).
I-3 (downstream scripts): 02_fileview/03_router (stateless views),
05_sort_functions (SORT_INFO — deferred), and 06_schema_aliases bound to the
proxies only by getContract("Indexer") + filename order, untested; 06 would
revert on its ANCHOR attestations under until-freeze-gate. Chose option (a):
gated all four behind legacySuperseded() (matching 01/04/05/09) so they skip
wherever CreateX is present and become a local/devnet-only concern. The
Sepolia surface is the EFS core only; the guard is defense-in-depth for a
plain `yarn deploy --network sepolia`. Logged in docs/decisions.md; reflected
in DEPLOYMENT.md §5.
I-4 (after-gate self-UID re-assert): the after-freeze-gate branch re-bound
the proxies and went straight to register+transfer without re-reading the
on-chain self-UID getters. Since until/after-gate run as separate processes,
drift could register a permanent schema whose UID didn't match what the
deployed ListEntry/Alias proxies self-derived. Added a guard at the top of
registerAndTransfer (the path that actually registers) reading
ListEntryResolver.listEntrySchemaUID() and AliasResolver.redirectSchemaUID()
from the deployed proxies and asserting each == the about-to-be-registered
UID; aborts loudly on mismatch.
I-5 (Safe fail-fast + spec): (a) resolveSafe silently fell back to a second
signer when EFS_SAFE_ADDRESS was unset — on a real network that would
transfer permanent upgrade authority to an arbitrary address. It now
hard-fails (throws) on any non-hardhat network when EFS_SAFE_ADDRESS is
unset/zero/invalid, validating a well-formed checksummed address; the
test-signer fallback is allowed only on hardhat. (b) Corrected
SEPOLIA_FREEZE_TABLE.md prose: the upgrade admin uses single-step
Ownable/OwnableUpgradeable (OZ v5 ProxyAdmin is single-step), not
Ownable2Step, and added a note that the single-step transfer is irreversible
so the Safe address must be verified before the after-freeze-gate run (now
enforced by I-5a). No contract changes.
Verified: yarn hardhat compile clean; full unit suite 457 passing
(429 + 28 new golden-vector tests) / 1 pending (the fork test self-skips when
not forking); fork rehearsal (Deploy.fork.test.ts + `deploy:efs --network
hardhat`) passes end-to-end incl. the I-4 self-UID re-assert.
Permanence-tier: Durable
Refs: ADR-0048, docs/DEPLOYMENT.md
Reviewed-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ews) The Sepolia freeze ceremony (`yarn deploy:efs`, the EFSCore tag) stands up the frozen foundation — 6 CREATE3 proxies + 9 registered schemas — but deploys NONE of the three stateless read views, so the app can't read after a freeze. This closes that gap with a separate, clearly NON-FROZEN, post-freeze views deploy the operator runs AFTER the ceremony. New `EFSViews` step (`deploy/10_efs_views.ts` + `deploy/lib/views.ts`) deploys EFSFileView, EFSRouter, and ListReader against the proxy addresses the core registered: it binds via the `Indexer` / `EdgeResolver` / `ListEntryResolver` named deployments that `00_efs_core.ts` saves, reads the now-frozen schema UIDs off the proxies (`indexer.DATA_SCHEMA_UID()` etc.), and NEVER redeploys a proxy or registers a schema. The views are in no UID and freely redeployable, so the step is idempotent (redeploy-or-no-op via `redeployIfArgsChanged`). A guard fails clearly if the core hasn't run (proxies/schema UIDs absent). New `deploy:efs-views` hardhat task + package.json script (same encrypted-key runner) runs ONLY the `EFSViews` tag. 09 investigation: the resolver deploys (ListResolver, ListEntryResolver) + LIST/LIST_ENTRY registration in 09 are fully superseded by the orchestrated register-last core (00) and stay neutralized on CreateX (I-3). The ONLY part of 09 still needed on CreateX was the stateless ListReader view — now owned by the new EFSViews step. 02/03/09 remain the local/devnet view path (they run only when CreateX is absent); EFSViews runs only when CreateX is present, so the two paths never double-deploy or collide on a named handle. Double-deploy avoidance: EFSViews is gated on CreateX presence (not `legacySuperseded`) — it is the intended view deploy on Sepolia/mainnet/fork and short-circuits off CreateX, where 02/03/09 own the views. Confirmed no deployedContracts/handle collision. E2e (the deferred D2): `test/DeployE2E.fork.test.ts` (forking-gated, self-skips when not forking) runs `orchestrate()` (frozen foundation) → `deployViews()` → writes a real round-trip (anchor + DATA + PIN placement + MIRROR + TAG + LIST + LIST_ENTRY) → reads it ALL back THROUGH the views with real assertions: EFSRouter.request() serves the file (200, external-body URL == the written ipfs mirror); EFSFileView.getFilesAtPath() returns the placed DATA UID; ListReader getMode/length/entries/targetAsUID return the written list + entry whose target == the DATA UID. Also asserts the router bound to the frozen DATA UID (guards a view reading a drifted UID). No false-greens. Verified: `hardhat compile` clean; non-fork suite 457 passing / 2 pending (does not require forking); e2e passes with forking on; `deploy:efs` then `deploy:efs-views` both exit 0 in sequence on `--network hardhat`; committed `packages/nextjs/contracts/deployedContracts.ts` pin untouched. Permanence-tier: Ephemeral Refs: docs/DEPLOYMENT.md, ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…TY revocable) PROPERTY flips false->true before freeze: it is a claim/value, not an identity Schelling point like DATA, so it belongs with the revocable claim-schemas. Revoking a value withdraws it from the default view without erasing the bytes (storage stays append-only, ADR-0009). ADR-0051 elevates ADR-0009's 'readers filter on iteration' into a uniform API rule: every read excludes revoked + superseded by default; full history is an explicit opt-in. Upgradeable logic, not frozen. Freeze impact: PROPERTY UID recomputes (revocable in the UID); no other schema affected. Code flip + golden-vector + revoked-aware reads land next. Permanence-tier: Etched Refs: ADR-0048, ADR-0049, ADR-0050, docs/SEPOLIA_FREEZE_TABLE.md Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…51/0052) Flip PROPERTY from revocable:false to true (ADR-0052) — a value is a claim the author can withdraw, not an identity Schelling point like DATA. This recomputes PROPERTY's frozen UID (no other schema affected): old (mock 0x…0ef5): 0xe03f507b…aaec689 new (mock 0x…0ef5): 0x9a3ce6ff…6323674 Flipped in deploy/lib/schemas.ts (the freeze source of truth), the legacy deploy/01_indexer.ts registration, and the golden vector. The golden-vector literal was updated first and verified RED against the unflipped schemas.ts (catches drift), then GREEN after the flip — it remains a real frozen-literal assertion, not a tautology. Honor PROPERTY revocation on the read path (the correctness coupling): EFSIndexer.onAttest no longer rejects revocable PROPERTY attestations (ANCHOR/DATA still must be non-revocable); onRevoke already flags _isRevoked for any EFS-native schema, so a revoked PROPERTY is tracked. EFSRouter._getContentType now checks indexer.isRevoked(propertyUID) in addition to the active-PIN check, so a revoked PROPERTY value reads as absent by default (application/octet-stream) even while its binding PIN is active. Matches the existing isRevoked idiom used for mirrors. ADR-0051 default-hide-revoked coverage. Audited every read surface: placement (getActivePinTarget is active-only), mirrors (router + EFSFileView filter isRevoked), directory children (showRevoked defaults false), and list entries (ListEntryResolver swap-and-pops on revoke, so ListReader.entries is active-only by construction) all already comply. The one leak was the lens-scoped PROPERTY value lookup — fixed above. Deferred: the generic EFSIndexer referencing-index getters (getReferencing*, getReferencingAttestationCount) have no includeRevoked opt-in; bringing them under the uniform default is a multi-function additive ABI change across an Etched surface + TS callers, kept out of this freeze PR and flagged in docs/FUTURE_WORK.md (the serving/router path is fully covered). Tests (RED first): PROPERTY revocation round-trip in EFSRouter.test.ts (place contentType, read present, revoke the value, read absent; plus the binding-PIN revoke path still works); a false-green guard confirmed the new test fails when the read-path isRevoked check is reverted; ListReader C6b confirms a revoked LIST_ENTRY is absent by default. Specs 02 (PROPERTY revocable + two removal paths) and 03 (ADR-0051 default-hide convention + audit + deferral) updated. These view-layer changes are upgradeable logic; only PROPERTY's UID is the Etched change. Suite: 460 passing, 2 pending (self-skipping fork-e2e). deployedContracts.ts pin untouched. Permanence-tier: Etched Refs: ADR-0051, ADR-0052, ADR-0048 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aa091383e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The EAS/SchemaRegistry constants in deploy-lib/addresses.ts are Sepolia-only (also live on the pinned fork). On --network mainnet the orchestrated deploy would init resolvers against the Sepolia EAS and register/read through the wrong contracts. 00_efs_core now hard-fails on any chainId other than Sepolia (11155111) or its fork (31337), after the CreateX-present check (so bare-node skips are unaffected) and before any resolver init/registration. Wiring unverified mainnet addresses now is the footgun this guards — mainnet-forward, not mainnet-now. Permanence-tier: Durable Refs: PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The §Transport section listed 11 allowed MIRROR schemes in one paragraph but still said only 5 /transports/* anchors are created + ranked. Bootstrap now seeds all 11 (un-squattable) and the router/debug-UI accept them. Updated the anchor list (added ftp/s3/gs/dat/rsync/bittorrent) and the preference text: original 5 ranked per ADR-0012, the 6 new off-chain schemes in the lowest tier with https (router priority 4; all non-web3 served as message/external-body). Permanence-tier: Durable Refs: ADR-0011, ADR-0012, ADR-0023, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b91e44258b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
JamesCarnley
commented
Jun 11, 2026
JamesCarnley
left a comment
JamesCarnley
left a comment
Member
Author
There was a problem hiding this comment.
[gemini-3.5-flash · review]
Review mode: review-squad
Base/head: origin/main..schema-freeze
Findings
1. [P1] Unbounded Loop Scans in Attester/Lens Filters
- Why it is wrong: The indexer read queries
getChildrenByAddressListandgetAnchorsBySchemaAndAddressListfilter child arrays by walking them linearly in a while-loop. If the directory grows to 10k, 100k, or 1M items, and the queried lenses (attesters) have few or no items in it, the loop will scan the entire array in a single call. - File & Line references:
EFSIndexer.sol:L642-683(getAnchorsBySchemaAndAddressList)EFSIndexer.sol:L806-846(getChildrenByAddressList)
- Concrete regression or risk: High lookup latency or transaction reverts due to RPC gas limits/timeouts, effectively bricking directory views for users using sparse lenses.
- Minimally correct fix direction: Introduce an on-chain scan limit (e.g.
MAX_SCAN_BUDGET = 2048) inside the loop, and return the current position indexias anextCursorso the client can incrementally page the scan across multiple RPC calls.
2. [P1] Inconsistent Core Workflows Spec vs. Reshaped DATA Schema
- Why it is wrong: Under ADR-0049 and
specs/02-Data-Models-and-Schemas.md, theDATAschema was reshaped into an empty ("") pure-identity schema, moving itscontentHashandsizefields to reserved-keyPROPERTYbindings (via standard Anchor + PROPERTY + PIN triplets). However, the workflows spec still references the old payload layout and the deprecated deduplication index. - File & Line references:
specs/04-Core-Workflows.md#L8(Step 2 checkdataByContentKey)specs/04-Core-Workflows.md#L11(Step 4 DATA payload)specs/04-Core-Workflows.md#L22(IPFS paste DATA payload)specs/04-Core-Workflows.md#L72(Edit workflow)
- Concrete regression or risk: Misleads future developers into implementing client-side writers or off-chain tools using outdated schema layouts, causing runtime registration failures or orphaned data.
- Minimally correct fix direction: Update the workflows document to remove the step checking
dataByContentKey, use emptyDATA()payloads, and document thatcontentHashandsizeare bound via the standard property-binding triplet.
3. [P2] Outdated Deduplication Index Reference in On-chain Indexing Spec
- Why it is wrong: The indexing spec still lists
dataByContentKeyas an active content-addressed deduplication mapping. - File & Line references:
specs/03-Onchain-Indexing-Strategy.md#L25specs/03-Onchain-Indexing-Strategy.md#L92
- Concrete regression or risk: Developers auditing the indexing logic will expect
dataByContentKeyto be populated, whereas it is now a dead slot retained purely for layout upgrade safety (ADR-0049). - Minimally correct fix direction: Update the spec to document
dataByContentKeyas deprecated and retained only for layout safety, and describe how deduplication is now client-driven.
4. [P2] N+1 External Calls in getActiveTargetsByAttesterAndSchema
- Why it is wrong: The read helper fetches the target ID by executing an external call to
_eas.getAttestation(tagUID)inside a loop for each item of the paginated page. - File & Line references:
EdgeResolver.sol:L796-817(getActiveTargetsByAttesterAndSchema)
- Concrete regression or risk: Generates N storage-heavy external calls per read page, increasing client query latency significantly when paginating lists.
- Minimally correct fix direction: Store
targetIDdirectly inTagEntryat write-time, or return rawtagUIDs directly to the client and let the client batch-resolve them via multicall.
5. [P2] Heavy Read Fanout in Directory Item Builder
- Why it is wrong: For each item in a page,
_buildFileSystemItemsexecutes four external calls to retrieve metadata and active counts. - File & Line references:
EFSFileView.sol:L337-375
- Concrete regression or risk: High constant factor overhead (200 storage reads for a page of 50 items) leads to high view latency.
- Minimally correct fix direction: Cache count states at write-time or design batch-fetch/multicall-friendly client reads to fetch directory items without building full metadata on-chain.
6. [P2] Deterministic CREATE3 Address Drift Between Deployment Phases
- Why it is wrong: Deterministic deploy address pre-computation depends on the EOA deployer's address and transaction nonce.
- File & Line references:
packages/hardhat/deploy-lib/create3.ts#L85-L95packages/hardhat/deploy-lib/orchestrate.ts#L123-L143
- Concrete regression or risk: If the EOA executes any other transactions between deployment phases, proxy addresses will drift, causing UIDs to mismatch.
- Minimally correct fix direction: Keep the deployer EOA isolated. Retain the pre-execution verify-gate assertions checking precomputed UIDs.
7. [P2] Single-Step Ownership Transfer
- Why it is wrong: ProxyAdmin and resolver contract ownership are transferred to
EFS_SAFE_ADDRESSin a single-steptransferOwnershipcall. - File & Line references:
packages/hardhat/deploy-lib/orchestrate.ts#L407-L452
- Concrete regression or risk: Misconfiguration of the Safe address leads to permanent loss of upgrade/ownership privileges.
- Minimally correct fix direction: Implement a strict off-chain checksum check for
EFS_SAFE_ADDRESSor useOwnable2Stepfor proxies.
8. [P3] Lack of Access Control on Public indexRevocation
- Why it is wrong: The indexer's sync method
indexRevocationis public-facing and unguarded. - File & Line references:
EFSIndexer.sol:L1254-L1263
- Concrete regression or risk: Safe due to EAS checks and idempotency, but public state-modifying calls are best restricted.
- Minimally correct fix direction: Document the public entry point as intentionally unguarded or restrict its callers to active resolvers.
Open Questions / Assumptions
- We assume that NFC normalization of anchor names will remain client-enforced, as the contract resolver checks percent-encodings only. We suggest adding a section in the data models spec (
specs/02-Data-Models-and-Schemas.md) documenting this client-normalization rule. - We assume the deployer fallback role of
SystemAccountis final and replaces the throwaway deployer EOA.
Verification
yarn hardhat:test— passed (all tests green)yarn lint— passed (no ESLint warnings or errors)yarn next:check-types— passed (no TypeScript compile errors)
A revocable *schema* only PERMITS revocable attestations — EAS still accepts a REDIRECT submitted with revocable=false, which would make a sameAs/supersededBy/ symlink redirect permanent and uncorrectable, contradicting ADR-0050's retractability guarantee. AliasResolver.onAttest now rejects it with NotRevocable, mirroring the existing guard in ListEntryResolver. Added a write-time test asserting the revert. Note: EdgeResolver (PIN/TAG) and MirrorResolver enforce no such guard either — flagged to the human as a separate consistency call, not widened here. Permanence-tier: Durable Refs: ADR-0050, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
CreateItemModal.validateAnchorName rejected every '%' and omitted C0 controls/ DEL from its forbidden set, so it both blocked valid names (e.g. a%2Fb) and let some control bytes through to revert on-chain. Rewrote it as a faithful port of EFSIndexer._isValidAnchorName: accept reserved bytes only as UPPERCASE %XX escapes, reject bare reserved bytes (incl. controls/DEL), reject malformed/ lowercase/non-canonical escapes. Reserved chars are entered as their escape; no auto-encoding (the entered string is exactly what gets attested). Permanence-tier: Ephemeral Refs: ADR-0025, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
JamesCarnley
commented
Jun 11, 2026
JamesCarnley
left a comment
JamesCarnley
left a comment
Member
Author
There was a problem hiding this comment.
[codex-gpt-5 · review]
Full deep pass completed with six parallel specialist lanes (schema/spec governance, Solidity security, deploy/Safe/CREATE3, protocol invariants, UI/client, CI/tests), then reconciled locally in /tmp/efs-contracts-pr24 at b91e44258b5df6039a3f9a0cdad033d1c41e3f79.
I attempted to submit this as REQUEST_CHANGES, but GitHub rejects change requests from the PR owner account, so I am submitting it as an official Review comment. Treat these as blocking findings: the current head still has several freeze-blocking issues around revocable active-claim schemas, FileView lens semantics, explicit lens removal, SystemAccount burn immutability, Safe address validation, and freeze/runbook truth.
Additional findings that could not be anchored inline because the exact files/lines are outside this PR diff:
- [P1]
EFSFileView.getDataMirrors()uses the global referencing index and filters only revocation, so mirrors from any attester attached to a DATA UID are returned. Router mirror resolution is lens-scoped; this helper should require/use the winning lens attester or an attester-filtered referencing index. - [P2]
packages/nextjs/hooks/efs/useContainerName.tsstill appendsIndexer.DEPLOYER()as the display-name fallback lens, while this PR makes SystemAccount the canonical system author. PathBar/info-panel/document-title labels can therefore fall back to short hex for system-authored containers. - [P2] Required contract CI still runs plain
yarn hardhat:test; the.fork.test.tsdeploy rehearsals self-skip unlessMAINNET_FORKING_ENABLED=true, so Safe-native deploy/CREATE3/e2e views can regress while required CI stays green. - [P2] EdgeResolver pagination helpers still compare
total < start + length; callers using a defensive max length can overflow the addition and revert/OOG instead of clamping likeListEntryResolvernow does.
Verification run locally:
yarn install --immutablecompleted.yarn hardhat:testexited 0. Fork suites were pending/skipped becauseMAINNET_FORKING_ENABLEDwas not set in this local run.yarn next:check-typesexited 0.yarn hardhat:check-typesexited 0.yarn docs:checkexited 0.git diff --checkclean; review worktree remained clean.
I intentionally dropped one subagent-reported typecheck finding after local verification contradicted it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6eb555b82c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…reeze schemas (P1/P2) PIN/TAG/MIRROR/REDIRECT edges are 'active until explicitly revoked' — no applies=false, no expiry. A revocable *schema* only PERMITS revocable attestations; EAS still accepts revocable=false (welds the edge on forever) and nonzero expirationTime (the edge silently expires but EFS reads filter on revocation, not expiry, so it reads as active forever). EdgeResolver, MirrorResolver and AliasResolver now reject both at write time (NotRevocable / HasExpiration), matching the guard ListEntryResolver already had. Added write-time rejection tests for each (PIN, TAG, MIRROR, REDIRECT × both bits). Regenerated deployedContracts.ts for the three new resolver ABIs (deploy + bootstrap + seed verified clean on the pinned fork — no seeded edge uses expiry/non-revocable). Permanence-tier: Durable Refs: ADR-0044, ADR-0050, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e229c16b03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…, deploy, docs Resolver/router/view (P1): - EFSFileView.getFilesAtPath now reports the winning PLACEMENT (lens) attester, not the DATA author, so hardlinked/dedup'd items scope follow-up MIRROR/ PROPERTY reads to the correct lens (ADR-0013/0014). Buffered in lockstep. - EFSRouter: an explicit empty ?lenses= now returns no data instead of falling back to caller/SystemAccount — honors the user-removable system lens (ADR-0031/0039/0053). Absent ?lenses= keeps the caller→system fallback. SystemAccount (P1/P2): - One-way sealModules() latch (+ modulesSealed() view) freezes module membership; the burn ceremony asserts it so no new system-writer can be authorized post-burn — makes ADR-0053's 'pre-burn only' claim enforceable. - Idempotent scaffolding-anchor adoption now verifies attester==self + schema/ refUID/revocable/expiry/payload before reuse, else reverts PollutedAnchor. Deploy + storage gate (P1/P2): - orchestrate.ts requires code at EFS_SAFE_ADDRESS and sanity-checks getThreshold()/getOwners() before the irreversible ownership transfer on real networks (fork path unchanged). - Storage-layout gate now guards MirrorResolver and SystemAccount (snapshots + MirrorResolver V1->V2 preservation test). Docs/specs (P2): - DEPLOYMENT.md + SEPOLIA_FREEZE_TABLE.md: 7 CREATE3 proxies (incl SystemAccount) + sealModules() burn step; pre-register gate corrected to wireContracts-only with a post-batch transportsAnchorUID check; setSortsAnchor removed. - specs/03/04/05: empty-DATA model, reserved-key contentHash/size PROPERTYs, DataCreated(bytes32,address), property-index/REDIRECT dedup. dataByContentKey described as a retained dead slot (not 'gone'). - transports.ts rejects http:// (matches MirrorResolver allowlist). - ADR-0048/0050: Status/Related-line notes recording the 8->9 schema amendment (ADR discipline: no body edits). Regenerated deployedContracts.ts for the EFSFileView/EFSRouter/SystemAccount ABI deltas; full deploy+bootstrap+seal verified clean on the pinned fork. Reviewed by three adversarial subagent passes (no Critical/High/Medium). Permanence-tier: Durable Refs: ADR-0013, ADR-0014, ADR-0031, ADR-0039, ADR-0049, ADR-0050, ADR-0053, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 139a9ff7c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ront-run) The EOA path registers schemas and creates the canonical root in SEPARATE txs (SchemaRegistry.register ... then SystemAccount.bootstrap). On a public network there is a mempool window between ANCHOR registration and bootstrap where EFSIndexer.rootAnchorUID is still zero and the FIRST generic ANCHOR anyone attests is permanently accepted as root (EFSIndexer.sol:385) — a front-runner could make the canonical root attacker-authored. orchestrate() now throws before Step 6 (register) on any chainId other than 31337, so EOA register/bootstrap is confined to the local pinned fork (covers local, the fork, and the devnet VPS at anvil --chain-id 31337, none of which has an adversarial mempool). Real Sepolia/mainnet must use the Safe-native ceremony, whose Batch 2 registers + bootstraps atomically in one MultiSend (no window). The EOA path can still deploy+wire (--until-freeze-gate) anywhere; that mode returns before registration and is unaffected. DEPLOYMENT.md updated. Verified: Deploy.fork (chainId 31337) still runs orchestrate full + the after-freeze-gate retry end-to-end through the guard. Permanence-tier: Durable Refs: ADR-0053, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…(P2) The burn checklist verified SystemAccount.owner()==0 but listed only ProxyAdmin.renounceOwnership(), which clears the ProxyAdmin owner — a DISTINCT owner from the proxied SystemAccount's own OwnableUpgradeable owner. As written the verification would always fail (or be skipped with the Safe still owning SystemAccount). Added an explicit SystemAccount.renounceOwnership() burn step (after sealModules()/seal(), which already froze every owner power; authoring continues via the onlyAuthorizedModule relay) so owner()==0 is actually achievable, in both SEPOLIA_FREEZE_TABLE.md and DEPLOYMENT.md. Permanence-tier: Durable Refs: ADR-0053, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa818516f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…esume (P2) EFSIndexer (kernel): ANCHOR/DATA/PROPERTY are non-revocable permanent structure, but a non-revocable *schema* doesn't stop EAS storing a nonzero expirationTime, and EFS reads filter on revocation/index state — not EAS expiry — so an expiring anchor/DATA/property would keep resolving forever past expiry. onAttest now rejects expirationTime != 0 alongside the existing non-revocable checks for all three branches (return false → EAS reverts; no new error, ABI/pin unchanged). Tests added for each. safePlan.detectDeployPhase (Safe propose-mode resume): treated any nonzero MirrorResolver.transportsAnchorUID() as Phase-3-complete. setTransportsAnchor is one-shot, so a stale/edited Batch 3 could weld in the WRONG UID and the resume would emit an empty 'done' artifact while MIRROR ancestry validates against the wrong subtree. Now asserts the wired UID equals the realized /transports anchor (resolvePath(root, "transports") — the value the execute path feeds the setter) and throws on mismatch instead of reporting success. Verified: 522 unit passing, Deploy.fork + DeploySafe.fork green, pin unchanged. Permanence-tier: Durable Refs: ADR-0009, ADR-0049, ADR-0052, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Partial supersessions (ADR-0049 amending ADR-0002, ADR-0050 amending ADR-0048) were recorded as parentheticals on the old ADR's Status/Related line — the one mutable slot the README already designates for supersession pointers. External review (PR #24 thread) read those edits as immutability violations because the pattern was practiced but not written. Codified it in the Discipline section, with the human's explicit approval (2026-06-11): body stays untouched, the pointer lives on the document itself, no separate tombstone log. Permanence-tier: Durable Refs: PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Six-angle expert review (schema/proxy-burn/kernel/resolver/EAS-evolution/deploy) of the freeze + upgradeable contracts. The frozen schema surface is sound — zero freeze-blockers. This commit lands the actionable non-frozen-surface fixes; the rest are recorded in FUTURE_WORK as pre-mainnet follow-ups. - BURN completeness (proxy/burn auditor, HIGH): the documented burn renounced every ProxyAdmin + SystemAccount's own owner, but NOT the OwnableUpgradeable owners of EFSIndexer and MirrorResolver (a distinct owner from the ProxyAdmin). Post-burn the Safe could still call EFSIndexer.setSortsAnchor (a one-shot setter never set in this freeze — SORT_INFO deferred) and weld a permanent value into the 'immutable' kernel. Burn checklist now renounces both resolver owners + verifies owner()==0 + setSortsAnchor reverts (SEPOLIA_FREEZE_TABLE.md + DEPLOYMENT.md). 'Immutable after burn' is now actually true. - VERIFY-GATE cross-refs (deploy auditor, M-1): the gate checked only the two self-derived UIDs; the UIDs threaded into initialize() as typing/config refs were unchecked. verify.ts now asserts EFSIndexer ANCHOR/PROPERTY/DATA, EdgeResolver PIN/TAG, and AliasResolver data/anchor against the schemaUIDs map before the irreversible register. - EXPECTED-SAFE pin (deploy auditor, M-4): the Safe keys every CREATE3 address + schema UID; a typo to a different *valid* Safe passed every shape check and would silently key the whole permanent deploy wrong. Pinned EXPECTED_EFS_SAFE (0x1Ad8…11D7) and assert the supplied Safe matches on real networks (override via EFS_SAFE_EXPECTED_OVERRIDE for one-off testnet Safes). - Corrected the verify.ts golden-vector comment (the real field-string guard is the off-chain SchemaGoldenVectors.test.ts, not the self-consistent re-derive). No contract change (deployedContracts.ts pin unchanged). Verify gate validated green on both Deploy.fork (EOA) and DeploySafe.fork (Safe) ceremonies. Permanence-tier: Durable Refs: ADR-0030, ADR-0032, ADR-0048, ADR-0053, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d64a7d7fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…emony (P2) The Safe propose flow ran the verify gate at Phase 1 against live proxies but with impl="", so runVerifyGate SKIPPED the impl-direct initialize() check. Phase 0 only emits Batch 1 (it never runs the gate), and the Safe path deploys impls inside the MultiSend — never through create3.ts's checked helper — so the impls behind the to-be-registered proxies were NEVER verified _disableInitializers- locked before the permanent schema registrations. An impl missing the lock could have schemas registered against its proxy unverified. buildDeploysFromOnchain now reads each live proxy's EIP-1967 implementation slot (mirroring the existing readProxyAdmin) and passes that real impl address to the gate, so the impl-direct check runs on the Safe ceremony too — for all 7 proxies, before register. Reverts if any proxy reports a zero implementation slot. Verified: DeploySafe.fork green incl. the Phase-1 propose-resume test that drives buildDeploysFromOnchain; verify gate GREEN; pin unchanged (no contract change). Permanence-tier: Durable Refs: ADR-0048, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c132cd7711
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
In the paste-link flow, Fetch Info for URI A set pasteContentHash/pasteSize/ pasteContentType, but the URI input handler never reset them. Editing the URI to B before submit would then bind A's hash/size/type as permanent reserved-key PROPERTY claims on B's DATA — a wrong, lens-scoped integrity claim that makes a verifier see a false mismatch on B. The URI onChange now clears all three fetched content-metadata fields whenever the URI actually changes; re-fetch after editing. Permanence-tier: Ephemeral Refs: ADR-0049, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ster Two expert audits (subgraph-indexability + Ethereum logistics) run for the events/tooling question. Captured durably: (1) the edge+mirror layer emits no indexable events — PIN placement/supersession, TAG sets/weights, and MIRROR uri are invisible to a pure-event subgraph; upgrade-safe to add until the mainnet burn (PIN-supersession unrecoverable post-burn), with concrete event signatures to add (ideally before the Sepolia freeze so indexers bind to final ABIs). Also flags a Tier-2 spec/03 claim that's currently false for the edge layer. (2) the launch logistics register — web3:// gateway, ENS, RPC/CORS, verification, monitoring, indexer hosting (Ponder), and Arweave-as-canonical off-chain durability — pointers to consolidate into LAUNCH_CHECKLIST. Permanence-tier: Durable Refs: PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5708e3315b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…truction An audit found EFS was not reconstructable from logs: EdgeResolver (PIN/TAG) and MirrorResolver emitted nothing of their own, and EAS's Attested carries no field data — so file placement, PIN supersession, tags+weights, and mirror URIs were invisible to a pure-event indexer. Added (resolver LOGIC, not schema — upgrade- safe until the mainnet burn, landed pre-freeze so indexers bind the final ABI): - EdgeResolver: PinSet(definition,attester,targetSchema,pinUID,targetID, supersededPinUID), PinCleared, TagSet(...,weight), TagCleared. Indexed topics = the active-slot key (definition,attester,targetSchema). supersededPinUID makes silent cardinality-1 PIN replacement observable (0 on fresh/same-target; prior pinUID on a real replacement) — unrecoverable post-burn otherwise. - MirrorResolver: MirrorSet(dataUID,attester,transportDefinition,mirrorUID,uri) + MirrorCleared. The kernel's MirrorCreated omits the URI; this carries it. - EFSIndexer: AnchorCreated gained . A pure-event subgraph can now reconstruct placement/supersession/tags/mirrors/ property-bindings/revocation with zero eth_calls (verified by two adversarial review passes). Tests for every event incl. supersede, same-target re-attest, and stale-revoke-emits-no-clear. specs/03 updated (the 'no contract reads' claim is now true for the edge/mirror layer). Pin regenerated; deploy+bootstrap+seal + Deploy.fork + DeploySafe.fork all green. Permanence-tier: Durable Refs: ADR-0009, ADR-0041, ADR-0052, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…--via-safe (P2) The runbook's EOA --until-freeze-gate -> --after-freeze-gate block targeted real Sepolia, but --after-freeze-gate (register) now hard-fails off chainId 31337 (the front-run guard in orchestrate.ts). An operator following it would spend gas on --until-freeze-gate then hit the guard with no way to finish. Relabeled the block fork/devnet-only, switched the example to --network localhost (anvil 31337), and added a callout that the real Sepolia/mainnet freeze uses the Safe-native ceremony (--via-safe) whose Batch 2 registers+bootstraps atomically. Permanence-tier: Durable Refs: PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2af47d3e7d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…trip (P2) Allowing reserved characters as canonical %XX anchor names (ADR-0025) exposed a deep-link round-trip bug in the explorer. pathSegments are decodeURIComponent-d once at parse time, so a segment is the on-chain name (e.g. "a%2Fb"). The resolve loop stored that decoded value verbatim as urlSegment; the URL builder then emitted it raw, so breadcrumb/refresh produced /explorer/a%2Fb, which the next parse decodes to "a/b" and resolvePath looks up the wrong anchor — an item you can create but cannot revisit. Store encodeURIComponent(segment) as urlSegment instead: "a%2Fb" -> "a%252Fb" in the URL -> decodes back to "a%2Fb" on the next parse. No-op for plain ASCII (docs, readme.txt), and correct for non-ASCII + percent escapes. name keeps the decoded human form for display. Ephemeral debug UI; no contract/pin change. Permanence-tier: Ephemeral Refs: ADR-0025, PR #24 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 807c89a505
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function validateAnchorName(name: string): string | null { | ||
| if (name.length === 0) return "Name cannot be empty."; | ||
| if (name === "." || name === "..") return "Name cannot be '.' or '..'."; | ||
| const bytes = new TextEncoder().encode(name); |
There was a problem hiding this comment.
When the debug UI creates a name containing canonically equivalent Unicode, e.g. precomposed é vs decomposed e\u0301, this validator encodes the raw input and handleSubmit later attests the same raw newName. The spec says clients MUST NFC-normalize before encoding because the contract cannot verify it, so this path can mint distinct permanent anchors for the same human/path segment and break the canonical-name invariant. Normalize the input before validation/attestation, or reject inputs where name !== name.normalize('NFC').
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Freezes the nine EFS EAS schemas for Sepolia and builds everything needed to deploy them durably. Nothing is registered on-chain — the freeze is gated on James's signature on
docs/SEPOLIA_FREEZE_TABLE.md; merging this PR does not register schemas.The frozen set (field string · revocable · resolver):
string name, bytes32 schemaUIDstring value""(empty — pure identity)bytes32 definitionbytes32 definition, int256 weightbytes32 transportDefinition, string uribool allowsDuplicates, bool appendOnly, uint8 targetType, bytes32 targetSchema, uint256 maxEntriesbytes32 listUID, bytes32 targetbytes32 target, uint16 kindWhat this PR delivers:
EFSUpgradeableResolverpattern (immutable EAS +_disableInitializers+ ERC-7201 namespaced config set in a guardedinitialize); self-derived schema UIDs moved constructor→initialize so they bind to the proxy (the ListEntry-class bug, fixed).SystemAccount— the neutral, code-governed system write-identity that authors the bootstrap scaffolding and is the last default lens (ADR-0053).docs/DEPLOYMENT.mdrunbook.Why
Sepolia hackathon data must persist for years, so the schema shapes must be correct and frozen before registration — changing a frozen field string / revocable flag / resolver address orphans all data under that schema. Contract logic must stay upgradeable behind stable proxy addresses (the proxy address is hashed into the UID) until deliberately burned to immutable. This PR gets the shapes right and makes the logic upgradeable — the two halves of "freeze the schemas, keep the code malleable."
Permanence tier
Etched — the 9 schema UIDs and the proxy/SystemAccount addresses become permanent at registration/deploy. This is why the freeze is human-gated (James signs the table; registration is a separate step). Durable — deploy scripts, views, specs, runbook.
Specs / ADRs touched
Proposed— accepted on freeze sign-off): 0048 (freeze set + proxy/burn), 0049 (DATA empty; contentHash/size as reserved-key PROPERTYs), 0050 (REDIRECT), 0051 (reads exclude revoked/superseded by default), 0052 (PROPERTY non-revocable interned value), 0053 (SystemAccount).overview.md(nine-schema table, lenses),02-Data-Models-and-Schemas.md,03-Onchain-Indexing-Strategy.md.SEPOLIA_FREEZE_TABLE.md(the sign-off gate),DEPLOYMENT.md(the runbook).Test plan
yarn hardhat test→ 475 passing / 3 pending (the 3 pending are forking-gated rehearsals that self-skip unforked).yarn next:check-typesclean.deployedContracts.tspin clean.Agents involved
Claude Opus 4.8 — architecture review, schema-freeze build, deploy pipeline, SystemAccount, and all phases. Each phase received a spec-compliance + adversarial review pass; a final whole-branch freeze-coherence review confirmed the 9 schemas agree byte-identical across all sources (contracts ↔ schemas.ts ↔ golden vectors ↔ freeze table ↔ specs ↔ ADRs).
Deliberately NOT in this PR
ValueRegistry(the opt-in on-chain value interner) — deferred; theSystemAccountauthorization seam is ready for it (ADR-0052/0053).Known minor follow-ups (non-blocking)
EFSIndexer.solNatSpec (~L47) lists PROPERTY among revocable-native schemas; it's non-revocable. Harmless, and on upgradeable logic.UpgradeWithState.test.ts(the runtime test passes green) — regenerate typechain in a follow-up.🤖 Generated with Claude Code