Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accumulators for intra-block book-keeping #105

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 90 additions & 9 deletions tuxedo-core/aggregator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ pub fn tuxedo_verifier(_: TokenStream, body: TokenStream) -> TokenStream {
}

/// This macro treats the supplied enum as an aggregate constraint checker. As such, it implements the `From`
/// trait for eah of the inner types. Then it implements the `ConstraintChecker` trait for this type for this
/// trait for each of the inner types. Then it implements the `ConstraintChecker` trait for this
/// enum by delegating to an inner type.
///
/// It also declares an associated error type. The error type has a variant for each inner constraint checker,
/// just like this original enum. however, the contained values in the error enum are of the corresponding types
/// for the inner constraint checker.
/// In order to implement the `ConstraintChecker` trait, this macro must declare a few additional associated
/// aggregate types including an aggregate error enum and an aggregate accumulator struct, and this accumulator's
/// associated value type.
#[proc_macro_attribute]
pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> TokenStream {
let ast = parse_macro_input!(body as ItemEnum);
Expand Down Expand Up @@ -115,20 +115,39 @@ 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 mut accumulator_type_name = outer_type.to_string();
accumulator_type_name.push_str("Accumulator");
let accumulator_type = Ident::new(&accumulator_type_name, outer_type.span());

let mut accumulator_value_type_name = outer_type.to_string();
accumulator_value_type_name.push_str("AccumulatorValueType");
let accumulator_value_type = Ident::new(&accumulator_value_type_name, outer_type.span());

let vis = ast.vis;
let inner_types = inner_types.clone();
let inner_types2 = inner_types.clone();
let inner_types3 = inner_types.clone();
let inner_types4 = inner_types.clone();
let inner_types5 = inner_types.clone();
let variants2 = variants.clone();
let variants3 = variants.clone();
let variants4 = variants.clone();
let variants5 = variants.clone();
let variants6 = variants.clone();
Comment on lines 132 to +141
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's gotta be a better way to do this right? Does anyone know? I'm pretty much a macro newb. This crate is by far the most complex one I've ever written.


let output = quote! {
// Preserve the original enum, and write the From impls
#[tuxedo_core::aggregate]
#original_code


/// This type is generated by the `#[tuxedo_constraint_checker]` macro.
/// It is a combined error type for the errors of each individual checker.
/// It is an aggregate error type associated with the aggregate constraint checker.
/// It has a variant that encapsulates the error type of each individual constituent constraint checker.
///
/// This type is accessible downstream as `<OuterConstraintChecker as ConstraintChecker>::Error`
#[derive(Debug)]
Expand All @@ -138,18 +157,80 @@ pub fn tuxedo_constraint_checker(attrs: TokenStream, body: TokenStream) -> Token
)*
}

/// This type is generated by the `#[tuxedo_constraint_checker]` macro.
/// It is an aggregate accumulator type associated with the aggregate constraint checker.
///
/// This type is accessible downstream as `<OuterConstraintChecker as ConstraintChecker>::Error`
#[derive(Debug)]
#vis struct #accumulator_type;

/// This type is generated by the `#[tuxedo_constraint_checker]` macro.
/// It is an aggregate enum with a variant for the associated value type for each constituent accumulator.
#[derive(Debug)]
// TODO conflicting impls here too? I'm really not getting this error
// Supposedly it conflicts with this from core: impl<T> From<T> for T;
// #[tuxedo_core::aggregate]
#vis enum #accumulator_value_type {
#(
#variants2(<<#inner_types2 as tuxedo_core::ConstraintChecker<#verifier>>::Accumulator as tuxedo_core::constraint_checker::Accumulator>::ValueType),
)*
}

impl tuxedo_core::constraint_checker::Accumulator for #accumulator_type {
type ValueType = #accumulator_value_type;

