hats-finance / Tokemak-0x4a2d708ea6b0c04186ecb774cfad1e50fb5efc0b

0 stars 0 forks source link

Lack of Validation and Inconsistency #19

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: @lucasts95 Twitter username: -- Submission hash (on-chain): 0x14ccbf0c1b06650406d913619cd4dce02b4d1ee003a407e65ce364cdeabfbd2e Severity: low

Description: Description\

  1. The function validate currently lacks constraints to verify the values of minInDays, maxInDays, relaxStepInDays, relaxThresholdInDays, and tightenStepInDays.
  2. The parameters minInDays, maxInDays, relaxStepInDays, relaxThresholdInDays, and tightenStepInDays, as defined in the test files (test/integration/strategy/LMPStrategy.t.sol / test/strategy/LMPStrategyTestHelpers.sol), are inconsistent with their declarations in the documentation, posing risks to the project's consistency.
            swapCostOffset: LMPStrategyConfig.SwapCostOffsetConfig({
                initInDays: 28,
                tightenThresholdInViolations: 5,
                tightenStepInDays: 3,
                relaxThresholdInDays: 20,
                relaxStepInDays: 3,
                maxInDays: 60,
                minInDays: 10
            })

Doc:

After 5 or greater constraint violation swaps within 10 swaps, the swap loss offset period is decreased by 3 day until it reaches the minimum allowed value of 10.

After no swaps transactions occur for the period of 30 days, the swap loss offset period is increased by 1 day until it reaches the maximum allowed value of 50 days.

Attack Scenario\

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional) If these parameters are established with expected data values, it is recommended to declare them as constants. This practice ensures code consistency and stability.

lucasts95 commented 5 months ago

Proof of Concept (PoC) File

add the test in test/strategy/LMPStrategy.t.sol.

    function test_inconsistent() public {

        assertEq(defaultStrat.swapCostOffsetMinInDays(), 10);
        assertEq(defaultStrat.swapCostOffsetTightenStepInDays(), 3);

        assertNotEq(defaultStrat.swapCostOffsetMaxInDays(), 50);
        assertNotEq(defaultStrat.swapCostOffsetRelaxThresholdInDays(), 30);
        assertNotEq(defaultStrat.swapCostOffsetRelaxStepInDays(), 1);  
    }

the result:

$ forge test --mt test_inconsistent -vvv
[⠑] Compiling...
[⠆] Compiling 1 files with 0.8.17
[⠰] Solc 0.8.17 finished in 5.76sCompiler run successful!
[⠔] Solc 0.8.17 finished in 5.76s

Running 1 test for test/strategy/LMPStrategy.t.sol:LMPStrategyTest
[PASS] test_inconsistent() (gas: 9308)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.31ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)
ahuja88 commented 5 months ago

The values and their expected range has evolved slightly since the time of documentation based on simulation results. We now have range values that can be used for validating the strategy configuration parameters.