-
Notifications
You must be signed in to change notification settings - Fork 619
Nebula: Add Move funds page #7298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThis update introduces a new "Move Funds" feature in the Nebula app, including UI components for connecting wallets, selecting tokens, and transferring funds. It adds new React components, extends button and input props for enhanced control, updates middleware logic to allow unauthenticated access to the move-funds page, and modifies component usage of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MoveFundsPage
participant MoveFundsConnectButton
participant Wallet
participant SendFunds
participant TokenAPI
participant TransactionAPI
User->>MoveFundsPage: Visit /move-funds
MoveFundsPage->>MoveFundsConnectButton: Render connect button
User->>MoveFundsConnectButton: Click connect
MoveFundsConnectButton->>Wallet: Initiate wallet connection/auth
Wallet-->>MoveFundsConnectButton: Connection result
MoveFundsConnectButton-->>MoveFundsPage: Update connection status
alt Connected
MoveFundsPage->>SendFunds: Render send funds form
User->>SendFunds: Select wallet, chain, token, enter details
SendFunds->>TokenAPI: Fetch available tokens and balances
TokenAPI-->>SendFunds: Return token list
User->>SendFunds: Submit form
SendFunds->>TransactionAPI: Prepare and send transaction
TransactionAPI-->>SendFunds: Transaction result
SendFunds-->>User: Show success/error notification
else Not connected
MoveFundsPage-->>User: Show connect button
end
Suggested labels
Suggested reviewers
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7298 +/- ##
=======================================
Coverage 55.57% 55.57%
=======================================
Files 909 909
Lines 58673 58673
Branches 4158 4158
=======================================
Hits 32607 32607
Misses 25959 25959
Partials 107 107
🚀 New features to boost your workflow:
|
size-limit report 📦
|
c9243c8 to
d861fcb
Compare
There was a problem hiding this 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
🔭 Outside diff range comments (1)
apps/dashboard/src/components/buttons/MismatchButton.tsx (1)
84-91: 🛠️ Refactor suggestion
disableNoFundsPopupshould be optional for backward compatibilityMaking the prop mandatory forces every existing call-site (outside this PR) to be updated, otherwise TS will break the build.
Declare it optional and default it tofalsewhen destructuring:-type MistmatchButtonProps = React.ComponentProps<typeof Button> & { +type MistmatchButtonProps = React.ComponentProps<typeof Button> & { txChainId: number; isLoggedIn: boolean; isPending: boolean; checkBalance?: boolean; client: ThirdwebClient; - disableNoFundsPopup: boolean; + disableNoFundsPopup?: boolean; };-const { +const { ... checkBalance = true, + disableNoFundsPopup = false, ...buttonProps } = props;
🧹 Nitpick comments (5)
apps/dashboard/src/app/nebula-app/move-funds/page.tsx (1)
7-7: Consider renaming for clarity.The component name
RecoverPagemight be unclear since it's primarily for moving funds. ConsiderMoveFundsPageorMoveFundsLayoutto better reflect its purpose.apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx (1)
38-39:theme === "light"fallback mis-handles"system"
next-themesreturns"system"until it resolves the real theme. Treating every non-"light"value as dark causes a flash of the dark SDK theme on a light-system device.-getSDKTheme(theme === "light" ? "light" : "dark") +getSDKTheme(theme === "light" ? "light" : theme === "dark" ? "dark" : "light")apps/dashboard/src/components/buttons/MismatchButton.tsx (1)
202-205: Minor:typecan becomeundefinedWhen neither
showSwitchChainPopovernorshowNoFundsPopupis true and the caller hasn’t providedtype,buttonProps.typeisundefined, yielding an invalid DOM attribute (type="undefined"). The browser will disable form submission.Safest is to default to
"button":- type={ - showSwitchChainPopover || showNoFundsPopup - ? "button" - : buttonProps.type - } + type={ + showSwitchChainPopover || showNoFundsPopup + ? "button" + : buttonProps.type ?? "button" + }apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (2)
439-453: No balance-vs-amount validationA user can enter an amount greater than their balance and the form will happily attempt the transfer, only to fail async.
A quick client-side check improves UX:+const balanceQuery = useWalletBalance({ + address: props.accountAddress, + chain: defineChain(form.watch("chainId")), + client: dashboardClient, + tokenAddress: + getAddress(form.watch("token")?.token_address ?? "") === + getAddress(NATIVE_TOKEN_ADDRESS) + ? undefined + : form.watch("token")?.token_address, +}); ... -<TransactionButton ... disabled={sendAndConfirmTransaction.isPending}> +<TransactionButton + ... + disabled={ + sendAndConfirmTransaction.isPending || + (balanceQuery.data && + toWei(form.watch("amount"), balanceQuery.data.decimals) > + balanceQuery.data.value) + } +>
665-670: Hard-coded Insight URL duplicates env logicThe base host is re-assembled inline (
thirdwebvsthirdweb-dev). Centralise this in a util/env var to avoid drift and make future host changes trivial.E.g.
const INSIGHT_HOST = process.env.NEXT_PUBLIC_INSIGHT_HOST ?? (isProd ? "https://insight.thirdweb.com" : "https://insight.thirdweb-dev.com"); ... const url = new URL("/v1/tokens", INSIGHT_HOST);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/dashboard/src/@/components/ui/decimal-input.tsx(2 hunks)apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx(1 hunks)apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx(1 hunks)apps/dashboard/src/app/nebula-app/move-funds/page.tsx(1 hunks)apps/dashboard/src/components/buttons/MismatchButton.tsx(5 hunks)apps/dashboard/src/components/buttons/TransactionButton.tsx(3 hunks)apps/dashboard/src/middleware.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/dashboard/src/app/nebula-app/move-funds/page.tsx (3)
apps/dashboard/src/app/nebula-app/(app)/icons/NebulaIcon.tsx (1)
NebulaIcon(1-24)apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx (1)
MoveFundsConnectButton(28-49)apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (1)
MoveFundsPage(81-132)
apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx (1)
apps/dashboard/src/@/constants/thirdweb-client.client.ts (1)
getClientThirdwebClient(3-11)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
apps/dashboard/src/@/components/ui/decimal-input.tsx (1)
8-8: LGTM! Clean implementation of placeholder support.The optional
placeholderprop is correctly added to the interface and properly passed to the underlyingInputcomponent.Also applies to: 17-17
apps/dashboard/src/middleware.ts (1)
55-59: LGTM! Proper implementation of unauthenticated access.The middleware correctly excludes the "move-funds" path from requiring authentication, which aligns with the new wallet connection flow where users need to access the page before logging in.
apps/dashboard/src/app/nebula-app/move-funds/page.tsx (1)
9-35: LGTM! Well-structured page layout.The component structure is clean and well-organized:
- Proper header with branding and navigation elements
- Responsive support link visibility (hidden on mobile)
- Clean integration of MoveFundsConnectButton and MoveFundsPage components
apps/dashboard/src/components/buttons/TransactionButton.tsx (1)
35-35: LGTM! Proper prop forwarding with explicit default handling.The
disableNoFundsPopupprop is correctly added to the interface and properly forwarded toMismatchButtonwith explicit default value handling to ensure consistent behavior.Also applies to: 47-47, 73-75
apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (1)
241-260:⚠️ Potential issueERC-20 amount sent without decimal conversion
transfer({ amount: values.amount, … })expects the wei-denominated quantity (or a formatted string recognised by the SDK).
Passing the human-readable value (“1.5”) for a token with, say, 6 decimals will send1.5 wei, not1.5 × 10^6units.Convert using the token’s
decimals:-import { transfer } from "thirdweb/extensions/erc20"; +import { transfer } from "thirdweb/extensions/erc20"; +import { toUnits } from "thirdweb"; // new import ... - tx = transfer({ - to: values.receiverAddress, - amount: values.amount, - contract: erc20Contract, - }); + tx = transfer({ + to: values.receiverAddress, + amount: toUnits(values.amount, values.token.decimals), + contract: erc20Contract, + });⛔ Skipped due to learnings
Learnt from: MananTank PR: thirdweb-dev/js#7081 File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/assets/create/create-token-page-impl.tsx:110-118 Timestamp: 2025-05-20T18:54:15.781Z Learning: In the thirdweb dashboard's token asset creation flow, the `transferBatch` function from `thirdweb/extensions/erc20` accepts the raw quantity values from the form without requiring explicit conversion to wei using `toUnits()`. The function appears to handle this conversion internally or is designed to work with the values in the format they're already provided.
d861fcb to
ea9e08d
Compare
ea9e08d to
584a9f8
Compare
There was a problem hiding this 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
♻️ Duplicate comments (2)
apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (2)
197-200:parseFloatloses precision for large-decimal ERC-20 valuesThis was raised earlier and left as-is, so just flagging as a reminder.
983-988: Handle non-2xx responses from Insight APISame issue flagged previously: network / 4xx / 5xx failures propagate as unhandled rejections.
🧹 Nitpick comments (3)
apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (3)
224-229: Dead code: remove the commented-out filter or implement the intended logic
filteredConnectedWalletscurrently just aliasesconnectedWallets; the original filtering forinApp/smartwallets is commented out.
Either restore the filter or drop the alias entirely to avoid future confusion.-const filteredConnectedWallets = connectedWallets; -// .filter( -// (w) => w.id === "inApp" || w.id === "smart", -// );; +// Keep only the affected wallet types (inApp & smart) +const filteredConnectedWallets = connectedWallets.filter( + (w) => w.id === "inApp" || w.id === "smart", +);
315-323: Unnecessary use of.values()iterator
for…of tokensis clearer and avoids the extra iterator object.-for (const { token, amount } of tokens.values()) { +for (const { token, amount } of tokens) {
700-738: Potential request-storm while the user types a custom token address
manualTokenQueryfires on every key-stroke once the input is a valid hex string, which can trigger dozens of RPC calls per second.
Debounce the query key or wrap the state update in asetTimeout/useDebouncehook to avoid rate-limiting issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/dashboard/src/@/components/ui/decimal-input.tsx(2 hunks)apps/dashboard/src/app/nebula-app/(app)/components/ChatPageLayout.tsx(2 hunks)apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx(1 hunks)apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx(1 hunks)apps/dashboard/src/app/nebula-app/move-funds/page.tsx(1 hunks)apps/dashboard/src/app/nebula-app/providers.tsx(0 hunks)apps/dashboard/src/components/buttons/MismatchButton.tsx(6 hunks)apps/dashboard/src/components/buttons/TransactionButton.tsx(3 hunks)apps/dashboard/src/middleware.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/src/app/nebula-app/providers.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/dashboard/src/@/components/ui/decimal-input.tsx
- apps/dashboard/src/middleware.ts
- apps/dashboard/src/app/nebula-app/move-funds/page.tsx
- apps/dashboard/src/app/nebula-app/(app)/components/ChatPageLayout.tsx
- apps/dashboard/src/components/buttons/TransactionButton.tsx
- apps/dashboard/src/app/nebula-app/move-funds/connect-button.tsx
- apps/dashboard/src/components/buttons/MismatchButton.tsx
🧰 Additional context used
🧠 Learnings (1)
apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (3)
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:255-277
Timestamp: 2025-06-06T23:47:55.100Z
Learning: The `transfer` function from `thirdweb/extensions/erc20` accepts human-readable amounts via the `amount` property and automatically handles conversion to base units (wei) by fetching the token decimals internally. Manual conversion using `toWei()` is not required when using the `amount` property.
Learnt from: MananTank
PR: thirdweb-dev/js#7081
File: apps/dashboard/src/app/(app)/team/[team_slug]/[project_slug]/assets/create/create-token-page-impl.tsx:110-118
Timestamp: 2025-05-20T18:54:15.781Z
Learning: In the thirdweb dashboard's token asset creation flow, the `transferBatch` function from `thirdweb/extensions/erc20` accepts the raw quantity values from the form without requiring explicit conversion to wei using `toUnits()`. The function appears to handle this conversion internally or is designed to work with the values in the format they're already provided.
Learnt from: MananTank
PR: thirdweb-dev/js#7298
File: apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx:424-424
Timestamp: 2025-06-06T23:46:08.785Z
Learning: The thirdweb project has an ESLint rule that restricts direct usage of `defineChain`. When it's necessary to use `defineChain` directly, it's acceptable to disable the rule with `// eslint-disable-next-line no-restricted-syntax`.
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Unit Tests
- GitHub Check: Build Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/dashboard/src/app/nebula-app/move-funds/move-funds.tsx (1)
249-259:toWei()assumes 18 decimals – may under/over-send on chains where the native token uses a different precision
toWei(amount)hard-codes 18 decimals. While most EVM natives are 18, that’s not guaranteed (e.g. Klaytn, Metis).
Consider derivingnativeDecimalsviagetWalletBalanceorchain.nativeCurrency.decimalsand usingtoWei(amount, nativeDecimals)(supported by thirdweb 4.1+) to stay future-proof.

PR-Codex overview
This PR enhances the functionality of the dashboard application by adding new components and improving existing ones, particularly around fund movement and wallet connections. It introduces new UI elements and modifies logic for better user experience during transactions.
Detailed summary
placeholderprop toDecimalInputcomponent.TWAutoConnectinChatPageLayout.TransactionButtonto includedisableNoFundsPopupprop.MoveFundsConnectButtonfor moving funds functionality.MoveFundsPagewith a structured UI for fund transfers.MoveFundsPage.TokenSelectorPopoverfor selecting tokens with manual address input.RenderTokenBalance.Summary by CodeRabbit
New Features
Bug Fixes