Show Mev protection only for supported providers#9198
Conversation
📝 WalkthroughWalkthroughMEV protection capability determination is refactored from a static provider property to a runtime token-pair-aware method. The interface introduces ChangesMEV Protection Token-Pair Capability
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/OneInchProvider.kt (1)
43-44: ⚖️ Poor tradeoffConsider extracting common MEV protection logic.
The same
mevProtectionAllowedimplementation appears inBaseUniswapV3Provider,AllBridgeProvider, andOneInchProvider. While this duplication may be intentional to allow future provider-specific customization, consider extracting it to a common extension function or base implementation if all providers will consistently use this logic.🔧 Possible approach
In a common utilities file:
fun IMultiSwapProvider.defaultMevProtectionAllowed(tokenIn: Token, tokenOut: Token): Boolean = tokenIn.blockchainType == tokenOut.blockchainType && tokenIn.blockchainType.isEvmThen in providers:
override fun mevProtectionAllowed(tokenIn: Token, tokenOut: Token): Boolean = defaultMevProtectionAllowed(tokenIn, tokenOut)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/OneInchProvider.kt` around lines 43 - 44, Multiple providers (OneInchProvider, BaseUniswapV3Provider, AllBridgeProvider) implement identical mevProtectionAllowed logic; extract it into a shared function (e.g., defaultMevProtectionAllowed) and have each provider delegate to it to remove duplication. Create the utility function with the same signature (accepting tokenIn and tokenOut) in a common utilities file or on the IMultiSwapProvider interface, and replace each provider's mevProtectionAllowed body to call that shared function (keep function name mevProtectionAllowed in providers for API stability).app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/IMultiSwapProvider.kt (1)
24-24: ⚡ Quick winConsider adding KDoc to explain MEV protection eligibility.
The
mevProtectionAllowedmethod determines a critical security feature but lacks documentation. Future implementers would benefit from understanding that MEV protection should typically returntrueonly for same-chain swaps on EVM blockchains.📝 Proposed documentation
+ /** + * Determines whether MEV protection is available for the given token pair. + * Typically returns true only when both tokens are on the same EVM blockchain, + * as MEV protection is specific to EVM chains and requires same-chain swaps. + * + * `@param` tokenIn The input token + * `@param` tokenOut The output token + * `@return` true if MEV protection can be applied, false otherwise + */ fun mevProtectionAllowed(tokenIn: Token, tokenOut: Token): Boolean = false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/IMultiSwapProvider.kt` at line 24, Add a KDoc comment to the IMultiSwapProvider interface explaining the purpose and expected behavior of mevProtectionAllowed(tokenIn: Token, tokenOut: Token): Boolean — note that it indicates whether MEV protection should be applied for a given swap, that implementations should generally return true only for same-chain swaps on EVM-compatible chains, describe the parameters (tokenIn and tokenOut) and that the return value true enables MEV protection while false disables it; attach this documentation directly above the mevProtectionAllowed declaration in IMultiSwapProvider.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/IMultiSwapProvider.kt`:
- Line 24: Add a KDoc comment to the IMultiSwapProvider interface explaining the
purpose and expected behavior of mevProtectionAllowed(tokenIn: Token, tokenOut:
Token): Boolean — note that it indicates whether MEV protection should be
applied for a given swap, that implementations should generally return true only
for same-chain swaps on EVM-compatible chains, describe the parameters (tokenIn
and tokenOut) and that the return value true enables MEV protection while false
disables it; attach this documentation directly above the mevProtectionAllowed
declaration in IMultiSwapProvider.
In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/OneInchProvider.kt`:
- Around line 43-44: Multiple providers (OneInchProvider, BaseUniswapV3Provider,
AllBridgeProvider) implement identical mevProtectionAllowed logic; extract it
into a shared function (e.g., defaultMevProtectionAllowed) and have each
provider delegate to it to remove duplication. Create the utility function with
the same signature (accepting tokenIn and tokenOut) in a common utilities file
or on the IMultiSwapProvider interface, and replace each provider's
mevProtectionAllowed body to call that shared function (keep function name
mevProtectionAllowed in providers for API stability).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9451ee55-fe26-43e1-95b1-f9c23d319ab3
📒 Files selected for processing (5)
app/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/SwapConfirmViewModel.ktapp/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/AllBridgeProvider.ktapp/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/BaseUniswapV3Provider.ktapp/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/IMultiSwapProvider.ktapp/src/main/java/io/horizontalsystems/bankwallet/modules/multiswap/providers/OneInchProvider.kt
#9184
Summary by CodeRabbit