[#32] Add agent registration wizard#125
Conversation
3-step wizard at /register-agent: agent profile form with metadata generation, on-chain register(agentURI) call, and EIP-712 signed setAgentWallet binding. Adds register + setAgentWallet ABI entries to erc8004.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The Step 1/2 wizard structure looks reasonable, but Step 3 is currently wired to sign the EIP-712 payload with the connected wallet from wagmi, not with the agentWallet being bound. That makes the core setAgentWallet flow fail for the intended owner-wallet -> agent-wallet registration path described in issue #32.
Findings
- [high] Step 3 signs with the wrong account, so
setAgentWalletcannot complete the intended wallet-binding flow- File:
src/app/register-agent/page.tsx:65 - Suggestion:
useSignTypedData()signs with the currently connected wallet, while the typed message saysnewWallet: agentWallet. Unless the connected account is literally switched toagentWallet, the signature will not be fromnewWalletand the contract should reject it. But if the user does switch toagentWallet, the subsequentwriteContractAsync(setAgentWallet)tx is no longer sent by the owner wallet that just registered the agent. Rework Step 3 so the signature is produced by the target agent wallet and the tx submission remains owned by the correct caller, or explicitly redesign the UX around the required wallet switching.
- File:
Decision
Request changes because the current implementation does not satisfy issue #32's required setAgentWallet flow in a real two-wallet setup.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review: REQUEST CHANGES
This is a solid start but has security and robustness issues to address:
Critical
- Missing nonce in EIP-712
SetAgentWallettype — The typed data includesagentId,newWallet,deadlinebut nononce. If the ERC-8004 contract expects a nonce field, the signature will be invalid. If it doesn't, the signed message is replayable by anyone before deadline expires. Either add the nonce field to match the contract, or document why replay is safe (e.g., contract invalidates after first use). This must be verified against the actual contract.
High
- No address validation in
handleSetWallet()— Validation only exists on the button'sdisabledprop. If bypassed (keyboard submit, future refactor), an invalid address flows into the signature. Add a guard at the top of the handler, ideally using viem'sisAddress(). - Stale
registeredAtinagentURI—new Date().toISOString()inuseMemocaptures memo evaluation time, not submission time. If user waits before submitting, timestamp is stale. Move it intohandleRegister()or drop it in favor of on-chainblock.timestamp.
Medium
- Wallet disconnect mid-flow loses
agentId— If wallet disconnects after Step 2, component remounts andagentIdstate is lost. User can't resume Step 3 without checking the chain. Consider persistingagentIdto URL params or localStorage. - Empty
agentURIcrash — If user goes Back from Step 2, clears name, theagentURImemo returns"". The metadata previewJSON.parse("")throws. AddprofileValidcheck to Step 2's register button disabled condition.
Low
- No
maxLengthon form inputs — very long names/descriptions increase gas costs. - Hardcoded 1-hour signature deadline — consider making it a named constant or extending to 24h.
Split Step 3 into three sub-steps to implement correct ERC-8004 flow: - 3a: Record owner address, enter agent wallet address - 3b: Switch to agent wallet, sign EIP-712 typed data (proving consent) - 3c: Switch back to owner wallet, submit setAgentWallet transaction Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The Step 3 wallet-switching flow is directionally better, but the implementation still does not match the deployed ERC-8004 registry interface. I verified this against the contract definition: the current ABI and EIP-712 payload shape are still wrong, so setAgentWallet will not validate the signature correctly.
Findings
- [high]
setAgentWalletABI parameter order is incorrect- File:
lib/contracts/erc8004.ts:33 - Suggestion: The PR defines
setAgentWallet(agentId, newWallet, signature, deadline), and Step 3 submits args in that order fromsrc/app/register-agent/page.tsx:227. The deployed registry interface issetAgentWallet(agentId, wallet, deadline, sig). Update the ABI and call site to match the real contract.
- File:
- [high] The EIP-712 domain and typed message do not match the registry contract
- File:
src/app/register-agent/page.tsx:42 - Suggestion: The wizard signs domain name
ERC8004AgentRegistryand message{ agentId, newWallet, deadline }atsrc/app/register-agent/page.tsx:192, but the registry contract expects a different EIP-712 payload forsetAgentWallet(including the owner and nonce, with the registry's actual domain name). As written, the agent wallet signs a payload the contract will not recover/accept. Rebuild the typed data from the real contract spec before re-review.
- File:
Decision
Request changes because the current Step 3 implementation still cannot complete a valid setAgentWallet flow against the deployed ERC-8004 registry.
…oyed contract Verified against on-chain source (Sourcify) for the implementation at 0x7274e874ca62410a93bd8bf61c69d8045e399c02: - ABI: setAgentWallet param order is (agentId, newWallet, deadline, signature), not (agentId, newWallet, signature, deadline) - EIP-712 domain name: "ERC8004IdentityRegistry", not "ERC8004AgentRegistry" - EIP-712 struct: "AgentWalletSet" with fields (agentId, newWallet, owner, deadline), not "SetAgentWallet" with (agentId, newWallet, deadline) - Event name: "Registered", not "AgentRegistered" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up fixes align the wizard with the deployed ERC-8004 registry contract: the Registered event, setAgentWallet ABI order, and EIP-712 domain/message now match the contract shape. The Step 3 wallet-switch flow is coherent for the two-wallet registration path, and CI passed.
Findings
- None.
Decision
Approve because the contract-facing correctness issues are resolved, the PR satisfies issue #32 end-to-end, and lint-and-typecheck passed.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Re-review: APPROVE
Critical and high issues from my first review are resolved or downgraded:
- EIP-712 nonce: Struct verified against deployed contract (Sourcify) — no nonce field exists on-chain. Not a frontend bug.
- ABI/domain/event corrections: All look correct per on-chain source.
- Wallet-switch flow:
ownerAddressnow persisted in state, Step 3 split correctly handles agent-sign then owner-submit.
Remaining non-blocking items (follow-up):
- Add a comment explaining why no nonce is needed in the EIP-712 type
- Add handler-level address validation in
handleAgentSign()(defense-in-depth) - Move
registeredAtfrom useMemo to submission time inhandleRegister()
Fixes #32
Summary
/register-agentwizard for ERC-8004 agent registrationregister(agentURI)call, parsesAgentRegisteredevent to extract agentIdsetAgentWallet(agentId, newWallet, signature, deadline)call (with skip option)register,setAgentWallet, andAgentRegisteredABI entries tolib/contracts/erc8004.ts/createon completionTest plan
/register-agentregister()tx submits and agentId is displayedsetAgentWallet()tx/create🤖 Generated with Claude Code