Timestamp Inherent (#100)

* Sketch piece draft

* Builds, Runs, Logs look okay, but no inherent extrinsics in block.

* comment wrong tests

* A few more logs. Seems like the authoring task is not putting inherents into the block.

Let's dig deeper.

* cargo fmt

* Actually return the inherent extrinsic!

* Explore Aura slot a little bit too. This will probably be removed before we merge this PR.

* toml sort

* Sketch plan for constructing utxo-style inherent extrinsics.

* cargo fmt

* Sketch some actual transaction construction logic

* Implement InherentDataProvider for tuxedo parent block inherent data provider

* wire parent block idp into service

* Update piece to require consuming previous timestamp.

* off by one error

* update piece for first block exception

* cargo fmt

* Attempt at checking timestamp inherent. Not working yet.

* toml sort

* correct max drift calculation.

Now nodes can import one block before the authoring node unpeers with them.

* Add logs to `check_inherents`

* Better logs. I'm struggling with a multinode network. Not sure if its because of this inherent stuff.

* Use `StateVersion::V0` when calculating extrinsics root to match Substrate's network module

* Remove incorrect comment.

This has nothing to do with the `StateVersion` enum.

* remove commented code

* Sketch some cleanup ideas.

They don't fully make sense yet because I haven't expanded the storage to differentiate best time from any other noted time. That will be next.

* cargo fmt

* Start sketching toward separate notions of best and noted time.

* Actually distinguish `BestTimestamp` vs `NotedTimestamp` at the type level.

* attempt to make clippy happy

* cargo fmt

* structure test suite

* Sketch actual test cases

* fmt

* rename set -> update?

* First working test

* Fix previous best check

* moar update tests

* moar update tests

* Write out last of update timestamp tests.

* first cleanup test

* moar cleanup tests

* multiple cleanup tests

* featuregate all test modules

* Finsih cleanup tests

* test first block hack

* cargo fmt

* forbid timestamp calls in pool

* Sketch idea for `TuxedoInherent` trait, an attempt at a reusable interface for Tuxedo pieces intending to be Inherents.

* fix warnings

* Revise inherent interface and sketch into timestamp piece

* move create into timestamp piece, a little more revising the trait

* add static to PoeConfig too. I'm not sure why this started coming up here.

* make shit compile

* cargo fmt

* Fix off by one error

* prune the aura inherent stuff. It was useful for learning

* Revert "forbid timestamp calls in pool"

This reverts commit e818e8c.

* Logging and generics fucking everywhere. This is gettinga  little ugly. I hope it somehow works out in the end.

* Move is_inherent to Constraint checker; implement is_signed

* cargo fmt

* move checking logic into timestamp piece

* fix inherent identifier

* rework internal trait to always use Vecs

* start working on the gagregator. This is getting complicated.

* finish first draft of compiling aggregator.

* First draft of executive compiles

* cargo fmt

* Whole node compiles

* Prune comment

* First block hack in create flow. It actually runs and imports blocks!!!!

* Important TODO about making sure every expected inherent is present (and no extras).

* cargo fmt

* Rework check inherents to make sure none are missing or extra

* actually implement is_inherent methods

* update some old comments

* Annotate transactions with original hash identities

* toml sort

* enforce inherents at beginning of the bloock

* cleanup some way-too-verbose debugging

* Cleanup and fix some old TODOs

* cargo fmt

* bring back transaction priority import

* `is_inherent` should not have a default impl in the internal inherent trait

* cleanup logging target

* Simplify inherent traits and bounds considerably

* Make simple constraint checker actually simple (no inherents allowed). Also make tests pass.

* Delete a bunch of accidentally included amoeba tests

* cargo fmt

* revert unnecessary trait derivation

* fully qualified path in macro

* revert unrelated change

* restore pool checks for inherents

* Cleanup comments in timestamp tuxedo piece

* don't be so heavy handed with the service

* cargo fmt

* prune stray file

* comments in macro

* cleanup wrapping and unwrapping in macros

* followup to simple constraint checkers cannot be inherents.

* prune more too-verbose logging

* simplify trait bounds on transform helper

* relax trait bound on verifier

* brush up constraint checker

* revise inherents file

* fmt

* clippy

* unfuck piece tests after some botched search and replace

* Update is_signed tests for clarified semantics

* Update TestConstraintChecker to reflect inherents

* Tests for inherent ordering

* Oops, fix logic error. Tests FTW.

* switch a stray log! to debug!

* Fix extrinsics root that changed because the TestTransaction now has another field and thus its encoding is longer.

* Better expect messages in service

* better expect messages in the executive.

* Better examples for when to use full `VonstraintChecker` trait.

* Move constants to config trait

* remove old comment

* Attempt to refactor to single timestamp type.

* fixed update_timestamp_tests

Signed-off-by: muraca <mmuraca247@gmail.com>

* fixed cleanup_tests and first_block_special_case_tests

Signed-off-by: muraca <mmuraca247@gmail.com>

* fixed some comments and typos

Signed-off-by: muraca <mmuraca247@gmail.com>

* remove unnecessary scale_info skip_type_params

* remove kind of off-topic comment

* add comment from review suggestion

* remove extra space

* fix comment typo

* revert ugly consequences of `use super::*;`

Signed-off-by: muraca <mmuraca247@gmail.com>

* simplify inherent order checking logic

---------

Co-authored-by: muraca <mmuraca247@gmail.com>

Cargo.toml

@@ -12,13 +12,15 @@ members = [
 	"wardrobe/amoeba",
 	"wardrobe/money",
 	"wardrobe/poe",
+	"wardrobe/timestamp",
 	"wardrobe/kitties",
 	"wardrobe/runtime_upgrade",
 ]
 resolver = "2"
 
 [workspace.dependencies]
 # Generic dependencies
+async-trait = "0.1.73"
 clap = "4.3.0"
 hex-literal = "0.4.1"
 jsonrpsee = "0.16.2"

node/Cargo.toml

@@ -45,6 +45,7 @@ sp-blockchain = { workspace = true }
 
 # Local Dependencies
 node-template-runtime = { package = "tuxedo-template-runtime", path = "../tuxedo-template-runtime" }
+tuxedo-core = { path = "../tuxedo-core" }
 
 [[bin]]
 name = "node-template"

node/src/service.rs

@@ -234,6 +234,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
         );
 
         let slot_duration = sc_consensus_aura::slot_duration(&*client)?;
