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

Improve locking mechanism for parachains #14

Merged
merged 6 commits into from
Aug 17, 2023
Merged

Improve locking mechanism for parachains #14

merged 6 commits into from
Aug 17, 2023

Conversation

xlc
Copy link
Contributor

@xlc xlc commented Jul 25, 2023

If this is accepted, I plan to implement this to close paritytech/polkadot#7539

Draft PR for Polkadot paritytech/polkadot#7560

@shawntabrizi
Copy link
Member

shawntabrizi commented Jul 25, 2023

This makes sense to me. Basically, the idea is:

  • lots of chains have shown to have problems at genesis
  • we want to make it as easy as possible for chains to fix themselves at the genesis
  • we want chains to be locked by default, with the explicit choice to unlock themselves

So the solution is: detect when a chain is broken at genesis, allow the chain manager to fix it, lock the chain as soon as it is shown to be "alive".

This obviously will not work for every possible scenario which parachain teams may encounter. For example a chain which set the wrong sudo key or multi-sig and stuff like that, which can produce blocks, but then does not have access to their governance. That is fine, still better than the current situation for sure.

The question here is then:

Shouldn't we just lock a parachain on the production of block 1? I don't see an advantage to a chain getting locked, then getting unlocked, then getting locked again.

Chains with some malicious intention could always fake being broken at genesis to then unlock their chain, so might as well assume every chain is unlocked until it produces it first block.

This should overall simplify the logic of this locking system, since there is a single definite point where we add the lock, and no need to detect for a pause or anything else to trigger an unlock.

So suggestion to the RFC is:

  • remove all the auto-locking logic which is there today.
  • add a check which will lock a chain after their first block is produced.
  • keep the logic which allows an XCM / Root call to unlock the chain.

@xlc
Copy link
Contributor Author

xlc commented Jul 25, 2023

Shouldn't we just lock a parachain on the production of block 1? I don't see an advantage to a chain getting locked, then getting unlocked, then getting locked again.

The chain could be locked before it is onboarded. For example, create a crowdloan will apply the lock. The para manager can also voluntarily lock the parachain. My proposal will allow rescue parachain for such cases.

@shawntabrizi
Copy link
Member

shawntabrizi commented Jul 25, 2023

We lock a parachain during a crowdloan to prevent the code from significantly changing from what the users thought they were contributing to.

However, it seems like:

  • historically, lots of chains have uploaded a minimal runtime which would get later upgraded to something totally different
  • teams could simply "not make a block" for a long time to bypass the lock
  • crowdloans are going to be deprecated in favor of the new core assignment systems

As for para managers voluntarily locking their parachain, seems like they might only want to do it after the first block anyway.

@arrudagates
Copy link

So suggestion to the RFC is:

  • remove all the auto-locking logic which is there today.
  • add a check which will lock a chain after their first block is produced.
  • keep the logic which allows an XCM / Root call to unlock the chain.

I think this is a great solution/compromise, it doesn't add as much complexity to the locking system and also solves the most common issue that teams would face. Besides, runtime issues unrelated to block production (like incorrect sudo keys) can always be tested before and after the parachain code registration using tools like Chopsticks, and then can be easily fixed before the parachain actually gets a slot/buys core time by updating the code before the chain is allowed to produce block 1.

@xlc
Copy link
Contributor Author

xlc commented Jul 25, 2023

I agree crowdloan lock introduces more trouble than assurances.

One more question about the suggested approach: How to handle existing parachains? With my suggestion, parachains that never produced block will automatically be unlocked. With the new approach, they won't, and should we have a migration for them?

@arrudagates
Copy link

arrudagates commented Jul 25, 2023 via email

@xlc
Copy link
Contributor Author

xlc commented Jul 25, 2023

I can't think much reasons why we shouldn't do that. But I also want to see a few more supports before making the adjustments.

@shawntabrizi
Copy link
Member