fn initial_value(intermediate: Self::ValueType) -> Self::ValueType {
match intermediate {
#(
Self::ValueType::#variants3(inner) => Self::ValueType::#variants3(<<#inner_types3 as tuxedo_core::ConstraintChecker<#verifier>>::Accumulator as tuxedo_core::constraint_checker::Accumulator>::initial_value(inner)),
)*
}
}

fn key_path(intermediate: Self::ValueType) -> & 'static [u8] {
match intermediate {
#(
Self::ValueType::#variants4(inner) => <<#inner_types4 as tuxedo_core::ConstraintChecker<#verifier>>::Accumulator as tuxedo_core::constraint_checker::Accumulator>::key_path(inner),
)*
}
}

fn accumulate(acc: Self::ValueType, next: Self::ValueType) -> Result<Self::ValueType, ()> {
match (acc, next) {
#(
(Self::ValueType::#variants5(inner_acc), Self::ValueType::#variants5(inner_next)) => {
<<#inner_types5 as tuxedo_core::ConstraintChecker<#verifier>>::Accumulator as tuxedo_core::constraint_checker::Accumulator>::accumulate(inner_acc, inner_next)
.map(|inner_result| {
Self::ValueType::#variants5(inner_result)
})
}
)*
_ => panic!("Accumulate was called on aggregate runtime, but accumulator and intermediate value had different variants.")
}
}
}

impl tuxedo_core::ConstraintChecker<#verifier> for #outer_type {
type Error = #error_type;
type Accumulator = #accumulator_type;

