From d6ca28ae321c7ac04b4aa6772724b0751919dbce Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 11 Mar 2026 17:51:18 +0000 Subject: [PATCH] fix: AttackRunner round-3 review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - **Bug**: Fix JSON malformation in _snapshotPositions — closing literal was '"}}}' (three braces) but only '"}}' is needed (close discovery{} + positions{}). The third brace prematurely closed the root object, making every snapshot unparseable downstream. - **Nit**: _executeStake local variable renamed taxRateIndex → taxRate to match the IStake interface and Stake.sol. JSONL field key '.taxRateIndex' is kept for backward compatibility with existing attack files; the comment and NatDoc header now say so. - **Nit**: recenter_is_up now emits JSON null (not false) before the first recenter call, via a new _hasRecentered flag. Downstream parsers can distinguish "no recenter yet" from "last recenter moved price down" (false). _hasRecentered is set to true alongside _lastRecenterIsUp in the recenter handler. - **Nit**: Added a comment to _logSnapshot explaining that pool.slot0() is a view call and forge-std finalises broadcast state before executing it, so tick/sqrtPrice are always post-broadcast accurate. Co-Authored-By: Claude Sonnet 4.6 --- onchain/script/backtesting/AttackRunner.s.sol | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/onchain/script/backtesting/AttackRunner.s.sol b/onchain/script/backtesting/AttackRunner.s.sol index 1a148ad..e0a33b6 100644 --- a/onchain/script/backtesting/AttackRunner.s.sol +++ b/onchain/script/backtesting/AttackRunner.s.sol @@ -130,7 +130,7 @@ interface IUniswapV3Factory { * buy Swap WETH→KRK via SwapRouter. Fields: amount (wei string), token (ignored, WETH assumed) * sell Swap KRK→WETH via SwapRouter. Fields: amount (wei string or "all"), token (ignored) * recenter Call LM.recenter() via recenterAccess account. Emits a snapshot. - * stake Call Stake.snatch(). Fields: amount (wei string), taxRateIndex (0-29) + * stake Call Stake.snatch(). Fields: amount (wei string), taxRateIndex (raw taxRate value passed to Stake.snatch) * unstake Call Stake.exitPosition(). Fields: positionId * mint_lp Add LP via NPM. Fields: tickLower, tickUpper, amount0 (wei string), amount1 (wei string) * burn_lp Remove LP via NPM. Fields: tokenId @@ -172,6 +172,10 @@ contract AttackRunner is Script { /// @dev Direction of the most recent recenter: true = price moved up, false = price moved down. /// Read by _logSnapshot to include in post-recenter snapshots. bool internal _lastRecenterIsUp; + /// @dev Set to true after the first recenter call. Allows _logSnapshot to emit + /// recenter_is_up=null on the initial snapshot (before any recenter has occurred) + /// rather than the ambiguous false default. + bool internal _hasRecentered; // ─── Entry point ───────────────────────────────────────────────────────── @@ -249,6 +253,7 @@ contract AttackRunner is Script { // recenter() reverts (not returns false) when amplitude is insufficient, // so a successful call is always a real recenter regardless of direction. _lastRecenterIsUp = ILM(lmAddr).recenter(); + _hasRecentered = true; vm.stopBroadcast(); isRecenter = true; } else if (_eq(op, "stake")) { @@ -306,11 +311,13 @@ contract AttackRunner is Script { } /// @dev Stake KRK via Stake.snatch() with no snatching. + /// Attack files use the field key ".taxRateIndex" for backward compatibility; + /// the value is passed directly as a raw taxRate to Stake.snatch(). function _executeStake(string memory line) internal { uint256 amount = vm.parseUint(vm.parseJsonString(line, ".amount")); - uint32 taxRateIndex = uint32(vm.parseJsonUint(line, ".taxRateIndex")); + uint32 taxRate = uint32(vm.parseJsonUint(line, ".taxRateIndex")); // JSONL key kept for compat vm.startBroadcast(ADV_PK); - IStake(stakeAddr).snatch(amount, advAddr, taxRateIndex, new uint256[](0)); + IStake(stakeAddr).snatch(amount, advAddr, taxRate, new uint256[](0)); vm.stopBroadcast(); } @@ -429,6 +436,8 @@ contract AttackRunner is Script { uint256 advKrk = IERC20(krkAddr).balanceOf(advAddr); // Emit snapshot as a single JSON line. + // NOTE: pool.slot0() is read as a view call; forge-std finalises broadcast state before + // executing view calls, so the sqrtPriceX96/tick values are always post-broadcast. console.log(_buildSnapshotJson( seq, tick, lmEthFree, lmWethFree, lmEthTotal, fLiq, fLo, fHi, fEthValue, @@ -438,7 +447,7 @@ contract AttackRunner is Script { outstandingSupply, totalSupply, anchorShare, capitalInefficiency, anchorWidth, discoveryDepth, advEth, advKrk, - _lastRecenterIsUp + _lastRecenterIsUp, _hasRecentered )); } @@ -473,7 +482,8 @@ contract AttackRunner is Script { uint256 discoveryDepth, uint256 advEth, uint256 advKrk, - bool recenterIsUp + bool recenterIsUp, + bool hasRecentered ) internal pure @@ -487,7 +497,7 @@ contract AttackRunner is Script { outstandingSupply, totalSupply, anchorShare, capitalInefficiency, anchorWidth, discoveryDepth, advEth, advKrk, - recenterIsUp + recenterIsUp, hasRecentered ) ); } @@ -561,7 +571,7 @@ contract AttackRunner is Script { int256(dHi).istr(), ',"ethValue":"', dEthValue.str(), - '"}}}' + '"}}' // close discovery{} then positions{}; root object is closed by _snapshotFooter ); } @@ -576,7 +586,8 @@ contract AttackRunner is Script { uint256 discoveryDepth, uint256 advEth, uint256 advKrk, - bool recenterIsUp + bool recenterIsUp, + bool hasRecentered ) internal pure @@ -603,8 +614,10 @@ contract AttackRunner is Script { advEth.str(), '","adversary_krk":"', advKrk.str(), + // Emit null before the first recenter so downstream parsers can distinguish + // "no recenter yet" from "last recenter moved price down" (false). '","recenter_is_up":', - recenterIsUp ? "true" : "false", + hasRecentered ? (recenterIsUp ? "true" : "false") : "null", "}" ); }