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

Restore governance lock periods to 7 days in Polkadot #86

Merged
merged 3 commits into from
Nov 14, 2023

Conversation

rphmeier
Copy link
Contributor

This is a revert of paritytech/polkadot@d73503c which has not been released.

Deploying without this change would lead to referendum participants and delegates having their tokens locked for 4 times longer than expected.

Participants in any referendums ending after the runtime upgrade is applied would be affected.
Even more of an issue is delegates, where all current delegates would be affected by 4 times longer lock up issues. The affected user-base is enormous, and there is little to no possibility of reaching all these users to get them to adapt in time.

Although a 7 Day lock multiplier wasn't the originally intended target for Polkadot, breaking user expectations with this change is an even larger issue.

@rphmeier rphmeier changed the title Restore governance lock periods to 7 days Restore governance lock periods to 7 days in Polkadot Nov 11, 2023
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a revert of paritytech/polkadot@d73503c which has not been released.

It is released, but the Polkadot runtime upgrade is already being cancelled due to XCM issues.

Prior investigation lead to this: #20 (comment)

@xlc
Copy link
Contributor

xlc commented Nov 11, 2023

I see two possible migrations:

  • Gradually increase the lock period from 7 days to 28 days over, say, a year. We could increase the value by, say, 3 days for every runtime upgrade, until it is upgraded to 28 days.
  • Have a migration to reduce existing convictions in such way that the conviction is reduced but the lock period remains the same or reduced. This ensures people won't unexpected have longer locked period. They will have reduced convictions in future votes but that's not too bad. They can just redelegate with the new convictions.

@kianenigma
Copy link
Contributor

gathering some data on this might be useful. For example, hoe many people are voting/delegating with large convictions such that this will imply months of added lock time.

While we agreed upon releasing 1000000 runtime that extending locks is fine, I now find the second suggestion from @xlc to be the more conservative and therefore preferred choice.

Either a user decides to lower their conviction, or extend their lock, but the default behavior should have been the former.

@bkchr
Copy link
Contributor

bkchr commented Nov 12, 2023

  • Have a migration to reduce existing convictions in such way that the conviction is reduced but the lock period remains the same or reduced. This ensures people won't unexpected have longer locked period. They will have reduced convictions in future votes but that's not too bad. They can just redelegate with the new convictions.

I would do the same. I would propose that we let the runtime upgrade getting applied and then we do another one that adjusts everyones locking period that had voted before the runtime upgrade was applied, even when the referenda was finished after the runtime upgrade was applied.

@rphmeier
Copy link
Contributor Author

then we do another one that adjusts everyones locking period that had voted before the runtime upgrade was applied

We would also need to do a migration which updates all delegation convictions for every delegation which had been put in place before the runtime upgrade was applied. Many users have already updated their convictions manually in anticipation of this.

@bkchr
Copy link
Contributor

bkchr commented Nov 12, 2023

Many users have already updated their convictions manually in anticipation of this.

Reverting the change as you proposed here would then have the same result for them.

@rphmeier
Copy link
Contributor Author

Reverting the change as you proposed here would then have the same result for them.

Not really, because the intention is still to increase the conviction locking periods (for example, to those outlined in polkadot-fellows/RFCs#20 (comment))

I argue that we should have clear communication about the exact changes to convictions and a plan for migrating delegations and votes before releasing code that would change the deployed status-quo

@ggwpez
Copy link
Member

ggwpez commented Nov 13, 2023

For the follow up changes i propose this: paritytech/polkadot-sdk#125 (comment)
But it would still need a migration to correct the current behaviour.

relay/polkadot/src/governance/mod.rs Outdated Show resolved Hide resolved
@bkchr
Copy link
Contributor

bkchr commented Nov 14, 2023

/merge

Copy link

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@github-actions github-actions bot enabled auto-merge (squash) November 14, 2023 20:26
@github-actions github-actions bot merged commit 5d98c2e into main Nov 14, 2023
6 checks passed
@bkchr bkchr deleted the rh-revert-gov-locks branch November 14, 2023 20:51
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.

10 participants