@xlc i think unlocking them seems fine. If we can contact the teams, perhaps we can simply ask them what they would like us to do. Wouldn't be too crazy to make a hand-coded migration for something like this.

@xlc
Copy link
Contributor Author

xlc commented Jul 26, 2023

It is only the parachains with lease but never produced blocks are qualified for unlock, so I will image it won't be an easy task to contact them.
On the other hand, unlock parachain wouldn't cause any issues for them. They can always relock it. So this could be fine as long as we communicated enough to all the teams and the communities about this change.


- Parachain teams

## Explanation
Copy link
Contributor

Choose a reason for hiding this comment

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

Describe which areas of the runtime will be altered - pallets, interfaces, XCM calls, origins?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not exactly sure what to put for those without making this too much implementation details. This doesn't alter public interfaces and which pallet to modify shouldn't matter.

xlc and others added 3 commits July 28, 2023 13:55
Co-authored-by: asynchronous rob <rphmeier@gmail.com>
Co-authored-by: asynchronous rob <rphmeier@gmail.com>
@xlc
Copy link
Contributor Author

xlc commented Jul 28, 2023

@shawntabrizi @arrudagates @rphmeier I have updated the RFC to use @shawntabrizi 's mechanism. i.e. only lock parachain when a new block is introduced. Please take another look.

@arrudagates
Copy link

Looks good to me! It is possible to also solve the issue of parachains being unable to swap leases if they halt while being locked, but with Polkadot 2.0 being worked on, that won't be necessary.

@gavofyork
Copy link
Contributor

With Agile Coretime and the removal of crowdloans, there's no need for locking.

@xlc
Copy link
Contributor Author

xlc commented Jul 28, 2023

With Agile Coretime and the removal of crowdloans, there's no need for locking.

That's why this is a short term solution and #16 should result in a long term solution compatible with Agile Coretime.

@xlc xlc mentioned this pull request Jul 29, 2023
3 tasks
@eskimor
Copy link

eskimor commented Aug 2, 2023

With Agile Coretime and the removal of crowdloans, there's no need for locking.

How so? Having a single account that can do whatever to a parachain does not sound desirable for an established parachain with significant funds on it.

@rphmeier
Copy link
Contributor

How so? Having a single account that can do whatever to a parachain does not sound desirable for an established parachain with significant funds on it.

It's definitely part of the security model for a parachain.
One thing a parachain might do is set the account to a burn address, effectively removing the manager altogether.
Another thing a parachain might do is set the account to be a sovereign account of a smart contract running on some other chain, which is used in recovery proceedings.
Or a parachain might set the account to be a normal account on the relay chain, which some individual has a private key for.

Users should simply be aware of this for their trust model.

@gavofyork
Copy link
Contributor

gavofyork commented Aug 14, 2023

With Agile Coretime and the removal of crowdloans, there's no need for locking.

How so? Having a single account that can do whatever to a parachain does not sound desirable for an established parachain with significant funds on it.

What @rphmeier said, though the possibility I was especially considering was setting it to the Sovereign Account of some semi-trusted collective such as the Ecosystem Technical Fellowship or the Polkadot Alliance.

The original point of the lock was to ensure that those engaging in a crowdloan which would necessarily have been prior to parachain launch would have a guarantee that the parachain would launch with code corresponding to the advertised code hash.

@bkchr bkchr merged commit c368187 into polkadot-fellows:main Aug 17, 2023
@xlc xlc deleted the rfc14 branch August 17, 2023 08:51
@anaelleltd anaelleltd added Implemented Is merged or live as a feature/service. Implementing Is actively being worked on. and removed Implemented Is merged or live as a feature/service. labels Sep 10, 2024
@xlc xlc added Implemented Is merged or live as a feature/service. and removed Implementing Is actively being worked on. labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Implemented Is merged or live as a feature/service.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Always treat parachain that never produced block for a significant amount of time as unlocked.
8 participants