It isn’t immediately clear whether the endTime() value returned is inclusive
or exclusive. Based on its usage in _getLiquidationAmount(), one can infer that it is exclusive: if position.expiredWith.endTime() <= block.timestamp for ascertaining position expiry status.
However, the exclusivity of the endTime value breaks the inverse property: f◦f−1(x) = x. What we have is _timestampToEpoch(_epochToTimestamp(epoch)) = epoch + 1, which isn’t intuitive.
Mitigation
Consider making endTime() inclusive instead, where _epochToTimestamp() returns a second lesser.
Details
It isn’t immediately clear whether the
endTime()
value returned is inclusive or exclusive. Based on its usage in_getLiquidationAmount()
, one can infer that it is exclusive: ifposition.expiredWith.endTime() <= block.timestamp
for ascertaining position expiry status. However, the exclusivity of the endTime value breaks the inverse property:f◦f−1(x) = x
. What we have is_timestampToEpoch(_epochToTimestamp(epoch)) = epoch + 1
, which isn’t intuitive.Mitigation
Consider making
endTime()
inclusive instead, where_epochToTimestamp()
returns a second lesser.Otherwise, make it clear that
endTime()
is exclusive.