mev-beta/docs/CRITICAL_FIXES_IMPLEMENTATION_PLAN.md

Critical Fixes Implementation Plan

Generated: 2025-11-01 Based On: LOGIC_AUDIT_REPORT.md Priority: IMMEDIATE - Production Blocking Issues

---

Executive Summary

This document outlines the implementation plan for 6 critical issues identified in the comprehensive logic audit. These issues prevent production deployment and must be fixed immediately.

Current Status: ⚠️ NOT PRODUCTION READY Target Status: ✅ PRODUCTION READY Estimated Time: 2-3 days for critical fixes

---

Critical Issue #1: DFS Path Building Bug

Location

pkg/arbitrage/multihop.go:234-246

Problem

// WRONG: Modifies underlying array
newPath := append(currentPath, pool)
newTokens := append(currentTokens, nextToken)
mhs.dfsArbitragePaths(...)
delete(visited, nextToken)  // Backtrack - but slice is still modified!

Impact

• Paths become contaminated with pools from other branches
• Invalid arbitrage paths that don't form profitable loops
• All multi-hop arbitrage is compromised

Fix Implementation

// CORRECT: Explicit copy
newPath := make([]Pool, len(currentPath)+1)
copy(newPath, currentPath)
newPath[len(currentPath)] = pool

newTokens := make([]string, len(currentTokens)+1)
copy(newTokens, currentTokens)
newTokens[len(currentTokens)] = nextToken

Testing

func TestDFSPathIsolation(t *testing.T) {
    // Test that paths don't contaminate each other
    // Create two branches from same root
    // Verify each path contains only its own pools
}

---

Critical Issue #2: Cache Poisoning

Location

pkg/arbitrage/multihop.go:708-720

Problem

// WRONG: Only checks first path's timestamp
if len(paths) > 0 && time.Since(paths[0].LastUpdated) < mhs.cacheExpiry {
    return paths, true  // Returns potentially stale paths!
}

Impact

• Trading on 30+ second old price data
• Guaranteed transaction failures and losses
• Flash loan repayment failures

Fix Implementation

// CORRECT: Check each path individually
validPaths := make([]Path, 0, len(paths))
for _, path := range paths {
    if time.Since(path.LastUpdated) <= mhs.cacheExpiry {
        validPaths = append(validPaths, path)
    }
}

if len(validPaths) == 0 {
    return nil, false  // All paths stale, rebuild
}

return validPaths, true

Testing

func TestCacheExpiryPerPath(t *testing.T) {
    // Create paths with different timestamps
    // Verify only fresh paths are returned
    // Verify stale paths trigger rebuild
}

---

Critical Issue #3: Slippage Formula Mathematically Incorrect

Location

pkg/uniswap/pool_detector.go and calculation helpers

Problem

// WRONG: Not how AMM slippage works
slippage = tradeSize / 2.0

Impact

• Slippage estimates 2-5x incorrect
• Over-estimation rejects profitable trades
• Under-estimation causes failed transactions

Fix Implementation

// CORRECT: Proper constant product formula for Uniswap V2
func calculateSlippage(reserveIn, reserveOut, amountIn *big.Int) *big.Float {
    // dy = (y * dx * 997) / (x * 1000 + dx * 997)

    // Convert to big.Float for precision
    x := new(big.Float).SetInt(reserveIn)
    y := new(big.Float).SetInt(reserveOut)
    dx := new(big.Float).SetInt(amountIn)

    // Calculate: dx * 997
    numerator1 := new(big.Float).Mul(dx, big.NewFloat(997))

    // Calculate: y * dx * 997
    numerator := new(big.Float).Mul(y, numerator1)

    // Calculate: x * 1000
    denom1 := new(big.Float).Mul(x, big.NewFloat(1000))

    // Calculate: dx * 997
    denom2 := new(big.Float).Mul(dx, big.NewFloat(997))

    // Calculate: x * 1000 + dx * 997
    denominator := new(big.Float).Add(denom1, denom2)

    // Calculate: dy = numerator / denominator
    dy := new(big.Float).Quo(numerator, denominator)

    // Calculate market price: y / x
    marketPrice := new(big.Float).Quo(y, x)

    // Calculate execution price: dy / dx
    executionPrice := new(big.Float).Quo(dy, dx)

    // Slippage = (marketPrice - executionPrice) / marketPrice
    priceDiff := new(big.Float).Sub(marketPrice, executionPrice)
    slippage := new(big.Float).Quo(priceDiff, marketPrice)

    return slippage
}

// For Uniswap V3, use different formula based on concentrated liquidity
func calculateSlippageV3(sqrtPriceX96, liquidity, amountIn *big.Int, tickSpacing int) *big.Float {
    // V3-specific concentrated liquidity math
    // Implementation based on Uniswap V3 Core whitepaper
}

Testing

func TestSlippageCalculation(t *testing.T) {
    // Known reserve amounts and trade sizes
    // Verify slippage matches expected AMM math
    // Compare with on-chain actual execution
}

---

Critical Issue #4: Gas Price Race Condition

Location

pkg/execution/executor.go:768

Problem

// WRONG: Shared TransactOpts modified concurrently
transactOpts.GasPrice = biddingStrategy.PriorityFee  // Line 768

Impact

• Gas price from one opportunity bleeds into another
• Underpayment or overpayment for transactions
• Financial loss and failed transactions

Fix Implementation

