Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR implements multi-chain support for AI-generated image NFTs, enabling deployment and operation on both Optimism and Base networks. The changes migrate from a single-network architecture to a dynamic multi-chain system using CAIP-2 network identifiers.
Changes:
- Deployed GenImNFTv4 and CollectorNFTv1 contracts on Base Mainnet
- Modernized deployment scripts with balance checks and config validation
- Migrated backend from
sepoliaTestboolean tonetworkCAIP-2 parameter - Implemented multi-chain NFT fetching hooks in frontend
- Added ChainBadge component for visual network identification
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/chain-utils/src/addresses.ts | Added Base network addresses and helper functions for multi-chain support |
| scw_js/getChain.js | Implemented getExpectedNetworks() returning array instead of single network |
| scw_js/genimg_x402_token.js | Migrated from sepoliaTest to network parameter, added nonce retry logic |
| website/hooks/useMultiChainNFTs.ts | New hooks to fetch NFTs from all chains in parallel |
| website/components/ChainBadge.tsx | New component showing which network each NFT is on |
| eth/scripts/deploy-genimg-v4.ts | Added MIN_DEPLOYMENT_BALANCE check and modernized config structure |
| eth/deployments/*.json | New Base deployment artifacts |
| x402_facilitator/package-lock.json | Updated vitest dependencies |
Files not reviewed (1)
- x402_facilitator/package-lock.json: Language not supported
| { | ||
| "proxyAddress": "0x80f95d330417a4acEfEA415FE9eE28db7A0A1Cdb", | ||
| "proxyAddress": "0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0", | ||
| "options": { | ||
| "validateOnly": true, | ||
| "validateOnly": false, | ||
| "dryRun": false, | ||
| "verify": true, | ||
| "authorizeAgentWallet": "0xAAEBC1441323B8ad6Bdf6793A8428166b510239C", | ||
| "waitConfirmations": 2 | ||
| "verify": false, | ||
| "waitConfirmations": 1 | ||
| }, | ||
| "metadata": { | ||
| "description": "Upgrade GenImNFTv3 to v4 - CVE-2025-11-26 Security Fix", | ||
| "version": "4.0.0", | ||
| "environment": "production", | ||
| "securityFix": "CVE-2025-11-26", | ||
| "attackerAddress": "0x8B6B008A0073D34D04ff00210E7200Ab00003300" | ||
| "description": "Test upgrade configuration for GenImNFTv4", | ||
| "version": "4.0.0-test", | ||
| "environment": "development", | ||
| "securityFix": "CVE-2025-11-26" | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The upgrade configuration file contains a test proxy address (0xA51c1fc2f0D1a1b8494Ed1FE312d7C3a78Ed91C0) and test environment settings. This configuration should not be committed to the main branch as it could lead to accidental upgrades of test contracts instead of production contracts. Consider either:
- Adding this file to
.gitignoreand maintaining it locally only - Using a template file (e.g.,
upgrade-genimg-v4.config.template.json) with placeholder values - Adding clear warnings in comments that this is a test configuration
| } | ||
|
|
||
| if (isNonceError && attempt < MAX_TRANSFER_RETRIES) { | ||
| console.log(`⏳ Nonce conflict detected, waiting ${RETRY_DELAY_MS}ms before retry with fresh nonce...`); |
There was a problem hiding this comment.
The nonce error retry logic waits for 2 seconds before retrying, but the comment mentions "retry with fresh nonce" which might be misleading. Viem's walletClient.write methods automatically manage nonces, so the retry should work. However, if parallel requests are causing nonce conflicts, the issue might persist after the delay.
Consider adding explicit nonce management or using Viem's built-in nonce management strategies. Alternatively, clarify the comment to indicate that the retry relies on Viem's automatic nonce management rather than manually fetching a fresh nonce.
| console.log(`⏳ Nonce conflict detected, waiting ${RETRY_DELAY_MS}ms before retry with fresh nonce...`); | |
| console.log(`⏳ Nonce conflict detected, waiting ${RETRY_DELAY_MS}ms before retry (relying on viem's automatic nonce management)...`); |
| LLMv1ABI, | ||
| getGenAiNFTMainnetNetworks, | ||
| getGenAiNFTTestnetNetworks, | ||
| isTestnet, |
There was a problem hiding this comment.
Unused import isTestnet.
| isTestnet, |
| import { entryList } from "../layouts/styles"; | ||
| import { useAutoNetwork } from "../hooks/useAutoNetwork"; | ||
| import { getGenAiNFTAddress, GenImNFTv4ABI, GENAI_NFT_NETWORKS } from "@fretchen/chain-utils"; | ||
| import { getGenAiNFTAddress, GenImNFTv4ABI, GENAI_NFT_NETWORKS, isTestnet } from "@fretchen/chain-utils"; |
There was a problem hiding this comment.
Unused import getGenAiNFTAddress.
| import { useWriteContract, useWaitForTransactionReceipt, useSwitchChain } from "wagmi"; | ||
| import { useNFTListedStatus } from "../hooks/useNFTListedStatus"; | ||
| import { getGenAiNFTAddress, GenImNFTv4ABI, GENAI_NFT_NETWORKS, isMainnet } from "@fretchen/chain-utils"; | ||
| import { getGenAiNFTAddress, GenImNFTv4ABI, isMainnet, fromCAIP2 } from "@fretchen/chain-utils"; |
There was a problem hiding this comment.
Unused import isMainnet.
| import { getGenAiNFTAddress, GenImNFTv4ABI, isMainnet, fromCAIP2 } from "@fretchen/chain-utils"; | |
| import { getGenAiNFTAddress, GenImNFTv4ABI, fromCAIP2 } from "@fretchen/chain-utils"; |
No description provided.