Compound Proposal 63: Temporary Patch for COMP Distribution Bug (9/29/21)

Starting a thread to discuss this proposal, will link to it from the proposal body and begin discussion below in a moment.

The current plan is to temporarily disable COMP claims until a full patch can be tested. More info coming soon

13 Likes

As long as distribution is halted I’m fine with it. If we can still accrue comp that would be ideal.

3 Likes

The proposal was posted last night and will be in a review phase until 10/1 when voting will begin at 10pm https://compound.finance/governance/proposals/63

2 Likes

Suggest voting against this proposal because it would brick the integrations which expect being able to call claimComp which will be always reverting with the proposed change.

    function grantCompInternal(address user, uint amount) internal returns (uint) {
        require(false, "COMP rewards paused");  // <--

        Comp comp = Comp(getCompAddress());
        uint compRemaining = comp.balanceOf(address(this));
        if (amount > 0 && amount <= compRemaining) {
            comp.transfer(user, amount);
            return 0;
        }
        return amount;
    }

Example integration that would break: YearnV2-Generic-Lev-Comp-Farm/Strategy.sol at ceeceb624196e3d7eda5d706984d4001c1c73307 · jmonteer/YearnV2-Generic-Lev-Comp-Farm · GitHub

8 Likes

Strong AGAINST this proposal 63 vote from me as well. (Note this forum post is also being used for proposal 64, which fixes the issue that 63 had. I’m not against 64.)

I’ve just finished simulating this governance proposal on forked mainnet, and it breaks parts of OUSD (which currently uses Compound as an investment strategy), including large deposits.

We do have ways to work around this, however, it feels really wrong to just start reverting on everyone like this. Not everyone has upgradable contracts or reconfigurations available by governance.

7 Likes

Should vote against, i think a simple return instead of revert to make the function essentially a ‘noop’ without the side effects of the claim, may be less intrusive for upstream integrations, would patch the issue while a long term fix can be put in place.

Folks are definitely receptive to these valid concerns with respect to integrations. Another approach that uses a simple filter to skip claims potentially amplified by the bug – in a way that favors potentially skipping some valid claims over permitting any invalid claims – is under review. Eyes and comments from integrators and others welcome!

8 Likes

Hi, I’m from idle.finance, the proposal as it’s implemented right now would break our integration too eg here idle-contracts/contracts/IdleTokenGovernance.sol at c0368b4fb4f0aec1ec7a93043e4da35dc658a5a1 · Idle-Labs/idle-contracts · GitHub so we suggest to vote against this proposal.

This approach instead

seems to be good enough for us

5 Likes

I like the revised approach you posted, @allthecolors. Just affecting some accounts, and not all accounts, and simply not sending COMP at this time, vs reverting, seems like a great temporary way of handling this. Much better.

3 Likes

Proposal 64

7 Likes

Hey man after this temporary disabled claim on comp ability is over should expect to see a comp balance because since Wednesday I did not know why it was a line on my claim comp ability to view my earnings.

1 Like

Some one needs to speak :speaking_head: up because I know that defi and or centralized platforms work to leave earnings in wallets of users and that is rule your jeopardizing my hard inventory and my earnings I legally and fairly supplied.speak

I feel that if your gonna rewrite the code you should tell me what to expect once your done rewriting the code because logically speaking I have not even hit the claim button on my comp that you disabled so I demand to know my balance before I agree to your restoration and me having to wait for you all to rewrite the code.thank you tell me my comp balance I’m gonna see it anyway 3 days and counting.

Hey @Theo, we apologize for having you and others in the dark about everyone’s COMP rewards. Please bear with us as we’re all hard at work fixing this and doing damage control.

Everyone is still earning COMP at the regular rate. We temporarily disabled the ability to view accrued COMP and also to claim it from the UI while we conduct our investigation. The ability for users to view their accrued COMP will be restored before proposal 64 goes to vote.

Proposal 64 does two things:

  • Fixes the accrual bug
  • Temporarily disables claiming COMP for the users potentially affected by the bug

After proposal 64 executes, there’ll be a follow-up proposal to fix the accrued COMP amounts for the users affected by this bug (set to the amount they fairly and rightfully earned). At this point, we’ll restore the ability for everyone to claim their COMP.

With that being said, everyone will continue accruing COMP at their normal and expected rates. This is true in the moments leading up to proposal 64 as well as after. The COMP fairly earned will without a doubt be given to users.

Users who have not interacted with the affected markets (those without COMP rewards) since proposal 62 was executed will be able to claim their COMP just fine. For those who have, I estimate it’ll take about a week for their ability to claim COMP to be restored.

Once again, I want to make it clear that COMP rewards are still accruing at normal and expected rates and that everyone will be able to claim their rightfully earned COMP - the majority right away and the rest shortly after.

We thank everyone for their patience and their trust in Compound protocol!

7 Likes

If I understand correctly, can you pick up rewards from users who have not yet taken them? Is it okay at all? Doesn’t sound like decentralization

1 Like

i am a casual user and this is confusing, can someone answer the below questions please.

  1. I have almost a year worth of COMP token sitting there unclaimed, now i cannot see the balance nor claim it from GUI.
  2. Today: is there a way to see how many COMP token i have unclaimed + claim those tokens via API or other methods
  3. After this proposal: I will be able to see and claim the COMP tokens again from GUI. But sounds like you guys going to do something to it to reduce the balance already received for some random people due to the bug. Is that it? But why disable the VIEWING of everyone’s COMP balance, the whole point of defi is it’s open. Makes no sense.

Thanks

Wouldn’t it make much more sense to just set comp accrual speed for all markets to non-zero to avoid the bug?

you can see your comp balance through the API https://api.compound.finance/api/v2/governance/comp/account?address=

Just add you address at the end of that.

in the response, comp_allocated is what you are looking for.

Hey everyone, I have to go now so I can’t answer the questions above, but I’m here to post the link to the upgrade scenario to ensure that proposal 64 does what it’s expected to do.

https://github.com/TylerEther/compound-protocol/blob/dynamic-comp-rewards-distribution-2/spec/sim/0064-fix-comp-accruals/hypothetical_mainnet_upgrade.scen

1 Like


Think there’s some issue when doing the math here on the bugged txs