// CORRECT: Deep copy TransactOpts per execution
func (e *Executor) executeArbitrage(opp Opportunity) error {
    // Create new TransactOpts for this execution
    txOpts := &bind.TransactOpts{
        From:     e.baseOpts.From,
        Signer:   e.baseOpts.Signer,
        Value:    big.NewInt(0),
        GasLimit: 0,  // Will be estimated
        Context:  context.Background(),
    }

    // Calculate gas price for THIS opportunity
    gasPrice := e.calculateGasPrice(opp)
    txOpts.GasPrice = gasPrice

    // Use this isolated txOpts
    tx, err := e.contract.ExecuteArbitrage(txOpts, ...)

    return err
}

Testing

func TestConcurrentGasPriceIsolation(t *testing.T) {
    // Execute multiple opportunities concurrently
    // Verify each uses correct gas price
    // Check for no interference between executions
}

---

Critical Issue #5: Float-to-Int Conversion Loses Precision

Location

pkg/arbitrage/calculator.go (profit calculations)

Problem

// WRONG: Truncates all decimals
profitFloat := calculateProfit()  // Returns big.Float
profitInt, _ := profitFloat.Int(nil)  // Truncates!

Impact

• Valid profits <1 wei are rejected
• Small arbitrage (0.0005 ETH) shows as 0 profit
• Missing profitable opportunities

Fix Implementation

// CORRECT: Multiply by 1e18 before converting
func convertProfitToWei(profitETH *big.Float) *big.Int {
    // Multiply by 10^18 to preserve decimal places
    weiMultiplier := new(big.Float).SetInt(new(big.Int).Exp(
        big.NewInt(10),
        big.NewInt(18),
        nil,
    ))

    profitWei := new(big.Float).Mul(profitETH, weiMultiplier)

    // Convert to int (now represents wei)
    result := new(big.Int)
    profitWei.Int(result)

    return result
}

// Usage
profitFloat := calculateProfit()  // Returns profit in ETH as big.Float
profitWei := convertProfitToWei(profitFloat)  // Convert to wei

if profitWei.Cmp(minProfitWei) >= 0 {
    // Execute arbitrage
}

Testing

func TestProfitPrecision(t *testing.T) {
    // Test 0.0001 ETH profit converts correctly
    // Test 0.0000001 ETH profit converts correctly
    // Verify no truncation occurs
}

---

Critical Issue #6: Opportunity Handler Race Condition

Location

pkg/arbitrage/detection_engine.go:749-752

Problem

// WRONG: Unbounded concurrency
go h.handler(opp)  // Creates thousands of goroutines!

Impact

• High opportunity rate creates OOM conditions
• System instability and crashes
• Resource exhaustion

Fix Implementation

// CORRECT: Semaphore for backpressure
type HandlerPool struct {
    sem        chan struct{}
    maxWorkers int
}

func NewHandlerPool(maxWorkers int) *HandlerPool {
    return &HandlerPool{
        sem:        make(chan struct{}, maxWorkers),
        maxWorkers: maxWorkers,
    }
}

func (p *HandlerPool) Handle(handler func(Opportunity), opp Opportunity) {
    // Acquire semaphore (blocks if at max capacity)
    p.sem <- struct{}{}

    go func() {
        defer func() {
            <-p.sem  // Release semaphore
        }()

        handler(opp)
    }()
}

// Usage in detection engine
handlerPool := NewHandlerPool(10)  // Max 10 concurrent handlers

for _, opp := range opportunities {
    handlerPool.Handle(h.handler, opp)
}

Testing

func TestHandlerBackpressure(t *testing.T) {
    // Generate 1000 opportunities
    // Verify max 10 concurrent handlers
    // Check no OOM condition
}

---

Implementation Order

Day 1: Critical Math & Concurrency

1. Morning: Fix #3 (Slippage Formula) - Most complex
2. Afternoon: Fix #5 (Float-to-Int Precision)
3. EOD: Testing for both

Day 2: Path & Cache Issues

1. Morning: Fix #1 (DFS Path Building)
2. Afternoon: Fix #2 (Cache Poisoning)
3. EOD: Integration testing

Day 3: Execution & Stabilization

1. Morning: Fix #4 (Gas Price Race)
2. Morning: Fix #6 (Handler Backpressure)
3. Afternoon: Full system testing
4. EOD: Performance validation

---

Testing Strategy

Unit Tests

• Each fix gets dedicated test
• Edge cases covered
• Regression prevention

Integration Tests

# Run full test suite
go test ./pkg/arbitrage/... -v -race
go test ./pkg/execution/... -v -race
go test ./pkg/uniswap/... -v -race

Load Testing

# Simulate high opportunity rate
# Verify no resource exhaustion
# Check profit calculations under load

Fork Testing

# Test against Arbitrum mainnet fork
# Verify actual arbitrage execution
# Validate profit calculations match on-chain

---

Success Criteria

• All 6 critical issues fixed
• All new tests passing
• No race conditions detected (go test -race)
• Fork testing shows profitable arbitrage
• System handles 1000+ opportunities/sec
• Memory usage stable under load
• Profit calculations match on-chain execution

---

Rollout Plan

1. Dev Environment: Apply and test all fixes
2. Fork Testing: Validate against mainnet fork
3. Testnet Deployment: Deploy to Arbitrum testnet
4. Monitoring: 24h observation period
5. Production: Gradual rollout with kill switch ready

---

Documentation Updates

After fixes applied:

• Update LOGIC_AUDIT_REPORT.md with fix status
• Create FIXES_APPLIED.md with before/after examples
• Update PROJECT_SPECIFICATION.md with corrected algorithms
• Add fix details to CHANGELOG.md

---

Status: 📋 Planning Complete - Ready for Implementation Next Action: Begin Day 1 implementation (Slippage Formula fix)