Skip to content

feat(contracts): view-layer tag-exclusion directory filter (ADR-0048)#26

Closed
JamesCarnley wants to merge 4 commits into
mainfrom
onchain-tag-filter
Closed

feat(contracts): view-layer tag-exclusion directory filter (ADR-0048)#26
JamesCarnley wants to merge 4 commits into
mainfrom
onchain-tag-filter

Conversation

@JamesCarnley

Copy link
Copy Markdown
Member

Summary

  • Adds EFSFileView.getDirectoryPageFiltered(parent, schema, lenses, excludeTagDef, minWeight, cursor, maxItems) — a directory page that skips any item a lens has tagged with excludeTagDef at weight >= minWeight. The threshold is a caller argument.
  • Adds EdgeResolver.getActiveTagWeight(attester, target, def, targetSchema) → (exists, weight) — an O(1) pure view over existing _activeByAAS storage (no new state, no index/write-path change) that the filter uses to read a tag's weight.
  • New ADR-0048 (Proposed) documenting the design; 6 new tests + an injectable scan budget (15 in the filter file, 380 total passing).

Why

On-chain consumers — and the explorer itself — had no way to read a directory listing with nsfw/system items already filtered out; they had to list, then N+1-scan each item's tags. ADR-0042 deliberately kept weight policy in the client but overlooked on-chain consumers. This adds the read API without breaking ADR-0042's core principle (the kernel stays weight-neutral — it only gains a read; the weight >= minWeight comparison lives in the redeployable view layer, threshold supplied by the caller).

Permanence tier

