Skip to content

feat: add ERC20 funding token support to frontend (team qwerty / unstoppable hackathon )#17

Open
dehydrated-bear wants to merge 6 commits intoStabilityNexus:mainfrom
dehydrated-bear:feat/erc20-frontend-support
Open

feat: add ERC20 funding token support to frontend (team qwerty / unstoppable hackathon )#17
dehydrated-bear wants to merge 6 commits intoStabilityNexus:mainfrom
dehydrated-bear:feat/erc20-frontend-support

Conversation

@dehydrated-bear
Copy link
Copy Markdown

@dehydrated-bear dehydrated-bear commented Dec 13, 2025

pr for the issue:
#8

1)Create Vault Page:

Added radio buttons to choose between "Native ETH" or "ERC20 Token"

When ERC20 is selected, a new field appears to enter the token address

Form labels update automatically (e.g., "Minimum Token Target" instead of "Minimum ETH Target")

Validates that the ERC20 address is correct before allowing submission

2)Vault Details Page:

Shows the correct token symbol for whatever currency the vault is using

Progress bars display the right currency (cBTC, USDC, or whatever token)

Automatically detects if a vault uses native currency or an ERC20 token

3)Contribution Flow:

For ERC20 tokens: Users first approve the vault to spend their tokens, then contribute

For native cBTC: Works the same as before with direct payments

Button text updates to show which currency you're sending

4)New File:

Added erc20abi.json to interact with any ERC20 token

Summary by CodeRabbit

  • New Features

    • ERC20 token funding support: create vaults that accept ERC20 tokens with address validation and non-zero-address checks.
    • Vault details and purchase flows show funding token symbol and support ERC20 purchases with approve+buy flow; token decimals are auto-detected and displayed.
    • Form labels, placeholders and buttons adapt to selected currency; UI reflects loading and readiness states.
  • Bug Fixes / UX

    • Validation errors for invalid token addresses; deadline input enforces required date and stronger submission guards; waits for transaction receipts to improve reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 13, 2025

Warning

Rate limit exceeded

@dehydrated-bear has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 55 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f1ef8b2 and 9d67e77.

📒 Files selected for processing (1)
  • src/VaultActions.tsx (10 hunks)

Walkthrough

Adds ERC20 funding-token support across Create, Details, and VaultActions: validates and reads ERC20 addresses/decimals/symbols, conditionally renders ERC20 inputs, fetches token metadata, performs ERC20 approve→purchase flow, and preserves native (ETH) behavior.

Changes

Cohort / File(s) Change Summary
Form validation & creation
src/Create.tsx
Adds fundingToken + fundingTokenDecimals; enforces non-empty/valid/non-zero ERC20 address when ERC20 selected; uses usePublicClient + erc20abi to read decimals on token change; displays token decimals block; parses minimum target with parseUnits for ERC20 vs parseEther for native; includes token address and parsed amount in deploy call; tweaks deadline input to valueAsDate.
Vault details & display
src/Details.tsx
Reads vault fundingToken via useReadContract; conditionally reads ERC20 symbol via erc20abi; derives fundingCurrencySymbol and updates "Funds Collected" UI to show ERC20 symbol or native symbol.
Vault actions & purchasing
src/VaultActions.tsx
Adds reads for fundingToken, token symbol and decimals; determines isNativeCurrency and currencySymbol; branches purchase flow: native = send value; ERC20 = approve(vault, tokenAmount) then purchaseTokens(tokenAmount) using parseUnits; waits for transaction receipts; updates UI placeholders/labels/disabled states and guards on public client/account readiness.
ABI definition
src/abi/erc20abi.json
Adds standard ERC20 ABI JSON (events: Approval, Transfer; view funcs: allowance, balanceOf, decimals, name, symbol, totalSupply; state-changing funcs: approve, transfer, transferFrom, allowance helpers).

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as Frontend (React)
    participant RPC as Public Client / Node
    participant EC as ERC20 Contract
    participant VC as Vault Contract

    User->>UI: Initiate purchase (select ERC20 or native, submit)
    UI->>RPC: read vault.fundingToken
    RPC-->>UI: fundingToken
    alt fundingToken != zero (ERC20)
        UI->>RPC: read ERC20 decimals & symbol (erc20abi)
        RPC-->>UI: decimals, symbol
        UI->>EC: approve(vaultAddress, tokenAmount) (user signs)
        RPC-->>UI: approval tx receipt
        UI->>VC: purchaseTokens(tokenAmount) (user signs)
        RPC-->>UI: purchase tx receipt
    else native currency
        UI->>VC: purchaseTokens() with value (user signs)
        RPC-->>UI: purchase tx receipt
    end
    UI-->>User: show completion or error
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pay attention to ERC20 approval → purchase sequencing, receipt-waiting and error paths in src/VaultActions.tsx.
  • Verify address validation and zero-address enforcement plus decimals fetch in src/Create.tsx.
  • Confirm conditional read/display logic and symbol fallback in src/Details.tsx.
  • Check ABI usage and parameter/units conversions across calls (parseUnits vs parseEther).

