Move overflow guard to the actual vulnerable site:
ThreePositionStrategy._computeFloorTickWithSignal() line 262 where
vwapX96 >> 32 is cast to int128 for _tickAtPriceRatio. Values
exceeding int128.max now skip mirror tick (fallback to scarcity/clamp)
instead of reverting.
Remove incorrect require from Optimizer._buildInputs() which guarded
a non-existent int256 cast path.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add address(0) guard to fee transfer condition in _scrapePositions so
that when feeDestination is uninitialized, fees accrue as deployable
liquidity instead of reverting on safeTransfer to the zero address.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract bear-mode default values (0, 3e17, 100, 3e17) into file-level
constants in IOptimizer.sol so both Optimizer._bearDefaults() and
LiquidityManager.recenter()'s catch block reference a single source of
truth instead of independent hardcoded literals.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The @return annotations were orphaned after _buildInputs() was inserted
between the NatSpec block and getLiquidityParams(). Move them to directly
precede getLiquidityParams() where they belong.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The pure override in OptimizerInputCapture could not write to storage,
and getLiquidityParams calls calculateParams via staticcall which
prevents both storage writes and event emissions.
Fix: extract the input-building normalization from getLiquidityParams
into _buildInputs() (internal view, behavior-preserving refactor).
The test harness now exposes _buildInputs() via getComputedInputs(),
allowing tests to assert actual normalized slot values.
Updated tests for pricePosition, timeSinceRecenter, volatility,
momentum, and utilizationRate to assert non-zero captured values.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add FeeDestinationSet and FeeDestinationLocked events to LiquidityManager,
emitted on every setFeeDestination() call and lock engagement respectively.
Update tests to assert both events are emitted in all code paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the two per-slot require checks with a loop over all 8 input slots
so future subclasses using slots 2-7 are protected from silent uint256 wrap.
Add testCalculateParamsRevertsOnNegativeMantissaSlots2to7 to verify the guard.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The previous guard blocked setFeeDestination when feeDestination.code.length > 0
but did not persist feeDestinationLocked — a revert undoes all state changes. An
attacker could CREATE2-deploy bytecode to the EOA fee destination, triggering the
block, then SELFDESTRUCT to clear the code, then call setFeeDestination again
successfully (lock was never committed).
Fix: detect bytecode at the current feeDestination first; if found, set
feeDestinationLocked = true and RETURN (not revert) so the storage write is
committed. A subsequent SELFDESTRUCT cannot undo a committed storage slot.
Updated NatSpec documents both the protection and the remaining limitation
(atomic CREATE2+SELFDESTRUCT in a single tx cannot be detected).
Added testSetFeeDestination_CREATE2BytecodeDetection_Locks covering:
set EOA → vm.etch (simulate CREATE2 deploy) → verify lock committed → vm.etch
empty (simulate selfdestruct) → verify setter still blocked.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix class-level NatSpec: use accurate wording (width computed from
anchorWidth param provided by Optimizer) instead of imprecise
LiquidityManager attribution
- Fix inline comment in _setAnchorPosition (same stale 1-100% claim)
- Update PRODUCT-TRUTH.md and ARCHITECTURE.md which had the same
incorrect 1-100% range claim
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the misleading "(1-100% width)" range claim from the ANCHOR NatSpec.
Anchor width enforcement lives in LiquidityManager, not this abstract, so
the comment is replaced with a note pointing to where enforcement actually occurs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the MAX_ANCHOR_WIDTH=100 constant and the corresponding clamp on
anchorWidth in LiquidityManager.recenter(). The optimizer is now free to
choose any anchor width; evolution run 7 immediately exploited AW=153.
Update IOptimizer.sol NatSpec to reflect no clamping. Update the
testAnchorWidthAbove100IsClamped test to testAnchorWidthAbove100IsNotClamped,
asserting the tick range matches the full AW=150 width.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move the orphaned NatSpec block (originally for calculateParams) from
above getLiquidityParams to directly precede calculateParams, and give
getLiquidityParams only its own @inheritdoc block.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Optimizer: add `is IOptimizer` and mark getLiquidityParams() with
`override`, making the interface conformance explicit at the base level.
OptimizerV3 inherits it transitively via Optimizer.
- OptimizerV3Push3: add `is IOptimizer` and implement getLiquidityParams()
that calls calculateParams() with zeroed inputs, returning bear-mode
defaults (ci=0, anchorShare=0.3e18, anchorWidth=100, discoveryDepth=0.3e18).
Behaviour is identical to the previous try/catch fallback used by
LiquidityManager and the backtesting deployer.
- Update backtesting comments to reflect that getLiquidityParams() now
exists on OptimizerV3Push3 (returns bear defaults via zeroed inputs).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract magic number into named constant MAX_ANCHOR_WIDTH = 100 in LiquidityManager.sol
- Document effective ceiling in IOptimizer.sol natspec for anchorWidth return value
- Add testAnchorWidthAbove100IsClamped in LiquidityManager.t.sol asserting that
optimizer-returned anchorWidth=150 is silently clamped to 100 (not rejected)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add IOptimizer interface with getLiquidityParams() signature to IOptimizer.sol
so upgrade-compatibility is explicit and static analysis can catch ABI mismatches.
Update LiquidityManager to hold optimizer as IOptimizer instead of concrete Optimizer.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes#635
## Changes
The implementation is complete and committed. All 211 tests pass.
## Summary of changes
### `onchain/src/Optimizer.sol`
- **Replaced raw slot inputs** with normalized indicators in `getLiquidityParams()`:
- Slot 2 `pricePosition`: where current price sits within VWAP ± 11 000 ticks (0 = lower bound, 0.5e18 = at VWAP, 1e18 = upper bound)
- Slot 3 `volatility`: `|shortTwap − longTwap| / 1000 ticks`, capped at 1e18
- Slot 4 `momentum`: 0 = falling, 0.5e18 = flat, 1e18 = rising (5-min vs 30-min TWAP delta)
- Slot 5 `timeSinceRecenter`: `elapsed / 86400s`, capped at 1e18
- Slot 6 `utilizationRate`: 1e18 if current tick is within anchor position range, else 0
- **Extended `setDataSources()`** to accept `liquidityManager` + `token0isWeth` (needed for correct tick direction in momentum/utilizationRate)
- **Added `_vwapToTick()`** helper: converts `vwapX96 = price × 2⁹⁶` to tick via `sqrt(vwapX96) << 48`, with TickMath bounds clamping
- All slots gracefully default to 0 when data sources are unconfigured or TWAP history is insufficient (try/catch on `pool.observe()`)
### `onchain/src/OptimizerV3Push3.sol`
- Updated NatSpec to document the new `[0, 1e18]` slot semantics
### New tests (`onchain/test/`)
- `OptimizerNormalizedInputsTest`: 18 tests covering all new slots, token ordering, TWAP fallback, and a bounded fuzz test
- `mocks/MockPool.sol`: configurable `slot0()` + `observe()` with TWAP tick math
- `mocks/MockLiquidityManagerPositions.sol`: configurable anchor position bounds
Co-authored-by: openhands <openhands@all-hands.dev>
Reviewed-on: https://codeberg.org/johba/harb/pulls/649
Reviewed-by: review_bot <review_bot@noreply.codeberg.org>
Three defensive layers so every Push3 program runs without reverting:
Layer A (transpiler/index.ts): assign bear defaults (CI=0, AS=0.3e18,
AW=100, DD=0.3e18) to all four outputs at the top of calculateParams.
Any output the evolved program does not overwrite keeps the safe default.
Layer B (transpiler/transpiler.ts): graceful stack underflow — dpop/bpop
return '0'/'false' instead of throwing, and the final output-pop falls
back to bear-default literals when fewer than 4 values remain on the
stack. Wrong output count no longer aborts transpilation.
Layer C (transpiler/transpiler.ts + index.ts): wrap the entire function
body in `unchecked {}` so integer overflow wraps (matching Push3), and
emit `(b == 0 ? 0 : a / b)` for every DYADIC./ (div-by-zero → 0,
matching Push3 no-op semantics).
Layer 2 (Optimizer.sol getLiquidityParams): clamp the three fraction
outputs (capitalInefficiency, anchorShare, discoveryDepth) to [0, 1e18]
after abi.decode so a buggy evolved program cannot produce out-of-range
values even if it runs without reverting.
Regenerated OptimizerV3Push3.sol with the updated transpiler; all 193
tests pass (34 Optimizer/OptimizerV3Push3 tests explicitly).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Optimizer.sol: move CALCULATE_PARAMS_GAS_LIMIT constant to top of
contract (after error declaration) to avoid mid-contract placement.
Expand natspec with EIP-150 63/64 note: callers need ~203 175 gas to
deliver the full 200 000 budget to the inner staticcall.
- Optimizer.sol: add ret.length < 128 guard before abi.decode in
getLiquidityParams(). Malformed return data (truncated / wrong ABI)
from an evolved program now falls back to _bearDefaults() instead of
propagating an unhandled revert. The 128-byte minimum is the ABI
encoding of (uint256, uint256, uint24, uint256) — four 32-byte slots.
- Optimizer.sol: add cross-reference comment to _bearDefaults() noting
that its values must stay in sync with LiquidityManager.recenter()'s
catch block to prevent silent divergence.
- FitnessEvaluator.t.sol: add CALCULATE_PARAMS_GAS_LIMIT mirror constant
(must match Optimizer.sol). Disqualify candidates whose measured gas
exceeds the production cap with fitness=0 and error="gas_over_limit"
— prevents the pipeline from selecting programs that are functionally
dead on-chain (would always produce bear defaults in production).
- batch-eval.sh: update output format comment to document the gas_used
field and over-gas-limit error object added by this feature.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Optimizer.getLiquidityParams() now forwards calculateParams through a
staticcall capped at 200 000 gas. Programs that exceed the budget or
revert fall back to bear defaults (CI=0, AS=30%, AW=100, DD=0.3e18),
so a bloated evolved optimizer can never OOG-revert inside recenter().
- FitnessEvaluator.t.sol measures gas used by calculateParams against
fixed representative inputs (50% staked, 5% avg tax) after each
bootstrap. A soft penalty of GAS_PENALTY_FACTOR (1e13 wei/gas) is
subtracted from total fitness before the JSON score line is emitted.
Leaner programs win ties; gas_used is included in the output for
observability. At ~15k gas (current seed) the penalty is ~1.5e17 wei;
at the 200k hard cap boundary it reaches ~2e18 wei.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove unreachable else branch in VWAP recording logic. The branch was
only reachable if lastRecenterTick==0 and cumulativeVolume>0, which
requires tick==0 on the very first recenter — virtually impossible on a
live pool. Collapse else-if into else and delete the corresponding
testVWAPElseBranch test that exercised the path via vm.store.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fix the @return NatSpec for recenter() isUp: the previous description
was wrong for the token0=WETH ordering (claimed tick above center, but
the actual check is currentTick < centerTick when token0isWeth). The
correct invariant is isUp=true ↔ KRK price in ETH rose (buy event /
net ETH inflow), regardless of token ordering.
Also address review nit: StrategyExecutor._logRecenter() now logs
'direction=BOOTSTRAP' instead of 'direction=DOWN' when no anchor
position existed before the recenter (aLiqPre==0), eliminating the
misleading directional label on the first recenter.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add NatSpec to recenter() documenting that the function always reverts
on failure (never silently returns false), listing all four revert
conditions, and clarifying that both true/false return values represent
a successfully-executed recenter with the value indicating price
direction (up vs down relative to previous anchor centre).
Also fix StrategyExecutor.maybeRecenter() to capture the isUp return
value from lm.recenter() and include it in the log output, making
price direction visible in backtesting replays.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- LiquidityManager.setFeeDestination: add CREATE2 bypass guard — also
blocks re-assignment when the current feeDestination has since acquired
bytecode (was a plain address when set, contract deployed to it later)
- LiquidityManager.setFeeDestination: expand NatSpec to document the
EOA-mutability trade-off and the CREATE2 guard explicitly
- Test: add testSetFeeDestinationEOAToContract_Locks covering the
realistic EOA→contract transition (the primary lock-activation path)
- red-team.sh: add comment that DEPLOYER_PK is Anvil account-0 and must
only be used against a local ephemeral Anvil instance
- ARCHITECTURE.md: document feeDestination conditional-lock semantics and
contrast with Kraiken's strictly set-once liquidityManager/stakingPool
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add `feeDestinationLocked` bool to LiquidityManager
- Replace one-shot setter with conditional trapdoor: EOAs may be set
repeatedly, but setting a contract address locks permanently
- Remove `AddressAlreadySet` error (superseded by the new lock mechanic)
- Replace fragile SLOT7 storage hack in red-team.sh with a proper
`setFeeDestination()` call using the deployer key
- Update tests: replace AddressAlreadySet test with three new tests
covering EOA multi-set, contract lock, and locked revert
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add require(mantissa >= 0) guards in calculateParams before the uint256()
casts on inputs[0] and inputs[1], preventing negative int256 values from
wrapping to huge uint256 numbers and corrupting liquidity calculations.
Add two regression tests covering the revert paths for both slots.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace pool.observe() TWAP price source with current pool tick (pool.slot0()) sampled once per recenter
- Remove _getTWAPOrFallback() and TWAPFallback event (added by PR #575)
- _scrapePositions now takes int24 currentTick instead of uint256 prevTimestamp; price computed via _priceAtTick before the burn loop
- Volume weighting (ethFee * 100) is unchanged — fees proxy swap volume over the recenter interval
- Direction fix from #566 (shouldRecordVWAP only on sell events) is preserved
- Remove test_twapReflectsAveragePriceNotJustLastSwap (tested reverted TWAP behaviour)
- ORACLE_CARDINALITY / increaseObservationCardinalityNext retained for _isPriceStable()
- All 188 tests pass
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add virtual to Optimizer.calculateParams() for UUPS override
- Create OptimizerV3.sol: UUPS-upgradeable optimizer with transpiled Push3 logic
- Update deploy-optimizer.sh to deploy OptimizerV3 instead of Optimizer
- Add ~/.foundry/bin to PATH in evolve.sh, fitness.sh, deploy-optimizer.sh
- Fix unsafe int32 intermediate cast: int56(int32(elapsed)) → int56(uint56(elapsed))
to prevent TWAP tick sign inversion for intervals above int32 max (~68 years)
- Remove redundant lastRecenterTimestamp state variable; capture prevTimestamp
from existing lastRecenterTime instead (saves ~20k gas per recenter)
- Use pool.increaseObservationCardinalityNext(ORACLE_CARDINALITY) in constructor
instead of recomputing the pool address; extract magic 100 to named constant
- Add TWAPFallback(uint32 elapsed) event emitted when pool.observe() reverts
so monitoring can distinguish degraded operation from normal bootstrap
- Remove conditional bypass paths in test_twapReflectsAveragePriceNotJustLastSwap;
assert vwapAfter > 0 and vwapAfter > initialPriceX96 unconditionally
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add lastRecenterTimestamp to track recenter interval for TWAP
- Increase pool observation cardinality to 100 in constructor
- In _scrapePositions, use pool.observe([elapsed, 0]) to get TWAP tick
over the full interval between recenters; falls back to anchor midpoint
when elapsed==0 or pool.observe() reverts (insufficient history)
- Add test_twapReflectsAveragePriceNotJustLastSwap: verifies TWAP-based
VWAP reflects the average price across the recenter interval, not just
the last-swap anchor snapshot
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- _scrapePositions natspec: 'ETH inflow' → 'ETH outflow / price fell or at bootstrap'
- Inline comment above VWAP block: remove inverted 'KRK sold out / ETH inflow' rationale,
replace with a neutral forward-reference to recenter() where the direction logic lives
- VWAPFloorProtection.t.sol: remove unused Kraiken and forge-std/Test.sol imports
(both are already provided by UniSwapHelper)
- test_floorConservativeAfterBuyOnlyAttack: add assertFalse(token0isWeth) guard so a
future change to the setUp parameter cannot silently invert the gap-direction assertion
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Investigation findings:
- VWAP WAS being fed during buy-only cycles (shouldRecordVWAP = true on ETH inflow / price rising).
Over 80 buy-recenter cycles VWAP converged toward the inflated current price.
- When VWAP ≈ currentTick, mirrorTick = currentTick + vwapDistance ≈ currentTick, placing
the floor near the inflated price. Adversary sells back through the high floor, extracting
nearly all LM ETH.
- Optimizer parameters (anchorShare, CI) were not the primary cause.
Fix (LiquidityManager.sol):
Flip shouldRecordVWAP from buy direction to sell direction. VWAP is now recorded only when
price falls (ETH outflow / sell events) or at initial bootstrap (cumulativeVolume == 0).
Buy-only attack cycles leave VWAP frozen at the historical baseline, keeping mirrorTick and
the floor conservatively anchored far from the inflated current price.
Also updated onchain/AGENTS.md to document the corrected recording direction.
Regression test (VWAPFloorProtection.t.sol):
- test_vwapNotInflatedByBuyOnlyAttack: asserts getVWAP() stays at bootstrap after N buy cycles.
- test_floorConservativeAfterBuyOnlyAttack: asserts floor center is far below inflated tick.
- test_vwapBootstrapsOnFirstFeeEvent: confirms bootstrap path unchanged.
- test_recenterSucceedsOnSellDirectionWithoutReverts: confirms sell-direction recenters work.
All 187 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When feeDestination == address(this), _scrapePositions() now skips the
fee safeTransfer calls so collected WETH/KRK stays in the LM balance
and is redeployed as liquidity on the next _setPositions() call.
Also fixes _getOutstandingSupply(): kraiken.outstandingSupply() already
subtracts balanceOf(liquidityManager), so when feeDestination IS the LM
the old code double-subtracted LM-held KRK, causing an arithmetic
underflow once positions were scraped. The subtraction is now skipped
for the self-referencing case.
VWAP recording is refactored to a single unconditional block so it fires
regardless of fee destination.
New test testSelfFeeDestination_FeesAccrueAsLiquidity() demonstrates
that a two-recenter cycle with self-feeDestination completes without
underflow and without leaking WETH to any external address.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
