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.

Sign up for GitHub

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

Jump to bottom

registerar.schedule_code_upgrade doesn't work #641

Closed
xlc opened this issue May 9, 2023 · 2 comments · Fixed by #1176 · May be fixed by paritytech/polkadot#7412
Closed

registerar.schedule_code_upgrade doesn't work #641

xlc opened this issue May 9, 2023 · 2 comments · Fixed by #1176 · May be fixed by paritytech/polkadot#7412
Labels
I2-bug The node fails to follow expected behavior.

Comments

@xlc
Copy link
Contributor

xlc commented May 9, 2023

registerar.schedule_code_upgrade right now will update the para wasm and set upgrade go ahead signal. This doesn't work with Cumulus. Because Cumulus requires PendingValidationCode is set when go ahead signal is received. But because the upgrade it triggered directly on relaychain, PendingValidationCode is empty and therefore panic the chain.

registerar.schedule_code_upgrade shouldn't set go ahead signal and parachain can just perform client upgrade to force the new wasm.
@bkchr
Copy link
Member

bkchr commented May 9, 2023

Yeah, schedule_code_upgrade doesn't make any sense. Basically we should merge schedule_code_upgrade and force_code_upgrade. It really doesn't make any sense to set the GoAheadSignal.

@bkchr bkchr added the I3-bug label May 9, 2023
@riusricardo riusricardo mentioned this issue Jun 21, 2023
Draft
@al3mart al3mart mentioned this issue Jul 4, 2023
Closed
@Sophia-Gold Sophia-Gold transferred this issue from paritytech/polkadot Aug 24, 2023
@the-right-joyce the-right-joyce added I2-bug The node fails to follow expected behavior. and removed I3-bug labels Aug 25, 2023
@Daanvdplas Daanvdplas mentioned this issue Aug 25, 2023
Merged
bkchr added a commit that referenced this issue Oct 15, 2023
@Daanvdplas @bkchr
fix: GoAhead signal only set when runtime upgrade is enacted from par…
91c4360 
…achain side (#1176)

The runtime code of a parachain can be replaced on the relay-chain via:

[cumulus]:
[enact_authorized_upgrade](https://github.com/paritytech/polkadot-sdk/blob/1a38d6d6be42b30e8be3ffccec75a4ec995fef9d/cumulus/pallets/parachain-system/src/lib.rs#L661);
this is used for a runtime upgrade when a parachain is not bricked.

[polkadot] (these are used when a parachain is bricked):
-
[force_set_current_code](https://github.com/paritytech/polkadot-sdk/blob/1a38d6d6be42b30e8be3ffccec75a4ec995fef9d/polkadot/runtime/parachains/src/paras/mod.rs#L823):
immediately changes the runtime code of a given para without a pvf check
(root).
-
[force_schedule_code_upgrade](https://github.com/paritytech/polkadot-sdk/blob/1a38d6d6be42b30e8be3ffccec75a4ec995fef9d/polkadot/runtime/parachains/src/paras/mod.rs#L864):
schedules a change to the runtime code of a given para including a pvf
check of the new code (root).
-
[schedule_code_upgrade](https://github.com/paritytech/polkadot-sdk/blob/1a38d6d6be42b30e8be3ffccec75a4ec995fef9d/polkadot/runtime/common/src/paras_registrar.rs#L395):
schedules a change to the runtime code of a given para including a pvf
check of the new code. Besides root, the parachain or parachain manager
can call this extrinsic given that the parachain is unlocked.

Polkadot signals a parachain to be ready for a runtime upgrade through
the
[GoAhead](https://github.com/paritytech/polkadot-sdk/blob/e49493442a9377be9344c06a4990e17423783d41/polkadot/primitives/src/v5/mod.rs#L1229)
signal.

When in cumulus `enact_authorized_upgrade` is executed, the same
underlying helper function of `force_schedule_code_upgrade` &
`schedule_code_upgrade`:
[schedule_code_upgrade](https://github.com/paritytech/polkadot/blob/09b61286da11921a3dda0a8e4015ceb9ef9cffca/runtime/parachains/src/paras/mod.rs#L1778),
is called on the relay-chain, which sets the `GoAhead` signal (if the
pvf is accepted).

If Cumulus receives the `GoAhead` signal from polkadot without having
the `PendingValidationCode` ready, it will panic
([ref](paritytech/polkadot#7412)). For
`enact_authorized_upgrade` we know for sure the `PendingValidationCode`
is set. On the contrary, for `force_schedule_code_upgrade` &
`schedule_code_upgrade` this is not the case.

This PR includes a flag such that the `GoAhead` signal will only be set
when a runtime upgrade is enacted by the parachain
(`enact_authorized_upgrade`).

additional info: paritytech/polkadot#7412

Closes #641

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Bastian Köcher <info@kchr.de>
@Polkadot-Forum
Copy link

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v1-3-0/4614/1

claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
@musnit
Add codeowners file (paritytech#641)
a65d911 
* add codeowners file

* try with group

* team fix
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
106ff2c
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
938652b
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
5619eef
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
390764f
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
aa28154
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
2500efc
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
6e407a6
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
75a3360
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
2b09d14
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
1a4c4d0
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
26f21d4
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
@svyatonik @serban300
swap target <-> source in race logs (paritytech#641)
a004fc4
bkchr pushed a commit that referenced this issue Apr 10, 2024
@svyatonik @bkchr
swap target <-> source in race logs (#641)
dad5374
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
I2-bug The node fails to follow expected behavior.
Projects
None yet
Milestone
No milestone
4 participants
@xlc @bkchr @the-right-joyce @Polkadot-Forum