manuallyFixAccounting changes #2034
Conversation
…strategist manuallyFixAccounting calls doAccounting to check the fuse is still not blown Removed accountingGovernor
naddison36
requested review from
sparrowDom,
DanielVF and
shahthepro
as code owners
| ) external onlyStrategist whenPaused { | ||
| require( | ||
| _validatorsDelta >= -1 && | ||
| _validatorsDelta <= 1 && |
There was a problem hiding this comment.
the -1 -> 1 accepted validator delta and -32 -> 32 accepted consensus delta could be constants so that it is easier to configure these values if/when we want.
Also if these ranges are not able to fix the accounting we need to re-deploy the contract via OUSD governance (slow process) to introduce larger deltas. I think we should be comfortable changing up to 3 or even 5 validators (and 332 or 532 ETH in consensus rewards) without being worried that such transaction could rug anyone or produce so much rewards for the attacker (controlling the strategist) to somehow damage the protocol.
There was a problem hiding this comment.
Thanks, I'll update to 2 and 332
| accountingValid = _doAccounting(); | ||
| } | ||
|
|
||
| function _doAccounting() internal returns (bool accountingValid) { |
There was a problem hiding this comment.
I know this works for now, I am slightly worried that we will in future forget that internal _doAccounting needs to be in whenNotPaused context when doing the accounting and in whenPaused context when manually correcting the accounting.
What if we introduce a bool property to the internal _doAccounting function and be explicit whether a _pause() call should be made depending on who the caller of the transaction is. Smth like:
function _doAccounting(bool callPause) internal...
...
// pause and fail the accounting
if (callPause) {
_pause();
}
// fail the accounting
return false;
....
There was a problem hiding this comment.
using a pauseOnFail param also saves as storage slot read. I'll make the change
Increased the allowed delta values of manuallyFixAccounting
…icka/fix-accounting
…icka/fix-accounting
Contract changes
manuallyFixAccountingnow uses delta valuesmanuallyFixAccountingcallsdoAccountingto check the fuse is still not blownaccountingGovernoras the Strategist now callsmanuallyFixAccounting