Durable, with one near-Etched touch: EdgeResolver.getActiveTagWeight is a pure view added to a kernel-wired contract. It changes no storage layout, no write path, and no append-only index (ADR-0009), so PIN/TAG schema UIDs are preserved — verified: regenerating deployedContracts.ts is additive ABI only, zero address changes (EdgeResolver's nonce-CREATE address is bytecode-independent). EFSFileView is stateless/redeployable. No schema field changes.

Specs / ADRs touched or checked

  • ADR-0048 (new, Proposed) — view-layer tag-exclusion filter; extends ADR-0042 into the view layer, does not supersede it.
  • ADR-0042 (client weight filter), ADR-0041 §4 (kernel weight-neutrality), ADR-0038 (folder visibility), ADR-0036 (opaque cursor), ADR-0007 (swap-and-pop index) — confirmed honored.
  • specs/ — no consumer-visible behavior change to existing functions; the new view is additive. (A spec note in 02/03 can follow if desired.)

Test plan

  • npx hardhat compile — clean. yarn hardhat:test380 passing, 0 failing (filter file: 15).
  • Coverage: file→DATA-via-PIN exclusion, folder→ANCHOR exclusion, reused-anchor (shared README DATA), weight threshold inclusive (>=), lens scoping (non-lens tag ignored), revoked-tag regression (item reappears + getActiveTagWeight returns (false,0)), swap-and-pop re-index survivor lookup, phase-1 budget actually trips (injectable budget; verified the test goes red if the guard is removed), phase-0 all-excluded paging, empty-page-with-non-empty-cursor, excludeTagDef == 0 degenerate.
  • deployedContracts.ts regenerated against the pinned fork — additive ABI, no address drift; deploy-pin-check should pass.

Agents involved

  • Dev: Claude Opus 4.8 (design via architect subagent; implementation + review-fix subagents)
  • Review: Claude Opus 4.8 ×2 — storage-correctness/kernel-safety pass and adversarial DoS/pagination pass (both: no blockers)
  • Orchestrator: James (human)

JamesCarnley and others added 2 commits June 12, 2026 12:55
On-chain consumers (and the explorer) had no way to read a directory
listing with nsfw/system items filtered out — they had to list then
N+1-scan tags client-side. ADR-0042 deliberately kept weight policy in
the client, but overlooked on-chain consumers. This adds the read API
without breaking that kernel-neutrality.

- EdgeResolver.getActiveTagWeight(attester, target, def, targetSchema)
  → (exists, weight): O(1) pure view over the existing _activeByAAS /
  _activeByAASIndex storage. No new state, no index/write-path change.
  Returns the raw weight; the kernel still interprets nothing.
- EFSFileView.getDirectoryPageFiltered(...): the existing directory page
  plus a per-item exclusion predicate — skip an item if any lens has an
  active TAG `excludeTagDef` on it with weight >= minWeight (threshold is
  a caller arg). Handles the file→DATA-via-PIN vs folder→ANCHOR tag-target
  asymmetry; adds a phase-1 scan budget so a 100%-excluded page can't loop
  the source unbounded. Opaque cursor format unchanged.

Extends ADR-0042 into the view layer (kernel stays weight-neutral); does
not supersede it. LIST items pass through unfiltered in v1 (documented).
Regenerated deployedContracts.ts: additive ABI only, no address change
(EdgeResolver's nonce-CREATE address is preserved, so PIN/TAG schema UIDs
are unchanged).

Two independent expert reviews (storage-correctness + adversarial DoS)
found no blockers; their convergent finding — the phase-1 budget was
under-tested — is fixed (budget is now injectable and a test trips it).

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Reviewed-by: Claude Opus 4.8 <noreply@anthropic.com>
Tested-by: Claude Opus 4.8 <noreply@anthropic.com>
Permanence-tier: Durable
Refs: ADR-0048, ADR-0042, ADR-0041
Drop an unused `rootUID` binding (kept the side-effecting createAnchor
call) and apply prettier to EFSFileViewFiltered.test.ts — the `ci` lint
job flagged one no-unused-vars error and formatting. No logic change.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Permanence-tier: Durable

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 027b8748e0

ℹ️ 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".

Comment thread packages/hardhat/contracts/EFSFileView.sol Outdated

@JamesCarnley JamesCarnley left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[claude-opus-4.8 · perf-quick-pass]

Fast performance pass on PR #26 (getDirectoryPageFiltered + getActiveTagWeight). Scope: gas / per-call work / pagination / client follow-up. Not a full audit.

Verdict: no blocking performance findings. The hot path is correctly built on O(1) point-reads, per-call work is bounded, and pagination degrades safely. One minor (P3) cleanup and one positive note below.


Confirmed good

  • Per-item predicate is genuinely O(1). _isItemExcluded resolves the tag target via getActivePinTarget (single _activeBySlot[...] SLOAD) and reads weight via getActiveTagWeight (index lookup _activeByAASIndex[...] + one struct read on _activeByAAS[...] — no list scan, despite operating over the variable-length AAS bucket). No accidental O(n²), no reverse scan of an append-only array. Worst case per item is attesters.length (≤ MAX_ATTESTERS_PER_QUERY = 20) × ~2 SLOADs for files / ×1 for folders.
  • Worst-case page gas is bounded for eth_call. Phase-1 work is capped at _FILE_SCAN_BUDGET_PER_CALL (2048) candidates inspected, and the fetch is clamped to remainingBudget so a 100%-excluded page can't loop the whole phase-1 source in one call. 2048 × 20 lenses × 2 cold SLOADs is the dominant term and stays well inside node eth_call gas caps. Budget sizing (2048) matches the phase-0 sibling — sane.
  • No point-read helper bypassed. The filter uses the O(1) helpers throughout; it does not scan any large append-only structure where a direct read exists. getActiveTagWeight is exactly the right new primitive (pure read over existing storage, no new state, kernel stays weight-neutral).
  • Pagination UX is correct. An all-excluded page returns empty items with a non-empty cursor (cursor is empty iff phase == 1 && fileSourceDone), so the client keeps paging instead of forcing an unbounded single call. The defensive batch.length == 0 && nextCursor != 0 break avoids infinite loops.

P3 — duplicate anchor decode per included item (minor, optional)

EFSFileView.sol_isItemExcluded (L560-565) and _buildFileSystemItems (L616-622) each call eas.getAttestation(uid) + abi.decode(..., (string, bytes32)) on the same anchor UID. For every item that survives the filter, its anchor is fetched and decoded twice in one call: once to classify folder-vs-file in the predicate, once to build the result row. Excluded items decode only once (they never reach _buildFileSystemItems), so the duplication scales with the returned page size (≤ maxItems), not the scan budget.

  • What breaks first: gas / eth_call latency — getAttestation is an external STATICCALL into EAS plus an abi.decode of a dynamic string; doing it twice per kept item is the largest avoidable constant factor on this path.
  • Why it's only P3: bounded by maxItems (typically tens), not by the global dataset, so it never threatens the call. The sibling getDirectoryPageBySchemaAndAddressList doesn't pay this because it has no predicate.
  • Smallest practical improvement: have the predicate return the already-decoded anchorType (or an isFolder bool) up to the caller, or fold the folder/file classification so _buildFileSystemItems reuses it — avoids the second getAttestation/decode for kept items. Pure refactor, no behavior change. Fine to defer; flagging so it's a deliberate choice rather than an oversight.

Positive note — client follow-up is a real perf win (ADR-0048 §Consequences)

ADR-0048 already notes the explorer can later replace its client-side global tag scan with one getDirectoryPageFiltered call. Worth underscoring from a perf lens: the current client does list pageseparate global tag scan per load to hide items (the N+1 / items × attesters × tags shape this PR exists to kill). Collapsing that into a single lens-scoped, O(1)-per-item, budget-bounded eth_call removes a per-render global scan and a serial RPC fan-out from the explorer's hot path. Clear win — worth prioritizing the client swap as a fast-follow, not leaving it open-ended.

@JamesCarnley JamesCarnley left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[claude-opus-4.8 · adr-governance-auditor]

Governance/ADR-honesty pass. Verdict: approve-with-required-record-fixes — the design is sound and accurately documented, but the branch leaves the accepted ADR set and specs out of sync with what merges. None of these block the code; they block "future readers stay honest." Findings are P2/P3 (record-keeping), no P1.

The headline question — is "extends, not supersedes" defensible?yes. ADR-0042's actual Decision (effective TAG = active ∧ weight >= 0; kernel weight-neutral) is untouched. ADR-0048 adds a view-layer reader whose threshold is a caller argument, with minWeight = 0 reproducing 0042's rule. I verified the code matches the claim:

  • EdgeResolver.getActiveTagWeight (EdgeResolver.sol) returns the raw stored int256 verbatim via an index lookup — no sign/magnitude branch. Kernel weight-neutrality (ADR-0041 §4) is preserved: the kernel gains a read, not an interpretation. ✅
  • The weight >= minWeight comparison lives entirely in EFSFileView._isItemExcluded (stateless, redeployable — not Etched). ✅
  • The file/folder tag-target asymmetry in the code (folder → ANCHOR bucket; file → DATA-via-PIN bucket) matches the ADR's load-bearing section exactly. ✅

So 0048 supersedes nothing. But two true statements in ADR-0042 are now incomplete as a description of the system, and immutability means the fix is a forward-pointer in the index, not an edit to 0042's body.


P2 — ADR-0048 missing from the README index + no forward-pointer on ADR-0042's line (record drift)

docs/adr/README.md has no entry for ADR-0048 (confirmed: grep 0048 docs/adr/README.md → nothing), and ADR-0042's index line (L125) carries no "extended by ADR-0048" note. This is exactly the drift this repo's index convention exists to prevent — see the established precedent:

  • ADR-0007: "...array element type widened from bytes32 to TagEntry per ADR-0041"
  • ADR-0044: "LIST_ENTRY shape partly revised by ADR-0046; LIST maxEntries widened by ADR-0047"
  • ADR-0003: "...applies: bool field semantics superseded by ADR-0041"

A future reader who opens ADR-0042 and reads "no new contract function ... lives entirely in client code" (0042 L42) has no breadcrumb telling them a view-layer reader now exists. ADR-0042 is Accepted and immutable, so the body stays — but the index annotation is the sanctioned, in-place-safe mechanism (it touches README, not the ADR).

Fix (this PR):

  1. Add an ADR-0048 entry under "View APIs" in docs/adr/README.md.
  2. Annotate the ADR-0042 line: …descriptive labels](./0042-…md) *(Extended into the view layer by ADR-0048 — kernel stays weight-neutral; threshold becomes a caller arg)*.

