Add runtime_upgrades() runtime API for DomainRuntimeUpgrades #3337
Conversation
teor2345
added
the
breaking-runtime
teor2345
requested review from
NingLin-P,
vedhavyas and
nazar-pc
as code owners
| // The runtime_upgrades() API is only present in API versions 2 and later. On earlier versions, | ||
| // we need to call legacy code. | ||
| // TODO: remove version check before next network | ||
| let domains_api_version = runtime_api | ||
| .api_version::<dyn DomainsApi<CBlock, CBlock::Header>>(consensus_block_hash)? | ||
| // It is safe to return a default version of 1, since there will always be version 1. | ||
| .unwrap_or(1); | ||
|
|
||
| let is_upgraded = if domains_api_version >= 2 { | ||
| let runtime_upgrades = runtime_api.runtime_upgrades(consensus_block_hash)?; | ||
| runtime_upgrades.contains(&runtime_id) | ||
| } else { | ||
| let header = consensus_client.header(consensus_block_hash)?.ok_or( | ||
| sp_blockchain::Error::MissingHeader(format!( | ||
| "No header found for {consensus_block_hash:?}" | ||
| )), | ||
| )?; | ||
| header | ||
| .digest() | ||
| .logs | ||
| .iter() | ||
| .filter_map(|log| log.as_domain_runtime_upgrade()) | ||
| .any(|upgraded_runtime_id| upgraded_runtime_id == runtime_id) | ||
| }; |
There was a problem hiding this comment.
I think we'll need this compatibility code for inherents, so that runtime upgrades on Taurus will work. I avoided repeating the compatibility code by calling this method instead.
I think we'll be able to remove this code (edit: the compatibility code) after the runtime on Taurus is upgraded, and most nodes on Taurus and Mainnet are upgraded?
There was a problem hiding this comment.
I didn't read into this PR to understand the context, but as a general rule inherents can't be removed without making it impossible to import older blocks. They are kind of like host functions for runtime environment that once added can't be removed.
There was a problem hiding this comment.
I didn't read into this PR to understand the context, but as a general rule inherents can't be removed without making it impossible to import older blocks. They are kind of like host functions for runtime environment that once added can't be removed.
No inherents are added or removed in this PR, it just changes how we process them to avoid some duplicate work.
This PR changes the runtime code the domain client calls when it processes a domain runtime upgrade inherent. Previously, we searched through the block header to determine if the runtime was upgraded. Now we ask the runtime for a list of upgraded runtime IDs (which the domains pallet has already determined by doing a similar search.)
There was a problem hiding this comment.
I think we'll be able to remove this code (edit: the compatibility code) after the runtime on Taurus is upgraded, and most nodes on Taurus and Mainnet are upgraded.
Nazar is correct, we can't remove the compatibility code even after runtime/node upgraded. Because the runtime code is on-chain state, it applies to block that after the upgrade and before the next upgrade, older block will still use the old runtime (which doesn't have the new runtime API). Image a node (running on the latest release that doesn't contain the compatibility code) just joined the network and sync from genesis, it will fail to call the new API until the block that upgrade the runtime.
This PR adds a new
runtime_upgrades()domains runtime API, which returns the contents ofDomainRuntimeUpgrades.It simplifies the fraud proof and inherents code that searches through the block header for upgrades, to use the new API instead. (No inherents are added or removed, we just change how we process them to avoid duplicating the same work inside and outside the runtime.)
I also made some changes to:
Part of #3281.
This PR doesn't need spec changes, because the changes are too low-level. The changes in my spec PR describe the implementation before and after the changes in this PR.
subspace/protocol-specs#51
Code contributor checklist: