Skip to content

Prepare v0.1.0 for publish#5

Merged
vraspar merged 4 commits into
mainfrom
vraspar/pre-publish-v0.1.0
Apr 6, 2026
Merged

Prepare v0.1.0 for publish#5
vraspar merged 4 commits into
mainfrom
vraspar/pre-publish-v0.1.0

Conversation

@vraspar
Copy link
Copy Markdown
Contributor

@vraspar vraspar commented Apr 6, 2026

Summary

  • Add missing spec functions: setAgentWallet, unsetAgentWallet
  • Make agentURI optional in registerAgent (zero-arg register overload)
  • Make responseHash optional in appendResponse (default zeroHash)
  • Add receipt parsing helpers: parseRegisterReceipt, parseGiveFeedbackReceipt
  • Add readAllFeedbackBatched — chunks clientAddresses to avoid RPC limits
  • Add README.md and CHANGELOG.md
  • Add repository/homepage/bugs to package.json
  • Lower Node engine from >=22 to >=18
  • Bump version to 0.1.0

Test plan

  • 49 unit tests passing
  • biome check, tsc, publint --strict, attw, knip, size-limit all green
  • npm pack --dry-run verified (56 files, 34.6 kB)
  • Fork tests (CI)
  • pnpm publish --access public after merge

🤖 Generated with Claude Code

vraspar and others added 2 commits April 5, 2026 19:24
- Add pollingInterval: 100 to fork public client (Anvil mines instantly,
  default 4s polling wastes time per waitForTransactionReceipt)
- Warn when FORK_URL not set and falling back to public RPC

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Spec gaps:
- Add setAgentWallet + unsetAgentWallet
- Make agentURI optional in registerAgent (zero-arg overload)
- Make responseHash optional in appendResponse (default zeroHash)

New helpers:
- parseRegisterReceipt — extract agentId from tx receipt
- parseGiveFeedbackReceipt — extract feedbackIndex from tx receipt
- readAllFeedbackBatched — chunk clientAddresses to avoid RPC limits

Docs & metadata:
- Add README.md and CHANGELOG.md
- Add repository/homepage/bugs to package.json
- Lower Node engine from >=22 to >=18
- Bump version to 0.1.0

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread README.md Outdated

## Stability

ERC-8004 is Draft status. The on-chain contracts are UUPS upgradeable. At 0.x, minor version bumps may contain breaking changes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why do we need to mention it ? Isn't this common knowledge

Comment thread README.md Outdated
## Install