Possibly related issues

Poem

🐰 I sniffed the token, checked its little cents,

Approved a vault with tiny, careful hops,
Parsed units, signed, and watched the ledger tense —
Carrots or ETH, the funding never stops. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding ERC20 funding token support to the frontend. It directly reflects the core feature introduced across Create, Details, and VaultActions components.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/Details.tsx (1)

222-244: Critical: ERC20 vault balance displayed incorrectly.

For ERC20-funded vaults, balanceOfVault?.data?.value returns the vault's native currency balance, not the ERC20 token balance. This will show incorrect "Funds Collected" amounts for ERC20 vaults.

You need to fetch the ERC20 token balance separately using balanceOf(vaultAddress) on the funding token contract when !isNativeCurrency.

Example fix approach:

+ // Fetch ERC20 token balance if a funding token is set
+ const { data: erc20Balance } = useReadContract({
+   abi: erc20abi,
+   address: fundingToken,
+   functionName: "balanceOf",
+   args: [address],
+   chainId: citreaTestnet.id,
+   query: {
+     enabled: !!fundingToken && fundingToken !== "0x0000000000000000000000000000000000000000" && !!address,
+   },
+ }) as { data: bigint | undefined };
+
+ // Use ERC20 balance for ERC20 vaults, native balance otherwise
+ const displayedBalance = isNativeCurrency 
+   ? balanceOfVault?.data?.value 
+   : erc20Balance;

Then use displayedBalance in the UI instead of balanceOfVault?.data?.value.

src/Create.tsx (1)

101-119: Funding type radio buttons are outside the form.

Similar to the ERC20 input, the funding type radio buttons are outside the <form> element. Move them inside for proper form semantics.

🧹 Nitpick comments (4)
src/Details.tsx (1)

83-86: Consider extracting zero address to a constant.

The zero address string is repeated across multiple files. Extract to a shared constant for maintainability.

// In a shared constants file
export const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" as const;
src/VaultActions.tsx (2)

96-98: Fixed timeout is unreliable for transaction confirmation.

Using setTimeout(resolve, 6000) doesn't guarantee block confirmations. Network congestion or slow blocks could cause the second transaction to fail if approval isn't confirmed yet.

Consider using wagmi's waitForTransactionReceipt for reliable confirmation:

import { waitForTransactionReceipt } from '@wagmi/core';

// After approval tx
await waitForTransactionReceipt(config, { 
  hash: approveTx,
  confirmations: 1 
});

Also applies to: 113-114, 124-125


99-127: Consider checking existing allowance before approving.

If the user already has sufficient allowance, the approval step is unnecessary and wastes gas. You could check allowance(userAddress, vaultAddress) first.

src/Create.tsx (1)

56-56: Hardcoded contract addresses should be extracted to constants.

Factory address 0x7be12F651D421Edf31fd6488244aC20e8cEb5987 and developer address 0x1bAab7d90eceB510f9424a41A86D9eA5ADce8717 are hardcoded. Consider extracting these to a configuration file for easier maintenance and environment switching.

Also applies to: 66-66, 76-76

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b816112 and c88a687.

📒 Files selected for processing (4)
  • src/Create.tsx (8 hunks)
  • src/Details.tsx (4 hunks)
  • src/VaultActions.tsx (6 hunks)
  • src/abi/erc20abi.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/VaultActions.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
src/Details.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
🔇 Additional comments (4)
src/Details.tsx (1)

39-59: ERC20 funding token fetching looks correct.

The conditional fetching of fundingToken and fundingTokenSymbol with proper guard conditions is well implemented.

src/VaultActions.tsx (1)

252-270: Good UX: Dynamic currency symbol in button text.

The button and placeholder text correctly reflect the funding currency being used.

src/Create.tsx (1)

27-36: Validator closure may have stale fundingType value.

validateFundingToken(watch("fundingType")) captures fundingType at registration time, not at validation time. If the user switches from ERC20 to ETH after entering an invalid address, validation might behave unexpectedly.

