Skip to content

fix/PRO-3743/PW-fix-Pulse#414

Merged
RanaBug merged 1 commit intostagingfrom
fix/PRO-3743/PW-fix-Pulse
Oct 1, 2025
Merged

fix/PRO-3743/PW-fix-Pulse#414
RanaBug merged 1 commit intostagingfrom
fix/PRO-3743/PW-fix-Pulse

Conversation

@RanaBug
Copy link
Collaborator

@RanaBug RanaBug commented Oct 1, 2025

Description

  • Fix for Pillar Wallet to be able to use Pulse app

How Has This Been Tested?

  • Existing Unit Tests and Manual Testing

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features

    • Added WalletConnect (via Wagmi) support alongside Privy.
    • SDKs now auto-initialize based on the active wallet connection.
  • Bug Fixes

    • Improved reliability with stricter provider/account checks and error handling to prevent failed initializations.
    • Reacts to wallet state changes to keep SDKs in sync.
  • Refactor

    • Streamlined initialization into explicit async routines for clearer control flow.
  • Tests

    • Updated test setup to mock Wagmi connection state and connect behavior.

@RanaBug RanaBug requested a review from IAmKio October 1, 2025 10:32
@RanaBug RanaBug self-assigned this Oct 1, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Introduces Wagmi-based connection handling alongside existing Privy flow in two Pulse hooks. Both hooks now initialize SDKs via either Privy or WalletConnect (via Wagmi) with added provider/account checks, async initialization, and error handling. Test setup adds a wagmi.useConnect mock to simulate a non-connected environment.

Changes

Cohort / File(s) Summary
Pulse Intent SDK hook updates
src/apps/pulse/hooks/useIntentSdk.ts
Adds Wagmi integration (useAccount/useConnect), async initialize function, WalletConnect provider path when Privy not used, provider/account validations, try/catch error handling, and updated effect dependencies (isWagmiConnected, connectors).
Pulse Modular SDK hook updates
src/apps/pulse/hooks/useModularSdk.ts
Refactors to multi-path initialization: Privy or WalletConnect via Wagmi, early return on no paying tokens, wallet client creation, robust provider/account checks, centralized async initialize, error handling, and expanded dependencies.
Test setup Wagmi mocks
src/test-utils/setupTests.ts
Extends wagmi mock with useConnect returning { connectors: [], connect: vi.fn(), isPending: false, error: null } to support new hooks in tests.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Component
  participant H as Hook use...Sdk (useEffect)
  participant P as Privy
  participant W as Wagmi
  participant WC as WalletConnect Provider
  participant S as SDK (Intent/Modular)

  C->>H: Mount / deps change
  rect rgba(230,245,255,0.6)
    note over H: Async initializeSdk()
    alt Privy path
      H->>P: isReady && authenticated && walletProvider?
      P-->>H: Ethereum provider
      H->>S: create wallet client + init SDK
      S-->>H: instance set
    else Wagmi path
      H->>W: isConnected? connectors?
      W-->>H: WalletConnect connector
      H->>WC: getProvider() + accounts
      WC-->>H: provider + account
      H->>S: create wallet client + init SDK
      S-->>H: instance set
    end
  end
  H-->>C: sdk state updated
  opt Error
    H->>H: try/catch sets error and resets sdk
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • Added wagmi WC #327 — Introduces Wagmi/WalletConnect integration and related hooks/mocks; overlaps with adding useAccount/useConnect and WC provider flows.

Suggested reviewers

  • IAmKio
  • vignesha22

Poem

