Add Solidity linting with solhint, Foundry formatter, and pre-commit hooks (#51)

## Changes

### Configuration
- Added .solhint.json with recommended rules + custom config
  - 160 char line length (warn)
  - Double quotes enforcement (error)
  - Explicit visibility required (error)
  - Console statements allowed (scripts/tests need them)
  - Gas optimization warnings enabled
  - Ignores test/helpers/, lib/, out/, cache/, broadcast/

- Added foundry.toml [fmt] section
  - 160 char line length
  - 4-space tabs
  - Double quotes
  - Thousands separators for numbers
  - Sort imports enabled

- Added .lintstagedrc.json for pre-commit auto-fix
  - Runs solhint --fix on .sol files
  - Runs forge fmt on .sol files

- Added husky pre-commit hook via lint-staged

### NPM Scripts
- lint:sol - run solhint
- lint:sol:fix - auto-fix solhint issues
- format:sol - format with forge fmt
- format:sol:check - check formatting
- lint / lint:fix - combined commands

### Code Changes
- Added explicit visibility modifiers (internal) to constants in scripts and tests
- Fixed quote style in DeployLocal.sol
- All Solidity files formatted with forge fmt

## Verification
-  forge fmt --check passes
-  No solhint errors (warnings only)
-  forge build succeeds
-  forge test passes (107/107)

resolves #44

Co-authored-by: johba <johba@harb.eth>
Reviewed-on: https://codeberg.org/johba/harb/pulls/51
This commit is contained in:
johba 2025-10-04 15:17:09 +02:00
parent f8927b426e
commit d7c2184ccf
45 changed files with 2853 additions and 1225 deletions

View file

@ -10,24 +10,25 @@ pragma solidity ^0.8.19;
* - Edge case classification and recovery
* @dev Uses setUp() pattern for consistent test initialization
*/
import "forge-std/Test.sol";
import { Kraiken } from "../src/Kraiken.sol";
import "../src/interfaces/IWETH9.sol";
import { LiquidityAmounts } from "@aperture/uni-v3-lib/LiquidityAmounts.sol";
import { PoolAddress, PoolKey } from "@aperture/uni-v3-lib/PoolAddress.sol";
import "@aperture/uni-v3-lib/TickMath.sol";
import {LiquidityAmounts} from "@aperture/uni-v3-lib/LiquidityAmounts.sol";
import {WETH} from "solmate/tokens/WETH.sol";
import {PoolAddress, PoolKey} from "@aperture/uni-v3-lib/PoolAddress.sol";
import "@uniswap-v3-core/interfaces/IUniswapV3Factory.sol";
import "@uniswap-v3-core/interfaces/IUniswapV3Pool.sol";
import "../src/interfaces/IWETH9.sol";
import {Kraiken} from "../src/Kraiken.sol";
import "forge-std/Test.sol";
import { WETH } from "solmate/tokens/WETH.sol";
import { LiquidityManager } from "../src/LiquidityManager.sol";
import {Stake, ExceededAvailableStake} from "../src/Stake.sol";
import {LiquidityManager} from "../src/LiquidityManager.sol";
import {ThreePositionStrategy} from "../src/abstracts/ThreePositionStrategy.sol";
import "../src/helpers/UniswapHelpers.sol";
import {UniSwapHelper} from "./helpers/UniswapTestBase.sol";
import {TestEnvironment} from "./helpers/TestBase.sol";
import "../src/Optimizer.sol";
import { ExceededAvailableStake, Stake } from "../src/Stake.sol";
import { ThreePositionStrategy } from "../src/abstracts/ThreePositionStrategy.sol";
import "../src/helpers/UniswapHelpers.sol";
import "../test/mocks/MockOptimizer.sol";
import { TestEnvironment } from "./helpers/TestBase.sol";
import { UniSwapHelper } from "./helpers/UniswapTestBase.sol";
// Test constants
uint24 constant FEE = uint24(10_000); // 1% fee
@ -53,10 +54,8 @@ uint256 constant VWAP_TEST_BALANCE = 100 ether;
// Error handling constants
bytes32 constant AMPLITUDE_ERROR = keccak256("amplitude not reached.");
bytes32 constant EXPENSIVE_HARB_ERROR =
keccak256("HARB extremely expensive: perform swap to normalize price before recenter");
bytes32 constant PROTOCOL_DEATH_ERROR =
keccak256("Protocol death: Insufficient ETH reserves to support HARB at extremely low prices");
bytes32 constant EXPENSIVE_HARB_ERROR = keccak256("HARB extremely expensive: perform swap to normalize price before recenter");
bytes32 constant PROTOCOL_DEATH_ERROR = keccak256("Protocol death: Insufficient ETH reserves to support HARB at extremely low prices");
// Dummy.sol
contract Dummy {
@ -80,7 +79,7 @@ contract LiquidityManagerTest is UniSwapHelper {
LiquidityManager lm;
Optimizer optimizer;
address feeDestination = makeAddr("fees");
// Test environment instance
TestEnvironment testEnv;
@ -115,7 +114,7 @@ contract LiquidityManagerTest is UniSwapHelper {
if (address(testEnv) == address(0)) {
testEnv = new TestEnvironment(feeDestination);
}
// Use test environment to set up protocol
(
IUniswapV3Factory _factory,
@ -127,7 +126,7 @@ contract LiquidityManagerTest is UniSwapHelper {
Optimizer _optimizer,
bool _token0isWeth
) = testEnv.setupEnvironment(token0shouldBeWeth, RECENTER_CALLER);
// Assign to state variables
factory = _factory;
pool = _pool;
@ -184,12 +183,11 @@ contract LiquidityManagerTest is UniSwapHelper {
}
}
/// @notice Validates recenter operation results
/// @param isUp Whether the recenter moved positions up or down
function _validateRecenterResult(bool isUp) internal view {
Response memory liquidityResponse = inspectPositions(isUp ? "shift" : "slide");
// Debug logging
console.log("=== POSITION ANALYSIS ===");
console.log("Floor ETH:", liquidityResponse.ethFloor);
@ -198,31 +196,23 @@ contract LiquidityManagerTest is UniSwapHelper {
console.log("Floor HARB:", liquidityResponse.harbergFloor);
console.log("Anchor HARB:", liquidityResponse.harbergAnchor);
console.log("Discovery HARB:", liquidityResponse.harbergDiscovery);
// TEMPORARILY COMMENT OUT THIS ASSERTION TO SEE ACTUAL VALUES
// assertGt(
// liquidityResponse.ethFloor, liquidityResponse.ethAnchor, "slide - Floor should hold more ETH than Anchor"
// );
assertGt(
liquidityResponse.harbergDiscovery,
liquidityResponse.harbergAnchor * 5,
"slide - Discovery should hold more HARB than Anchor"
);
assertGt(liquidityResponse.harbergDiscovery, liquidityResponse.harbergAnchor * 5, "slide - Discovery should hold more HARB than Anchor");
// Check anchor-discovery contiguity (depends on token ordering)
if (token0isWeth) {
// When WETH is token0, discovery comes before anchor
assertEq(
liquidityResponse.discoveryTickUpper,
liquidityResponse.anchorTickLower,
"Discovery and Anchor positions must be contiguous (WETH as token0)"
liquidityResponse.discoveryTickUpper, liquidityResponse.anchorTickLower, "Discovery and Anchor positions must be contiguous (WETH as token0)"
);
} else {
// When WETH is token1, discovery comes after anchor
assertEq(
liquidityResponse.anchorTickUpper,
liquidityResponse.discoveryTickLower,
"Anchor and Discovery positions must be contiguous (WETH as token1)"
liquidityResponse.anchorTickUpper, liquidityResponse.discoveryTickLower, "Anchor and Discovery positions must be contiguous (WETH as token1)"
);
}
assertEq(liquidityResponse.harbergFloor, 0, "slide - Floor should have no HARB");
@ -253,7 +243,6 @@ contract LiquidityManagerTest is UniSwapHelper {
}
}
/// @notice Retrieves liquidity position information for a specific stage
/// @param s The liquidity stage (FLOOR, ANCHOR, DISCOVERY)
/// @return currentTick Current price tick of the pool
@ -368,7 +357,7 @@ contract LiquidityManagerTest is UniSwapHelper {
/// @notice Allows contract to receive ETH directly
/// @dev Required for WETH unwrapping operations during testing
receive() external payable {}
receive() external payable { }
/// @notice Override to provide LiquidityManager reference for liquidity-aware functions
/// @return liquidityManager The LiquidityManager contract instance
@ -415,7 +404,7 @@ contract LiquidityManagerTest is UniSwapHelper {
// Fund account and convert to WETH
vm.deal(account, accountBalance);
vm.prank(account);
weth.deposit{value: accountBalance}();
weth.deposit{ value: accountBalance }();
// Setup initial liquidity
recenterWithErrorHandling(false);
@ -425,29 +414,29 @@ contract LiquidityManagerTest is UniSwapHelper {
// EXTREME PRICE HANDLING TESTS
// ========================================
/// @notice Tests system behavior when price approaches Uniswap MAX_TICK boundary
/// @notice Tests system behavior when price approaches Uniswap MAX_TICK boundary
/// @dev Validates that massive trades can push price to extreme boundary conditions (MAX_TICK - 15000)
/// without system failure. Tests system stability at tick boundaries.
function testTickBoundaryReaching() public {
// Skip automatic setup to reduce blocking liquidity
disableAutoSetup();
// Custom minimal setup
// Custom minimal setup
deployProtocolWithTokenOrder(DEFAULT_TOKEN0_IS_WETH);
vm.deal(account, 15000 ether);
vm.deal(account, 15_000 ether);
vm.prank(account);
weth.deposit{value: 15000 ether}();
weth.deposit{ value: 15_000 ether }();
// Grant recenter access
vm.prank(feeDestination);
lm.setRecenterAccess(RECENTER_CALLER);
// Setup approvals without creating blocking positions
vm.startPrank(account);
weth.approve(address(lm), type(uint256).max);
harberg.approve(address(lm), type(uint256).max);
vm.stopPrank();
// Record initial state - should be around -123891 (1 cent price)
(, int24 initialTick,,,,,) = pool.slot0();
// Pool starts with 0 liquidity, positions created during first trade
@ -456,64 +445,64 @@ contract LiquidityManagerTest is UniSwapHelper {
// Stage 1: Large initial push to approach MAX_TICK
buyRaw(8000 ether);
(, int24 stage1Tick,,,,,) = pool.slot0();
// Stage 2: Additional push if not yet at extreme boundary
if (stage1Tick < TickMath.MAX_TICK - 15000) {
// Stage 2: Additional push if not yet at extreme boundary
if (stage1Tick < TickMath.MAX_TICK - 15_000) {
buyRaw(2500 ether);
(, int24 stage2Tick,,,,,) = pool.slot0();
// Stage 3: Final push with remaining ETH if still needed
if (stage2Tick < TickMath.MAX_TICK - 15000) {
if (stage2Tick < TickMath.MAX_TICK - 15_000) {
uint256 remaining = weth.balanceOf(account) - 500 ether; // Keep some ETH for safety
buyRaw(remaining);
}
}
(, int24 postBuyTick,,,,,) = pool.slot0();
// Verify we reached extreme boundary condition
int24 targetBoundary = TickMath.MAX_TICK - 15000; // 872272
// Verify we reached extreme boundary condition
int24 targetBoundary = TickMath.MAX_TICK - 15_000; // 872272
assertGe(postBuyTick, targetBoundary, "Should reach extreme expensive boundary to validate boundary behavior");
// Test successfully demonstrates reaching extreme tick boundaries with buyRaw()
// In real usage, client-side detection would trigger normalization swaps
// Verify that recenter() fails at extreme tick positions (as expected)
try lm.recenter() {
revert("Recenter should fail at extreme tick positions");
} catch {
// Expected behavior - recenter fails when trying to create positions near MAX_TICK
}
// Test passes: buyRaw() successfully reached tick boundaries
}
// testEmptyPoolBoundaryJump() removed - was only needed for debugging "hidden liquidity mystery"
// testEmptyPoolBoundaryJump() removed - was only needed for debugging "hidden liquidity mystery"
// Mystery was solved: conservative price limits in performSwap() were preventing MAX_TICK jumps
function testLiquidityAwareTradeLimiting() public {
// Test demonstrates liquidity-aware trade size limiting
// Check calculated limits based on current position boundaries
uint256 buyLimit = buyLimitToLiquidityBoundary();
uint256 sellLimit = sellLimitToLiquidityBoundary();
(, int24 initialTick,,,,,) = pool.slot0();
uint256 testAmount = 100 ether;
// Regular buy() should be capped to position boundary
buy(testAmount);
(, int24 cappedTick,,,,,) = pool.slot0();
// Raw buy() should not be capped
buyRaw(testAmount);
(, int24 rawTick,,,,,) = pool.slot0();
// Verify that raw version moved price more than capped version
assertGt(rawTick - cappedTick, 0, "Raw buy should move price more than capped buy");
// The exact limits depend on current position configuration:
// - buyLimit was calculated as ~7 ETH in current setup
// - buyLimit was calculated as ~7 ETH in current setup
// - Regular buy(100 ETH) was capped to ~7 ETH, moved 2957 ticks
// - Raw buyRaw(100 ETH) used full 100 ETH, moved additional 734 ticks
}
@ -527,11 +516,7 @@ contract LiquidityManagerTest is UniSwapHelper {
OTHER_ERROR
}
function classifyFailure(bytes memory reason)
internal
view
returns (FailureType failureType, string memory details)
{
function classifyFailure(bytes memory reason) internal view returns (FailureType failureType, string memory details) {
if (reason.length >= 4) {
bytes4 selector = bytes4(reason);
@ -539,9 +524,7 @@ contract LiquidityManagerTest is UniSwapHelper {
if (selector == 0xae47f702) {
// FullMulDivFailed()
return (
FailureType.ARITHMETIC_OVERFLOW, "FullMulDivFailed - arithmetic overflow in liquidity calculations"
);
return (FailureType.ARITHMETIC_OVERFLOW, "FullMulDivFailed - arithmetic overflow in liquidity calculations");
}
if (selector == 0x4e487b71) {
@ -648,7 +631,7 @@ contract LiquidityManagerTest is UniSwapHelper {
console.log("Details:", details);
// This might be acceptable if we're at extreme prices
if (currentTick <= TickMath.MIN_TICK + 50000 || currentTick >= TickMath.MAX_TICK - 50000) {
if (currentTick <= TickMath.MIN_TICK + 50_000 || currentTick >= TickMath.MAX_TICK - 50_000) {
console.log("Overflow at extreme tick - this may be acceptable edge case handling");
} else {
console.log("Overflow at normal tick - this indicates a problem");
@ -713,9 +696,9 @@ contract LiquidityManagerTest is UniSwapHelper {
// Diagnose the scenario type
console.log("\n=== SCENARIO DIAGNOSIS ===");
if (postBuyTick >= TickMath.MAX_TICK - 15000) {
if (postBuyTick >= TickMath.MAX_TICK - 15_000) {
console.log("[DIAGNOSIS] EXTREME EXPENSIVE HARB - should trigger normalization");
} else if (postBuyTick <= TickMath.MIN_TICK + 15000) {
} else if (postBuyTick <= TickMath.MIN_TICK + 15_000) {
console.log("[DIAGNOSIS] EXTREME CHEAP HARB - potential protocol death");
} else {
console.log("[DIAGNOSIS] NORMAL RANGE - may still have arithmetic issues");
@ -797,9 +780,7 @@ contract LiquidityManagerTest is UniSwapHelper {
uint256 traderBalanceAfter = weth.balanceOf(account);
// Core unit test assertion: protocol should not allow trader profit
assertGe(
traderBalanceBefore, traderBalanceAfter, "Protocol must prevent trader profit through arbitrary trading"
);
assertGe(traderBalanceBefore, traderBalanceAfter, "Protocol must prevent trader profit through arbitrary trading");
}
/// @notice Helper to execute a sequence of random trades and recentering
@ -816,12 +797,12 @@ contract LiquidityManagerTest is UniSwapHelper {
// Handle extreme price conditions to prevent test failures
(, int24 currentTick,,,,,) = pool.slot0();
if (currentTick < -887270) {
if (currentTick < -887_270) {
// Price too low - small buy to stabilize
uint256 wethBal = weth.balanceOf(account);
if (wethBal > 0) buy(wethBal / 100);
}
if (currentTick > 887270) {
if (currentTick > 887_270) {
// Price too high - small sell to stabilize
uint256 harbBal = harberg.balanceOf(account);
if (harbBal > 0) sell(harbBal / 100);
@ -844,7 +825,6 @@ contract LiquidityManagerTest is UniSwapHelper {
recenterWithErrorHandling(true);
}
// ========================================
// ANTI-ARBITRAGE STRATEGY TESTS
// ========================================
@ -857,24 +837,24 @@ contract LiquidityManagerTest is UniSwapHelper {
// Phase 1: Record initial state and execute first large trade
(, int24 initialTick,,,,,) = pool.slot0();
uint256 wethBefore = weth.balanceOf(account);
console.log("=== PHASE 1: Initial Trade ===");
console.log("Initial tick:", vm.toString(initialTick));
// Execute first large trade (buy HARB) to move price significantly
buy(30 ether);
uint256 wethAfter1 = weth.balanceOf(account);
uint256 wethSpent = wethBefore - wethAfter1;
uint256 harbReceived = harberg.balanceOf(account);
console.log("Spent", wethSpent / 1e18, "ETH, received", harbReceived / 1e18);
// Phase 2: Trigger recenter to rebalance liquidity positions
console.log("\n=== PHASE 2: Recenter Operation ===");
recenterWithErrorHandling(false);
// Record liquidity distribution after recenter
Response memory liquidity = inspectPositions("after-recenter");
console.log("Post-recenter - Floor ETH:", liquidity.ethFloor / 1e18);
@ -883,60 +863,60 @@ contract LiquidityManagerTest is UniSwapHelper {
// Phase 3: Execute reverse trade to test round-trip slippage
console.log("\n=== PHASE 3: Reverse Trade ===");
uint256 wethBeforeReverse = weth.balanceOf(account);
sell(harbReceived);
uint256 wethAfterReverse = weth.balanceOf(account);
uint256 wethReceived = wethAfterReverse - wethBeforeReverse;
(, int24 finalTick,,,,,) = pool.slot0();
console.log("Sold", harbReceived / 1e18, "received", wethReceived / 1e18);
console.log("Final tick:", vm.toString(finalTick));
// Phase 4: Analyze slippage and validate anti-arbitrage mechanism
console.log("\n=== PHASE 4: Slippage Analysis ===");
uint256 netLoss = wethSpent - wethReceived;
uint256 slippagePercentage = (netLoss * 10000) / wethSpent; // Basis points
uint256 slippagePercentage = (netLoss * 10_000) / wethSpent; // Basis points
console.log("Net loss:", netLoss / 1e18, "ETH");
console.log("Slippage:", slippagePercentage, "basis points");
// Phase 5: Validate asymmetric slippage profile and attack protection
console.log("\n=== PHASE 5: Validation ===");
// Critical assertions for anti-arbitrage protection
assertGt(netLoss, 0, "Round-trip trade must result in net loss (positive slippage)");
assertGt(slippagePercentage, 50, "Slippage must be significant (>0.5%) to deter arbitrage");
// Validate liquidity distribution maintains asymmetric profile
// Get actual liquidity amounts (not ETH amounts at current price)
{
(uint128 anchorLiquidityAmount,,) = lm.positions(ThreePositionStrategy.Stage.ANCHOR);
(uint128 floorLiquidityAmount,,) = lm.positions(ThreePositionStrategy.Stage.FLOOR);
(uint128 discoveryLiquidityAmount,,) = lm.positions(ThreePositionStrategy.Stage.DISCOVERY);
uint256 edgeLiquidityAmount = uint256(floorLiquidityAmount) + uint256(discoveryLiquidityAmount);
assertGt(edgeLiquidityAmount, anchorLiquidityAmount, "Edge positions must have more liquidity than anchor");
uint256 liquidityRatio = (uint256(anchorLiquidityAmount) * 100) / edgeLiquidityAmount;
assertLt(liquidityRatio, 50, "Anchor should be <50% of edge liquidity for shallow/deep profile");
console.log("Anchor liquidity ratio:", liquidityRatio, "%");
}
// Validate price stability (round-trip shouldn't cause extreme displacement)
int24 tickMovement = finalTick - initialTick;
int24 absMovement = tickMovement < 0 ? -tickMovement : tickMovement;
console.log("Total tick movement:", vm.toString(absMovement));
// The large price movement is actually evidence that the anti-arbitrage mechanism works!
// The slippage is massive (80% loss), proving the strategy is effective
// Adjust expectations based on actual behavior - this is a feature, not a bug
assertLt(absMovement, 100000, "Round-trip should not cause impossible price displacement");
assertLt(absMovement, 100_000, "Round-trip should not cause impossible price displacement");
console.log("\n=== ANTI-ARBITRAGE STRATEGY VALIDATION COMPLETE ===");
console.log("PASS: Round-trip slippage:", slippagePercentage, "basis points");
console.log("PASS: Asymmetric liquidity profile maintained");