Consider using a validation function that reads fundingType dynamically:

const validateFundingToken = (value?: `0x${string}`, formValues?: Inputs) => {
  if (formValues?.fundingType !== "ERC20") return true;
  // ... rest of validation
};

// In register:
{...register("fundingToken", {
  validate: (value, formValues) => validateFundingToken(value, formValues),
})}
src/abi/erc20abi.json (1)

1-283: Standard ERC20 ABI looks good.

The ABI includes all necessary functions for the ERC20 interactions used in this PR (symbol, decimals, approve, balanceOf). The inclusion of decreaseAllowance/increaseAllowance from OpenZeppelin is fine as they're commonly available.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/VaultActions.tsx (1)

25-63: Use native currency symbol (not name) and add a fallback when ERC20 symbol() is missing/reverts.
Many chains have name: "Ether" but symbol: "ETH". Also arbitrary ERC20s sometimes don’t implement symbol() cleanly—blocking the UI on currencySymbol can dead-end users.

-  const nativecurrency = account.chain?.nativeCurrency.name;
+  const nativecurrency = account.chain?.nativeCurrency.symbol;
@@
-  const currencySymbol = isNativeCurrency ? nativecurrency : tokenSymbol;
+  const currencySymbol = isNativeCurrency ? nativecurrency : (tokenSymbol ?? "TOKEN");
src/Create.tsx (2)

51-86: Make the decimals fetch conditional on ERC20 funding type (avoid extra RPC + stale decimals).
Right now, decimals can still be fetched/retained even if the user switches back to “ETH”.

-  const fundingTokenAddress = watch("fundingToken");
+  const fundingTokenAddress = watch("fundingToken");
+  const fundingType = watch("fundingType");
@@
-      if (!fundingTokenAddress || !isAddress(fundingTokenAddress) || fundingTokenAddress === "0x0000000000000000000000000000000000000000") {
+      if (fundingType !== "ERC20") {
+        setValue("fundingTokenDecimals", "");
+        return;
+      }
+      if (!fundingTokenAddress || !isAddress(fundingTokenAddress) || fundingTokenAddress === "0x0000000000000000000000000000000000000000") {
         setValue("fundingTokenDecimals", "");
         return;
       }
@@
-  }, [fundingTokenAddress, publicClient, setValue]);
+  }, [fundingType, fundingTokenAddress, publicClient, setValue]);

410-420: valueAsDate is a good improvement; confirm contract expects local-midnight semantics.
<input type="date"> + valueAsDate yields a Date at local midnight; converting to unix seconds can shift by timezone vs “end of day”.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c88a687 and ba7ac6b.

