fix: address review findings in FitnessEvaluator (#604)

- Wrap upgradeTo() in try/catch: malformed candidate bytecode no longer
  aborts the entire batch; emit {"fitness":0,"error":"upgrade_failed"} and
  continue to the next candidate
- Bootstrap recenter: require() after 5 retry attempts so silent failure
  (all scores identically equal to free WETH only) is surfaced as a hard
  test failure rather than silently producing meaningless results
- mint_lp: capture the NPM tokenId returned by mint() and push it to
  _mintedNpmTokenIds; burn_lp now uses a 1-based index into that array
  (same pattern as stake/unstake), making attack files fork-block-independent
- Remove dead atkBaseSnap variable and its compiler-warning suppression
- Remove orphaned vm.snapshot() after vm.revertTo() in the attack loop
- Fix misleading comment on delete _stakedPositionIds

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

onchain/test/FitnessEvaluator.t.sol

@@ -169,6 +169,11 @@ contract FitnessEvaluator is Test {
     ///      vm.snapshot/revertTo reverts this array's storage between attacks.
     uint256[] internal _stakedPositionIds;
 
+    /// @dev NPM tokenIds returned by mint_lp ops (in insertion order).
+    ///      burn_lp references positions by 1-based index into this array so that
+    ///      attack files are fork-block-independent (tokenIds vary by fork tip).
+    uint256[] internal _mintedNpmTokenIds;
+
     // ─── Entry point ─────────────────────────────────────────────────────────
 
     /**
@@ -217,17 +222,24 @@ contract FitnessEvaluator is Test {
             baseSnap = vm.snapshot();
 
             // Etch candidate optimizer bytecode and upgrade proxy.
+            // Wrapped in try/catch: a malformed candidate (compiler bug, bad transpiler output)
+            // would otherwise abort the entire batch.  On failure, emit fitness=0 and continue;
+            // vm.revertTo(baseSnap) at the top of the next iteration cleans up state.
             bytes memory candidateBytecode = vm.parseBytes(bytecodeHex);
             vm.etch(IMPL_SLOT, candidateBytecode);
-            // Test contract is the UUPS admin (set during initialize in _deploy).
-            UUPSUpgradeable(optProxy).upgradeTo(IMPL_SLOT);
+            bool upgradeOk = true;
+            try UUPSUpgradeable(optProxy).upgradeTo(IMPL_SLOT) { }
+            catch {
+                upgradeOk = false;
+            }
+            if (!upgradeOk) {
+                console.log(string.concat('{"candidate_id":"', candidateId, '","fitness":0,"error":"upgrade_failed"}'));
+                continue;
+            }
 
             // Bootstrap: fund LM, set recenterAccess, initial recenter.
             _bootstrap();
 
-            // Post-bootstrap snapshot used to reset state between attacks.
-            uint256 atkBaseSnap = vm.snapshot();
-
             // Score: sum lm_eth_total across all attack sequences.
             uint256 totalFitness = 0;
             Vm.DirEntry[] memory entries = vm.readDir(attacksDir);
@@ -238,15 +250,10 @@ contract FitnessEvaluator is Test {
                 uint256 score = _runAttack(entries[i].path);
                 totalFitness += score;
                 vm.revertTo(atkSnap);
-                atkSnap = vm.snapshot(); // refresh for next attack
             }
 
             // Emit score as a JSON line (parsed by batch-eval.sh).
             console.log(string.concat('{"candidate_id":"', candidateId, '","fitness":', _uint2str(totalFitness), "}"));
-
-            // Restore atkBaseSnap snapshot variable (already consumed above) — not needed
-            // since we always revert to baseSnap at the top of the loop.
-            (atkBaseSnap); // suppress unused variable warning
         }
 
         // Close manifest files.
@@ -340,18 +347,18 @@ contract FitnessEvaluator is Test {
 
         // e. Initial recenter: no ANCHOR position exists yet so amplitude check is skipped;
         //    recenterAccess is set so TWAP stability check is also skipped.
-        vm.prank(recenterAddr);
-        try ILM(lmAddr).recenter() { }
-        catch {
-            // If recenter fails on first attempt (e.g. pool not ready), mine some blocks and retry.
-            for (uint256 _attempt = 0; _attempt < 4; _attempt++) {
-                vm.roll(block.number + 50);
-                vm.prank(recenterAddr);
-                try ILM(lmAddr).recenter() returns (bool) {
-                    break;
-                } catch { }
-            }
+        //    If all retries fail, revert with a clear message — silent failure would make every
+        //    candidate score identically (all lm_eth_total = free WETH only, no positions).
+        bool recentered = false;
+        for (uint256 _attempt = 0; _attempt < 5; _attempt++) {
+            if (_attempt > 0) vm.roll(block.number + 50);
+            vm.prank(recenterAddr);
+            try ILM(lmAddr).recenter() returns (bool) {
+                recentered = true;
+                break;
+            } catch { }
         }
+        require(recentered, "FitnessEvaluator: bootstrap recenter failed after 5 attempts");
     }
 
     // ─── Attack execution ─────────────────────────────────────────────────────
@@ -363,9 +370,11 @@ contract FitnessEvaluator is Test {
     function _runAttack(string memory attackFile) internal returns (uint256) {
         // Reset file read position so each call to _runAttack starts from line 1.
         vm.closeFile(attackFile);
-        // Clear any staked positions from a prior attack (snapshot revert handles on-chain state,
-        // but the dynamic array in test storage also needs resetting).
+        // vm.revertTo() reverts all EVM state including test contract storage, so these
+        // arrays are already empty after revert.  Explicit delete is a defensive reset
+        // for the first attack (no preceding revert) and any future call-path changes.
         delete _stakedPositionIds;
+        delete _mintedNpmTokenIds;
 
         string memory line = vm.readLine(attackFile);
         while (bytes(line).length > 0) {
@@ -439,7 +448,9 @@ contract FitnessEvaluator is Test {
             uint256 amount1 = vm.parseUint(vm.parseJsonString(line, ".amount1"));
             (address t0, address t1) = token0isWeth ? (WETH_ADDR, krkAddr) : (krkAddr, WETH_ADDR);
             vm.prank(advAddr);
-            INonfungiblePositionManager(NPM_ADDR).mint(
+            // Track the returned tokenId so burn_lp can reference it by 1-based index,
+            // making attack files fork-block-independent (NPM tokenIds depend on fork tip).
+            (uint256 mintedTokenId,,,) = INonfungiblePositionManager(NPM_ADDR).mint(
                 INonfungiblePositionManager.MintParams({
                     token0: t0,
                     token1: t1,
@@ -454,8 +465,17 @@ contract FitnessEvaluator is Test {
                     deadline: block.timestamp + 3600
                 })
             );
+            _mintedNpmTokenIds.push(mintedTokenId);
         } else if (_eq(op, "burn_lp")) {
-            uint256 tokenId = vm.parseJsonUint(line, ".tokenId");
+            // .tokenId in the attack file is a 1-based index into _mintedNpmTokenIds
+            // (positions created by mint_lp ops in this run), not a raw NPM tokenId.
+            // This mirrors the stake/unstake index pattern and avoids fork-block sensitivity.
+            uint256 tokenIndex = vm.parseJsonUint(line, ".tokenId");
+            require(
+                tokenIndex >= 1 && tokenIndex <= _mintedNpmTokenIds.length,
+                "FitnessEvaluator: burn_lp tokenId out of range (must be 1-based index of a prior mint_lp op)"
+            );
+            uint256 tokenId = _mintedNpmTokenIds[tokenIndex - 1];
             (,,,,,, , uint128 liquidity,,,,) = INonfungiblePositionManager(NPM_ADDR).positions(tokenId);
             if (liquidity == 0) return;
             vm.startPrank(advAddr);