```bash
pnpm add @x402r/erc8004 viem
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If view is dep ? do we need to list view ? why not pnpm install or something

Comment thread README.md Outdated
@@ -0,0 +1,124 @@
# @x402r/erc8004

TypeScript SDK for [ERC-8004](https://eips.ethereum.org/EIPS/eip-8004) Identity and Reputation registries. Pure [viem](https://viem.sh), zero additional dependencies.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems marking jargon. keep it technical

Comment thread package.json
@@ -1,12 +1,20 @@
{
"name": "@x402r/erc8004",
"version": "0.0.0",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

maybe release alpha version first for testing with SDK, than can release actual stable one ?

@vraspar
Copy link
Copy Markdown
Contributor Author

vraspar commented Apr 6, 2026

SDK Review — Full Audit (v0.1.0 Pre-publish)

Comprehensive audit against ERC-8004 spec, deployed contract ABIs, agent0-sdk, and chain deployments.


Bugs

1. registerAgent empty string fallthrough

register.ts uses if (agentURI) to decide which contract overload to call. Empty string "" is falsy in JS, so registerAgent(client, { agentURI: '' }) silently calls the zero-arg register() instead of register(""). Worse: agentURI: '' with metadata silently drops the metadata entirely.

// Current (buggy)
if (agentURI && metadata && metadata.length > 0) { ... }
if (agentURI) { ... }

// Fix
if (agentURI !== undefined && metadata && metadata.length > 0) { ... }
if (agentURI !== undefined) { ... }

Missing SDK Functions

2. No EIP-712 signing helper for setAgentWallet (medium)

The contract requires an EIP-712 signature from newWallet proving consent. The SDK takes signature: Hex but provides no helper to construct or sign the typed data:

AgentWalletSet(uint256 agentId, address newWallet, address owner, uint256 deadline)

Callers must manually build this with viem's signTypedData + the registry's eip712Domain(). The agent0-sdk handles this internally. A signAgentWalletConsent(walletClient, params) helper would close the gap.

3. Missing getVersion() for both registries (low)

Both ABIs have getVersion() → string (pure). No SDK wrapper. Useful for detecting contract upgrades on UUPS proxies.

4. Missing getIdentityRegistry() for reputation registry (low)

reputationRegistryAbi has getIdentityRegistry() → address. Useful for verifying which identity registry the reputation registry is linked to.


Event/Receipt Parser Gaps

5. parseGiveFeedbackReceipt only extracts 3 of 11 event fields (low)

Returns { agentId, clientAddress, feedbackIndex } but drops value, valueDecimals, tag1, tag2, endpoint, feedbackURI, feedbackHash. Callers who need feedbackHash (the content-addressability anchor) must re-parse logs manually.

6. Missing receipt parsers for 4 events (low)

Write Function Event Emitted Receipt Parser
registerAgent Registered parseRegisterReceipt
giveFeedback NewFeedback parseGiveFeedbackReceipt ⚠️ (partial)
setAgentURI URIUpdated ❌ missing
setMetadata MetadataSet ❌ missing
revokeFeedback FeedbackRevoked ❌ missing
appendResponse ResponseAppended ❌ missing

Inconsistent with the pattern established by parseRegisterReceipt.


Documentation/DX Gaps

7. resolveAgent JSDoc should clarify agentWallet can be address(0) (low)

After NFT transfer or unsetAgentWallet(), agentWallet is address(0). The ownerMismatch flag catches this, but the JSDoc says "the agent NFT was transferred but the wallet wasn't updated" — it also fires after explicit unsetAgentWallet() with no transfer. The ResolvedAgent type should warn that callers should use owner when ownerMismatch is true.

8. giveFeedback.value is int128 (signed) — type doesn't communicate this (low)

ABI uses int128 (signed integer — negative feedback is valid). SDK types it as bigint which is correct for viem, but nothing in the type or JSDoc indicates negative values are meaningful. A JSDoc @remarks Negative values represent negative feedback would help.


Chain Coverage

9. Addresses are verified correct — all 4 addresses match both erc-8004-contracts README and agent0-ts DEFAULT_REGISTRIES.

10. 7 chains covered, 20+ deployed chains missing. CREATE2 addresses are identical across chains, so adding chains is just new map entries. Notable missing mainnets for v0.1.0:

Chain ID
Avalanche 43114
BSC 56
Scroll 534352
Linea 59144
Mantle 5000
Gnosis 100
Celo 42220

The design plan says "only list chains we've verified deployment on" — this is intentional, but worth expanding for v0.1.0 if deployments are confirmed.


ABI Coverage Summary

Identity Registry (30 ABI entries)

Function SDK Status
register() registerAgent
register(string) registerAgent ⚠️ empty string bug
register(string, MetadataEntry[]) registerAgent ⚠️ empty string bug
setAgentURI setAgentURI
setAgentWallet setAgentWallet ✅ (no signing helper)
unsetAgentWallet unsetAgentWallet
getAgentWallet getAgentWallet
getMetadata getMetadata
setMetadata setMetadata
balanceOf isRegistered (>0n)
ownerOf Used by verifyAgentId, resolveAgent
tokenURI Used by resolveAgent
getVersion
ERC-721/UUPS/Ownable (15 functions) 🔧 No wrapper needed

Reputation Registry (18 ABI entries)

Function SDK Status
giveFeedback giveFeedback
revokeFeedback revokeFeedback
appendResponse appendResponse
getSummary getSummary
readFeedback readFeedback
readAllFeedback readAllFeedback + readAllFeedbackBatched
getClients getClients
getLastIndex getLastIndex
getResponseCount getResponseCount
getIdentityRegistry
getVersion
UUPS/Ownable (4 functions) 🔧 No wrapper needed

agent0-sdk Comparison

agent0-sdk Feature @x402r/erc8004
agent.setWallet() (internal EIP-712 signing) ❌ Gap — SDK takes raw signature
IPFS feedback file prep Intentionally out of scope (requires helia)
Subgraph queries Intentionally out of scope (requires graphql-request)
Direct contract reads (readFeedback, getClients, etc.) ✅ Broader coverage than agent0-sdk

Spec Security Considerations

Spec Requirement SDK Behavior
Filter by reviewer address (anti-Sybil) getSummary/readAllFeedback require explicit clientAddresses[]
ownerOf verification before trusting agentId verifyAgentId() — JSDoc marks it mandatory
Immutable on-chain audit trail ✅ SDK returns raw on-chain data
feedbackHash mandatory for non-IPFS URIs ⚠️ SDK defaults to zeroHash — no enforcement (let contract decide)
Submitter cannot be agent owner No SDK enforcement (let contract revert)
valueDecimals must be 0-18 No SDK enforcement (let contract revert)

Summary: One bug (registerAgent empty string), one medium gap (EIP-712 helper), and several low-priority consistency items. Core contract coverage is complete for both registries.


Generated with Claude Code using review-sdk skill

Bug fix:
- registerAgent: if(agentURI) → if(agentURI !== undefined) to handle empty string

New functions:
- signAgentWalletConsent — EIP-712 signing helper for setAgentWallet
- getVersion (identity + reputation) — read contract version string
- getIdentityRegistry — get linked Identity Registry from Reputation Registry
- parseMetadataSetReceipt, parseURIUpdatedReceipt (identity)
- parseFeedbackRevokedReceipt, parseResponseAppendedReceipt (reputation)

Improvements:
- parseGiveFeedbackReceipt now returns all 11 event fields
- 7 new chains added (Avalanche, BSC, Scroll, Linea, Mantle, Gnosis, Celo)
- JSDoc: resolveAgent agentWallet=address(0), giveFeedback int128 signed value
- README: technical description, npm install, updated API tables, 14 chains
- Version → 0.1.0-alpha.0 for SDK integration testing

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@A1igator
Copy link
Copy Markdown

A1igator commented Apr 6, 2026

SDK Review

Found 10 issues (reviewed: tests, conventions, dead code, SDK design):

1. [Dead Code — 85] metadata silently dropped when agentURI is omitted in registerAgent

The PR makes agentURI optional but restructures the dispatch logic so that if a caller passes { metadata: [{key: 'k', value: '0x...'}] } without agentURI, all three branches skip the metadata — it falls to register() with no args. The type signature now allows this call shape, making metadata a dead parameter path.

parameters.registryAddress,
)
if (agentURI !== undefined && metadata && metadata.length > 0) {
return walletClient.writeContract({
address: registry,
abi: identityRegistryAbi,
functionName: 'register',
args: [
agentURI,
metadata.map((m) => ({
metadataKey: m.key,
metadataValue: m.value,
})),
],
chain: walletClient.chain,
account,
})
}
if (agentURI !== undefined) {
return walletClient.writeContract({
address: registry,
abi: identityRegistryAbi,
functionName: 'register',
args: [agentURI],
chain: walletClient.chain,
account,


2. [SDK Design — 85] signAgentWalletConsent takes two positional client args — breaks the single-client pattern

Every other function in this SDK is (client, parameters). This function takes (walletClient, publicClient, parameters) — a novel signature shape. The publicClient is only needed for a single ownerOf read. viem and wagmi never expose two-client function signatures. Fix: accept owner in parameters (caller fetches it), or split into buildAgentWalletConsentTypedData + sign.

/**
* Sign EIP-712 typed data proving consent from `newWallet` for `setAgentWallet`.
*
* The walletClient must be the `newWallet` signer. The returned signature
* is passed to `setAgentWallet` along with the same `agentId` and `deadline`.


3. [Tests — 85] signAgentWalletConsent test claims "correct EIP-712 structure" but omits chainId and verifyingContract

The test uses expect.objectContaining for domain and only asserts name and version. chainId (replay protection) and verifyingContract (registry binding) — the two security-critical EIP-712 fields — are not checked. The mock has enough info to assert both.

}),
).rejects.toThrow('network timeout')
})
})
// --- resolveAgent ---
describe('resolveAgent', () => {
it('merges ownerOf + getAgentWallet + tokenURI', async () => {
const client = mockPublic({
ownerOf: ADDR_A,
getAgentWallet: ADDR_A,
tokenURI: 'https://example.com/agent.json',
})
const agent = await resolveAgent(client, {
registryAddress: REGISTRY,
agentId: 42n,
})
expect(agent).toEqual({


4. [Conventions — 80] Hardcoded EIP-712 domain name/version with no fork test coverage

'ERC8004IdentityRegistry' and version: '1' are hardcoded without linking to the contract's eip712Domain() output. If these don't match the deployed contract, every signature is silently rejected. None of the three new write-path functions (signAgentWalletConsent, setAgentWallet, unsetAgentWallet) have fork tests — the PR body acknowledges this gap.

})
const chainId = publicClient.chain?.id
if (!chainId) {
throw new Error('publicClient chain not configured')


5. [SDK Design — 80] readAllFeedbackBatched exposes an RPC transport concern as a top-level SDK function

Chunking clientAddresses into batches of 50 is an infrastructure workaround for RPC limits, not a protocol operation. It creates a decision point (readAllFeedback vs readAllFeedbackBatched) that SDK consumers shouldn't have to make. Merge the batching into readAllFeedback as an optional batchSize parameter.

import type { PublicClient } from 'viem'
import { readAllFeedback } from './readAllFeedback.js'
import type {
FeedbackEntry,
ReadAllFeedbackBatchedParameters,
} from './types.js'
/**
* Read all feedback for an agent, batching `clientAddresses` into chunks
* to avoid RPC response size limits.
*
* Defaults to batches of 50 addresses. If `clientAddresses` fits within
* a single batch, delegates directly to `readAllFeedback`.
*/
export async function readAllFeedbackBatched(
publicClient: PublicClient,
parameters: ReadAllFeedbackBatchedParameters,
): Promise<readonly FeedbackEntry[]> {
const { batchSize = 50, ...rest } = parameters
const { clientAddresses } = rest
if (clientAddresses.length <= batchSize) {
return readAllFeedback(publicClient, rest)
}
const results: FeedbackEntry[] = []
for (let i = 0; i < clientAddresses.length; i += batchSize) {
const chunk = clientAddresses.slice(i, i + batchSize)
const batch = await readAllFeedback(publicClient, {
...rest,
clientAddresses: chunk,
})
results.push(...batch)
}
return results
}


6. [Dead Code — 80] GetVersionParameters not exported from root src/index.ts

Both getIdentityVersion and getReputationVersion are exported from root, but their parameter type GetVersionParameters is not. Every other root-exported function has its parameter type also root-exported. Consumers must reach into sub-path imports just for the type.

erc8004/src/index.ts

Lines 9 to 24 in cadfecd

GetAgentWalletParameters,
GetMetadataParameters,
IsRegisteredParameters,
MetadataSetResult,
RegisterAgentParameters,
RegisterResult,
ResolveAgentParameters,
ResolvedAgent,
SetAgentURIParameters,
SetAgentWalletParameters,
SetMetadataParameters,
SignAgentWalletConsentParameters,
UnsetAgentWalletParameters,
URIUpdatedResult,
VerifyAgentIdParameters,
} from './identity/index.js'


7. [Conventions — 75] walletClient.account! bypasses the established requireAccount helper

Every other write function calls requireAccount(walletClient) for a typed account with a clear error message. signAgentWalletConsent uses walletClient.account! directly — if account is undefined, viem throws a confusing internal error instead of the SDK's own message.

args: [parameters.agentId],


8. [Conventions — 75] feedbackHash typed as `0x${string}` instead of Hex

GiveFeedbackResult.feedbackHash uses the raw template literal while ResponseAppendedResult.responseHash (same PR, same pattern) uses Hex. They're structurally identical, but the inconsistency within the same PR is a naming convention violation.


9. [Conventions — 75] Unnecessary as Promise<T> casts on readContract returns

Three new functions (identity/getVersion.ts, reputation/getVersion.ts, reputation/getIdentityRegistry.ts) add as Promise<string> or as Promise<Address> casts. Existing functions like getAgentWallet return readContract(...) directly — viem infers return types from the ABI. The casts suppress type inference and diverge from the established pattern.

return publicClient.readContract({
address: registry,
abi: identityRegistryAbi,
functionName: 'getVersion',
}) as Promise<string>
}


10. [Tests — 75] parseGiveFeedbackReceipt test only verifies 3 of 10 returned fields

The test encodes a full NewFeedback event with all fields but only asserts agentId, clientAddress, and feedbackIndex. The function's main value is correctly mapping all 10 event fields — a typo swapping tag1/tag2 would pass. Use toEqual with all expected fields.

const client = mockWallet()
await appendResponse(client, {
registryAddress: REPUTATION_REGISTRY,
agentId: 42n,
clientAddress: ADDR_B,
feedbackIndex: 1n,
responseURI: 'https://response.example.com',
responseHash: zeroHash,
})


Generated with Claude Code using review-sdk skill

Fixes:
- registerAgent: throw when metadata provided without agentURI (dead param path)
- signAgentWalletConsent: move publicClient into params (single-client pattern)
- signAgentWalletConsent: use requireAccount instead of account! assertion
- parseGiveFeedbackReceipt: use Hex type instead of template literal
- Remove as Promise<T> casts from getVersion/getIdentityRegistry
- Export GetVersionParameters from root barrel

Design:
- Merge readAllFeedbackBatched into readAllFeedback as optional batchSize param
- Delete readAllFeedbackBatched.ts (separate file was unnecessary API surface)

Tests:
- signAgentWalletConsent: assert chainId + verifyingContract (security fields)
- signAgentWalletConsent: add requireAccount test
- registerAgent: add metadata-without-URI test
- parseGiveFeedbackReceipt: assert all 10 fields with toEqual

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vraspar
Copy link
Copy Markdown
Contributor Author

vraspar commented Apr 6, 2026

Review round 2 — pushback on #4 (hardcoded EIP-712 domain)

Both reviewers flagged that signAgentWalletConsent hardcodes name: 'ERC8004IdentityRegistry' and version: '1' rather than reading from eip712Domain().

Keeping as-is for now. Rationale:

  • This is what agent0-sdk does — it's the established pattern in the ecosystem.
  • A UUPS domain change would break all existing signatures regardless of whether SDKs read it dynamically or not. The domain is effectively immutable.
  • Reading eip712Domain() adds an extra RPC call per signing. The current approach is zero-overhead.
  • The correct validation layer for this is a fork test (already planned for CI), which will catch any domain mismatch against the real deployed contract.

If the domain ever changes (extremely unlikely), we bump a patch version. The fork test will fail in CI before any release ships with the wrong domain.

@vraspar vraspar merged commit 6d51526 into main Apr 6, 2026
2 checks passed
@vraspar vraspar deleted the vraspar/pre-publish-v0.1.0 branch April 6, 2026 07:07
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