+        let client_for_cidp = client.clone();
 
         let aura = sc_consensus_aura::start_aura::<AuraPair, _, _, _, _, _, _, _, _, _, _>(
             StartAuraParams {
@@ -242,16 +243,27 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
                 select_chain,
                 block_import,
                 proposer_factory,
-                create_inherent_data_providers: move |_, ()| async move {
-                    let timestamp = sp_timestamp::InherentDataProvider::from_system_time();
-
-                    let slot =
-						sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration(
-							*timestamp,
-							slot_duration,
-						);
-
-                    Ok((slot, timestamp))
+                create_inherent_data_providers: move |parent_hash, ()| {
+                    let parent_block = client_for_cidp
+                        .clone()
+                        .block(parent_hash)
+                        .expect("Block backend should not error.")
+                        .expect("Parent block should be found in database for authoring to work.")
+                        .block;
+
+                    async move {
+                        let parent_idp =
+                            tuxedo_core::inherents::ParentBlockInherentDataProvider(parent_block);
+                        let timestamp = sp_timestamp::InherentDataProvider::from_system_time();
+
+                        let slot =
+                            sp_consensus_aura::inherents::InherentDataProvider::from_timestamp_and_slot_duration(
+                                *timestamp,
+                                slot_duration,
+                            );
+
+                        Ok((slot, parent_idp, timestamp))
+                    }
                 },
                 force_authoring,
                 backoff_authoring_blocks,

tuxedo-core/Cargo.toml

@@ -7,6 +7,7 @@ repository = "https://github.com/Off-Narrative-Labs/Tuxedo"
 version = "1.0.0-dev"
 
 [dependencies]
+async-trait = { optional = true, workspace = true }
 log = { workspace = true }
 parity-scale-codec = { features = [ "derive" ], workspace = true }
 parity-util-mem = { optional = true, workspace = true }
@@ -19,6 +20,7 @@ derive-no-bound = { path = "no_bound" }
 sp-api = { default_features = false, workspace = true }
 sp-core = { default_features = false, workspace = true }
 sp-debug-derive = { features = [ "force-debug" ], default_features = false, workspace = true }
+sp-inherents = { default_features = false, workspace = true }
 sp-io = { features = [ "with-tracing" ], default_features = false, workspace = true }
 sp-runtime = { default_features = false, workspace = true }
 sp-std = { default_features = false, workspace = true }
@@ -30,12 +32,14 @@ array-bytes = { workspace = true }
 [features]
 default = [ "std" ]
 std = [
+	"async-trait",
 	"sp-debug-derive/std",
 	"parity-scale-codec/std",
 	"sp-core/std",
 	"sp-std/std",
 	"serde",
 	"sp-api/std",
+	"sp-inherents/std",
 	"sp-io/std",
 	"sp-runtime/std",
 	"parity-util-mem",

tuxedo-core/aggregator/src/lib.rs

@@ -32,20 +32,35 @@ pub fn aggregate(_: TokenStream, body: TokenStream) -> TokenStream {
         )
     });
     let variants = variant_type_pairs.clone().map(|(v, _t)| v);
+    let variants2 = variants.clone();
     let inner_types = variant_type_pairs.map(|(_v, t)| t);
+    let inner_types2 = inner_types.clone();
 
     let output = quote! {
         // First keep the original code in tact
         #original_code
 
-        // Now write all the From impls
+        // Now write all the wrapping From impls
         #(
             impl From<#inner_types> for #outer_type {
                 fn from(b: #inner_types) -> Self {
                     Self::#variants(b)
                 }
             }
         )*
+
+        // Finally write all the un-wrapping From impls
+        #(
+            impl From<#outer_type> for #inner_types2 {
+                fn from(a: #outer_type) -> Self {
+                    if let #outer_type::#variants2(b) = a {
+                        b
+                    } else {
+                        panic!("wrong type or something...")
+                    }
+                }
+            }
+        )*
     };
 
     output.into()