This is the single most important record fix: without it the "extends not supersedes" claim is true but undiscoverable from the document it extends.

P2 — Status should flip Proposed → Accepted on merge

ADR-0048 is Status: Proposed (L3) but the code is merge-ready, CI-green, ABI regenerated, 380 tests pass. Per docs/adr/README.md: Accepted = "currently in force. Code reflects this decision." On merge, code reflects this decision, so leaving it Proposed makes the status legend lie. (Proposed = "under discussion, not yet acted on" — false once merged.) Flip to Accepted as part of the merge commit. If the human deliberately wants it to land as Proposed (e.g. the view-layer reader is provisional), say so explicitly in the PR — but the default per discipline is Accepted.

P3 — Spec sync: specs/02 §TAG and overview.md describe effective-TAG as client/explorer-only; an on-chain view filter now exists

The repo rule (specs/README.md): "When code changes alter system behavior visible to consumers ... update the relevant spec in the same PR." The new getDirectoryPageFiltered / getActiveTagWeight are additive (no existing behavior changed), but they are a new consumer-visible read API, and the specs' current framing is now incomplete:

  • specs/02-Data-Models-and-Schemas.md L183: "This distinction applies only to the descriptive-label filter path ... all contract helpers use the kernel 'active = unrevoked' definition unchanged." — still true for helpers, but a reader now infers "no contract ever reads weight for a threshold," which EFSFileView.getDirectoryPageFiltered contradicts.
  • specs/overview.md L67: "For the explorer label-filter only, effective = active with weight >= 0 (ADR-0042)." — "only" is now narrower than reality.

Fix (this PR): one sentence in 02 §TAG noting that a view-layer reader (EFSFileView.getDirectoryPageFiltered, ADR-0048) now applies a caller-chosen weight threshold off the kernel's raw getActiveTagWeight, with the kernel still weight-neutral. Keeps the load-bearing "kernel doesn't interpret weight" invariant crisp while telling readers where threshold logic legitimately lives. P3 because it's additive and the kernel invariant is genuinely unchanged — but it's still same-PR-due under the spec rule.


No internal contradiction introduced

Checked ADR-0041 §4, ADR-0042, ADR-0038, ADR-0036 against the merged code and ADR-0048's claims. The kernel-weight-neutrality invariant (AGENTS.md hardened invariant) holds: getActiveTagWeight reads, never interprets. No accepted ADR is left self-contradictory by this branch. The only inconsistency is the record-completeness gap above (index + specs), not a semantic contradiction.

Verification context (from coordinating agent)

CI green; 380 tests pass; additive ABI, no address change so PIN/TAG schema UIDs unchanged.

Comment thread docs/adr/0048-view-layer-tag-exclusion-filter.md Outdated
Comment thread docs/adr/0048-view-layer-tag-exclusion-filter.md

@JamesCarnley JamesCarnley left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[claude-opus-4.8 · principal-merge-blocker]

Verdict: APPROVE (advisory — human holds the only account that can truly sign off). 0 merge blockers. 1 non-blocking spec-consistency finding (P2) and 1 nit (P3).

I reviewed this independently against ADR-0048, ADR-0041 (§4 weight-neutrality), ADR-0042, ADR-0038, ADR-0036, ADR-0009, specs/02 §TAG, and specs/03. I did not trust the prior reviews; I traced every storage key against the TAG write + revoke paths myself. Findings first.


P2 — specs/03-Onchain-Indexing-Strategy.md enumerates a closed "Three variants" list of EFSFileView; the new public view isn't added

What: specs/03 lines 91-97 introduce EFSFileView's read surface with "Three variants:" and list getDirectoryPage, getDirectoryPageByAddressList, getDirectoryPageBySchemaAndAddressList. This PR adds a fourth public view function (getDirectoryPageFiltered) and a new kernel reader (EdgeResolver.getActiveTagWeight), but no specs/ file is touched.

Why it matters (governing model): AGENTS.md pre-PR checklist and specs/README.md both state the rule — "When code changes alter system behavior visible to consumers, update the relevant spec in the same PR." getDirectoryPageFiltered is a consumer-visible addition on a Durable surface (the Vite client at efs-project/client consumes EFSFileView). A future contributor reading the closed "Three variants" list will cargo-cult it and miss the filtered read — exactly the stale-semantics drift the spec-update rule exists to prevent.

Why NOT a blocker (pre-launch posture): the branch is not undocumented — ADR-0048 fully specifies both functions, the asymmetry, the budgets, and the LIST pass-through. There is no runtime/correctness/ADR contradiction, and the contract surface is Durable (redeployable view), not launch-locked. So this is a documentation-consistency gap, not accidental architectural drift.

Fix direction: in the same PR, add a fourth bullet to the specs/03 variants list (and ideally a one-line note in specs/02 §TAG that a view-layer exclusion reader now exists, pointing at ADR-0048). One or two sentences; no design change.


P3 — getActiveTagWeight lives on a near-Etched surface (EdgeResolver, wired into EFSIndexer); ABI signature is now effectively permanent at launch

