fix(volume-fee): remove hardcoded stablecoins check for Safe fee #5436
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThe changes modify the logic within the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant SafeAppFeeAtom
Caller->>SafeAppFeeAtom: Invoke fee calculation
SafeAppFeeAtom->>SafeAppFeeAtom: Check if currencies are stable
SafeAppFeeAtom->>SafeAppFeeAtom: Bypass stablecoin fee flag check
SafeAppFeeAtom-->>Caller: Return fee calculation result
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.ts (1)
60-66: Udderly fantastic addition of debug logging! 🐄The debug logging will help track fee skipping for correlated tokens, making it easier to diagnose fee-related issues in production.
Consider adding the
tradeTypeto the debug log object to provide more context about the trading scenario:console.debug('[Volume Fee] Skipping fee for correlated tokens', { inputCurrencyAddress, outputCurrencyAddress, correlatedTokens, + tradeType: tradeState.tradeType, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/cowswap-frontend/src/modules/volumeFee/state/safeAppFeeAtom.ts(2 hunks)apps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/cowswap-frontend/src/modules/volumeFee/state/safeAppFeeAtom.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (2)
apps/cowswap-frontend/src/modules/volumeFee/state/volumeFeeAtom.ts (2)
49-58: MOOving in the right direction with better readability! 🐮The extraction of the correlation logic into the
isCorrelatedvariable makes the code more self-documenting and easier to understand.
68-68: Clean and COWncise return! 🎯The simplified return statement maintains the same logic while being more readable.
| bps, | ||
| fiatAmount, | ||
| isStableCoinTrade, | ||
| }) |
There was a problem hiding this comment.
were you intending to add this message?
There was a problem hiding this comment.
Yes. It's not easy for Elena to test the volume fee calculation, so I added this debug log.
Summary
In #5187 we added
isSafeAppStableCoinsFeeEnabledflag in order to not add fees to stablecoins in Safe.But since we have #5427, we don't need this check because it's already covered by the data from CMS.
To Test
Summary by CodeRabbit