Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add _setCollateralPaused() #21

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

beepidibop
Copy link

Summary:
Comptroller.sol: added _setCollateralPaused() and changed getHypotheticalAccountLiquidityInternal() to reflect new error
ErrorReporter.sol: added new COLLATERAL_PAUSED error for the comptroller
ComptrollerStorage.sol: added V6 to store collateralGuardianPaused state

Addition to the Pause Guardian storage slots in V2. 
Appended as V6 to avoid introducing storage slot conflicts.
Added new error COLLATERAL_PAUSED for when a collateral is paused and someone tries to borrow with paused collateral.
Added _setCollateralPaused(), changed getHypotheticalAccountLiquidityInternal() to reflect new error.
@beepidibop
Copy link
Author

beepidibop commented Jan 30, 2022

Just realized the issue required the use of checkMembership but I didn't use it. h/t @EHermstad

If checkMembership is to be used, it'll just need to be changed to public and line 727 will need to be changed to
if (collateralGuardianPaused[address(asset)] == true && borrowAmount > 0 && vars.cTokenBalance > 0 && checkMembership(account,asset)){

IMO this will only waste gas because why checkMembership again when getHypotheticalAccountLiquidityInternal is already passing markets that the user is in. This also introduces a change from external to public for checkMembership, which wastes gas for external callers who just want to check if someone's in a market.

Allows users to borrow even if they're in a market where the collateral is paused if they have 0 cTokenBalance (In market, but not supplied. Happens when someone borrows but don't supply the same token)
@nourharidy
Copy link
Member

nourharidy commented Jan 30, 2022

LGTM. The purpose of the issue was to call checkMembership() for each paused collateral in the borrowAllowed hook. But this is actually a much more gas efficient and cleaner approach. Well done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants