Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds ERC‑20 approval to ad funding and a required fundAmount, expands AdManager ABI, introduces chain fetching and selection UI with skeleton loading states, simplifies the Create Ad UI, adjusts environment variable handling, and renames/aligns chain service/hook APIs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Create Ad UI / AddLiquidity
participant Chains as useGetAllChains
participant Hook as useAds
participant ERC20 as ERC20 Token
participant AdMgr as AdManager Contract
User->>UI: Open Create Ad
UI->>Chains: fetch chains
Chains-->>UI: list
User->>UI: Select chains, fill ad + fundAmount
UI->>Hook: submit createAd request
Hook->>ERC20: approve(spender=AdMgr, amount=fundAmount)
ERC20-->>Hook: approve tx receipt (status)
alt approval reverted
Hook-->>UI: error "Transaction not approved"
else approval success
Hook->>AdMgr: createAd(..., initialAmount=fundAmount, ...)
AdMgr-->>Hook: createAd tx receipt (status)
alt create reverted
Hook-->>UI: error "Transaction failed, Retry"
else success
Hook->>UI: confirm / onSuccess
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/frontend/.env.example(0 hunks)apps/frontend/abis/AdManager.abi.ts(3 hunks)apps/frontend/app/(app)/(auth-protected)/ads-management/create/page.tsx(0 hunks)apps/frontend/app/(app)/(auth-protected)/home/page.tsx(7 hunks)apps/frontend/components/ad-management-ui/AddLiquidity.tsx(7 hunks)apps/frontend/components/ad-management-ui/CreateAd.tsx(1 hunks)apps/frontend/components/dashboard/SkeletonAdCard.tsx(1 hunks)apps/frontend/hooks/useAds.ts(2 hunks)apps/frontend/hooks/useChains.tsx(1 hunks)apps/frontend/lib/env_variables.ts(1 hunks)apps/frontend/package.json(1 hunks)apps/frontend/services/chains.service.ts(1 hunks)apps/frontend/types/ads.ts(1 hunks)apps/frontend/utils/urls.ts(1 hunks)
💤 Files with no reviewable changes (2)
- apps/frontend/app/(app)/(auth-protected)/ads-management/create/page.tsx
- apps/frontend/.env.example
🧰 Additional context used
🧬 Code graph analysis (5)
apps/frontend/hooks/useChains.tsx (1)
apps/frontend/services/chains.service.ts (1)
getAllChains(9-15)
apps/frontend/components/ad-management-ui/CreateAd.tsx (1)
apps/frontend/components/ad-management-ui/AddLiquidity.tsx (1)
AddLiquidity(23-377)
apps/frontend/components/ad-management-ui/AddLiquidity.tsx (1)
apps/frontend/hooks/useChains.tsx (1)
useGetAllChains(5-13)
apps/frontend/app/(app)/(auth-protected)/home/page.tsx (2)
apps/frontend/hooks/useAds.ts (1)
useGetAllAds(283-288)apps/frontend/components/dashboard/SkeletonAdCard.tsx (1)
SkeletonAdCard(4-56)
apps/frontend/hooks/useAds.ts (5)
apps/frontend/services/tokens.service.ts (1)
getTokens(9-12)apps/frontend/abis/ERC20.abi.ts (1)
ERC20_ABI(1-222)apps/frontend/utils/wagmi-config.ts (1)
config(5-12)apps/frontend/abis/AdManager.abi.ts (1)
AD_MANAGER_ABI(1-941)apps/frontend/services/ads.service.ts (1)
confirmAdTx(59-63)
🪛 Biome (2.1.2)
apps/frontend/app/(app)/(auth-protected)/home/page.tsx
[error] 52-52: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 73-73: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 94-94: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 115-115: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 136-136: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (1)
apps/frontend/lib/env_variables.ts (1)
3-6: LGTM!The type definition correctly expands the union to include the three environment variable keys.
| const response = await createAd({ | ||
| routeId: routes!.data[0]!.id!, | ||
| creatorDstAddress: account.address!, | ||
| maxAmount: parseUnits( | ||
| max, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), | ||
| minAmount: parseUnits( | ||
| min, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), | ||
|
|
||
| metadata: { | ||
| title, | ||
| description, | ||
| }, | ||
| }) | ||
| await fundAd({ | ||
| poolAmountTopUp: parseUnits( | ||
| fundAmount: parseUnits( | ||
| amount, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), | ||
| adId: response.adId, | ||
| amountBigInt: parseUnits( | ||
| amount, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| ), | ||
| }) |
There was a problem hiding this comment.
Guard against missing bridge routes before creating an ad.
routes!.data[0]!.id! assumes the bridge-routes query has finished and returned at least one route. While the query is still loading—or if the backend returns an empty list—this will throw at runtime, blocking ad creation. Bail out when the route list is empty before dereferencing it.
-import { Button, Modal, Select, Skeleton } from "antd"
+import { Button, Modal, Select, Skeleton, message } from "antd"
@@
- const handleCreateAd = async () => {
+ const handleCreateAd = async () => {
+ if (!routes?.data?.length) {
+ message.error("No bridge routes are available for the selected chains yet.")
+ return
+ }
+
+ const [route] = routes.data
+ if (!route?.id) {
+ message.error("Bridge route response is missing an identifier.")
+ return
+ }
+
try {
const response = await createAd({
- routeId: routes!.data[0]!.id!,
+ routeId: route.id,
creatorDstAddress: account.address!,
maxAmount: parseUnits(
max,
liquidity_chain?.nativeCurrency?.decimals!
).toString(),📝 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 response = await createAd({ | |
| routeId: routes!.data[0]!.id!, | |
| creatorDstAddress: account.address!, | |
| maxAmount: parseUnits( | |
| max, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| minAmount: parseUnits( | |
| min, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| metadata: { | |
| title, | |
| description, | |
| }, | |
| }) | |
| await fundAd({ | |
| poolAmountTopUp: parseUnits( | |
| fundAmount: parseUnits( | |
| amount, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| adId: response.adId, | |
| amountBigInt: parseUnits( | |
| amount, | |
| liquidity_chain.nativeCurrency.decimals | |
| ), | |
| }) | |
| import { Button, Modal, Select, Skeleton, message } from "antd" | |
| // … other imports and component setup … | |
| const handleCreateAd = async () => { | |
| // Guard against missing or empty routes | |
| if (!routes?.data?.length) { | |
| message.error("No bridge routes are available for the selected chains yet.") | |
| return | |
| } | |
| const [route] = routes.data | |
| if (!route?.id) { | |
| message.error("Bridge route response is missing an identifier.") | |
| return | |
| } | |
| try { | |
| const response = await createAd({ | |
| routeId: route.id, | |
| creatorDstAddress: account.address!, | |
| maxAmount: parseUnits( | |
| max, | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| minAmount: parseUnits( | |
| min, | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| metadata: { | |
| title, | |
| description, | |
| }, | |
| fundAmount: parseUnits( | |
| amount, | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| }) | |
| // … rest of your success handling … | |
| } catch (error) { | |
| // … error handling … | |
| } | |
| } | |
| // … rest of component … |
🤖 Prompt for AI Agents
In apps/frontend/components/ad-management-ui/AddLiquidity.tsx around lines 48 to
68, the code dereferences routes!.data[0]!.id! which will throw if the
bridge-routes query is still loading or returns an empty list; before creating
the ad, check that routes is defined, not loading, and routes.data has at least
one element (e.g. if (!routes || !routes.data || routes.data.length === 0)
return/throw/show user error), avoid using non-null assertions for the route id,
and surface a clear error or disable the submit flow until a valid route id is
available so createAd is only called with a definite routeId.
| <Select | ||
| loading={loadingChains} | ||
| className="w-full !h-[40px]" | ||
| options={chains?.rows | ||
| .filter((chain) => Number(chain.chainId) !== other_chain?.id!) | ||
| .map((chain) => { | ||
| return { | ||
| label: chain.name, | ||
| value: chain.chainId, | ||
| } | ||
| })} | ||
| allowClear={{ | ||
| clearIcon: <GiCancel className="text-red-500" size={15} />, | ||
| }} | ||
| onChange={(value: number) => { | ||
| setLiquidity_chain(supported_chains[value]) | ||
| }} | ||
| onClear={() => setLiquidity_chain(undefined)} | ||
| /> | ||
| <p className="text-lg">{liquidity_chain.name}</p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div> | ||
| <p className="text-grey-300"> Destination Chain</p> | ||
| <div className="flex gap-2 items-center"> | ||
| <Image | ||
| src={chain_icons[other_chain.id]} | ||
| alt="" | ||
| height={30} | ||
| width={30} | ||
| className="rounded-full" | ||
| <div> | ||
| <Select | ||
| loading={loadingChains} | ||
| className="w-full !h-[40px]" | ||
| options={chains?.rows | ||
| .filter( | ||
| (chain) => Number(chain.chainId) !== liquidity_chain?.id! | ||
| ) | ||
| .map((chain) => { | ||
| return { | ||
| label: chain.name, | ||
| value: chain.chainId, | ||
| } | ||
| })} | ||
| allowClear={{ | ||
| clearIcon: <GiCancel className="text-red-500" size={15} />, | ||
| }} | ||
| onChange={(value: number) => { | ||
| setOther_chain(supported_chains[value]) | ||
| }} | ||
| onClear={() => setOther_chain(undefined)} | ||
| /> |
There was a problem hiding this comment.
Filter/select only supported chains when binding selection state.
The Select displays every chain returned by useGetAllChains, but the component later maps the chosen value through supported_chains. If a chain that is not in supported_chains is picked, supported_chains[value] is undefined, leaving liquidity_chain/other_chain unset. From the user’s perspective the dropdown still shows the choice, yet the form keeps warning “Please select Base chain,” effectively blocking progress. Restrict the options to supported IDs (and guard the assignment) so users can only pick chains your logic understands.
-import { Button, Modal, Select, Skeleton } from "antd"
+import { Button, Modal, Select, Skeleton, message } from "antd"
@@
- <Select
+ <Select
loading={loadingChains}
className="w-full !h-[40px]"
- options={chains?.rows
- .filter((chain) => Number(chain.chainId) !== other_chain?.id!)
- .map((chain) => {
- return {
- label: chain.name,
- value: chain.chainId,
- }
- })}
+ options={chains?.rows
+ ?.filter((chain) => supported_chains[Number(chain.chainId)])
+ ?.filter((chain) => Number(chain.chainId) !== other_chain?.id)
+ ?.map((chain) => ({
+ label: chain.name,
+ value: chain.chainId,
+ }))}
allowClear={{
clearIcon: <GiCancel className="text-red-500" size={15} />,
}}
- onChange={(value: number) => {
- setLiquidity_chain(supported_chains[value])
- }}
+ onChange={(value: string) => {
+ const selectedChain = supported_chains[Number(value)]
+ if (!selectedChain) {
+ message.error("Selected base chain is not supported yet.")
+ return
+ }
+ setLiquidity_chain(selectedChain)
+ }}
onClear={() => setLiquidity_chain(undefined)}
@@
- <Select
+ <Select
loading={loadingChains}
className="w-full !h-[40px]"
- options={chains?.rows
- .filter(
- (chain) => Number(chain.chainId) !== liquidity_chain?.id!
- )
- .map((chain) => {
- return {
- label: chain.name,
- value: chain.chainId,
- }
- })}
+ options={chains?.rows
+ ?.filter((chain) => supported_chains[Number(chain.chainId)])
+ ?.filter(
+ (chain) => Number(chain.chainId) !== liquidity_chain?.id
+ )
+ ?.map((chain) => ({
+ label: chain.name,
+ value: chain.chainId,
+ }))}
allowClear={{
clearIcon: <GiCancel className="text-red-500" size={15} />,
}}
- onChange={(value: number) => {
- setOther_chain(supported_chains[value])
- }}
+ onChange={(value: string) => {
+ const selectedChain = supported_chains[Number(value)]
+ if (!selectedChain) {
+ message.error("Selected destination chain is not supported yet.")
+ return
+ }
+ setOther_chain(selectedChain)
+ }}
onClear={() => setOther_chain(undefined)}🤖 Prompt for AI Agents
In apps/frontend/components/ad-management-ui/AddLiquidity.tsx around lines 86 to
131, the Select options include all chains from useGetAllChains but you later
map the selected value to supported_chains by index, which can produce undefined
and leave liquidity_chain/other_chain unset; fix this by restricting the options
to only chains present in supported_chains (filter chains.rows by
supported_chains.some(sc => sc.id === Number(chain.chainId))) and change the
onChange handler to lookup the selected chain by id (e.g. find supported_chains
entry where sc.id === Number(value)) and only call
setLiquidity_chain/setOther_chain if that find returns a valid entry (otherwise
clear or no-op), ensuring proper numeric conversions and a guarded assignment.
| const token = await getTokens({ chainId: String(response.chainId!) }) | ||
| const approveHash = await writeContractAsync({ | ||
| address: token.data[0].address, | ||
| abi: ERC20_ABI, | ||
| chainId: Number(response.chainId), | ||
| functionName: "createAd", | ||
| args: [ | ||
| response.signature, | ||
| response.authToken, | ||
| BigInt(response.timeToExpire), | ||
| response.adId, | ||
| response.adToken, | ||
| BigInt(response.orderChainId), | ||
| response.adRecipient, | ||
| ], | ||
| functionName: "approve", | ||
| args: [response.contractAddress, data.fundAmount], | ||
| }) | ||
| const receipt = await waitForTransactionReceipt(config, { hash: txHash }) | ||
|
|
||
| if (receipt.status === "success") { | ||
| await confirmAdTx({ | ||
| txHash: receipt.transactionHash, | ||
| signature: response.signature, | ||
| adId: response.adId, | ||
| const approveReceipt = await waitForTransactionReceipt(config, { | ||
| hash: approveHash, | ||
| }) | ||
| if (approveReceipt.status === "success") { | ||
| const txHash = await writeContractAsync({ | ||
| address: response.contractAddress, | ||
| abi: AD_MANAGER_ABI, | ||
| chainId: Number(response.chainId), | ||
| functionName: "createAd", | ||
| args: [ | ||
| response.signature, | ||
| response.authToken, | ||
| BigInt(response.timeToExpire), | ||
| response.adId, | ||
| response.adToken, | ||
| data.fundAmount, | ||
| BigInt(response.orderChainId), | ||
| response.adRecipient, |
There was a problem hiding this comment.
Approve & deposit the exact ad token amount in base units
Two blockers here:
- We grab
token.data[0]and approve that address, but the contract expects the specificresponse.adToken. If the API returns tokens in a different order,createAdwill revert with an ad-token mismatch. fundAmountis still the UI string.approve/createAdexpect the raw uint256 amount; passing the string straight through won’t encode correctly and ignores token decimals.
Instead, locate the metadata for response.adToken, convert the user amount with those decimals, and reuse that bigint for both transactions:
-import { getTokens } from "@/services/tokens.service"
+import { getTokens } from "@/services/tokens.service"
+import { parseUnits } from "viem"
…
- const approveHash = await writeContractAsync({
- address: token.data[0].address,
+ const tokenMetadata = token.data.find(
+ (item) =>
+ item.address.toLowerCase() === response.adToken.toLowerCase()
+ )
+ if (!tokenMetadata) {
+ throw Error("Ad token metadata not found")
+ }
+ const fundAmountBigInt = parseUnits(
+ data.fundAmount,
+ tokenMetadata.decimals
+ )
+ const approveHash = await writeContractAsync({
+ address: tokenMetadata.address,
abi: ERC20_ABI,
chainId: Number(response.chainId),
functionName: "approve",
- args: [response.contractAddress, data.fundAmount],
+ args: [response.contractAddress, fundAmountBigInt],
})
…
args: [
response.signature,
response.authToken,
BigInt(response.timeToExpire),
response.adId,
response.adToken,
- data.fundAmount,
+ fundAmountBigInt,
BigInt(response.orderChainId),
response.adRecipient,
],Apply the same fundAmountBigInt when wiring the fund transaction so both steps use the exact approved amount.
📝 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 token = await getTokens({ chainId: String(response.chainId!) }) | |
| const approveHash = await writeContractAsync({ | |
| address: token.data[0].address, | |
| abi: ERC20_ABI, | |
| chainId: Number(response.chainId), | |
| functionName: "createAd", | |
| args: [ | |
| response.signature, | |
| response.authToken, | |
| BigInt(response.timeToExpire), | |
| response.adId, | |
| response.adToken, | |
| BigInt(response.orderChainId), | |
| response.adRecipient, | |
| ], | |
| functionName: "approve", | |
| args: [response.contractAddress, data.fundAmount], | |
| }) | |
| const receipt = await waitForTransactionReceipt(config, { hash: txHash }) | |
| if (receipt.status === "success") { | |
| await confirmAdTx({ | |
| txHash: receipt.transactionHash, | |
| signature: response.signature, | |
| adId: response.adId, | |
| const approveReceipt = await waitForTransactionReceipt(config, { | |
| hash: approveHash, | |
| }) | |
| if (approveReceipt.status === "success") { | |
| const txHash = await writeContractAsync({ | |
| address: response.contractAddress, | |
| abi: AD_MANAGER_ABI, | |
| chainId: Number(response.chainId), | |
| functionName: "createAd", | |
| args: [ | |
| response.signature, | |
| response.authToken, | |
| BigInt(response.timeToExpire), | |
| response.adId, | |
| response.adToken, | |
| data.fundAmount, | |
| BigInt(response.orderChainId), | |
| response.adRecipient, | |
| // at the top of the file | |
| import { getTokens } from "@/services/tokens.service" | |
| import { parseUnits } from "viem" | |
| … | |
| const token = await getTokens({ chainId: String(response.chainId!) }) | |
| // find the exact ad token metadata | |
| const tokenMetadata = token.data.find( | |
| (item) => | |
| item.address.toLowerCase() === response.adToken.toLowerCase() | |
| ) | |
| if (!tokenMetadata) { | |
| throw Error("Ad token metadata not found") | |
| } | |
| // convert the UI string into on‐chain base units once | |
| const fundAmountBigInt = parseUnits( | |
| data.fundAmount, | |
| tokenMetadata.decimals | |
| ) | |
| const approveHash = await writeContractAsync({ | |
| address: tokenMetadata.address, | |
| abi: ERC20_ABI, | |
| chainId: Number(response.chainId), | |
| functionName: "approve", | |
| args: [response.contractAddress, fundAmountBigInt], | |
| }) | |
| const approveReceipt = await waitForTransactionReceipt(config, { | |
| hash: approveHash, | |
| }) | |
| if (approveReceipt.status === "success") { | |
| const txHash = await writeContractAsync({ | |
| address: response.contractAddress, | |
| abi: AD_MANAGER_ABI, | |
| chainId: Number(response.chainId), | |
| functionName: "createAd", | |
| args: [ | |
| response.signature, | |
| response.authToken, | |
| BigInt(response.timeToExpire), | |
| response.adId, | |
| response.adToken, | |
| fundAmountBigInt, | |
| BigInt(response.orderChainId), | |
| response.adRecipient, |
🤖 Prompt for AI Agents
In apps/frontend/hooks/useAds.ts around lines 33 to 58, the code approves
token.data[0] and passes the UI string fundAmount directly, which can cause an
ad-token mismatch and incorrect encoding; change the logic to find the token
metadata whose address matches response.adToken (not blindly using
token.data[0]), convert the user-entered fundAmount string into the correct
base-unit BigInt using that token's decimals, and then use that token.address
for the approve call and reuse the computed fundAmountBigInt for both the
approve args and the createAd args so both transactions use the exact same raw
uint256 amount.
| > => { | ||
| return { | ||
| WALLET_CONNECT_PROJECT_ID: process.env.WALLET_CONNECT_PROJECT_ID!, | ||
| NEXT_PUBLIC_SIGN_DOMAIN: process.env.WALLET_CONNECT_PROJECT_ID!, |
There was a problem hiding this comment.
Critical: Wrong environment variable assignment.
Line 13 assigns NEXT_PUBLIC_SIGN_DOMAIN from process.env.WALLET_CONNECT_PROJECT_ID! instead of process.env.NEXT_PUBLIC_SIGN_DOMAIN!. This is a copy-paste error that will cause NEXT_PUBLIC_SIGN_DOMAIN to have an incorrect value at runtime.
Apply this diff to fix the assignment:
- NEXT_PUBLIC_SIGN_DOMAIN: process.env.WALLET_CONNECT_PROJECT_ID!,
+ NEXT_PUBLIC_SIGN_DOMAIN: process.env.NEXT_PUBLIC_SIGN_DOMAIN!,📝 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.
| NEXT_PUBLIC_SIGN_DOMAIN: process.env.WALLET_CONNECT_PROJECT_ID!, | |
| NEXT_PUBLIC_SIGN_DOMAIN: process.env.NEXT_PUBLIC_SIGN_DOMAIN!, |
🤖 Prompt for AI Agents
In apps/frontend/lib/env_variables.ts around line 13, NEXT_PUBLIC_SIGN_DOMAIN is
incorrectly assigned from process.env.WALLET_CONNECT_PROJECT_ID due to a
copy‑paste error; change the assignment to use
process.env.NEXT_PUBLIC_SIGN_DOMAIN! (preserving the non‑null assertion) so the
exported NEXT_PUBLIC_SIGN_DOMAIN value comes from the correct environment
variable.
| NEXT_PUBLIC_SIGN_DOMAIN: process.env.WALLET_CONNECT_PROJECT_ID!, | ||
| NEXT_PUBLIC_SIGN_URI: process.env.NEXT_PUBLIC_SIGN_URI!, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify environment variable documentation.
The code now requires NEXT_PUBLIC_SIGN_DOMAIN and NEXT_PUBLIC_SIGN_URI, but according to the PR changes, these keys were removed from .env.example. This creates a documentation gap that could confuse developers during project setup.
Run the following script to verify the state of .env.example:
If these variables are missing from .env.example, please add them to help developers configure the application correctly.
🏁 Script executed:
#!/bin/bash
# Description: Check if NEXT_PUBLIC_SIGN_DOMAIN and NEXT_PUBLIC_SIGN_URI are documented in .env.example
if [ -f "apps/frontend/.env.example" ]; then
echo "Checking for NEXT_PUBLIC_SIGN_DOMAIN:"
rg -n "NEXT_PUBLIC_SIGN_DOMAIN" apps/frontend/.env.example || echo " -> NOT FOUND"
echo ""
echo "Checking for NEXT_PUBLIC_SIGN_URI:"
rg -n "NEXT_PUBLIC_SIGN_URI" apps/frontend/.env.example || echo " -> NOT FOUND"
echo ""
echo "Current .env.example contents:"
cat apps/frontend/.env.example
else
echo ".env.example not found at apps/frontend/.env.example"
fiLength of output: 52
🏁 Script executed:
#!/bin/bash
echo "Searching for .env.example files in the repository"
files=$(fd --hidden --type f --max-depth 4 .env.example || find . -name .env.example)
if [ -z "$files" ]; then
echo "No .env.example files found"
exit 0
fi
echo "$files" | while read -r f; do
echo "File: $f"
rg -n "NEXT_PUBLIC_SIGN_DOMAIN" "$f" || echo " -> NEXT_PUBLIC_SIGN_DOMAIN NOT FOUND"
rg -n "NEXT_PUBLIC_SIGN_URI" "$f" || echo " -> NEXT_PUBLIC_SIGN_URI NOT FOUND"
echo ""
doneLength of output: 166
Add NEXT_PUBLIC_SIGN_DOMAIN and NEXT_PUBLIC_SIGN_URI to frontend env docs
apps/frontend currently has no .env.example listing these variables; without them the non-null assertions will throw at runtime. Create or update apps/frontend/.env.example (or the top-level .env.example) to include:
NEXT_PUBLIC_SIGN_DOMAIN=
NEXT_PUBLIC_SIGN_URI=
🤖 Prompt for AI Agents
In apps/frontend/lib/env_variables.ts around lines 13-14, the code uses non-null
assertions for NEXT_PUBLIC_SIGN_DOMAIN and NEXT_PUBLIC_SIGN_URI but those
variables are missing from the frontend environment example, causing runtime
failures; update apps/frontend/.env.example (or the top-level .env.example if
you prefer centralized docs) to add the two entries NEXT_PUBLIC_SIGN_DOMAIN= and
NEXT_PUBLIC_SIGN_URI= (as blank placeholders) so developers know to set them
before running the app.
| "next": "15.5.2", | ||
| "next-themes": "^0.4.6", | ||
| "proof-bridge": "link:", | ||
| "proof-bridge-frontend": "link:", |
There was a problem hiding this comment.
Remove the self-referential dependency
The package is already named proof-bridge-frontend; adding a link: dependency with the same name wires the app to depend on itself. Yarn/PNPM will refuse to install this (cycle detection), so the workspace won’t build. Drop this dependency or point it at the actual external package instead.
- "proof-bridge-frontend": "link:",📝 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.
| "proof-bridge-frontend": "link:", |
🤖 Prompt for AI Agents
In apps/frontend/package.json at line 29, the dependency
"proof-bridge-frontend": "link:" creates a self-referential dependency that
causes workspace cycle detection to fail; remove this entry (or replace it with
the correct external package name/version) so the frontend package does not
depend on itself, then run the package manager install to verify the workspace
builds.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/frontend/components/ad-management-ui/AddLiquidity.tsx (3)
44-70: Address unresolved route guard and error handling issues.The non-null assertion at line 47 (
routes?.data[0]?.id!) remains from the previous review. Ifroutesis still loading or returns an empty array, this will throw at runtime. Additionally, the empty catch block at line 69 silently swallows errors, preventing users from understanding what went wrong.Apply this diff to guard route access and handle errors properly:
+import { message } from "antd" const handleCreateAd = async () => { + if (!routes?.data?.length) { + message.error("No bridge routes available for the selected chains.") + return + } + + const route = routes.data[0] + if (!route?.id) { + message.error("Bridge route is missing an identifier.") + return + } + try { const response = await createAd({ - routeId: routes?.data[0]?.id!, + routeId: route.id, creatorDstAddress: account.address!, maxAmount: parseUnits( max, liquidity_chain?.nativeCurrency?.decimals! ).toString(), minAmount: parseUnits( min, liquidity_chain?.nativeCurrency?.decimals! ).toString(), metadata: { title, description, }, fundAmount: parseUnits( amount, liquidity_chain?.nativeCurrency?.decimals! ).toString(), }) toggleModal() - } catch (error) {} + } catch (error) { + message.error("Failed to create ad. Please try again.") + console.error("Ad creation error:", error) + } }
83-104: Restrict base chain options to supported chains only.The Select displays all chains from
useGetAllChains, but line 100 maps the selected value throughsupported_chains. If a user picks an unsupported chain,supported_chains[value]returnsundefined, leavingliquidity_chainunset. This causes the warning at line 242 to persist despite a selection being visible in the dropdown, blocking progress.Apply this diff to filter options and validate selection:
+import { message } from "antd" <Select loading={loadingChains} className="w-full !h-[40px]" options={chains?.rows - .filter((chain) => Number(chain.chainId) !== other_chain?.id!) + ?.filter((chain) => supported_chains[Number(chain.chainId)]) + ?.filter((chain) => Number(chain.chainId) !== other_chain?.id) .map((chain) => { return { label: chain.name, value: chain.chainId, } })} allowClear={{ clearIcon: <GiCancel className="text-red-500" size={15} />, }} onChange={(value: number) => { - setLiquidity_chain(supported_chains[value]) + const selectedChain = supported_chains[Number(value)] + if (!selectedChain) { + message.error("Selected base chain is not supported yet.") + return + } + setLiquidity_chain(selectedChain) }} onClear={() => setLiquidity_chain(undefined)} />
108-131: Restrict destination chain options to supported chains only.Same issue as the base chain Select: displaying all chains but only supporting a subset leads to silent failures when users pick unsupported chains.
Apply this diff to filter options and validate selection:
+import { message } from "antd" <Select loading={loadingChains} className="w-full !h-[40px]" options={chains?.rows + ?.filter((chain) => supported_chains[Number(chain.chainId)]) .filter( - (chain) => Number(chain.chainId) !== liquidity_chain?.id! + (chain) => Number(chain.chainId) !== liquidity_chain?.id ) .map((chain) => { return { label: chain.name, value: chain.chainId, } })} allowClear={{ clearIcon: <GiCancel className="text-red-500" size={15} />, }} onChange={(value: number) => { - setOther_chain(supported_chains[value]) + const selectedChain = supported_chains[Number(value)] + if (!selectedChain) { + message.error("Selected destination chain is not supported yet.") + return + } + setOther_chain(selectedChain) }} onClear={() => setOther_chain(undefined)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/frontend/app/(app)/(auth-protected)/home/page.tsx(7 hunks)apps/frontend/components/ad-management-ui/AddLiquidity.tsx(6 hunks)apps/frontend/eslint.config.mjs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/frontend/app/(app)/(auth-protected)/home/page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/frontend/components/ad-management-ui/AddLiquidity.tsx (2)
apps/frontend/hooks/useChains.tsx (1)
useGetAllChains(5-13)apps/frontend/services/ads.service.ts (1)
createAd(24-27)
| routeId: routes?.data[0]?.id!, | ||
| creatorDstAddress: account.address!, | ||
| maxAmount: parseUnits( | ||
| max, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), | ||
| minAmount: parseUnits( | ||
| min, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), | ||
|
|
||
| metadata: { | ||
| title, | ||
| description, | ||
| }, | ||
| }) | ||
| await fundAd({ | ||
| poolAmountTopUp: parseUnits( | ||
| fundAmount: parseUnits( | ||
| amount, | ||
| liquidity_chain.nativeCurrency.decimals | ||
| liquidity_chain?.nativeCurrency?.decimals! | ||
| ).toString(), |
There was a problem hiding this comment.
Remove non-null assertions on decimal lookups.
Lines 51, 55, and 64 use non-null assertions (!) on liquidity_chain?.nativeCurrency?.decimals. If liquidity_chain or its nativeCurrency is undefined, parseUnits will receive undefined and throw at runtime.
Apply this diff to validate decimals before calling parseUnits:
+import { message } from "antd"
const handleCreateAd = async () => {
...
+ const decimals = liquidity_chain?.nativeCurrency?.decimals
+ if (decimals === undefined) {
+ message.error("Chain currency information is missing.")
+ return
+ }
+
try {
const response = await createAd({
routeId: route.id,
creatorDstAddress: account.address!,
maxAmount: parseUnits(
max,
- liquidity_chain?.nativeCurrency?.decimals!
+ decimals
).toString(),
minAmount: parseUnits(
min,
- liquidity_chain?.nativeCurrency?.decimals!
+ decimals
).toString(),
metadata: {
title,
description,
},
fundAmount: parseUnits(
amount,
- liquidity_chain?.nativeCurrency?.decimals!
+ decimals
).toString(),
})
...📝 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.
| routeId: routes?.data[0]?.id!, | |
| creatorDstAddress: account.address!, | |
| maxAmount: parseUnits( | |
| max, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| minAmount: parseUnits( | |
| min, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| metadata: { | |
| title, | |
| description, | |
| }, | |
| }) | |
| await fundAd({ | |
| poolAmountTopUp: parseUnits( | |
| fundAmount: parseUnits( | |
| amount, | |
| liquidity_chain.nativeCurrency.decimals | |
| liquidity_chain?.nativeCurrency?.decimals! | |
| ).toString(), | |
| // at the top of the file – add the import | |
| import { message } from "antd" | |
| // … inside AddLiquidity.tsx … | |
| const handleCreateAd = async () => { | |
| // … | |
| // guard against missing decimals | |
| const decimals = liquidity_chain?.nativeCurrency?.decimals | |
| if (decimals === undefined) { | |
| message.error("Chain currency information is missing.") | |
| return | |
| } | |
| try { | |
| const response = await createAd({ | |
| routeId: routes?.data[0]?.id!, | |
| creatorDstAddress: account.address!, | |
| maxAmount: parseUnits(max, decimals).toString(), | |
| minAmount: parseUnits(min, decimals).toString(), | |
| metadata: { | |
| title, | |
| description, | |
| }, | |
| fundAmount: parseUnits(amount, decimals).toString(), | |
| }) | |
| // … | |
| } catch (err) { | |
| // … | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/frontend/components/ad-management-ui/AddLiquidity.tsx around lines 47 to
65, the code uses non-null assertions on
liquidity_chain?.nativeCurrency?.decimals when calling parseUnits, which can
pass undefined and throw; replace those assertions by reading decimals into a
local variable, validate it (throw or show an error/disable the submit if
undefined or not a number), and only call parseUnits when decimals is a defined
number; apply this for maxAmount, minAmount, and fundAmount, and ensure
route/user-facing flow handles the validation error (e.g., early return or
disabled button) instead of relying on !
| rules: { | ||
| "@typescript-eslint/no-non-null-asserted-optional-chain": "off", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the necessity of disabling this type safety rule.
Disabling @typescript-eslint/no-non-null-asserted-optional-chain permits patterns like foo?.bar! where optional chaining (?.) returns undefined if the chain is broken, yet the non-null assertion (!) claims the result is definitely not undefined. This contradicts TypeScript's type safety guarantees.
Ensure that disabling this rule is truly necessary and doesn't mask type errors in the codebase. Consider refactoring code to avoid this pattern rather than disabling the safety check.
Run the following script to identify where non-null assertions on optional chains are used:
🏁 Script executed:
#!/bin/bash
# Description: Find uses of non-null assertion on optional chains in the frontend app
# Search for the pattern: optional chaining followed by non-null assertion
rg -nP --type=ts --type=tsx -C3 '\?\.\w+!' apps/frontend/Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for optional chaining followed by non-null assertion in TS/TSX files
rg -nP -g '*.ts' -g '*.tsx' -C3 '\?\.\w+!' apps/frontend/Length of output: 3412
Refactor/remove optional chaining with non-null assertions in AddLiquidity.tsx.
Instances at lines 47, 51, 55, 64, 89 and 115 use patterns like foo?.bar! that bypass TypeScript’s null-safety. Either explicitly handle potential undefined values (e.g. via checks or default values) or remove the global override for @typescript-eslint/no-non-null-asserted-optional-chain so these unsafe patterns remain flagged.
🤖 Prompt for AI Agents
In apps/frontend/eslint.config.mjs around lines 22-24, the rule disabling
"@typescript-eslint/no-non-null-asserted-optional-chain" allows unsafe
`foo?.bar!` patterns; remove this global override so the linter will flag them,
then update AddLiquidity.tsx at lines 47, 51, 55, 64, 89, and 115 to eliminate
`?....!` usage by either adding explicit null/undefined checks, using safe
default values, or restructuring code to guarantee defined values before access
(do not reintroduce non-null assertions).
2 participants
Summary by CodeRabbit
New Features
Refactor
Style
Chores