Skip to content

Commit

Permalink
Rewrite state_get_runtime_version to use shared code (#2736)
Browse files Browse the repository at this point in the history 
Fix #2468

This PR does two things:

- Split `runtime_call` in half, where the first half is now a separate
function named `runtime_lock`.
- Make `state_get_runtime_version` use this new `runtime_lock` function.

This is better so that we don't repeat a lot of code, and also fixes a
few potential panics.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@tomaka @mergify
tomaka and mergify[bot] authored Sep 13, 2022
1 parent d2c7918 commit 3beb9b8
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 173 deletions.
164 changes: 84 additions & 80 deletions bin/light-base/src/json_rpc_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,89 @@ impl<TPlat: Platform> Background<TPlat> {
Ok(result)
}

/// Obtain a lock to the runtime of the given block against the runtime service.
// TODO: return better error?
async fn runtime_lock<'a>(
self: &'a Arc<Self>,
block_hash: &[u8; 32],
) -> Result<runtime_service::RuntimeLock<'a, TPlat>, RuntimeCallError> {
let cache_lock = self.cache.lock().await;

// Try to find the block in the cache of recent blocks. Most of the time, the call target
// should be in there.
let lock = if cache_lock.recent_pinned_blocks.contains(block_hash) {
// The runtime service has the block pinned, meaning that we can ask the runtime
// service to perform the call.
self.runtime_service
.pinned_block_runtime_lock(cache_lock.subscription_id.clone().unwrap(), block_hash)
.await
.ok()
} else {
None
};

Ok(if let Some(lock) = lock {
lock
} else {
// Second situation: the block is not in the cache of recent blocks. This isn't great.
drop::<futures::lock::MutexGuard<_>>(cache_lock);

// The only solution is to download the runtime of the block in question from the network.

// TODO: considering caching the runtime code the same way as the state trie root hash

// In order to grab the runtime code and perform the call network request, we need
// to know the state trie root hash and the height of the block.
let (state_trie_root_hash, block_number) =
self.state_trie_root_hash(block_hash).await.unwrap(); // TODO: don't unwrap

// Download the runtime of this block. This takes a long time as the runtime is rather
// big (around 1MiB in general).
let (storage_code, storage_heap_pages) = {
let mut code_query_result = self
.sync_service
.clone()
.storage_query(
block_number,
block_hash,
&state_trie_root_hash,
iter::once(&b":code"[..]).chain(iter::once(&b":heappages"[..])),
3,
Duration::from_secs(20),
NonZeroU32::new(1).unwrap(),
)
.await
.map_err(runtime_service::RuntimeCallError::StorageQuery)
.map_err(RuntimeCallError::Call)?;
let heap_pages = code_query_result.pop().unwrap();
let code = code_query_result.pop().unwrap();
(code, heap_pages)
};

// Give the code and heap pages to the runtime service. The runtime service will
// try to find any similar runtime it might have, and if not will compile it.
let pinned_runtime_id = self
.runtime_service
.compile_and_pin_runtime(storage_code, storage_heap_pages)
.await;

let precall = self
.runtime_service
.pinned_runtime_lock(
pinned_runtime_id.clone(),
*block_hash,
block_number,
state_trie_root_hash,
)
.await;

// TODO: consider keeping pinned runtimes in a cache instead
self.runtime_service.unpin_runtime(pinned_runtime_id).await;

precall
})
}

/// Performs a runtime call to a random block.
// TODO: maybe add a parameter to check for a runtime API?
async fn runtime_call(
Expand All @@ -1322,86 +1405,7 @@ impl<TPlat: Platform> Background<TPlat> {
) -> Result<Vec<u8>, RuntimeCallError> {
// This function contains two steps: obtaining the runtime of the block in question,
// then performing the actual call. The first step is the longest and most difficult.
let precall = {
let cache_lock = self.cache.lock().await;

// Try to find the block in the cache of recent blocks. Most of the time, the call target
// should be in there.
let lock = if cache_lock.recent_pinned_blocks.contains(block_hash) {
// The runtime service has the block pinned, meaning that we can ask the runtime
// service to perform the call.
self.runtime_service
.pinned_block_runtime_lock(
cache_lock.subscription_id.clone().unwrap(),
block_hash,
)
.await
.ok()
} else {
None
};

if let Some(lock) = lock {
lock
} else {
// Second situation: the block is not in the cache of recent blocks. This isn't great.
drop::<futures::lock::MutexGuard<_>>(cache_lock);

// The only solution is to download the runtime of the block in question from the network.

// TODO: considering caching the runtime code the same way as the state trie root hash

// In order to grab the runtime code and perform the call network request, we need
// to know the state trie root hash and the height of the block.
let (state_trie_root_hash, block_number) =
self.state_trie_root_hash(block_hash).await.unwrap(); // TODO: don't unwrap

// Download the runtime of this block. This takes a long time as the runtime is rather
// big (around 1MiB in general).
let (storage_code, storage_heap_pages) = {
let mut code_query_result = self
.sync_service
.clone()
.storage_query(
block_number,
block_hash,
&state_trie_root_hash,
iter::once(&b":code"[..]).chain(iter::once(&b":heappages"[..])),
3,
Duration::from_secs(20),
NonZeroU32::new(1).unwrap(),
)
.await
.map_err(runtime_service::RuntimeCallError::StorageQuery)
.map_err(RuntimeCallError::Call)?;
let heap_pages = code_query_result.pop().unwrap();
let code = code_query_result.pop().unwrap();
(code, heap_pages)
};

// Give the code and heap pages to the runtime service. The runtime service will
// try to find any similar runtime it might have, and if not will compile it.
let pinned_runtime_id = self
.runtime_service
.compile_and_pin_runtime(storage_code, storage_heap_pages)
.await;

let precall = self
.runtime_service
.pinned_runtime_lock(
pinned_runtime_id.clone(),
*block_hash,
block_number,
state_trie_root_hash,
)
.await;

// TODO: consider keeping pinned runtimes in a cache instead
self.runtime_service.unpin_runtime(pinned_runtime_id).await;

precall
}
};
let precall = self.runtime_lock(block_hash).await?;

let (runtime_call_lock, virtual_machine) = precall
.start(
Expand Down
105 changes: 12 additions & 93 deletions bin/light-base/src/json_rpc_service/state_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! All legacy JSON-RPC method handlers that relate to the chain or the storage.
use super::{Background, Platform, RuntimeCallError, SubscriptionTy};
use super::{Background, Platform, SubscriptionTy};

use crate::runtime_service;

Expand Down Expand Up @@ -1162,98 +1162,12 @@ impl<TPlat: Platform> Background<TPlat> {
),
};

// This function contains two steps: obtaining the runtime of the block in question,
// then obtaining the specification. The first step is the longest and most difficult.
let specification = {
let cache_lock = self.cache.lock().await;

// Try to find the block in the cache of recent blocks. Most of the time, the call
// target should be in there.
let spec = if cache_lock.recent_pinned_blocks.contains(&block_hash) {
// The runtime service has the block pinned, meaning that we can ask the runtime
// service for the specification.
self.runtime_service
.pinned_block_runtime_lock(
cache_lock.subscription_id.clone().unwrap(),
&block_hash,
)
.await
.ok()
.map(|rt| rt.specification())
} else {
None
};

// If the block isn't a recent block or if the subscription is obsolete, fall back
// to downloading it.
if let Some(spec) = spec {
spec
} else {
// Second situation: the block is not in the cache of recent blocks. This
// isn't great.
drop::<futures::lock::MutexGuard<_>>(cache_lock);

// The only solution is to download the runtime of the block in question from the network.

// TODO: considering caching the runtime code the same way as the state trie root hash
// TODO: DRY with runtime_call()

// In order to grab the runtime code and perform the call network request, we need
// to know the state trie root hash and the height of the block.
let (state_trie_root_hash, block_number) =
self.state_trie_root_hash(&block_hash).await.unwrap(); // TODO: don't unwrap

// Download the runtime of this block. This takes a long time as the runtime is rather
// big (around 1MiB in general).
let (storage_code, storage_heap_pages) = {
let mut code_query_result = self
.sync_service
.clone()
.storage_query(
block_number,
&block_hash,
&state_trie_root_hash,
iter::once(&b":code"[..]).chain(iter::once(&b":heappages"[..])),
3,
Duration::from_secs(24),
NonZeroU32::new(1).unwrap(),
)
.await
.map_err(runtime_service::RuntimeCallError::StorageQuery)
.map_err(RuntimeCallError::Call)
.unwrap(); // TODO: don't unwrap /!\
let heap_pages = code_query_result.pop().unwrap();
let code = code_query_result.pop().unwrap();
(code, heap_pages)
};

// Give the code and heap pages to the runtime service. The runtime service will
// try to find any similar runtime it might have, and if not will compile it.
let pinned_runtime_id = self
.runtime_service
.compile_and_pin_runtime(storage_code, storage_heap_pages)
.await;

let runtime = self
.runtime_service
.pinned_runtime_lock(
pinned_runtime_id.clone(),
block_hash,
block_number,
state_trie_root_hash,
)
.await;

// TODO: consider keeping pinned runtimes in a cache instead
self.runtime_service.unpin_runtime(pinned_runtime_id).await;

runtime.specification()
}
};

// Now that we have the runtime specification, turn it into a JSON-RPC response.
let response = match specification {
Ok(spec) => {
let response = match self
.runtime_lock(&block_hash)
.await
.map(|l| l.specification())
{
Ok(Ok(spec)) => {
let runtime_spec = spec.decode();
methods::Response::state_getRuntimeVersion(methods::RuntimeVersion {
spec_name: runtime_spec.spec_name.into(),
Expand All @@ -1270,6 +1184,11 @@ impl<TPlat: Platform> Background<TPlat> {
})
.to_json_response(request_id)
}
Ok(Err(error)) => json_rpc::parse::build_error_response(
request_id,
json_rpc::parse::ErrorResponse::ServerError(-32000, &error.to_string()),
None,
),
Err(error) => json_rpc::parse::build_error_response(
request_id,
json_rpc::parse::ErrorResponse::ServerError(-32000, &error.to_string()),
Expand Down
4 changes: 4 additions & 0 deletions bin/wasm-node/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

- A limit to the number of substreams a remote can maintain open over a connection is now enforced. ([#2724](https://github.com/paritytech/smoldot/pull/2724))

### Fixed

- No longer panic when calling `state_getRuntimeVersion` is unable to download the runtime code of an old block from the network. ([#2736](https://github.com/paritytech/smoldot/pull/2736))

## 0.6.32 - 2022-09-07

### Fixed
Expand Down

0 comments on commit 3beb9b8

Please sign in to comment.