Not an action item for this PR — flagging for the 50-year-test record. getActiveTagWeight(address,bytes32,bytes32,bytes32)→(bool,int256) becomes an ABI-visible signature that downstream subgraphs/clients may bind to. It is a pure view over existing storage (no layout change, no write-path change, no append-only-index touch — ADR-0009 intact), and on the pinned fork the EdgeResolver nonce-CREATE address is unchanged so PIN/TAG schema UIDs are preserved. The signature is clean and parameter-ordered consistently with the sibling readers. No change requested; recorded so the launch checklist treats this signature as locked.


Independent correctness verification (the load-bearing items)

getActiveTagWeight keying — VERIFIED CORRECT. It computes _edgeHash(attester, target, definition, TAG_SCHEMA_UID) then reads _activeByAASIndex[definition][attester][targetSchema][edgeHash]. This is byte-identical to:

  • the write path _onAttestTag (EdgeResolver L414-420): same edgeHash (schema = TAG_SCHEMA_UID), index stored at _activeByAASIndex[definition][attester][targetSchema][edgeHash] = arr.length.
  • the revoke path _swapAndPopTagAt (L496): delete indexMap[edgeHash] zeroes the index, and on a swap it recomputes the moved entry's hash with TAG_SCHEMA_UID and rewrites its index.

A revoked tag therefore returns (false, 0) — confirmed by the dedicated revocation test (item reappears in the page AND getActiveTagWeight flips to (false,0)), and the swap-and-pop re-index test proves the survivor stays findable at its correct weight when the first of two entries in a slot is revoked. The read is O(1) (index lookup + struct read, never a list scan) and side-effect-free. weight is returned verbatim with no sign interpretation — ADR-0041 §4 kernel weight-neutrality preserved.

File/folder tag-target asymmetry — VERIFIED CORRECT. _isItemExcluded decodes the anchor ((string, bytes32), guarded by data.length > 0) using the same anchorType == bytes32(0) ⇒ folder rule as _buildFileSystemItems — consistent classification.

  • Folder branch tests getActiveTagWeight(lens, itemAnchorUID, excludeTagDef, ANCHOR_SCHEMA_UID).
  • File branch resolves DATA per-lens via getActivePinTarget(itemAnchorUID, lens, dataSchemaUID) (arg order = (definition=fileAnchor, attester=lens, targetSchema=dataSchemaUID), matching the placement-PIN semantics), then tests the tag on the DATA UID in bucket dataSchemaUID. The "footgun" test (tag on a file ANCHOR excludes nothing) and the reused-README-DATA test (tag on shared DATA, resolved via PIN) both pass. Bucket is correct.

Lens scoping / inclusive >= / negative thresholds / excludeTagDef == 0: all correct and tested. Exclusion is ANY-lens; a non-lens attester's tag does not exclude. excludeTagDef == 0 is safe because _validateDefinition reverts on a zero definition at write time, so _activeByAASIndex[0][...] is always empty → (false,0) → degenerates to "exclude nothing" (== unfiltered page; tested).

Bounding / DoS: the phase-1 scan budget closes the real gap — the filtered variant can DROP phase-1 items (the sibling can't), so a 100%-excluded page would otherwise loop the whole source in one eth_call. The budget is injectable via _fileScanBudgetPerCall()/_folderScanBudgetPerCall() (internal view virtual), and EFSFileViewTestable trips it with a small budget: the test asserts an empty page + non-empty cursor on the first call, strictly increasing fileIdx across resumes (forward progress, no skip/dup), and eventual termination. Removing the guard makes that test fail. The cursor format (ADR-0036) is unchanged; exclusion is a stateless predicate on already-walked positions. Documented limitation (budget bounds the view loop, not the indexer's internal raw scan) is accepted and shared with the sibling.

Additive ABI / pin: deployedContracts.ts diff is purely the two new ABI entries — no address change, PIN/TAG schema UIDs preserved. The EFSRouter/ListEntryResolver/ListReader/ListResolver hunks are prettier whitespace reflow only (no semantic change).

Permanence tier: classification in ADR-0048 is right — Durable (EFSFileView, redeployable/stateless) plus one near-Etched read (EdgeResolver.getActiveTagWeight, pure view over existing storage, no layout/write/append-only change).

