diff --git a/core/src/config/default_extrinsic_params.rs b/core/src/config/default_extrinsic_params.rs index 4a0a8dd521..28cef1ec32 100644 --- a/core/src/config/default_extrinsic_params.rs +++ b/core/src/config/default_extrinsic_params.rs @@ -2,7 +2,9 @@ // This file is dual-licensed as Apache-2.0 or GPL-3.0. // see LICENSE for license details. -use super::Config; +use crate::config::transaction_extensions::CheckMortalityParams; + +use super::{Config, HashFor}; use super::{ExtrinsicParams, transaction_extensions}; /// The default [`super::ExtrinsicParams`] implementation understands common signed extensions @@ -26,8 +28,8 @@ pub type DefaultExtrinsicParams = transaction_extensions::AnyOf< /// [`DefaultExtrinsicParams`]. This may expose methods that aren't applicable to the current /// chain; such values will simply be ignored if so. pub struct DefaultExtrinsicParamsBuilder { - /// `None` means the tx will be immortal, else it's mortal for N blocks (if possible). - mortality: Option, + /// `None` means the tx will be immortal, else it's mortality is described. + mortality: transaction_extensions::CheckMortalityParams, /// `None` means the nonce will be automatically set. nonce: Option, /// `None` means we'll use the native token. @@ -39,7 +41,7 @@ pub struct DefaultExtrinsicParamsBuilder { impl Default for DefaultExtrinsicParamsBuilder { fn default() -> Self { Self { - mortality: None, + mortality: CheckMortalityParams::default(), tip: 0, tip_of: 0, tip_of_asset_id: None, @@ -55,10 +57,43 @@ impl DefaultExtrinsicParamsBuilder { Default::default() } + /// Make the transaction immortal, meaning it will never expire. This means that it could, in + /// theory, be pending for a long time and only be included many blocks into the future. + pub fn immortal(mut self) -> Self { + self.mortality = transaction_extensions::CheckMortalityParams::immortal(); + self + } + /// Make the transaction mortal, given a number of blocks it will be mortal for from /// the current block at the time of submission. + /// + /// # Warning + /// + /// This will ultimately return an error if used for creating extrinsic offline, because we need + /// additional information in order to set the mortality properly. + /// + /// When creating offline transactions, you must use [`Self::mortal_from_unchecked`] instead to set + /// the mortality. This provides all of the necessary information which we must otherwise be online + /// in order to obtain. pub fn mortal(mut self, for_n_blocks: u64) -> Self { - self.mortality = Some(for_n_blocks); + self.mortality = transaction_extensions::CheckMortalityParams::mortal(for_n_blocks); + self + } + + /// Configure a transaction that will be mortal for the number of blocks given, and from the + /// block details provided. Prefer to use [`Self::mortal()`] where possible, which prevents + /// the block number and hash from being misaligned. + pub fn mortal_from_unchecked( + mut self, + for_n_blocks: u64, + from_block_n: u64, + from_block_hash: HashFor, + ) -> Self { + self.mortality = transaction_extensions::CheckMortalityParams::mortal_from_unchecked( + for_n_blocks, + from_block_n, + from_block_hash, + ); self } @@ -88,11 +123,7 @@ impl DefaultExtrinsicParamsBuilder { /// Build the extrinsic parameters. pub fn build(self) -> as ExtrinsicParams>::Params { - let check_mortality_params = if let Some(for_n_blocks) = self.mortality { - transaction_extensions::CheckMortalityParams::mortal(for_n_blocks) - } else { - transaction_extensions::CheckMortalityParams::immortal() - }; + let check_mortality_params = self.mortality; let charge_asset_tx_params = if let Some(asset_id) = self.tip_of_asset_id { transaction_extensions::ChargeAssetTxPaymentParams::tip_of(self.tip, asset_id) diff --git a/core/src/config/transaction_extensions.rs b/core/src/config/transaction_extensions.rs index b82b35ce4d..e59986c601 100644 --- a/core/src/config/transaction_extensions.rs +++ b/core/src/config/transaction_extensions.rs @@ -295,6 +295,15 @@ impl ExtrinsicParams for CheckMortality { type Params = CheckMortalityParams; fn new(client: &ClientState, params: Self::Params) -> Result { + // If a user has explicitly configured the transaction to be mortal for n blocks, but we get + // to this stage and no injected information was able to turn this into MortalFromBlock{..}, + // then we hit an error as we are unable to construct a mortal transaction here. + if matches!(¶ms.0, CheckMortalityParamsInner::MortalForBlocks(_)) { + return Err(ExtrinsicParamsError::custom( + "CheckMortality: We cannot construct an offline extrinsic with only the number of blocks it is mortal for. Use mortal_from_unchecked instead.", + )); + } + Ok(CheckMortality { // if nothing has been explicitly configured, we will have a mortal transaction // valid for 32 blocks if block info is available. @@ -347,8 +356,15 @@ impl TransactionExtension for CheckMortality { pub struct CheckMortalityParams(CheckMortalityParamsInner); enum CheckMortalityParamsInner { + /// The transaction will be immortal. Immortal, + /// The transaction is mortal for N blocks. This must be "upgraded" into + /// [`CheckMortalityParamsInner::MortalFromBlock`] to ultimately work. MortalForBlocks(u64), + /// The transaction is mortal for N blocks, but if it cannot be "upgraded", + /// then it will be set to immortal instead. This is the default if unset. + MortalForBlocksOrImmortalIfNotPossible(u64), + /// The transaction is mortal and all of the relevant information is provided. MortalFromBlock { for_n_blocks: u64, from_block_n: u64, @@ -358,8 +374,8 @@ enum CheckMortalityParamsInner { impl Default for CheckMortalityParams { fn default() -> Self { - // default to being mortal for 32 blocks if possible: - CheckMortalityParams(CheckMortalityParamsInner::MortalForBlocks(32)) + // default to being mortal for 32 blocks if possible, else immortal: + CheckMortalityParams(CheckMortalityParamsInner::MortalForBlocksOrImmortalIfNotPossible(32)) } } @@ -392,7 +408,8 @@ impl CheckMortalityParams { impl Params for CheckMortalityParams { fn inject_block(&mut self, from_block_n: u64, from_block_hash: HashFor) { match &self.0 { - CheckMortalityParamsInner::MortalForBlocks(n) => { + CheckMortalityParamsInner::MortalForBlocks(n) + | CheckMortalityParamsInner::MortalForBlocksOrImmortalIfNotPossible(n) => { self.0 = CheckMortalityParamsInner::MortalFromBlock { for_n_blocks: *n, from_block_n, diff --git a/core/src/error.rs b/core/src/error.rs index 9a1742a558..9517b30856 100644 --- a/core/src/error.rs +++ b/core/src/error.rs @@ -208,29 +208,20 @@ pub enum ExtrinsicParamsError { UnknownTransactionExtension(String), /// Some custom error. #[error("Error constructing extrinsic parameters: {0}")] - Custom(Box), + Custom(Box), } -/// Anything implementing this trait can be used in [`ExtrinsicParamsError::Custom`]. -#[cfg(feature = "std")] -pub trait CustomError: std::error::Error + Send + Sync + 'static {} -#[cfg(feature = "std")] -impl CustomError for T {} - -/// Anything implementing this trait can be used in [`ExtrinsicParamsError::Custom`]. -#[cfg(not(feature = "std"))] -pub trait CustomError: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static {} -#[cfg(not(feature = "std"))] -impl CustomError for T {} +impl ExtrinsicParamsError { + /// Create a custom [`ExtrinsicParamsError`] from a string. + pub fn custom>(error: S) -> Self { + let error: String = error.into(); + let error: Box = Box::from(error); + ExtrinsicParamsError::Custom(error) + } +} impl From for ExtrinsicParamsError { fn from(value: core::convert::Infallible) -> Self { match value {} } } - -impl From> for ExtrinsicParamsError { - fn from(value: Box) -> Self { - ExtrinsicParamsError::Custom(value) - } -} diff --git a/core/src/utils/era.rs b/core/src/utils/era.rs index 75ae36a791..d86ef78a94 100644 --- a/core/src/utils/era.rs +++ b/core/src/utils/era.rs @@ -2,7 +2,13 @@ // This file is dual-licensed as Apache-2.0 or GPL-3.0. // see LICENSE for license details. -use scale_decode::DecodeAsType; +use alloc::{format, vec::Vec}; +use codec::{Decode, Encode}; +use scale_decode::{ + IntoVisitor, TypeResolver, Visitor, + ext::scale_type_resolver, + visitor::{TypeIdFor, types::Composite, types::Variant}, +}; use scale_encode::EncodeAsType; // Dev note: This and related bits taken from `sp_runtime::generic::Era` @@ -16,8 +22,6 @@ use scale_encode::EncodeAsType; Debug, serde::Serialize, serde::Deserialize, - DecodeAsType, - EncodeAsType, scale_info::TypeInfo, )] pub enum Era { @@ -105,3 +109,126 @@ impl codec::Decode for Era { } } } + +/// Define manually how to encode an Era given some type information. Here we +/// basically check that the type we're targeting is called "Era" and then codec::Encode. +impl EncodeAsType for Era { + fn encode_as_type_to( + &self, + type_id: R::TypeId, + types: &R, + out: &mut Vec, + ) -> Result<(), scale_encode::Error> { + // Visit the type to check that it is an Era. This is only a rough check. + let visitor = scale_type_resolver::visitor::new((), |_, _| false) + .visit_variant(|_, path, _variants| path.last() == Some("Era")); + + let is_era = types + .resolve_type(type_id.clone(), visitor) + .unwrap_or_default(); + if !is_era { + return Err(scale_encode::Error::custom_string(format!( + "Type {type_id:?} is not a valid Era type; expecting either Immortal or MortalX variant" + ))); + } + + // if the type looks valid then just scale encode our Era. + self.encode_to(out); + Ok(()) + } +} + +/// Define manually how to decode an Era given some type information. Here we check that the +/// variant we're decoding is one of the expected Era variants, and that the field is correct if so, +/// ensuring that this will fail if trying to decode something that isn't an Era. +pub struct EraVisitor(core::marker::PhantomData); + +impl IntoVisitor for Era { + type AnyVisitor = EraVisitor; + fn into_visitor() -> Self::AnyVisitor { + EraVisitor(core::marker::PhantomData) + } +} + +impl Visitor for EraVisitor { + type Value<'scale, 'resolver> = Era; + type Error = scale_decode::Error; + type TypeResolver = R; + + // Unwrap any newtype wrappers around the era, eg the CheckMortality extension (which actually + // has 2 fields, but scale_info seems to autoamtically ignore the PhantomData field). This + // allows us to decode directly from CheckMortality into Era. + fn visit_composite<'scale, 'resolver>( + self, + value: &mut Composite<'scale, 'resolver, Self::TypeResolver>, + _type_id: TypeIdFor, + ) -> Result, Self::Error> { + if value.remaining() != 1 { + return Err(scale_decode::Error::custom_string(format!( + "Expected any wrapper around Era to have exactly one field, but got {} fields", + value.remaining() + ))); + } + + value + .decode_item(self) + .expect("1 field expected; checked above.") + } + + fn visit_variant<'scale, 'resolver>( + self, + value: &mut Variant<'scale, 'resolver, Self::TypeResolver>, + _type_id: TypeIdFor, + ) -> Result, Self::Error> { + let variant = value.name(); + + // If the variant is immortal, we know the outcome. + if variant == "Immortal" { + return Ok(Era::Immortal); + } + + // Otherwise, we expect a variant Mortal1..Mortal255 where the number + // here is the first byte, and the second byte is conceptually a field of this variant. + // This weird encoding is because the Era is compressed to just 1 byte if immortal and + // just 2 bytes if mortal. + // + // Note: We _could_ just assume we'll have 2 bytes to work with and decode the era directly, + // but checking the variant names ensures that the thing we think is an Era actually _is_ + // one, based on the type info for it. + let first_byte = variant + .strip_prefix("Mortal") + .and_then(|s| s.parse::().ok()) + .ok_or_else(|| { + scale_decode::Error::custom_string(format!( + "Expected MortalX variant, but got {variant}" + )) + })?; + + // We need 1 field in the MortalN variant containing the second byte. + let mortal_fields = value.fields(); + if mortal_fields.remaining() != 1 { + return Err(scale_decode::Error::custom_string(format!( + "Expected Mortal{} to have one u8 field, but got {} fields", + first_byte, + mortal_fields.remaining() + ))); + } + + let second_byte = mortal_fields + .decode_item(u8::into_visitor()) + .expect("At least one field should exist; checked above.") + .map_err(|e| { + scale_decode::Error::custom_string(format!( + "Expected mortal variant field to be u8, but: {e}" + )) + })?; + + // Now that we have both bytes we can decode them into the era using + // the same logic as the codec::Decode impl does. + Era::decode(&mut &[first_byte, second_byte][..]).map_err(|e| { + scale_decode::Error::custom_string(format!( + "Failed to codec::Decode Era from Mortal bytes: {e}" + )) + }) + } +} diff --git a/testing/integration-tests/src/full_client/blocks.rs b/testing/integration-tests/src/full_client/blocks.rs index 76c2e221bd..c5a84c2a5d 100644 --- a/testing/integration-tests/src/full_client/blocks.rs +++ b/testing/integration-tests/src/full_client/blocks.rs @@ -261,47 +261,49 @@ async fn fetch_block_and_decode_extrinsic_details() { } } +/// A helper function to submit a transaction with some params and then get it back in a block, +/// so that we can test the decoding of it. +async fn submit_extrinsic_and_get_it_back( + api: &subxt::OnlineClient, + params: subxt::config::DefaultExtrinsicParamsBuilder, +) -> subxt::blocks::ExtrinsicDetails> { + let alice = dev::alice(); + let bob = dev::bob(); + + let tx = node_runtime::tx() + .balances() + .transfer_allow_death(bob.public_key().into(), 10_000); + + let signed_extrinsic = api + .tx() + .create_signed(&tx, &alice, params.build()) + .await + .unwrap(); + + let in_block = signed_extrinsic + .submit_and_watch() + .await + .unwrap() + .wait_for_finalized() + .await + .unwrap(); + + let block_hash = in_block.block_hash(); + let block = api.blocks().at(block_hash).await.unwrap(); + let extrinsics = block.extrinsics().await.unwrap(); + let extrinsic_details = extrinsics.iter().find(|e| e.is_signed()).unwrap(); + extrinsic_details +} + #[cfg(fullclient)] #[subxt_test] async fn decode_transaction_extensions_from_blocks() { let ctx = test_context().await; let api = ctx.client(); - let alice = dev::alice(); - let bob = dev::bob(); - macro_rules! submit_transfer_extrinsic_and_get_it_back { - ($tip:expr) => {{ - let tx = node_runtime::tx() - .balances() - .transfer_allow_death(bob.public_key().into(), 10_000); - - let signed_extrinsic = api - .tx() - .create_signed( - &tx, - &alice, - DefaultExtrinsicParamsBuilder::new().tip($tip).build(), - ) - .await - .unwrap(); - - let in_block = signed_extrinsic - .submit_and_watch() - .await - .unwrap() - .wait_for_finalized() - .await - .unwrap(); - - let block_hash = in_block.block_hash(); - let block = api.blocks().at(block_hash).await.unwrap(); - let extrinsics = block.extrinsics().await.unwrap(); - let extrinsic_details = extrinsics.iter().find(|e| e.is_signed()).unwrap(); - extrinsic_details - }}; - } - - let transaction1 = submit_transfer_extrinsic_and_get_it_back!(1234); + let transaction1 = + submit_extrinsic_and_get_it_back(&api, DefaultExtrinsicParamsBuilder::new().tip(1234)) + .await; let extensions1 = transaction1.transaction_extensions().unwrap(); let nonce1 = extensions1.nonce().unwrap(); @@ -313,7 +315,9 @@ async fn decode_transaction_extensions_from_blocks() { .unwrap() .tip(); - let transaction2 = submit_transfer_extrinsic_and_get_it_back!(5678); + let transaction2 = + submit_extrinsic_and_get_it_back(&api, DefaultExtrinsicParamsBuilder::new().tip(5678)) + .await; let extensions2 = transaction2.transaction_extensions().unwrap(); let nonce2 = extensions2.nonce().unwrap(); let nonce2_static = extensions2.find::().unwrap().unwrap(); @@ -368,13 +372,69 @@ async fn decode_transaction_extensions_from_blocks() { { assert_eq!(e.name(), *expected_name); } +} - // check that era decodes: - for extensions in [&extensions1, &extensions2] { - let era: Era = extensions +#[cfg(fullclient)] +#[subxt_test] +async fn decode_block_mortality() { + let ctx = test_context().await; + let api = ctx.client(); + + // Explicit Immortal: + { + let tx = + submit_extrinsic_and_get_it_back(&api, DefaultExtrinsicParamsBuilder::new().immortal()) + .await; + + let mortality = tx + .transaction_extensions() + .unwrap() .find::>() .unwrap() .unwrap(); - assert_eq!(era, Era::Immortal) + + assert_eq!(mortality, Era::Immortal); + } + + // Explicit Mortal: + for for_n_blocks in [4, 16, 128] { + let tx = submit_extrinsic_and_get_it_back( + &api, + DefaultExtrinsicParamsBuilder::new().mortal(for_n_blocks), + ) + .await; + + let mortality = tx + .transaction_extensions() + .unwrap() + .find::>() + .unwrap() + .unwrap(); + + assert!(matches!(mortality, Era::Mortal { + period, + phase: _, // depends on current block so don't test it. + } if period == for_n_blocks)); + } + + // Implicitly, transactions should be mortal: + { + let tx = + submit_extrinsic_and_get_it_back(&api, DefaultExtrinsicParamsBuilder::default()).await; + + let mortality = tx + .transaction_extensions() + .unwrap() + .find::>() + .unwrap() + .unwrap(); + + assert!(matches!( + mortality, + Era::Mortal { + period: 32, + phase: _, // depends on current block so don't test it. + } + )); } }