fn check (
&self,
inputs: &[tuxedo_core::types::Output<#verifier>],
peeks: &[tuxedo_core::types::Output<#verifier>],
outputs: &[tuxedo_core::types::Output<#verifier>],
) -> Result<TransactionPriority, Self::Error> {
) -> Result<tuxedo_core::constraint_checker::ConstraintCheckingSuccess<<Self::Accumulator as tuxedo_core::constraint_checker::Accumulator>::ValueType>, Self::Error> {
match self {
#(
Self::#variants2(inner) => inner.check(inputs, peeks, outputs).map_err(|e| Self::Error::#variants2(e)),
Self::#variants6(inner) => inner.check(inputs, peeks, outputs)
.map(|old| {
//TODO I would really rather have an into or from impl for ConstraintCheckingSuccess, but that's just won't compile
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this still a necessary TODO comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was stuck/confused on this last night. Matteo's transform method is a good way forward.

tuxedo_core::constraint_checker::ConstraintCheckingSuccess {
priority: old.priority,
accumulator_value: <Self::Accumulator as tuxedo_core::constraint_checker::Accumulator>::ValueType::#variants6(old.accumulator_value),
}
})
.map_err(|e| Self::Error::#variants6(e)),
)*
}
}
Expand Down
129 changes: 122 additions & 7 deletions tuxedo-core/src/constraint_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,111 @@ use sp_std::{fmt::Debug, vec::Vec};

use crate::{dynamic_typing::DynamicallyTypedData, types::Output, Verifier};
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
#[cfg(feature = "std")]
use serde::{Deserialize, Serialize};
use sp_runtime::transaction_validity::TransactionPriority;

/// A type representing a successful result of checking a transaction's constraints.
#[derive(Debug, Clone, Default, PartialEq)]
pub struct ConstraintCheckingSuccess<ValueType> {
/// The priority of this transaction that should be reported to the transaction pool.
pub priority: TransactionPriority,
/// An intermediate value that should be passed to an accumulator that track transient intra-block data.
pub accumulator_value: ValueType,
}

impl<ValueType> ConstraintCheckingSuccess<ValueType> {
pub fn transform<NewValueType: From<ValueType>>(
self,
) -> ConstraintCheckingSuccess<NewValueType> {
ConstraintCheckingSuccess::<NewValueType> {
priority: self.priority,
accumulator_value: self.accumulator_value.into(),
}
}
}

impl<ValueType> From<TransactionPriority> for ConstraintCheckingSuccess<ValueType>
where
ValueType: Default,
{
fn from(priority: TransactionPriority) -> ConstraintCheckingSuccess<ValueType> {
ConstraintCheckingSuccess::<ValueType> {
priority,
accumulator_value: ValueType::default(),
}
}
}

impl<ValueType> From<(TransactionPriority, ValueType)> for ConstraintCheckingSuccess<ValueType> {
fn from(t: (TransactionPriority, ValueType)) -> ConstraintCheckingSuccess<ValueType> {
ConstraintCheckingSuccess::<ValueType> {
priority: t.0,
accumulator_value: t.1,
}
}
}

/// An accumulator allows a Tuxedo piece to do some internal bookkeeping during the course
/// of a single block. The Bookkeeping must be done through this accumulator-like interface
/// where each transaction yields some intermediate value that is then folded into the accumulator
/// via some logic that is encapsulated in the implementation.
///
/// Many Tuxedo pieces will not need accumulators. In such a case, the constraint checker should
/// simply use () as the accumulator type.
///
/// Some typical usecases for an accumulator would be making sure that the block author reward
/// does not exceed the block's transaction fees. Or making sure that a timestamp transaction occurs
/// at most once in a single block.
///
/// The accumulator is reset automatically at the end of every block. It is not possible to store
/// data here for use in subsequent blocks.
pub trait Accumulator {
/// The type that is given and also the type of the accumulation result.
/// The most general accumulators will use two different types for those,
/// but let's do that iff we ever need it. Probably will need to so a simple counter can be implemented.
type ValueType: Debug + Encode + Decode;

/// The accumulator value that should be used to start a fresh accumulation
/// at the beginning of each new block.
///
/// This is a function and takes a value, as opposed to being a constant for an important reason.
/// Aggregate runtimes made from multiple pieces will need to give a different initial value depending
/// which of the constituent constraint checkers is being called.
fn initial_value(_: Self::ValueType) -> Self::ValueType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this just always be the Default for the ValueType associated type? Should ValueType just instead implement the Default trait?

Copy link
Contributor Author

@JoshOrndorff JoshOrndorff Oct 9, 2023

Choose a reason for hiding this comment

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

I thought about that same thing. All the examples of accumulators that I thought of can indeed be implemented using the default value as the initial value.

I ended up going with a separate method for these reasons:

  1. Semantics. While it seems the initial value of the accumulator will typically be the default, there is no fundamental reason it must be this way. Someone may write an unintuitive impl of Default for that purpose.
  2. To mirror https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.fold
  3. The main one: For aggregate runtimes. When the constraint checker is composed with the macro from individual constraint checkers, it needs to do an actual match and return a different initial value depending on which piece is being called. Default does allow taking a param and returning a different result based on that param.


// TODO This needs to be improved because keeping them unique is kind of a footgun right now.
// It is a function and not a const because in aggregate runtimes, we will need to determine,
// at the executive level, what the key is, which means we need to match in the aggregate implementation.
/// A unique key for this accumulator in the runtime. Like with storage types,
/// Runtime authors must take care that this key is not used anywhere else in the runtime.
///
/// This is a function and takes a value, as opposed to being a constant for an important reason.
/// Aggregate runtimes made from multiple pieces will need to give a different key path depending
/// which of the constituent constraint checkers is being called.
fn key_path(_: Self::ValueType) -> &'static [u8];

/// This function is responsible for combining or "folding" the intermediate value
/// from the current transaction into the accumulated value so far in this block.
fn accumulate(a: Self::ValueType, b: Self::ValueType) -> Result<Self::ValueType, ()>;
}

impl Accumulator for () {
type ValueType = ();

fn initial_value(_: ()) -> () {
()
}

fn key_path(_: Self::ValueType) -> &'static [u8] {
b"stub_acc"
}

fn accumulate(_: (), _: ()) -> Result<(), ()> {
Ok(())
}
}

/// A simplified constraint checker that a transaction can choose to call. Checks whether the input
/// and output data from a transaction meets the codified constraints.
///
Expand All @@ -22,13 +122,16 @@ pub trait SimpleConstraintChecker: Debug + Encode + Decode + Clone {
/// The error type that this constraint checker may return
type Error: Debug;

/// A transient accumulator that can be used to track intermediate data during the course of a block's execution.
type Accumulator: Accumulator;

/// The actual check validation logic
fn check(
&self,
input_data: &[DynamicallyTypedData],
peek_data: &[DynamicallyTypedData],
output_data: &[DynamicallyTypedData],
) -> Result<TransactionPriority, Self::Error>;
) -> Result<ConstraintCheckingSuccess<<Self::Accumulator as Accumulator>::ValueType>, Self::Error>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think type aliasing for stuff like this is preferred. Things can start to look really ugly without it..

}

/// A single constraint checker that a transaction can choose to call. Checks whether the input
Expand All @@ -44,13 +147,16 @@ pub trait ConstraintChecker<V: Verifier>: Debug + Encode + Decode + Clone {
/// the error type that this constraint checker may return
type Error: Debug;

/// A transient accumulator that can be used to track intermediate data during the course of a block's execution.
type Accumulator: Accumulator;

/// The actual check validation logic
fn check(
&self,
inputs: &[Output<V>],
peeks: &[Output<V>],
outputs: &[Output<V>],
) -> Result<TransactionPriority, Self::Error>;
) -> Result<ConstraintCheckingSuccess<<Self::Accumulator as Accumulator>::ValueType>, Self::Error>;
}

// This blanket implementation makes it so that any type that chooses to
Expand All @@ -60,12 +166,16 @@ impl<T: SimpleConstraintChecker, V: Verifier> ConstraintChecker<V> for T {
// Use the same error type used in the simple implementation.
type Error = <T as SimpleConstraintChecker>::Error;

// Use the same accumulator type used in the simple implementation.
type Accumulator = <T as SimpleConstraintChecker>::Accumulator;

fn check(
&self,
inputs: &[Output<V>],
peeks: &[Output<V>],
outputs: &[Output<V>],
) -> Result<TransactionPriority, Self::Error> {
) -> Result<ConstraintCheckingSuccess<<Self::Accumulator as Accumulator>::ValueType>, Self::Error>
{
// Extract the input data
let input_data: Vec<DynamicallyTypedData> =
inputs.iter().map(|o| o.payload.clone()).collect();
Expand All @@ -87,6 +197,7 @@ impl<T: SimpleConstraintChecker, V: Verifier> ConstraintChecker<V> for T {
#[cfg(feature = "std")]
pub mod testing {
use super::*;
use scale_info::TypeInfo;

/// A testing checker that passes (with zero priority) or not depending on
/// the boolean value enclosed.
Expand All @@ -99,14 +210,16 @@ pub mod testing {
impl SimpleConstraintChecker for TestConstraintChecker {
type Error = ();

type Accumulator = ();

fn check(
&self,
_input_data: &[DynamicallyTypedData],
_peek_data: &[DynamicallyTypedData],
_output_data: &[DynamicallyTypedData],
) -> Result<TransactionPriority, ()> {
) -> Result<ConstraintCheckingSuccess<()>, ()> {
if self.checks {
Ok(0)
Ok(0.into())
} else {
Err(())
}
Expand All @@ -117,7 +230,7 @@ pub mod testing {
fn test_checker_passes() {
let result =
SimpleConstraintChecker::check(&TestConstraintChecker { checks: true }, &[], &[], &[]);
assert_eq!(result, Ok(0));
assert_eq!(result, Ok(0.into()));
}

#[test]
Expand All @@ -126,4 +239,6 @@ pub mod testing {
SimpleConstraintChecker::check(&TestConstraintChecker { checks: false }, &[], &[], &[]);
assert_eq!(result, Err(()));
}

// TODO add tests for accumulator stuff.
}
Loading