Panic when updating DefaultPowerReduction

[VegeBun-csj]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

ERR CONSENSUS FAILURE!!! err="failed to apply block; error commit failed for application: error changing validator set: duplicate entry Validator{1E425B3D29A4D3052E4A57F3C942A8FB13961967 PubKeyEd25519{819B2CF5A95750A20DC0EA9AFEE92E7A4895D85FF52BCB21A16C03C6D6798A1D} VP:510000 A:0} in [Validator{1E425B3D29A4D3052E4A57F3C942A8FB13961967 PubKeyEd25519{819B2CF5A95750A20DC0EA9AFEE92E7A4895D85FF52BCB21A16C03C6D6798A1D} VP:510000 A:0}Validator{1E425B3D29A4D3052E4A57F3C942A8FB13961967 PubKeyEd25519{819B2CF5A95750A20DC0EA9AFEE92E7A4895D85FF52BCB21A16C03C6D6798A1D} VP:510000 A:0}]"

Cosmos SDK Version

0.47.5

How to reproduce?

we use decimal 18 before and meet voting power overflow, So we changed the DefaultPowerReduction to fix it. But when i start the chain and delegate to a validator, it meets the above error. This is my commit, are there any question in this change?

[VegeBun-csj]

@tac0turtle can you help me :)

[tac0turtle]

it looks like you have two validators with the same key, did you run some sort of migration or in the gentx you need to make sure you use unique validators

[VegeBun-csj]

I don't have two validators with same key. This error occurred when I was delegating. I launched the chain with a modified version of DefaultPowerReduction at a certain block height, and the chain is running well and transactions can be sent. However, when I attempt to delegate to one of the validators, this error occurs. Is it possible that this is due to not upgrading the chain? If so, is there a way to resolve it without upgrading? @tac0turtle

[zhangjiannan]

I guess simply updating DefaultPowerReduction doesn't work because some places have to apply the change. Would be great if we can confirm:

1. is updating DefaultPowerReduction the correct way to fix VP overflow when decimal=18?
2. what else should be done other than updating DefaultPowerReduction to 10^18?

[facundomedica]

Might be related: #9447

[alexanderbez]

You cannot just simply update DefaultPowerReduction as @facundomedica pointed out via the linked issue. In order to change the power reduction function, you need an on-chain handler to perform a migration of ALL validators s.t. their power is updated based on the new value.

[VegeBun-csj]

You cannot just simply update DefaultPowerReduction as @facundomedica pointed out via the linked issue. In order to change the power reduction function, you need an on-chain handler to perform a migration of ALL validators s.t. their power is updated based on the new value.

Thanks guys @facundomedica @alexanderbez. How to do an on-chain handler to perform a migration of ALL validators? On-chain governance? In my view, we are using cosmos sdk 0.47.5, so we can execute arbitrary logic on param changes metioned in this pr , how to do it? looking forward to your reply:)

[alexanderbez]

... so we can execute arbitrary logic on param changes ...

DefaultPowerReduction is NOT on-chain param -- it's a constant. It's not meant to be changed.

If you do change it however, you are responsible for updating all validator's powers via the app's upgrade handler. I think EVMOS did this but I'm not sure. Maybe you can reach out to them to see how/if they did it.

[zhangjiannan]

@alexanderbez thanks for the update! Can we confirm that DefaultPowerReduction only affects validator's powers? Or, do we only need to update all validators' powers via the upgrade handler?

Just want to make sure if we update, there aren't other places broken.

[VegeBun-csj]

@facundomedica @alexanderbez Hi, I have created some code to upgrade all validator's powers via the app's upgrade handler, but it also got the same error (duplicated validator entry). Are there any problems with this change which updates the validator's voting power? Thanks

[alexanderbez]

@zhangjiannan I believe the safest thing to do is to update all validator's powers.

[zhangjiannan]

thanks @alexanderbez however without updating DefaultPowerReduction voting power calculation for new validators will still be incorrect which causes vp overflow.

[zhangjiannan]

The duplicated validator entry error comes with updated vp values, therefore the validator voting powers have already been updated. Now the question is why there is a duplicated entry, probably because AddValidatorTokensAndShares couldn't delete the previous entry but created a new entry. Looking into it now.

[VegeBun-csj]

@alexanderbez hi, i have added some validator updating codes in upgrade handler function with this link, but it doesnt work well. Are there any problems in this and can you give me some suggestions?

[facundomedica]

@VegeBun-csj I believe you first have to delete the validator in the power index: DeleteValidatorByPowerIndex, as the key changes you might see duplicated validators

[VegeBun-csj]

@facundomedica @alexanderbez We find this issue and need to perform a delete operation first, followed by a setValidator operation. So we referred to this piece of code called TestingUpdateValidator to update our UpgradeHandler, also using cosmos sdk 0.47.5. However, during the process of referencing the implementation of TestingUpdateValidator, we encountered an issue with accessing the storeKey (as it is a private variable in the keeper) in this line. Could you please suggest any ways to access this storeKey in my current context?

[VegeBun-csj]

Thanks for helping on this issue! We have updated power stores through the upgrade handler. A first round of testing suggests that the migration is successful. However, further testing is underway. We will update when we make further progress. Thank you once again for your assistance :)

[theoneislowly213]

yes

[zhangjiannan]

The network had been fully recovered on Feb 15th and all validators' vp has been updated. Thanks to @alexanderbez @facundomedica for help and discussion.

[alexanderbez]

Nice job 🙌 🙌