0x37 - Healthy positions may be liquidated because of inappropriate judgment

[sherlock-admin3]

0x37

Medium

Healthy positions may be liquidated because of inappropriate judgment

Summary

Healthy positions may be liquidated in one special case

Vulnerability Detail

In is_liquidatable(), there are some comments about the position's health. A position becomes liquidatable when its current value is less than a configurable fraction of the initial collateral.
The problem is that when pnl.remaining equals required, this position should be healthy and cannot be liquidated. But this edge case will be though as unhealthy and can be liquidated.

@external
@view
def is_liquidatable(position: PositionState, pnl: PnL) -> bool:
    """
    A position becomes liquidatable when its current value is less than
    a configurable fraction of the initial collateral, scaled by
    leverage.
    """
    percent : uint256 = self.PARAMS.LIQUIDATION_THRESHOLD * position.leverage
    required: uint256 = (position.collateral * percent) / 100
    return not (pnl.remaining > required)

Impact

Healthy positions may be liquidated and take some loss.

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/params.vy#L117-L138

Tool used

Manual Review

Recommendation

-    return not (pnl.remaining > required)
+    return not (pnl.remaining >= required)

[spacegliderrrr]

Escalate

Issue should be low/info. It only covers an extreme edge case where pnl.remaining == required. It is extremely unlikely for the two values to match exactly and it wouldn't make a difference as the position would be liquidateable in the next block anyway.

[sherlock-admin3]

Escalate

Issue should be low/info. It only covers an extreme edge case where pnl.remaining == required. It is extremely unlikely for the two values to match exactly and it wouldn't make a difference as the position would be liquidateable in the next block anyway.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[oxwhite]

The impact is high even if the likelihood is low(Not extremely low). Therefore it is submitted as medium. You can't say it is going to be liquidated in the next block anyway by simply ignoring other possibilities. How do you certain of that? How about if the user wants to add liquidity and protect his position when it is close to threshold?or if the price goes towards the direction of user's position when exact time equal to pnl.remaining? If the point being questioned here is the low probability, There are accepted reports with different roots which include similar edge case in previous contests. Here is an example:code-423n4/2023-10-nextgen-findings#1323 . Here is another one in recent contest accepted by the judge:https://codehawks.cyfrin.io/c/2024-08-fjord/results?lt=contest&page=1&sc=xp&sj=score&t=report.

[Waydou21]

I agree, the issue is a low at best, even the impact is not a high as when being on such fine edge is synonym for liquidatable, I agree with spacegliderr comment as this means it'll be liquidated in the next block due to fees

[oxwhite]

The code allows to this edge case happen. Even if it happens only once, there is loss for users, hence making the impact high. Beside the tx can be observed by the malicious actors and manipulated based on this case. Therefore it deserves M and a fix

[sabatha7]

Escalate

Issue should be low/info. It only covers an extreme edge case where pnl.remaining == required. It is extremely unlikely for the two values to match exactly and it wouldn't make a difference as the position would be liquidateable in the next block anyway.

Extremely unlikely edge case on the P&L? that isnt true.

I agree, the issue is a low at best, even the impact is not a high as when being on such fine edge is synonym for liquidatable, I agree with spacegliderr comment as this means it'll be liquidated in the next block due to fees

Due to fees? That isnt the same scenario the round down of the healthy position here will be considerable plus the total liquidated amount.

edit: i suspect you have misunderstood the reason being if the position is liquidated wrong they cannot re-liquidate it again or not during the next block even if the position theoretically is still healthy for how long after false liquidation then any given position then if they do liquidated some position 'by fees' during the next block I doubt it will still be the same position. The auditors here have clearly specified pnl.remaining == required there will be wrong liquidation.

code-423n4/2023-07-tapioca-findings#1164
sherlock-audit/2023-03-notional-judging#203

[WangSecurity]

After confirming, this comment is outdated:

A position becomes liquidatable when its current value is less than a configurable fraction of the initial collateral, scaled by leverage.

The code works as intended and the position should be liquidated when PnL equals required and the comment has to be changed. Hence, this issue is invalid as the code works as expected. Planning to accept the escalation and invalidate the issue.

[sabatha7]

@WangSecurity what of issue validity rules; "If the protocol team provides specific information in the README or CODE COMMENTS, that information stands above all judging rules. In case of contradictions between the README and CODE COMMENTS, the README is the chosen source of truth." Hence this is valid. For example iamnmt

