feat: integrate depositAsset bridge functionality for ETH and ERC20 t…#227
feat: integrate depositAsset bridge functionality for ETH and ERC20 t…#227big14way wants to merge 1 commit intoExplore-Beyond-Innovations:mainfrom
Conversation
…okens - Add depositAsset contract integration in Ethereum provider - Implement automatic ERC20 token approval handling - Support ETH deposits with msg.value parameter - Add bridge contract configuration for Sepolia testnet - Capture and store commitment hash for L2 operations - Update deposit and claim-burn components with new functionality - Add comprehensive error handling and user feedback - Store transaction details in localStorage for cross-chain tracking Closes Explore-Beyond-Innovations#148
|
@big14way is attempting to deploy a commit to the ZeroXBridge Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughImplements L1->L2 deposit flow via a new depositAsset API in EthereumProvider, wires it into Deposit and Claim/Burn components, and configures Sepolia bridge address. Handles ETH and ERC20 paths, approvals, transaction execution, receipt parsing for commitmentHash, and persists identifiers to localStorage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant D as Deposit UI
participant EP as EthereumProvider
participant ERC as ERC20 Token (if ERC20)
participant BR as L1 Bridge Contract
participant NW as Ethereum Network
U->>D: Click "Deposit" (amount, token)
D->>EP: depositAsset(assetType, tokenAddress, amount)
EP->>NW: getNetwork()
EP->>BR: Resolve bridge at BRIDGE_CONTRACTS[chainId]
alt ERC20 path
EP->>ERC: allowance(user, BR)
alt Insufficient allowance
EP->>ERC: approve(BR, amountInWei)
ERC-->>EP: approval tx receipt
end
EP->>BR: depositAsset(assetType, tokenAddress, amountInWei, user)
else ETH path
EP->>BR: depositAsset(..., user) with msg.value=amountInWei
end
BR-->>EP: tx receipt (logs)
EP->>EP: parse logs -> commitmentHash
EP-->>D: {success, transactionHash, commitmentHash}
D->>D: Persist to localStorage (if present)
D-->>U: Show success/failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(no out-of-scope functional code changes identified) Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
app/config.ts (1)
22-26: Externalize bridge addresses per environment and prepare for multi-chain.Hardcoding the Sepolia bridge in-source is fine for a first pass, but it will become brittle as you add networks or move between staging/prod.
- Load addresses from env (e.g., NEXT_PUBLIC_BRIDGE_) or a typed registry keyed by chainId.
- Consider freezing the map with const assertions to prevent accidental mutation.
Proposed minimal refactor:
-// Bridge contract addresses -export const BRIDGE_CONTRACTS: Record<number, string> = { - [sepolia.id]: '0x8F25bFe32269632dfd8D223D51FF145414d8107b', - // Add other networks as needed -} +// Bridge contract addresses (env-first, typed fallback) +export const BRIDGE_CONTRACTS: Readonly<Record<number, string>> = { + [sepolia.id]: process.env.NEXT_PUBLIC_BRIDGE_11155111 + ?? '0x8F25bFe32269632dfd8D223D51FF145414d8107b', +} as const;Before merging, please verify the Sepolia address against your latest deployment notes/Etherscan for 11155111. If you want, I can generate a small health-check util that reads BRIDGE_CONTRACTS for the connected chain and validates checksum/bytecode.
app/components/claim-burn.tsx (1)
232-236: Guard localStorage usage to avoid SSR edge cases.If this component ever renders server-side or is imported by a Server Component, direct
localStorageaccess may throw. Add a window guard.- if (result.commitmentHash) { - localStorage.setItem('latestCommitmentHash', result.commitmentHash); - localStorage.setItem('latestDepositTx', result.transactionHash || ''); - } + if (result.commitmentHash && typeof window !== 'undefined') { + window.localStorage.setItem('latestCommitmentHash', result.commitmentHash); + window.localStorage.setItem('latestDepositTx', result.transactionHash || ''); + }app/components/deposit.tsx (1)
96-112: Persisting identifiers is good; consider also persisting a typed envelope for future UX.Storing both
latestCommitmentHashandlatestDepositTxis useful. As a small improvement, store a JSON blob keyed by chainId to support multi-chain and richer status.Example:
- if (result.commitmentHash) { - localStorage.setItem('latestCommitmentHash', result.commitmentHash); - localStorage.setItem('latestDepositTx', result.transactionHash || ''); - } + if (result.commitmentHash && typeof window !== 'undefined') { + const entry = { + chainId: 11155111, // TODO: read from provider + commitmentHash: result.commitmentHash, + txHash: result.transactionHash, + token, + amount, + ts: Date.now(), + }; + window.localStorage.setItem('latestDeposit', JSON.stringify(entry)); + }app/components/Ethereum-provider.tsx (2)
58-67: Graceful early return is good; consider surfacing connect action.Returning
{ success: false, error: "Wallet not connected" }is fine. Optionally, exportconnectWalletin the context and use it in callers to prompt connection flows automatically when this error occurs.
132-137: Return the transaction hash from the response instead of the receipt where possible.Both are fine, but returning
tx.hashbefore waiting (and keeping receipt.hash after wait) enables optimistic UI if needed. Not a blocker.- return { - success: true, - commitmentHash, - transactionHash: receipt.hash, - }; + return { + success: true, + commitmentHash, + transactionHash: receipt.hash, // optionally expose tx.hash too + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
app/components/Ethereum-provider.tsx(4 hunks)app/components/claim-burn.tsx(2 hunks)app/components/deposit.tsx(4 hunks)app/config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/components/Ethereum-provider.tsx (1)
app/config.ts (1)
BRIDGE_CONTRACTS(23-26)
app/components/deposit.tsx (2)
app/ThemeContext.tsx (1)
useTheme(36-42)app/components/Ethereum-provider.tsx (1)
useEthereum(196-202)
🔇 Additional comments (7)
app/config.ts (1)
8-9: Sepolia chain integration looks correct.Adding sepolia to wagmi chains and transports is consistent. No functional issues spotted here.
Also applies to: 16-19
app/components/claim-burn.tsx (2)
73-81: Good hook wiring: depositAsset exposed via Ethereum context.Importing and destructuring depositAsset from useEthereum is the right integration point.
217-226: Error propagation from depositAsset is handled well.Early-exiting on
!result.successwith a surfaced message keeps UX predictable.app/components/deposit.tsx (2)
53-67: Solid preflight validation and in-flight guarding.The checks for connection, positive amount, and
isProcessinggating are correct and keep the flow robust.
197-205: Button state UX is clear.Dynamic label (“Connect Ethereum Wallet” / “Processing...”) and disabled state logic are spot on.
app/components/Ethereum-provider.tsx (2)
112-131: The script will print the top ofEthereum-provider.tsx, including thebridgeABIdeclaration, so we can confirm whether event ABI entries (e.g.,AssetDeposited) are present and thus determine if filtering by event name andlog.addressis needed or if the ABI itself must be extended. Once we have that, we can finalize the review.
75-82: Ensure the bridge event ABI is included soparseLogcan decode the commitmentHashThe current ABI array only defines the
depositAssetfunction. Without the event signature,bridgeContract.interface.parseLog(...)will never recognize any logs, socommitmentHashwill always beundefined.• File: app/components/Ethereum-provider.tsx
Lines 75–82: thebridgeABIconstant must include the event that emits the commitment hash.Suggested diff:
- const bridgeABI = [ - "function depositAsset(uint256 assetType, address tokenAddress, uint256 amount, address user) external payable returns (bytes32)" - ]; + const bridgeABI = [ + "function depositAsset(uint256 assetType, address tokenAddress, uint256 amount, address user) external payable returns (bytes32)", + // TODO: replace with your bridge’s actual event signature + "event AssetDeposited(bytes32 indexed commitmentHash, address indexed user, uint256 assetType, address tokenAddress, uint256 amount)" + ];Please confirm the exact event name and parameter order from your deployed bridge contract on Sepolia (chainId 11155111) so we can adapt the ABI and the
parseLogcall accordingly.
| // Determine asset type and token address | ||
| const assetType = token === "ETH" ? 0 : 1; | ||
| let tokenAddress; | ||
|
|
||
| if (assetType === 0) { | ||
| tokenAddress = "0x0000000000000000000000000000000000000000"; | ||
| } else { | ||
| // For ERC20 tokens, you'd need to map token symbols to addresses | ||
| // This is a placeholder - in production, you'd have a token registry | ||
| const tokenAddresses: Record<string, string> = { | ||
| USDC: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", | ||
| USDT: "0xdAC17F958D2ee523a2206206994597C13D831ec7", | ||
| }; | ||
| tokenAddress = tokenAddresses[token]; | ||
|
|
||
| if (!tokenAddress) { | ||
| setError(`Token ${token} not supported`); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Token address mapping is mainnet-only; deposits on Sepolia will fail for ERC20.
You’re passing mainnet USDC/USDT addresses while the bridge is configured for Sepolia. On Sepolia those contracts either don’t exist or don’t match the expected ERC20s, causing allowance checks and deposits to revert.
Recommended approach:
- Maintain a chain-aware token registry (addresses, decimals) keyed by chainId (e.g., 11155111 for Sepolia).
- Pull the connected chainId (either via the Ethereum context or by modifying
depositAssetto resolve decimals/addresses internally). - Surface “Unsupported token on this network” early if missing.
Minimal inline change (temporary) to avoid accidental mainnet addresses:
- const tokenAddresses: Record<string, string> = {
- USDC: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48",
- USDT: "0xdAC17F958D2ee523a2206206994597C13D831ec7",
- };
- tokenAddress = tokenAddresses[token];
+ // TODO: Replace with a proper per-chain registry.
+ const tokenAddressesByChain: Record<number, Record<string, string>> = {
+ 11155111: {
+ // Fill with your Sepolia token deployments
+ // USDC: "<SEPOLIA_USDC_ADDRESS>",
+ // USDT: "<SEPOLIA_USDT_ADDRESS>",
+ },
+ };
+ const chainId = 11155111; // TEMP: read from Ethereum provider/network
+ tokenAddress = tokenAddressesByChain[chainId]?.[token];If you’d like, I can extract a typed TokenRegistry into app/config and wire it here and in claim-burn for consistency.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine asset type and token address | |
| const assetType = token === "ETH" ? 0 : 1; | |
| let tokenAddress; | |
| if (assetType === 0) { | |
| tokenAddress = "0x0000000000000000000000000000000000000000"; | |
| } else { | |
| // For ERC20 tokens, you'd need to map token symbols to addresses | |
| // This is a placeholder - in production, you'd have a token registry | |
| const tokenAddresses: Record<string, string> = { | |
| USDC: "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48", | |
| USDT: "0xdAC17F958D2ee523a2206206994597C13D831ec7", | |
| }; | |
| tokenAddress = tokenAddresses[token]; | |
| if (!tokenAddress) { | |
| setError(`Token ${token} not supported`); | |
| return; | |
| } | |
| } | |
| // Determine asset type and token address | |
| const assetType = token === "ETH" ? 0 : 1; | |
| let tokenAddress; | |
| if (assetType === 0) { | |
| tokenAddress = "0x0000000000000000000000000000000000000000"; | |
| } else { | |
| // For ERC20 tokens, you'd need to map token symbols to addresses | |
| // This is a placeholder - in production, you'd have a token registry | |
| // TODO: Replace with a proper per-chain registry. | |
| const tokenAddressesByChain: Record<number, Record<string, string>> = { | |
| 11155111: { | |
| // Fill with your Sepolia token deployments | |
| // USDC: "<SEPOLIA_USDC_ADDRESS>", | |
| // USDT: "<SEPOLIA_USDT_ADDRESS>", | |
| }, | |
| }; | |
| const chainId = 11155111; // TEMP: read from Ethereum provider/network | |
| tokenAddress = tokenAddressesByChain[chainId]?.[token]; | |
| if (!tokenAddress) { | |
| setError(`Token ${token} not supported`); | |
| return; | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/components/deposit.tsx around lines 68 to 87, the code uses hard-coded
mainnet ERC20 addresses (USDC/USDT) which will fail on Sepolia; change the logic
to be chain-aware by resolving token addresses and decimals from a TokenRegistry
keyed by chainId: obtain the current chainId from the web3/ethers provider or
component props, look up tokenAddresses = tokenRegistry[chainId] and then
tokenAddress = tokenAddresses?.[token]; if the lookup fails, call
setError("Unsupported token on this network") and return; ensure the registry
contains decimals and addresses for each supported chain (e.g., mainnet and
Sepolia) and update any allowance/decimals handling to use the registry values.
| const amountInWei = ethers.parseEther(amount); | ||
|
|
||
| let tx; | ||
|
|
||
| if (assetType === 0) { | ||
| // ETH deposit | ||
| tx = await bridgeContract.depositAsset( | ||
| assetType, | ||
| tokenAddress, | ||
| amountInWei, | ||
| address, | ||
| { value: amountInWei } | ||
| ); | ||
| } else { | ||
| // ERC20 deposit - first check and handle approval | ||
| const erc20ABI = [ | ||
| "function allowance(address owner, address spender) view returns (uint256)", | ||
| "function approve(address spender, uint256 amount) returns (bool)" | ||
| ]; | ||
|
|
||
| const tokenContract = new ethers.Contract(tokenAddress, erc20ABI, signer); | ||
| const allowance = await tokenContract.allowance(address, bridgeAddress); | ||
|
|
||
| if (allowance < amountInWei) { | ||
| const approvalTx = await tokenContract.approve(bridgeAddress, amountInWei); | ||
| await approvalTx.wait(); | ||
| } | ||
|
|
||
| tx = await bridgeContract.depositAsset(assetType, tokenAddress, amountInWei, address); | ||
| } |
There was a problem hiding this comment.
ERC20 amount uses parseEther — breaks for non-18 decimals (USDC/USDT).
For ERC20s, amounts must be in token units (decimals vary). Using parseEther will overshoot by 12 orders for 6-decimal tokens and cause allowance/transfer reverts.
Fix by resolving decimals and using parseUnits; also rename the variable to avoid “wei” implying ETH only:
- const amountInWei = ethers.parseEther(amount);
+ // Resolve amount units
+ let amountInUnits: bigint;ETH path:
- if (assetType === 0) {
+ if (assetType === 0) {
// ETH deposit
- tx = await bridgeContract.depositAsset(
+ const amountInWei = ethers.parseEther(amount);
+ tx = await bridgeContract.depositAsset(
assetType,
tokenAddress,
- amountInWei,
+ amountInWei,
address,
{ value: amountInWei }
);
} else {
// ERC20 deposit - first check and handle approval
const erc20ABI = [
"function allowance(address owner, address spender) view returns (uint256)",
- "function approve(address spender, uint256 amount) returns (bool)"
+ "function approve(address spender, uint256 amount) returns (bool)",
+ "function decimals() view returns (uint8)"
];
const tokenContract = new ethers.Contract(tokenAddress, erc20ABI, signer);
+ const decimals: number = await tokenContract.decimals();
+ amountInUnits = ethers.parseUnits(amount, decimals);
- const allowance = await tokenContract.allowance(address, bridgeAddress);
+ const allowance = await tokenContract.allowance(address, bridgeAddress);
- if (allowance < amountInWei) {
- const approvalTx = await tokenContract.approve(bridgeAddress, amountInWei);
+ if (allowance < amountInUnits) {
+ const approvalTx = await tokenContract.approve(bridgeAddress, amountInUnits);
await approvalTx.wait();
}
- tx = await bridgeContract.depositAsset(assetType, tokenAddress, amountInWei, address);
+ tx = await bridgeContract.depositAsset(assetType, tokenAddress, amountInUnits, address);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const amountInWei = ethers.parseEther(amount); | |
| let tx; | |
| if (assetType === 0) { | |
| // ETH deposit | |
| tx = await bridgeContract.depositAsset( | |
| assetType, | |
| tokenAddress, | |
| amountInWei, | |
| address, | |
| { value: amountInWei } | |
| ); | |
| } else { | |
| // ERC20 deposit - first check and handle approval | |
| const erc20ABI = [ | |
| "function allowance(address owner, address spender) view returns (uint256)", | |
| "function approve(address spender, uint256 amount) returns (bool)" | |
| ]; | |
| const tokenContract = new ethers.Contract(tokenAddress, erc20ABI, signer); | |
| const allowance = await tokenContract.allowance(address, bridgeAddress); | |
| if (allowance < amountInWei) { | |
| const approvalTx = await tokenContract.approve(bridgeAddress, amountInWei); | |
| await approvalTx.wait(); | |
| } | |
| tx = await bridgeContract.depositAsset(assetType, tokenAddress, amountInWei, address); | |
| } | |
| // Resolve amount units | |
| let amountInUnits: bigint; | |
| let tx; | |
| if (assetType === 0) { | |
| // ETH deposit | |
| const amountInWei = ethers.parseEther(amount); | |
| tx = await bridgeContract.depositAsset( | |
| assetType, | |
| tokenAddress, | |
| amountInWei, | |
| address, | |
| { value: amountInWei } | |
| ); | |
| } else { | |
| // ERC20 deposit - first check and handle approval | |
| const erc20ABI = [ | |
| "function allowance(address owner, address spender) view returns (uint256)", | |
| "function approve(address spender, uint256 amount) returns (bool)", | |
| "function decimals() view returns (uint8)" | |
| ]; | |
| const tokenContract = new ethers.Contract(tokenAddress, erc20ABI, signer); | |
| const decimals: number = await tokenContract.decimals(); | |
| amountInUnits = ethers.parseUnits(amount, decimals); | |
| const allowance = await tokenContract.allowance(address, bridgeAddress); | |
| if (allowance < amountInUnits) { | |
| const approvalTx = await tokenContract.approve(bridgeAddress, amountInUnits); | |
| await approvalTx.wait(); | |
| } | |
| tx = await bridgeContract.depositAsset( | |
| assetType, | |
| tokenAddress, | |
| amountInUnits, | |
| address | |
| ); | |
| } |
1 participant
…okens
Closes #148
Summary by CodeRabbit