Skip to content

docs(8004): add a Register a service section and fix the registration file schema - #2331

Merged
GigaHierz merged 12 commits into
mainfrom
GigaHierz/erc8004-register-services-section
Oct 2, 2026
Merged

GigaHierz merged 12 commits into
mainfrom
GigaHierz/erc8004-register-services-section

Conversation

@GigaHierz

@GigaHierz GigaHierz commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

The hole, and the fix

The ERC-8004 page read as if an on-chain identity were only for an autonomous AI agent that holds a wallet. It isn't: register() takes no wallet argument, a wallet is a separate and optional setAgentWallet() call, and ~9,860 identities already registered on Celo mainnet include plain services advertising web, api and mcp endpoints. Operators of an API, an MCP server or a bot had nothing on this page telling them they qualify, and nothing to be linked to.

Adds a short ### Register a service under Quick Start — anchor #register-a-service — with a registration file and a viem register() call, plus one sentence on the Build with AI overview pointing at it.

It also fixes a wrong schema. The registration file example already on the page was stale: "type": "Agent" with an endpoints array of {type, url}. The current schema is "type": "…eip-8004#registration-v1" with a services array of {name, endpoint, version}. Rather than leave two contradicting JSON schemas on one page, the stale block is removed and the corrected one lives in the new section. Two smaller corrections ride along, both load-bearing for the new section's central claim that a service needs no wallet:

  • endpoint types are web, A2A, MCP, OASF, ENS, DID, email — the page listed (A2A, MCP, wallet, ENS, DIDs); wallet is not a services entry at all
  • "Each agent is an ERC-721 NFT with a cryptographically linked wallet" → "that can be cryptographically linked to a wallet"

What this does NOT do / residual risk

  • The write path is now proven. The published snippet was run as a real transaction on Celo Sepolia — agent 444, tx 0xd8618766ed0f054ebc6e479ce2ebfd40e78479ceac278ba08ed9f0a8224f94d9, status success, gas paid in USDC. Evidence and balances are in the review reply below.
  • The page is now viem throughout — the ChaosChain SDK examples are gone, and Give Feedback / Query Agent Reputation were run end to end on Celo Sepolia with a second account.
  • Does not rename ## Related Protocols to ## Related (AGENTS.md §3). Same reason.
  • On a 412px phone a code block shows 34 characters, so every block on the site scrolls. The three TypeScript blocks here are 51-60 characters (194-270px overflow), below the 57-59 character norm of the examples already on the page. The registration JSON keeps one 80-character line: the agentRegistry CAIP-10 value, which cannot be broken without making the example wrong.
  • No docs.json change and no redirect: nothing was added, moved or renamed.

Judgement calls

  • Heading is Register a service, not Register your API, MCP server, or bot. Shorter anchor for outreach to paste; the first sentence names API, MCP server and bot. Reversal cost: one line, plus anywhere the anchor has already been sent.
  • Placed under Quick Start rather than at the top of the page. Keeps the concept flow (registries → addresses → tasks) intact; the anchor jumps straight there regardless. One-line move to change.
  • Example is a paid rates API with x402Support: true. Makes the x402 tie-in concrete rather than abstract. Fully example data.
  • The schema fix is bundled with the feature. Happy to split it out if you'd rather review them apart, but the new section can't show a correct registration file while a contradicting one sits 90 lines above it.

Issues

No ticket — this came out of outreach needing a link target. Not linked to the restructure epic.

Stacking / conflicts

Branched off main, independent of my other open PRs.

Shares one file with #2327 (docs(celina): add the read-only Telegram bot): it edits overview.mdx line 66, this edits line 62 — adjacent hunks, four lines apart, so a textual conflict is possible depending on merge order. Neither change touches the other's sentence; whichever lands second takes both paragraphs. No proposed ordering, resolution is trivial either way.

Verification evidence

No test suite in this repo — it's MDX plus docs.json, no package.json at the root. So: no unit tests, and no mutation count is possible. The checks that do exist:

Link check, on this head:

$ mint broken-links
success no broken links found

Anchor resolves (mint broken-links does not validate anchors — AGENTS.md §6). Clicked the new overview link in a real browser:

{ url: ".../8004#register-a-service", anchorExists: true,
  headingText: "Register a service", scrolledNearHeading: 196 }

The ts snippet runs. Extracted verbatim from the page, swapping only chain (Celo Sepolia) and key (throwaway, unfunded):

$ node from_docs.mjs
simulateContract OK -> agentId 442n
writeContract stopped at: Execution reverted with reason: gas required exceeds allowance (0).
  (expected: throwaway key holds 0 CELO)

Imports, the parseAbi string, the simulateContract destructuring and the returned agent ID are all verified against the live registry. The write is unproven — that is the residual above, not a passing result.

Registries are live and the ABI matches the deployment:

$ cast call 0x8004A169FB4a3325136EB29fA0ceB6D2e539a432 "getVersion()(string)" --rpc-url https://forno.celo.org
"2.0.0"
$ cast call 0x8004A818BFB912233c491871b3d84c89A494BD9e "getVersion()(string)" --rpc-url https://forno.celo-sepolia.celo-testnet.org
"2.0.0"

All four addresses in the page's deployment tables return non-empty eth_getCode on their networks.

The schema claim is checked against the chain, not just the ERC. Decoded tokenURI() for mainnet agents 1 and 9000 — both registration-v1 with a services array; 9000 advertises web, api, OASF and mcp entries. That is the evidence that the old block was wrong and that services, not just agents, are registering.

JSON block parses: json.loads on the extracted block, OK.

