QA Report

[c4-bot-5]

See the markdown file with the details of this report here.

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as high quality report

[0xEVom]

GA-05: reckless admin mistake, invalid
QA-09: invalid

[kalinbas]

QA-01 Depending on the network 1 minute may be enough, also TWAP is only used to verify prices in the standard scenario where Chainlink prices are used mainly. 30 minutes is way to much when prices move fast.
QA-02 If chainlink is down, only liquidiate, decreaseLiquidity and borrow functionality would be down. Thats why there is the emergency mode to disable chainlink completely if needed.
QA-03 This will never reach 0 for a normal token (and this code is already deployed) - so we will leave it
QA-04 Only called by admin so not that important
QA-05: Invalid
QA-06 Will be part of adding new tokens taking care that pools have enough history. If there is a problem with it, the history size may be increased by anyone.
QA-07 Ok agree
QA-08 No, its ok that ETH is sent to any address.
QA-09: Invalid

[c4-sponsor]

kalinbas (sponsor) acknowledged

[c4-judge]

jhsagd76 marked the issue as grade-a

[c4-judge]

jhsagd76 marked the issue as selected for report

[jhsagd76]

Invalid

• QA-01: Depending on the network 1 minute may be enough, also TWAP is only used to verify prices in the standard scenario where Chainlink prices are used mainly. 30 minutes is way to much when prices move fast.
• QA-02: If chainlink is down, only liquidiate, decreaseLiquidity and borrow functionality would be down. Thats why there is the emergency mode to disable chainlink completely if needed.
• QA-05: The current condition is correct.
• QA-06: Will be part of adding new tokens taking care that pools have enough history. If there is a problem with it, the history size may be increased by anyone.
• QA-08: No, its ok that ETH is sent to any address.
• QA-09: Yep, its deprecated. But the use in the code is correct (0 approval is guaranteed each time)

Low

• QA-03

Downgraded Low

• The logic behind MIN_PRICE_DIFFERENCE is heavily flawed as it allows for heavy arbitraging #98
• No grace period applied which would then allow positions to be liquidated after sequencer goes down since now users don't have enough time to deposit funds #119
• RouterSwapParams lacks a deadlining logic and could lead to unfavourable swaps #129

NC

• QA-04
• QA-07

Summary

1 Low 2 NC + 3 Downgraded QA Low

[thebrittfactor]

Just a note that C4 is excluding the invalid entries from the official report.