Harden ENS lean-mode recovery and docs#159
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
AGIJobManagerPrime/contracts/ens/ENSJobPagesInspector.sol
Lines 251 to 252 in 3d1081f
Query the modern resolver with the supported selector
The new auth probe calls isApprovedFor(address,bytes32,address), but the in-tree modern resolver family exposed here is isApprovedFor(bytes32,address) (contracts/test/MockPublicResolverApprove.sol:18-19), and the new test at test/ensAuthoritySnapshot.test.js:207-214 expects that shape. Against that resolver this branch always falls through to isApprovedForAll, so inspectJob() will report false/unknown authorisations for jobs that are actually authorised via per-node approvals.
ℹ️ 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".
| let value = unwrap(await safe(`resolver.authorisations(${jobId})`, () => provider.readContract(nodeResolver, LEGACY_AUTH_ABI, 'authorisations', [node, nodeOwner, target])[0]), null); | ||
| if (value !== null) return value; | ||
| value = unwrap(await safe(`resolver.isApprovedFor(${jobId})`, () => provider.readContract(nodeResolver, MODERN_AUTH_ABI, 'isApprovedFor', [nodeOwner, node, target])[0]), null); | ||
| if (value !== null) return value; | ||
| return unwrap(await safe(`resolver.isApprovedForAll(${jobId})`, () => provider.readContract(nodeResolver, OPERATOR_AUTH_ABI, 'isApprovedForAll', [nodeOwner, target])[0]), null); |
There was a problem hiding this comment.
For wrapped subnames, ens.owner(node) is the NameWrapper contract, not the wrapped token holder that actually owns the approval state. This snapshot uses nodeOwner directly in authorisations(...)/isApprovedForAll(...), unlike inventory-job-pages.ts:176 and ENSJobPagesInspector._resolveAuthOwner(...), so a wrapped-root deployment will misread healthy job pages as permission drift and overreport needsAuthRepair in the generated mainnet audit.
Useful? React with 👍 / 👎.
1 participant
Motivation
handleHook(uint8,uint256)) without changing Prime runtime.Description
contracts/ens/ENSJobPages.solso repeated or conflicting snapshots validate label/root/node consistency and revert on mismatch, preventing silent overwrite/no-op corruption.repairAuthoritySnapshot(jobId, "")when authority is not yet established; require explicit label or existing authority to avoid ambiguous legacy repairs.migrateLegacyWrappedJobPageExplicit(...)and factored the migration into an internal helper so operators can adopt legacy wrapped nodes when the manager does not expose V1 views.isAuthorised(bytes32,address)probes from inspector and inventory scripts and aligned auth observation to canonical resolver families (authorisations(...),isApprovedFor(...),isApprovedForAll(...)) to avoid unsafe assumptions about resolver ABI.scripts/ens/to chunketh_getLogscalls to respect public RPC block-range limits (MAX_LOG_BLOCK_RANGE) so audits and log-driven repair plans work reliably against mainnet providers.agijob-prefix, (b) clearly separate preview vs effective identity, (c) declare keeper-assisted metadata repair as the normal path under unchanged Prime, and (d) remove references to nonexistent manager helpers likesyncEnsForJob.test/ensAuthoritySnapshot.test.jsto exercise: explicit legacy migration without V1 views, blocking of no-label ambiguous repair, and conflict-safety for re-snapshots.Files changed (high level):
isAuthorised(...)fallback probe.Design constraints respected:
AGIJobManagerPrime(zero Prime runtime diff targeted and preserved).handleHook(uint8,uint256)compatibility and non-blocking semantics.Testing
node scripts/ens/audit-mainnet.tsto producescripts/ens/output/audit-mainnet.jsonwhich completed and shows manager/pages pointers, root integrity, and lean manager mode status (succeeded).npm run docs:ens:genand validated docs checks withnpm run docs:ens:check(succeeded).node --check scripts/ens/phase0-mainnet-snapshot.mjs(succeeded); patched scripts to chunk logs after an initial public-RPC block-range error.test/ensAuthoritySnapshot.test.js; test file updated and exercised during local runs, however full Truffle compile/test and the full bytecode/size benchmark runs were attempted but did not complete within the interactive session window (long-running compile/test operations must be completed by CI/operator outside this session).Tests and checks executed in this session (status):
node scripts/ens/audit-mainnet.ts— succeeded (wrote output JSON).npm run docs:ens:gen— succeeded (regenerated ENS reference).npm run docs:ens:check— succeeded (ENS docs checks passed).node --check scripts/ens/phase0-mainnet-snapshot.mjs— succeeded after adding chunked log retrieval.node scripts/ens/inventory-job-pages.ts/node scripts/ens/phase0-mainnet-snapshot.mjs— started and exercised, but long-range mainnet scans were intentionally not run to completion in-session; scripts are now chunked and ready for full runs.npm run size, and other heavy compile/test size runs were initiated but did not finish in the session; they should be run in CI/maintainer environment to produce bytecode numbers (no Prime runtime change was made so no Prime size regression is expected).Codex Task