From b757399a9124f6a80315761d80e39de1fd6c6bf6 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 13:24:08 +0000 Subject: [PATCH 1/9] First pass making matching on signed exts more general and handlng SkipCheckifFeeless --- subxt/examples/setup_config_custom.rs | 5 +- .../examples/setup_config_signed_extension.rs | 21 +- subxt/src/blocks/extrinsic_types.rs | 57 ++- subxt/src/config/default_extrinsic_params.rs | 5 - subxt/src/config/extrinsic_params.rs | 47 +-- subxt/src/config/signed_extensions.rs | 396 ++++++------------ subxt/src/tx/tx_client.rs | 3 +- 7 files changed, 201 insertions(+), 333 deletions(-) diff --git a/subxt/examples/setup_config_custom.rs b/subxt/examples/setup_config_custom.rs index 6768787600..b1da57913c 100644 --- a/subxt/examples/setup_config_custom.rs +++ b/subxt/examples/setup_config_custom.rs @@ -1,6 +1,6 @@ use codec::Encode; use subxt::client::OfflineClientT; -use subxt::config::{Config, ExtrinsicParams, ExtrinsicParamsEncoder}; +use subxt::config::{Config, ExtrinsicParams, ExtrinsicParamsEncoder, ExtrinsicParamsError}; use subxt_signer::sr25519::dev; #[subxt::subxt( @@ -66,14 +66,13 @@ impl CustomExtrinsicParamsBuilder { // Describe how to fetch and then encode the params: impl ExtrinsicParams for CustomExtrinsicParams { type OtherParams = CustomExtrinsicParamsBuilder; - type Error = std::convert::Infallible; // Gather together all of the params we will need to encode: fn new>( _nonce: u64, client: Client, other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(Self { genesis_hash: client.genesis_hash(), tip: other_params.tip, diff --git a/subxt/examples/setup_config_signed_extension.rs b/subxt/examples/setup_config_signed_extension.rs index ab985a8838..bec9b2d2c6 100644 --- a/subxt/examples/setup_config_signed_extension.rs +++ b/subxt/examples/setup_config_signed_extension.rs @@ -1,9 +1,11 @@ use codec::Encode; use scale_encode::EncodeAsType; +use scale_info::PortableRegistry; use subxt::client::OfflineClientT; use subxt::config::signed_extensions; use subxt::config::{ Config, DefaultExtrinsicParamsBuilder, ExtrinsicParams, ExtrinsicParamsEncoder, + ExtrinsicParamsError, }; use subxt_signer::sr25519::dev; @@ -34,10 +36,6 @@ impl Config for CustomConfig { signed_extensions::CheckMortality, signed_extensions::ChargeAssetTxPayment, signed_extensions::ChargeTransactionPayment, - signed_extensions::SkipCheckIfFeeless< - Self, - signed_extensions::ChargeAssetTxPayment, - >, // And add a new one of our own: CustomSignedExtension, ), @@ -51,20 +49,25 @@ pub struct CustomSignedExtension; // Give the extension a name; this allows `AnyOf` to look it // up in the chain metadata in order to know when and if to use it. impl signed_extensions::SignedExtension for CustomSignedExtension { - const NAME: &'static str = "CustomSignedExtension"; type Decoded = (); + fn matches( + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, + ) -> Result { + Ok(identifier == "CustomSignedExtension") + } } // Gather together any params we need for our signed extension, here none. impl ExtrinsicParams for CustomSignedExtension { type OtherParams = (); - type Error = std::convert::Infallible; fn new>( _nonce: u64, _client: Client, _other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CustomSignedExtension) } } @@ -87,8 +90,8 @@ impl ExtrinsicParamsEncoder for CustomSignedExtension { pub fn custom( params: DefaultExtrinsicParamsBuilder, ) -> <::ExtrinsicParams as ExtrinsicParams>::OtherParams { - let (a, b, c, d, e, f, g, h) = params.build(); - (a, b, c, d, e, f, g, h, ()) + let (a, b, c, d, e, f, g) = params.build(); + (a, b, c, d, e, f, g, ()) } #[tokio::main] diff --git a/subxt/src/blocks/extrinsic_types.rs b/subxt/src/blocks/extrinsic_types.rs index 070c84e6da..3f73803b31 100644 --- a/subxt/src/blocks/extrinsic_types.rs +++ b/subxt/src/blocks/extrinsic_types.rs @@ -13,7 +13,7 @@ use crate::{ }; use crate::config::signed_extensions::{ - ChargeAssetTxPayment, ChargeTransactionPayment, CheckNonce, SkipCheckIfFeeless, + ChargeAssetTxPayment, ChargeTransactionPayment, CheckNonce, }; use crate::config::SignedExtension; use crate::dynamic::DecodedValue; @@ -660,24 +660,22 @@ impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> { }) } - fn find_by_name(&self, name: &str) -> Option> { - let signed_extension = self - .iter() - .find_map(|e| e.ok().filter(|e| e.name() == name))?; - Some(signed_extension) - } - /// Searches through all signed extensions to find a specific one. /// If the Signed Extension is not found `Ok(None)` is returned. /// If the Signed Extension is found but decoding failed `Err(_)` is returned. pub fn find>(&self) -> Result, Error> { - self.find_by_name(S::NAME) - .map(|s| { - s.as_signed_extra::().map(|e| { - e.expect("signed extra name is correct, because it was found before; qed.") - }) - }) - .transpose() + for ext in self.iter() { + let Ok(ext) = ext else { continue }; + match ext.as_signed_extension::() { + // We found a match; return it: + Ok(Some(e)) => return Ok(Some(e)), + // No error, but no match either; next! + Ok(None) => continue, + // Error? return it + Err(e) => return Err(e), + } + } + Ok(None) } /// The tip of an extrinsic, extracted from the ChargeTransactionPayment or ChargeAssetTxPayment @@ -696,20 +694,13 @@ impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> { .flatten() .map(|e| e.tip()) }) - .or_else(|| { - self.find::>>() - .ok() - .flatten() - .map(|skip_check| skip_check.inner_signed_extension().tip()) - }) } /// The nonce of the account that submitted the extrinsic, extracted from the CheckNonce signed extension. /// /// Returns `None` if `nonce` was not found or decoding failed. pub fn nonce(&self) -> Option { - let nonce = self.find::().ok()??.0; - Some(nonce) + self.find::().ok()? } } @@ -744,20 +735,20 @@ impl<'a, T: Config> ExtrinsicSignedExtension<'a, T> { self.as_type() } - /// Decodes the `extra` bytes of this Signed Extension into a static type. - fn as_type(&self) -> Result { - let value = E::decode_as_type(&mut &self.bytes[..], self.ty_id, self.metadata.types())?; - Ok(value) - } - - /// Decodes the `extra` bytes of this Signed Extension into its associated `Decoded` type. - /// Returns `Ok(None)` if the identitfier of this Signed Extension object does not line up with the `NAME` constant of the provided Signed Extension type. - pub fn as_signed_extra>(&self) -> Result, Error> { - if self.identifier != S::NAME { + /// Decodes the bytes of this Signed Extension into its associated `Decoded` type. + /// Returns `Ok(None)` if the data we have doesn't match the Signed Extension we're asking to + /// decode with. + pub fn as_signed_extension>(&self) -> Result, Error> { + if !S::matches(self.identifier, self.ty_id, self.metadata.types())? { return Ok(None); } self.as_type::().map(Some) } + + fn as_type(&self) -> Result { + let value = E::decode_as_type(&mut &self.bytes[..], self.ty_id, self.metadata.types())?; + Ok(value) + } } #[cfg(test)] diff --git a/subxt/src/config/default_extrinsic_params.rs b/subxt/src/config/default_extrinsic_params.rs index df78ffa34c..880591e7f0 100644 --- a/subxt/src/config/default_extrinsic_params.rs +++ b/subxt/src/config/default_extrinsic_params.rs @@ -17,7 +17,6 @@ pub type DefaultExtrinsicParams = signed_extensions::AnyOf< signed_extensions::CheckMortality, signed_extensions::ChargeAssetTxPayment, signed_extensions::ChargeTransactionPayment, - signed_extensions::SkipCheckIfFeeless>, ), >; @@ -132,9 +131,6 @@ impl DefaultExtrinsicParamsBuilder { let charge_transaction_params = signed_extensions::ChargeTransactionPaymentParams::tip(self.tip); - let skip_check_params = - signed_extensions::SkipCheckIfFeelessParams::from(charge_asset_tx_params.clone()); - ( (), (), @@ -143,7 +139,6 @@ impl DefaultExtrinsicParamsBuilder { check_mortality_params, charge_asset_tx_params, charge_transaction_params, - skip_check_params, ) } } diff --git a/subxt/src/config/extrinsic_params.rs b/subxt/src/config/extrinsic_params.rs index f3f0cc332e..42f3b620af 100644 --- a/subxt/src/config/extrinsic_params.rs +++ b/subxt/src/config/extrinsic_params.rs @@ -10,39 +10,41 @@ use crate::{client::OfflineClientT, Config}; use core::fmt::Debug; -/// An error that can be emitted when trying to construct -/// an instance of [`ExtrinsicParams`]. +/// An error that can be emitted when trying to construct an instance of [`ExtrinsicParams`], +/// encode data from the instance, or match on signed extensions. #[derive(thiserror::Error, Debug)] #[non_exhaustive] pub enum ExtrinsicParamsError { - /// A signed extension was encountered that we don't know about. - #[error("Error constructing extrinsic parameters: Unknown signed extension '{0}'")] + /// Cannot find a type id in the metadata. The context provides some additional + /// information about the source of the error (eg the signed extension name). + #[error("Cannot find type id '{type_id} in the metadata (context: {context})")] + MissingTypeId { + /// Type ID. + type_id: u32, + /// Some arbitrary context to help narrow the source of the error. + context: &'static str, + }, + /// A signed extension in use on some chain was not provided. + #[error("The chain expects a signed extension with the name {0}, but we did not provide one")] UnknownSignedExtension(String), - /// Cannot find the type id of a signed extension in the metadata. - #[error("Cannot find extension's '{0}' type id '{1} in the metadata")] - MissingTypeId(String, u32), - /// User provided a different signed extension than the one expected. - #[error("Provided a different signed extension for '{0}', the metadata expect '{1}'")] - ExpectedAnotherExtension(String, String), - /// The inner type of a signed extension is not present in the metadata. - #[error("The inner type of the signed extension '{0}' is not present in the metadata")] - MissingInnerSignedExtension(String), - /// The inner type of the signed extension is not named. - #[error("The signed extension's '{0}' type id '{1}' does not have a name in the metadata")] - ExpectedNamedTypeId(String, u32), - /// Some custom error.s + /// Some custom error. #[error("Error constructing extrinsic parameters: {0}")] - Custom(CustomError), + Custom(CustomExtrinsicParamsError), } /// A custom error. -type CustomError = Box; +pub type CustomExtrinsicParamsError = Box; impl From for ExtrinsicParamsError { fn from(value: std::convert::Infallible) -> Self { match value {} } } +impl From for ExtrinsicParamsError { + fn from(value: CustomExtrinsicParamsError) -> Self { + ExtrinsicParamsError::Custom(value) + } +} /// This trait allows you to configure the "signed extra" and /// "additional" parameters that are a part of the transaction payload @@ -53,15 +55,12 @@ pub trait ExtrinsicParams: ExtrinsicParamsEncoder + Sized + 'static { /// help construct your [`ExtrinsicParams`] object. type OtherParams; - /// The type of error returned from [`ExtrinsicParams::new()`]. - type Error: Into; - - /// Construct a new instance of our [`ExtrinsicParams`] + /// Construct a new instance of our [`ExtrinsicParams`]. fn new>( nonce: u64, client: Client, other_params: Self::OtherParams, - ) -> Result; + ) -> Result; } /// This trait is expected to be implemented for any [`ExtrinsicParams`], and diff --git a/subxt/src/config/signed_extensions.rs b/subxt/src/config/signed_extensions.rs index 24f536bd72..ee48829551 100644 --- a/subxt/src/config/signed_extensions.rs +++ b/subxt/src/config/signed_extensions.rs @@ -9,12 +9,12 @@ use super::extrinsic_params::{ExtrinsicParams, ExtrinsicParamsEncoder, ExtrinsicParamsError}; use crate::utils::Era; -use crate::{client::OfflineClientT, Config, Metadata}; +use crate::{client::OfflineClientT, Config}; use codec::{Compact, Encode}; use core::fmt::Debug; use scale_decode::DecodeAsType; use scale_encode::EncodeAsType; -use std::marker::PhantomData; +use scale_info::PortableRegistry; use std::collections::HashMap; @@ -22,14 +22,23 @@ use std::collections::HashMap; /// same as [`ExtrinsicParams`] in describing how to encode the extra and /// additional data. pub trait SignedExtension: ExtrinsicParams { - /// The name of the signed extension. This is used to associate it - /// with the signed extensions that the node is making use of. - const NAME: &'static str; - /// The type representing the `extra` bytes of a signed extension. /// Decoding from this type should be symmetrical to the respective /// `ExtrinsicParamsEncoder::encode_extra_to()` implementation of this signed extension. type Decoded: DecodeAsType; + + /// This should return true if the signed extension matches the details given. + /// Often, this will involve just checking that the identifier given matches that of the + /// extension in question. + /// + /// The first match that returns true will be the entry that this signed extension + /// is used to encode values for. This takes `&mut self`, allowing the extension to + /// cache values if it likes when it finds the type it'll be encoding for. + fn matches( + identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result; } /// The [`CheckSpecVersion`] signed extension. @@ -38,13 +47,12 @@ pub struct CheckSpecVersion(u32); impl ExtrinsicParams for CheckSpecVersion { type OtherParams = (); - type Error = std::convert::Infallible; fn new>( _nonce: u64, client: Client, _other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CheckSpecVersion(client.runtime_version().spec_version)) } } @@ -56,8 +64,14 @@ impl ExtrinsicParamsEncoder for CheckSpecVersion { } impl SignedExtension for CheckSpecVersion { - const NAME: &'static str = "CheckSpecVersion"; type Decoded = (); + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "CheckSpecVersion") + } } /// The [`CheckNonce`] signed extension. @@ -66,13 +80,12 @@ pub struct CheckNonce(Compact); impl ExtrinsicParams for CheckNonce { type OtherParams = (); - type Error = std::convert::Infallible; fn new>( nonce: u64, _client: Client, _other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CheckNonce(Compact(nonce))) } } @@ -84,8 +97,14 @@ impl ExtrinsicParamsEncoder for CheckNonce { } impl SignedExtension for CheckNonce { - const NAME: &'static str = "CheckNonce"; - type Decoded = Compact; + type Decoded = u64; + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "CheckNonce") + } } /// The [`CheckTxVersion`] signed extension. @@ -94,13 +113,12 @@ pub struct CheckTxVersion(u32); impl ExtrinsicParams for CheckTxVersion { type OtherParams = (); - type Error = std::convert::Infallible; fn new>( _nonce: u64, client: Client, _other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CheckTxVersion(client.runtime_version().transaction_version)) } } @@ -112,8 +130,14 @@ impl ExtrinsicParamsEncoder for CheckTxVersion { } impl SignedExtension for CheckTxVersion { - const NAME: &'static str = "CheckTxVersion"; type Decoded = (); + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "CheckTxVersion") + } } /// The [`CheckGenesis`] signed extension. @@ -130,13 +154,12 @@ impl std::fmt::Debug for CheckGenesis { impl ExtrinsicParams for CheckGenesis { type OtherParams = (); - type Error = std::convert::Infallible; fn new>( _nonce: u64, client: Client, _other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CheckGenesis(client.genesis_hash())) } } @@ -148,8 +171,14 @@ impl ExtrinsicParamsEncoder for CheckGenesis { } impl SignedExtension for CheckGenesis { - const NAME: &'static str = "CheckGenesis"; type Decoded = (); + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "CheckGenesis") + } } /// The [`CheckMortality`] signed extension. @@ -208,13 +237,12 @@ impl std::fmt::Debug for CheckMortality { impl ExtrinsicParams for CheckMortality { type OtherParams = CheckMortalityParams; - type Error = std::convert::Infallible; fn new>( _nonce: u64, client: Client, other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(CheckMortality { era: other_params.era, checkpoint: other_params.checkpoint.unwrap_or(client.genesis_hash()), @@ -227,19 +255,24 @@ impl ExtrinsicParamsEncoder for CheckMortality { self.era.encode_to(v); } fn encode_additional_to(&self, v: &mut Vec) { - self.checkpoint.encode_to(v) + self.checkpoint.encode_to(v); } } impl SignedExtension for CheckMortality { - const NAME: &'static str = "CheckMortality"; type Decoded = Era; + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "CheckMortality") + } } /// The [`ChargeAssetTxPayment`] signed extension. -#[derive(Clone, Debug, DecodeAsType, EncodeAsType)] +#[derive(Clone, Debug, DecodeAsType)] #[decode_as_type(trait_bounds = "T::AssetId: DecodeAsType")] -#[encode_as_type(trait_bounds = "T::AssetId: EncodeAsType")] pub struct ChargeAssetTxPayment { tip: Compact, asset_id: Option, @@ -309,13 +342,12 @@ impl ChargeAssetTxPaymentParams { impl ExtrinsicParams for ChargeAssetTxPayment { type OtherParams = ChargeAssetTxPaymentParams; - type Error = std::convert::Infallible; fn new>( _nonce: u64, _client: Client, other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(ChargeAssetTxPayment { tip: Compact(other_params.tip), asset_id: other_params.asset_id, @@ -330,12 +362,18 @@ impl ExtrinsicParamsEncoder for ChargeAssetTxPayment { } impl SignedExtension for ChargeAssetTxPayment { - const NAME: &'static str = "ChargeAssetTxPayment"; type Decoded = Self; + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "ChargeAssetTxPayment") + } } /// The [`ChargeTransactionPayment`] signed extension. -#[derive(Clone, Debug, DecodeAsType, EncodeAsType)] +#[derive(Clone, Debug, DecodeAsType)] pub struct ChargeTransactionPayment { tip: Compact, } @@ -366,13 +404,12 @@ impl ChargeTransactionPaymentParams { impl ExtrinsicParams for ChargeTransactionPayment { type OtherParams = ChargeTransactionPaymentParams; - type Error = std::convert::Infallible; fn new>( _nonce: u64, _client: Client, other_params: Self::OtherParams, - ) -> Result { + ) -> Result { Ok(ChargeTransactionPayment { tip: Compact(other_params.tip), }) @@ -386,212 +423,13 @@ impl ExtrinsicParamsEncoder for ChargeTransactionPayment { } impl SignedExtension for ChargeTransactionPayment { - const NAME: &'static str = "ChargeTransactionPayment"; - type Decoded = Self; -} - -/// Information needed to encode the [`SkipCheckIfFeeless`] signed extension. -#[derive(Debug)] -struct SkipCheckIfFeelessEncodingData { - metadata: Metadata, - type_id: u32, -} - -impl SkipCheckIfFeelessEncodingData { - /// Construct [`SkipCheckIfFeelessEncodingData`]. - fn new( - metadata: Metadata, - extension: &str, - inner_extension: &str, - ) -> Result { - let skip_check_type_id = metadata - .extrinsic() - .signed_extensions() - .iter() - .find_map(|ext| { - if ext.identifier() == extension { - Some(ext.extra_ty()) - } else { - None - } - }); - let Some(skip_check_type_id) = skip_check_type_id else { - return Err(ExtrinsicParamsError::UnknownSignedExtension( - inner_extension.to_owned(), - )); - }; - - // Ensure that the `SkipCheckIfFeeless` type has the same inner signed extension as provided. - let Some(skip_check_ty) = metadata.types().resolve(skip_check_type_id) else { - return Err(ExtrinsicParamsError::MissingTypeId( - inner_extension.to_owned(), - skip_check_type_id, - )); - }; - - // The substrate's `SkipCheckIfFeeless` contains 2 types: the inner signed extension and a phantom data. - // Phantom data does not have a type associated, so we need to find the inner signed extension. - let Some(inner_type_id) = skip_check_ty - .type_params - .iter() - .find_map(|param| param.ty.map(|ty| ty.id)) - else { - return Err(ExtrinsicParamsError::MissingInnerSignedExtension( - inner_extension.to_owned(), - )); - }; - - // Get the inner type of the `SkipCheckIfFeeless` extension to check if the naming matches the provided parameters. - let Some(inner_extension_ty) = metadata.types().resolve(inner_type_id) else { - return Err(ExtrinsicParamsError::MissingTypeId( - inner_extension.to_owned(), - inner_type_id, - )); - }; - - let Some(inner_extension_name) = inner_extension_ty.path.segments.last() else { - return Err(ExtrinsicParamsError::ExpectedNamedTypeId( - inner_extension.to_owned(), - inner_type_id, - )); - }; - - if inner_extension_name != inner_extension { - return Err(ExtrinsicParamsError::ExpectedAnotherExtension( - inner_extension.to_owned(), - inner_extension_name.to_owned(), - )); - } - - Ok(SkipCheckIfFeelessEncodingData { - metadata, - type_id: inner_type_id, - }) - } -} - -/// The [`SkipCheckIfFeeless`] signed extension. -#[derive(Debug, DecodeAsType, EncodeAsType)] -#[decode_as_type(trait_bounds = "S: DecodeAsType")] -#[encode_as_type(trait_bounds = "S: EncodeAsType")] -pub struct SkipCheckIfFeeless -where - T: Config, - S: SignedExtension + DecodeAsType + EncodeAsType, -{ - inner: S, - // Dev note: This is `Option` because `#[derive(DecodeAsType)]` requires the - // `Default` bound on skipped parameters. - // This field is populated when the [`SkipCheckIfFeeless`] is constructed from - // [`ExtrinsicParams`] (ie, when subxt submits extrinsics). However, it is not - // populated when decoding signed extensions from the node. - #[decode_as_type(skip)] - #[encode_as_type(skip)] - encoding_data: Option, - #[decode_as_type(skip)] - #[encode_as_type(skip)] - _phantom: PhantomData, -} - -impl SkipCheckIfFeeless -where - T: Config, - S: SignedExtension + DecodeAsType + EncodeAsType, -{ - /// The inner signed extension. - pub fn inner_signed_extension(&self) -> &S { - &self.inner - } -} - -impl ExtrinsicParams for SkipCheckIfFeeless -where - T: Config, - S: SignedExtension + DecodeAsType + EncodeAsType, - >::OtherParams: Default, -{ - type OtherParams = SkipCheckIfFeelessParams; - type Error = ExtrinsicParamsError; - - fn new>( - nonce: u64, - client: Client, - other_params: Self::OtherParams, - ) -> Result { - let other_params = other_params.0.unwrap_or_default(); - - let metadata = client.metadata(); - let encoding_data = SkipCheckIfFeelessEncodingData::new(metadata, Self::NAME, S::NAME)?; - let inner_extension = S::new(nonce, client, other_params).map_err(Into::into)?; - - Ok(SkipCheckIfFeeless { - inner: inner_extension, - encoding_data: Some(encoding_data), - _phantom: PhantomData, - }) - } -} - -impl ExtrinsicParamsEncoder for SkipCheckIfFeeless -where - T: Config, - S: SignedExtension + DecodeAsType + EncodeAsType, -{ - fn encode_extra_to(&self, v: &mut Vec) { - if let Some(encoding_data) = &self.encoding_data { - let _ = self.inner.encode_as_type_to( - encoding_data.type_id, - encoding_data.metadata.types(), - v, - ); - } - } -} - -impl SignedExtension for SkipCheckIfFeeless -where - T: Config, - S: SignedExtension + DecodeAsType + EncodeAsType, - >::OtherParams: Default, -{ - const NAME: &'static str = "SkipCheckIfFeeless"; type Decoded = Self; -} - -/// Parameters to configure the [`SkipCheckIfFeeless`] signed extension. -pub struct SkipCheckIfFeelessParams(Option<>::OtherParams>) -where - T: Config, - S: SignedExtension; - -impl std::fmt::Debug for SkipCheckIfFeelessParams -where - T: Config, - S: SignedExtension, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("SkipCheckIfFeelessParams").finish() - } -} - -impl> Default for SkipCheckIfFeelessParams -where - T: Config, - S: SignedExtension, -{ - fn default() -> Self { - SkipCheckIfFeelessParams(None) - } -} - -impl SkipCheckIfFeelessParams -where - T: Config, - S: SignedExtension, -{ - /// Skip the check if the transaction is feeless. - pub fn from(extrinsic_params: >::OtherParams) -> Self { - SkipCheckIfFeelessParams(Some(extrinsic_params)) + fn matches( + _identifier: &str, + type_id: u32, + types: &PortableRegistry, + ) -> Result { + type_id_matches_ext_name(type_id, types, "ChargeTransactionPayment") } } @@ -614,37 +452,45 @@ macro_rules! impl_tuples { $($ident: SignedExtension,)+ { type OtherParams = ($($ident::OtherParams,)+); - type Error = ExtrinsicParamsError; fn new>( nonce: u64, client: Client, other_params: Self::OtherParams, - ) -> Result { - // First, push encoders to map as we are given them: - let mut map = HashMap::new(); + ) -> Result { + let metadata = client.metadata(); + let types = metadata.types(); + + // For each signed extension in the tuple, find the matching index in the metadata, if + // there is one, and add it to a map with that index as the key. + let mut exts_by_index = HashMap::new(); $({ - let e: Box - = Box::new($ident::new(nonce, client.clone(), other_params.$index).map_err(Into::into)?); - map.insert($ident::NAME, e); + for (idx, e) in metadata.extrinsic().signed_extensions().iter().enumerate() { + // Skip over any exts that have a match already: + if exts_by_index.contains_key(&idx) { + continue + } + // Break and record as soon as we find a match: + if $ident::matches(e.identifier(), e.extra_ty(), types)? { + let ext = $ident::new(nonce, client.clone(), other_params.$index)?; + let boxed_ext: Box = Box::new(ext); + exts_by_index.insert(idx, boxed_ext); + break + } + } })+ - // Next, based on metadata, push to vec in the order the node needs: + // Next, turn these into an ordered vec, erroring if we haven't matched on any exts yet. let mut params = Vec::new(); - let metadata = client.metadata(); - let types = metadata.types(); - for ext in metadata.extrinsic().signed_extensions() { - if let Some(ext) = map.remove(ext.identifier()) { - params.push(ext) - } else { - if is_type_empty(ext.extra_ty(), types) && is_type_empty(ext.additional_ty(), types) { - // If we don't know about the signed extension, _but_ it appears to require zero bytes - // to encode its extra and additional data, then we can safely ignore it as it makes - // no difference either way. - continue; + for (idx, e) in metadata.extrinsic().signed_extensions().iter().enumerate() { + let Some(ext) = exts_by_index.remove(&idx) else { + if is_type_empty(e.extra_ty(), types) { + continue + } else { + return Err(ExtrinsicParamsError::UnknownSignedExtension(e.identifier().to_owned())); } - return Err(ExtrinsicParamsError::UnknownSignedExtension(ext.identifier().to_owned())); - } + }; + params.push(ext); } Ok(AnyOf { @@ -719,3 +565,39 @@ fn is_type_empty(type_id: u32, types: &scale_info::PortableRegistry) -> bool { | TypeDef::Primitive(_) => false, } } + +/// Resolve a type ID and check that its path equals the extension name provided. +/// As an exception; if the name of the extension is the transparent "SkipCheckIfFeeless" +/// signed extension, then we look at the inner type of that to determine whether it's a match. +fn type_id_matches_ext_name( + type_id: u32, + types: &PortableRegistry, + ext_name: &'static str, +) -> Result { + let Some(ty) = types.resolve(type_id) else { + return Err(ExtrinsicParamsError::MissingTypeId { + type_id, + context: ext_name, + }); + }; + let Some(name) = ty.path.segments.last() else { + return Ok(false); + }; + if name == "SkipCheckIfFeeless" { + // SkipCheckIfFeeless is a transparent wrapper that can be applied around any signed extension. + // It should have 2 generic types: the inner signed extension and a phantom data type. Phantom data does + // not have a type associated, so we find the type that does to find the inner signed extension. + // If this doesn't pan out, don't error, just don't match. + let Some(inner_type_id) = ty + .type_params + .iter() + .find_map(|param| param.ty.map(|ty| ty.id)) + else { + return Ok(false); + }; + + type_id_matches_ext_name(inner_type_id, types, ext_name) + } else { + Ok(name == ext_name) + } +} diff --git a/subxt/src/tx/tx_client.rs b/subxt/src/tx/tx_client.rs index 5b5df91106..6bf6c9ab57 100644 --- a/subxt/src/tx/tx_client.rs +++ b/subxt/src/tx/tx_client.rs @@ -124,8 +124,7 @@ impl> TxClient { account_nonce, self.client.clone(), other_params, - ) - .map_err(Into::into)?; + )?; // Return these details, ready to construct a signed extrinsic from. Ok(PartialExtrinsic { From 6c6e8049f8652663ee005f76d0413ae8fcc1e7f5 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 13:37:45 +0000 Subject: [PATCH 2/9] remove unneeded derives (only exts we can decode into are handled by the user) --- subxt/src/config/signed_extensions.rs | 41 ++------------------------- 1 file changed, 3 insertions(+), 38 deletions(-) diff --git a/subxt/src/config/signed_extensions.rs b/subxt/src/config/signed_extensions.rs index ee48829551..456688488a 100644 --- a/subxt/src/config/signed_extensions.rs +++ b/subxt/src/config/signed_extensions.rs @@ -12,8 +12,8 @@ use crate::utils::Era; use crate::{client::OfflineClientT, Config}; use codec::{Compact, Encode}; use core::fmt::Debug; +use derivative::Derivative; use scale_decode::DecodeAsType; -use scale_encode::EncodeAsType; use scale_info::PortableRegistry; use std::collections::HashMap; @@ -42,7 +42,6 @@ pub trait SignedExtension: ExtrinsicParams { } /// The [`CheckSpecVersion`] signed extension. -#[derive(Clone, Debug, EncodeAsType, DecodeAsType)] pub struct CheckSpecVersion(u32); impl ExtrinsicParams for CheckSpecVersion { @@ -75,7 +74,6 @@ impl SignedExtension for CheckSpecVersion { } /// The [`CheckNonce`] signed extension. -#[derive(Clone, Debug, EncodeAsType, DecodeAsType)] pub struct CheckNonce(Compact); impl ExtrinsicParams for CheckNonce { @@ -108,7 +106,6 @@ impl SignedExtension for CheckNonce { } /// The [`CheckTxVersion`] signed extension. -#[derive(Clone, Debug, EncodeAsType, DecodeAsType)] pub struct CheckTxVersion(u32); impl ExtrinsicParams for CheckTxVersion { @@ -141,17 +138,8 @@ impl SignedExtension for CheckTxVersion { } /// The [`CheckGenesis`] signed extension. -#[derive(Clone, EncodeAsType, DecodeAsType)] -#[decode_as_type(trait_bounds = "T::Hash: DecodeAsType")] -#[encode_as_type(trait_bounds = "T::Hash: EncodeAsType")] pub struct CheckGenesis(T::Hash); -impl std::fmt::Debug for CheckGenesis { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("CheckGenesis").field(&self.0).finish() - } -} - impl ExtrinsicParams for CheckGenesis { type OtherParams = (); @@ -182,16 +170,12 @@ impl SignedExtension for CheckGenesis { } /// The [`CheckMortality`] signed extension. -#[derive(Clone, EncodeAsType, DecodeAsType)] -#[decode_as_type(trait_bounds = "T::Hash: DecodeAsType")] -#[encode_as_type(trait_bounds = "T::Hash: EncodeAsType")] pub struct CheckMortality { era: Era, checkpoint: T::Hash, } /// Parameters to configure the [`CheckMortality`] signed extension. -#[derive(Clone, Debug)] pub struct CheckMortalityParams { era: Era, checkpoint: Option, @@ -226,15 +210,6 @@ impl CheckMortalityParams { } } -impl std::fmt::Debug for CheckMortality { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("CheckMortality") - .field("era", &self.era) - .field("checkpoint", &self.checkpoint) - .finish() - } -} - impl ExtrinsicParams for CheckMortality { type OtherParams = CheckMortalityParams; @@ -271,7 +246,8 @@ impl SignedExtension for CheckMortality { } /// The [`ChargeAssetTxPayment`] signed extension. -#[derive(Clone, Debug, DecodeAsType)] +#[derive(Derivative, DecodeAsType)] +#[derivative(Clone(bound = "T::AssetId: Clone"), Debug(bound = "T::AssetId: Debug"))] #[decode_as_type(trait_bounds = "T::AssetId: DecodeAsType")] pub struct ChargeAssetTxPayment { tip: Compact, @@ -291,22 +267,11 @@ impl ChargeAssetTxPayment { } /// Parameters to configure the [`ChargeAssetTxPayment`] signed extension. -#[derive(Debug)] pub struct ChargeAssetTxPaymentParams { tip: u128, asset_id: Option, } -// Dev note: `#[derive(Clone)]` implies `T: Clone` instead of `T::AssetId: Clone`. -impl Clone for ChargeAssetTxPaymentParams { - fn clone(&self) -> Self { - Self { - tip: self.tip, - asset_id: self.asset_id.clone(), - } - } -} - impl Default for ChargeAssetTxPaymentParams { fn default() -> Self { ChargeAssetTxPaymentParams { From f502acac27787d912997a99ce89cc7e5f82c09e6 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 13:59:04 +0000 Subject: [PATCH 3/9] No SkipCheckIfFeeless in integration tests either --- .../integration-tests/src/full_client/blocks/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/testing/integration-tests/src/full_client/blocks/mod.rs b/testing/integration-tests/src/full_client/blocks/mod.rs index 8485f1f4cd..021eb66c98 100644 --- a/testing/integration-tests/src/full_client/blocks/mod.rs +++ b/testing/integration-tests/src/full_client/blocks/mod.rs @@ -6,7 +6,7 @@ use crate::{test_context, utils::node_runtime}; use codec::{Compact, Encode}; use futures::StreamExt; use subxt::config::signed_extensions::{ - ChargeAssetTxPayment, CheckMortality, CheckNonce, SkipCheckIfFeeless, + ChargeAssetTxPayment, CheckMortality, CheckNonce, }; use subxt::config::DefaultExtrinsicParamsBuilder; use subxt::config::SubstrateConfig; @@ -280,25 +280,23 @@ async fn decode_signed_extensions_from_blocks() { let extensions1 = transaction1.signed_extensions().unwrap(); let nonce1 = extensions1.nonce().unwrap(); - let nonce1_static = extensions1.find::().unwrap().unwrap().0; + let nonce1_static = extensions1.find::().unwrap().unwrap(); let tip1 = extensions1.tip().unwrap(); let tip1_static: u128 = extensions1 - .find::>>() + .find::>() .unwrap() .unwrap() - .inner_signed_extension() .tip(); let transaction2 = submit_transfer_extrinsic_and_get_it_back!(5678); let extensions2 = transaction2.signed_extensions().unwrap(); let nonce2 = extensions2.nonce().unwrap(); - let nonce2_static = extensions2.find::().unwrap().unwrap().0; + let nonce2_static = extensions2.find::().unwrap().unwrap(); let tip2 = extensions2.tip().unwrap(); let tip2_static: u128 = extensions2 - .find::>>() + .find::>() .unwrap() .unwrap() - .inner_signed_extension() .tip(); assert_eq!(nonce1, 0); From 32733f3ec7c90db5441974add8b070762a462ae6 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 17:33:32 +0000 Subject: [PATCH 4/9] Cargo fmt --- subxt/src/blocks/extrinsic_types.rs | 2 +- .../integration-tests/src/full_client/blocks/mod.rs | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/subxt/src/blocks/extrinsic_types.rs b/subxt/src/blocks/extrinsic_types.rs index 3f73803b31..95840fc8fc 100644 --- a/subxt/src/blocks/extrinsic_types.rs +++ b/subxt/src/blocks/extrinsic_types.rs @@ -672,7 +672,7 @@ impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> { // No error, but no match either; next! Ok(None) => continue, // Error? return it - Err(e) => return Err(e), + Err(e) => return Err(e.into()), } } Ok(None) diff --git a/testing/integration-tests/src/full_client/blocks/mod.rs b/testing/integration-tests/src/full_client/blocks/mod.rs index 021eb66c98..8ec992672a 100644 --- a/testing/integration-tests/src/full_client/blocks/mod.rs +++ b/testing/integration-tests/src/full_client/blocks/mod.rs @@ -5,9 +5,7 @@ use crate::{test_context, utils::node_runtime}; use codec::{Compact, Encode}; use futures::StreamExt; -use subxt::config::signed_extensions::{ - ChargeAssetTxPayment, CheckMortality, CheckNonce, -}; +use subxt::config::signed_extensions::{ChargeAssetTxPayment, CheckMortality, CheckNonce}; use subxt::config::DefaultExtrinsicParamsBuilder; use subxt::config::SubstrateConfig; use subxt::utils::Era; @@ -279,9 +277,9 @@ async fn decode_signed_extensions_from_blocks() { let transaction1 = submit_transfer_extrinsic_and_get_it_back!(1234); let extensions1 = transaction1.signed_extensions().unwrap(); - let nonce1 = extensions1.nonce().unwrap(); + let nonce1 = extensions1.nonce().unwrap().unwrap(); let nonce1_static = extensions1.find::().unwrap().unwrap(); - let tip1 = extensions1.tip().unwrap(); + let tip1 = extensions1.tip().unwrap().unwrap(); let tip1_static: u128 = extensions1 .find::>() .unwrap() @@ -316,7 +314,7 @@ async fn decode_signed_extensions_from_blocks() { "CheckMortality", "CheckNonce", "CheckWeight", - "SkipCheckIfFeeless", + "ChargeAssetTxPayment", ]; assert_eq!(extensions1.iter().count(), expected_signed_extensions.len()); From f94bec37509dd510b5b86e216edeeeedaed2750f Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 17:36:58 +0000 Subject: [PATCH 5/9] Remove SkipCheckIfFeeless specific logic --- subxt/src/config/signed_extensions.rs | 96 ++++++------------- .../src/full_client/blocks/mod.rs | 4 +- 2 files changed, 32 insertions(+), 68 deletions(-) diff --git a/subxt/src/config/signed_extensions.rs b/subxt/src/config/signed_extensions.rs index 456688488a..1111b34e6a 100644 --- a/subxt/src/config/signed_extensions.rs +++ b/subxt/src/config/signed_extensions.rs @@ -36,8 +36,8 @@ pub trait SignedExtension: ExtrinsicParams { /// cache values if it likes when it finds the type it'll be encoding for. fn matches( identifier: &str, - type_id: u32, - types: &PortableRegistry, + _type_id: u32, + _types: &PortableRegistry, ) -> Result; } @@ -65,11 +65,11 @@ impl ExtrinsicParamsEncoder for CheckSpecVersion { impl SignedExtension for CheckSpecVersion { type Decoded = (); fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "CheckSpecVersion") + Ok(identifier == "CheckSpecVersion") } } @@ -97,11 +97,11 @@ impl ExtrinsicParamsEncoder for CheckNonce { impl SignedExtension for CheckNonce { type Decoded = u64; fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "CheckNonce") + Ok(identifier == "CheckNonce") } } @@ -129,11 +129,11 @@ impl ExtrinsicParamsEncoder for CheckTxVersion { impl SignedExtension for CheckTxVersion { type Decoded = (); fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "CheckTxVersion") + Ok(identifier == "CheckTxVersion") } } @@ -161,11 +161,11 @@ impl ExtrinsicParamsEncoder for CheckGenesis { impl SignedExtension for CheckGenesis { type Decoded = (); fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "CheckGenesis") + Ok(identifier == "CheckGenesis") } } @@ -237,11 +237,11 @@ impl ExtrinsicParamsEncoder for CheckMortality { impl SignedExtension for CheckMortality { type Decoded = Era; fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "CheckMortality") + Ok(identifier == "CheckMortality") } } @@ -329,11 +329,11 @@ impl ExtrinsicParamsEncoder for ChargeAssetTxPayment { impl SignedExtension for ChargeAssetTxPayment { type Decoded = Self; fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "ChargeAssetTxPayment") + Ok(identifier == "ChargeAssetTxPayment") } } @@ -390,11 +390,11 @@ impl ExtrinsicParamsEncoder for ChargeTransactionPayment { impl SignedExtension for ChargeTransactionPayment { type Decoded = Self; fn matches( - _identifier: &str, - type_id: u32, - types: &PortableRegistry, + identifier: &str, + _type_id: u32, + _types: &PortableRegistry, ) -> Result { - type_id_matches_ext_name(type_id, types, "ChargeTransactionPayment") + Ok(identifier == "ChargeTransactionPayment") } } @@ -530,39 +530,3 @@ fn is_type_empty(type_id: u32, types: &scale_info::PortableRegistry) -> bool { | TypeDef::Primitive(_) => false, } } - -/// Resolve a type ID and check that its path equals the extension name provided. -/// As an exception; if the name of the extension is the transparent "SkipCheckIfFeeless" -/// signed extension, then we look at the inner type of that to determine whether it's a match. -fn type_id_matches_ext_name( - type_id: u32, - types: &PortableRegistry, - ext_name: &'static str, -) -> Result { - let Some(ty) = types.resolve(type_id) else { - return Err(ExtrinsicParamsError::MissingTypeId { - type_id, - context: ext_name, - }); - }; - let Some(name) = ty.path.segments.last() else { - return Ok(false); - }; - if name == "SkipCheckIfFeeless" { - // SkipCheckIfFeeless is a transparent wrapper that can be applied around any signed extension. - // It should have 2 generic types: the inner signed extension and a phantom data type. Phantom data does - // not have a type associated, so we find the type that does to find the inner signed extension. - // If this doesn't pan out, don't error, just don't match. - let Some(inner_type_id) = ty - .type_params - .iter() - .find_map(|param| param.ty.map(|ty| ty.id)) - else { - return Ok(false); - }; - - type_id_matches_ext_name(inner_type_id, types, ext_name) - } else { - Ok(name == ext_name) - } -} diff --git a/testing/integration-tests/src/full_client/blocks/mod.rs b/testing/integration-tests/src/full_client/blocks/mod.rs index 8ec992672a..efba9f0a14 100644 --- a/testing/integration-tests/src/full_client/blocks/mod.rs +++ b/testing/integration-tests/src/full_client/blocks/mod.rs @@ -277,9 +277,9 @@ async fn decode_signed_extensions_from_blocks() { let transaction1 = submit_transfer_extrinsic_and_get_it_back!(1234); let extensions1 = transaction1.signed_extensions().unwrap(); - let nonce1 = extensions1.nonce().unwrap().unwrap(); + let nonce1 = extensions1.nonce().unwrap(); let nonce1_static = extensions1.find::().unwrap().unwrap(); - let tip1 = extensions1.tip().unwrap().unwrap(); + let tip1 = extensions1.tip().unwrap(); let tip1_static: u128 = extensions1 .find::>() .unwrap() From d859d644c89216f9973591a21010192c6eb8a185 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 17:41:16 +0000 Subject: [PATCH 6/9] clippy --- subxt/src/blocks/extrinsic_types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/subxt/src/blocks/extrinsic_types.rs b/subxt/src/blocks/extrinsic_types.rs index 95840fc8fc..3f73803b31 100644 --- a/subxt/src/blocks/extrinsic_types.rs +++ b/subxt/src/blocks/extrinsic_types.rs @@ -672,7 +672,7 @@ impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> { // No error, but no match either; next! Ok(None) => continue, // Error? return it - Err(e) => return Err(e.into()), + Err(e) => return Err(e), } } Ok(None) From 4c0507d4a5d9f1f24718ca602aef817486f986e5 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 22 Nov 2023 17:59:09 +0000 Subject: [PATCH 7/9] matches to just return bool, not result --- .../examples/setup_config_signed_extension.rs | 8 +-- subxt/src/blocks/extrinsic_types.rs | 2 +- subxt/src/config/signed_extensions.rs | 64 +++++-------------- 3 files changed, 19 insertions(+), 55 deletions(-) diff --git a/subxt/examples/setup_config_signed_extension.rs b/subxt/examples/setup_config_signed_extension.rs index bec9b2d2c6..94836c4485 100644 --- a/subxt/examples/setup_config_signed_extension.rs +++ b/subxt/examples/setup_config_signed_extension.rs @@ -50,12 +50,8 @@ pub struct CustomSignedExtension; // up in the chain metadata in order to know when and if to use it. impl signed_extensions::SignedExtension for CustomSignedExtension { type Decoded = (); - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CustomSignedExtension") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CustomSignedExtension" } } diff --git a/subxt/src/blocks/extrinsic_types.rs b/subxt/src/blocks/extrinsic_types.rs index 3f73803b31..884d5d69b0 100644 --- a/subxt/src/blocks/extrinsic_types.rs +++ b/subxt/src/blocks/extrinsic_types.rs @@ -739,7 +739,7 @@ impl<'a, T: Config> ExtrinsicSignedExtension<'a, T> { /// Returns `Ok(None)` if the data we have doesn't match the Signed Extension we're asking to /// decode with. pub fn as_signed_extension>(&self) -> Result, Error> { - if !S::matches(self.identifier, self.ty_id, self.metadata.types())? { + if !S::matches(self.identifier, self.ty_id, self.metadata.types()) { return Ok(None); } self.as_type::().map(Some) diff --git a/subxt/src/config/signed_extensions.rs b/subxt/src/config/signed_extensions.rs index 1111b34e6a..eebdb04aea 100644 --- a/subxt/src/config/signed_extensions.rs +++ b/subxt/src/config/signed_extensions.rs @@ -34,11 +34,7 @@ pub trait SignedExtension: ExtrinsicParams { /// The first match that returns true will be the entry that this signed extension /// is used to encode values for. This takes `&mut self`, allowing the extension to /// cache values if it likes when it finds the type it'll be encoding for. - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result; + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool; } /// The [`CheckSpecVersion`] signed extension. @@ -64,12 +60,8 @@ impl ExtrinsicParamsEncoder for CheckSpecVersion { impl SignedExtension for CheckSpecVersion { type Decoded = (); - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CheckSpecVersion") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CheckSpecVersion" } } @@ -96,12 +88,8 @@ impl ExtrinsicParamsEncoder for CheckNonce { impl SignedExtension for CheckNonce { type Decoded = u64; - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CheckNonce") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CheckNonce" } } @@ -128,12 +116,8 @@ impl ExtrinsicParamsEncoder for CheckTxVersion { impl SignedExtension for CheckTxVersion { type Decoded = (); - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CheckTxVersion") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CheckTxVersion" } } @@ -160,12 +144,8 @@ impl ExtrinsicParamsEncoder for CheckGenesis { impl SignedExtension for CheckGenesis { type Decoded = (); - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CheckGenesis") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CheckGenesis" } } @@ -236,12 +216,8 @@ impl ExtrinsicParamsEncoder for CheckMortality { impl SignedExtension for CheckMortality { type Decoded = Era; - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "CheckMortality") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "CheckMortality" } } @@ -328,12 +304,8 @@ impl ExtrinsicParamsEncoder for ChargeAssetTxPayment { impl SignedExtension for ChargeAssetTxPayment { type Decoded = Self; - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "ChargeAssetTxPayment") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "ChargeAssetTxPayment" } } @@ -389,12 +361,8 @@ impl ExtrinsicParamsEncoder for ChargeTransactionPayment { impl SignedExtension for ChargeTransactionPayment { type Decoded = Self; - fn matches( - identifier: &str, - _type_id: u32, - _types: &PortableRegistry, - ) -> Result { - Ok(identifier == "ChargeTransactionPayment") + fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool { + identifier == "ChargeTransactionPayment" } } @@ -436,7 +404,7 @@ macro_rules! impl_tuples { continue } // Break and record as soon as we find a match: - if $ident::matches(e.identifier(), e.extra_ty(), types)? { + if $ident::matches(e.identifier(), e.extra_ty(), types) { let ext = $ident::new(nonce, client.clone(), other_params.$index)?; let boxed_ext: Box = Box::new(ext); exts_by_index.insert(idx, boxed_ext); From 434dc0842902a0f3c8b56fd3223da2c4bb2a8e9f Mon Sep 17 00:00:00 2001 From: James Wilson Date: Thu, 23 Nov 2023 09:58:53 +0000 Subject: [PATCH 8/9] remove now-invalid comment --- subxt/src/config/signed_extensions.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/subxt/src/config/signed_extensions.rs b/subxt/src/config/signed_extensions.rs index eebdb04aea..f0864cee61 100644 --- a/subxt/src/config/signed_extensions.rs +++ b/subxt/src/config/signed_extensions.rs @@ -30,10 +30,6 @@ pub trait SignedExtension: ExtrinsicParams { /// This should return true if the signed extension matches the details given. /// Often, this will involve just checking that the identifier given matches that of the /// extension in question. - /// - /// The first match that returns true will be the entry that this signed extension - /// is used to encode values for. This takes `&mut self`, allowing the extension to - /// cache values if it likes when it finds the type it'll be encoding for. fn matches(identifier: &str, _type_id: u32, _types: &PortableRegistry) -> bool; } From e1b2a4ad44ff3019c003ee615a525dd058164b14 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Thu, 23 Nov 2023 10:37:49 +0000 Subject: [PATCH 9/9] return error from find if .iter() errors --- subxt/src/blocks/extrinsic_types.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/subxt/src/blocks/extrinsic_types.rs b/subxt/src/blocks/extrinsic_types.rs index 884d5d69b0..92f70d565c 100644 --- a/subxt/src/blocks/extrinsic_types.rs +++ b/subxt/src/blocks/extrinsic_types.rs @@ -665,7 +665,9 @@ impl<'a, T: Config> ExtrinsicSignedExtensions<'a, T> { /// If the Signed Extension is found but decoding failed `Err(_)` is returned. pub fn find>(&self) -> Result, Error> { for ext in self.iter() { - let Ok(ext) = ext else { continue }; + // If we encounter an error while iterating, we won't get any more results + // back, so just return that error as we won't find the signed ext anyway. + let ext = ext?; match ext.as_signed_extension::() { // We found a match; return it: Ok(Some(e)) => return Ok(Some(e)),