📒 Files selected for processing (2)
  • src/Create.tsx (8 hunks)
  • src/VaultActions.tsx (6 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/VaultActions.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
src/Create.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
🔇 Additional comments (3)
src/VaultActions.tsx (1)

280-322: Good: refund/redeem now gates on wallet connection.
Disabling these actions when !account.address avoids confusing failures.

src/Create.tsx (2)

29-38: ERC20 token address validation + “inside form” placement look good.
Good enforcement of non-zero valid addresses, and moving the block inside <form> fixes semantics/accessibility.

Also applies to: 163-194


321-331: UI label/placeholder switching for ERC20 vs native looks consistent.
The dynamic “Minimum Token/ETH Target” and exchange-rate placeholder reduce user error.

Also applies to: 385-396

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/Create.tsx (1)

127-129: Replace setTimeout with proper transaction receipt confirmation.

Using a fixed 6-second delay doesn't guarantee the approval transaction is confirmed before proceeding. Network conditions vary, and this could cause the second transaction to fail if the approval isn't mined yet.

+  const publicClient = usePublicClient({ chainId: citreaTestnet.id });
   // ...
   const tx1 = await writeContractAsync({
     // approval call
   });
-  // Wait for approximately 6 seconds for 3 block confirmations
-  await new Promise((resolve) => setTimeout(resolve, 6000));
+  // Wait for approval transaction to be confirmed
+  if (publicClient) {
+    await publicClient.waitForTransactionReceipt({
+      hash: tx1 as `0x${string}`,
+      confirmations: 1,
+    });
+  }
   console.log("1st Transaction submitted:", tx1);
src/VaultActions.tsx (3)

168-202: onSubmitForm2 (Add PKT) has unnecessary ERC20 guards and uses parseEther unconditionally.

This handler adds Proof-of-Funding tokens (not funding tokens), so the ERC20 decimals/symbol guards at lines 176-179 are unnecessary. Additionally, it always uses parseEther at line 191 which is correct for PTA tokens (assuming 18 decimals), but the guards create confusion about the intent.

Consider simplifying the guards to only check publicClient availability:

 const onSubmitForm2: SubmitHandler<Inputs> = async (data) => {
-  // Guard: Ensure fundingToken has loaded and we have required data
-  if (fundingTokenLoading || !fundingToken) {
-    console.error("Funding token not loaded yet");
-    return;
-  }
-
-  // Guard: For ERC20 tokens, ensure decimals and symbol are loaded
-  if (!isNativeCurrency && (tokenDecimals === undefined || tokenSymbol === undefined)) {
-    console.error("Token data not loaded yet");
-    return;
-  }
-
   if (!publicClient) {
     console.error("Public client not available");
     return;
   }

208-222: Remaining handlers still use setTimeout(6000) instead of transaction receipt confirmation.

onSubmitForm3, handleWithdraw, handleRefund, and handleRedeem all use the fixed 6-second delay pattern. For consistency and reliability, these should use waitForTransactionReceipt like the updated onSubmitForm1 and onSubmitForm2.

Example fix for handleWithdraw:

 const handleWithdraw = async () => {
+  if (!publicClient) {
+    console.error("Public client not available");
+    return;
+  }
   try {
     const tx1 = await writeContractAsync({
       abi: vaultabi,
       address: address as `0x${string}`,
       functionName: "withdrawFunds",
       chainId: citreaTestnet.id,
     });
-    // Wait for approximately 6 seconds for 3 block confirmations
-    await new Promise((resolve) => setTimeout(resolve, 6000));
+    const receipt = await publicClient.waitForTransactionReceipt({
+      hash: tx1 as `0x${string}`,
+    });
-    console.log("1st Transaction submitted:", tx1);
+    console.log("Withdraw transaction confirmed:", receipt);
   } catch (error) {
     console.error("Contract call failed:", error);
   }
 };

Apply the same pattern to onSubmitForm3, handleRefund, and handleRedeem.

Also applies to: 225-238, 241-254, 256-269


330-351: Semantic HTML issue: <p> wrapping a <div> is invalid.

The "Refund" tab content has <p> as the outer element containing a <div>, which is invalid HTML (block element inside inline element). Same issue exists in the "Redeem" tab (lines 353-372).

 {activeTab === "Refund" && (
-  <p>
-    <div>
+  <div>
       <p className="">
         Get a refund if the project did not manage to raise the
         minimum amount by the deadline
       </p>
       <button ...>
         ...
       </button>
-    </div>
-  </p>
+  </div>
 )}

Apply the same fix to the "Redeem" tab.

🧹 Nitpick comments (2)
src/Create.tsx (2)

117-126: Hardcoded factory contract address should be extracted to a constant.

The factory address 0x7be12F651D421Edf31fd6488244aC20e8cEb5987 is duplicated in the approval call and deployment call. Extract to a named constant for maintainability and to reduce risk of typos if it needs to change.

+const FACTORY_ADDRESS = "0x7be12F651D421Edf31fd6488244aC20e8cEb5987" as const;
+
 const Create = () => {
   // ...
   const tx1 = await writeContractAsync({
     abi: abi,
     address: data.pta,
     functionName: "approve",
     args: [
-      "0x7be12F651D421Edf31fd6488244aC20e8cEb5987",
+      FACTORY_ADDRESS,
       parseEther(data.ptaAmount),
     ],
     // ...
   });
   // ...
   const tx2 = await writeContractAsync({
     abi: factoryabi,
-    address: "0x7be12F651D421Edf31fd6488244aC20e8cEb5987",
+    address: FACTORY_ADDRESS,
     functionName: "deployFundingVault",
     // ...
   });

159-161: Remove unnecessary empty lines.

Minor: There are two consecutive empty lines at the start of the return statement that affect code cleanliness.

   return (
-
-
     <div className="mx-auto max-w-7xl p-5">
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba7ac6b and 08b6438.

📒 Files selected for processing (2)
  • src/Create.tsx (9 hunks)
  • src/VaultActions.tsx (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Create.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
🔇 Additional comments (7)
src/Create.tsx (4)

29-38: Well-structured custom validator for ERC20 token address.

The validator properly checks for ERC20 mode, validates address format using isAddress, and rejects the zero address. This is a clean implementation.


54-86: Good decimals fetching implementation with proper guards.

The useEffect correctly handles edge cases: clearing decimals for invalid/zero addresses, guarding against missing publicClient, and catching errors. This addresses the previous review concern about fundingTokenDecimals not being populated.


91-115: Proper ERC20 decimals validation and parsing implemented.

The code now correctly validates decimals presence for ERC20 mode and uses parseUnits with the token's actual decimals instead of hardcoding 18. This addresses the critical issue from the previous review about incorrect scaling for tokens like USDC.


189-219: ERC20 input correctly placed inside form with proper validation display.

The ERC20 funding token block is now inside the <form> element as recommended in the previous review. The conditional rendering, validation error display, and decimals feedback are well implemented.

src/VaultActions.tsx (3)

28-67: Solid implementation of funding token detection and ERC20 metadata fetching.

The code properly:

  • Tracks loading state for fundingToken
  • Conditionally enables ERC20 reads only when a non-zero funding token exists
  • Determines isNativeCurrency only after loading completes

This addresses the previous review concern about premature native currency detection.


91-161: Well-implemented dual-path purchase flow with proper transaction confirmations.

The ERC20 flow correctly:

  1. Guards against incomplete data loading
  2. Uses parseUnits with actual token decimals
  3. Performs approve→purchase with waitForTransactionReceipt between steps

This addresses previous critical concerns about decimals usage and race conditions.


396-409: Inconsistent disabled logic in "Add PKT" tab uses nativecurrency instead of currencySymbol.

Lines 403 and 408 use nativecurrency while the "Fund Project" tab uses currencySymbol. Since "Add PKT" adds Proof-of-Funding tokens (not related to the funding currency), using nativecurrency is likely intentional. However, the button disabled condition at lines 407-410 includes ERC20 checks that seem out of place for this action.

Verify the intended behavior: should "Add PKT" be gated on ERC20 token data loading, or should it only require wallet connection?

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/VaultActions.tsx (1)

290-321: Disable “Send” when wallet is disconnected and while submitting (prevents double-submit).
Currently currencySymbol can be truthy for ERC20 even without a connected wallet, and the button isn’t disabled during isSubmitting.

               <button
                 disabled={
                   fundingTokenLoading ||
+                  !account.address ||
+                  isSubmitting ||
                   !currencySymbol ||
                   (!isNativeCurrency && (tokenDecimals === undefined || tokenSymbol === undefined))
                 }
...
                 <span className="text-white">
-                  {fundingTokenLoading
+                  {!account.address
+                    ? "Connect Wallet"
+                    : fundingTokenLoading
                     ? "Loading..."
                     : currencySymbol
                       ? `${isSubmitting ? "Processing..." : `Send ${currencySymbol}`}`
-                      : "Connect Wallet"}
+                      : "Loading currency..."}
                 </span>
               </button>
🧹 Nitpick comments (2)
src/VaultActions.tsx (2)

25-68: isNativeCurrency should not treat “missing fundingToken” as native; handle “unknown/error” explicitly.
Right now !fundingToken makes isNativeCurrency true (Line 64) but onSubmitForm1 rejects !fundingToken (Line 93), which can enable confusing UI states when the read fails or is disabled.

+  const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000" as const;
+
   // Determine if using native currency or ERC20 token
   // Only consider native currency AFTER fundingToken has loaded and is zero address
-  const isNativeCurrency =
-    !fundingTokenLoading && (!fundingToken || fundingToken === "0x0000000000000000000000000000000000000000");
+  const isFundingTokenKnown = !fundingTokenLoading && fundingToken !== undefined;
+  const isNativeCurrency = isFundingTokenKnown && fundingToken === ZERO_ADDRESS;

   // currencySymbol is only valid when all data has loaded
   const currencySymbol = isNativeCurrency ? nativecurrency : tokenSymbol;

202-264: Prefer receipt waits over setTimeout(6000) for remaining actions too (Withdraw/Refund/Redeem/WithdrawUnsoldTokens).
You already standardized on publicClient.waitForTransactionReceipt elsewhere; these are still timing-based and can misreport success/failure.

-      // Wait for approximately 6 seconds for 3 block confirmations
-      await new Promise((resolve) => setTimeout(resolve, 6000));
-      console.log("1st Transaction submitted:", tx1);
+      if (!publicClient) throw new Error("Public client not available");
+      const receipt = await publicClient.waitForTransactionReceipt({ hash: tx1 as `0x${string}` });
+      console.log("Transaction confirmed:", receipt);
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 08b6438 and 6a92c59.

📒 Files selected for processing (1)
  • src/VaultActions.tsx (8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/VaultActions.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
🔇 Additional comments (1)
src/VaultActions.tsx (1)

3-7: Good: ERC20 ABI + parseUnits import aligns with multi-decimals tokens.

…mation

- Replace all arbitrary setTimeout delays with waitForTransactionReceipt
- Add publicClient null guards to all transaction handlers
- Rename variables for clarity (txHash, approveHash, purchaseHash)
- Consistent error logging across all vault actions
- Remove 'submitted' logs, keep only 'confirmed' status
- All 8 writeContractAsync calls properly typed and handled
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/VaultActions.tsx (2)

382-406: Refund and Redeem buttons use wrong loading state.

Both buttons reference isSubmitting from the Fund Project form (line 88). This causes:

  1. When Fund Project form is submitting, these buttons incorrectly show "Processing..."
  2. When user clicks Refund/Redeem, no loading indicator appears

Add dedicated loading states for these actions:

+ const [isRefunding, setIsRefunding] = useState(false);
+ const [isRedeeming, setIsRedeeming] = useState(false);

  const handleRefund = async () => {
+   setIsRefunding(true);
    // ... existing code ...
+   setIsRefunding(false);
  };

Then update button text:

- {isSubmitting ? "Processing..." : `Refund`}
+ {isRefunding ? "Processing..." : `Refund`}

449-471: Withdraw Unsold PTK button lacks disabled state.

The input is disabled when wallet is disconnected (via nativecurrency check), but the submit button (line 464) has no disabled attribute. Users can still click submit with empty/disabled input.

- <button className="flex h-[34px] ...">
+ <button
+   disabled={!account.address}
+   className="flex h-[34px] ...">
♻️ Duplicate comments (1)
src/VaultActions.tsx (1)

216-220: Missing wallet connection guard in onSubmitForm3.

Unlike onSubmitForm1 and onSubmitForm2, this handler doesn't verify account.address before attempting to write. While the button is disabled when wallet isn't connected (line 462 uses nativecurrency), the submit handler should include defensive guards for consistency and safety.

  const onSubmitForm3: SubmitHandler<Inputs> = async (data) => {
+   if (!account.address) {
+     console.error("Wallet not connected");
+     return;
+   }
+
    if (!publicClient) {
      console.error("Public client not available");
      return;
    }
🧹 Nitpick comments (1)
src/VaultActions.tsx (1)

196-199: Acknowledge: Participation token decimals are hardcoded.

The TODO correctly flags that parseEther assumes 18 decimals for participation tokens. If the vault's participation token uses different decimals, this will cause incorrect amounts.

Consider fetching participation token decimals similar to the funding token pattern:

+ // Fetch participation token address and decimals
+ const { data: participationToken } = useReadContract({
+   abi: vaultabi,
+   address: address as `0x${string}`,
+   functionName: "token", // or appropriate function name
+   chainId: citreaTestnet.id,
+   query: { enabled: !!address },
+ });
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a92c59 and f1ef8b2.

📒 Files selected for processing (1)
  • src/VaultActions.tsx (9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/VaultActions.tsx (1)
src/CitreaTestnet.ts (1)
  • citreaTestnet (3-13)
🔇 Additional comments (5)
src/VaultActions.tsx (5)

26-37: Good fix: Native currency symbol is now correctly used.

The change from .name to .symbol on line 26 correctly displays "cBTC" instead of the full currency name. The fundingToken hook now properly tracks loading state for UI gating.


61-67: Currency determination logic is correctly implemented.

The isNativeCurrency check now properly waits for fundingTokenLoading to be false before evaluating, preventing premature native currency path selection. The currencySymbol derivation correctly falls back between native and ERC20.


91-119: Comprehensive guards are well-implemented.

All necessary preconditions are checked: wallet connection, vault address, funding token loading, ERC20 token data (when applicable), and public client availability. This prevents transactions from being submitted in invalid states.


136-168: ERC20 flow correctly implements approve-then-purchase pattern with proper receipt waiting.

The implementation now uses parseUnits with actual token decimals (addressing the previous issue), waits for transaction receipts instead of using setTimeout, and properly sequences the approve → purchaseTokens calls.


347-365: Fund Project button states are well-implemented.

The disabled conditions comprehensively check wallet connection, route params, funding token loading, currency symbol, and ERC20 token data. Button text properly reflects all loading and connection states.

@dehydrated-bear
Copy link
Copy Markdown
Author

@Zahnentferner please review this , i have done the backend and the frontend for the bene project and i have completed this task :

image

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