Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

payment_queryInfo: Make it work with WeightV2 #12633

Merged
merged 3 commits into from
Nov 8, 2022
Merged

payment_queryInfo: Make it work with WeightV2 #12633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2867b1b
`payment_queryInfo`: Make it work with `WeighV2`
bkchr Nov 7, 2022
2ef9d08
Fix tests
bkchr Nov 7, 2022
8b3948a
Review comment
bkchr Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions frame/transaction-payment/rpc/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
2 changes: 2 additions & 0 deletions frame/transaction-payment/rpc/runtime-api/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -25,4 +26,5 @@ std = [
"pallet-transaction-payment/std",
"sp-api/std",
"sp-runtime/std",
"sp-weights/std",
]
4 changes: 4 additions & 0 deletions frame/transaction-payment/rpc/runtime-api/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Balance> where
Balance: Codec + MaybeDisplay,
{
#[changed_in(2)]
fn query_info(uxt: Block::Extrinsic, len: u32) -> RuntimeDispatchInfo<Balance, sp_weights::OldWeight>;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
fn query_info(uxt: Block::Extrinsic, len: u32) -> RuntimeDispatchInfo<Balance>;
fn query_fee_details(uxt: Block::Extrinsic, len: u32) -> FeeDetails<Balance>;
}

#[api_version(2)]
pub trait TransactionPaymentCallApi<Balance, Call>
where
Balance: Codec + MaybeDisplay,
Expand Down
47 changes: 38 additions & 9 deletions frame/transaction-payment/rpc/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -82,8 +82,10 @@ impl From<Error> for i32 {
}

impl<C, Block, Balance>
TransactionPaymentApiServer<<Block as BlockT>::Hash, RuntimeDispatchInfo<Balance>>
for TransactionPayment<C, Block>
TransactionPaymentApiServer<
<Block as BlockT>::Hash,
RuntimeDispatchInfo<Balance, sp_weights::OldWeight>,
> for TransactionPayment<C, Block>
where
Block: BlockT,
C: ProvideRuntimeApi<Block> + HeaderBackend<Block> + Send + Sync + 'static,
Expand All @@ -94,7 +96,7 @@ where
&self,
encoded_xt: Bytes,
at: Option<Block::Hash>,
) -> RpcResult<RuntimeDispatchInfo<Balance>> {
) -> RpcResult<RuntimeDispatchInfo<Balance, sp_weights::OldWeight>> {
let api = self.client.runtime_api();
let at = BlockId::hash(at.unwrap_or_else(|| self.client.info().best_hash));

Expand All @@ -107,14 +109,41 @@ 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()
})
}

let api_version = api
.api_version::<dyn TransactionPaymentRuntimeApi<Block, Balance>>(&at)
.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::<String>,
))
})?;

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())
} 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()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this really should be using the new weight and not the old one, as it's already in V2.

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The RPC was always returning the old weight and we should stick to this. Otherwise this pr is useless, as downstream would still fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

will we ever return the new weight from RPC then?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess since RPCs don't have versioning like state_call, it makes sense to never update them.

Copy link
Member Author

Choose a reason for hiding this comment

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

state_call also doesn't have any real versioning. We can achieve versioning by using the runtime api versions.

will we ever return the new weight from RPC then?

We want to deprecate these RPC calls anyway.

class: res.class,
partial_fee: res.partial_fee,
})
}
}

fn query_fee_details(
Expand Down
15 changes: 11 additions & 4 deletions frame/transaction-payment/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -94,9 +94,15 @@ impl<Balance: AtLeast32BitUnsigned + Copy> FeeDetails<Balance> {
#[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<Balance> {
#[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<Balance, Weight = frame_support::weights::Weight> {
/// Weight of this dispatch.
pub weight: Weight,
/// Class of this dispatch.
Expand Down Expand Up @@ -131,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() {
Expand Down
6 changes: 3 additions & 3 deletions primitives/weights/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<Balance> {
Expand Down