@WangSecurity

Why is the rule "List of Issue categories that are not considered valid" applied but not this rule?

finding was awarded for 2024-06-magicsea-judging and was rewarded and many additional findings only just recently for 2024 findings. The comments that are provided during the contest code will prioritised according to the rules. Thus we will validate this finding regardless.

[oxwhite]

After confirming, this comment is outdated:

A position becomes liquidatable when its current value is less than a configurable fraction of the initial collateral, scaled by leverage.

The code works as intended and the position should be liquidated when PnL equals required and the comment has to be changed. Hence, this issue is invalid as the code works as expected. Planning to accept the escalation and invalidate the issue.

@WangSecurity logically if the remaining value is equal to the required value, the position should not be liquidated. I do not think it is reasonable simply invalidate that a submission based on the comment. The comment, the function names and docs can lie, what is important is that what the code allows. In this case the code allows the user to be liquidated while it shouldn't , because the user has enough collateral. Besides the user may protect their position towards the threshold, but current implementation takes this chance from user. On the other hand, as ı said above it is not logical to invalidate/validate based on the comments provided, however for a second let's focus on "outdated" concept and look at a more critical example. If an outdate concept is enough to invalidate a bug, how about if an outdated function left in the code that leads to a critical exploit? Is it reasonable to invalidate the all reports that are submitted in this case? Based on your reasoning, we should be able to invalidate this case as well. But It is protocol's responsibility to remove the outdated function before the contest in this case. What auditor or judges should pay attention is that does the provided code allows an exploit or not? if yes, then it should be valid.
See the exploit happened due to a deprecated function.
The reason of escalation is the edge case here. However that does not decrease the severity. Because it leads to loss of funds. Since there is an edge case it is medium. Otherwise that would be an H. Also there are previous reports with similar edge case accepted by Sherlock. Here is a recent one:sherlock-audit/2024-08-winnables-raffles-judging#26
Now what makes this one different in terms of severity? This and other accepted similar reports also disagree with the reason of escalation.

[WangSecurity]

@sabatha7
The code comments indeed stand above the rules, but don't forget about this rule:

The judge can decide that CODE COMMENTS are outdated based on contextual evidence. In that case, the judging guidelines are the chosen source of truth.

Here, the comment is outdated. Hence, the rules apply. About other findings, firstly, historical decisions are not sources of truth. Secondly, it's hard to understand exactly what you mean by that since there's no reference (i.e. link).

@oxwhite

logically if the remaining value is equal to the required value, the position should not be liquidated. I do not think it is reasonable simply invalidate that a submission based on the comment. The comment, the function names and docs can lie, what is important is that what the code allows. In this case the code allows the user to be liquidated while it shouldn't , because the user has enough collateral. Besides the user may protect their position towards the threshold, but current implementation takes this chance from user.

The key part is that the current behaviour is intended and the user should be liquidatable when remaining == required. Hence, here the protocol works as expected.

On the other hand, as ı said above it is not logical to invalidate/validate based on the comments provided, however for a second let's focus on "outdated" concept and look at a more critical example. If an outdate concept is enough to invalidate a bug, how about if an outdated function left in the code that leads to a critical exploit? Is it reasonable to invalidate the all reports that are submitted in this case? Based on your reasoning, we should be able to invalidate this case as well. But It is protocol's responsibility to remove the outdated function before the contest in this case. What auditor or judges should pay attention is that does the provided code allows an exploit or not? if yes, then it should be valid.

I don't see how this part provides any value to the discussion and it's not relevant here.

See the exploit happened due to a deprecated function

This is a completely different protocol, completely different issue and here we have nothing to do with "deprecated function".

The reason of escalation is the edge case here. However that does not decrease the severity. Because it leads to loss of funds. Since there is an edge case it is medium. Otherwise that would be an H

This is not a loss of funds because the user has to be liquidatable at this point. It's not the mistake in the code and it works as intended here.

Also there are previous reports with similar edge case accepted by Sherlock. Here is a recent one:sherlock-audit/2024-08-winnables-raffles-judging#26
Now what makes this one different in terms of severity? This and other accepted similar reports also disagree with the reason of escalation

The issue you referenced is completely different, it was validate on a completely different rule and is irrelevant here. Also, don't forget that historical decisions are not sources of truth.

Planning to accept the escalation and invalidate the issue.

[oxwhite]

@WangSecurity

