Harden ENSJobPages: Prime-fallback hooks, explicit ENS-side repair, inspector ABI-awareness, and operator tooling - #156
Conversation
![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: f85ca5406d
ℹ️ 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 _managerSupportsViewV1(address manager) internal view returns (bool ok) { | ||
| uint256 version; | ||
| (ok, version) = _staticcallWord(manager, abi.encodeWithSelector(PRIME_VIEW_VERSION_SELECTOR)); | ||
| ok = ok && version == 1; |
There was a problem hiding this comment.
_managerSupportsViewV1() now classifies a manager as “rich” only if ensJobManagerViewInterfaceVersion() exists and returns 1, but the repo’s AGIJobManager exposes getJobCore/getJobSpecURI/getJobCompletionURI (contracts/AGIJobManager.sol:1186-1244) without that version getter. When ENSJobPages is wired to AGIJobManager, handleHook() therefore takes the fallback path, and that path expects jobEmployerOf/jobAssignedAgentOf selectors that only exist on AGIJobManagerPrime (contracts/AGIJobManagerPrime.sol:1095-1100), so create hooks start reverting and the later hooks silently lose their metadata/auth updates.
Useful? React with 👍 / 👎.
| } : { label: '', name: '', uri: '', node: ethers.ZeroHash }; | ||
|
|
||
| const node = authorityReady ? effective.node : (labelProbeNode !== ethers.ZeroHash ? labelProbeNode : preview.node); | ||
| const node = authority[0] ? effective.node : preview.node; |
There was a problem hiding this comment.
For jobs that already have an imported exact label but do not yet have authorityEstablished, jobLabelSnapshot() can legitimately differ from previewJobEnsLabel(). This now always inspects preview.node whenever authority[0] is false, so the inventory probes <current-prefix><jobId> instead of the snapshotted legacy node. In that pre-authority repair state, ownership/resolver/text status is reported for the wrong subdomain and operators can be told that a legacy page is missing or unmanaged even when the real imported node already exists.
Useful? React with 👍 / 👎.
| authReadSupported, | ||
| employerAuthorisedObserved: employerAuth, | ||
| agentAuthorisedObserved: agentAuth, | ||
| authorisationsAsExpected: authReadSupported ? ((employer === ethers.ZeroAddress || employerAuth === true) && (agent === ethers.ZeroAddress || agentAuth === true || terminalSet.has(jobId))) : false, |
There was a problem hiding this comment.
terminalSet only relaxes the agent side of this predicate, but ENSJobPages._revokePermissions() and _lockJobENS() revoke both employer and agent authorisations (contracts/ens/ENSJobPages.sol:908-925). As a result, any correctly finalized/refunded/expired job with readable resolver auth state will still fail authorisationsAsExpected because employerAuth is required to remain true, so inventory-job-pages will classify healthy terminal jobs as permissions-drift and can drive unnecessary repair actions.
Useful? React with 👍 / 👎.
…nspector ABI-awareness, and operator tooling ### Motivation - Make the ENS job-page subsystem production-ready for an unchanged `AGIJobManagerPrime` by ensuring `handleHook(uint8,uint256)` works when the manager does not expose V1 view getters and by exposing operator-safe repair flows. - Eliminate unsafe implicit authority binding across multiple root versions and avoid `issued/ready` overclaiming by depending on observed chain state where possible. - Improve read-side diagnostics so resolver auth-read absence is distinct from an explicit false, and update operator scripts/runbooks to be log-driven and mainnet-accurate. ### Description - Added runtime manager capability detection and a Prime-fallback path in `ENSJobPages.handleHook(...)` so hooks use `IAGIJobManagerPrimeViewV1` when available and fall back to `jobEmployerOf(jobId)` / `jobAssignedAgentOf(jobId)` when not, emitting precise best-effort failure reasons for missing URIs. - Introduced explicit ENS-side, owner-callable repair/replay functions that do NOT require V1 getters: `repairAuthoritySnapshotExplicit`, `repairResolver`, `repairSpecTextExplicit`, `repairCompletionTextExplicit`, `repairTextsExplicit`, `repairAuthorisationsExplicit`, `replayCreateExplicit`, `replayAssignExplicit`, `replayCompletionExplicit`, `replayRevokeExplicit`, and `replayLockExplicit` in `contracts/ens/ENSJobPages.sol`. - Hardened authority snapshot semantics with `_establishAuthorityForRootVersion(...)` and made `repairAuthoritySnapshot(...)` refuse ambiguous multi-root repairs unless `repairAuthoritySnapshotExplicit(...)` is used. - Rebased compatibility booleans: `jobEnsIssued` now checks observed on-chain node existence and `jobEnsReady` requires node + resolver + base metadata presence, not sticky write-attempt flags. - Made `ENSJobPagesInspector` resolver-auth checks ABI-aware: it reports `authReadSupported`, `authObservationIncomplete`, and separates unknown vs false when `isAuthorised(bytes32,address)` is absent. - Updated and added operator scripts that are log-driven and tolerant of the unchanged Prime ABI: `scripts/ens/phase0-mainnet-snapshot.mjs`, `scripts/ens/inventory-job-pages.ts`, `scripts/ens/repair-job-page.ts`, and new `scripts/ens/repair-from-logs.ts`; they emit machine-readable JSON under `scripts/ens/output/`. - Updated docs and runbooks to reflect preview vs effective semantics, explicit repair paths, and current mainnet Prime/ENS defaults (`AGIJobManagerPrime` default addressed in deploy scripts and runbooks). - Added test doubles and unit tests to cover fallback manager behavior and inspector auth-read absence handling: `contracts/test/MockAGIJobManagerPrimeFallback.sol`, `contracts/test/MockPublicResolverNoAuthRead.sol`, and extended `test/ensAuthoritySnapshot.test.js`. ### Testing - Syntax and script validation: ran Node-based syntax checks for `scripts/ens/*` and `repair-from-logs.ts`; all ENS operator scripts passed syntax evaluation (`ok`), and new scripts emit JSON payloads as designed. (succeeded) - Local contract compile checks: compiled patched ENS contracts and new test mocks using a local/bundled `solc` invocation as a syntax/bytecode sanity check; compilation succeeded for the targeted contracts in this environment. (succeeded) - Unit tests: extended `test/ensAuthoritySnapshot.test.js` with cases demonstrating preview/effective separation, multi-root explicit repair requirement, unchanged-Prime fallback hook behavior, and inspector auth-read absence handling; these tests are added and runnable in CI, and were exercised via Truffle artifacts verification where possible. (tests added; full Truffle execution not completed in this environment) - Hardhat/phase-0 and RPC-dependent flows: attempted `npm run ens:phase0:mainnet` and full Hardhat compile in this sandbox but remote RPC and external compiler download failed due to network restrictions (`ENETUNREACH`), so live chain re-audits and canonical 0.8.23 full compile/size remeasure must be run from an operator workstation before mainnet cutover. (blocked by network in this run)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b31331c81
ℹ️ 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".
| (coreOk, ) = manager.staticcall(abi.encodeWithSelector(PRIME_GET_JOB_CORE_SELECTOR, uint256(0))); | ||
| (specOk, ) = manager.staticcall(abi.encodeWithSelector(PRIME_GET_JOB_SPEC_URI_SELECTOR, uint256(0))); | ||
| (completionOk, ) = manager.staticcall(abi.encodeWithSelector(PRIME_GET_JOB_COMPLETION_URI_SELECTOR, uint256(0))); |
There was a problem hiding this comment.
If a legacy AGIJobManager has ever cancelled/deleted job 0, its getJobCore/getJobSpecURI/getJobCompletionURI views all revert for jobId=0 (contracts/AGIJobManager.sol:651-656,1186-1243). In that state _managerSupportsViewV1() returns false even though the V1 surface is present, so handleHook() and the owner replay*() helpers drop into the Prime-only fallback and start calling jobEmployerOf/jobAssignedAgentOf, which legacy managers do not implement.
Useful? React with 👍 / 👎.
| const resolvedNode = authority[0] | ||
| ? authority[5] | ||
| : (resolvedLabel && jobsRootNode !== ethers.ZeroHash | ||
| ? ethers.solidityPackedKeccak256(['bytes32', 'bytes32'], [jobsRootNode, ethers.id(resolvedLabel)]) | ||
| : ethers.ZeroHash); | ||
| : (resolvedLabel && jobsRootNode !== ethers.ZeroHash ? ethers.solidityPackedKeccak256(['bytes32', 'bytes32'], [jobsRootNode, ethers.id(resolvedLabel)]) : ethers.ZeroHash); |
There was a problem hiding this comment.
When operators pass ROOT_VERSION_ID to repair a job under an older root, this planner still hashes resolvedLabel under jobsRootNode() (the current root). That makes the ownership/existence probe look at the wrong ENS name, so an already-existing historical page can be reported as missing or contract-managed incorrectly, and EXECUTE=1 can queue the wrong follow-up action after repairAuthoritySnapshotExplicit() switches authority to the requested root.
Useful? React with 👍 / 👎.
| for (const log of [...completedLogs, ...refundedLogs, ...expiredLogs]) { | ||
| const jobId = Number(ethers.getBigInt(log.topics[1])); | ||
| if (JOB_ID !== null && jobId !== JOB_ID) continue; | ||
| const job = jobs.get(jobId) || { jobId, employer: ethers.ZeroAddress, specURI: '', completionURI: '', terminalObserved: false }; | ||
| job.terminalObserved = true; | ||
| jobs.set(jobId, job); |
There was a problem hiding this comment.
For cancelled Prime jobs, cancelJob() emits JobCancelled, then REVOKE+LOCK hooks, and deletes the job (contracts/AGIJobManagerPrime.sol:1064-1067). This log-derived terminal set ignores that event, so cancelled jobs keep terminalObserved=false, allowAuth=true, and the generated repair calls re-authorize participants instead of replaying revoke/lock.
Useful? React with 👍 / 👎.
1 participant
Motivation
AGIJobManagerPrimeby ensuringhandleHook(uint8,uint256)works when the manager does not expose V1 view getters and by exposing operator-safe repair flows.issued/readyoverclaiming by depending on observed chain state where possible.Description
ENSJobPages.handleHook(...)so hooks useIAGIJobManagerPrimeViewV1when available and fall back tojobEmployerOf(jobId)/jobAssignedAgentOf(jobId)when not, emitting precise best-effort failure reasons for missing URIs.repairAuthoritySnapshotExplicit,repairResolver,repairSpecTextExplicit,repairCompletionTextExplicit,repairTextsExplicit,repairAuthorisationsExplicit,replayCreateExplicit,replayAssignExplicit,replayCompletionExplicit,replayRevokeExplicit, andreplayLockExplicitincontracts/ens/ENSJobPages.sol._establishAuthorityForRootVersion(...)and maderepairAuthoritySnapshot(...)refuse ambiguous multi-root repairs unlessrepairAuthoritySnapshotExplicit(...)is used.jobEnsIssuednow checks observed on-chain node existence andjobEnsReadyrequires node + resolver + base metadata presence, not sticky write-attempt flags.ENSJobPagesInspectorresolver-auth checks ABI-aware: it reportsauthReadSupported,authObservationIncomplete, and separates unknown vs false whenisAuthorised(bytes32,address)is absent.scripts/ens/phase0-mainnet-snapshot.mjs,scripts/ens/inventory-job-pages.ts,scripts/ens/repair-job-page.ts, and newscripts/ens/repair-from-logs.ts; they emit machine-readable JSON underscripts/ens/output/.AGIJobManagerPrimedefault addressed in deploy scripts and runbooks).contracts/test/MockAGIJobManagerPrimeFallback.sol,contracts/test/MockPublicResolverNoAuthRead.sol, and extendedtest/ensAuthoritySnapshot.test.js.Testing
scripts/ens/*andrepair-from-logs.ts; all ENS operator scripts passed syntax evaluation (ok), and new scripts emit JSON payloads as designed. (succeeded)solcinvocation as a syntax/bytecode sanity check; compilation succeeded for the targeted contracts in this environment. (succeeded)test/ensAuthoritySnapshot.test.jswith cases demonstrating preview/effective separation, multi-root explicit repair requirement, unchanged-Prime fallback hook behavior, and inspector auth-read absence handling; these tests are added and runnable in CI, and were exercised via Truffle artifacts verification where possible. (tests added; full Truffle execution not completed in this environment)npm run ens:phase0:mainnetand full Hardhat compile in this sandbox but remote RPC and external compiler download failed due to network restrictions (ENETUNREACH), so live chain re-audits and canonical 0.8.23 full compile/size remeasure must be run from an operator workstation before mainnet cutover. (blocked by network in this run)Codex Task