diff --git a/onchain/DEBUGGING_REFLECTION.md b/onchain/DEBUGGING_REFLECTION.md deleted file mode 100644 index 7e396b5..0000000 --- a/onchain/DEBUGGING_REFLECTION.md +++ /dev/null @@ -1,96 +0,0 @@ -# Debugging Reflection: Floor Position Calculation Bug - -## Summary -Fixed a critical bug where the floor position was placed at extreme ticks (141,200+) instead of bordering the anchor position due to incorrect token amount calculation in the discovery position logic. - -## The Debugging Journey - -### Initial Misunderstandings -1. **ETH Flow Direction**: Initially thought ETH flows OUT when traders buy KRAIKEN (wrong - it flows IN) -2. **Anchor Share Interpretation**: Misunderstood that 95% anchorShare reduces floor allocation to 90.1% -3. **Capital Inefficiency**: Didn't grasp that 0% means KRAIKEN is undervalued at 70% for reserves - -### Key Discoveries -1. **VWAP Stores Price²**: Found that VWAP stores squared price, leading to incorrect ETH requirement calculations -2. **Operator Precedence Bug**: Fixed missing parentheses in midpoint calculation -3. **Final Root Cause**: Discovery position was calculating ETH amount instead of KRAIKEN amount - -## Missing Knowledge & Tools - -### 1. **Better Logging Infrastructure** -- **Issue**: Console.log wasn't working properly in Forge scripts -- **Impact**: Had to rely on events and CSV output instead of direct debugging -- **Ideal Solution**: Built-in structured logging that works in all contexts with: - - Decimal formatting for uint256 values - - Automatic unit conversion (wei to ether) - - Structured output for complex calculations - -### 2. **Calculation Tracing Tools** -- **Issue**: Couldn't easily trace intermediate calculation values -- **Impact**: Had to create multiple test contracts to understand the math -- **Ideal Solution**: A calculation debugger that shows: - ``` - Outstanding Supply Calculation: - - Total Supply: 99,174,468 KRAIKEN - - Anchor KRAIKEN: 3,014,586 KRAIKEN ✓ - - Discovery KRAIKEN: 0.00002 ETH ✗ (wrong token!) - - Final Outstanding: 96,159,882 KRAIKEN (should be 2,475,277) - ``` - -### 3. **Token Flow Visualization** -- **Issue**: Hard to track which tokens go where during position minting -- **Impact**: Took longer to realize discovery amount was in wrong token -- **Ideal Solution**: Visual token flow diagram showing: - - Token types and amounts at each step - - Which calculations use ETH vs KRAIKEN - - Token destinations (pool vs liquidity manager) - -### 4. **Unit Testing for Intermediate Values** -- **Issue**: Tests only validated final outcomes, not intermediate calculations -- **Impact**: Bug passed all tests despite wrong intermediate values -- **Ideal Solution**: Tests that assert on intermediate calculations: - ```solidity - assertEq(discoveryAmount, expectedKraikenAmount, "Discovery should calculate KRAIKEN"); - assertEq(outstandingAfterSubtraction, expectedOutstanding, "Outstanding should exclude positions"); - ``` - -### 5. **Type Safety for Token Amounts** -- **Issue**: Both ETH and KRAIKEN amounts are uint256, easy to mix up -- **Impact**: Wrong token amount was used without compile-time detection -- **Ideal Solution**: Typed token amounts: - ```solidity - struct EthAmount { uint256 value; } - struct KraikenAmount { uint256 value; } - ``` - -### 6. **Invariant Monitoring** -- **Issue**: No runtime checks for protocol invariants -- **Impact**: Extreme tick placement wasn't flagged as suspicious -- **Ideal Solution**: Runtime invariant checks: - - "Floor should be within X ticks of anchor in normal conditions" - - "Outstanding supply should approximate actual circulating tokens" - - "ETH scarcity shouldn't occur with net ETH inflows" - -## Process Improvements - -### What Worked Well -1. **CSV Output**: Positions CSV clearly showed the problem -2. **Event Logs**: EthScarcity events provided crucial calculation values -3. **User Feedback**: Your observation about token counts vs ETH was the key insight - -### What Could Be Better -1. **Faster Iteration**: Need better tools to test hypotheses without full recompilation -2. **Root Cause Analysis**: Should have questioned "why 91M tokens?" earlier -3. **Token Type Awareness**: Need to always verify which token is being calculated - -## Recommendations - -1. **Add Calculation Assertions**: Add sanity checks in critical calculations -2. **Improve Test Coverage**: Test intermediate values, not just final results -3. **Document Token Types**: Clearly annotate when working with ETH vs KRAIKEN amounts -4. **Create Debug Mode**: Add a compile flag for detailed calculation logging -5. **Invariant Tests**: Add fuzzing tests that check protocol invariants hold - -## Conclusion - -The bug was subtle - using the correct Uniswap math function but for the wrong token. Better tooling for calculation visibility and type safety would have caught this much faster. The fix was simple once the root cause was identified, highlighting the importance of understanding exactly what each calculation represents. \ No newline at end of file diff --git a/onchain/TODO_DEBUGGING_IMPROVEMENTS.md b/onchain/TODO_DEBUGGING_IMPROVEMENTS.md new file mode 100644 index 0000000..46da671 --- /dev/null +++ b/onchain/TODO_DEBUGGING_IMPROVEMENTS.md @@ -0,0 +1,75 @@ +# TODO: Debugging Improvements + +Based on the floor position calculation bug investigation, here are the key areas that need improvement: + +## 1. Document Uniswap V3 Mechanics & Token Flows +**Problem**: Confusion about ETH flow direction during trades and which token amounts are calculated in different positions. + +**Tasks**: +- Create a visual diagram showing token flows during trades (when ETH flows IN vs OUT) +- Document the relationship between token0/token1 and WETH/KRAIKEN +- Explain how `getAmount0ForLiquidity` vs `getAmount1ForLiquidity` work with different tick ranges +- Add examples showing what happens in each position type (ANCHOR, DISCOVERY, FLOOR) when token0isWeth=true +- Create a reference table: "When calculating X, use Y function to get Z token" + +**Acceptance Criteria**: Developer can quickly determine which token is being calculated in any liquidity operation without trial and error. + +## 2. Document Optimizer Parameters & Effects +**Problem**: Misunderstanding of how parameters like anchorShare and capitalInefficiency affect the protocol behavior. + +**Tasks**: +- Create detailed documentation for each optimizer parameter: + - `capitalInefficiency` (0-100%): How it affects KRAIKEN valuation for reserves (0% = 70% value, 100% = 170% value) + - `anchorShare` (0-100%): How it reduces floor ETH allocation (95% = 90.1% floor, not 95% anchor) + - `anchorWidth` (0-100): How it sets anchor position range as percentage of price + - `discoveryDepth` (0-100%): How it controls discovery liquidity density (2x-10x multiplier) +- Add examples showing how each parameter affects floor position placement +- Create scenarios: "In a bull market with X parameters, expect floor at Y" + +**Acceptance Criteria**: Developer can predict where positions will be placed given optimizer parameters and market conditions. + +## 3. Implement Calculation Tracing & Visualization Tools +**Problem**: Console.log doesn't work properly in Forge scripts, making it hard to debug complex calculations. Token flows are not visible. + +**Tasks**: +- Create a `DebugEvent` system that works in all contexts: + ```solidity + event DebugCalculation(string label, uint256 value, string unit); + event DebugTokenFlow(string from, string to, address token, uint256 amount); + ``` +- Build a script to parse these events and display them nicely: + ``` + [CALC] Outstanding Supply: 91,373,741 KRAIKEN + [FLOW] LiquidityManager -> UniswapPool: 96,159,882 KRAIKEN + [CALC] After subtraction: 91,373,741 KRAIKEN (ERROR: should be ~2.4M) + ``` +- Add debug mode flag to contracts that enables detailed calculation logging +- Create visualization tool that shows token balances at each step of recenter + +**Acceptance Criteria**: Developer can trace exact values through complex calculations without modifying contracts. + +## 4. Improve Code Quality: Type Safety, Tests & Invariants +**Problem**: Easy to mix up ETH and KRAIKEN amounts (both uint256). Tests don't catch intermediate calculation errors. + +**Tasks**: +- Implement type-safe token amounts: + ```solidity + library TokenAmounts { + struct EthAmount { uint256 value; } + struct KraikenAmount { uint256 value; } + } + ``` +- Add unit tests for intermediate calculations: + ```solidity + function test_discoveryAmountIsKraiken() { + // Test that discoveryAmount represents KRAIKEN, not ETH + } + ``` +- Implement runtime invariant checks: + ```solidity + require(floorTick - anchorUpperTick < 50000, "Floor too far from anchor"); + require(outstandingSupply < totalSupply / 2, "Outstanding supply unrealistic"); + ``` +- Add invariant fuzzing tests that verify protocol assumptions always hold + +**Acceptance Criteria**: Bugs like wrong token type in calculations are caught at compile time or immediately during testing. \ No newline at end of file diff --git a/onchain/test/mocks/BullMarketOptimizer.sol b/onchain/test/mocks/BullMarketOptimizer.sol index eae16f0..c4dfdd7 100644 --- a/onchain/test/mocks/BullMarketOptimizer.sol +++ b/onchain/test/mocks/BullMarketOptimizer.sol @@ -26,7 +26,7 @@ contract BullMarketOptimizer { returns (uint256 capitalInefficiency, uint256 anchorShare, uint24 anchorWidth, uint256 discoveryDepth) { capitalInefficiency = 0; // 0% - aggressive bull market stance - anchorShare = 95 * 10 ** 16; // 95% - reduces floor to 90.1% of ETH + anchorShare = 1e18; // 95% - reduces floor to 90.1% of ETH anchorWidth = 50; // 50% - medium width for concentrated liquidity discoveryDepth = 1e18; // Maximum discovery depth }