"The key part is that the current behaviour is intended and the user should be liquidatable when remaining == required. Hence, here the protocol works as expected."

That can be said if you accept that the comment is outdated and current implementation is intended, which was not known to auditors during the contest. Also, I would appreciate it if you could provide any information on how you determined the comment was outdated. Was this simply communicated by the protocol after submissions?

"I don't see how this part provides any value to the discussion and it's not relevant here."

This is pointing out that it is unreasonable to validate or invalidate an issue while ignoring the code and focusing on the outdated comment. It aligns with what is stated here:

"The judge can decide that CODE COMMENTS are outdated based on contextual evidence. In that case, the judging guidelines are the chosen source of truth. Example: The code comments state that 'a swap can never fail' even though the code is built to support failing swaps."

The example shows that the code is more important than the comments.

"This is not a loss of funds because the user has to be liquidatable at this point. It's not the mistake in the code and it works as intended here."

Again, it is not considered a loss of funds because it is regarded as intended behavior, based on the claim of an "outdated comment," which was likely made by the protocol after the contest. This was neither mentioned in the README nor publicly discussed in the Discord during the contest. Please correct me if I am wrong on this.

In conclusion, if your reasoning for invalidation is still simply based on the 'outdated comment,' I have nothing further to discuss.

[sabatha7]

@WangSecurity Thank you for announcing these valid suggestions.

1. Indeed the computation for Required, and for Pnl are calculated based on the honest participants' actual positions' pot.
2. There is no taggable contextual evidence that suggests that Percent, or Required variable are subtracted a healthy amount, (or Pnl is added a healthy amount) to the extend where Pnl == Required isn't healthy based on point nr. 1 (Indeed the computation for Required, and for Pnl are calculated based on the honest participants' actual positions' pot.)
3. We have to visit the actual contextual evidence.
4. We will assume that it will be the "fees" discussion acting as the contextual evidence being shown?
5. Losses are already factored into the pnl.remaining before invoking is_liquidatable
6. Base fees, and Quote fees are visited after POSITIONS.close has already been visited. This shows that amt_final.fee remaining are manipulatable by, or applied by the liquidator, and apart from that the context of the code suggest that zero fees will be ignored. Thus on the escalation; the position will be liquidated in any case on the next block according to fees, this is not asserted, or constrained to be a constant true (in contextual evidence according to how the code was developed). The kind gentleman will be asked to realise a valid escalation scope.

[WangSecurity]

That can be said if you accept that the comment is outdated and current implementation is intended, which was not known to auditors during the contest. Also, I would appreciate it if you could provide any information on how you determined the comment was outdated. Was this simply communicated by the protocol after submissions?

Yes, I've clarified with the sponsor if the code comment was correct and the code worked incorrectly. It ended up the opposite and the code works correctly. But that's sufficient contextual evidence to apply the rule about outdated code comments that I've shared in the previous comment. Any Watson could do the same during the contest.

The example shows that the code is more important than the comments

It doesn't necessarily mean that the code is more important than code comments. And I don't validate/invalidate the finding based on the code comment being outdated or not. I invalidate this finding based on the fact that the code works as intended and it's only a recommendation to consider changing from pnl.remaining > required to pnl.remaining >= required.

Again, it is not considered a loss of funds because it is regarded as intended behavior, based on the claim of an "outdated comment," which was likely made by the protocol after the contest. This was neither mentioned in the README nor publicly discussed in the Discord during the contest. Please correct me if I am wrong on this.

As said above, it's the contextual evidence that makes the comment outdated.

@sabatha7 Thank you for elaborating on the finding here, but, unfortunately, the code works correctly here and it's only a recommendation, as said above. Planning to accept the escalation and invalidate the issue.

[sabatha7]

@WangSecurity

Thank you.

How could one have audited a Black Box context.

Please clarify on the context.

Clearly an additional amount is being subtracted from the remaining pnl prematurely. To say rounding down a healthy position is the intended behavior context even if that rounding down breaches 1% lost is that what we are justifying i.e are we suggesting that a jump from remain equals requaired to remain < required can not greater than 1% movement? Please observe your response since a movement < 1% can generate a loss of 100% of the remaining position also pnl is allowed to float and tokens are allowed to dump or hike greatly there is absolutely no reason to liquidated the position here regardless of the final bias of the judgement .

[sabatha7]

@WangSecurity

