From f394cfe55982c817bc87939f399ffd934b66c113 Mon Sep 17 00:00:00 2001 From: Jay Geng Date: Tue, 16 Apr 2024 21:08:15 -0400 Subject: [PATCH] tighen the allowed usage of `Hash` --- soroban-sdk-macros/src/derive_enum.rs | 9 ++++- soroban-sdk-macros/src/derive_fn.rs | 37 +++++++----------- soroban-sdk-macros/src/derive_spec_fn.rs | 13 +------ soroban-sdk-macros/src/derive_struct.rs | 8 +++- soroban-sdk-macros/src/derive_struct_tuple.rs | 7 +++- soroban-sdk-macros/src/lib.rs | 5 ++- soroban-sdk-macros/src/map_type.rs | 39 ++++++++++++------- 7 files changed, 63 insertions(+), 55 deletions(-) diff --git a/soroban-sdk-macros/src/derive_enum.rs b/soroban-sdk-macros/src/derive_enum.rs index c7eb592e8..c952fcea7 100644 --- a/soroban-sdk-macros/src/derive_enum.rs +++ b/soroban-sdk-macros/src/derive_enum.rs @@ -85,6 +85,7 @@ pub fn derive_type_enum( into_xdr, } = map_tuple_variant( path, + &vis, enum_ident, &case_num_lit, &case_name_str_lit, @@ -333,6 +334,7 @@ fn map_empty_variant( fn map_tuple_variant( path: &Path, + vis: &Visibility, enum_ident: &Ident, case_num_lit: &Literal, case_name_str_lit: &Literal, @@ -343,9 +345,14 @@ fn map_tuple_variant( errors: &mut Vec, ) -> VariantTokens { let spec_case = { + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let field_types = fields .iter() - .map(|f| match map_type(&f.ty) { + .map(|f| match map_type(&f.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index 76104d690..f0247b788 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -1,3 +1,4 @@ +use crate::map_type::map_type; use itertools::MultiUnzip; use proc_macro2::TokenStream as TokenStream2; use quote::{format_ident, quote}; @@ -6,22 +7,31 @@ use syn::{ punctuated::Punctuated, spanned::Spanned, token::{Colon, Comma}, - Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, Type, TypePath, TypeReference, + Attribute, Error, FnArg, Ident, Pat, PatIdent, PatType, Path, ReturnType, Type, TypePath, + TypeReference, }; #[allow(clippy::too_many_arguments)] -pub fn derive_fn( +pub fn derive_pub_fn( crate_path: &Path, call: &TokenStream2, ident: &Ident, attrs: &[Attribute], inputs: &Punctuated, + output: &ReturnType, trait_ident: Option<&Ident>, client_ident: &str, ) -> Result { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); + let allow_hash = ident == "__check_auth"; + if let ReturnType::Type(_, ty) = output { + if let Err(e) = map_type(ty, allow_hash) { + errors.push(e); + } + } + // Prepare the env input. let env_input = inputs.first().and_then(|a| match a { FnArg::Typed(pat_type) => { @@ -55,28 +65,9 @@ pub fn derive_fn( .enumerate() .map(|(i, a)| match a { FnArg::Typed(pat_ty) => { - if ident != "__check_auth" { - let mut ty = &*pat_ty.ty; - if let Type::Reference(TypeReference { elem, .. }) = ty { - ty = elem; - } - if let Type::Path(TypePath { - path: syn::Path { segments, .. }, - .. - }) = ty - { - if segments.last().map_or(false, |s| s.ident == "Hash" && !s.arguments.is_none()) { - errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, - since there is no guarantee the received input is from a secure hash function. - If you still intend to use a hash with such a guarantee, please use `ByteN`")); - } else { - () - } - } else { - () - } + if let Err(e) = map_type(&pat_ty.ty, allow_hash) { + errors.push(e); } - let ident = format_ident!("arg_{}", i); let arg = FnArg::Typed(PatType { attrs: vec![], diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index 86cc12290..1d7db9c85 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -61,7 +61,7 @@ pub fn derive_fn_spec( errors.push(Error::new(a.span(), "argument not supported")); "".to_string() }; - match map_type(&pat_type.ty) { + match map_type(&pat_type.ty, ident == "__check_auth") { Ok(type_) => { let name = name.try_into().unwrap_or_else(|_| { const MAX: u32 = 30; @@ -71,15 +71,6 @@ pub fn derive_fn_spec( )); StringM::::default() }); - - if let ScSpecTypeDef::Hash(_) = type_ { - if ident != "__check_auth" { - errors.push(Error::new(a.span(), "`Hash` cannot be used as argument to a public user function, - since there is no guarantee the received input is from a secure hash function. - If you still intend to use a hash with such a guarantee, please use `ByteN`")); - } - } - ScSpecFunctionInputV0 { doc: "".try_into().unwrap(), name, @@ -109,7 +100,7 @@ pub fn derive_fn_spec( // Prepare the output. let spec_result = match output { - ReturnType::Type(_, ty) => vec![match map_type(ty) { + ReturnType::Type(_, ty) => vec![match map_type(ty, false) { Ok(spec) => spec, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_struct.rs b/soroban-sdk-macros/src/derive_struct.rs index a35f9d9c5..2b51e3b66 100644 --- a/soroban-sdk-macros/src/derive_struct.rs +++ b/soroban-sdk-macros/src/derive_struct.rs @@ -25,7 +25,11 @@ pub fn derive_type_struct( ) -> TokenStream2 { // Collect errors as they are encountered and emit them at the end. let mut errors = Vec::::new(); - + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let fields = &data.fields; let field_count_usize: usize = fields.len(); let (spec_fields, field_idents, field_names, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields @@ -43,7 +47,7 @@ pub fn derive_type_struct( errors.push(Error::new(field_ident.span(), format!("struct field name is too long: {}, max is {MAX}", field_name.len()))); StringM::::default() }), - type_: match map_type(&field.ty) { + type_: match map_type(&field.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/derive_struct_tuple.rs b/soroban-sdk-macros/src/derive_struct_tuple.rs index a41fe2a4a..a6b6ccdd8 100644 --- a/soroban-sdk-macros/src/derive_struct_tuple.rs +++ b/soroban-sdk-macros/src/derive_struct_tuple.rs @@ -24,6 +24,11 @@ pub fn derive_type_struct_tuple( let fields = &data.fields; let field_count_usize: usize = fields.len(); + let allow_hash = if let Visibility::Public(_) = vis { + false + } else { + true + }; let (field_specs, field_idx_lits, try_from_xdrs, try_into_xdrs): (Vec<_>, Vec<_>, Vec<_>, Vec<_>) = fields .iter() @@ -36,7 +41,7 @@ pub fn derive_type_struct_tuple( let field_spec = ScSpecUdtStructFieldV0 { doc: docs_from_attrs(&field.attrs).try_into().unwrap(), // TODO: Truncate docs, or display friendly compile error. name: field_name.try_into().unwrap_or_else(|_| StringM::default()), - type_: match map_type(&field.ty) { + type_: match map_type(&field.ty, allow_hash) { Ok(t) => t, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/lib.rs b/soroban-sdk-macros/src/lib.rs index 048bc5f4b..29d471401 100644 --- a/soroban-sdk-macros/src/lib.rs +++ b/soroban-sdk-macros/src/lib.rs @@ -18,7 +18,7 @@ use derive_client::{derive_client_impl, derive_client_type}; use derive_enum::derive_type_enum; use derive_enum_int::derive_type_enum_int; use derive_error_enum_int::derive_type_error_enum_int; -use derive_fn::{derive_contract_function_registration_ctor, derive_fn}; +use derive_fn::{derive_contract_function_registration_ctor, derive_pub_fn}; use derive_spec_fn::derive_fn_spec; use derive_struct::derive_type_struct; use derive_struct_tuple::derive_type_struct_tuple; @@ -231,12 +231,13 @@ pub fn contractimpl(metadata: TokenStream, input: TokenStream) -> TokenStream { .map(|m| { let ident = &m.sig.ident; let call = quote! { ::#ident }; - derive_fn( + derive_pub_fn( &crate_path, &call, ident, &m.attrs, &m.sig.inputs, + &m.sig.output, trait_ident, &client_ident, ) diff --git a/soroban-sdk-macros/src/map_type.rs b/soroban-sdk-macros/src/map_type.rs index 3d0b6c6ee..fd234e0ff 100644 --- a/soroban-sdk-macros/src/map_type.rs +++ b/soroban-sdk-macros/src/map_type.rs @@ -9,7 +9,7 @@ use syn::{ }; #[allow(clippy::too_many_lines)] -pub fn map_type(t: &Type) -> Result { +pub fn map_type(t: &Type, allow_hash: bool) -> Result { match t { Type::Path(TypePath { qself: None, @@ -61,8 +61,8 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Result(Box::new(ScSpecTypeResult { - ok_type: Box::new(map_type(ok)?), - error_type: Box::new(map_type(err)?), + ok_type: Box::new(map_type(ok, allow_hash)?), + error_type: Box::new(map_type(err, allow_hash)?), }))) } "Option" => { @@ -74,7 +74,7 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Option(Box::new(ScSpecTypeOption { - value_type: Box::new(map_type(t)?), + value_type: Box::new(map_type(t, allow_hash)?), }))) } "Vec" => { @@ -86,7 +86,7 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Vec(Box::new(ScSpecTypeVec { - element_type: Box::new(map_type(t)?), + element_type: Box::new(map_type(t, allow_hash)?), }))) } "Map" => { @@ -98,8 +98,8 @@ pub fn map_type(t: &Type) -> Result { ))?, }; Ok(ScSpecTypeDef::Map(Box::new(ScSpecTypeMap { - key_type: Box::new(map_type(k)?), - value_type: Box::new(map_type(v)?), + key_type: Box::new(map_type(k, allow_hash)?), + value_type: Box::new(map_type(v, allow_hash)?), }))) } "BytesN" => { @@ -113,14 +113,21 @@ pub fn map_type(t: &Type) -> Result { Ok(ScSpecTypeDef::BytesN(ScSpecTypeBytesN { n })) } "Hash" => { - let n = match args.as_slice() { - [GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?, - [..] => Err(Error::new( + if allow_hash { + let n = match args.as_slice() { + [GenericArgument::Const(Expr::Lit(ExprLit { lit: Lit::Int(int), .. }))] => int.base10_parse()?, + [..] => Err(Error::new( + t.span(), + "incorrect number of generic arguments, expect one for BytesN", + ))?, + }; + Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) + } else { + Err(Error::new( t.span(), - "incorrect number of generic arguments, expect one for BytesN", - ))?, - }; - Ok(ScSpecTypeDef::Hash(ScSpectTypeHash { n })) + "Hash cannot be used in contexts where there is no guarantee it comes from a secure hash function", + )) + } } _ => Err(Error::new( angle_bracketed.span(), @@ -132,10 +139,12 @@ pub fn map_type(t: &Type) -> Result { } } Type::Tuple(TypeTuple { elems, .. }) => { + let map_type_reject_hash = + |t: &Type| -> Result { map_type(t, false) }; Ok(ScSpecTypeDef::Tuple(Box::new(ScSpecTypeTuple { value_types: elems .iter() - .map(map_type) + .map(map_type_reject_hash) .collect::, Error>>()? // TODO: Implement conversion to VecM from iters to omit this collect. .try_into() .map_err(|e| {