fix: AttackRunner round-3 review findings

- **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 <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-11 17:51:18 +00:00
parent 297442083d
commit d6ca28ae32

View file

@ -130,7 +130,7 @@ interface IUniswapV3Factory {
* buy Swap WETHKRK via SwapRouter. Fields: amount (wei string), token (ignored, WETH assumed)
* sell Swap KRKWETH 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",
"}"
);
}