From 2867b1bd56bece95058fa15f8420ff6c4df11a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 7 Nov 2022 15:22:57 +0100 Subject: [PATCH 1/3] `payment_queryInfo`: Make it work with `WeighV2` The runtime api for querying the payment info depends on the `Weight` type and broke for old runtimes that still use the `WeighV1`. This pull requests fixes this by: 1. Bumping the version of the runtime api. 2. Making the node side code use the correct runtime api function depending on the version of the runtime api. 3. Make the RPC always return `WeighV1`. Users of the api should switch to `state_call` and decide based on the version of the runtime api which `Weight` type is being returned. --- Cargo.lock | 2 + frame/transaction-payment/rpc/Cargo.toml | 1 + .../rpc/runtime-api/Cargo.toml | 2 + .../rpc/runtime-api/src/lib.rs | 4 ++ frame/transaction-payment/rpc/src/lib.rs | 41 +++++++++++++++---- frame/transaction-payment/src/types.rs | 14 +++++-- primitives/weights/src/lib.rs | 6 +-- 7 files changed, 54 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 510161e225732..48205d9bd86da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6326,6 +6326,7 @@ dependencies = [ "sp-core", "sp-rpc", "sp-runtime", + "sp-weights", ] [[package]] @@ -6336,6 +6337,7 @@ dependencies = [ "parity-scale-codec", "sp-api", "sp-runtime", + "sp-weights", ] [[package]] diff --git a/frame/transaction-payment/rpc/Cargo.toml b/frame/transaction-payment/rpc/Cargo.toml index 16c2cc55efefb..9dd42c12c8bbf 100644 --- a/frame/transaction-payment/rpc/Cargo.toml +++ b/frame/transaction-payment/rpc/Cargo.toml @@ -21,3 +21,4 @@ sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" sp-core = { version = "6.0.0", path = "../../../primitives/core" } sp-rpc = { version = "6.0.0", path = "../../../primitives/rpc" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } +sp-weights = { version = "4.0.0", path = "../../../primitives/weights" } diff --git a/frame/transaction-payment/rpc/runtime-api/Cargo.toml b/frame/transaction-payment/rpc/runtime-api/Cargo.toml index 5e1cb46753524..c0b816684a2f3 100644 --- a/frame/transaction-payment/rpc/runtime-api/Cargo.toml +++ b/frame/transaction-payment/rpc/runtime-api/Cargo.toml @@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "3.0.0", default-features = pallet-transaction-payment = { version = "4.0.0-dev", default-features = false, path = "../../../transaction-payment" } sp-api = { version = "4.0.0-dev", default-features = false, path = "../../../../primitives/api" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../../../primitives/runtime" } +sp-weights = { version = "4.0.0", default-features = false, path = "../../../../primitives/weights" } [features] default = ["std"] @@ -25,4 +26,5 @@ std = [ "pallet-transaction-payment/std", "sp-api/std", "sp-runtime/std", + "sp-weights/std", ] diff --git a/frame/transaction-payment/rpc/runtime-api/src/lib.rs b/frame/transaction-payment/rpc/runtime-api/src/lib.rs index 6944593daa57a..10fd2a9e61fc1 100644 --- a/frame/transaction-payment/rpc/runtime-api/src/lib.rs +++ b/frame/transaction-payment/rpc/runtime-api/src/lib.rs @@ -25,13 +25,17 @@ use sp_runtime::traits::MaybeDisplay; pub use pallet_transaction_payment::{FeeDetails, InclusionFee, RuntimeDispatchInfo}; sp_api::decl_runtime_apis! { + #[api_version(2)] pub trait TransactionPaymentApi where Balance: Codec + MaybeDisplay, { + #[changed_in(2)] + fn query_info(uxt: Block::Extrinsic, len: u32) -> RuntimeDispatchInfo; fn query_info(uxt: Block::Extrinsic, len: u32) -> RuntimeDispatchInfo; fn query_fee_details(uxt: Block::Extrinsic, len: u32) -> FeeDetails; } + #[api_version(2)] pub trait TransactionPaymentCallApi where Balance: Codec + MaybeDisplay, diff --git a/frame/transaction-payment/rpc/src/lib.rs b/frame/transaction-payment/rpc/src/lib.rs index 0c7e26cec0f58..1c8dbd626680e 100644 --- a/frame/transaction-payment/rpc/src/lib.rs +++ b/frame/transaction-payment/rpc/src/lib.rs @@ -26,7 +26,7 @@ use jsonrpsee::{ types::error::{CallError, ErrorCode, ErrorObject}, }; use pallet_transaction_payment_rpc_runtime_api::{FeeDetails, InclusionFee, RuntimeDispatchInfo}; -use sp_api::ProvideRuntimeApi; +use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_core::Bytes; use sp_rpc::number::NumberOrHex; @@ -82,8 +82,10 @@ impl From for i32 { } impl - TransactionPaymentApiServer<::Hash, RuntimeDispatchInfo> - for TransactionPayment + TransactionPaymentApiServer< + ::Hash, + RuntimeDispatchInfo, + > for TransactionPayment where Block: BlockT, C: ProvideRuntimeApi + HeaderBackend + Send + Sync + 'static, @@ -94,7 +96,7 @@ where &self, encoded_xt: Bytes, at: Option, - ) -> RpcResult> { + ) -> RpcResult> { let api = self.client.runtime_api(); let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash)); @@ -107,14 +109,35 @@ where Some(format!("{:?}", e)), )) })?; - api.query_info(&at, uxt, encoded_len).map_err(|e| { + + fn map_err(error: impl ToString, desc: &'static str) -> CallError { CallError::Custom(ErrorObject::owned( Error::RuntimeError.into(), - "Unable to query dispatch info.", - Some(e.to_string()), + desc, + Some(error.to_string()), )) - .into() - }) + } + + if api + .api_version::>(&at) + .map_err(|e| map_err(e, "Transaction payment runtime api not present."))? + .unwrap_or(0) < + 2 + { + #[allow(deprecated)] + api.query_info_before_version_2(&at, uxt, encoded_len) + .map_err(|e| map_err(e, "Unable to query dispatch info.").into()) + } else { + let res = api + .query_info(&at, uxt, encoded_len) + .map_err(|e| map_err(e, "Unable to query dispatch info."))?; + + Ok(RuntimeDispatchInfo { + weight: sp_weights::OldWeight(res.weight.ref_time()), + class: res.class, + partial_fee: res.partial_fee, + }) + } } fn query_fee_details( diff --git a/frame/transaction-payment/src/types.rs b/frame/transaction-payment/src/types.rs index fff41ef6937f5..057364a153e45 100644 --- a/frame/transaction-payment/src/types.rs +++ b/frame/transaction-payment/src/types.rs @@ -24,7 +24,7 @@ use serde::{Deserialize, Serialize}; use sp_runtime::traits::{AtLeast32BitUnsigned, Zero}; use sp_std::prelude::*; -use frame_support::{dispatch::DispatchClass, weights::Weight}; +use frame_support::dispatch::DispatchClass; /// The base fee and adjusted weight and length fees constitute the _inclusion fee_. #[derive(Encode, Decode, Clone, Eq, PartialEq)] @@ -94,9 +94,15 @@ impl FeeDetails { #[derive(Eq, PartialEq, Encode, Decode, Default)] #[cfg_attr(feature = "std", derive(Debug, Serialize, Deserialize))] #[cfg_attr(feature = "std", serde(rename_all = "camelCase"))] -#[cfg_attr(feature = "std", serde(bound(serialize = "Balance: std::fmt::Display")))] -#[cfg_attr(feature = "std", serde(bound(deserialize = "Balance: std::str::FromStr")))] -pub struct RuntimeDispatchInfo { +#[cfg_attr( + feature = "std", + serde(bound(serialize = "Balance: std::fmt::Display, Weight: Serialize")) +)] +#[cfg_attr( + feature = "std", + serde(bound(deserialize = "Balance: std::str::FromStr, Weight: Deserialize<'de>")) +)] +pub struct RuntimeDispatchInfo { /// Weight of this dispatch. pub weight: Weight, /// Class of this dispatch. diff --git a/primitives/weights/src/lib.rs b/primitives/weights/src/lib.rs index e1ac7fcd4e892..954fea91e28dc 100644 --- a/primitives/weights/src/lib.rs +++ b/primitives/weights/src/lib.rs @@ -26,8 +26,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -extern crate self as sp_weights; - mod weight_v2; use codec::{CompactAs, Decode, Encode, MaxEncodedLen}; @@ -70,6 +68,8 @@ pub mod constants { MaxEncodedLen, TypeInfo, )] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "std", serde(transparent))] pub struct OldWeight(pub u64); /// The weight of database operations that the runtime can invoke. @@ -106,7 +106,7 @@ impl RuntimeDbWeight { /// coeff_integer * x^(degree) + coeff_frac * x^(degree) /// ``` /// -/// The `negative` value encodes whether the term is added or substracted from the +/// The `negative` value encodes whether the term is added or subtracted from the /// overall polynomial result. #[derive(Clone, Encode, Decode, TypeInfo)] pub struct WeightToFeeCoefficient { From 2ef9d08c35302eaa397e4f075376117e271fcc5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 7 Nov 2022 15:39:31 +0100 Subject: [PATCH 2/3] Fix tests --- frame/transaction-payment/src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/transaction-payment/src/types.rs b/frame/transaction-payment/src/types.rs index 057364a153e45..d1a480b64e116 100644 --- a/frame/transaction-payment/src/types.rs +++ b/frame/transaction-payment/src/types.rs @@ -137,6 +137,7 @@ mod serde_balance { #[cfg(test)] mod tests { use super::*; + use frame_support::weights::Weight; #[test] fn should_serialize_and_deserialize_properly_with_string() { From 8b3948a9417a40959daf1c4046cab469d2e6c21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 7 Nov 2022 23:38:50 +0100 Subject: [PATCH 3/3] Review comment --- frame/transaction-payment/rpc/src/lib.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/frame/transaction-payment/rpc/src/lib.rs b/frame/transaction-payment/rpc/src/lib.rs index 1c8dbd626680e..19007d37963ec 100644 --- a/frame/transaction-payment/rpc/src/lib.rs +++ b/frame/transaction-payment/rpc/src/lib.rs @@ -118,12 +118,18 @@ where )) } - if api + let api_version = api .api_version::>(&at) - .map_err(|e| map_err(e, "Transaction payment runtime api not present."))? - .unwrap_or(0) < - 2 - { + .map_err(|e| map_err(e, "Failed to get transaction payment runtime api version"))? + .ok_or_else(|| { + CallError::Custom(ErrorObject::owned( + Error::RuntimeError.into(), + "Transaction payment runtime api wasn't found in the runtime", + None::, + )) + })?; + + if api_version < 2 { #[allow(deprecated)] api.query_info_before_version_2(&at, uxt, encoded_len) .map_err(|e| map_err(e, "Unable to query dispatch info.").into())