Again kindly show how it is; "working correctly" . The formula is hardcoded to show that the position is still healthy when remaining is equal to required kindly show how this isn't true.

#12 was given valid high. The poc refers to negative p&l please accurately define negative p&l. also #12 is invalid and highly unlikely and works as intended there is no guarantee and incentive for anyone to do that . This #17 is the 0x37 finding that is valid tbh.

[sabatha7]

pleading for fair treatment and neutral biases.

[WangSecurity]

Clearly an additional amount is being subtracted from the remaining pnl prematurely. To say rounding down a healthy position is the intended behavior context even if that rounding down breaches 1% lost is that what we are justifying i.e are we suggesting that a jump from remain equals requaired to remain < required can not greater than 1% movement? Please observe your response since a movement < 1% can generate a loss of 100% of the remaining position also pnl is allowed to float and tokens are allowed to dump or hike greatly there is absolutely no reason to liquidated the position here regardless of the final bias of the judgement

This issue is not about "additional amount is being subtracted from the remaining pnl prematurely" or rounding down. The issue is solely about using > instead of >=, which is only a design recommendation, and there's nothing wrong with using >.

Again kindly show how it is; "working correctly" . The formula is hardcoded to show that the position is still healthy when remaining is equal to required kindly show how this isn't true.

The formula is not hardcoded to show that the position is still healthy when remaining is equal to required, the equation just checks if the remaining is higher than required. Again, there's nothing wrong with the position being liquidatable when remaining == required.

#12 was given valid high. The poc refers to negative p&l please accurately define negative p&l. also #12 is invalid and highly unlikely and works as intended there is no guarantee and incentive for anyone to do that . This #17 is the 0x37 finding that is valid tbh.

#12 is a completely different issue with nothing to do with this.

pleading for fair treatment and neutral biases.

The judgement is not biased and is fair. I honestly see this issue as invalid and not biased towards any of the outcomes.

The decision remains: accept the escalation and invalidate the issue.

[sabatha7]

@WangSecurity

I honestly trust you are right.

See what I was referring The formula is kinda hardcoded to show that the position is still healthy when remaining is equal to required I was referring to the valuation operations (liquidatable function will accept the result of the best valuation of the position given as pnl remaining at which point losses fees are accounted already thus pnl remaining == required is healthy)

@WangSecurity

This issue is not about "additional amount is being subtracted from the remaining pnl prematurely" or rounding down.

We are to try our best effort that will be to unpin #17 as the main finding and try to pin #67 in its place such as #67 refers better to the matters of the pnl remaining balance, and prematurely subtracting in the root cause. If that would be in the best interest.

[WangSecurity]

See what I was referring The formula is kinda hardcoded to show that the position is still healthy when remaining is equal to required I was referring to the valuation operations (liquidatable function will accept the result of the best valuation of the position given as pnl remaining at which point losses fees are accounted already thus pnl remaining == required is healthy)

Unfortunately, it doesn't change the fact that remaining > required to be non-liquidatable is intended. As I've said several times previously, it's not an error or a mistake in the code, it's intended.

We are to try our best effort that will be to unpin #17 as the main finding and try to pin #67 in its place such as #67 refers better to the matters of the pnl remaining balance, and prematurely subtracting in the root cause. If that would be in the best interest.

#67 doesn't refer to anything like this. #67 just pasted the is_liquidatable function and said the position would be liquidatable when remaining == required. It doesn't even point out any issues, it just explains how the function works.

My decision remains, accept the escalation and invalidate the issue.

[sabatha7]

@WangSecurity

Glad to be on your side.

Edit:

This issue is not about "additional amount is being subtracted from the remaining pnl prematurely" or rounding down.

in terms of #67

I was looking at the portion where #67 goes if N = 60 and we expect a maximal price movement of 1%/minute with regard to forcefully rounding down the pnl remaining. This adequately discusses the issue with inconsiderate balancing or inappropriate additional/deduction amounts.

In other words for each different movement of price we are accounting 1% each time . In other words #67 was suggesting the threshold is already configuring some floor level and the threshold is as balanced as it needed to be in terms of as far as the threshold is concerned then there was non actual need to reach into the healthy amount at this point considering that pnl remaining == required is alright and saying that it isn't alright will actually forces a 1% movement (might as well be paying an additional fees or forcefully paying a 1% tips toward the protocol at the expense of the position without any actual consent / I wish to alleviate that the 1% is not on the position valuation when we reconcile rather the 1% entails the different movements or if you could possibly have len(n)*1% movements then len(n-1)*1% robs you an allocated movement or there is a round down it would have been an issue of wrong logistics (wrong liquidation) rather than an issue of wrong parcel (a position missing 1% in the contents) thus a parcel being delivered to a wrong pointer address causing unexpected consequences (the position is wrongfully lost altogether).