Verification results

  • Storage keying of getActiveTagWeight traced byte-for-byte against write (_onAttestTag) and revoke (_swapAndPopTagAt) paths — match.
  • File/folder asymmetry + DATA-via-PIN bucket — correct.
  • Coordinator-reported: CI green (ci, contract-tests, deploy-pin-check), 380 tests passing, deployedContracts.ts additive with zero address change — consistent with the additive ABI-only diff I see.

No merge blocker. Recommend adding the one-line spec note (P2) before/with merge to satisfy the same-PR spec-update rule; everything else is clean.

@JamesCarnley JamesCarnley left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[claude-opus-4.8 · invariant-breaker]

Storage-correctness / kernel-safety / DoS pass over PR #26 (head 027b874). I traced getActiveTagWeight against the TAG write + swap-and-pop revoke paths, the filter's schema buckets, and the phase-0/phase-1 budgets. No runtime blockers. Verdict below, then residual risks + the explicit OOG/infinite-loop answer.

Append-only invariant (ADR-0009) — clean

EdgeResolver.getActiveTagWeight (EdgeResolver.sol:768) adds no storage slot, no write, no index mutation. It is a pure index-lookup-then-struct-read over the existing _activeByAASIndex_activeByAAS. Key parity verified against the write path:

  • Reader builds _edgeHash(attester, target, definition, TAG_SCHEMA_UID); the write path at EdgeResolver.sol:216 builds the same hash with attestation.schema (== TAG_SCHEMA_UID for tags). Match.
  • Nesting [definition][attester][targetSchema] and the index+1 / 0=absent sentinel match _onAttestTag (:414-420) exactly.
  • targetSchema is stored as target.schema of the refUID'd attestation (:212), so a TAG on an ANCHOR stores ANCHOR_SCHEMA_UID and a TAG on a DATA stores DATA_SCHEMA_UID — exactly the two buckets the filter reads. No key mismatch; no stale/missed-live-tag path.

Swap-and-pop survivor (the highest-risk case): correct. _swapAndPopTagAt (:463-497) moves the last entry into the vacated pos and writes indexMap[movedHash] = pos + 1. So after revoking a non-last entry, the moved survivor's position index stays consistent and getActiveTagWeight reading _activeByAASIndex[...][movedHash] → arr[pos] returns the survivor with the correct weight. Directly covered by the test at EFSFileViewFiltered.test.ts:702 (revokes the FIRST of two, asserts the SECOND stays findable with correct weight) — a real regression guard, not a happy-path stub.

Schema-blind reads — none in the filter

_isItemExcluded (EFSFileView.sol): folder branch (anchorType == 0) tests excludeTagDef on the ANCHOR UID under ANCHOR_SCHEMA_UID; file branch resolves the DATA UID per-lens via getActivePinTarget(anchor, lens, dataSchemaUID) and tests under dataSchemaUID. Buckets match the write path above; the file→DATA-via-PIN indirection matches the client's dual-bucket resolveTagSet. The footgun (testing a file's anchor UID) is explicitly guarded and covered (:503). The folder/file classifier (abi.decode (string,bytes32) guarded by data.length>0, anchorType==0 ⇒ folder) is byte-identical to _buildFileSystemItems (:619-622), so the filter introduces no new decode-revert surface and can't disagree with what gets rendered.

DoS / unbounded loop — bounded; no new vector

  • Phase 0: budget caps scanned over _childrenWithEdge (a clean slice — 1 inspected == 1 source entry). scanned++/folderIdx++ happen before the revoked AND excluded checks, so excluded items consume budget and advance the walker. Terminates; cursor monotonic.
  • Phase 1: scanned++ happens before the _isItemExcluded skip (excluded items consume budget). fileIdx = nextFileCur is strictly monotonic; nextFileCur == 0 and the batch.length == 0 guard both break. The want = min(maxItems-count, remainingBudget) clamp keeps inspected-item count ≤ budget regardless of exclusion rate. A 100%-excluded page returns a non-empty cursor at the budget boundary — covered and verified red-without-guard at :529.
  • Inner indexer scan: getAnchorsBySchemaAndAddressList (EFSIndexer.sol:551) can scan up to total raw positions to fill one page when the array is dense with revoked/non-lens entries, returning nextCursor = i (raw position). Because fileIdx advances monotonically and is never revisited, cumulative inner raw-scan across all indexer calls in one eth_call is bounded by total — identical to the sibling getDirectoryPageBySchemaAndAddressList, which this PR does not change. The new phase-1 budget bounds only the genuinely-new per-item exclusion work (O(lenses) O(1) reads). So the filtered variant is no worse than the already-shipped sibling on inner scan and strictly better-bounded on the new work.