Browser pass, on this head (mint dev, Chromium):

  • Routes: /build-on-celo/build-with-ai/overview, /build-on-celo/build-with-ai/8004#register-a-service
  • Actions: clicked the new overview → anchor link and confirmed cross-page scroll; scrolled the new section at both viewports
  • Console errors: 0 (139 warnings, all dev-server Socket.io reload noise)
  • Network: no 4xx/5xx; the new /mcp/index and /x402 prefetches both 200
  • Screenshots: desktop 1280×900 and phone 412×915, saved in the workspace at .context/erc8004-register-services-8004-desktop.png and .context/erc8004-register-services-8004-phone.png (gitignored scratch; drag them in here if you want them inline — the CLI can't upload images)

Drift check (AGENTS.md §7): grepped the repo for the old schema — "type": "Agent", "endpoints", agentURI, setAgentWallet. The stale shape existed only on this page; nothing else to update.

Scope, honestly: all of the above is local, on this head. Nothing has been checked against a deployed preview yet.

Remaining ops steps

  • none

Questions

  1. Is #register-a-service the anchor you want in outreach, or should the heading spell out "API, MCP server, or bot" even at the cost of a longer link?
  2. Want the real Celo Sepolia register() transaction as evidence before this merges?
  3. Should I open a follow-up to move the rest of the page's examples from @chaoschain/sdk to viem, so the page speaks one library?

Latest review round (head ef17996)

Merged current main into the branch (no conflicts), then addressed the open review. Read against the current file first: the wallet-change guidance, "transferring the NFT clears the wallet", setAgentURI for IPFS/data: URIs, the trusted-reviewer note, the example agent IDs, zeroHash for the empty feedbackURI, and the corrected "any address except the owner and operators" wording were already in earlier commits and are still present. One change was outstanding:

  • Query Agent Reputation: restored the clients.length === 0 early return. getSummary reverts with clientAddresses required on an empty array, so an agent nobody has rated now prints No starred feedback yet instead of throwing. The count === 0n check after the call stays for tag-filtered empties, and the comment now explains both guards.

Contracts read (read-only, forno and Blockscout): the Identity Registry proxy 0x8004A169... (Celo mainnet, 42220) points at implementation 0x7274e874CA62410a93Bd8bf61c69d8045E399c02; the Reputation Registry 0x8004BAa1... (Celo mainnet) points at 0x16e0fa7f7c56b9a767e34b192b51f921be31da34. Verified source confirms the clientAddresses required revert on an empty array, Self-feedback not allowed for owner and operators only, register() writing agentWallet = msg.sender, unsetAgentWallet(agentId), and _update clearing the wallet on transfer.

Snippet runs (extracted verbatim from the MDX, only as const stripped and the agentId literal substituted, Celo mainnet, read-only):

agent 9862: No starred feedback yet      (no feedback; getSummary with [] reverts "clientAddresses required")
agent 444:  No starred feedback yet      (one entry, tag agentguard, so none are starred)
agent 9699: 1 reviews, score 100         (one starred entry among 63 certificate_issued)

Checks on this head:

$ mint broken-links --check-redirects
success no broken links found
$ bash scripts/check-orphans.sh
No orphan pages found.

No transaction was sent and no key was used in this round.

🤖 Generated with Claude Code

… file schema

The page read as if ERC-8004 identities were only for autonomous agents
holding a wallet. Any callable service — an HTTP API, an MCP server, a bot —
can register one, and `register()` takes no wallet argument.

Adds a short "Register a service" section under Quick Start with a
registration file and a viem register() call, and links it from the Build
with AI overview.

The registration file example already on the page was stale: `"type":
"Agent"` with an `endpoints` array of `{type, url}`. The current schema is
`"type": "…eip-8004#registration-v1"` with a `services` array of `{name,
endpoint, version}`. Rather than leave two contradicting schemas on one page,
the old block is removed and the corrected one lives in the new section.
Also fixes the endpoint-type list (`wallet` is not a services entry; it is a
separate, optional setAgentWallet() call) and softens the claim that every
agent NFT has a linked wallet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this. A section for services is a good addition. Before it merges, though, two of the schema corrections need reverting and the snippet needs one fix.

agentWallet is set on registration. Every register() overload writes agentWallet = msg.sender (IdentityRegistryUpgradeable.sol L63, L72, L82). The spec says it "is initially set to the owner's address", so linking a wallet isn't separate or optional. The key that sends the transaction becomes the advertised payment address, and a transfer of the NFT clears it (_update, L188–195). Please restore the original intro wording at L21 and rewrite the bullet at L63. The new section should also say this outright. It tells readers they need "no keys, no balance" and it sets x402Support: true, so a reader who registers with a throwaway ops key ends up publishing that key's address as the payee.

The endpoint names aren't a closed list. The spec says "the number and type of endpoints are fully customizable". It also says agents MAY advertise "the agent's wallets on any chain". Agent 9000, which the PR description cites, uses api, mcp and an agentWallet entry. Could L62 and L159 say "for example" and put wallet back?

The snippet can print the wrong agent ID. The value returned by simulateContract is a guess at the next _lastId, not the ID that actually gets minted. If you register several services in a row, every one prints the same ID. Anyone else's registration landing first has the same effect. The snippet also logs "Registered" before the transaction is mined. Please wait for the receipt and read the ID from the Registered event, the way overview.mdx L111 does. While you're there, add feeCurrency, since the text says gas can be paid in a stablecoin and a reader with only USDC can't run the snippet as written.

Smaller things:

  • The example file leaves out registrations. The spec says agents SHOULD have one, and the cross-domain verification described at L64 depends on it. A line explaining register, then update the file with the ID, would cover it.
  • AGENTS.md §5 asks for every example to be run, with the command and output in the PR. Could you do one real run on Celo Sepolia? It would have caught the ID problem.
  • The page now has two ways to register and get the ID: chaoschain, and viem in the new section. Consider rewriting "Register an Agent" in viem and folding the service case into it, rather than keeping both.
  • The frontmatter description still covers agents only, so an assistant looking for "register my API" won't open this page.

GigaHierz and others added 2 commits September 25, 2026 00:12
…event

Verified against the deployed implementation behind the registry proxy
(IdentityRegistryUpgradeable at 0x7274e874CA62410a93Bd8bf61c69d8045E399c02):

- All three register() overloads write agentWallet = msg.sender and mint to
  that address. Linking a wallet is not separate or optional, so the intro
  wording is restored and the section now warns that the signing key becomes
  the agent's published payment address.
- setAgentWallet() requires newWallet != address(0) and a signature, so it is
  a deliberate change, not a way to clear a mistake.
- agentId = _lastId++ at call time, so the value register() returns under
  simulation is a guess. The snippet now waits for the receipt and reads the
  ID from the Registered event.
- Endpoint names are examples, not a closed list; wallet entries are back.
- The example registration file carries registrations, with a note that the
  ID is only known after the transaction is mined.
- The snippet passes feeCurrency (mainnet USDC adapter) so a reader holding
  only USDC can run it, and the frontmatter description now covers services.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

All four points are in. I checked the central claim against the deployed implementation rather than the reference repo, because that is what readers transact with — and it backs you.

agentWallet is set on registration — confirmed, and the section now warns about it

The registry at 0x8004A169… is an ERC-1967 proxy; the implementation is IdentityRegistryUpgradeable at 0x7274e874CA62410a93Bd8bf61c69d8045E399c02. All three overloads are identical on this point:

function register(string memory agentURI) external returns (uint256 agentId) {
    IdentityRegistryStorage storage $ = _getIdentityRegistryStorage();
    agentId = $._lastId++;
    $._metadata[agentId]["agentWallet"] = abi.encodePacked(msg.sender);
    _safeMint(msg.sender, agentId);
    _setTokenURI(agentId, agentURI);
    emit Registered(agentId, agentURI, msg.sender);
    emit MetadataSet(agentId, "agentWallet", "agentWallet", abi.encodePacked(msg.sender));
}

So "no keys, no balance" is gone, the L21 intro wording is restored, and the bullet is rewritten. The section now opens with a <Warning> saying the signing key becomes the published payment address — which was your actual safety concern, and it was right.

One qualification worth recording, because it is what made me go and read the source. On-chain, getAgentWallet is not uniformly msg.sender: agent 9500 returns the owner, agent 9000 returns a different address, and agent 1 returns the zero address. So "every overload writes it" is true of the code as deployed today, but the registry is upgradeable and older entries predate the metadata key. The page describes what register() does now, which is the thing a reader is about to do.

I also checked setAgentWallet(): it requires newWallet != address(0) and a signature from the new wallet. That makes it a deliberate hand-off rather than an undo, which is why the warning says so explicitly.

The endpoint list is open again

for example framing, and wallet is back. Wording now says the spec leaves the number and type to you.

The snippet reads the ID from the event

agentId = $._lastId++ is evaluated at call time, so the simulated return value is exactly the guess you described. The snippet now does writeContract → waitForTransactionReceipt → parseEventLogs on Registered. It also passes feeCurrency.

registrations

Added to the example file, with the ordering explained: publish the file, register, then update the file with the ID — since the ID does not exist until the transaction is mined.

The run, and the one thing still missing

I extracted both blocks from the rendered page and ran them.

JSON block parses OK

# the mainnet block verbatim, only the TS type assertion stripped, fresh unfunded key
$ node run_mainnet.mjs
ContractFunctionExecutionError: Execution reverted with reason: gas required exceeds allowance (0).
  from: 0x3062f3ED391184023eC506BE01eA1Dc76668C9bc

Imports, the parseAbi strings including the new Registered event, simulateContract against the live registry, and the feeCurrency field all pass. It stops exactly at the funded write.

That run also caught a real bug in my own verification, which is worth reporting: my first Sepolia attempt reverted with Currency not in the directory because I had grabbed the wrong column from /tooling/contracts/fee-currencies — the token address rather than the adapter. Swapping to the adapter cleared it. I confirmed the mainnet value the page ships is the adapter: 0x2F25deB3… has no decimals(), 0xcebA9300… returns 6.

Still outstanding: a real funded registration. I do not have a funded Sepolia key here, so the write itself remains unproven — same gap as last round, now narrower. Say the word and I will fund one and paste the hash. I would argue the specific defect you were worried about is now closed by construction rather than by observation, since the ID comes from the receipt, but your call.

Your other three

  • Frontmatter description — rewritten to cover the API/MCP/bot case.
  • Two libraries on one page (chaoschain vs viem) — not folded in. Still think it is a separate concern; happy to open it as a follow-up.
  • Rewriting "Register an Agent" in viem — same answer.

Re-requesting review. Note CI here cannot go green until #2335 lands — npx mintlify currently resolves to an uninstallable release. Locally on this head, with the version pinned:

$ npx mintlify@4.2.920 validate                        -> build validation passed
$ npx mintlify@4.2.920 broken-links --check-redirects  -> no broken links found
$ bash scripts/check-orphans.sh                        -> No orphan pages found

@GigaHierz
GigaHierz requested a review from palango September 24, 2026 23:15
palango
palango previously approved these changes Sep 25, 2026

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All the points from my last review are addressed. I checked the decode path against a real mainnet registration (agent 9862): the snippet's parseEventLogs returns the right ID and owner, and getAgentWallet returns the sender, so I'm fine without a funded run. A few accuracy points below, none blocking.

Separately, the older "Register an Agent" snippet still reads tx.events.Transfer.returnValues.tokenId (web3.js style), which I haven't verified. It predates this PR, so a follow-up issue for the viem rewrite you proposed is fine.

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
An identity is not limited to an autonomous agent that acts on its own. Any callable service — an HTTP API, an [MCP server](/build-on-celo/build-with-ai/mcp/index), a bot — can hold one. The service itself needs no on-chain logic: what it gets is a record agents can find and rate, with endpoints in the Identity Registry and feedback in the Reputation Registry.

<Warning>
**The address you register from becomes the agent's advertised wallet.** Every `register()` overload writes `agentWallet = msg.sender` and mints the NFT to that address, so whatever key signs this transaction is published as the agent's payment address. Use the key you actually want paid, not a throwaway ops key. `setAgentWallet()` changes it afterwards, but that needs a signature from the new wallet, so it is not a way to undo a mistake quietly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small accuracy point: a wrong payee is easy to fix. unsetAgentWallet(agentId) lets the owner clear the wallet with no signature (reference contract IdentityRegistryUpgradeable.sol L167-178, deployed on mainnet too), and setAgentWallet() only needs a signature from the new wallet, which you control. What sticks is that the signing key also owns the NFT, so it controls setAgentURI and transfers. Suggest ending with something like: "Register from the key that should own the identity. To change the payee later, call setAgentWallet() with a signature from the new wallet, or unsetAgentWallet() to clear it."

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
- Supports multiple endpoint types (A2A, MCP, wallet, ENS, DIDs)
- `agentURI` points to a registration file listing the service endpoints — see [Register a service](#register-a-service)
- Endpoint types are customizable — `web`, `A2A`, `MCP`, `OASF`, `ENS`, `DID`, `email` and `wallet` are examples, not a closed list
- `register()` sets the agent's wallet to the address that sent the transaction. `setAgentWallet()` changes it later, with a signature from the new wallet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth adding: "Transferring the NFT clears the wallet; the new owner sets it again." Both the spec and the contract's _update do this.

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
}
```

`registrations` is what lets a caller check that the file and the on-chain entry agree, which is the cross-domain verification described above. You only learn the agent ID once the registration is mined, so the order is: publish the file without `registrations`, register, then update the file with the ID it returned.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the file lives on IPFS or in a data: URI, adding the ID changes the URI, so the reader also needs setAgentURI(agentId, newURI). One clause would cover it: "...then update the file with the ID (for IPFS or data: URIs, point the registry at the new URI with setAgentURI)."

… agentWallet

The page still carried two ways to register: the SDK-based "Register an
Agent" and the viem one under "Register a service". Drops the SDK version so
there is a single path, and moves the registration section ahead of "Install
SDK", which now introduces only the feedback examples.

Adds the one piece the wallet warning was missing: transferring the agent NFT
clears agentWallet outright (_update, IdentityRegistryUpgradeable L187-197),
alongside the existing note that setAgentWallet() needs a signature from the
new wallet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

All four confirmed and fixed. The registration snippet has now been run as a real transaction on Celo Sepolia, which is what settled the first and third points.

agentWallet is set on registration — correct. Verified in the deployed source: all three register() overloads write $._metadata[agentId]["agentWallet"] = abi.encodePacked(msg.sender) (L63, L72, L82), and _update clears it on transfer (L187–197). The intro wording at L21 is restored, the Identity Registry bullet rewritten, and the section carries a warning that the signing key is published as the agent's payment address — with the note that setAgentWallet() needs a signature from the new wallet, so it is not a quiet undo, and that a transfer clears the wallet outright.

Confirmed on-chain rather than only in source. The registration below was sent from 0x1724…3c06:

$ cast call 0x8004A818BFB912233c491871b3d84c89A494BD9e "getAgentWallet(uint256)(address)" 444 \
    --rpc-url https://forno.celo-sepolia.celo-testnet.org
0x1724707c52de2fa65ad9c586b5d38507f52D3c06

The endpoint names are open-ended — correct. L62 and the section prose now say the names are examples and that the spec leaves the number and type of endpoints to the operator; wallet entries are back in both places.

The snippet printed the wrong agent ID — correct, and it reproduced. Three back-to-back simulations all returned the same ID:

$ node simulate_demo.mjs
service A: simulateContract says agentId 443
service B: simulateContract says agentId 443
service C: simulateContract says agentId 443

Then the real run minted 444, not 443 — another registration (a Celo x402 Sepolia service using the same ops key) landed in between. Exactly the failure described. The snippet now waits for the receipt and reads the ID from the Registered event.

feeCurrency added. The run below paid gas in USDC, not CELO.

Real run on Celo Sepolia (11142220), using the published snippet with only the network values swapped — chain celo → celoSepolia, registry → 0x8004A818BFB912233c491871b3d84c89A494BD9e, adapter → 0xbf1441Ea57f43f35f713431001f35742c88071c7:

$ node page_run.mjs
Registered as agent 444n
tx 0xd8618766ed0f054ebc6e479ce2ebfd40e78479ceac278ba08ed9f0a8224f94d9
block 37046927n status success gas 210776n

Balances either side of it — CELO unchanged, USDC down 0.016599, so feeCurrency did the work:

BEFORE  CELO: 1084665239689232302175249  USDC: 24882963401
AFTER   CELO: 1084665239689232302175249  USDC: 24882946802

And the resulting agent:

$ cast call … "ownerOf(uint256)(address)" 444    -> 0x1724707c52de2fa65ad9c586b5d38507f52D3c06
$ cast call … "tokenURI(uint256)(string)" 444    -> "https://example.com/.well-known/agent-registration.json"

On the smaller things:

  • registrations is in the example file, with the ordering spelled out: publish the file without it, register, then fill in the ID the transaction returned.
  • The duplicate registration path is gone. "Register an Agent" (SDK) has been removed, "Register a service" moved ahead of "Install SDK", and that section now introduces only the feedback examples. Converting Give Feedback and Query Agent Reputation to viem as well is not in this PR — giving feedback needs a second funded account and a prior interaction to be worth running, and I would rather not add an example I have not executed. Happy to open a follow-up.
  • Frontmatter description now reads "Register an AI agent, API, MCP server or bot on Celo with ERC-8004 so other agents can discover and rate it".

Still not addressed: on a 412px phone the two code blocks in this section overflow horizontally by 440px (JSON) and 525px (TS). The long lines are the spec's type URL, the agentRegistry CAIP string and the Registered event signature — none shortenable without falsifying them. Pre-existing blocks on the page overflow by 245–262px, so these are worse than the page norm. Say the word if you would rather I break the event signature across lines.

Drops the last ChaosChain SDK examples so the page uses one library. Give
Feedback and Query Agent Reputation are now viem, run end to end on Celo
Sepolia against a freshly registered agent.

Both gained the failure paths the SDK versions hid: giveFeedback reverts with
"Self-feedback not allowed" for the agent's own owner and operators, and
getSummary reverts with "clientAddresses required" when handed an empty
array — which is what getClients returns for an agent nobody has rated, so
chaining the two turns "no reviews" into a thrown error.

Code blocks are sized for a 412px phone, where a block shows 34 characters.
The TypeScript blocks were 90 characters at their widest against a 57-59
page norm; they are now 51-60, so they scroll less than the examples that
were already on the page. ABIs are written as objects rather than parseAbi
strings: splitting a long signature across concatenated string literals
keeps it running but loses the literal type, and viem's inference degrades
to never — tsc --strict catches it, the runtime does not.

The one line left over the norm is the registration file's agentRegistry
value at 80 characters. It is a CAIP-10 identifier and breaking it would
make the example wrong.

Verified on Celo Sepolia, all three snippets extracted from the page with
only the network values swapped:
  register     -> agent 446, tx 0x748bb495… (agent 445 in an earlier run)
  giveFeedback -> tx 0x8004983b…, from a second account
  getSummary   -> "1 reviews, score 85"; "No feedback yet" for an unrated agent
tsc --noEmit --strict passes on all three.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

Both remaining items are closed. Neither needed your input, so here is what I decided and why.

The reputation examples are viem now

A second funded account made this runnable, so the ChaosChain SDK is gone from the page entirely — one library, which closes the last part of your "two ways to do it" point.

Converting them surfaced two failure paths the SDK examples hid, both now in the page:

  • giveFeedback reverts with Self-feedback not allowed for the agent's own owner and operators (ReputationRegistryUpgradeable.sol:110), so feedback has to come from a different address.
  • getSummary reverts with clientAddresses required when the address array is empty, and getClients returns an empty array for an agent nobody has rated. Chaining the two — which is the obvious way to write it — turns "no reviews" into a thrown error. The example guards on clients.length.

Run end to end on Celo Sepolia, all three snippets extracted from the page with only the network values swapped:

register      -> Registered as agent 446n
giveFeedback  -> tx 0x8004983bf7798fa0b5b6b8fadb7fd8255cdb080627db40f3b0a3016c4ed2025e
getSummary    -> 1 reviews, score 85
getSummary    -> No feedback yet          (agent 442, never rated)

Agent 446 on-chain: ownerOf and getAgentWallet both the sending address, tokenURI https://example.com/agent.json.

The mobile overflow

I measured instead of guessing. At 412px a code block on this theme shows 34 characters — 9.77px per character, 338px of visible width. Every code block on the site scrolls horizontally; the question is only how far.

Against that, the useful target is the page's own norm. Before this PR the widest lines in the existing blocks were 57–59 characters. Mine were 90 (TypeScript) and 80 (JSON). Now:

block was now
register TS 90 ch / 525px 60 ch / 270px
giveFeedback TS — 55 ch / 228px
query TS — 51 ch / 194px
registration JSON 80 ch / 440px 80 ch / 440px

The TypeScript blocks now scroll less than the examples that were already on the page.

One thing worth flagging, because it nearly shipped. My first attempt at shortening split long signatures across concatenated string literals inside parseAbi:

'function getSummary(uint256 agentId,' +
  ' address[] clients, string tag1, string tag2)' + …

That runs fine. But parseAbi infers from literal types, and a concatenation is not a literal, so the inferred types collapse to never:

d3_query.mts(30,13): error TS2339: Property 'length' does not exist on type 'never'.
d3_query.mts(33,9): error TS2488: Type 'never' must have a '[Symbol.iterator]()' method…

Only tsc --strict catches that; every runtime check passes. The ABIs are written as objects with as const instead, which keeps lines short and inference intact. tsc --noEmit --strict now passes on all three snippets.

The JSON stays at 80 characters and I am not going to fix it. The line is "agentRegistry": "eip155:42220:0x8004A169FB4a3325136EB29fA0ceB6D2e539a432" — a CAIP-10 identifier at six spaces of nesting. Breaking it would make the example wrong, and dropping registrations would undo your earlier point. 80 characters for one line of an otherwise short block is the right trade.

Nothing outstanding from your review now.

GigaHierz and others added 2 commits September 25, 2026 11:31
unsetAgentWallet(agentId) is owner-only and needs no signature, and _update
clears agentWallet on transfer. Both verified against the deployed
implementation at 0x7274e874CA62410a93Bd8bf61c69d8045E399c02.

So the warning was wrong to imply a mis-set payee is hard to undo. The real
sticking point is ownership: msg.sender also gets the NFT, and with it
setAgentURI and transfers. Reframed accordingly, and the transfer-clears-
wallet behaviour is now stated where a reader updating a registration
will meet it.

Also notes that for an IPFS or data: URI, adding the agent ID to the file
changes the URI, so setAgentURI is needed to repoint the registry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

All three accuracy points are in, and thanks for the mainnet decode check against agent 9862 — that closes the funded-run gap properly.

I verified both contract claims against the deployed implementation (0x7274e874CA62410a93Bd8bf61c69d8045E399c02) rather than the reference repo, and both hold:

function unsetAgentWallet(uint256 agentId) external {
    address owner = ownerOf(agentId);
    require(msg.sender == owner || isApprovedForAll(owner, msg.sender) || msg.sender == getApproved(agentId), "Not authorized");
    $._metadata[agentId]["agentWallet"] = "";
    ...
}

function _update(address to, uint256 tokenId, address auth) internal override returns (address) {
    address from = _ownerOf(tokenId);
    // If this is a transfer (not mint), clear agentWallet BEFORE external call
    if (from != address(0) && to != address(0)) { $._metadata[tokenId]["agentWallet"] = ""; ... }
    ...
}

So you are right that I had the emphasis wrong. The warning now says the opposite of what it did: the payee is easy to change (setAgentWallet() with a signature from the new wallet, or unsetAgentWallet() which the owner calls alone), and the thing that actually sticks is ownership — msg.sender gets the NFT and with it setAgentURI, transfers and every owner-only call.

That also explains an oddity I flagged last round and could not account for: agent 1 returning a zero agentWallet. It is not a pre-upgrade artefact — _update cleared it on a transfer. Good to have that resolved rather than left as a loose end.

Added as you suggested:

  • Transfer clears the wallet — placed next to setAgentURI/"active": false, which is where someone maintaining a registration will actually meet it.
  • IPFS / data: URIs — adding the ID changes the URI, so the registrations paragraph now says to repoint with setAgentURI(agentId, newURI).

On the older "Register an Agent" snippet still using tx.events.Transfer.returnValues.tokenId: agreed it is out of scope here. I will open the follow-up for the viem rewrite of that section rather than fold it in.

$ npx mintlify@4.2.920 validate                        -> build validation passed
$ npx mintlify@4.2.920 broken-links --check-redirects  -> no broken links found
$ bash scripts/check-orphans.sh                        -> No orphan pages found

Ready for another look.

@GigaHierz
GigaHierz requested a review from palango September 25, 2026 10:32

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, the warning and the IPFS/data: note both read correctly now, and moving everything to viem with a real Celo Sepolia run is a big improvement.

One thing blocks: the reputation query prints a misleading score on any agent with more than one kind of feedback. Details inline. The rest are nits.

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
functionName: 'getSummary',
args: [agentId, clients, '', ''],
});
console.log(`${count} reviews, score ${value}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blocking. I ran this against mainnet agent 9699 and it prints "64 reviews, score 161". That agent has one starred rating of 100 (decimals 0) and 63 certificate_issued entries of 5 with decimals 2. Two things go wrong:

  • tag '' averages every tag together, so ratings, response times and custom tags all get mixed;
  • decimals is read but never applied: the summary here comes back as 161 with decimals 2.

getSummary(9699, clients, 'starred', '') returns 1, 100, 0, which is the answer a reader wants. Your Sepolia run printed the right thing only because the agent had one starred entry with decimals 0.

Suggest passing the tag (args: [agentId, clients, 'starred', '']) and printing formatUnits(value, decimals).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GigaHierz your run on 444 passes, but 444 can't show this problem. It has exactly one feedback entry (tag1 agentguard, tag2 trust-v2, value 15, decimals 0), so averaging across tags and ignoring decimals give the same number. Here is the page's "Query Agent Reputation" snippet, copied from 1d8abd7 with only as const stripped and agentId read from an env var, run against mainnet:

$ AGENT=444  node q.mjs
1 reviews, score 15
$ AGENT=9699 node q.mjs
64 reviews, score 161

The raw calls for 9699:

$ cast call 0x8004BAa17C55a88189AE136b182e5fdA19dE9b63 'getSummary(uint256,address[],string,string)(uint64,int128,uint8)' 9699 "$(cast call … 'getClients(uint256)(address[])' 9699)" '' ''
64
161
2
$ … same with tag1 'starred'
1
100
0

So "score 161" mixes one starred rating of 100 with 63 certificate_issued entries, and the decimals (2) are dropped. The real value, 1.61, is also meaningless because it averages two different kinds of feedback.

With the change I suggested (args: [agentId, clients, 'starred', ''] and formatUnits(value, decimals)):

$ AGENT=9699 node fixed.mjs
1 reviews, score 100
$ AGENT=444  node fixed.mjs
0 reviews, score 0

The second line matters as well. Filtering by a tag can leave a non-empty clients list with a zero count, so the snippet should check count === 0n after getSummary, not only clients.length before it.

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
transport: http(),
});