it doesn't change the fact that remaining > required to be non-liquidatable is intended. It may not be entirely correct to claim this way. It was supposed to be the owner of the position who willingly says I intended to give 1% (fuel expenditure to deliver my parcel or you are allowed to deliver 1 block / city fewer than in reality) as a tip in order to lose the entire position. Tbh this premature reaching into to the healthy position by rounding down are owed to the honest participant and are being locked away in the contract. Am now kindly asking for a possible upgrade to high if the locked away can not be recovered if that is possible according to the Could Denial-of-Service... section.

@WangSecurity

This issue is not about "additional amount is being subtracted from the remaining pnl prematurely" or rounding down.

#58 also referred to the concern for proper balancing of the amounts where Inappropriate additional amounts / deductions are concerned.

[WangSecurity]

#67 doesn't identify any issues as I've said before. #67 just lists the function and says the position is liquidatable if remaining==required.
#67 is not an issue in any regard, it just explains how the protocol works, i.e. if pnl.remaining ==required then it will be liquidatable

Hence, my decision is as was previously stated: accept the escalation and invalidate the reports because there's no issue. The decision will be applied in a couple of hours.

[sabatha7]

@WangSecurity

Now kindly asking for the 3rd, or 4th time for the context. If there isn't any valid context how are we defaulted to invalidating the finding if the decision of the judge truthfully finds it to be valid and we have given all the valid understanding. To my own understanding they have taken a non existing context plus a timout of the escalation duration to overpower a valid find whilst invalid finding such as #12 were used to sponge the dishonest creation of the excess value within the reward pool.

#67 just lists the function and says the position is liquidatable if remaining==required. how does this not concern the requirement for there to be adequate balance during the valuations and liquidations or how does it concern validity far less than a context we are not being openly told.

The amounts that are being earned by the lp at this liquidation should ideally be taken from the protocol if the protocol wants to donate and not be taken from healthy position since truthfully there was a wrong liquidation then we can say the pnl.remaining is locked away and the lp earned a fabricated amount that was blamed on the healthy position. The protocol must [edit:shift] the blame of the fabrication to their own accounts.

KINDLY asking the escalations to raise documented or substantiated proofs rightfully due to the underlying concern that they have not yet done so. should we either upgrade this finding to high or leave as medium tbh will be fair and in the best interests.

[oxwhite]

@WangSecurity
Excuse me but your only proof for reasoning is the comment which is expressed by the sponsor that is "outdated" after the contest. You do not have any solid evidence that invalidates the report . Personally ı think judging should be based on more clear evidence other than simply an outdated comment or sponsor's word expressed after the contest. I am curious how or what would prevent any sponsor to say any such code is intended for any future contest? I would definitely be ok with invalidation if a good evidence was provided, however the reasoning of invalidation does not make any sense. My point have never been blindly try to validate the report(you can see my approach from my other escalated issue), but from the beginning ı have never understood the reasoning provided. Regards.

[sabatha7]

@WangSecurity

Also the core team should kindly not treat my comments as if I am representing everyone who was tagged on this finding I am merely driving 1 points . Perhaps everyone else should be given a fair go 🎠

[WangSecurity]

As I've said previously, the judging here is based on the fact that the code here works as it should be working. It cannot be a valid finding, because it's not an issue. The Watsons could ask the sponsor questions about how the protocol is intended to work or if these comments express how the code should work, but if the Watsons didn't, it's not grounds for validating the issue.
The fact that the code here is not completely up to comments doesn't make it valid "issue". Moreover, the comments can be outdated based on the contextual evidence and sponsors reply is enough contextual evidence, as was said previously.

The decision remains, accept the escalation and leave the issue as it is. The decision will be applied in a couple of hours. If the comments repeat the same arguments, the escalation will be resolved without a reply.

[sabatha7]

@WangSecurity

