From 8a2f6616a126fd57f3706970bfeba7518f3b463e Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Tue, 19 Mar 2024 11:13:50 -0400 Subject: [PATCH 1/2] one remaining `Verifier` -> `ConstraintChecker` --- tuxedo-core/src/types.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuxedo-core/src/types.rs b/tuxedo-core/src/types.rs index 87079952..1f193705 100644 --- a/tuxedo-core/src/types.rs +++ b/tuxedo-core/src/types.rs @@ -164,7 +164,7 @@ pub enum UtxoError { } /// The Result of dispatching a UTXO transaction. -pub type DispatchResult = Result<(), UtxoError>; +pub type DispatchResult = Result<(), UtxoError>; /// An opaque piece of Transaction output data. This is how the data appears at the Runtime level. After /// the verifier is checked, strongly typed data will be extracted and passed to the constraint checker. From 9a18761586134bf7f99d34cb9f3e0b1cdc5cef39 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Wed, 20 Mar 2024 14:07:39 -0400 Subject: [PATCH 2/2] Cleaner separation of `Verifier` / `Constraint Cheker` duties (#188) --- tuxedo-core/aggregator/src/lib.rs | 113 +++++------ tuxedo-core/src/constraint_checker.rs | 177 +++++++++++++----- tuxedo-core/src/executive.rs | 30 +-- tuxedo-core/src/genesis.rs | 2 +- tuxedo-core/src/inherents.rs | 165 ++++++++-------- tuxedo-core/src/lib.rs | 1 + tuxedo-core/src/types.rs | 2 +- tuxedo-core/src/verifier.rs | 22 +++ tuxedo-core/src/verifier/htlc.rs | 6 + tuxedo-core/src/verifier/multi_signature.rs | 7 + tuxedo-core/src/verifier/simple_signature.rs | 10 + tuxedo-parachain-core/src/validate_block.rs | 4 +- tuxedo-template-runtime/src/genesis.rs | 15 +- tuxedo-template-runtime/src/lib.rs | 11 +- wardrobe/kitties/src/lib.rs | 2 +- wardrobe/money/src/lib.rs | 2 +- wardrobe/parachain/src/lib.rs | 41 ++-- wardrobe/parachain/src/tests.rs | 32 ++-- wardrobe/timestamp/src/lib.rs | 47 ++--- .../timestamp/src/update_timestamp_tests.rs | 42 ++--- 20 files changed, 409 insertions(+), 322 deletions(-) diff --git a/tuxedo-core/aggregator/src/lib.rs b/tuxedo-core/aggregator/src/lib.rs index 5d7eaa8b..c785f988 100644 --- a/tuxedo-core/aggregator/src/lib.rs +++ b/tuxedo-core/aggregator/src/lib.rs @@ -101,11 +101,19 @@ pub fn tuxedo_verifier(_: TokenStream, body: TokenStream) -> TokenStream { let mut redeemer_type_name = outer_type.to_string(); redeemer_type_name.push_str("Redeemer"); let redeemer_type = Ident::new(&redeemer_type_name, outer_type.span()); + let type_for_new_unspendable = inner_types + .clone() + .next() + .expect("At least one verifier variant expected."); // TODO there must be a better way to do this, right? let inner_types2 = inner_types.clone(); let variants2 = variants.clone(); let variants3 = variants.clone(); + let variant_for_new_unspendable = variants + .clone() + .next() + .expect("At least one verifier variant expected."); let as_variants = variants.clone().map(|v| { let s = format!("as_{}", v); @@ -160,6 +168,14 @@ pub fn tuxedo_verifier(_: TokenStream, body: TokenStream) -> TokenStream { )* } } + + // The aggregation macro assumes that the first variant is able to produce a new unspendable instance. + // In the future this could be made nicer (but maybe not worth the complexity) by allowing an additional + // annotation to the one that can be used as unspendable eg `#[unspendable]` + // If this ever becomes a challenge just add an explicit `Unspendable` variant first. + fn new_unspendable() -> Option { + #type_for_new_unspendable::new_unspendable().map(|inner| Self::#variant_for_new_unspendable(inner)) + } } }; output.into() @@ -173,9 +189,8 @@ pub fn tuxedo_verifier(_: TokenStream, body: TokenStream) -> TokenStream { /// just like this original enum. however, the contained values in the error enum are of the corresponding types /// for the inner constraint checker. #[proc_macro_attribute] -pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> TokenStream { +pub fn tuxedo_constraint_checker(_attrs: TokenStream, body: TokenStream) -> TokenStream { let ast = parse_macro_input!(body as ItemEnum); - let verifier = parse_macro_input!(attrs as Ident); let original_code = ast.clone(); let outer_type = ast.ident; @@ -204,23 +219,16 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token error_type_name.push_str("Error"); let error_type = Ident::new(&error_type_name, outer_type.span()); - let mut inherent_hooks_name = outer_type.to_string(); - inherent_hooks_name.push_str("InherentHooks"); - let inherent_hooks = Ident::new(&inherent_hooks_name, outer_type.span()); - let vis = ast.vis; // TODO there must be a better way to do this, right? let inner_types2 = inner_types.clone(); let inner_types3 = inner_types.clone(); let inner_types4 = inner_types.clone(); - let inner_types6 = inner_types.clone(); - let inner_types7 = inner_types.clone(); let variants2 = variants.clone(); let variants3 = variants.clone(); let variants4 = variants.clone(); let variants5 = variants.clone(); - let variants6 = variants.clone(); let output = quote! { // Preserve the original enum, and write the From impls @@ -235,27 +243,39 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token #[derive(Debug)] #vis enum #error_type { #( - #variants(<#inner_types as tuxedo_core::ConstraintChecker<#verifier>>::Error), + #variants(<#inner_types as tuxedo_core::ConstraintChecker>::Error), )* } - /// This type is generated by the `#[tuxedo_constraint_checker]` macro. - /// It is a combined set of inherent hooks for the inherent hooks of each individual checker. - /// - /// This type is accessible downstream as `::InherentHooks` - #[derive(Debug, scale_info::TypeInfo)] - #vis enum #inherent_hooks { - #( - #variants2(<#inner_types2 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks), - )* - } + impl tuxedo_core::ConstraintChecker for #outer_type { + type Error = #error_type; - impl tuxedo_core::inherents::InherentInternal<#verifier, #outer_type> for #inherent_hooks { + fn check ( + &self, + inputs: &[tuxedo_core::dynamic_typing::DynamicallyTypedData], + peeks: &[tuxedo_core::dynamic_typing::DynamicallyTypedData], + outputs: &[tuxedo_core::dynamic_typing::DynamicallyTypedData], + ) -> Result { + match self { + #( + Self::#variants5(inner) => inner.check(inputs, peeks, outputs).map_err(|e| Self::Error::#variants5(e)), + )* + } + } + + fn is_inherent(&self) -> bool { + match self { + #( + Self::#variants2(inner) => inner.is_inherent(), + )* + } - fn create_inherents( + } + + fn create_inherents( authoring_inherent_data: &InherentData, - previous_inherents: Vec<(tuxedo_core::types::Transaction<#verifier, #outer_type>, sp_core::H256)>, - ) -> Vec> { + previous_inherents: Vec<(tuxedo_core::types::Transaction, sp_core::H256)>, + ) -> Vec> { let mut all_inherents = Vec::new(); @@ -272,7 +292,7 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token }) .collect(); - let inherents = <#inner_types3 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks::create_inherents(authoring_inherent_data, previous_inherents) + let inherents = <#inner_types3 as tuxedo_core::ConstraintChecker>::create_inherents(authoring_inherent_data, previous_inherents) .iter() .map(|tx| tx.transform::<#outer_type>()) .collect::>(); @@ -284,13 +304,13 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token all_inherents } - fn check_inherents( + fn check_inherents( importing_inherent_data: &sp_inherents::InherentData, - inherents: Vec>, + inherents: Vec>, result: &mut sp_inherents::CheckInherentsResult, ) { #( - let relevant_inherents: Vec> = inherents + let relevant_inherents: Vec> = inherents .iter() .filter_map(|tx| { match tx.checker { @@ -300,7 +320,7 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token }) .collect(); - <#inner_types4 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks::check_inherents(importing_inherent_data, relevant_inherents, result); + <#inner_types4 as tuxedo_core::ConstraintChecker>::check_inherents(importing_inherent_data, relevant_inherents, result); // According to https://paritytech.github.io/polkadot-sdk/master/sp_inherents/struct.CheckInherentsResult.html // "When a fatal error occurs, all other errors are removed and the implementation needs to abort checking inherents." @@ -311,12 +331,12 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token } #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { - let mut all_transactions: Vec> = Vec::new(); + fn genesis_transactions() -> Vec> { + let mut all_transactions: Vec> = Vec::new(); #( let transactions = - <<#inner_types6 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks as tuxedo_core::inherents::InherentInternal<#verifier, #inner_types6>>::genesis_transactions(); + <#inner_types2 as tuxedo_core::ConstraintChecker>::genesis_transactions(); all_transactions.extend( transactions .into_iter() @@ -329,35 +349,6 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token } } - - impl tuxedo_core::ConstraintChecker<#verifier> for #outer_type { - type Error = #error_type; - - type InherentHooks = #inherent_hooks; - - fn check ( - &self, - inputs: &[tuxedo_core::types::Output<#verifier>], - peeks: &[tuxedo_core::types::Output<#verifier>], - outputs: &[tuxedo_core::types::Output<#verifier>], - ) -> Result { - match self { - #( - Self::#variants5(inner) => inner.check(inputs, peeks, outputs).map_err(|e| Self::Error::#variants5(e)), - )* - } - } - - fn is_inherent(&self) -> bool { - match self { - #( - Self::#variants6(inner) => <#inner_types7 as tuxedo_core::ConstraintChecker<#verifier>>::is_inherent(inner), - )* - } - - } - - } }; output.into() diff --git a/tuxedo-core/src/constraint_checker.rs b/tuxedo-core/src/constraint_checker.rs index 498160bf..674d53c1 100644 --- a/tuxedo-core/src/constraint_checker.rs +++ b/tuxedo-core/src/constraint_checker.rs @@ -1,15 +1,40 @@ //! A constraint checker is a piece of logic that determines whether a transaction as a whole is valid //! and should be committed. Most tuxedo pieces will provide one or more constraint checkers. -//! Constraint Checkers do not typically calculate the correct final state, but rather determine whether the +//! Constraint Checkers do not calculate the correct final state, but rather determine whether the //! proposed final state (as specified by the output set) meets the necessary constraints. - +//! +//! Constraint Checkers can be used to codify the laws of a monetary system, a chemistry or physics simulation, +//! NFT kitties, public elections and much more. +//! +//! The primary way for developers to write a constraint checker is with the `SimpleConstraintChecker` +//! trait. It provides a method called `check` which determines whether the relationship between the inputs +//! and outputs (and peeks) is valid. For example making sure no extra money was created, or making sure the chemical +//! reaction balances. +//! +//! ## Inherents +//! +//! If you need to tap in to [Substrate's inherent system](https://docs.substrate.io/learn/transaction-types/#inherent-transactions) +//! you may choose to also implement the `InherentHooks` trait for the same type that implements `SimpleConstraintChecker`. +//! When installing a constraint checker in your runtime, make sure to wrap it with the `InherentAdapter` type. +//! See the `inherents` module for more details. +//! +//! ## Constraint Checker Internals +//! +//! One of Tuxedo's killer features is its ability to aggregating pieces recursively. +//! To achieve this we have to consider that many intermediate layers in the aggregation tree +//! will have multiple inherent types. For this reason, we provide a much more flexible interface +//! that the aggregation macro can use called `ConstraintChecker`. Do not implement `ConstraintChecker` +//! directly. + +use sp_core::H256; +use sp_inherents::{CheckInherentsResult, InherentData}; use sp_std::{fmt::Debug, vec::Vec}; -use crate::{dynamic_typing::DynamicallyTypedData, inherents::InherentInternal, types::Output}; +use crate::{dynamic_typing::DynamicallyTypedData, types::Transaction, Verifier}; use parity_scale_codec::{Decode, Encode}; use sp_runtime::transaction_validity::TransactionPriority; -/// A simplified constraint checker that a transaction can choose to call. +/// A particular constraint checker that a transaction can choose to be checked by. /// Checks whether the input and output data from a transaction meets the codified constraints. /// /// Additional transient information may be passed to the constraint checker by including it in the fields @@ -19,7 +44,7 @@ pub trait SimpleConstraintChecker: Debug + Encode + Decode + Clone { /// The error type that this constraint checker may return type Error: Debug; - /// The actual check validation logic + /// The on chain logic that makes the final check for whether a transaction is valid. fn check( &self, input_data: &[DynamicallyTypedData], @@ -28,81 +53,113 @@ pub trait SimpleConstraintChecker: Debug + Encode + Decode + Clone { ) -> Result; } -/// A single constraint checker that a transaction can choose to call. Checks whether the input -/// and output data from a transaction meets the codified constraints. +/// The raw and fully powerful `ConstraintChecker` interface used by the +/// Tuxedo Executive. /// -/// This full ConstraintChecker should only be implemented if the piece logic cannot be expressed with -/// the SimpleConstraintChecker. For example, if you need to enforce the verifier is a particular type -/// or contains a certain value. Another reason would be if you need to implement an inherent. +/// You should never manually manually implement this trait. +/// If you are: +/// * Working on a simple non-inherent constraint checker -> Use the `SimpleConstraintChecker` trait instead +/// and rely on its blanket implementation. +/// * Working on an inherent constraint checker -> Implement `SimpleConstraintChecker` and `InherentHooks` and use the +/// `InherentAdapter` wrapper type. +/// * Considering an aggregate constraint checker that is part inherent, part not -> let the macro handle it for you. /// -/// Additional transient information may be passed to the constraint checker by including it in the fields -/// of the constraint checker struct itself. Information passed in this way does not come from state, nor -/// is it stored in state. -pub trait ConstraintChecker: Debug + Encode + Decode + Clone { +/// If you are trying to implement some complex inherent logic that requires the interaction of +/// multiple inherents, or features a variable number of inherents in each block, you might be +/// able to express it by implementing this trait, but such designs are probably too complicated. +/// Think long and hard before implementing this trait directly. +pub trait ConstraintChecker: Debug + Encode + Decode + Clone { /// The error type that this constraint checker may return type Error: Debug; - /// Optional Associated Inherent processing logic. If this transaction type is not an inherent, use (). - /// If it is an inherent, use Self, and implement the TuxedoInherent trait. - type InherentHooks: InherentInternal; - - /// The actual check validation logic + /// The on chain logic that makes the final check for whether a transaction is valid. fn check( &self, - inputs: &[Output], - peeks: &[Output], - outputs: &[Output], + input_data: &[DynamicallyTypedData], + peek_data: &[DynamicallyTypedData], + output_data: &[DynamicallyTypedData], ) -> Result; /// Tells whether this extrinsic is an inherent or not. /// If you return true here, you must provide the correct inherent hooks above. fn is_inherent(&self) -> bool; + + /// Create the inherent extrinsics to insert into a block that is being authored locally. + /// The inherent data is supplied by the authoring node. + fn create_inherents( + authoring_inherent_data: &InherentData, + previous_inherents: Vec<(Transaction, H256)>, + ) -> Vec>; + + /// Perform off-chain pre-execution checks on the inherents. + /// The inherent data is supplied by the importing node. + /// The inherent data available here is not necessarily the + /// same as what is available at authoring time. + fn check_inherents( + importing_inherent_data: &InherentData, + inherents: Vec>, + results: &mut CheckInherentsResult, + ); + + /// Return the genesis transactions that are required for the inherents. + #[cfg(feature = "std")] + fn genesis_transactions() -> Vec>; } -// This blanket implementation makes it so that any type that chooses to -// implement the Simple trait also implements the more Powerful trait. -// This way the executive can always just call the more Powerful trait. -impl ConstraintChecker for T { +// We automatically supply every single simple constraint checker with a dummy set +// of inherent hooks. This allows "normal" non-inherent constraint checkers to satisfy the +// executive's expected interfaces without the piece author worrying about inherents. +impl ConstraintChecker for T { // Use the same error type used in the simple implementation. type Error = ::Error; - type InherentHooks = (); - fn check( &self, - inputs: &[Output], - peeks: &[Output], - outputs: &[Output], + input_data: &[DynamicallyTypedData], + peek_data: &[DynamicallyTypedData], + output_data: &[DynamicallyTypedData], ) -> Result { - // Extract the input data - let input_data: Vec = - inputs.iter().map(|o| o.payload.clone()).collect(); + SimpleConstraintChecker::check(self, input_data, peek_data, output_data) + } - // Extract the peek data - let peek_data: Vec = - peeks.iter().map(|o| o.payload.clone()).collect(); + fn is_inherent(&self) -> bool { + false + } - // Extract the output data - let output_data: Vec = - outputs.iter().map(|o| o.payload.clone()).collect(); + fn create_inherents( + _authoring_inherent_data: &InherentData, + _previous_inherents: Vec<(Transaction, H256)>, + ) -> Vec> { + Vec::new() + } - // Call the simple constraint checker - SimpleConstraintChecker::check(self, &input_data, &peek_data, &output_data) + fn check_inherents( + _: &InherentData, + inherents: Vec>, + _: &mut CheckInherentsResult, + ) { + // Inherents should always be empty for this stub implementation. Not just in valid blocks, but as an invariant. + // The way we determined which inherents got here is by matching on the constraint checker. + assert!( + inherents.is_empty(), + "inherent extrinsic was passed to check inherents stub implementation." + ) } - fn is_inherent(&self) -> bool { - false + #[cfg(feature = "std")] + fn genesis_transactions() -> Vec> { + Vec::new() } } /// Utilities for writing constraint-checker-related unit tests #[cfg(test)] pub mod testing { + use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; use serde::{Deserialize, Serialize}; - use super::*; - use crate::{types::Output, verifier::TestVerifier}; + use super::{ConstraintChecker, DynamicallyTypedData, TransactionPriority}; /// A testing checker that passes (with zero priority) or not depending on /// the boolean value enclosed. @@ -114,15 +171,14 @@ pub mod testing { pub inherent: bool, } - impl ConstraintChecker for TestConstraintChecker { + impl ConstraintChecker for TestConstraintChecker { type Error = (); - type InherentHooks = (); fn check( &self, - _input_data: &[Output], - _peek_data: &[Output], - _output_data: &[Output], + _input_data: &[DynamicallyTypedData], + _peek_data: &[DynamicallyTypedData], + _output_data: &[DynamicallyTypedData], ) -> Result { if self.checks { Ok(0) @@ -134,6 +190,25 @@ pub mod testing { fn is_inherent(&self) -> bool { self.inherent } + + fn create_inherents( + _: &sp_inherents::InherentData, + _: Vec<(crate::types::Transaction, sp_core::H256)>, + ) -> Vec> { + unimplemented!() + } + + fn check_inherents( + _: &sp_inherents::InherentData, + _: Vec>, + _: &mut sp_inherents::CheckInherentsResult, + ) { + unimplemented!() + } + + fn genesis_transactions() -> Vec> { + unimplemented!() + } } #[test] diff --git a/tuxedo-core/src/executive.rs b/tuxedo-core/src/executive.rs index 93cddf0d..3e541e20 100644 --- a/tuxedo-core/src/executive.rs +++ b/tuxedo-core/src/executive.rs @@ -8,8 +8,9 @@ use crate::{ constraint_checker::ConstraintChecker, + dynamic_typing::DynamicallyTypedData, ensure, - inherents::{InherentInternal, PARENT_INHERENT_IDENTIFIER}, + inherents::PARENT_INHERENT_IDENTIFIER, types::{DispatchResult, OutputRef, Transaction, UtxoError}, utxo_set::TransparentUtxoSet, verifier::Verifier, @@ -40,7 +41,7 @@ where B: BlockT>, B::Header: HeaderT, // Tuxedo always uses u32 for block number. V: Verifier, - C: ConstraintChecker, + C: ConstraintChecker, { /// Does pool-style validation of a tuxedo transaction. /// Does not commit anything to storage. @@ -73,9 +74,9 @@ where let stripped_encoded = stripped.encode(); // Check that the verifiers of all inputs are satisfied - // Keep a Vec of the input utxos for passing to the constraint checker + // Keep a Vec of the input data for passing to the constraint checker // Keep track of any missing inputs for use in the tagged transaction pool - let mut input_utxos = Vec::new(); + let mut input_data = Vec::new(); let mut missing_inputs = Vec::new(); for input in transaction.inputs.iter() { if let Some(input_utxo) = TransparentUtxoSet::::peek_utxo(&input.output_ref) { @@ -87,19 +88,19 @@ where .verify(&stripped_encoded, Self::block_height(), &redeemer), UtxoError::VerifierError ); - input_utxos.push(input_utxo); + input_data.push(input_utxo.payload); } else { missing_inputs.push(input.output_ref.clone().encode()); } } - // Make a Vec of the peek utxos for passing to the constraint checker + // Make a Vec of the peek data for passing to the constraint checker // Keep track of any missing peeks for use in the tagged transaction pool // Use the same vec as previously to keep track of missing peeks - let mut peek_utxos = Vec::new(); + let mut peek_data = Vec::new(); for output_ref in transaction.peeks.iter() { if let Some(peek_utxo) = TransparentUtxoSet::::peek_utxo(output_ref) { - peek_utxos.push(peek_utxo); + peek_data.push(peek_utxo.payload); } else { missing_inputs.push(output_ref.encode()); } @@ -152,10 +153,17 @@ where }); } + // Extract the payload data from each output + let output_data: Vec = transaction + .outputs + .iter() + .map(|o| o.payload.clone()) + .collect(); + // Call the constraint checker transaction .checker - .check(&input_utxos, &peek_utxos, &transaction.outputs) + .check(&input_data, &peek_data, &output_data) .map_err(UtxoError::ConstraintCheckerError)?; // Return the valid transaction @@ -448,7 +456,7 @@ where ); // Call into constraint checker's own inherent hooks to create the actual transactions - C::InherentHooks::create_inherents(&data, previous_blocks_inherents) + C::create_inherents(&data, previous_blocks_inherents) } pub fn check_inherents(block: B, data: InherentData) -> sp_inherents::CheckInherentsResult { @@ -471,7 +479,7 @@ where .take_while(|tx| tx.checker.is_inherent()) .collect(); - C::InherentHooks::check_inherents(&data, inherents, &mut result); + C::check_inherents::(&data, inherents, &mut result); result } diff --git a/tuxedo-core/src/genesis.rs b/tuxedo-core/src/genesis.rs index 72c879b8..ead021a8 100644 --- a/tuxedo-core/src/genesis.rs +++ b/tuxedo-core/src/genesis.rs @@ -131,7 +131,7 @@ impl TuxedoGenesisConfig { impl BuildStorage for TuxedoGenesisConfig where V: Verifier, - C: ConstraintChecker, + C: ConstraintChecker, Transaction: Encode, Output: Encode, { diff --git a/tuxedo-core/src/inherents.rs b/tuxedo-core/src/inherents.rs index fb246154..51f3f388 100644 --- a/tuxedo-core/src/inherents.rs +++ b/tuxedo-core/src/inherents.rs @@ -30,15 +30,16 @@ //! that update environmental data), needs to include this foundational previous block inherent data provider //! so that the Tuxedo executive can scrape it to find the output references of the previous inherent transactions. -use parity_scale_codec::Encode; +use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; +use serde::{Deserialize, Serialize}; use sp_core::H256; use sp_inherents::{ CheckInherentsResult, InherentData, InherentIdentifier, IsFatalError, MakeFatalError, }; use sp_std::{vec, vec::Vec}; -use crate::{types::Transaction, ConstraintChecker, Verifier}; +use crate::{types::Transaction, ConstraintChecker, SimpleConstraintChecker, Verifier}; /// An inherent identifier for the Tuxedo parent block inherent pub const PARENT_INHERENT_IDENTIFIER: InherentIdentifier = *b"prnt_blk"; @@ -94,93 +95,109 @@ impl sp_inherents::InherentDataProvider /// understand exactly how Substrate's block authoring and Tuxedo's piece aggregation works /// (which you probably don't) you can directly implement the `InherentInternal` trait /// which is more powerful (and dangerous). -pub trait TuxedoInherent>: Sized { +pub trait InherentHooks: SimpleConstraintChecker + Sized { type Error: Encode + IsFatalError; const INHERENT_IDENTIFIER: InherentIdentifier; /// Create the inherent extrinsic to insert into a block that is being authored locally. /// The inherent data is supplied by the authoring node. - fn create_inherent( + fn create_inherent( authoring_inherent_data: &InherentData, - previous_inherent: (Transaction, H256), - ) -> Transaction; + previous_inherent: (Transaction, H256), + ) -> Transaction; /// Perform off-chain pre-execution checks on the inherent. /// The inherent data is supplied by the importing node. /// The inherent data available here is not guaranteed to be the /// same as what is available at authoring time. - fn check_inherent( + fn check_inherent( importing_inherent_data: &InherentData, - inherent: Transaction, + inherent: Transaction, results: &mut CheckInherentsResult, ); /// Return the genesis transactions that are required for this inherent. #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { + fn genesis_transactions() -> Vec> { Vec::new() } } -/// Almost identical to TuxedoInherent, but allows returning multiple extrinsics -/// (as aggregate runtimes will need to) and removes the requirement that the generic -/// outer constraint checker be buildable from `Self` so we can implement it for (). +/// An adapter type to declare, at the runtime level, that Tuxedo pieces provide custom inherent hooks. /// -/// If you are trying to implement some complex inherent logic that requires the interaction of -/// multiple inherents, or features a variable number of inherents in each block, you might be -/// able to express it by implementing this trait, but such designs are probably too complicated. -/// Think long and hard before implementing this trait directly. -pub trait InherentInternal>: Sized { - /// Create the inherent extrinsic to insert into a block that is being authored locally. - /// The inherent data is supplied by the authoring node. - fn create_inherents( - authoring_inherent_data: &InherentData, - previous_inherents: Vec<(Transaction, H256)>, - ) -> Vec>; - - /// Perform off-chain pre-execution checks on the inherents. - /// The inherent data is supplied by the importing node. - /// The inherent data available here is not guaranteed to be the - /// same as what is available at authoring time. - fn check_inherents( - importing_inherent_data: &InherentData, - inherents: Vec>, - results: &mut CheckInherentsResult, - ); - - /// Return the genesis transactions that are required for the inherents. - #[cfg(feature = "std")] - fn genesis_transactions() -> Vec>; +/// This adapter type satisfies the executive's expectations by implementing both `ConstraintChecker`. +/// The inherent logic checks to be sure that exactly one inherent is present before plumbing through to +/// the underlying `TuxedoInherent` implementation. +/// +/// This type should encode exactly like the inner type. +#[derive( + Serialize, Deserialize, Eq, PartialEq, Debug, Decode, Default, Encode, TypeInfo, Clone, Copy, +)] +pub struct InherentAdapter(C); + +/// Helper to transform an entire transaction by wrapping the constraint checker. +fn wrap_transaction(unwrapped: Transaction) -> Transaction> { + Transaction { + inputs: unwrapped.inputs, + peeks: unwrapped.peeks, + outputs: unwrapped.outputs, + checker: InherentAdapter(unwrapped.checker), + } } -/// An adapter to transform structured Tuxedo inherents into the more general and powerful -/// InherentInternal trait. -#[derive(Debug, Default, TypeInfo, Clone, Copy)] -pub struct TuxedoInherentAdapter(T); +// I wonder if this should be a deref impl instead? +/// Helper to transform an entire transaction by unwrapping the constraint checker. +fn unwrap_transaction(wrapped: Transaction>) -> Transaction { + Transaction { + inputs: wrapped.inputs, + peeks: wrapped.peeks, + outputs: wrapped.outputs, + checker: wrapped.checker.0, + } +} -impl, T: TuxedoInherent + 'static> InherentInternal - for TuxedoInherentAdapter +impl ConstraintChecker + for InherentAdapter { - fn create_inherents( + type Error = ::Error; + + fn check( + &self, + input_data: &[crate::dynamic_typing::DynamicallyTypedData], + peek_data: &[crate::dynamic_typing::DynamicallyTypedData], + output_data: &[crate::dynamic_typing::DynamicallyTypedData], + ) -> Result { + SimpleConstraintChecker::check(&self.0, input_data, peek_data, output_data) + } + + fn is_inherent(&self) -> bool { + true + } + + fn create_inherents( authoring_inherent_data: &InherentData, - previous_inherents: Vec<(Transaction, H256)>, - ) -> Vec> { + previous_inherents: Vec<(Transaction, H256)>, + ) -> Vec> { if previous_inherents.len() > 1 { panic!("Authoring a leaf inherent constraint checker, but multiple previous inherents were supplied.") } - let previous_inherent = previous_inherents.first().cloned(); - - vec![>::create_inherent( + let (previous_inherent, hash) = previous_inherents + .first() + .cloned() + .expect("Previous inherent exists."); + let current_inherent = wrap_transaction(::create_inherent( authoring_inherent_data, - previous_inherent.expect("Previous inherent exists."), - )] + (unwrap_transaction(previous_inherent), hash), + )); + + vec![current_inherent] } - fn check_inherents( + fn check_inherents( importing_inherent_data: &InherentData, - inherents: Vec>, + inherents: Vec>, results: &mut CheckInherentsResult, ) { if inherents.is_empty() { @@ -201,40 +218,20 @@ impl, T: TuxedoInherent + 'static> In } let inherent = inherents .first() - .expect("Previous inherent exists.") - .clone(); - >::check_inherent(importing_inherent_data, inherent, results) - } - - #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { - >::genesis_transactions() - } -} - -impl> InherentInternal for () { - fn create_inherents( - _: &InherentData, - _: Vec<(Transaction, H256)>, - ) -> Vec> { - Vec::new() - } - - fn check_inherents( - _: &InherentData, - inherents: Vec>, - _: &mut CheckInherentsResult, - ) { - // Inherents should always be empty for this stub implementation. Not just in valid blocks, but as an invariant. - // The way we determined which inherents got here is by matching on the constraint checker. - assert!( - inherents.is_empty(), - "inherent extrinsic was passed to check inherents stub implementation." + .cloned() + .expect("Previous inherent exists."); + ::check_inherent( + importing_inherent_data, + unwrap_transaction(inherent), + results, ) } #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { - Vec::new() + fn genesis_transactions() -> Vec> { + ::genesis_transactions() + .into_iter() + .map(|gtx| wrap_transaction(gtx)) + .collect() } } diff --git a/tuxedo-core/src/lib.rs b/tuxedo-core/src/lib.rs index d4712515..cab2a8db 100644 --- a/tuxedo-core/src/lib.rs +++ b/tuxedo-core/src/lib.rs @@ -22,6 +22,7 @@ pub mod genesis; pub use aggregator::{aggregate, tuxedo_constraint_checker, tuxedo_verifier}; pub use constraint_checker::{ConstraintChecker, SimpleConstraintChecker}; pub use executive::Executive; +pub use inherents::{InherentAdapter, InherentHooks}; pub use verifier::Verifier; /// A Tuxedo-specific target for diagnostic node log messages diff --git a/tuxedo-core/src/types.rs b/tuxedo-core/src/types.rs index 1f193705..d1679b92 100644 --- a/tuxedo-core/src/types.rs +++ b/tuxedo-core/src/types.rs @@ -107,7 +107,7 @@ impl Decode for Transaction { // so we do a minimal implementation. impl Extrinsic for Transaction where - C: TypeInfo + ConstraintChecker + 'static, + C: TypeInfo + ConstraintChecker + 'static, V: TypeInfo + Verifier + 'static, { type Call = Self; diff --git a/tuxedo-core/src/verifier.rs b/tuxedo-core/src/verifier.rs index 272931e8..30eb23ea 100644 --- a/tuxedo-core/src/verifier.rs +++ b/tuxedo-core/src/verifier.rs @@ -33,6 +33,20 @@ pub trait Verifier: Debug + Encode + Decode + Clone { /// Main function in the trait. Does the checks to make sure an output can be spent. fn verify(&self, simplified_tx: &[u8], block_height: u32, redeemer: &Self::Redeemer) -> bool; + + /// A way to create a new instance of the verifier whose semantics cannot be spent. + /// This may be a signature check with a pubkey of 0 or a hashlock with a hash o 0 + /// or a bitcoin script that directly returns false, etc. + /// + /// This is only required in chains that use inherents, and thus a default implementation + /// is provided. + fn new_unspendable() -> Option { + log::debug!( + target: crate::LOG_TARGET, + "In new_unspendable default function implementation. About to return hardcoded `None`." + ); + None + } } /// A simple verifier that allows anyone to consume an output at any time @@ -64,6 +78,10 @@ impl Verifier for Unspendable { fn verify(&self, _simplified_tx: &[u8], _: u32, _: &()) -> bool { false } + + fn new_unspendable() -> Option { + Some(Self) + } } // Idea: It could be useful to allow delay deciding whether the redemption should succeed @@ -84,6 +102,10 @@ impl Verifier for TestVerifier { fn verify(&self, _simplified_tx: &[u8], _: u32, _: &()) -> bool { self.verifies } + + fn new_unspendable() -> Option { + Some(Self { verifies: false }) + } } #[cfg(test)] diff --git a/tuxedo-core/src/verifier/htlc.rs b/tuxedo-core/src/verifier/htlc.rs index ec805984..9e70eacb 100644 --- a/tuxedo-core/src/verifier/htlc.rs +++ b/tuxedo-core/src/verifier/htlc.rs @@ -70,6 +70,12 @@ impl Verifier for BlakeTwoHashLock { fn verify(&self, _: &[u8], _: u32, secret: &Self::Redeemer) -> bool { BlakeTwo256::hash(secret) == self.hash_lock } + + fn new_unspendable() -> Option { + Some(BlakeTwoHashLock { + hash_lock: H256::zero(), + }) + } } /// Allows a UTXO to be spent, and therefore acknowledged by an intended recipient by revealing diff --git a/tuxedo-core/src/verifier/multi_signature.rs b/tuxedo-core/src/verifier/multi_signature.rs index a4a6f1d3..7a85f8f7 100644 --- a/tuxedo-core/src/verifier/multi_signature.rs +++ b/tuxedo-core/src/verifier/multi_signature.rs @@ -96,6 +96,13 @@ impl Verifier for ThresholdMultiSignature { valid_sigs.len() >= self.threshold.into() } + + fn new_unspendable() -> Option { + Some(Self { + threshold: 1, + signatories: Vec::new(), + }) + } } #[cfg(test)] diff --git a/tuxedo-core/src/verifier/simple_signature.rs b/tuxedo-core/src/verifier/simple_signature.rs index 8a08af1a..680e5037 100644 --- a/tuxedo-core/src/verifier/simple_signature.rs +++ b/tuxedo-core/src/verifier/simple_signature.rs @@ -49,6 +49,10 @@ impl Verifier for Sr25519Signature { fn verify(&self, simplified_tx: &[u8], _: u32, sig: &Signature) -> bool { sp_io::crypto::sr25519_verify(sig, simplified_tx, &Public::from_h256(self.owner_pubkey)) } + + fn new_unspendable() -> Option { + Some(Self::new(H256::zero())) + } } /// Pay To Public Key Hash (P2PKH) @@ -70,6 +74,12 @@ impl Verifier for P2PKH { BlakeTwo256::hash(pubkey) == self.owner_pubkey_hash && sp_io::crypto::sr25519_verify(signature, simplified_tx, pubkey) } + + fn new_unspendable() -> Option { + Some(Self { + owner_pubkey_hash: H256::zero(), + }) + } } #[cfg(test)] diff --git a/tuxedo-parachain-core/src/validate_block.rs b/tuxedo-parachain-core/src/validate_block.rs index 70c1cb06..5bb1388f 100644 --- a/tuxedo-parachain-core/src/validate_block.rs +++ b/tuxedo-parachain-core/src/validate_block.rs @@ -74,7 +74,7 @@ where B::Header: HeaderT, // Tuxedo always uses u32 for block number. Transaction: Extrinsic, V: TypeInfo + Verifier + 'static, - C: TypeInfo + ConstraintChecker + 'static, // + Into>, + C: TypeInfo + ConstraintChecker + 'static, // + Into>, { sp_runtime::runtime_logger::RuntimeLogger::init(); log::info!(target: "tuxvb", "🕵️🕵️🕵️🕵️Entering validate_block implementation"); @@ -230,7 +230,7 @@ where // Consider an alternative way to express the bounds here: // Transaction: Extrinsic V: TypeInfo + Verifier + 'static, - C: TypeInfo + ConstraintChecker + 'static, + C: TypeInfo + ConstraintChecker + 'static, { // The commented stuff is Basti's algo. // It is nicer than my hack because it searches the transactions, diff --git a/tuxedo-template-runtime/src/genesis.rs b/tuxedo-template-runtime/src/genesis.rs index 923a6240..7baf8cb9 100644 --- a/tuxedo-template-runtime/src/genesis.rs +++ b/tuxedo-template-runtime/src/genesis.rs @@ -3,12 +3,12 @@ use super::{ kitties::{KittyData, Parent}, money::Coin, - OuterConstraintChecker, OuterConstraintCheckerInherentHooks, OuterVerifier, WASM_BINARY, + OuterConstraintChecker, OuterVerifier, WASM_BINARY, }; use hex_literal::hex; use tuxedo_core::{ - inherents::InherentInternal, verifier::{Sr25519Signature, ThresholdMultiSignature, UpForGrabs}, + ConstraintChecker, }; /// Helper type for the ChainSpec. @@ -24,7 +24,7 @@ pub fn development_genesis_config() -> RuntimeGenesisConfig { let signatories = vec![SHAWN_PUB_KEY_BYTES.into(), ANDREW_PUB_KEY_BYTES.into()]; // The inherents are computed using the appropriate method, and placed before the extrinsics. - let mut genesis_transactions = OuterConstraintCheckerInherentHooks::genesis_transactions(); + let mut genesis_transactions = OuterConstraintChecker::genesis_transactions(); genesis_transactions.extend([ // Money Transactions @@ -57,7 +57,6 @@ mod tests { use std::sync::Arc; use tuxedo_core::{ dynamic_typing::{DynamicallyTypedData, UtxoData}, - inherents::InherentInternal, types::{Output, OutputRef}, }; @@ -82,7 +81,7 @@ mod tests { let signatories = vec![shawn_pub_key_bytes.into(), andrew_pub_key_bytes.into()]; - let mut genesis_transactions = OuterConstraintCheckerInherentHooks::genesis_transactions(); + let mut genesis_transactions = OuterConstraintChecker::genesis_transactions(); genesis_transactions.extend([ // Money Transactions Coin::<0>::mint(100, Sr25519Signature::new(shawn_pub_key_bytes)), @@ -127,7 +126,8 @@ mod tests { }, }; - let inherents_len = OuterConstraintCheckerInherentHooks::genesis_transactions().len(); + let inherents_len = + OuterConstraintChecker::genesis_transactions::().len(); let tx = default_runtime_genesis_config() .get_transaction(inherents_len) @@ -171,7 +171,8 @@ mod tests { }, }; - let inherents_len = OuterConstraintCheckerInherentHooks::genesis_transactions().len(); + let inherents_len = + OuterConstraintChecker::genesis_transactions::().len(); let tx = default_runtime_genesis_config() .get_transaction(1 + inherents_len) diff --git a/tuxedo-template-runtime/src/lib.rs b/tuxedo-template-runtime/src/lib.rs index 6a2c8a38..d932cfc4 100644 --- a/tuxedo-template-runtime/src/lib.rs +++ b/tuxedo-template-runtime/src/lib.rs @@ -37,6 +37,7 @@ use tuxedo_core::{ tuxedo_constraint_checker, tuxedo_verifier, types::Transaction as TuxedoTransaction, verifier::{Sr25519Signature, ThresholdMultiSignature, UpForGrabs}, + InherentAdapter, }; pub use amoeba; @@ -163,7 +164,7 @@ impl parachain_piece::ParachainPieceConfig for Runtime { /// For any given Tuxedo runtime there is a finite set of such constraint checkers. /// For example, this may check that input token values exceed output token values. #[derive(Serialize, Deserialize, Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)] -#[tuxedo_constraint_checker(OuterVerifier)] +#[tuxedo_constraint_checker] #[cfg(feature = "parachain")] pub enum OuterConstraintChecker { /// Checks monetary transactions in a basic fungible cryptocurrency @@ -184,21 +185,21 @@ pub enum OuterConstraintChecker { /// the losing claims can be removed from storage. PoeDispute(poe::PoeDispute), /// Set the block's timestamp via an inherent extrinsic. - SetTimestamp(timestamp::SetTimestamp), + SetTimestamp(InherentAdapter>), /// Upgrade the Wasm Runtime RuntimeUpgrade(runtime_upgrade::RuntimeUpgrade), // TODO This one is last for now so that I can write a hacky algorithm to scrape // the inherent data and assume it is last. /// Set some parachain related information via an inherent extrinsic. - ParachainInfo(parachain_piece::SetParachainInfo), + ParachainInfo(InherentAdapter>), } /// A constraint checker is a piece of logic that can be used to check a transaction. /// For any given Tuxedo runtime there is a finite set of such constraint checkers. /// For example, this may check that input token values exceed output token values. #[derive(Serialize, Deserialize, Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)] -#[tuxedo_constraint_checker(OuterVerifier)] +#[tuxedo_constraint_checker] #[cfg(not(feature = "parachain"))] pub enum OuterConstraintChecker { /// Checks monetary transactions in a basic fungible cryptocurrency @@ -219,7 +220,7 @@ pub enum OuterConstraintChecker { /// the losing claims can be removed from storage. PoeDispute(poe::PoeDispute), /// Set the block's timestamp via an inherent extrinsic. - SetTimestamp(timestamp::SetTimestamp), + SetTimestamp(InherentAdapter>), /// Upgrade the Wasm Runtime RuntimeUpgrade(runtime_upgrade::RuntimeUpgrade), diff --git a/wardrobe/kitties/src/lib.rs b/wardrobe/kitties/src/lib.rs index 49d7d416..014a639e 100644 --- a/wardrobe/kitties/src/lib.rs +++ b/wardrobe/kitties/src/lib.rs @@ -173,7 +173,7 @@ impl KittyData { where V: Verifier, OV: Verifier + From, - OC: tuxedo_core::ConstraintChecker + From, + OC: tuxedo_core::ConstraintChecker + From, { Transaction { inputs: vec![], diff --git a/wardrobe/money/src/lib.rs b/wardrobe/money/src/lib.rs index b012cc84..b61354c5 100644 --- a/wardrobe/money/src/lib.rs +++ b/wardrobe/money/src/lib.rs @@ -82,7 +82,7 @@ impl Coin { where V: Verifier, OV: Verifier + From, - OC: tuxedo_core::ConstraintChecker + From>, + OC: tuxedo_core::ConstraintChecker + From>, { Transaction { inputs: vec![], diff --git a/wardrobe/parachain/src/lib.rs b/wardrobe/parachain/src/lib.rs index 9f4ee664..a35ba393 100644 --- a/wardrobe/parachain/src/lib.rs +++ b/wardrobe/parachain/src/lib.rs @@ -29,16 +29,17 @@ use sp_core::H256; use sp_inherents::{CheckInherentsResult, InherentData}; use sp_runtime::transaction_validity::TransactionPriority; use sp_std::{vec, vec::Vec}; +use tuxedo_parachain_core::tuxedo_core::dynamic_typing::DynamicallyTypedData; +use tuxedo_parachain_core::tuxedo_core::SimpleConstraintChecker; // We get all the Tuxedo core stuff through the re-export so we don't risk crossed versions. use tuxedo_parachain_core::ParachainInherentDataUtxo; use tuxedo_parachain_core::{ tuxedo_core::{ ensure, - inherents::{TuxedoInherent, TuxedoInherentAdapter}, + inherents::InherentHooks, support_macros::{CloneNoBound, DebugNoBound, DefaultNoBound}, types::{Input, Output, OutputRef, Transaction}, - verifier::UpForGrabs, - ConstraintChecker, Verifier, + Verifier, }, SetRelayParentNumberStorage, }; @@ -106,17 +107,14 @@ pub enum ParachainError { #[scale_info(skip_type_params(T))] pub struct SetParachainInfo(PhantomData); -impl> ConstraintChecker - for SetParachainInfo -{ +impl SimpleConstraintChecker for SetParachainInfo { type Error = ParachainError; - type InherentHooks = TuxedoInherentAdapter; fn check( &self, - input_data: &[Output], - _peek_data: &[Output], - output_data: &[Output], + input_data: &[DynamicallyTypedData], + _peek_data: &[DynamicallyTypedData], + output_data: &[DynamicallyTypedData], ) -> Result { log::debug!( target: LOG_TARGET, @@ -127,7 +125,6 @@ impl> Constrai ensure!(!output_data.is_empty(), Self::Error::MissingNewInfo); ensure!(output_data.len() == 1, Self::Error::ExtraOutputs); let current: ParachainInherentData = output_data[0] - .payload .extract::() .map_err(|_| Self::Error::BadlyTyped)? .into(); @@ -139,7 +136,6 @@ impl> Constrai ensure!(!input_data.is_empty(), Self::Error::MissingPreviousInfo); ensure!(input_data.len() == 1, Self::Error::ExtraInputs); let previous: ParachainInherentData = input_data[0] - .payload .extract::() .map_err(|_| Self::Error::BadlyTyped)? .into(); @@ -163,20 +159,14 @@ impl> Constrai Ok(0) } - - fn is_inherent(&self) -> bool { - true - } } -impl, T: ParachainPieceConfig + 'static> TuxedoInherent - for SetParachainInfo -{ +impl InherentHooks for SetParachainInfo { // Same error type as in frame type Error = sp_inherents::MakeFatalError<()>; const INHERENT_IDENTIFIER: sp_inherents::InherentIdentifier = INHERENT_IDENTIFIER; - fn create_inherent( + fn create_inherent( authoring_inherent_data: &InherentData, (_previous_inherent, previous_id): (Transaction, H256), ) -> Transaction { @@ -206,7 +196,8 @@ impl, T: ParachainPieceConfig + 'static> TuxedoIn let new_output = Output { payload: ParachainInherentDataUtxo::from(current_info).into(), - verifier: UpForGrabs.into(), + verifier: V::new_unspendable() + .expect("Must be able to create unspendable verifier to use parachain inherent."), }; let t = Transaction { @@ -224,7 +215,7 @@ impl, T: ParachainPieceConfig + 'static> TuxedoIn t } - fn check_inherent( + fn check_inherent( _importing_inherent_data: &InherentData, _inherent: Transaction, _result: &mut CheckInherentsResult, @@ -239,7 +230,7 @@ impl, T: ParachainPieceConfig + 'static> TuxedoIn } #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { + fn genesis_transactions() -> Vec> { let payload = new_data_from_relay_parent_number(0).into(); vec![Transaction { @@ -247,7 +238,9 @@ impl, T: ParachainPieceConfig + 'static> TuxedoIn peeks: Vec::new(), outputs: vec![Output { payload, - verifier: UpForGrabs.into(), + verifier: V::new_unspendable().expect( + "Must be able to create unspendable verifier to use timestamp inherent.", + ), }], checker: Self::default(), }] diff --git a/wardrobe/parachain/src/tests.rs b/wardrobe/parachain/src/tests.rs index 73bd72a1..5cc3ea2d 100644 --- a/wardrobe/parachain/src/tests.rs +++ b/wardrobe/parachain/src/tests.rs @@ -17,9 +17,9 @@ impl ParachainPieceConfig for MockConfig { #[test] fn update_parachain_info_happy_path() { let old: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let inputs: Vec> = vec![old.into()]; + let inputs = vec![old]; let new: DynamicallyTypedData = new_data_from_relay_parent_number(4).into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -30,9 +30,9 @@ fn update_parachain_info_happy_path() { #[test] fn update_parachain_info_relay_block_not_increasing() { let old: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let inputs: Vec> = vec![old.into()]; + let inputs = vec![old]; let new: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -44,9 +44,9 @@ fn update_parachain_info_relay_block_not_increasing() { fn update_parachain_info_extra_inputs() { let old1: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); let old2: DynamicallyTypedData = Bogus.into(); - let inputs: Vec> = vec![old1.into(), old2.into()]; + let inputs = vec![old1, old2]; let new: DynamicallyTypedData = new_data_from_relay_parent_number(4).into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -56,9 +56,9 @@ fn update_parachain_info_extra_inputs() { #[test] fn update_parachain_info_missing_input() { - let inputs: Vec> = vec![]; + let inputs = vec![]; let new: DynamicallyTypedData = new_data_from_relay_parent_number(4).into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -69,9 +69,9 @@ fn update_parachain_info_missing_input() { #[test] fn update_parachain_info_bogus_input() { let old: DynamicallyTypedData = Bogus.into(); - let inputs: Vec> = vec![old.into()]; + let inputs = vec![old]; let new: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -82,10 +82,10 @@ fn update_parachain_info_bogus_input() { #[test] fn update_parachain_info_extra_outputs() { let old: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let inputs: Vec> = vec![old.into()]; + let inputs = vec![old]; let new1: DynamicallyTypedData = new_data_from_relay_parent_number(4).into(); let new2: DynamicallyTypedData = Bogus.into(); - let outputs: Vec> = vec![new1.into(), new2.into()]; + let outputs = vec![new1.into(), new2.into()]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -96,8 +96,8 @@ fn update_parachain_info_extra_outputs() { #[test] fn update_parachain_info_missing_output() { let old: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let inputs: Vec> = vec![old.into()]; - let outputs: Vec> = vec![]; + let inputs = vec![old]; + let outputs = vec![]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), @@ -108,9 +108,9 @@ fn update_parachain_info_missing_output() { #[test] fn update_parachain_info_bogus_output() { let old: DynamicallyTypedData = new_data_from_relay_parent_number(3).into(); - let inputs: Vec> = vec![old.into()]; + let inputs = vec![old]; let new: DynamicallyTypedData = Bogus.into(); - let outputs: Vec> = vec![new.into()]; + let outputs = vec![new]; assert_eq!( SetParachainInfo::(Default::default()).check(&inputs, &[], &outputs), diff --git a/wardrobe/timestamp/src/lib.rs b/wardrobe/timestamp/src/lib.rs index bb09b6ad..03347759 100644 --- a/wardrobe/timestamp/src/lib.rs +++ b/wardrobe/timestamp/src/lib.rs @@ -1,13 +1,10 @@ //! Allow block authors to include a timestamp via an inherent transaction. //! //! This is roughly analogous to FRAME's pallet timestamp. It relies on the same client-side inherent data provider, -//! as well as Tuxedo's own previous block inehrent data provider. +//! as well as Tuxedo's own previous block inherent data provider. //! //! In each block, the block author must include a single `SetTimestamp` transaction that peeks at the //! Timestamp UTXO that was created in the previous block, and creates a new one with an updated timestamp. -//! -//! This piece currently features a prominent hack which will need to be cleaned up in due course. -//! It abuses the UpForGrabs verifier. This should be replaced with an Unspendable verifier and an eviction workflow. #![cfg_attr(not(feature = "std"), no_std)] @@ -24,11 +21,10 @@ use sp_timestamp::InherentError::TooFarInFuture; use tuxedo_core::{ dynamic_typing::{DynamicallyTypedData, UtxoData}, ensure, - inherents::{TuxedoInherent, TuxedoInherentAdapter}, + inherents::InherentHooks, support_macros::{CloneNoBound, DebugNoBound, DefaultNoBound}, types::{Output, OutputRef, Transaction}, - verifier::UpForGrabs, - ConstraintChecker, SimpleConstraintChecker, Verifier, + SimpleConstraintChecker, Verifier, }; #[cfg(test)] @@ -147,17 +143,14 @@ pub enum TimestampError { #[scale_info(skip_type_params(T))] pub struct SetTimestamp(PhantomData); -impl> ConstraintChecker - for SetTimestamp -{ +impl SimpleConstraintChecker for SetTimestamp { type Error = TimestampError; - type InherentHooks = TuxedoInherentAdapter; fn check( &self, - input_data: &[tuxedo_core::types::Output], - peek_data: &[tuxedo_core::types::Output], - output_data: &[tuxedo_core::types::Output], + input_data: &[DynamicallyTypedData], + peek_data: &[DynamicallyTypedData], + output_data: &[DynamicallyTypedData], ) -> Result { log::debug!( target: LOG_TARGET, @@ -173,7 +166,6 @@ impl> ConstraintChe // Make sure the only output is a new best timestamp ensure!(!output_data.is_empty(), Self::Error::MissingNewTimestamp); let new_timestamp = output_data[0] - .payload .extract::() .map_err(|_| Self::Error::BadlyTyped)?; ensure!( @@ -191,7 +183,6 @@ impl> ConstraintChe // We don't expect any additional peeks typically, but they are harmless. ensure!(!peek_data.is_empty(), Self::Error::MissingPreviousTimestamp); let old_timestamp = peek_data[0] - .payload .extract::() .map_err(|_| Self::Error::BadlyTyped)?; @@ -209,19 +200,13 @@ impl> ConstraintChe Ok(0) } - - fn is_inherent(&self) -> bool { - true - } } -impl, T: TimestampConfig + 'static> TuxedoInherent - for SetTimestamp -{ +impl InherentHooks for SetTimestamp { type Error = sp_timestamp::InherentError; const INHERENT_IDENTIFIER: sp_inherents::InherentIdentifier = sp_timestamp::INHERENT_IDENTIFIER; - fn create_inherent( + fn create_inherent( authoring_inherent_data: &InherentData, previous_inherent: (Transaction, H256), ) -> tuxedo_core::types::Transaction { @@ -249,7 +234,8 @@ impl, T: TimestampConfig + 'static> TuxedoInheren let new_output = Output { payload: new_timestamp.into(), - verifier: UpForGrabs.into(), + verifier: V::new_unspendable() + .expect("Must be able to create unspendable verifier to use timestamp inherent."), }; Transaction { @@ -260,7 +246,7 @@ impl, T: TimestampConfig + 'static> TuxedoInheren } } - fn check_inherent( + fn check_inherent( importing_inherent_data: &InherentData, inherent: Transaction, result: &mut CheckInherentsResult, @@ -303,7 +289,7 @@ impl, T: TimestampConfig + 'static> TuxedoInheren } #[cfg(feature = "std")] - fn genesis_transactions() -> Vec> { + fn genesis_transactions() -> Vec> { use std::time::{Duration, SystemTime}; let time = SystemTime::UNIX_EPOCH .elapsed() @@ -316,7 +302,9 @@ impl, T: TimestampConfig + 'static> TuxedoInheren peeks: Vec::new(), outputs: vec![Output { payload: Timestamp::new(time, 0).into(), - verifier: UpForGrabs.into(), + verifier: V::new_unspendable().expect( + "Must be able to create unspendable verifier to use timestamp inherent.", + ), }], checker: Self::default(), }] @@ -346,6 +334,9 @@ pub struct CleanUpTimestamp(PhantomData); impl SimpleConstraintChecker for CleanUpTimestamp { type Error = TimestampError; + // Cleaning up timestamps will not work until evictions are available because + // the timestamps stored on-chain are unspendable. + // FIXME: https://github.com/Off-Narrative-Labs/Tuxedo/issues/98 fn check( &self, input_data: &[DynamicallyTypedData], diff --git a/wardrobe/timestamp/src/update_timestamp_tests.rs b/wardrobe/timestamp/src/update_timestamp_tests.rs index e254c1d0..e886044e 100644 --- a/wardrobe/timestamp/src/update_timestamp_tests.rs +++ b/wardrobe/timestamp/src/update_timestamp_tests.rs @@ -19,11 +19,9 @@ fn update_timestamp_happy_path() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(3_000, 2).into(); - let out: Vec> = vec![new.into()]; - assert_eq!(checker.check(&[], &peek, &out), Ok(0)); + assert_eq!(checker.check(&[], &[old], &[new]), Ok(0)); } #[test] @@ -31,14 +29,11 @@ fn update_timestamp_with_input() { let checker = SetTimestamp::(Default::default()); let bogus: DynamicallyTypedData = Bogus.into(); - let inp: Vec> = vec![bogus.into()]; let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(3_000, 2).into(); - let out: Vec> = vec![new.into()]; assert_eq!( - checker.check(&inp, &peek, &out), + checker.check(&[bogus], &[old], &[new]), Err(InputsWhileSettingTimestamp) ); } @@ -48,11 +43,9 @@ fn update_timestamp_bogus_peek() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Bogus.into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(3_000, 2).into(); - let out: Vec> = vec![new.into()]; - assert_eq!(checker.check(&[], &peek, &out), Err(BadlyTyped)); + assert_eq!(checker.check(&[], &[old], &[new]), Err(BadlyTyped)); } #[test] @@ -60,9 +53,11 @@ fn update_timestamp_no_peek() { let checker = SetTimestamp::(Default::default()); let new: DynamicallyTypedData = Timestamp::new(3_000, 2).into(); - let out: Vec> = vec![new.into()]; - assert_eq!(checker.check(&[], &[], &out), Err(MissingPreviousTimestamp)); + assert_eq!( + checker.check(&[], &[], &[new]), + Err(MissingPreviousTimestamp) + ); } #[test] @@ -70,9 +65,8 @@ fn update_timestamp_no_output() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; - assert_eq!(checker.check(&[], &peek, &[]), Err(MissingNewTimestamp)); + assert_eq!(checker.check(&[], &[old], &[]), Err(MissingNewTimestamp)); } #[test] @@ -80,13 +74,11 @@ fn update_timestamp_too_many_outputs() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(3_000, 2).into(); let bogus: DynamicallyTypedData = Bogus.into(); - let out: Vec> = vec![new.into(), bogus.into()]; assert_eq!( - checker.check(&[], &peek, &out), + checker.check(&[], &[old], &[new, bogus]), Err(TooManyOutputsWhileSettingTimestamp) ); } @@ -96,12 +88,10 @@ fn update_timestamp_wrong_height() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(5_000, 3).into(); - let out: Vec> = vec![new.into()]; assert_eq!( - checker.check(&[], &peek, &out), + checker.check(&[], &[old], &[new]), Err(NewTimestampWrongHeight) ); } @@ -111,11 +101,9 @@ fn update_timestamp_output_earlier_than_input() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(2_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(1_000, 2).into(); - let out: Vec> = vec![new.into()]; - assert_eq!(checker.check(&[], &peek, &out), Err(TimestampTooOld)); + assert_eq!(checker.check(&[], &[old], &[new]), Err(TimestampTooOld)); } #[test] @@ -123,11 +111,9 @@ fn update_timestamp_output_newer_than_previous_best_nut_not_enough_to_meet_thres let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(1_000, 1).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(2_000, 2).into(); - let out: Vec> = vec![new.into()]; - assert_eq!(checker.check(&[], &peek, &out), Err(TimestampTooOld)); + assert_eq!(checker.check(&[], &[old], &[new]), Err(TimestampTooOld)); } #[test] @@ -135,12 +121,10 @@ fn update_timestamp_previous_timestamp_wrong_height() { let checker = SetTimestamp::(Default::default()); let old: DynamicallyTypedData = Timestamp::new(0, 0).into(); - let peek: Vec> = vec![old.into()]; let new: DynamicallyTypedData = Timestamp::new(2_000, 2).into(); - let out: Vec> = vec![new.into()]; assert_eq!( - checker.check(&[], &peek, &out), + checker.check(&[], &[old], &[new]), Err(PreviousTimestampWrongHeight) ); }