Can this OOG or infinite-loop? No. No infinite loop (monotonic cursor + two break guards + dual budgets). The only OOG vector is the pre-existing, shared one (a _childrenBySchema[parent][schema] array with tens of thousands of revoked/non-lens entries making a single indexer page-fill scan the whole array) — not introduced by this PR and explicitly documented in the ADR-0048 "Scan-budget scope" note and the function docstring.

Coverage — the dangerous gaps are actually covered

  • Revoked-tag regression is real: :667 asserts both item-reappears AND getActiveTagWeight → (false, 0).
  • Budget is actually tripped: :529 (and :588, :604) drive EFSFileViewTestable's shrunk budget to a non-empty-cursor/empty-page state and assert termination + no skip/dup; doc claims it goes red if the guard is removed.
  • Swap-and-pop survivor: :702, as above.

I re-ran test/EFSFileViewFiltered.test.ts locally: 15 passing. Compile clean.

Residual risks (non-blocking)

  1. Shared inner-scan OOG on pathological arrays (above) — pre-existing, now inherited by a second public view. Documented. If/when array GC or a maxTraversal cap lands for getAnchorsBySchemaAndAddressList, both views benefit; no action needed for this PR.
  2. No on-chain test of the inner-scan-dominated case — coverage exercises the view's own phase-1 budget, but there's no test seeding a dense revoked array to confirm the indexer-side per-call cost stays in gas limits. Worth a future fuzz/gas test on getAnchorsBySchemaAndAddressList directly (shared concern, not a blocker for this PR).
  3. The List*/EFSRouter edits in this diff are prettier line-wrapping only — zero behavioral change; confirmed.

The List/Router formatting noise aside, the storage and bounding story holds. Approving from the invariant/DoS angle (advisory — human lands).

JamesCarnley and others added 2 commits June 12, 2026 14:40
Apply the PR #26 review squad's convergent record/governance findings
(no code changes):
- Add ADR-0048 to docs/adr/README.md index; annotate ADR-0042's index
  line with a forward-pointer (ADR-0042 is immutable — README annotation
  is the sanctioned in-place mechanism) so a reader landing on the
  "client-only, no new contract function" ADR can discover the view-layer
  extension.
- Flip ADR-0048 Status Proposed -> Accepted (code reflects the decision).
- Spec sync (same-PR rule): note the on-chain caller-threshold filter in
  specs/overview.md and specs/02 §TAG; add getDirectoryPageFiltered as a
  4th EFSFileView variant and getActiveTagWeight to the TAG readers in
  specs/03.
- Log the two non-blocking P3s (duplicate anchor decode; shared inner
  indexer-scan bound) in FUTURE_WORK.

Reviewed-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Permanence-tier: Durable
Refs: ADR-0048, ADR-0042
The file-item exclusion predicate looped lenses and only checked whether
attesters[i] tagged the DATA that attesters[i] itself pinned — so when
one viewed lens tags ANOTHER viewed lens's DATA (view [Alice, Bob],
Alice pins DATA_A, Bob tags DATA_A as nsfw), the item was not excluded,
diverging from the client's union model (FileBrowser.resolveTagSet builds
the effective set from ALL viewed attesters).

Fix the file branch to the union semantic: resolve the deduplicated set
of DATA UIDs any lens placed at the item, then exclude if ANY lens has an
active excludeTagDef tag (weight >= minWeight) on ANY of them. The folder
branch was already correct (one anchor-UID target, all lenses checked).
Internal-logic only — getDirectoryPageFiltered's signature is unchanged,
so deployedContracts.ts is untouched (no ABI/address change). Added a
cross-lens regression test (verified red against the old code) + a
lens-scoping control; ADR-0048 updated to state the union semantic.

Reported-by: ChatGPT Codex <noreply@openai.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Permanence-tier: Durable
Refs: ADR-0048
@JamesCarnley

Copy link
Copy Markdown
Member Author

[claude-opus-4.8 · dev]

Folded into #27 (single PR, per the maintainer's call to ship Overview + the on-chain filter + the explorer wiring together). #27 contains this work plus the multi-tag extension, the explorer consuming the filter, the removal of the unfiltered getDirectoryPage fallback, and an 8-lens review pass. Closing as superseded — no unique commits are lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant