Check runtime API versions in the JSON-RPC layer (#2995)

Fix #2974 cc #949 This PR modifies `runtime_call` to support an "API version constraint". This constraint is then verified. A `runtime_call_no_api_check` function has also been added as an escape hatch for the `state_call` JSON-RPC function. It also updates the `payment_info` module to account for paritytech/substrate#12633 Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
paritytech · Nov 11, 2022 · a1c9198 · a1c9198
1 parent 150bc4d
commit a1c9198
Show file tree

Hide file tree

Showing 4 changed files with 145 additions and 17 deletions.
diff --git a/bin/light-base/src/json_rpc_service.rs b/bin/light-base/src/json_rpc_service.rs
@@ -57,6 +57,7 @@ use alloc::{
 use core::{
     iter,
     num::{NonZeroU32, NonZeroUsize},
+    ops,
     sync::atomic,
     time::Duration,
 };
@@ -1512,8 +1513,38 @@ impl<TPlat: Platform> Background<TPlat> {
     }
 
     /// Performs a runtime call to a random block.
-    // TODO: maybe add a parameter to check for a runtime API?
     async fn runtime_call(
+        self: &Arc<Self>,
+        block_hash: &[u8; 32],
+        runtime_api: &str,
+        required_api_version_range: impl ops::RangeBounds<u32>,
+        function_to_call: &str,
+        call_parameters: impl Iterator<Item = impl AsRef<[u8]>> + Clone,
+        total_attempts: u32,
+        timeout_per_request: Duration,
+        max_parallel: NonZeroU32,
+    ) -> Result<RuntimeCallResult, RuntimeCallError> {
+        let (return_value, api_version) = self
+            .runtime_call_inner(
+                block_hash,
+                Some((runtime_api, required_api_version_range)),
+                function_to_call,
+                call_parameters,
+                total_attempts,
+                timeout_per_request,
+                max_parallel,
+            )
+            .await?;
+        Ok(RuntimeCallResult {
+            return_value,
+            api_version: api_version.unwrap(),
+        })
+    }
+
+    /// Performs a runtime call to a random block.
+    ///
+    /// Similar to [`Background::runtime_call`], except that the API version isn't checked.
+    async fn runtime_call_no_api_check(
         self: &Arc<Self>,
         block_hash: &[u8; 32],
         function_to_call: &str,
@@ -1522,6 +1553,32 @@ impl<TPlat: Platform> Background<TPlat> {
         timeout_per_request: Duration,
         max_parallel: NonZeroU32,
     ) -> Result<Vec<u8>, RuntimeCallError> {
+        let (return_value, _api_version) = self
+            .runtime_call_inner(
+                block_hash,
+                None::<(&str, ops::RangeFull)>,
+                function_to_call,
+                call_parameters,
+                total_attempts,
+                timeout_per_request,
+                max_parallel,
+            )
+            .await?;
+        debug_assert!(_api_version.is_none());
+        Ok(return_value)
+    }
+
+    /// Performs a runtime call to a random block.
+    async fn runtime_call_inner(
+        self: &Arc<Self>,
+        block_hash: &[u8; 32],
+        runtime_api_check: Option<(&str, impl ops::RangeBounds<u32>)>,
+        function_to_call: &str,
+        call_parameters: impl Iterator<Item = impl AsRef<[u8]>> + Clone,
+        total_attempts: u32,
+        timeout_per_request: Duration,
+        max_parallel: NonZeroU32,
+    ) -> Result<(Vec<u8>, Option<u32>), 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 = self.runtime_lock(block_hash).await?;
@@ -1537,6 +1594,22 @@ impl<TPlat: Platform> Background<TPlat> {
             .await
             .unwrap(); // TODO: don't unwrap
 
+        // Check that the runtime version is correct.
+        let runtime_api_version = if let Some((api_name, version_range)) = runtime_api_check {
+            let version = virtual_machine
+                .runtime_version()
+                .decode()
+                .apis
+                .find_version(api_name);
+            match version {
+                None => return Err(RuntimeCallError::ApiNotFound),
+                Some(v) if version_range.contains(&v) => Some(v),
+                Some(v) => return Err(RuntimeCallError::ApiVersionUnknown { actual_version: v }),
+            }
+        } else {
+            None
+        };
+
         // Now that we have obtained the virtual machine, we can perform the call.
         // This is a CPU-only operation that executes the virtual machine.
         // The virtual machine might access the storage.
@@ -1559,7 +1632,7 @@ impl<TPlat: Platform> Background<TPlat> {
                 read_only_runtime_host::RuntimeHostVm::Finished(Ok(success)) => {
                     let output = success.virtual_machine.value().as_ref().to_vec();
                     runtime_call_lock.unlock(success.virtual_machine.into_prototype());
-                    break Ok(output);
+                    break Ok((output, runtime_api_version));
                 }
                 read_only_runtime_host::RuntimeHostVm::Finished(Err(error)) => {
                     runtime_call_lock.unlock(error.prototype);
@@ -1617,6 +1690,13 @@ enum RuntimeCallError {
     StartError(host::StartErr),
     ReadOnlyRuntime(read_only_runtime_host::ErrorDetail),
     NextKeyForbidden,
+    /// Required runtime API isn't supported by the runtime.
+    ApiNotFound,
+    /// Version requirement of runtime API isn't supported.
+    ApiVersionUnknown {
+        /// Version that the runtime supports.
+        actual_version: u32,
+    },
 }
 
 /// Error potentially returned by [`Background::state_trie_root_hash`].
@@ -1627,3 +1707,9 @@ enum StateTrieRootHashError {
     /// Error while fetching block header from network.
     NetworkQueryError,
 }
+
+#[derive(Debug)]
+struct RuntimeCallResult {
+    return_value: Vec<u8>,
+    api_version: u32,
+}
diff --git a/bin/light-base/src/json_rpc_service/state_chain.rs b/bin/light-base/src/json_rpc_service/state_chain.rs
@@ -62,6 +62,8 @@ impl<TPlat: Platform> Background<TPlat> {
         let result = self
             .runtime_call(
                 &block_hash,
+                "AccountNonceApi",
+                1..=1,
                 "AccountNonceApi_account_nonce",
                 iter::once(&account.0),
                 4,
@@ -71,10 +73,11 @@ impl<TPlat: Platform> Background<TPlat> {
             .await;
 
         let response = match result {
-            Ok(nonce) => {
+            Ok(result) => {
                 // TODO: we get a u32 when expecting a u64; figure out problem
                 // TODO: don't unwrap
-                let index = u32::from_le_bytes(<[u8; 4]>::try_from(&nonce[..]).unwrap());
+                let index =
+                    u32::from_le_bytes(<[u8; 4]>::try_from(&result.return_value[..]).unwrap());
                 methods::Response::system_accountNextIndex(u64::from(index))
                     .to_json_response(request_id)
             }
@@ -857,6 +860,8 @@ impl<TPlat: Platform> Background<TPlat> {
         let result = self
             .runtime_call(
                 &block_hash,
+                "TransactionPaymentApi",
+                1..=2,
                 json_rpc::payment_info::PAYMENT_FEES_FUNCTION_NAME,
                 json_rpc::payment_info::payment_info_parameters(extrinsic),
                 4,
@@ -866,7 +871,10 @@ impl<TPlat: Platform> Background<TPlat> {
             .await;
 
         let response = match result {
-            Ok(encoded) => match json_rpc::payment_info::decode_payment_info(&encoded) {
+            Ok(result) => match json_rpc::payment_info::decode_payment_info(
+                &result.return_value,
+                result.api_version,
+            ) {
                 Ok(info) => methods::Response::payment_queryInfo(info).to_json_response(request_id),
                 Err(error) => json_rpc::parse::build_error_response(
                     request_id,
@@ -915,7 +923,7 @@ impl<TPlat: Platform> Background<TPlat> {
         };
 
         let result = self
-            .runtime_call(
+            .runtime_call_no_api_check(
                 &block_hash,
                 function_to_call,
                 iter::once(call_parameters.0),
@@ -1104,6 +1112,8 @@ impl<TPlat: Platform> Background<TPlat> {
         let result = self
             .runtime_call(
                 &block_hash,
+                "Metadata",
+                1..=1,
                 "Metadata_metadata",
                 iter::empty::<Vec<u8>>(),
                 3,
@@ -1113,7 +1123,7 @@ impl<TPlat: Platform> Background<TPlat> {
             .await;
         let result = result
             .as_ref()
-            .map(|output| remove_metadata_length_prefix(&output));
+            .map(|output| remove_metadata_length_prefix(&output.return_value));
 
         let response = match result {
             Ok(Ok(metadata)) => {

diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md
@@ -2,9 +2,14 @@
 
 ## Unreleased
 
+### Added
+
+- Add support for version 2 of the `TransactionPaymentApi` runtime API. This fixes the `payment_queryInfo` JSON-RPC call with newer runtime versions. ([#2995](https://github.com/paritytech/smoldot/pull/2995))
+
 ### Changed
 
 - The `enableExperimentalWebRTC` field has been removed from `ClientConfig`, and replaced with a `forbidWebRtc` option. WebRTC is now considered stable enough to be enabled by default. ([#2977](https://github.com/paritytech/smoldot/pull/2977))
+- The version of the runtime API is now verified to match the excepted value when the `payment_queryInfo`, `state_getMetadata`, and `system_accountNextIndex` JSON-RPC functions are called. This means that without an update to the smoldot source code these JSON-RPC functions will stop working if the runtime API is out of range. However, this eliminates the likelihood that smoldot returns accidentally parses a value in a different way than intended and an incorrect result. ([#2995](https://github.com/paritytech/smoldot/pull/2995))
 - Reduced the number of networking round-trips after a connection has been opened by assuming that the remote supports the desired networking protocols instead of waiting for its confirmation. ([#2984](https://github.com/paritytech/smoldot/pull/2984))
 
 ## 0.7.6 - 2022-11-04

diff --git a/src/json_rpc/payment_info.rs b/src/json_rpc/payment_info.rs
@@ -32,28 +32,55 @@ pub fn payment_info_parameters(
 pub const PAYMENT_FEES_FUNCTION_NAME: &str = "TransactionPaymentApi_query_info";
 
 /// Attempt to decode the output of the runtime call.
+///
+/// Must be passed the version of the `TransactionPaymentApi` API, according to the runtime
+/// specification.
 pub fn decode_payment_info(
     scale_encoded: &'_ [u8],
+    api_version: u32,
 ) -> Result<methods::RuntimeDispatchInfo, DecodeError> {
-    match nom::combinator::all_consuming(nom_decode_payment_info::<nom::error::Error<&'_ [u8]>>)(
-        scale_encoded,
-    ) {
+    let is_api_v2 = match api_version {
+        1 => false,
+        2 => true,
+        _ => return Err(DecodeError::UnknownRuntimeVersion),
+    };
+
+    match nom::combinator::all_consuming(nom_decode_payment_info::<nom::error::Error<&'_ [u8]>>(
+        is_api_v2,
+    ))(scale_encoded)
+    {
         Ok((_, info)) => Ok(info),
-        Err(_) => Err(DecodeError()),
+        Err(_) => Err(DecodeError::ParseError),
     }
 }
 
 /// Potential error when decoding payment information runtime output.
 #[derive(Debug, derive_more::Display)]
-#[display(fmt = "Payment info parsing error")]
-pub struct DecodeError();
+pub enum DecodeError {
+    /// Failed to parse the return value of `TransactionPaymentApi_query_info`.
+    ParseError,
+    /// The `TransactionPaymentApi` API uses a version that smoldot doesn't support.
+    UnknownRuntimeVersion,
+}
 
 fn nom_decode_payment_info<'a, E: nom::error::ParseError<&'a [u8]>>(
-    value: &'a [u8],
-) -> nom::IResult<&'a [u8], methods::RuntimeDispatchInfo, E> {
+    is_api_v2: bool,
+) -> impl FnMut(&'a [u8]) -> nom::IResult<&'a [u8], methods::RuntimeDispatchInfo, E> {
     nom::combinator::map(
         nom::sequence::tuple((
-            nom::number::complete::le_u64,
+            move |bytes| {
+                if is_api_v2 {
+                    nom::number::complete::le_u64(bytes)
+                } else {
+                    nom::combinator::map(
+                        nom::sequence::tuple((
+                            crate::util::nom_scale_compact_u64,
+                            crate::util::nom_scale_compact_u64,
+                        )),
+                        |(ref_time, _proof_size)| ref_time,
+                    )(bytes)
+                }
+            },
             nom::combinator::map_opt(nom::number::complete::u8, |n| match n {
                 0 => Some(methods::DispatchClass::Normal),
                 1 => Some(methods::DispatchClass::Operational),
@@ -105,5 +132,5 @@ fn nom_decode_payment_info<'a, E: nom::error::ParseError<&'a [u8]>>(
             class,
             partial_fee,
         },
-    )(value)
+    )
 }