Conversation
WalkthroughAdded a chainId filter to dispensable-asset selection and passed the max stable-coin chainId from Buy. Adjusted TransactionDetails close-button styling. Made useModularSdk explicitly set modules-installed = false when API reports not installed. Added pulseNodeUrl option to Intent SDK and documented VITE_PULSE_NODE_URL in .env.example. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Buy Component
participant Utils as intent.getDispensableAssets
participant Data as PortfolioData
UI->>Utils: getDispensableAssets(input, portfolioData, maxStablecoinChainId)
Utils->>Data: iterate tokens
alt token.chainId == maxStablecoinChainId && usdEq > input && hasStableToken
Utils->>UI: include token as dispensable
else
Utils->>UI: skip token
end
sequenceDiagram
participant Hook as useModularSdk
participant API as isPulseModulesInstalled
Hook->>API: request module status
API-->>Hook: { allValidatorsPresent: true } or { allValidatorsPresent: false }
alt true
Hook->>Hook: set areModulesInstalled = true
else
Hook->>Hook: set areModulesInstalled = false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying x with
|
| Latest commit: |
f2075a1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://571850aa.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3809-enable-trading-bug.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
src/apps/pulse/utils/intent.ts (1)
22-50: Clarify the optionality ofmaxStablecoinChainIdparameter.The parameter
maxStablecoinChainIdis marked as optional (?:), but it's used as a required value in the condition at line 47 (maxStablecoinChainId === tokenItem.chainId). IfmaxStablecoinChainIdisundefined, the equality check will always befalse, causing the function to return empty arrays[[], [], []]for all inputs, effectively disabling dispensable asset selection.While the current call site in
Buy.tsxalways provides this parameter, the optional signature suggests the function can be called without it, which would break functionality silently.Recommendation: Either make the parameter required by removing the
?, or add explicit handling for the undefined case:Option 1: Make it required (preferred if always needed):
export function getDispensableAssets( input: string, portfolioData: PortfolioData | undefined, - maxStablecoinChainId?: number + maxStablecoinChainId: number ): [DispensableAsset[], bigint[], PayingToken[]] {Option 2: Add a guard clause if undefined should be valid:
export function getDispensableAssets( input: string, portfolioData: PortfolioData | undefined, maxStablecoinChainId?: number ): [DispensableAsset[], bigint[], PayingToken[]] { if (!portfolioData?.assets) { return [[], [], []]; } + // Return early if maxStablecoinChainId is not provided + if (maxStablecoinChainId === undefined) { + return [[], [], []]; + }
🧹 Nitpick comments (1)
src/apps/pulse/components/Transaction/TransactionDetails.tsx (1)
152-156: Note: PR description mentions "refresh icon" but this appears to be the Esc/close button.The nested
divwrapper and theEsccomponent button both usebg-[#1E1D24], creating two layers with identical background colors. While not harmful, this could be simplified by consolidating the styling into a single element.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/apps/pulse/components/Transaction/tests/__snapshots__/TransactionDetails.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
src/apps/pulse/components/Buy/Buy.tsx(1 hunks)src/apps/pulse/components/Transaction/TransactionDetails.tsx(1 hunks)src/apps/pulse/hooks/useModularSdk.ts(1 hunks)src/apps/pulse/utils/intent.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/apps/pulse/components/Transaction/TransactionDetails.tsx (1)
src/apps/pulse/components/Misc/Esc.tsx (1)
Esc(7-36)
src/apps/pulse/utils/intent.ts (1)
src/types/api.ts (1)
PortfolioData(739-748)
⏰ 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
🔇 Additional comments (3)
src/apps/pulse/components/Transaction/TransactionDetails.tsx (1)
149-149: Layout adjustment looks good.The flex layout classes properly center the close button container.
src/apps/pulse/hooks/useModularSdk.ts (1)
142-143: Good improvement for explicit state management.Adding the explicit
elsebranch to setareModulesInstalledtofalseimproves code clarity and ensures the state is always defined based on the module installation check.src/apps/pulse/components/Buy/Buy.tsx (1)
257-261: Correct usage of the updatedgetDispensableAssetssignature.The call site properly passes
maxStableCoinBalance.chainIdas the third parameter, aligning with the function's updated signature inintent.ts.
github-actions
bot
temporarily deployed
to
Preview (PRO-3809-Enable_Trading_Bug)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
85-88: Consider more specific error messages for configuration issues.The current error handling is generic and won't help users diagnose configuration-specific problems, such as an invalid or missing
pulseNodeUrl. Consider adding validation before SDK initialization or catching and re-throwing errors with more context about which configuration value caused the failure.Example enhancement:
+ // Validate required configuration + if (!options.bundlerApiKey) { + setError('Missing VITE_ETHERSPOT_BUNDLER_API_KEY environment variable.'); + return; + } + if (!options.pulseNodeUrl) { + console.warn('VITE_PULSE_NODE_URL not configured, SDK may not function correctly.'); + } + try { // 1: Check if connected via Privy wallet
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/hooks/useIntentSdk.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: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/apps/pulse/hooks/useIntentSdk.ts (1)
32-32: Based on my verification efforts, I was unable to find definitive documentation for the@etherspot/intent-sdkv1.0.0-alpha.10 package or confirmation that thepulseNodeUrlfield exists in its Options type. The codebase shows only a single usage ofVITE_PULSE_NODE_URL(the line being reviewed), with no environment configuration files or documentation present.Verify the pulseNodeUrl configuration field with the SDK team.
The addition of
pulseNodeUrlfrom the environment variable cannot be verified against public SDK documentation:
Unverified field: The
pulseNodeUrlfield's existence in the@etherspot/intent-sdk@1.0.0-alpha.10Options type could not be confirmed through available documentation. As an alpha-version SDK, field names and types may be unstable or undocumented.Empty string fallback risk: The field falls back to an empty string if
VITE_PULSE_NODE_URLis not set. Confirm with the SDK team whether an empty URL is safe or if validation/a warning should be added.Configuration visibility: No environment configuration files (.env, .env.example) exist in the codebase documenting this new variable or its expected format.
Before merging, verify:
- The Options type actually includes
pulseNodeUrl- Empty string is an acceptable fallback value
- Consider documenting the required env var format
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.env.example (1)
13-13: Minor: Address static analysis linting suggestions.The new environment variable has been added correctly, but two minor style notes from the linter:
- Key ordering:
VITE_PULSE_NODE_URLshould be placed beforeVITE_PX_DEVELOPMENT_IDto maintain alphabetical ordering.- Quote character: The value uses double quotes; consider whether this is consistent with your project's
.envconventions (though the API keys on lines 2–3 do use quotes).Apply this diff if you'd like to reorder alphabetically:
- VITE_PX_DEVELOPMENT_ID="src-apps-development-dir" VITE_REOWN_PROJECT_ID="Your ReOwn project ID" - VITE_SKANDHA_URL="https://rpc.etherspot.io/v2" - VITE_SWAP_BUTTON_SWITCH="true" - VITE_PAYMASTER_URL="http://localhost:5050" - VITE_SENTRY_ENVIRONMENT="staging" - VITE_PULSE_NODE_URL="https://pulse.etherspot.io" + VITE_PAYMASTER_URL="http://localhost:5050" + VITE_PX_DEVELOPMENT_ID="src-apps-development-dir" + VITE_PULSE_NODE_URL="https://pulse.etherspot.io" + VITE_REOWN_PROJECT_ID="Your ReOwn project ID" + VITE_SKANDHA_URL="https://rpc.etherspot.io/v2" + VITE_SENTRY_ENVIRONMENT="staging" + VITE_SWAP_BUTTON_SWITCH="true"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.env.example(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 13-13: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 13-13: [UnorderedKey] The VITE_PULSE_NODE_URL key should go before the VITE_PX_DEVELOPMENT_ID key
(UnorderedKey)
⏰ 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
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
Bug Fixes
Improvements
Documentation