Canto Liquidity Mining Protocol - Satyam_Sharma's results

141345 marked the issue as low quality report

141345 marked the issue as remove high or low quality report

edge case when exitTimestamp == nextWeek, rewards could be lost

File: canto_ambient/contracts/mixins/LiquidityMining.sol
24:     function crossTicks() internal {

29:         uint256 numElementsExit = tickTracking_[poolIdx][exitTick].length;
30:         tickTracking_[poolIdx][exitTick][numElementsExit - 1]
31:             .exitTimestamp = uint32(block.timestamp);

Due to the low likelihood, high severity might not be approproate

141345 marked the issue as sufficient quality report

Good point, requires quite a few conditions to actually cause damage (exactly aligned exitTimestamp and even then, the accrual has to happen a few weeks later with additional ticks that were in range since then for it to cause skipping ticks), but will definitely be fixed.

OpenCoreCH (sponsor) confirmed

dmvt changed the severity to 2 (Med Risk)

dmvt marked the issue as selected for report

I believe that the original code was correct. I will try to explain why as clearly as possible.

Parameters:

• tickTracking.exitTimestamp = nextWeek
• I will only consider the case tickTracking.enterTimestamp < time for simplicity, therefore tickActiveStart = time.
• In this situation dt = nextWeek - time is the only option since the tick has been exited in the nextWeek.

In the first loop when we arrive to this point:

                            if (tickTracking.exitTimestamp < nextWeek) {
                                // Exit was in this week, continue with next tick
                                tickActiveEnd = tickTracking.exitTimestamp;
                                tickTrackingIndex++;
                                dt = tickActiveEnd - tickActiveStart;
                            } else {
                                // Exit was in next week, we need to consider the current tick there (i.e. not increase the index)
                                tickActiveEnd = nextWeek;
                            }

Here is what happens:

• tickActiveEnd = nextWeek is set.
• tickTrackingIndex is not incremented and later dt is added to time.
• The time weighted position is accrued correctly from time to nextWeek since tickActiveEnd - tickActiveStart = nextWeek - time.

At this point, will not incrementing tickTrackingIndex have a harmful side effect?

Let's see in the second loop, where this time tickTracking.exitTimestamp < nextWeek. Here is what happens:

• tickActiveEnd = tickTracking.exitTimestamp is set.
• tickTrackingIndex is incremented and later dt = tickActiveEnd - tickActiveStart = tickTracking.exitTimestamp - time = 0 is added to time.
• The time weighted position is accrued correctly since tickActiveEnd - tickActiveStart = 0 and no accrual happens.

In the third loop we will use the next tick with the correct last accrual time variable.

Please correct me if necessary, as the explanation in the report is not very clear so I could be wrong. @OpenCoreCH @dmvt

This issue stands. Arguably it could be considered QA due to likelihood, but given that that sponsor saw enough value to fix it and there is an accounting error under vary narrow circumstances, medium makes sense to me. The only way I would have invalidated this due to another warden's objections is if that warden had proven their point with a test.