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

fix(runtime update): wait until upgrade on chain #1321

Merged
merged 13 commits into from
Jan 8, 2024
Merged

Conversation

niklasad1
Copy link
Member

@niklasad1 niklasad1 commented Dec 15, 2023

Close #1319

This PR ensures that the runtime upgrade from state_subscribeRuntimeVersion is enacted on a block, and only after that point the metadata is fetched and updated by subxt.

The implementation itself queries system::lastRuntimeUpgrade to ensure that, but there may be better alternatives.

I also tested it by just waiting for one more block, but that didn't work when I tested it but this solution does.

What is bad

It's not nice to depend on storage stuff in the subxt client but I haven't find a better way than to wait for around 5-10 blocks statically.

Other alternatives

Subscribe to the storage key system().last_runtime_upgrade() would work as well

@niklasad1 niklasad1 requested a review from a team as a code owner December 15, 2023 10:18
let spec_version = scale_val
.at("spec_version")
.and_then(|v| v.as_u128())
.expect("specVersion should exist on RuntimeVersion; qed")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: since we're talking to a thing that could return any sort of nonsense it's probably best to return an actual error here

let latest_block_ref = match self.client.backend().latest_finalized_block_ref().await {
Ok(block_ref) => block_ref,
Err(e) => return Some(Err(e)),
let at = match wait_for_runtime_upgrade(&self.client, &runtime_version).await {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit; maybe rename to wait_for_finalized_block_containing_update or something like that

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Thanks @niklasad1; def an improvement on what we have! Just keep the BlockRef around until after we've fetched metadata :)

subxt/src/client/online_client.rs Outdated Show resolved Hide resolved
)
.await
{
let metadata = match OnlineClient::fetch_metadata(self.client.backend(), at.hash()).await {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we fetch the metadata from the backend here? I guess it is not part of the "LastRuntimeUpgrade" storage item for which we already got the scale_val below, that contains the spec_version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep exactly; the storage item tells us that the runtime version has updated in this block (but doesn't contain the metadata too), and so once we see that a block is updated, we fetch the metadata at that block to use to update Subxt

@jsdw
Copy link
Collaborator

jsdw commented Jan 4, 2024

One question with this; we now wait for a finalized block that contains a newer spec version, and when we see this, we fetch metadata from it to apply in Subxt. Am I right in thinking that it's only the block after the one with the updates spec version that requires the new metadata? Or when we see this block with a new spec version, are we already too late (ie we should be using updated metadata to work with it, too)?

@niklasad1
Copy link
Member Author

niklasad1 commented Jan 4, 2024

Or when we see this block with a new spec version, are we already too late (ie we should be using updated metadata to work with it, too)

AFAIU, the runtime upgrade has already been applied when this storage item is changed such that it may be possible that transaction(s) on the block b before the metadata is "updated" may fail but after the metadata has been updated it should work for the new runtime at block b+1.

EDIT: tested it in staking-miner again with some additional logs

Legacy RPC backend

# state_subscribeRuntimeVersion emits an event that a runtime upgrade is planned
2024-01-05T09:03:12.787867Z  INFO subxt::client::online_client: Runtime upgrade initiated at #10

# As these logs indicates the runtime upgrade isn't enacted at block n+1
2024-01-05T09:03:12.788934Z  INFO subxt::client::online_client: block #10 didn't contain the runtime upgrade
2024-01-05T09:03:14.256722Z  INFO subxt::client::online_client: block #11 didn't contain the runtime upgrade
2024-01-05T09:03:22.260623Z  INFO subxt::client::online_client: block #12 didn't contain the runtime upgrade
2024-01-05T09:03:26.270366Z  INFO subxt::client::online_client: block #13 didn't contain the runtime upgrade

# Now the runtime upgrade applied on chain and subxt updates the metadata
2024-01-05T09:03:34.266723Z  INFO subxt::client::online_client: block #14 a new runtime
2024-01-05T09:03:34.322435Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::SignedMaxWeight`: Weight(ref_time: 1479424380000, proof_size: 13650590614545068195)
2024-01-05T09:03:34.322461Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxLength`: 3538944
2024-01-05T09:03:34.322466Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxVotesPerVoter`: 16
2024-01-05T09:03:34.322471Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MaxWinners`: 13
2024-01-05T09:03:34.322523Z  INFO polkadot-staking-miner: upgrade to version: 1009000 successful

Unstable RPC backend

2024-01-08T14:35:50.619839Z  INFO polkadot-staking-miner: Started prometheus endpoint on http://0.0.0.0:9999
2024-01-08T14:35:50.619885Z  INFO polkadot-staking-miner: Connected to chain: westend
2024-01-08T14:35:50.619967Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::SignedMaxWeight`: Weight(ref_time: 1479424380000, proof_size: 13650590614545068195)
2024-01-08T14:35:50.619991Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxLength`: 3538944
2024-01-08T14:35:50.620005Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxVotesPerVoter`: 16
2024-01-08T14:35:50.620018Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MaxWinners`: 1000
2024-01-08T14:35:50.620484Z  INFO subxt::client::online_client: Runtime upgrade initiated at #0
2024-01-08T14:35:50.621633Z  INFO subxt::client::online_client: block #0 new runtime
2024-01-08T14:35:50.663001Z DEBUG polkadot-staking-miner: upgrade to version: 1005000 failed: SameVersion
2024-01-08T14:37:12.788707Z  INFO subxt::backend::unstable: new runtime: Valid(RuntimeVersionEvent { spec: RuntimeSpec { spec_name: "westend", impl_name: "parity-westend", spec_version: 1009000, impl_version: 0, transaction_version: 24, apis: {"0xcbca25e39f142387": 2, "0xed99c5acb25eedf5": 3, "0xf78b278be53f454c": 2, "0x40fe3ad401f8959a": 6, "0xd2bc9897eed08f15": 3, "0x37e397fc7c91f5e4": 2, "0xbc9d89904f5b923f": 1, "0x18ef58a3b67ba770": 1, "0x17a6bc0d0062aeb3": 1, "0x49eaaf1b548a0cb0": 3, "0x91d5df18b0d2cf58": 2, "0x2a5e924655399e60": 1, "0xaf2c0297a23e6d3d": 10, "0xab3c0572291feb8b": 1, "0xf3ff14d5ab527059": 3, "0xfbc577b9d747efd6": 1, "0x687ad44ad37f03c2": 1, "0x37c8bb1350a9a2a8": 4, "0xdf6acb689907609b": 4} } })
2024-01-08T14:37:14.304309Z  INFO subxt::backend::unstable: runtimes: {0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93: Valid(RuntimeVersionEvent { spec: RuntimeSpec { spec_name: "westend", impl_name: "parity-westend", spec_version: 1009000, impl_version: 0, transaction_version: 24, apis: {"0xcbca25e39f142387": 2, "0xed99c5acb25eedf5": 3, "0xf78b278be53f454c": 2, "0x40fe3ad401f8959a": 6, "0xd2bc9897eed08f15": 3, "0x37e397fc7c91f5e4": 2, "0xbc9d89904f5b923f": 1, "0x18ef58a3b67ba770": 1, "0x17a6bc0d0062aeb3": 1, "0x49eaaf1b548a0cb0": 3, "0x91d5df18b0d2cf58": 2, "0x2a5e924655399e60": 1, "0xaf2c0297a23e6d3d": 10, "0xab3c0572291feb8b": 1, "0xf3ff14d5ab527059": 3, "0xfbc577b9d747efd6": 1, "0x687ad44ad37f03c2": 1, "0x37c8bb1350a9a2a8": 4, "0xdf6acb689907609b": 4} } })}
2024-01-08T14:37:14.304373Z  INFO subxt::backend::unstable: finalized hash blocks: [BlockRef { inner: BlockRefInner { hash: 0xa5b492fb4e581c37726d10bb09f83e92dad4f860293e2ff08c1254bce78f9626, unpin_flags: Mutex { data: {0xad31c9d0092326dd08ca97877d5b289b9eed4483b52c51b58f53b77e26f5dbfa, 0x71c06321fab60141afbb075d3050417029c3c0acf8a89c5d8be07dba7dbcecb4, 0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93}, poisoned: false, .. } } }]
2024-01-08T14:37:22.308694Z  INFO subxt::backend::unstable: runtimes: {0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93: Valid(RuntimeVersionEvent { spec: RuntimeSpec { spec_name: "westend", impl_name: "parity-westend", spec_version: 1009000, impl_version: 0, transaction_version: 24, apis: {"0xcbca25e39f142387": 2, "0xed99c5acb25eedf5": 3, "0xf78b278be53f454c": 2, "0x40fe3ad401f8959a": 6, "0xd2bc9897eed08f15": 3, "0x37e397fc7c91f5e4": 2, "0xbc9d89904f5b923f": 1, "0x18ef58a3b67ba770": 1, "0x17a6bc0d0062aeb3": 1, "0x49eaaf1b548a0cb0": 3, "0x91d5df18b0d2cf58": 2, "0x2a5e924655399e60": 1, "0xaf2c0297a23e6d3d": 10, "0xab3c0572291feb8b": 1, "0xf3ff14d5ab527059": 3, "0xfbc577b9d747efd6": 1, "0x687ad44ad37f03c2": 1, "0x37c8bb1350a9a2a8": 4, "0xdf6acb689907609b": 4} } })}
2024-01-08T14:37:22.308773Z  INFO subxt::backend::unstable: finalized hash blocks: [BlockRef { inner: BlockRefInner { hash: 0x71c06321fab60141afbb075d3050417029c3c0acf8a89c5d8be07dba7dbcecb4, unpin_flags: Mutex { data: {0x3588e20721ed1e8fd2bd9892e52be4c3125f02ae63d2b84ffc8ffc62180e503a, 0xa5b492fb4e581c37726d10bb09f83e92dad4f860293e2ff08c1254bce78f9626, 0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93}, poisoned: false, .. } } }]
2024-01-08T14:37:26.320546Z  INFO subxt::backend::unstable: runtimes: {0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93: Valid(RuntimeVersionEvent { spec: RuntimeSpec { spec_name: "westend", impl_name: "parity-westend", spec_version: 1009000, impl_version: 0, transaction_version: 24, apis: {"0xcbca25e39f142387": 2, "0xed99c5acb25eedf5": 3, "0xf78b278be53f454c": 2, "0x40fe3ad401f8959a": 6, "0xd2bc9897eed08f15": 3, "0x37e397fc7c91f5e4": 2, "0xbc9d89904f5b923f": 1, "0x18ef58a3b67ba770": 1, "0x17a6bc0d0062aeb3": 1, "0x49eaaf1b548a0cb0": 3, "0x91d5df18b0d2cf58": 2, "0x2a5e924655399e60": 1, "0xaf2c0297a23e6d3d": 10, "0xab3c0572291feb8b": 1, "0xf3ff14d5ab527059": 3, "0xfbc577b9d747efd6": 1, "0x687ad44ad37f03c2": 1, "0x37c8bb1350a9a2a8": 4, "0xdf6acb689907609b": 4} } })}
2024-01-08T14:37:26.320618Z  INFO subxt::backend::unstable: finalized hash blocks: [BlockRef { inner: BlockRefInner { hash: 0x26597ffaf69cf6263b737c1ac3c4e29f4c7b366857a45e200d77f2a1201b2c93, unpin_flags: Mutex { data: {0x71c06321fab60141afbb075d3050417029c3c0acf8a89c5d8be07dba7dbcecb4, 0x3588e20721ed1e8fd2bd9892e52be4c3125f02ae63d2b84ffc8ffc62180e503a, 0xdf65b786c794b83868a677772c3d24721f8b73e37a92d0a11a789ba1b5488bac}, poisoned: false, .. } } }]
2024-01-08T14:37:26.321308Z  INFO subxt::client::online_client: Runtime upgrade initiated at #14
2024-01-08T14:37:26.322241Z  INFO subxt::client::online_client: block #14 no new runtime
2024-01-08T14:37:34.316196Z  INFO subxt::client::online_client: block #15 new runtime
2024-01-08T14:37:34.369538Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::SignedMaxWeight`: Weight(ref_time: 1479424380000, proof_size: 13650590614545068195)
2024-01-08T14:37:34.369560Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxLength`: 3538944
2024-01-08T14:37:34.369565Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MinerMaxVotesPerVoter`: 16
2024-01-08T14:37:34.369569Z TRACE polkadot-staking-miner: updating metadata constant `ElectionProviderMultiPhase::MaxWinners`: 13
2024-01-08T14:37:34.369673Z  INFO polkadot-staking-miner: upgrade to version: 1009000 successful

Thus, it might be possible that transactions etc on block #14 gets rejected before the metadata is updated which could occur if fetching the metadata is very slow... but I think the implementation is correct or maybe understand your point?

@niklasad1 niklasad1 merged commit 298869b into master Jan 8, 2024
11 checks passed
@niklasad1 niklasad1 deleted the na-fix-1319 branch January 8, 2024 14:48
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.

runtime upgrade stream API: fetched metadata is "outdated" when upgrade is applied
3 participants