-
Notifications
You must be signed in to change notification settings - Fork 149
feat(bridge): add Near provider and provider feature-flags #6466
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: develop
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@shoom3301 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughBridging was reworked: feature-flag gating removed, provider management centralized via a new updater and bridging SDK calls, bridge fee handling extended to per-currency breakdowns with decimal alignment, recipient/signing flows adjusted for a ReceiverAccountBridgeProvider, and several hooks/atoms/tests moved or removed. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as App Updaters
participant BPU as BridgeProvidersUpdater
participant SDK as bridgingSdk
participant State as Bridge Providers Atom
participant Quote as Quote Flow
Note over App,BPU: Mount
App->>BPU: mount BridgeProvidersUpdater
BPU->>SDK: read feature flags
BPU->>State: set provider Set (dappIds)
BPU->>SDK: setAvailableProviders([dappIds])
rect rgb(220,240,220)
Note over User,Quote: Quote request (centralized)
User->>Quote: request tokens/networks
Quote->>SDK: bridgingSdk.getBuyTokens(params) / getTargetNetworks()
SDK->>Quote: aggregated result
Quote->>App: compute bridgeFeeAmounts (intermediate + destination)
end
rect rgb(240,230,220)
Note over App: Signing step selection
App->>App: inspect bridgeQuote.providerInfo.type
alt ReceiverAccountBridgeProvider
App->>App: set SigningStep.PreparingDepositAddress
else other
App->>App: set SigningStep.BridgingSigning
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
…s-update fix(bridge): support Near bridge provider
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
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts (1)
22-29: Approve with minor consistency suggestion.The refactored logic correctly uses
beforeFee.buyAmountand properly guards against null values. The dependency array is correctly updated to includebridgeQuote.Optional: Remove unnecessary optional chaining for consistency.
Line 27 uses optional chaining (
bridgeQuote?.) after the null check on line 23 already ensuresbridgeQuoteis non-null. For consistency, consider removing the optional chaining:- bridgeQuote?.amountsAndCosts.beforeFee.buyAmount.toString(), + bridgeQuote.amountsAndCosts.beforeFee.buyAmount.toString(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts
📚 Learning: 2025-09-25T08:46:43.815Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/modules/tradeQuote/services/fetchAndProcessQuote.test.ts:376-385
Timestamp: 2025-09-25T08:46:43.815Z
Learning: In fetchAndProcessQuote.ts, when bridgingSdk.getBestQuote() returns null, no loading state reset is needed because loading state management is handled through onQuoteResult() callback for results and processQuoteError() for errors. The null case is intentionally designed not to trigger any manager methods.
Applied to files:
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts (1)
apps/cowswap-frontend/src/modules/bridge/hooks/useBridgeQuoteAmounts.ts (1)
useBridgeQuoteAmounts(11-48)
⏰ 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: Setup
- GitHub Check: Cypress
🔇 Additional comments (1)
apps/cowswap-frontend/src/modules/bridge/hooks/useQuoteBridgeContext.ts (1)
20-20: LGTM! Explicit parameter improves clarity.Explicitly passing
truetouseBridgeQuoteAmountsmakes the override behavior clear and aligns with the updated bridge quote handling logic.
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
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts (1)
48-59: LGTM! Computation logic is sound.The updated signature and Fraction-based arithmetic are correct. The price impact calculation
100% - (output/input)is properly implemented.Optional consistency suggestion: Lines 53-54 convert to a number for the zero-check. For consistency with Fraction-based arithmetic throughout the refactor, consider using
FractionUtils.lte:- const fiatValueInputNum = +fiatValueInput.toFixed(6) - if (!fiatValueInputNum || fiatValueInputNum <= 0) return undefined + if (FractionUtils.lte(fiatValueInput, 0)) return undefinedHowever, the current approach using
toFixed(6)is acceptable for fiat value ranges and avoids potential division-by-zero issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts(2 hunks)package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts
📚 Learning: 2025-07-24T16:43:47.639Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.
Applied to files:
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts
🧬 Code graph analysis (1)
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts (1)
libs/common-utils/src/fractionUtils.ts (1)
FractionUtils(11-186)
⏰ 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). (1)
- GitHub Check: Cypress
🔇 Additional comments (2)
apps/cowswap-frontend/src/legacy/hooks/usePriceImpact/useFiatValuePriceImpact.ts (2)
5-6: LGTM! Import updates align with Fraction-based refactoring.The addition of
FractionUtilsand the shift fromCurrency/CurrencyAmounttoFraction/Percentcorrectly supports the type changes throughout this file.
39-42: LGTM! Conversion logic correctly prepares Fraction inputs.The conditional conversion using
FractionUtils.fractionLikeToFractionproperly handles the type transformation fromFractionLiketoFraction, with appropriate null-handling for the updated function signature.
elena-zh
left a 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.
I'm approving the PR with some issues that need to be addressed after the launch
Summary
providerIdparameter toappData.metadata.bridging. This value is used to link an order with bridge providerBridgingSdkmultiprovider support. Now we load tokens and target networks for all enabled providers.isBridgingEnabledfeature-flagFeature-flags
Summary by CodeRabbit
New Features
Bug Fixes