Analysis of Proposal 64
On October 1, Tyler L. created Proposal 64, an upgrade to the Compound Comptroller which patches the bug introduced in Proposal 62, which separated COMP distribution speeds between suppliers and borrowers and contained an unintentional bug. This new proposal a) patches the bug, preventing new users from being granted excessive COMP rewards, and b) locks COMP reward transfers for users who may have been affected by the bug. Further analysis is below on the means of the fix and which accounts will be prevented from COMP reward withdrawals.
Note: there will need to be another patch, after Proposal 64, to fully fix the issue and allow the rest of the accounts to continue standard accrual and transfers of COMP rewards. In the best case scenario, all accounts should continue to receive all rewards, past, present and future if some future (yet unbuilt) proposal is correctly applied.
Note: this patch (nor any other) has suggested recovering excess COMP that was given out to accounts. Instead, it aims to fix the accounting and return to proper accruals.
Effects of Proposal
Proposal 64 includes two adjustments to the Compound Comptroller functionality.
First, it changes line 1217 of distributeSupplierComp as follows:
- if (supplierIndex == 0 && supplyIndex > compInitialIndex) {
+ if (supplierIndex == 0 && supplyIndex >= compInitialIndex) {
and line 1256 of distributeBorrowerComp similarly:
- if (borrowerIndex == 0 && borrowIndex > compInitialIndex) {
+ if (borrowerIndex == 0 && borrowIndex >= compInitialIndex) {
Each missing equality check above excludes an entire set of uninitialized accounts from the conditional body. This in turn prevents those accounts from becoming initialized, since they incorrectly accrue COMP from an under-initialized position.
These changes prevent the bug from occuring for future users. Specifically: any account that would have been given excess COMP per the issue identified from Proposal 62, after this patch takes effect, will not be given excess COMP when later claiming COMP (or interacting with an affected market). Note: this does not undo the excess COMP that was accrued or distributed to accounts before this patch takes effect. Overall, this patch should return the Comptroller system to its normal path, except there are several accounts that did trigger the bug that will need to be properly accounted for before they can safely withdraw any future COMP rewards.
Thus, secondly, this patch includes the following patch in the beginning of grantCompInternal
on line 1350:
+ for (uint i = 0; i < allMarkets.length; ++i) {
+ address market = address(allMarkets[i]);
+
+ bool noOriginalSpeed = compBorrowSpeeds[market] == 0;
+ bool invalidSupply = noOriginalSpeed && compSupplierIndex[market][user] > 0;
+ bool invalidBorrow = noOriginalSpeed && compBorrowerIndex[market][user] > 0;
+
+ if (invalidSupply || invalidBorrow) {
+ return amount;
+ }
+ }
The accounts that tripped the bug would have been given a non-zero index in the markets which did not have COMP rewards associated with them. For users that did not trip the bug, they would have and retain a zero index in such markets. Note: we use the old storage value to check if they had not been given a COMP speed. If this is true on either the supply or borrow side, we simply return with a no-op (that is: we do not transfer COMP and we return the COMP accrued is the same as the input value).
This snippet will, in effect, prevent an account which may have received excess COMP from claiming COMP from the protocol. It will not revert, but simply will not transfer out any COMP until a future patch that would ensure proper accounting for all such affected accounts.
Note: The check is overly broad and will prevent some users who have not actually hit the bug from claiming COMP as well – namely anyone who has interacted with a market which does not currently have a COMP (borrow) speed (cREP, cWBTC, cSAI, cAAVE, cYFI, cTUSD, cMKR, cSUSHI)
Note: In order for this patch to remain effective, no changes to COMP speeds should be made until claiming is unblocked for all users in the future patch.
The patch looks for accounts which may have been granted excess COMP, but it’s not obvious from the on-chain system if those accounts actually did since that would require knowing if those accounts previously had a supply or borrow balance, but that balance may have since changed. As such, there will need to be a follow-up patch to allow the rest of accounts in this category from withdrawing. This precaution will continue to affect new accounts that claim COMP from afflicted markets, even after Prop 64 is applied.
Review
From a variety of analyses, it does appear that the patch works as intended. That is: new accounts will no longer trip the bug, and some subset of accounts deemed definitely safe will be able to continue standard COMP claims. There has been work on creating scenarios and web3-forks that show the intended result. Additionally, even if there were accounts that were false-negatives from the transfer function, this would be no worse than the functionality that is present today. But it is likely that this patch does fix the bug and prevent any excess withdrawals.
The biggest downside of this patch is that it continues to effectively lock several accounts from honest COMP reward withdrawals, even if those accounts first interact with the affected markets after Prop 64 would go live. Thus, it will be required that a new patch be created soon afterwards to ensure that all accounts become unblocked. This patch is thus a temporary patch. That said, it is one that allows the protocol to return to normal for a large number of users.
Other Factors
As opposed to Prop 63, this patch should not negatively affect any protocol integrations. Specifically, the worst situation for an integration is that it does not receive the COMP rewards it believes it is expected to receive. This situation is no different than if the Comptroller did not have the COMP balance to complete the transfer and should have been assumed when building product integrations. As opposed to Prop 63, this patch does not revert for any user.
This patch has not been audited officially and should be reviewed thoroughly by the community. The intent of the patch is to do no harm, but the pros and cons of the change should be thoroughly reviewed before any community member makes a vote on this proposal.
Note: it is possible to pass and executed Proposal 63 and then Proposal 64. This should be evaluated a potential option, as well.