Fix rm pause

[GalloDaSballo]

Closes #587

MUST REVIEW

Adds a Delay of X SECONDS before allowing liquidations

Use beginRMLiquidationCooldown to trigger the timer

Use stopRMLiquidationCooldown to cancel it

After a sequence of liquidations _checkLiquidateCoolDownAndReset is called, to reset the timer

This can still cause risks wrt to desynch of the value, but hopefully is an improvement and a simplification of the design

TO FINISH

• Is the delay enforced in ALL Liquidations for CDPs liquidatable only in RM?
• Is RM immediately exited after such liquidations have happened (and is the flag set back to UNSET_TIMESTAMP_FLAG right after?)
• How do we improve the "desynch" issue? (see BO calls I added)

Towards CEI Conformity

See rationale on how hooks are called

https://miro.com/app/board/uXjVMrhxeT8=/?share_link_id=950890293382

NOTE

We break CEI in the main external functions as a way to ensure we are recording the latest proper state for RM

This ensures that:
-> Repay / Closing / Opening that undos RM resets the Timer

This + Synching in Liquidations should cover the majority of Desynch Cases

• Check Redemptions for the desynch above

RM Triggers:
- Price Change - PRICE UP - If RM is still going, countdown is added, if RM is averted, I can manually reapply the glass
- Fee Split - STETH UP - if RM is still going, countdown is added, else I can manually reapply the glass or via any operation or manually

RM untriggers:
- Price Change - PRICE UP - If RM is still going, countdown is unchanged, if RM is averted, I can manually reapply the glass
- Fee Split - STETH UP - if RM is still going, countdown is unchanged, else I can manually reapply the glass or via any operation

- Open Cdp Positively - TCR UP - After such an operation, TCR is up and glass is applied automatically
- Repay / Adjust - TCR UP - After such an operation, TCR is up and glass is applied automatically
- Close - After such an operation, TCR is up and glass is applied automatically

- Liquidate - After such an operation, TCR is up and glass is applied automatically
- Redeem - After such an operation, TCR is up and glass is applied automatically

Debugging Branch

DO NOT MERGE THIS!!!
https://github.com/Badger-Finance/ebtc/tree/chore-logs-revert-batch-liquidate

Weird Stuff Left

• _sequenceLiqToBatchLiq can return 0 -> Reverts with underflow / overflow
• 1e18 check when stETH is slashed, still here...
• _sequenceLiqToBatchLiq seems to not be correct (TODO after)

[GalloDaSballo]

NOTE:
Test failing
[FAIL. Reason: CdpManager: ICR is not below liquidation threshold in current mode] test_snipeViaPriceDradwown() (gas: 1479873)

Rightfully fails since we mitigated the sniping

[rayeaster]

it still depends on external interaction to explicitly trigger the "desync" and is susceptible to potential (maybe of lower likelihood) attack mentioned in #587 (review)

Another brainstorm idea (maybe not necessarily a "real" worthy attack vector): Say after time wait pass and system is still in RM, now liquidator is eager to start liquidation work, but attacker could frontrun every liquidation attempt to trigger another time wait and put the system in wait forever (render Recovery Mode not working as expected).

[dapp-whisperer]

DRY

This was already duplicated code, but it's a good chance to consolidate.
I would probably rewrite this whole check to an internal function

_canLiquidateInCurrentMode(_icr, _TCR, _recovery)

Global state synced before any external operation

we should follow the rule of applyPendingGlobalState() before doing the RM check in the external setters

[rayeaster]

DRY

This was already duplicated code, but it's a good chance to consolidate. I would probably rewrite this whole check to an internal function _canLiquidateInCurrentMode(_icr, _TCR, _recovery)

Global state synced before any external operation

we should follow the rule of applyPendingGlobalState() before doing the RM check in the external setters

good point, how about add checkLiquidateCoolDownAndReset() in applyPendingGlobalState() directly?

[GalloDaSballo]

Pushing fixes for QA issues (error messages)

[dapp-whisperer]

Redemptions Optimization

Guiding ideas:

• dedup reads, we shouldn't be reading the same thing multiple time
• however, not willing to make deeper / more significant code changes at this point

The RedemptionTotals struct now tracks the aggregate amount of collateral surplus that is sent to redeemed parties. This is required to know what the collateral of the system will be after the redemption loop is done so we can virtually calculate the TCR for Grace Period.

When get this value by tracking the collSurplus for each fully redeemed Cdp, and adding them to the total in a way that fits the existing code

moved "setting up the next iteration stuff" to the bottom of the core redemption loop

Finally, there are some debug checks added to confirm that the live system debt and collShares at the end of the operation (after interactions) match what we expected when we did the TCR calculation for the Grace Period

Next Up

We see that the Grace Period calcs for liquidations and redemptions are identical. This means it's a good candidate to dedup and move into an internal function.

[dapp-whisperer]

Code Dedup

The liquidation and redemption grace period check logic has been distilled into _syncRecoveryModeGracePeriod function.
Much cleaner.

Debug checks left in redemptions code for now.

[rayeaster]

would be nice to add to comment: typically used by governance-privileged setters

[GalloDaSballo]

I'd say let's get rid of Debug events

Use the 1 invariant for Synching of TCR before and After and have Echidna run for a long time

[dapp-whisperer]

Grace Period Governance

There is now an auth-gated setter for the grace period, setGracePeriod
It shouldn't be able to set a grace period below the hardcoded minimum

A couple additional clarity renames were sent in.