From 8cb6368fe6ae62d6263a72025bd2c97628ffa678 Mon Sep 17 00:00:00 2001 From: Sean Yu Date: Wed, 27 Mar 2019 14:57:50 -0700 Subject: [PATCH] Adding a #[signed_root(skip_hashing)] macro Lets the user annotate fields of a struct to skip for signed root hashing. Also added tests in a `eth2/utils/tests` crate, so that we can test whether these derived macros work as intended. --- eth2/types/src/attestation.rs | 1 + eth2/types/src/beacon_block.rs | 1 + eth2/types/src/beacon_block_header.rs | 1 + eth2/types/src/deposit_input.rs | 1 + eth2/types/src/slashable_attestation.rs | 1 + eth2/types/src/transfer.rs | 1 + eth2/types/src/voluntary_exit.rs | 1 + eth2/utils/ssz_derive/src/lib.rs | 44 ++++++---- eth2/utils/ssz_derive/tests/test_derives.rs | 94 +++++++++++++++++++++ 9 files changed, 129 insertions(+), 16 deletions(-) create mode 100644 eth2/utils/ssz_derive/tests/test_derives.rs diff --git a/eth2/types/src/attestation.rs b/eth2/types/src/attestation.rs index dabccfde7..a8eeea909 100644 --- a/eth2/types/src/attestation.rs +++ b/eth2/types/src/attestation.rs @@ -25,6 +25,7 @@ pub struct Attestation { pub aggregation_bitfield: Bitfield, pub data: AttestationData, pub custody_bitfield: Bitfield, + #[signed_root(skip_hashing)] pub aggregate_signature: AggregateSignature, } diff --git a/eth2/types/src/beacon_block.rs b/eth2/types/src/beacon_block.rs index 6a3f1a354..77c1620f3 100644 --- a/eth2/types/src/beacon_block.rs +++ b/eth2/types/src/beacon_block.rs @@ -27,6 +27,7 @@ pub struct BeaconBlock { pub previous_block_root: Hash256, pub state_root: Hash256, pub body: BeaconBlockBody, + #[signed_root(skip_hashing)] pub signature: Signature, } diff --git a/eth2/types/src/beacon_block_header.rs b/eth2/types/src/beacon_block_header.rs index f4bee27e1..090d0a965 100644 --- a/eth2/types/src/beacon_block_header.rs +++ b/eth2/types/src/beacon_block_header.rs @@ -27,6 +27,7 @@ pub struct BeaconBlockHeader { pub previous_block_root: Hash256, pub state_root: Hash256, pub block_body_root: Hash256, + #[signed_root(skip_hashing)] pub signature: Signature, } diff --git a/eth2/types/src/deposit_input.rs b/eth2/types/src/deposit_input.rs index 3f8a6177a..380528dc0 100644 --- a/eth2/types/src/deposit_input.rs +++ b/eth2/types/src/deposit_input.rs @@ -25,6 +25,7 @@ use test_random_derive::TestRandom; pub struct DepositInput { pub pubkey: PublicKey, pub withdrawal_credentials: Hash256, + #[signed_root(skip_hashing)] pub proof_of_possession: Signature, } diff --git a/eth2/types/src/slashable_attestation.rs b/eth2/types/src/slashable_attestation.rs index 05c41a72b..e557285b8 100644 --- a/eth2/types/src/slashable_attestation.rs +++ b/eth2/types/src/slashable_attestation.rs @@ -27,6 +27,7 @@ pub struct SlashableAttestation { pub validator_indices: Vec, pub data: AttestationData, pub custody_bitfield: Bitfield, + #[signed_root(skip_hashing)] pub aggregate_signature: AggregateSignature, } diff --git a/eth2/types/src/transfer.rs b/eth2/types/src/transfer.rs index 4b10ce1ca..f291190b2 100644 --- a/eth2/types/src/transfer.rs +++ b/eth2/types/src/transfer.rs @@ -32,6 +32,7 @@ pub struct Transfer { pub slot: Slot, pub pubkey: PublicKey, #[derivative(Hash = "ignore")] + #[signed_root(skip_hashing)] pub signature: Signature, } diff --git a/eth2/types/src/voluntary_exit.rs b/eth2/types/src/voluntary_exit.rs index f64f950cb..0cdc63149 100644 --- a/eth2/types/src/voluntary_exit.rs +++ b/eth2/types/src/voluntary_exit.rs @@ -24,6 +24,7 @@ use test_random_derive::TestRandom; pub struct VoluntaryExit { pub epoch: Epoch, pub validator_index: u64, + #[signed_root(skip_hashing)] pub signature: Signature, } diff --git a/eth2/utils/ssz_derive/src/lib.rs b/eth2/utils/ssz_derive/src/lib.rs index 9ba1de416..ce2538785 100644 --- a/eth2/utils/ssz_derive/src/lib.rs +++ b/eth2/utils/ssz_derive/src/lib.rs @@ -38,7 +38,7 @@ extern crate proc_macro; use proc_macro::TokenStream; -use quote::quote; +use quote::{quote, ToTokens}; use syn::{parse_macro_input, DeriveInput}; /// Returns a Vec of `syn::Ident` for each named field in the struct. @@ -218,7 +218,7 @@ fn get_tree_hashable_named_field_idents<'a>( /// The field attribute is: `#[tree_hash(skip_hashing)]` fn should_skip_tree_hash(field: &syn::Field) -> bool { for attr in &field.attrs { - if attr.tts.to_string() == "( skip_hashing )" { + if attr.into_token_stream().to_string() == "# [ tree_hash ( skip_hashing ) ]" { return true; } } @@ -289,7 +289,7 @@ fn final_type_ident(field: &syn::Field) -> &syn::Ident { /// If you can think of a better way to do this, please make an issue! /// /// Fields are processed in the order they are defined. -#[proc_macro_derive(SignedRoot)] +#[proc_macro_derive(SignedRoot, attributes(signed_root))] pub fn ssz_signed_root_derive(input: TokenStream) -> TokenStream { let item = parse_macro_input!(input as DeriveInput); @@ -302,19 +302,7 @@ pub fn ssz_signed_root_derive(input: TokenStream) -> TokenStream { let mut field_idents: Vec<&syn::Ident> = vec![]; - for field in struct_data.fields.iter() { - let final_type_ident = final_type_ident(&field); - - if type_ident_is_signature(final_type_ident) { - break; - } else { - let ident = field - .ident - .as_ref() - .expect("ssz_derive only supports named_struct fields."); - field_idents.push(ident); - } - } + let field_idents = get_signed_root_named_field_idents(&struct_data); let output = quote! { impl ssz::SignedRoot for #name { @@ -330,3 +318,27 @@ pub fn ssz_signed_root_derive(input: TokenStream) -> TokenStream { }; output.into() } + +fn get_signed_root_named_field_idents(struct_data: &syn::DataStruct) -> Vec<&syn::Ident> { + struct_data + .fields + .iter() + .filter_map(|f| { + if should_skip_signed_root(&f) { + None + } else { + Some(match &f.ident { + Some(ref ident) => ident, + _ => panic!("ssz_derive only supports named struct fields"), + }) + } + }) + .collect() +} + +fn should_skip_signed_root(field: &syn::Field) -> bool { + field + .attrs + .iter() + .any(|attr| attr.into_token_stream().to_string() == "# [ signed_root ( skip_hashing ) ]") +} diff --git a/eth2/utils/ssz_derive/tests/test_derives.rs b/eth2/utils/ssz_derive/tests/test_derives.rs new file mode 100644 index 000000000..e025dc3a5 --- /dev/null +++ b/eth2/utils/ssz_derive/tests/test_derives.rs @@ -0,0 +1,94 @@ +use ssz::{SignedRoot, TreeHash}; +use ssz_derive::{SignedRoot, TreeHash}; + +#[derive(TreeHash, SignedRoot)] +struct CryptoKitties { + best_kitty: u64, + worst_kitty: u8, + kitties: Vec, +} + +impl CryptoKitties { + fn new() -> Self { + CryptoKitties { + best_kitty: 9999, + worst_kitty: 1, + kitties: vec![2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43], + } + } + + fn hash(&self) -> Vec { + let mut list: Vec> = Vec::new(); + list.push(self.best_kitty.hash_tree_root()); + list.push(self.worst_kitty.hash_tree_root()); + list.push(self.kitties.hash_tree_root()); + ssz::merkle_hash(&mut list) + } +} + +#[test] +fn test_cryptokitties_hash() { + let kitties = CryptoKitties::new(); + let expected_hash = vec![ + 201, 9, 139, 14, 24, 247, 21, 55, 132, 211, 51, 125, 183, 186, 177, 33, 147, 210, 42, 108, + 174, 162, 221, 227, 157, 179, 15, 7, 97, 239, 82, 220, + ]; + assert_eq!(kitties.hash(), expected_hash); +} + +#[test] +fn test_simple_tree_hash_derive() { + let kitties = CryptoKitties::new(); + assert_eq!(kitties.hash_tree_root(), kitties.hash()); +} + +#[test] +fn test_simple_signed_root_derive() { + let kitties = CryptoKitties::new(); + assert_eq!(kitties.signed_root(), kitties.hash()); +} + +#[derive(TreeHash, SignedRoot)] +struct Casper { + friendly: bool, + #[tree_hash(skip_hashing)] + friends: Vec, + #[signed_root(skip_hashing)] + dead: bool, +} + +impl Casper { + fn new() -> Self { + Casper { + friendly: true, + friends: vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10], + dead: true, + } + } + + fn expected_signed_hash(&self) -> Vec { + let mut list = Vec::new(); + list.push(self.friendly.hash_tree_root()); + list.push(self.friends.hash_tree_root()); + ssz::merkle_hash(&mut list) + } + + fn expected_tree_hash(&self) -> Vec { + let mut list = Vec::new(); + list.push(self.friendly.hash_tree_root()); + list.push(self.dead.hash_tree_root()); + ssz::merkle_hash(&mut list) + } +} + +#[test] +fn test_annotated_tree_hash_derive() { + let casper = Casper::new(); + assert_eq!(casper.hash_tree_root(), casper.expected_tree_hash()); +} + +#[test] +fn test_annotated_signed_root_derive() { + let casper = Casper::new(); + assert_eq!(casper.signed_root(), casper.expected_signed_hash()); +}