fix(runner): harden ERC8004 identity ensure roster sync#554
Merged
Conversation
Fix 1: Console roster token field normalization - Accept both camelCase (tokenId) and snake_case (token_id) from Console - Skip undefined token IDs in merge logic Fix 2: Replace totalSupply/ownerOf scan with Transfer event log scanning - Use eth_getLogs with Transfer event topic instead of totalSupply() - Chunk getLogs by <= 10,000 block ranges - Configurable start block via ARCLAYER_ERC8004_SCAN_FROM_BLOCK - Dedupe tokenIds and verify current ownership with ownerOf Fix 3: Fail closed when on-chain scan fails - Block auto-register when on-chain scan throws to avoid duplicate mint - Return clear failure message Fix 4: Retry Console sync after pending tx confirmation - Call syncToConsoleFn after finalizeResult.confirmed with tokenId - Handle retryable and permanent sync failures Fix 5: Honor --confirm-second-mint - When confirmSecondMint=true AND autoRegister=true, skip single-identity reuse and proceed to second mint path Fix 6: Honor explicit ARCLAYER_AGENT_ID when multiple identities - Add selectedAgentId parameter to ensureIdentity - Select matching tokenId when found in on-chain set - Pass config.agentId from index.ts Fix 7: Respect configured Console URL in syncToConsole - Use this.config.consoleUrl ?? process.env.ARCLAYER_CONSOLE_URL (same pattern as registerErc8004WithApproval) Tests: 16 new test cases covering all fixes
✅ Deploy Preview for rainbow-macaron-5d0d9a ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- getLogs chunk failure now throws instead of silent continue - getBlockNumber failure in real RPC mode now throws (bounded fallback only in test override mode) - balanceOf > 0 but no currently-owned tokens after scan throws incomplete scan error - Added tests: getLogs chunk error blocks auto-register, getBlockNumber failure blocks scan, incomplete scan blocks auto-register, provider tokenId 782224 read-first path safe
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b118774d2f
ℹ️ 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".
…, P2 durable sync pending, P3 selectedAgentId single identity P1: scanExistingIdentityOnChain now uses Transfer(*, wallet, tokenId) wildcard instead of Transfer(0x0, wallet, tokenId) so identities minted to another wallet and later transferred in are discovered. P2: When Console sync returns retryable after pending tx finalizes, a durable consoleSync.status=pending marker is written to identity.json. On next run, Step 1 retries the sync before exiting via already_confirmed. Success clears the marker. P3: Single discovered identity now checks selectedAgentId match. If ARCLAYER_AGENT_ID is set but does not match the wallet-owned tokenId, the run fails with a clear mismatch message instead of silently confirming. Tests: 12 new test cases covering all three fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Harden ERC-8004 identity ensure flow with 7 fixes and 16 new test cases.
Fixes
Fix 1: Console roster token field normalization
ConsoleRosterEntrytype now accepts bothtoken_id(snake_case) andtokenId(camelCase)Fix 2: Replace totalSupply/ownerOf scan with Transfer event log scanning
scanExistingIdentityOnChain()now useseth_getLogswith Transfer event signaturefrom=0x0andto=walletAddressARCLAYER_ERC8004_SCAN_FROM_BLOCKenv var (fallback: block 40,000,000)ownerOfFix 3: Fail closed when on-chain scan fails
autoRegister=trueand on-chain scan throws, blocks mint to avoid duplicatesFix 4: Retry Console sync after pending tx confirmation
Fix 5: Honor --confirm-second-mint
confirmSecondMint=trueANDautoRegister=true, skips single-identity reuseFix 6: Honor explicit ARCLAYER_AGENT_ID when multiple identities
selectedAgentIdparameter onensureIdentityselectedAgentIdmatchesconfig.agentIdfromindex.tsCLI commandFix 7: Respect configured Console URL in syncToConsole
syncToConsolenow usesthis.config.consoleUrl ?? process.env.ARCLAYER_CONSOLE_URLregisterErc8004WithApprovalTest Coverage
16 new tests in
identity-ensure.test.ts:tokenId— acceptedtoken_id— acceptedTest Results
@arclayer/runnertests: 725 passed (18 test files)@arclayer/runnerbuild: success@arclayer/langchain-adaptertests: 105 passed (4 test files)