feat(evm): add ERC-8004 agent identity approvals#415
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9e7147f95
ℹ️ 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".
| const provider = new JsonRpcProvider(rpcUrl); | ||
| const identityContract = new Contract(parsedRegistry.address, IDENTITY_ABI, provider); | ||
|
|
||
| const reputationRegistry = DEFAULT_REPUTATION_REGISTRIES[parsedRegistry.chainId]; | ||
| const reputationContract = reputationRegistry ? new Contract(reputationRegistry, REPUTATION_ABI, provider) : null; |
There was a problem hiding this comment.
Validate registry chain before resolving identity
The resolver parses the agentRegistry CAIP chain ID but then always reads contracts through rpcUrl, so if the request is on a different network than the declared registry chain, the owner/metadata/reputation are fetched from the wrong chain and displayed as if they came from the declared registry. In practice, a dapp can provide agentRegistry for eip155:43114 while connected to another EVM network and get misleading identity/trust data from a contract at the same address on that other network.
Useful? React with 👍 / 👎.
| const cached = cache.get(`${rpcUrl}:${declaration.agentRegistry}:${declaration.agentId}`); | ||
| if (cached && cached.expiresAt > Date.now()) { | ||
| return cached.value; | ||
| } |
There was a problem hiding this comment.
Evict expired entries from agent identity cache
The cache checks TTL on reads but never removes expired keys, so entries for one-off (rpcUrl, agentRegistry, agentId) combinations remain in memory indefinitely unless the same key is requested again. In a long-lived extension process that sees many unique agents, this creates unbounded cache growth and avoidable memory pressure.
Useful? React with 👍 / 👎.
|
@meeh0w just making sure this PR's review is properly prioritized :) |
|
@iJaack I have some questions :)
|
|
@meeh0w thank you for the feedback, I addressed all 3 points.
Net result:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61fee298ec
ℹ️ 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".
| const reputationRegistry = DEFAULT_REPUTATION_REGISTRIES[parsedRegistry.chainId]; | ||
| const reputationContract = reputationRegistry ? new Contract(reputationRegistry, REPUTATION_ABI, provider) : null; |
There was a problem hiding this comment.
Verify reputation registry matches identity registry
When the declared agentRegistry is on Avalanche but is not the identity registry associated with this hard-coded reputation contract, this still reads a score for the same numeric agentId from the default reputation registry and shows it as this agent’s trust level. Since reputation registries are scoped to an identity registry, a dapp can declare another registry at eip155:43114:<otherRegistry> and get an unrelated agent’s reputation attached to the owner/metadata fetched from <otherRegistry>; only use the default reputation contract after verifying its identity registry matches parsedRegistry.address (or key the mapping by identity registry address).
Useful? React with 👍 / 👎.
|
Hi @iJaack, thank you for your patience -- I was OOO for the past 2 weeks. Taking a look at this right now -- can you provide some example agent IDs and registry I could use for testing locally? |
|
@meeh0w yep, here are concrete values you can use locally. Main example I’ve been testing against on Avalanche mainnet:
That same pair is what I used in the focused tests in this PR. If useful, the equivalent raw identity-registry address is:
And for reference, the official Avalanche mainnet reputation registry is:
Example payloads:
{
"agentId": "1599",
"agentRegistry": "eip155:43114:0x8004A169FB4a3325136EB29fA0ceB6D2e539a432"
}request context on signing / tx calls {
"context": {
"agentIdentity": {
"agentId": "1599",
"agentRegistry": "eip155:43114:0x8004A169FB4a3325136EB29fA0ceB6D2e539a432"
}
}
}If you want, I can also add a second Fuji test vector in the tests / PR description so there’s an explicit testnet example too. |
meeh0w
left a comment
There was a problem hiding this comment.
Managed to finally test it, working fine! I say let's merge it in. There are some visual tweaks that can be done (e.g. data uri not rendering), but I don't want to hold you any longer.
The only thing you'll need to do is rebase and run pnpm changeset to generate a changelog for this.
Happy to re-approve afterwards 👍
| } | ||
|
|
||
| if (agentIdentity.metadataUri) { | ||
| items.push(linkItem('Metadata', { url: agentIdentity.metadataUri, name: agentIdentity.metadataUri })); |
There was a problem hiding this comment.
This is a data uri, so linkItem doesn't really work here.
There was a problem hiding this comment.
Good catch — I made metadata rendering scheme-aware. data: URIs now render as non-clickable text (Embedded data URI) instead of going through linkItem, while navigable schemes like http(s) and ipfs still render as links. I also added a focused unit test for both paths.
|
@meeh0w hey, thanks! Also, do you want me to solve the conflicts so that the PR is safe to merge? |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccccb42497
ℹ️ 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 getScore(uint256 agentId) view returns (int256)', | ||
| 'function getReputation(uint256 agentId) view returns (int256)', | ||
| 'function reputationScores(uint256 agentId) view returns (int256)', | ||
| 'function scores(uint256 agentId) view returns (int256)', | ||
| 'function reputations(uint256 agentId) view returns (int256)', |
There was a problem hiding this comment.
Use the ERC-8004 reputation read API
The configured 0x8004BAa... contract is the standard ERC-8004 Reputation Registry, whose read interface is getSummary(...)/readFeedback(...) rather than these score-style selectors (see https://eips.ethereum.org/EIPS/eip-8004#reputation-registry). Against the Avalanche registry, every call in this list will revert, so reputationScore stays null and approvals show unknown trust even for agents with feedback; wire this to the standard summary/read functions or to a documented scoring contract ABI.
Useful? React with 👍 / 👎.
|
@iJaack Yes, please :) |
|
I posted a PR to make it work in extension (will need VM Modules updated to stable versions after this is merged & released). |
|
@meeh0w OK this should be fine to merge now! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 445d1e562c
ℹ️ 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".
|
|
||
| const fallback = buildFallback(normalizedDeclaration); | ||
|
|
||
| const provider = new JsonRpcProvider(rpcUrl); |
There was a problem hiding this comment.
Pass custom RPC headers to identity calls
On networks configured with customRpcHeaders, the transaction/signing paths use getProvider(..., customRpcHeaders) for their normal RPC calls, but this resolver creates a separate bare JsonRpcProvider from only the URL. For RPC endpoints that require header-based auth, every identity eth_call here is sent unauthenticated and falls back to an unknown identity even though the same network works for the rest of the request; pass the network headers through or reuse the already configured provider.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@iJaack Can you re-use getProvider() helper here?
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39531ca15a
ℹ️ 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".
|
|
||
| const clientAddresses = dedupeAddresses(getFeedbackClients(feedbackResult)); | ||
| if (clientAddresses.length) { | ||
| const summary = getSummaryValue(await reputationContract.getSummary(agentId, clientAddresses, STARRED_TAG, '')); |
There was a problem hiding this comment.
Handle reputation summary failures without aborting approvals
When getSummary reverts for a particular agent/client set, this unhandled await rejects resolveReputationScore, which then rejects the surrounding Promise.all in resolveAgentIdentity; the eth_sign/eth_sendTransaction callers do not catch that, so an otherwise valid request with agent identity context fails before showing the approval instead of falling back to raw feedback or an unknown reputation. Catch summary/reputation read failures and continue to the average/null fallback so reputation outages do not block signing or transactions.
Useful? React with 👍 / 👎.
|
@meeh0w all review suggestion fixes have been applied, please do one final check, it should be ready to merge now! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a73c66413
ℹ️ 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".
| return null; | ||
| } | ||
|
|
||
| const feedbackResult = await reputationContract.readAllFeedback(agentId, [], STARRED_TAG, '', false); |
There was a problem hiding this comment.
Bound reputation reads before approval
When the declared agent has many starred feedback entries, this approval path calls readAllFeedback with an empty client filter, which materializes every matching client/index/value before the optional summary lookup. A popular or spammed agent can therefore make eth_sign/transaction approvals wait on a very large eth_call or hit provider response/gas limits and fall back to unknown reputation; use a bounded/trusted client set, an indexer, or a timeout/best-effort path before prompting.
Useful? React with 👍 / 👎.
Summary
avalanche_declareAgentIdentityto the shared RPC surfaceTesting
pnpm --filter @avalabs/vm-module-types buildpnpm --filter @avalabs/evm-module buildpnpm --filter @avalabs/evm-module exec jest --runInBand src/handlers/avalanche-declare-agent-identity/avalanche-declare-agent-identity.test.ts src/handlers/eth-sign/eth-sign.test.ts src/handlers/eth-send-transaction/eth-send-transaction.test.ts src/handlers/eth-send-transaction-batch/eth-send-transaction-batch.test.tsFollow-up to ava-labs/core-extension#809.