Now kindly asking for the 3rd, or 4th time for the context. If there isn't any valid context how are we defaulted to invalidating the finding if the decision of the judge truthfully finds it to be valid and we have given all the valid understanding. To my own understanding they have taken a non existing context plus a timout of the escalation duration to overpower a valid find whilst invalid finding such as #12 were used to sponge the dishonest creation of the excess value within the reward pool.

#67 just lists the function and says the position is liquidatable if remaining==required. how does this not concern the requirement for there to be adequate balance during the valuations and liquidations or how does it concern validity far less than a context we are not being openly told.

The amounts that are being earned by the lp at this liquidation should ideally be taken from the protocol if the protocol wants to donate and not be taken from healthy position since truthfully there was a wrong liquidation then we can say the pnl.remaining is locked away and the lp earned a fabricated amount that was blamed on the healthy position. The protocol must [edit:shift] the blame of the fabrication to their own accounts.

KINDLY asking the escalations to raise documented or substantiated proofs rightfully due to the underlying concern that they have not yet done so. should we either upgrade this finding to high or leave as medium tbh will be fair and in the best interests.

@WangSecurity

I made an edit on the previous commment Wanted to text shift
Also I am also kindly not going to comment more often just because there are a few hours remainin there are more wardens involved you know.

[sabatha7]

As I've said previously, the judging here is based on the fact that the code here works as it should be working. It cannot be a valid finding, because it's not an issue. The Watsons could ask the sponsor questions about how the protocol is intended to work or if these comments express how the code should work, but if the Watsons didn't, it's not grounds for validating the issue. The fact that the code here is not completely up to comments doesn't make it valid "issue". Moreover, the comments can be outdated based on the contextual evidence and sponsors reply is enough contextual evidence, as was said previously.

The decision remains, accept the escalation and leave the issue as it is. The decision will be applied in a couple of hours. If the comments repeat the same arguments, the escalation will be resolved without a reply.

@WangSecurity

What is should be?

should be fair or unfair?

The rules for issue validity are usually for fairness so I don't know how are we being exempted.

[WangSecurity]

Should be means the code is working as intended, expected. In other words, the position is liquidatable at "remaining == required" and it's intended to be like that.
The decision remains, accept the escalation and invalidate the issue. Planning to apply the decision in a couple of hours

[0xweb333]

@WangSecurity
So you are saying that is ok the protocol can liquidate positions with enough , exact required collateral?, and that is auditor fault to not asks team about that comments are updated?
This could lead to team invalidate all finding saying that code works as expected, and comments are obsolete
This finding should be medium at least, because users can lost all his funds even when his position has the required collateral

[sabatha7]

@WangSecurity

Seems as if you indeed are saying; should be unfair.

I can understand if the calculations are saying the position was sitting at pnl remaining == required since the previous block then fees will liquidated the position since some time has elapsed (fees are due to time elapsed). However, time had already elapsed and fees are already included in the calculation of pnl remaining by the time we want to reference pnl remaining in the is liquidatable function we know this since it is documented in the code.

For context under the same sherlock rules similar findings to this have been rewarded without initially griefing .

[0xweb333]

#67 doesn't identify any issues as I've said before. #67 just lists the function and says the position is liquidatable if remaining==required. #67 is not an issue in any regard, it just explains how the protocol works, i.e. if pnl.remaining ==required then it will be liquidatable

Hence, my decision is as was previously stated: accept the escalation and invalidate the reports because there's no issue. The decision will be applied in a couple of hours.

#98 includes a PoC where a healthy position is shown to be liquidatable
#98

[WangSecurity]

@0xweb333

@WangSecurity
So you are saying that is ok the protocol can liquidate positions with enough , exact required collateral?, and that is auditor fault to not asks team about that comments are updated?
This could lead to team invalidate all finding saying that code works as expected, and comments are obsolete
This finding should be medium at least, because users can lost all his funds even when his position has the required collateral

No, it doesn't work like that, and there's no need for the team to say that.

I can understand if the calculations are saying the position was sitting at pnl remaining == required since the previous block then fees will liquidated the position since some time has elapsed (fees are due to time elapsed). However, time had already elapsed and fees are already included in the calculation of pnl remaining by the time we want to reference pnl remaining in the is liquidatable function we know this since it is documented in the code.

None of the reports explain this, the reports just state that the position is liquidatable when remaining == required. What you say above is not in the reports so it doesn't affect the validity of the reports.

The decision remains, accept the escalation and invalidate the issue.

[WangSecurity]

Result:
Invalid
Has duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• spacegliderrrr: accepted