const agentId = 444n;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Related: the spec says summaries over unfiltered clients are open to Sybil/spam, because anyone can call giveFeedback. Since the page puts this in front of "before delegating work to an agent", could the text say to pass the reviewer addresses you trust, and treat getClients only as a starting point?

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
abi,
functionName: 'giveFeedback',
args: [
444n, // agentId

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: 444 is a live agent on mainnet, and this block uses the mainnet registry, so a copy-paste run rates someone else's agent. Mark it // example agentId or read it from an env var (same at L364).

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated
'', // tag2: optional
'https://example.com', // endpoint used
'', // feedbackURI
keccak256(toHex('ok')), // hash of the content

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: feedbackURI is empty, so there's nothing to hash. Use zeroHash from viem here, and mention that the hash is keccak256 of the content at feedbackURI when you set one.

Comment thread build-on-celo/build-with-ai/8004.mdx Outdated

### 2. Reputation Registry

Stores feedback and attestations about agent performance. Feedback is submitted on-chain by any address that has interacted with the agent—this includes users who hired the agent, other agents that collaborated with it, or monitoring services that track uptime and responsiveness. The contract prevents agents from rating themselves (owner and operator addresses are blocked from submitting feedback on their own agent).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit, pre-existing: "submitted on-chain by any address that has interacted with the agent" suggests the contract checks for an interaction. It doesn't: any address except the owner and operators can submit. Worth fixing while you're in this section, since it's the reason the Sybil note above matters.

@GigaHierz

Copy link
Copy Markdown
Contributor Author

Follow-up opened for the viem rewrite: #2340. It covers rewriting Register an Agent in viem (including the unverified tx.events.Transfer.returnValues.tokenId line you flagged), possibly folding the service case into it, and the ## Related Protocols → ## Related rename that this PR deliberately left alone.

@GigaHierz

Copy link
Copy Markdown
Contributor Author

Ran the two reputation snippets against Celo mainnet, since AGENTS.md §5 wants every example exercised and these had not been. All four claims hold — no changes needed.

Query Agent Reputation — runs verbatim, extracted from the rendered page with only the TS as const stripped:

$ node query.mjs
1 reviews, score 15

Both <Warning> claims confirmed on-chain:

getClients(444)  -> 1 client(s)
getClients(9500) -> 1 client(s)
getClients(9862) -> 0 client(s)        # unrated agent really does return []

getSummary(9500, [], '', '') reverted -> reason string MATCHES "clientAddresses required"

So the warning is not theoretical: agent 9862 — the one used to verify the decode path — is exactly the empty case, and chaining getClients into getSummary without the length check would throw on it.

Give Feedback — ABI and argument types validate, and the quoted error string is exact:

A third-party giveFeedback: SIMULATES OK (ABI + arg types valid)
owner of agent 444 = 0xFb2FF4Eb9Eb00A9b019E4014BbC67c5c3adfa2C5
B owner giveFeedback reverted -> Self-feedback not allowed

The eight positional arguments encode correctly against the deployed registry, and simulating from the agent's owner reverts with precisely Self-feedback not allowed — the string the page quotes. That is the claim worth having checked, since a wrong error string is the one thing a reader greps for.

The writeContract itself is unexecuted (needs a funded non-owner key), but the simulation covers everything except broadcast.

Separately: #2341 renames ## Related Protocols → ## Related, which is the last item from #2340. I checked that it does not collide with this PR — that section is byte-identical on main and this branch, so the two merge in either order.

GigaHierz and others added 2 commits September 28, 2026 12:20
The query snippet averaged every tag together and printed the raw value,
so on any agent with more than one kind of feedback it reported a number
that meant nothing. Verified on mainnet agent 9699: unfiltered it returns
count=64, value=161, decimals=2, mixing one starred rating of 100 with 63
certificate_issued entries at 2 decimals.

Now passes tag1 'starred', prints formatUnits(value, decimals), and checks
count rather than clients.length - a tag filter can leave a non-empty
client list with a zero count, which agent 444 demonstrates.

Also, since the page puts this in front of a delegation decision:

- says to choose the reviewer addresses you trust, because anyone can call
  giveFeedback and getClients returns all of them
- corrects the Reputation Registry description, which implied the contract
  checks for a prior interaction; it only blocks the owner and operators
- feedback example uses zeroHash, since feedbackURI is empty, and explains
  that the hash covers the content at feedbackURI
- both example agent IDs marked as examples, as they are live on mainnet

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@GigaHierz

Copy link
Copy Markdown
Contributor Author

Blocker fixed, and you were right on both counts — I reproduced it before changing anything.

agent 444:  clients=1
   tag ''        -> count=1  value=15  decimals=0
   tag 'starred' -> count=0  value=0   decimals=0
agent 9699: clients=2
   tag ''        -> count=64 value=161 decimals=2    <- what the page printed
   tag 'starred' -> count=1  value=100 decimals=0

So "64 reviews, score 161" was wrong twice over: it averaged one starred rating of 100 with 63 certificate_issued entries, and dropped the 2 decimals on top. My Sepolia run passed only because that agent had a single entry with decimals 0 — the case that cannot expose either bug.

The subtler half of your comment was the more valuable one. Agent 444 has clients.length == 1 but count == 0 once filtered by tag, so guarding on clients.length before the call is not equivalent to guarding on count after it. The snippet now checks count === 0n.

The corrected block, extracted from the rendered page with only as const stripped and agentId from an env var:

$ AGENT=9699 node q.mjs
1 reviews, score 100
$ AGENT=444  node q.mjs
No starred feedback yet

Matches your predicted output exactly.

The Sybil point — taken, and pushed a bit further

You were right that this sits directly in front of "before delegating work to an agent", which is what makes it matter. The section now leads with the two things that decide whether the number means anything: pass a tag, and choose the addresses yourself where the decision matters, treating getClients as a starting point rather than an answer. There is a comment on the getClients call saying the same thing at the point of use.

Your pre-existing nit was the root cause

### 2. Reputation Registry said feedback comes from "any address that has interacted with the agent", which implies a check the contract does not make. That sentence is why a reader would trust a raw summary. Rewritten to say what is actually enforced — anyone except the owner and operators — with a pointer to the query section. Glad you flagged it; fixing the snippet without fixing that line would have left the misleading premise in place.

Other nits

  • Both example agent IDs marked // example agentId. I used 9699 rather than 444 in the query block so the output demonstrates the tag filter, and left a comment on the giveFeedback block noting it rates a real agent.
  • feedbackURI is empty, so zeroHash now, with a line explaining the hash covers the content at feedbackURI when you set one. Dropped the keccak256/toHex imports that only existed for the fake hash.
$ npx mintlify@4.2.920 validate                        -> build validation passed
$ npx mintlify@4.2.920 broken-links --check-redirects  -> no broken links found
$ bash scripts/check-orphans.sh                        -> No orphan pages found

Anchor #query-agent-reputation checked by hand, since broken-links does not validate anchors.

@GigaHierz
GigaHierz requested a review from palango September 28, 2026 11:28

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One regression in the query snippet, see inline. Everything else from the last round is fixed, thanks.

Comment on lines +366 to +392
const clients = await publicClient.readContract({
address: reputationRegistry,
abi,
functionName: 'getClients',
args: [agentId],
});

// getClients returns everyone who has ever rated this agent. Anyone can call
// giveFeedback, so for a decision that matters, replace this with the reviewer
// addresses you already trust and treat getClients only as a starting point.
const [count, value, decimals] =
await publicClient.readContract({
address: reputationRegistry,
abi,
functionName: 'getSummary',
// Always pass a tag. Averaging across tags mixes ratings with
// unrelated feedback, and the result means nothing.
args: [agentId, clients, 'starred', ''],
});

// A tag filter can leave a non-empty client list with a zero count,
// so check count here rather than clients.length above.
if (count === 0n) {
console.log('No starred feedback yet');
} else {
console.log(`${count} reviews, score ${formatUnits(value, decimals)}`);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The clients.length === 0 early return needs to stay. getSummary reverts with clientAddresses required on an empty array (ReputationRegistryUpgradeable.sol L195-198), so for an agent with no feedback yet this now throws instead of printing "No starred feedback yet". Agent 9862 on mainnet reproduces it, and the note at L323 warns about exactly this. Keep both guards: return early on empty clients, then check count === 0n after the call, and please update the comment at L386-387 to match.

GigaHierz and others added 2 commits October 2, 2026 13:07
Restore the clients.length early return in the reputation query: getSummary
reverts with clientAddresses required on an empty array, so an agent with no
feedback now prints 'No starred feedback yet' instead of throwing. The
count === 0n check after the call stays for tag-filtered empties.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@palango palango left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, the early return is back and the comment explains why there are two guards. I ran the query against mainnet: getSummary reverts with "clientAddresses required" for an agent with no clients (9862), which the first guard now skips, and agent 9699 comes back as 1 review with score 100. All my earlier comments are addressed, and the registration file schema now matches what #2344 tells readers (services and x402Support). Approving.

@GigaHierz

Copy link
Copy Markdown
Contributor Author

Latest round addressed in ef17996, with origin/main merged in (no conflicts).

Of the points in the last review, only the blocking one was still open in the current file, and it is fixed: the "Query Agent Reputation" snippet returns early with "No starred feedback yet" when clients is empty (skipping getSummary, which reverts with clientAddresses required on an empty array), then checks count === 0n after the call for tag-filtered empties. The comment explains both guards.

The other points were already in earlier commits and are still in the file: the unsetAgentWallet guidance, "Transferring the NFT clears the wallet; the new owner sets it again", setAgentURI for IPFS and data: URIs, the note to pass reviewer addresses you trust, the // example agentId markers, zeroHash for the empty feedbackURI, and the corrected "any address except the owner and operators" wording.

Checked read-only, no transactions and no keys:

  • The verified Identity Registry implementation (0x7274e874…) writes agentWallet = msg.sender in register(), has unsetAgentWallet, and clears the wallet in _update on transfer.
  • The verified Reputation Registry implementation (0x16e0fa7f…) has require(!isAuthorizedOrOwner(msg.sender, agentId), "Self-feedback not allowed"), and getSummary with an empty array reverts with clientAddresses required (reproduced with cast call for agent 9862 on mainnet).
  • The snippet, extracted verbatim from the page with only as const stripped, on Celo mainnet: agent 9862 prints "No starred feedback yet"; agent 444 prints "No starred feedback yet" (its one entry is tagged agentguard, not starred); agent 9699 prints "1 reviews, score 100".
  • mint broken-links --check-redirects and the orphan check pass.

@GigaHierz
GigaHierz requested a review from palango October 2, 2026 12:35
@GigaHierz
GigaHierz merged commit 50ed71d into main Oct 2, 2026
5 checks passed
@GigaHierz
GigaHierz deleted the GigaHierz/erc8004-register-services-section branch October 2, 2026 12:38
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.

2 participants