Thump-thump goes my fluffy heart,
Two paths now dance—Privy, Wagmi start.
Wallets connect, accounts align,
SDKs awaken, all is fine.
I nibble tests with careful care—
Hop, compile, deploy, we’re there! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The title “fix/PRO-3743/PW-fix-Pulse” mirrors a branch name rather than concisely summarizing the change itself and does not clearly communicate that it enables Pillar Wallet support in the Pulse app. It includes internal identifiers and formatting noise instead of a straightforward description of the update. Please replace the branch-style title with a clear, single-sentence summary focused on the user-visible change, for example “Enable Pillar Wallet support in Pulse app,” to remove internal references and improve clarity.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive The description uses the required template headings and marks the change as a bug fix, but the content is overly generic and omits key details. The summary under Description is minimal and the How Has This Been Tested section merely states “Existing Unit Tests and Manual Testing” without specifying which tests were run, the environment used, or the results. Please expand the Description section with a clear overview of what was changed, and enhance the testing section by listing the specific unit tests or test suites executed, the environment or configurations used during manual testing, and the observed outcomes.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/PRO-3743/PW-fix-Pulse

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.

@cloudflare-workers-and-pages
Copy link

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 10ebc1a
Status: ✅  Deploy successful!
Preview URL: https://0ba2de0b.x-e62.pages.dev
Branch Preview URL: https://fix-pro-3743-pw-fix-pulse.x-e62.pages.dev

View logs

Copy link
Contributor

@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 (1)
src/apps/pulse/hooks/useModularSdk.ts (1)

48-112: Reset Modular SDK on disconnect

When neither Privy nor WalletConnect succeeds, the previous modularSdk instance stays in state. After a wallet disconnect, callers still see a non-null SDK and keep invoking methods against a closed provider. Mirror the connection teardown by clearing modularSdk (and any dependent flags if needed) whenever no connector is available so the hook accurately signals “not ready”.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d855159 and 10ebc1a.

📒 Files selected for processing (3)
  • src/apps/pulse/hooks/useIntentSdk.ts (3 hunks)
  • src/apps/pulse/hooks/useModularSdk.ts (4 hunks)
  • src/test-utils/setupTests.ts (1 hunks)
⏰ 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: lint
  • GitHub Check: unit-tests
  • GitHub Check: build

Comment on lines +26 to +88
const initializeSdk = async () => {
if (!accountAddress) return;

const options: Options = {
bundlerApiKey: import.meta.env.VITE_ETHERSPOT_BUNDLER_API_KEY || '',
modularAccount: accountAddress as Hex,
};

walletProvider
.getEthereumProvider()
.then((provider) => {
try {
// 1: Check if connected via Privy wallet
if (ready && authenticated && walletProvider) {
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);
setIntentSdk(sdk);
setError(null); // Clear any previous errors when SDK initializes
})
.catch((err) => {
console.error('Failed to initialize Intent SDK:', err);
setError('Failed to initialize Intent SDK. Please try again.');
});
}
}, [accountAddress, ready, authenticated, walletProvider]);
setError(null);
return;
}

// 2: Check if connected via WalletConnect (only if no Privy wallet)
const hasWallets = walletProvider !== undefined;
if (isWagmiConnected && !hasWallets) {
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);
setError(null);
}
}
}
}
} catch (err) {
console.error('Failed to initialize Intent SDK:', err);
setError('Failed to initialize Intent SDK. Please try again.');
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clear stale Intent SDK when no connector is active

If the user disconnects (Privy logout or Wagmi session ends), neither branch executes and intentSdk keeps the previous instance. Downstream hooks will keep issuing requests against a dead provider, producing misleading state. Please reset intentSdk (and related error state) whenever both the Privy and WalletConnect paths fail so the hook reflects the disconnected state.

🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 26–88, the initialization
function never clears a previously set intentSdk when neither the Privy nor
WalletConnect branches run (e.g., on disconnect), leaving a stale SDK; update
the function so that whenever you early-return because !accountAddress or after
executing both connector checks with no active connection, call
setIntentSdk(null) and clear related error state with setError(null) (and also
ensure the catch block clears intentSdk on fatal failures) so the hook
accurately reflects the disconnected state.

Copy link

@mightyboothkl mightyboothkl left a comment

Choose a reason for hiding this comment

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

LGTM

@RanaBug RanaBug merged commit 9d97cae into staging Oct 1, 2025
6 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 23, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 14, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Nov 27, 2025
3 tasks
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.

3 participants