@@ -115,12 +130,28 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token
     let variants = variant_type_pairs.clone().map(|(v, _t)| v);
     let inner_types = variant_type_pairs.map(|(_v, t)| t);
 
-    let vis = ast.vis;
+    // Set up the names of the new associated types.
     let mut error_type_name = outer_type.to_string();
     error_type_name.push_str("Error");
     let error_type = Ident::new(&error_type_name, outer_type.span());
-    let inner_types = inner_types.clone();
+
+    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 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
         #[tuxedo_core::aggregate]
@@ -138,9 +169,84 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token
             )*
         }
 
+        /// 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 `<OuterConstraintChecker as ConstraintChecker>::InherentHooks`
+        #[derive(Debug, scale_info::TypeInfo)]
+        #vis enum #inherent_hooks {
+            #(
+                #variants2(<#inner_types2 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks),
+            )*
+        }
+
+        impl tuxedo_core::inherents::InherentInternal<#verifier, #outer_type> for #inherent_hooks {
+
+            fn create_inherents(
+                authoring_inherent_data: &InherentData,
+                previous_inherents: Vec<(tuxedo_core::types::Transaction<#verifier, #outer_type>, sp_core::H256)>,
+            ) -> Vec<tuxedo_core::types::Transaction<#verifier, #outer_type>>  {
+
+                let mut all_inherents = Vec::new();
+
+                #(
+                    {
+                        // Filter the previous inherents down to just the ones that came from this piece
+                        let previous_inherents = previous_inherents
+                            .iter()
+                            .filter_map(|(tx, hash)| {
+                                match tx.checker {
+                                    #outer_type::#variants3(ref inner_checker) => Some((tx.transform::<#inner_types3>(), *hash )),
+                                    _ => None,
+                                }
+                            })
+                            .collect();
+
+                        let inherents = <#inner_types3 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks::create_inherents(authoring_inherent_data, previous_inherents)
+                            .iter()
+                            .map(|tx| tx.transform::<#outer_type>())
+                            .collect::<Vec<_>>();
+                        all_inherents.extend(inherents);
+                    }
+                )*
+
+                // Return the aggregate of all inherent extrinsics from all constituent constraint checkers.
+                all_inherents
+            }
+
+            fn check_inherents(
+                importing_inherent_data: &sp_inherents::InherentData,
+                inherents: Vec<tuxedo_core::types::Transaction<#verifier, #outer_type>>,
+                result: &mut sp_inherents::CheckInherentsResult,
+            ) {
+                #(
+                    let relevant_inherents: Vec<tuxedo_core::types::Transaction<#verifier, #inner_types4>> = inherents
+                        .iter()
+                        .filter_map(|tx| {
+                            match tx.checker {
+                                #outer_type::#variants4(ref inner_checker) => Some(tx.transform::<#inner_types4>()),
+                                _ => None,
+                            }
+                        })
+                        .collect();
+
+                    <#inner_types4 as tuxedo_core::ConstraintChecker<#verifier>>::InherentHooks::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."
+                    if result.fatal_error() {
+                        return;
+                    }
+                )*
+            }
+
+        }
+
         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>],
@@ -149,10 +255,20 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token
             ) -> Result<TransactionPriority, Self::Error> {
                 match self {
                     #(
-                        Self::#variants2(inner) => inner.check(inputs, peeks, outputs).map_err(|e| Self::Error::#variants2(e)),
+                        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_types6 as tuxedo_core::ConstraintChecker<#verifier>>::is_inherent(inner),
                     )*
                 }
+
             }
+
         }
     };