Skip to content

fix para-tests#1867

Closed
nbaztec wants to merge 2 commits intomasterfrom
nish-fix-para-tests
Closed

fix para-tests#1867
nbaztec wants to merge 2 commits intomasterfrom
nish-fix-para-tests

Conversation

@nbaztec
Copy link
Copy Markdown
Contributor

@nbaztec nbaztec commented Oct 13, 2022

What does it do?

fixes para-tests to use rpc endpoint to detect RT upgrade, similar to the subscription.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@nbaztec nbaztec added B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) labels Oct 13, 2022
@crystalin
Copy link
Copy Markdown
Collaborator

I think this is fixed already by #1862 (fix for waitMigration)

@nbaztec
Copy link
Copy Markdown
Contributor Author

nbaztec commented Oct 18, 2022

I think this is fixed already by #1862 (fix for waitMigration)

If that's the intended assertion for the test, then we can close this PR. I was under the assumption that we wanted to fix the wait behavior so it is not out-of-sync with rpc & storage. Do we know why we need to wait 1 block after the rpc signals that the RT upgrade is done?

@crystalin
Copy link
Copy Markdown
Collaborator

I'm not sure I understand your question, but the runtime upgrade always happens in 2 steps:

  • The new runtime is applied
  • The following block execute the runtime migration

The waitMigration allows to wait until the block doing the runtime migration is executed

@crystalin
Copy link
Copy Markdown
Collaborator

We can add the RPC check in the test, but I want to make sure we keep the on-chain test as it is more important to have this one correct

@nbaztec
Copy link
Copy Markdown
Contributor Author

nbaztec commented Oct 18, 2022

I'm not sure I understand your question, but the runtime upgrade always happens in 2 steps:

  • The new runtime is applied
  • The following block execute the runtime migration

Does this mean that:

  • RPC endpoints already report the new version when runtime is applied (as shown by the test). We use api.rpc.subscribeNewRuntime which returns the new version 1 block before the storage is updated.
  • And by extension, the storage is only updated when the runtime is executed
    ?

@crystalin
Copy link
Copy Markdown
Collaborator

crystalin commented Oct 18, 2022

Does this mean that:

  • RPC endpoints already report the new version when runtime is applied (as shown by the test). We use api.rpc.subscribeNewRuntime which returns the new version 1 block before the storage is updated.

I don't know when it reports it, but yes it is likely to be before the on-chain.
The reason is that the node import the block right when it is produced. But the on-chain is considered best, when the block is included in the relay chain (so 12s later usually).

  • And by extension, the storage is only updated when the runtime is executed
    ?

The storage gets updated in the block doing the migration (1 after the runtime is applied)
Ex: Moonbeam 1803 (applied at 2073477, and migrated at 2073478

@nbaztec
Copy link
Copy Markdown
Contributor Author

nbaztec commented Oct 18, 2022

Cool thanks, think it's fine to close this one then, if the behavior is well understood.

@nbaztec nbaztec closed this Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants