fix(pancakeswap): fix add/remove-liquidity failures and sandwich vulnerability

skills/pancakeswap/src/commands/add_liquidity.rs

@@ -8,8 +8,8 @@ pub struct AddLiquidityArgs {
     pub fee: u32,
     pub amount_a: String,
     pub amount_b: String,
-    pub tick_lower: i32,
-    pub tick_upper: i32,
+    pub tick_lower: Option<i32>,
+    pub tick_upper: Option<i32>,
     pub slippage: f64,
     pub chain: u64,
     pub dry_run: bool,
@@ -39,14 +39,37 @@ pub async fn run(args: AddLiquidityArgs) -> Result<()> {
     let amount0_desired = crate::config::human_to_minimal(amount_a_str, decimals0)?;
     let amount1_desired = crate::config::human_to_minimal(amount_b_str, decimals1)?;
 
-    // Validate tick spacing
     let spacing = crate::config::tick_spacing(args.fee)?;
-    if args.tick_lower % spacing != 0 || args.tick_upper % spacing != 0 {
-        anyhow::bail!(
-            "Ticks must be multiples of tickSpacing ({}) for fee tier {}. Got tickLower={}, tickUpper={}",
-            spacing, args.fee, args.tick_lower, args.tick_upper
-        );
-    }
+
+    // Resolve tick range: use provided values or auto-compute ±10% around current pool price
+    let (tick_lower, tick_upper) = match (args.tick_lower, args.tick_upper) {
+        (Some(tl), Some(tu)) => {
+            if tl % spacing != 0 || tu % spacing != 0 {
+                anyhow::bail!(
+                    "Ticks must be multiples of tickSpacing ({}) for fee tier {}. Got tickLower={}, tickUpper={}",
+                    spacing, args.fee, tl, tu
+                );
+            }
+            if tl >= tu {
+                anyhow::bail!("tickLower ({}) must be less than tickUpper ({})", tl, tu);
+            }
+            (tl, tu)
+        }
+        (None, None) => {
+            // Auto-compute: fetch current tick from pool, build ±10% price range.
+            // ±10% in price ≈ ±953 ticks (log(1.1)/log(1.0001)), rounded to 1000 for simplicity.
+            let pool = crate::rpc::get_pool_address(cfg.factory, token0, token1, args.fee, cfg.rpc_url).await
+                .map_err(|e| anyhow::anyhow!("Could not find pool (fee {}, chain {}): {}. Try specifying --tick-lower and --tick-upper manually.", args.fee, args.chain, e))?;
+            let (_sqrt_price, current_tick) = crate::rpc::get_slot0(&pool, cfg.rpc_url).await?;
+            let range = 1000i32.max(spacing * 20);
+            // Use Euclidean division so negative ticks round toward −∞ (correct tick alignment)
+            let tl = (current_tick - range).div_euclid(spacing) * spacing;
+            let tu = (current_tick + range).div_euclid(spacing) * spacing;
+            println!("Auto tick range: {} to {} (current tick: {}, ±{} ticks)", tl, tu, current_tick, range);
+            (tl, tu)
+        }
+        _ => anyhow::bail!("Provide both --tick-lower and --tick-upper, or omit both for auto ±10% range."),
+    };
 
     // Apply slippage to minimums using integer arithmetic (avoids f64 precision loss on large wei values)
     // slippage is in percent (e.g. 1.0 means 1%), convert to bps (100 bps)
@@ -60,20 +83,39 @@ pub async fn run(args: AddLiquidityArgs) -> Result<()> {
         .map(|d| d.as_secs() + 1200)
         .unwrap_or(9_999_999_999);
 
-    println!("Add Liquidity (chain {}):", args.chain);
-    println!("  Token0 (token0 < token1): {} {}", amount_a_str, sym0);
-    println!("  Token1:                   {} {}", amount_b_str, sym1);
-    println!("  Fee tier:                 {}%", args.fee as f64 / 10000.0);
-    println!("  Tick range:               {} to {}", args.tick_lower, args.tick_upper);
-    println!("  NPM:                      {}", cfg.npm);
-
-    // Fetch wallet address for use as recipient in mint
+    // Fetch wallet address early — needed for balance check and as mint recipient
     let wallet_address = if args.dry_run {
         "0x0000000000000000000000000000000000000001".to_string()
     } else {
         crate::onchainos::get_wallet_address().await?
     };
 
+    // Bug 1 fix: pre-flight balance check — bail before wasting gas on approve
+    if !args.dry_run {
+        let bal0 = crate::rpc::get_balance(token0, &wallet_address, cfg.rpc_url).await?;
+        let bal1 = crate::rpc::get_balance(token1, &wallet_address, cfg.rpc_url).await?;
+        if bal0 < amount0_desired {
+            anyhow::bail!(
+                "Insufficient {} balance: wallet has {} but {} required (minimal units). Deposit more {} before adding liquidity.",
+                sym0, bal0, amount0_desired, sym0
+            );
+        }
+        if bal1 < amount1_desired {
+            anyhow::bail!(
+                "Insufficient {} balance: wallet has {} but {} required (minimal units). Deposit more {} before adding liquidity.",
+                sym1, bal1, amount1_desired, sym1
+            );
+        }
+        println!("Balance check OK: {} {} available, {} {} available", bal0, sym0, bal1, sym1);
+    }
+
+    println!("Add Liquidity (chain {}):", args.chain);
+    println!("  Token0 (token0 < token1): {} {}", amount_a_str, sym0);
+    println!("  Token1:                   {} {}", amount_b_str, sym1);
+    println!("  Fee tier:                 {}%", args.fee as f64 / 10000.0);
+    println!("  Tick range:               {} to {}", tick_lower, tick_upper);
+    println!("  NPM:                      {}", cfg.npm);
+
     // Step 1: Approve token0 for NPM
     println!("\nStep 1: Approving {} for NonfungiblePositionManager...", sym0);
     eprintln!("WARNING: Approving {} {} to {} -- approving exact amount only. Use --dry-run to preview.", amount0_desired, sym0, cfg.npm);
@@ -109,8 +151,8 @@ pub async fn run(args: AddLiquidityArgs) -> Result<()> {
         token0,
         token1,
         args.fee,
-        args.tick_lower,
-        args.tick_upper,
+        tick_lower,
+        tick_upper,
         amount0_desired,
         amount1_desired,
         amount0_min,

skills/pancakeswap/src/commands/remove_liquidity.rs

@@ -29,12 +29,34 @@ pub async fn run(args: RemoveLiquidityArgs) -> Result<()> {
 
     let liquidity_to_remove = (effective_liquidity as f64 * args.liquidity_pct / 100.0) as u128;
 
+    // Bug 3 fix: compute actual token amounts from V3 liquidity math using the current
+    // pool price, instead of the incorrect tokens_owed proxy used previously.
+    // tokens_owed represents already-accrued fees credited to the position — it is
+    // completely unrelated to the amounts returned by decreaseLiquidity, which are
+    // derived from the position's liquidity and the current sqrtPrice.
+    let pool = crate::rpc::get_pool_address(cfg.factory, &pos.token0, &pos.token1, pos.fee, cfg.rpc_url).await?;
+    let (sqrt_price_x96, tick_current) = crate::rpc::get_slot0(&pool, cfg.rpc_url).await?;
+    let (amount0_out, amount1_out) = crate::rpc::amounts_from_liquidity(
+        sqrt_price_x96,
+        pos.tick_lower,
+        pos.tick_upper,
+        tick_current,
+        liquidity_to_remove,
+    );
+
+    let slippage_bps = (args.slippage * 100.0) as u128;
+    let amount0_min = amount0_out.saturating_mul(10000 - slippage_bps) / 10000;
+    let amount1_min = amount1_out.saturating_mul(10000 - slippage_bps) / 10000;
+
     println!("Remove Liquidity (chain {}):", args.chain);
     println!("  Position:     #{}", args.token_id);
     println!("  Pair:         {}/{}", sym0, sym1);
+    println!("  Current tick: {} (pool sqrtPriceX96: {})", tick_current, sqrt_price_x96);
     println!("  Total liq:    {}{}", effective_liquidity, if pos.liquidity == 0 && args.dry_run { " [synthetic for dry-run]" } else { "" });
     println!("  Remove:       {}% = {}", args.liquidity_pct, liquidity_to_remove);
     println!("  Tick range:   {} to {}", pos.tick_lower, pos.tick_upper);
+    println!("  Expected out: {} {} / {} {} (before slippage)", amount0_out, sym0, amount1_out, sym1);
+    println!("  Min out:      {} {} / {} {} ({}% slippage)", amount0_min, sym0, amount1_min, sym1, args.slippage);
     println!("  Owed fees:    {} {} / {} {}", pos.tokens_owed0, sym0, pos.tokens_owed1, sym1);
     println!("  NPM:          {}", cfg.npm);
 
@@ -51,17 +73,6 @@ pub async fn run(args: RemoveLiquidityArgs) -> Result<()> {
         crate::onchainos::get_wallet_address().await?
     };
 
-    // Compute slippage-based minimums using integer arithmetic.
-    // tokens_owed0/1 represent credits already accrued to the position; we use them as a
-    // conservative proxy for the amounts expected out of decreaseLiquidity. If the position
-    // has not yet accrued fees (tokens_owed == 0), we fall back to 0 to avoid reverting —
-    // this is safe for initial removals but offers no sandwich protection. Ideally the caller
-    // should pass the current reserve amounts derived from the pool's sqrt price, which
-    // requires off-chain math beyond the scope of this wrapper.
-    let slippage_bps = (args.slippage * 100.0) as u128;
-    let amount0_min = pos.tokens_owed0.saturating_mul(10000 - slippage_bps) / 10000;
-    let amount1_min = pos.tokens_owed1.saturating_mul(10000 - slippage_bps) / 10000;
-
     // Step 1: decreaseLiquidity
     println!("\nStep 1: Calling decreaseLiquidity...");
     println!("  amount0Min: {} (slippage {}%)", amount0_min, args.slippage);

skills/pancakeswap/src/main.rs

@@ -96,12 +96,12 @@ enum Commands {
         /// Human-readable amount for tokenB
         #[arg(long)]
         amount_b: String,
-        /// Lower tick boundary (must be multiple of tickSpacing)
+        /// Lower tick boundary (must be multiple of tickSpacing). Omit to auto-compute ±10% range from current pool price.
         #[arg(long, allow_hyphen_values = true)]
-        tick_lower: i32,
-        /// Upper tick boundary (must be multiple of tickSpacing)
+        tick_lower: Option<i32>,
+        /// Upper tick boundary (must be multiple of tickSpacing). Omit to auto-compute ±10% range from current pool price.
         #[arg(long, allow_hyphen_values = true)]
-        tick_upper: i32,
+        tick_upper: Option<i32>,
         /// Slippage tolerance in percent (e.g. 1.0 = 1%)
         #[arg(long, default_value = "1.0")]
         slippage: f64,

skills/pancakeswap/src/rpc.rs

@@ -299,6 +299,51 @@ pub async fn get_token_ids_for_owner(
     Ok(ids)
 }
 
+// ── V3 liquidity math ─────────────────────────────────────────────────────────
+
+/// Compute the actual token amounts held by a V3 position given the current pool price.
+///
+/// Uses f64 arithmetic (sufficient for slippage bound estimation — we only need
+/// ~1% accuracy, not wei-exact values).
+///
+/// Formula (from Uniswap V3 whitepaper):
+///   if tick < tickLower  → all token0: amount0 = L·(√B − √A) / (√A·√B)
+///   if tick ≥ tickUpper  → all token1: amount1 = L·(√B − √A)
+///   in range             → amount0 = L·(√B − √P) / (√P·√B)
+///                          amount1 = L·(√P − √A)
+///
+/// Returns (amount0, amount1) in minimal units (wei).
+pub fn amounts_from_liquidity(
+    sqrt_price_x96: u128,
+    tick_lower: i32,
+    tick_upper: i32,
+    tick_current: i32,
+    liquidity: u128,
+) -> (u128, u128) {
+    let q96 = (1u128 << 96) as f64;
+    let sqrt_p = sqrt_price_x96 as f64 / q96;
+    let sqrt_a = tick_to_sqrt_price(tick_lower);
+    let sqrt_b = tick_to_sqrt_price(tick_upper);
+    let liq = liquidity as f64;
+
+    if tick_current < tick_lower {
+        let amount0 = liq * (sqrt_b - sqrt_a) / (sqrt_a * sqrt_b);
+        (amount0 as u128, 0)
+    } else if tick_current >= tick_upper {
+        let amount1 = liq * (sqrt_b - sqrt_a);
+        (0, amount1 as u128)
+    } else {
+        let amount0 = liq * (sqrt_b - sqrt_p) / (sqrt_p * sqrt_b);
+        let amount1 = liq * (sqrt_p - sqrt_a);
+        (amount0 as u128, amount1 as u128)
+    }
+}
+
+/// sqrt(1.0001^tick) — the Q96 sqrt price at a given tick, as a plain f64.
+fn tick_to_sqrt_price(tick: i32) -> f64 {
+    f64::powf(1.0001_f64, tick as f64 / 2.0)
+}
+
 // ── Subgraph ──────────────────────────────────────────────────────────────────
 
 /// Query LP positions from TheGraph subgraph.