-
Notifications
You must be signed in to change notification settings - Fork 12
PillarWallet Testing #455
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
base: staging
Are you sure you want to change the base?
PillarWallet Testing #455
Conversation
WalkthroughAdds eruda and initializes a client-side debug console, injects extensive runtime logging across Pulse buy/home components, migrates Intent SDK initialization to Transaction Kit (hard-coding pulse node URL), and replaces module-install flow with a no-op that marks modules as installed. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / Components
participant useIntentSdk as useIntentSdk Hook
participant TxKit as Transaction Kit
participant Etherspot as Etherspot Provider
participant Intent as Intent SDK
App->>useIntentSdk: mount / useEffect
useIntentSdk->>TxKit: useTransactionKit()
TxKit-->>useIntentSdk: kit, accountAddress
alt kit available
useIntentSdk->>Etherspot: kit.getEtherspotProvider()
Etherspot-->>useIntentSdk: walletProvider
useIntentSdk->>useIntentSdk: log progress
useIntentSdk->>Intent: initialize (provider/client, hard-coded node URL)
Intent-->>useIntentSdk: intentSdk instance
useIntentSdk->>App: intentSdk ready (logged)
else kit unavailable
useIntentSdk->>useIntentSdk: log error / early return
useIntentSdk-->>App: intentSdk not initialized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📚 Learning: 2025-08-12T07:42:24.656ZApplied to files:
📚 Learning: 2025-08-20T09:14:16.888ZApplied to files:
📚 Learning: 2025-11-04T14:34:00.373ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (3)
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 |
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: 2
🧹 Nitpick comments (8)
package.json (1)
120-120: Confirm eruda placement vs deploy pipeline
erudais imported at runtime insrc/main.tsxbut currently only listed underdevDependencies. If your build/deploy pipeline installs only production deps (e.g.npm ci --only=prod), this can break builds due to a missing module. Consider either:
- Moving
erudato"dependencies"(and gating its usage by env), or- Ensuring your build step always installs devDependencies in this app repo.
src/main.tsx (1)
6-17: Gate Eruda debug console to non‑production / opt‑in environmentsRight now
eruda.init()runs in every browser environment, so the debug console ships to users as well. To avoid impacting production UX and to keep control over when debugging is enabled, consider gating this:-import eruda from 'eruda'; +import eruda from 'eruda'; // Initialize Eruda debug console for mobile debugging -if (typeof window !== 'undefined') { - eruda.init(); - console.log('Eruda debug console initialized - look for floating button on screen'); -} +if ( + typeof window !== 'undefined' && + (import.meta.env.DEV || import.meta.env.VITE_ENABLE_ERUDA === 'true') +) { + eruda.init(); + console.log( + 'Eruda debug console initialized - look for floating button on screen' + ); +}This keeps Eruda available for local/staging debugging without forcing it on every production user.
src/apps/pulse/hooks/useIntentSdk.ts (2)
109-112: Avoid usingalert()inside a shared hookUsing
alert('Failed to initialize Intent SDK. Please try again.' + err);inside a hook is intrusive and can fire multiple times (e.g. React 18 StrictMode double effect invocation) or in background flows. You already expose anerrorstate, which is better for surfacing issues via UI.Consider removing the
alertand relying solely on state and console logging:- } catch (err) { - alert('Failed to initialize Intent SDK. Please try again.' + err); - console.error('Failed to initialize Intent SDK:', err); - setError('Failed to initialize Intent SDK. Please try again.'); - } + } catch (err) { + console.error('Failed to initialize Intent SDK:', err); + setError('Failed to initialize Intent SDK. Please try again.'); + }
12-21: Large blocks of commented‑out Privy/WalletConnect logic add noiseA lot of the previous Privy / WalletConnect initialization paths are now commented out. Unless you expect to re‑enable them soon, consider either:
- Removing them entirely (git history will preserve the old implementation), or
- Wrapping them in a clearly marked fallback block with a TODO explaining under what conditions they should be restored.
This will keep the hook focused on the Transaction Kit path and reduce cognitive load for future readers.
Also applies to: 36-52, 53-89, 118-122
src/apps/pulse/components/Buy/Buy.tsx (1)
183-202: Gate verbose Buy‑flow logging behind a debug flagThe new logs in
handleBuySubmit, the debounced USD effect, andrefreshBuyIntent(including full intent/response dumps) are great for diagnosing issues, but they’re fairly chatty and include wallet‑related amounts and responses. For ongoing development this is fine; for staging/production it may be noisy.Consider wrapping these logs in a simple runtime guard so they can be enabled only when needed, e.g.:
const isPulseDebugEnabled = import.meta.env.DEV || import.meta.env.VITE_PULSE_DEBUG === 'true'; if (isPulseDebugEnabled) { console.log('=== BUY: refreshBuyIntent called ==='); console.log('intentSdk:', intentSdk); // ... }You can reuse the same flag across Buy, HomeScreen, and related hooks to keep logging consistent and easy to toggle.
Also applies to: 221-234, 267-304, 308-315, 350-371
src/apps/pulse/components/App/HomeScreen.tsx (1)
105-108: HomeScreen debug logging should also be gatedThe new logs around
previewBuychanges,handleRefresh, andrenderPrevieware useful for tracing flows but will add a lot of noise in the console over time (especially with the 15‑second auto‑refreshes).Like in
Buy.tsx, consider centralizing this behind a flag:const isPulseDebugEnabled = import.meta.env.DEV || import.meta.env.VITE_PULSE_DEBUG === 'true'; useEffect(() => { if (isPulseDebugEnabled) { console.log('=== HOMESCREEN: previewBuy state changed to:', previewBuy); } }, [previewBuy]);and similarly wrap the other
console.logcalls so you can turn them on/off per environment without code changes.Also applies to: 293-358, 911-916
src/apps/pulse/hooks/useModularSdk.ts (1)
31-31:areModulesInstalledis now effectively hard‑wired totrue, making the module status check dead codeWith the state initialized as:
const [areModulesInstalled, setAreModulesInstalled] = useState<boolean>(true);and your check effect guarded by
if (!areModulesInstalled && modularSdk && payingTokens.length > 0), there is currently no path that setsareModulesInstalledtofalsefrom outside that effect. As a result:
- The
useEffectat lines 124–153 never runs for a fresh mount (the condition is false on first render).modularSdk.pulse.isPulseModulesInstalled(...)is never called.isFetchingis never toggled by the module‑status flow.- Callers (e.g.
BuyButtonexpecting afalsestate to show “Enable Trading…” and trigger install flows) will never seeareModulesInstalled === false.If the intent is to temporarily bypass module detection/installation and always treat modules as installed, it would be clearer to either:
- Remove or explicitly comment out the
isPulseModulesInstalledeffect, and document thatareModulesInstalledis forced totrueas a temporary workaround; or- Reintroduce a tri‑state/“unknown” value (e.g.
boolean | undefined) and drive the effect off “unknown”, so the status check still runs and can set the flag totrue/false.If, instead, you still rely on the “modules need installation” UI path and backend checks, this current shape is likely a functional regression and you’ll want to restore a path where
areModulesInstalledcan befalseand let the effect fire at least once.Also applies to: 124-153, 155-181
src/apps/pulse/components/Buy/BuyButton.tsx (1)
108-130:isDisabled()side effects (logging) are evaluated 3× per render, and the “modules need installation” branch may never fireTwo points here:
- Multiple calls per render with logging
isDisabled()is invoked for:
disabled={isDisabled()}backgroundColor: isDisabled() ? …- The last argument to
getButtonText(..., isDisabled())Each call runs the same logic and emits logs (
Button disabled: …,Final disabled state: …). That’s harmless functionally but makes logs noisy and less useful when debugging.A small refactor would make this cleaner:
- const isDisabled = () => { + const isDisabled = () => { if (isInstalling || isFetching) { console.log('Button disabled: installing or fetching'); return true; } if (notEnoughLiquidity) { console.log('Button disabled: not enough liquidity'); return true; } if (!areModulesInstalled && payingTokens.length > 0) { console.log('Button enabled: modules need installation'); return false; } const disabled = isLoading || !token || !(parseFloat(usdAmount) > 0) || !expressIntentResponse; console.log('Final disabled state:', disabled); return disabled; }; + + const disabled = isDisabled();Then in JSX:
- disabled={isDisabled()} + disabled={disabled} style={{ - backgroundColor: isDisabled() ? '#29292F' : '#8A77FF', - color: isDisabled() ? 'grey' : '#FFFFFF', + backgroundColor: disabled ? '#29292F' : '#8A77FF', + color: disabled ? 'grey' : '#FFFFFF', }} ... - isDisabled() + disabledThis keeps logs, but evaluates them once per render.
- “Modules need installation” branch vs. current
useModularSdkbehaviorThe branch:
if (!areModulesInstalled && payingTokens.length > 0) { console.log('Button enabled: modules need installation'); return false; }assumes
areModulesInstalledcan befalse. However, the updateduseModularSdkhook now initializesareModulesInstalledtotrueand (in its current form) never sets it tofalse, which means this branch — and the corresponding “Enable Trading on …” UI — may never be hit.If you intend to support the “install modules” path, you probably want
useModularSdkto surface a realfalsevalue when modules are missing; otherwise, consider simplifying this branch and the related button text to align with the new assumption that modules are always treated as installed.Also applies to: 135-137
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
package.json(1 hunks)src/apps/pulse/components/App/HomeScreen.tsx(7 hunks)src/apps/pulse/components/Buy/Buy.tsx(6 hunks)src/apps/pulse/components/Buy/BuyButton.tsx(1 hunks)src/apps/pulse/hooks/useIntentSdk.ts(3 hunks)src/apps/pulse/hooks/useModularSdk.ts(2 hunks)src/main.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/hooks/useModularSdk.tssrc/apps/pulse/hooks/useIntentSdk.ts
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/App/HomeScreen.tsxsrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/hooks/useIntentSdk.tssrc/apps/pulse/components/Buy/Buy.tsx
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pulse/hooks/useIntentSdk.ts
🧬 Code graph analysis (1)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/apps/pulse/utils/intent.ts (1)
getDispensableAssets(22-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: unit-tests
| try { | ||
| if (isBuyTransaction) { | ||
| if (!intentSdk) { | ||
| console.error('Intent SDK not initialized'); | ||
| return; | ||
| } |
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.
🧩 Analysis chain
Prevent endless polling loop when Intent SDK is not initialized
In the buy‑polling branch:
if (isBuyTransaction) {
if (!intentSdk) {
console.error('Intent SDK not initialized');
return;
}
// ...
}If intentSdk is null, this logs an error and returns, but:
isPollingActive/isBackgroundPollingare not changed, and- The outer
setInterval(pollStatus, 2000)continues to run,
so you’ll keep logging every 2s and the transaction status never moves to a final state.
Consider either preventing polling from starting when intentSdk is missing, or failing fast here by stopping polling and surfacing an error, e.g.:
- if (isBuyTransaction) {
- if (!intentSdk) {
- console.error('Intent SDK not initialized');
- return;
- }
+ if (isBuyTransaction) {
+ if (!intentSdk) {
+ console.error('Intent SDK not initialized – stopping polling');
+ setIsPollingActive(false);
+ setIsBackgroundPolling(false);
+ setCurrentTransactionStatus('Transaction Failed');
+ setErrorDetails('Unable to fetch buy transaction status.');
+ return;
+ }You could also guard the whole polling effect so it never starts a buy‑poll interval until intentSdk is truthy.
🏁 Script executed:
#!/bin/bash
# Find the polling effect in HomeScreen.tsx and the setInterval call
rg -n -A 30 "setInterval.*pollStatus" src/apps/pulse/components/App/HomeScreen.tsxLength of output: 833
🏁 Script executed:
#!/bin/bash
# Look for the polling effect definition and dependencies around lines 686-691
rg -n -B 20 -A 20 "if \(!intentSdk\)" src/apps/pulse/components/App/HomeScreen.tsx | head -80Length of output: 1618
🏁 Script executed:
#!/bin/bash
# Find where isPollingActive and isBackgroundPolling are set/used in the polling context
rg -n "setIsPollingActive|setIsBackgroundPolling" src/apps/pulse/components/App/HomeScreen.tsxLength of output: 832
🏁 Script executed:
#!/bin/bash
# Look for the overall polling effect structure
ast-grep --pattern $'useEffect(() => {
$$$
setInterval($$$)
$$$
}, $_)' src/apps/pulse/components/App/HomeScreen.tsxLength of output: 0
Prevent endless polling loop when Intent SDK is not initialized
In the buy‑polling effect (line 671):
The effect's early return guard at line 676 does not check for intentSdk:
if (!userOpHash || !chainId || (!isPollingActive && !isBackgroundPolling)) {
return undefined;
}If intentSdk is null, this does not prevent the polling interval from starting at line 846. Once running, pollStatus will be called every 2 seconds, hit the check at lines 688–691, log an error, and return—but the interval continues indefinitely.
The suggested fix is valid: either add !intentSdk to the early return condition, or stop polling inside pollStatus when intentSdk is missing:
try {
if (isBuyTransaction) {
if (!intentSdk) {
console.error('Intent SDK not initialized – stopping polling');
setIsPollingActive(false);
setIsBackgroundPolling(false);
setCurrentTransactionStatus('Transaction Failed');
setErrorDetails('Unable to fetch buy transaction status.');
return;
}Alternatively, add !intentSdk to the effect's early return guard at line 676 to prevent the interval from starting at all.
🤖 Prompt for AI Agents
In src/apps/pulse/components/App/HomeScreen.tsx around lines 686 to 691, the
buy-polling effect can start an interval even when intentSdk is null, causing
endless polling that only logs an error; modify the effect's early return
condition (the guard a few lines above) to include a check for !intentSdk so the
interval never starts when intentSdk is missing, or alternatively add logic at
the start of pollStatus to clear/stop the interval when intentSdk is null —
implement the first option by adding !intentSdk to the existing OR condition so
the effect returns undefined and does not set the polling interval.
Deploying x with
|
| Latest commit: |
467a201
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8394b256.x-e62.pages.dev |
| Branch Preview URL: | https://fix-7702-testing.x-e62.pages.dev |
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: 3
♻️ Duplicate comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
92-109: Well done fixing the critical null-dereference issue!The guards for
kit,walletProvider, andwalletClientnow prevent the runtime crash flagged in the previous review. The early returns ensure initialization only proceeds when all required objects are available.
🧹 Nitpick comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
36-125: Consider reducing verbose console logging for production.The initialization flow includes 9 console.log statements (lines 36, 93, 100, 107, 111-117, 121-125). While helpful for debugging during development, this level of logging may be excessive for production. Consider using a debug flag or environment variable to conditionally enable verbose logging, or replace some logs with a single summary log on success/failure.
Example approach using environment-based debug flag:
+ const DEBUG = import.meta.env.MODE === 'development'; + try { - console.log('Attempting to initialize Intent SDK'); + if (DEBUG) console.log('Attempting to initialize Intent SDK'); if (!kit) { - console.log('Transaction Kit not yet available'); + if (DEBUG) console.log('Transaction Kit not yet available'); return; } // ... similar changes for other debug logs const sdk = new IntentSdk(walletClient as any, options); setIntentSdk(sdk); - console.log('Initialized Intent SDK via Transaction Kit wallet provider'); + console.log('Intent SDK initialized successfully'); // Keep one summary log setError(null); - console.log('Intent SDK initialization attempt finished');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/apps/pulse/hooks/useIntentSdk.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/pulse/hooks/useIntentSdk.ts
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/pulse/hooks/useIntentSdk.ts
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/hooks/useIntentSdk.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
119-119: Use a proper type adapter or narrow the type safely instead ofas any.The type cast on line 119 is part of a systematic pattern in this file (lines 46, 82, 119) and appears elsewhere in the codebase (e.g.,
the-exchange/index.tsxline 73). Theas anycast indicates a type mismatch between Transaction Kit's wallet client and IntentSdk's expected viem-based wallet client type.Verification shows:
- IntentSdk (v1.0.0-alpha.10) depends on viem@2.21.53
- walletClient comes from
kit.getEtherspotProvider().getWalletClient()(Transaction Kit)- Multiple uses of
as anyfor this same cast suggest a known incompatibilityRather than relying on
as anyacross multiple call sites, consider:
- Creating a type adapter or wrapper function to safely bridge Transaction Kit's wallet client to IntentSdk's expectations
- Verifying the actual runtime compatibility and updating IntentSdk type definitions if needed
- If using an alpha version of IntentSdk, checking if newer versions have improved type definitions
| // const { ready, authenticated, user } = usePrivy(); | ||
| // const { wallets } = useWallets(); | ||
| // const { isConnected: isWagmiConnected } = useAccount(); | ||
| // const { connectors } = useConnect(); | ||
|
|
||
| const privyWalletAddress = user?.wallet?.address; | ||
| // const privyWalletAddress = user?.wallet?.address; | ||
|
|
||
| const walletProvider = wallets.find( | ||
| (wallet) => wallet.address === privyWalletAddress | ||
| ); | ||
| // const walletProvider = wallets.find( | ||
| // (wallet) => wallet.address === privyWalletAddress | ||
| // ); |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
Lines 12-21 contain commented imports and logic from the previous Privy-based implementation. Once the Transaction Kit migration is confirmed working, this dead code should be removed to improve maintainability.
Apply this diff to remove the commented code:
const { walletAddress: accountAddress, kit } = useTransactionKit();
- // const { ready, authenticated, user } = usePrivy();
- // const { wallets } = useWallets();
- // const { isConnected: isWagmiConnected } = useAccount();
- // const { connectors } = useConnect();
-
- // const privyWalletAddress = user?.wallet?.address;
-
- // const walletProvider = wallets.find(
- // (wallet) => wallet.address === privyWalletAddress
- // );
+
const [intentSdk, setIntentSdk] = useState<IntentSdk | null>(null);📝 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 { ready, authenticated, user } = usePrivy(); | |
| // const { wallets } = useWallets(); | |
| // const { isConnected: isWagmiConnected } = useAccount(); | |
| // const { connectors } = useConnect(); | |
| const privyWalletAddress = user?.wallet?.address; | |
| // const privyWalletAddress = user?.wallet?.address; | |
| const walletProvider = wallets.find( | |
| (wallet) => wallet.address === privyWalletAddress | |
| ); | |
| // const walletProvider = wallets.find( | |
| // (wallet) => wallet.address === privyWalletAddress | |
| // ); | |
| const { walletAddress: accountAddress, kit } = useTransactionKit(); | |
| const [intentSdk, setIntentSdk] = useState<IntentSdk | null>(null); |
🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 12 to 21, remove the
leftover commented-out Privy/Wallet related imports and logic (the commented
const lines and wallets/find block) to eliminate dead code; delete those
commented lines and ensure surrounding code formatting remains correct and
imports are updated if any unused imports remain.
| // if (ready && authenticated && walletProvider) { | ||
| // console.log('Initializing Intent SDK via Privy wallet'); | ||
| // const provider = await walletProvider.getEthereumProvider(); | ||
| // const walletClient = createWalletClient({ | ||
| // account: walletProvider.address as Hex, | ||
| // transport: custom(provider), | ||
| // }); | ||
| // /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| // const sdk = new IntentSdk(walletClient as any, options); | ||
| // console.log('Initialized Intent SDK via Privy wallet'); | ||
| // setIntentSdk(sdk); | ||
| // setError(null); | ||
| // return; | ||
| // } | ||
|
|
||
| // // 2: Check if connected via WalletConnect (only if no Privy wallet) | ||
| // const hasWallets = walletProvider !== undefined; | ||
| // if (isWagmiConnected && !hasWallets) { | ||
| // console.log('Initializing Intent SDK via WalletConnect'); | ||
| // const walletConnectConnector = connectors.find( | ||
| // ({ id }) => id === 'walletConnect' | ||
| // ); | ||
|
|
||
| // if (walletConnectConnector) { | ||
| // const wcProvider: any = await walletConnectConnector.getProvider(); | ||
|
|
||
| // // Only proceed if the provider is actually connected | ||
| // if ( | ||
| // wcProvider && | ||
| // wcProvider.connected && | ||
| // wcProvider.accounts && | ||
| // wcProvider.accounts.length > 0 | ||
| // ) { | ||
| // // Get the connected account | ||
| // const accounts = await wcProvider.request({ | ||
| // method: 'eth_accounts', | ||
| // }); | ||
| // const wcAccount = accounts[0]; | ||
|
|
||
| // if (wcAccount) { | ||
| // const walletClient = createWalletClient({ | ||
| // account: wcAccount as Hex, | ||
| // transport: custom(wcProvider), | ||
| // }); | ||
| // const sdk = new IntentSdk(walletClient as any, options); | ||
| // setIntentSdk(sdk); | ||
| // console.log('Initialized Intent SDK via WalletConnect'); | ||
| // setError(null); | ||
| // } | ||
| // } | ||
| // } | ||
| // } |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove 52 lines of commented-out initialization logic.
Lines 38-89 contain the entire Privy and WalletConnect initialization flow that has been replaced by Transaction Kit. This commented code should be removed once the new implementation is verified in production.
Apply this diff to remove the commented initialization logic:
try {
console.log('Attempting to initialize Intent SDK');
- // 1: Check if connected via Privy wallet
- // if (ready && authenticated && walletProvider) {
- // console.log('Initializing Intent SDK via Privy wallet');
- // const provider = await walletProvider.getEthereumProvider();
- // const walletClient = createWalletClient({
- // account: walletProvider.address as Hex,
- // transport: custom(provider),
- // });
- // /* eslint-disable @typescript-eslint/no-explicit-any */
- // const sdk = new IntentSdk(walletClient as any, options);
- // console.log('Initialized Intent SDK via Privy wallet');
- // setIntentSdk(sdk);
- // setError(null);
- // return;
- // }
-
- // // 2: Check if connected via WalletConnect (only if no Privy wallet)
- // const hasWallets = walletProvider !== undefined;
- // if (isWagmiConnected && !hasWallets) {
- // console.log('Initializing Intent SDK via WalletConnect');
- // const walletConnectConnector = connectors.find(
- // ({ id }) => id === 'walletConnect'
- // );
-
- // if (walletConnectConnector) {
- // const wcProvider: any = await walletConnectConnector.getProvider();
-
- // // Only proceed if the provider is actually connected
- // if (
- // wcProvider &&
- // wcProvider.connected &&
- // wcProvider.accounts &&
- // wcProvider.accounts.length > 0
- // ) {
- // // Get the connected account
- // const accounts = await wcProvider.request({
- // method: 'eth_accounts',
- // });
- // const wcAccount = accounts[0];
-
- // if (wcAccount) {
- // const walletClient = createWalletClient({
- // account: wcAccount as Hex,
- // transport: custom(wcProvider),
- // });
- // const sdk = new IntentSdk(walletClient as any, options);
- // setIntentSdk(sdk);
- // console.log('Initialized Intent SDK via WalletConnect');
- // setError(null);
- // }
- // }
- // }
- // }
-
- // 3: Check if Transaction Kit is available
+
if (!kit) {🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 38 to 89, remove the entire
52-line block of commented-out Privy and WalletConnect initialization (the
commented code from the previous Intent SDK flow). Delete those commented lines,
run the linter/formatter, and remove any now-unused imports or variables
introduced solely for that removed code; then run tests or a quick local
verification to ensure no references remain.
| // ready, | ||
| // authenticated, | ||
| // walletProvider, | ||
| // isWagmiConnected, | ||
| // connectors, |
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.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out dependency array entries.
Lines 136-140 contain commented dependency entries from the previous implementation. Clean up the dependency array to only include active dependencies.
Apply this diff:
- }, [
- accountAddress,
- kit,
- // ready,
- // authenticated,
- // walletProvider,
- // isWagmiConnected,
- // connectors,
- ]);
+ }, [accountAddress, kit]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 136 to 140, the dependency
array contains leftover commented-out entries (ready, authenticated,
walletProvider, isWagmiConnected, connectors); remove these commented lines so
the dependency array only contains active dependencies and nothing commented,
keeping formatting consistent and ensuring no trailing commas or extraneous
whitespace remain.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
Refactor
New Features
Chores