- _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>
Add an explanatory comment to uniswapV3SwapCallback clarifying that
address(this) is pre-funded by _replaySwap before pool.swap() is
called, so no inline mint is required (unlike uniswapV3MintCallback).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix fee attribution: distribute fees only to positions whose tick range
contains the active tick at close time (in-range weight), not by raw
liquidity. FLOOR is priced far below current tick and rarely earns fees;
the old approach would over-credit it and corrupt capital-efficiency and
net-P&L numbers. Fallback to raw-liquidity weighting with a WARN log
when no position is in range.
- Warn on first-close skip: when _closePosition finds no open record
(first recenter, before any tracking), log [TRACKER][WARN] instead of
silently returning so the gap is visible in reports.
- Add tick range assertion: require() that the incoming close snapshot
tick range matches the stored open record — a mismatch would mean IL
is computed across different ranges (apples vs oranges).
- Fix finalBlock accuracy: logSummary now calls
tracker.logFinalSummary(tracker.lastNotifiedBlock()) instead of
lastRecenterBlock, so the summary reflects the actual last replay block
rather than potentially hundreds of blocks early.
- Initialize lastRecenterBlock = block.number in StrategyExecutor
constructor to defer the first recenter attempt by recenterInterval
blocks and document the invariant.
- Extract shared FormatLib: _str(uint256) and _istr(int256) were
copy-pasted in both PositionTracker and StrategyExecutor. Extracted to
FormatLib.sol internal library; both contracts now use `using FormatLib`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add PositionTracker.sol: tracks position lifecycle (open/close per
recenter), records tick ranges, liquidity, entry/exit blocks/timestamps,
token amounts (via LiquidityAmounts math), fees (proportional to
liquidity share), IL (LP exit value − HODL value at exit price), and
net P&L per position. Aggregates total fees, cumulative IL, net P&L,
rebalance count, Anchor time-in-range, and capital efficiency accumulators.
Logs with [TRACKER][TYPE] prefix; emits cumulative P&L every 500 blocks.
- Modify StrategyExecutor.sol: add IUniswapV3Pool + token0isWeth to
constructor (creates PositionTracker internally), call
tracker.notifyBlock() on every block for time-in-range, and call
tracker.recordRecenter() on each successful recenter. logSummary()
now delegates to tracker.logFinalSummary().
- Modify BacktestRunner.s.sol: pass sp.pool and token0isWeth to
StrategyExecutor constructor; log tracker address.
- forge fmt: reformat all backtesting scripts and affected src/test files
to project style (number_underscore=thousands, multiline_func_header=all).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add BacktestKraiken.sol: extends MockToken with Kraiken-compatible interface
(dual mint overloads — public mint(address,uint256) for EventReplayer and
restricted mint(uint256) for LiquidityManager; peripheryContracts() stubs
staking pool as address(0))
- Add KrAIkenDeployer.sol: library deploying OptimizerV3Push3 + LiquidityManager
on the shadow pool, wiring BacktestKraiken permissions, setting fee destination,
and funding LM with configurable initial mock-WETH capital (default 10 ETH)
- Add StrategyExecutor.sol: time-based recenter trigger (configurable block
interval, default 100 blocks); logs block, pre/post positions (Floor/Anchor/
Discovery tick ranges + liquidity), fees collected, and revert reason on skip;
negligible-impact assumption documented as TODO(#319)
- Modify EventReplayer.sol: add overloaded replay() accepting an optional
StrategyExecutor hook; maybeRecenter() called after each block advancement
without halting replay on failure
- Modify BacktestRunner.s.sol: replace tokenA/B with MockWETH + BacktestKraiken,
integrate KrAIkenDeployer + StrategyExecutor into broadcast block; configurable
via RECENTER_INTERVAL and INITIAL_CAPITAL_WETH env vars; executor.logSummary()
printed after replay
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Guard final drift sample with `idx % LOG_INTERVAL != 0` to prevent
double-counting stats when totalReplayed is an exact multiple of
LOG_INTERVAL (the loop's _logCheckpoint already fired for that state)
- Hoist pool.slot0() before the guard and pass finalSqrtPrice/finalTick
to _logSummary(), eliminating the redundant slot0 read inside it
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Cache pool.tickSpacing() as immutable in EventReplayer constructor
to avoid a repeated external call per _replayMint() invocation
- Rename driftCount → driftCheckpoints for consistency with log label
- Add sqrtDriftBps to the per-checkpoint progress log line, using the
now-live lastExpectedSqrtPrice field (previously written but never read)
- Guard _replaySwap(): skip and count events where amountSpecified ≤ 0,
which would silently flip exact-input into exact-output mode
- Add a final drift sample after the while-loop for trailing events not
covered by the last LOG_INTERVAL checkpoint
- Move EventReplayer construction outside the broadcast block in
BacktestRunner (it uses vm.* cheat codes incompatible with real RPC)
- Change second vm.closeFile() from try/catch to a direct call so errors
surface rather than being silently swallowed
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Check pos.creationTime == 0 before pos.owner != msg.sender so that
calling exitPosition on a non-existent position correctly reverts with
PositionNotFound instead of the misleading NoPermission(caller, 0x0).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace hardcoded Infura API key with INFURA_API_KEY env var; fail fast
with a helpful message if unset and no --rpc-url is given
- Add onchain/script/backtesting/.gitignore (cache/) instead of relying on
the opaque root pattern; remove force-tracked cache/.gitkeep (mkdirSync
creates the directory at runtime)
- Document resume constraint: reliable only when both --start-block and
--end-block are explicit, or --output is set
- Fix batch-number display: derive batchNum inside the loop from the actual
`from` block so it stays correct when resumeFromBlock isn't BATCH_SIZE-aligned
- Guard log.logIndex === null consistently with blockNumber/transactionHash
- console.warn on decode errors instead of silently discarding them
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add onchain/script/backtesting/fetch-events.ts — a tsx script that:
- Fetches Swap/Mint/Burn events from a Uniswap V3 pool via Infura (Base mainnet)
- Batches eth_getLogs in 2 000-block chunks with 100 ms inter-batch delay
- Decodes each log with viem and writes one JSON Line per event
- Supports resume: reads last block from existing cache file on re-run
- Retries with exponential back-off on 429 / rate-limit errors
- Prints per-batch progress: "Fetching blocks X-Y... N events (B/T batches)"
Also adds package.json, tsconfig.json, and cache/.gitkeep.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
OptimizerV3Push3 is an equivalence-proof contract with only isBullMarket().
It cannot serve as an ERC1967Proxy implementation because it has no initialize()
or getLiquidityParams(). The CI bootstrap was failing because the proxy
deployment reverted when calling initialize() on the Push3 implementation.
Switch deploy scripts to Optimizer.sol (the base UUPS contract) which has the
full interface required by ERC1967Proxy and LiquidityManager.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add 11 new targeted tests in Stake.t.sol to cover all reachable
uncovered branches and the untested permitAndSnatch() function:
- testRevert_TaxRateOutOfBounds_InSnatch: taxRate >= TAX_RATES.length in snatch()
- testRevert_PositionNotFound_NonLastInLoop: PositionNotFound inside the multi-position loop
- testRevert_TaxTooLow_NonLastInLoop: TaxTooLow inside the multi-position loop
- testSnatch_ExitLastPosition: _exitPosition() path for last snatched position
- testRevert_ExceededAvailableStake: no available stake, no positions provided
- testRevert_TooMuchSnatch_AvailableExceedsNeed: post-exit excess stake check
- testRevert_PositionNotFound_InChangeTax: changeTax() on non-existent position
- testRevert_TaxTooLow_InChangeTax: changeTax() with same/lower tax rate
- testRevert_NoPermission_InExitPosition: exitPosition() by non-owner
- testRevert_PositionNotFound_InPayTax: payTax() on non-existent position
- testPermitAndSnatch: EIP-712 permit + snatch in one transaction
Coverage achieved:
Lines: 99.33% (148/149)
Statements: 99.40% (167/168)
Branches: 93.55% (29/31) — 2 unreachable dead-code branches remain
Functions: 100.00% (15/15)
The 2 uncovered branches are dead code: the require() failure in
_shrinkPosition (caller always guards sharesToTake < pos.share) and
the PositionNotFound guard in exitPosition() (unreachable because
owner and creationTime are always set/cleared together, so
pos.owner==msg.sender implies pos.creationTime!=0 for any live caller).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove redundant `node_modules/` from onchain/.gitignore — the root
.gitignore already has `**/node_modules/` which covers the entire tree.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add `require(averageTaxRate <= 1e18, "Invalid tax rate")` to match
the existing `percentageStaked` guard and prevent silent acceptance
of out-of-range values.
- Expand contract-level NatSpec with a @dev note clarifying this is an
equivalence proof only: it intentionally exposes `isBullMarket` alone
and is not a deployable upgrade (full optimizer interface missing).
All 15 Foundry tests pass (15 unit + fuzz).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
onchain/ uses Foundry for dependency management, not yarn/npm.
Adding yarn.lock, package-lock.json, and node_modules/ to .gitignore
prevents accidental commits of JS toolchain artifacts in future.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yarn install was run during forge build troubleshooting; the generated
lock file was not intentional and is architecturally inconsistent with
the Foundry-only onchain/ toolchain. Also restores package-lock.json
to its pre-npm-install state.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PROBLEM:
Recenter operations were burning ~137,866 KRK tokens instead of minting
them, causing severe deflation when inflation should occur. This was due
to the liquidity manager burning ALL collected tokens from old positions
and then minting tokens for new positions separately, causing asymmetric
supply adjustments to the staking pool.
ROOT CAUSE:
During recenter():
1. _scrapePositions() collected tokens from old positions and immediately
burned them ALL (+ proportional staking pool adjustment)
2. _setPositions() minted tokens for new positions (+ proportional
staking pool adjustment)
3. The burn and mint operations used DIFFERENT totalSupply values in
their proportion calculations, causing imbalanced adjustments
4. When old positions had more tokens than new positions needed, the net
result was deflation
WHY THIS HAPPENED:
When KRK price increases (users buying), the same liquidity depth
requires fewer KRK tokens. The old code would:
- Burn 120k KRK from old positions (+ 30k from staking pool)
- Mint 10k KRK for new positions (+ 2.5k to staking pool)
- Net: -137.5k KRK total supply (WRONG!)
FIX:
1. Modified uniswapV3MintCallback() to use existing KRK balance first
before minting new tokens
2. Removed burn() from _scrapePositions() - keep collected tokens
3. Removed burn() from end of recenter() - don't burn "excess"
4. Tokens held by LiquidityManager are already excluded from
outstandingSupply(), so they don't affect staking calculations
RESULT:
Now during recenter, only the NET difference is minted or used:
- Collect old positions into LiquidityManager balance
- Use that balance for new positions
- Only mint additional tokens if more are needed
- Keep any unused balance for future recenters
- No more asymmetric burn/mint causing supply corruption
VERIFICATION:
- All 107 existing tests pass
- Added 2 new regression tests in test/SupplyCorruption.t.sol
- testRecenterDoesNotCorruptSupply: verifies single recenter preserves supply
- testMultipleRecentersPreserveSupply: verifies no accumulation over time
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>