diff --git a/soroban-sdk-macros/src/derive_client.rs b/soroban-sdk-macros/src/derive_client.rs index c0e160521..c5898caf4 100644 --- a/soroban-sdk-macros/src/derive_client.rs +++ b/soroban-sdk-macros/src/derive_client.rs @@ -10,7 +10,7 @@ use crate::{ fn is_muxed_address_type(arg: &FnArg) -> bool { if let FnArg::Typed(pat_type) = arg { - if let Ok(ScSpecTypeDef::MuxedAddress) = map_type(&pat_type.ty, false, false) { + if let Ok(ScSpecTypeDef::MuxedAddress) = map_type(&pat_type.ty, true, false) { return true; } } diff --git a/soroban-sdk-macros/src/derive_fn.rs b/soroban-sdk-macros/src/derive_fn.rs index 24c8fbacc..fc84ec021 100644 --- a/soroban-sdk-macros/src/derive_fn.rs +++ b/soroban-sdk-macros/src/derive_fn.rs @@ -1,5 +1,7 @@ use crate::{ - attribute::pass_through_attr_to_gen_code, map_type::map_type, syn_ext::ty_to_safe_ident_str, + attribute::pass_through_attr_to_gen_code, + map_type::map_type, + syn_ext::{fn_arg_type_validate_no_mut, ty_to_safe_ident_str}, }; use itertools::MultiUnzip; use proc_macro2::TokenStream as TokenStream2; @@ -63,8 +65,13 @@ pub fn derive_pub_fn( // signature_payload of type Bytes (32 size), to be a Hash. let allow_hash = ident == "__check_auth" && i == 0; + // Error if the type of the fn arg is mutable. + if let Err(e) = fn_arg_type_validate_no_mut(&pat_ty.ty) { + errors.push(e); + } + // Error if the type of the fn is not mappable. - if let Err(e) = map_type(&pat_ty.ty, false, allow_hash) { + if let Err(e) = map_type(&pat_ty.ty, true, allow_hash) { errors.push(e); } @@ -82,7 +89,13 @@ pub fn derive_pub_fn( ty: Box::new(Type::Verbatim(quote! { #crate_path::Val })), }); let passthrough_call = quote! { #ident }; + + let call_prefix = match *pat_ty.ty { + Type::Reference(TypeReference { .. }) => quote!(&), + _ => quote!(), + }; let call = quote! { + #call_prefix <_ as #crate_path::unwrap::UnwrapOptimized>::unwrap_optimized( <_ as #crate_path::TryFromValForContractFn<#crate_path::Env, #crate_path::Val>>::try_from_val_for_contract_fn( &env, diff --git a/soroban-sdk-macros/src/derive_spec_fn.rs b/soroban-sdk-macros/src/derive_spec_fn.rs index da9029832..ae8bae63b 100644 --- a/soroban-sdk-macros/src/derive_spec_fn.rs +++ b/soroban-sdk-macros/src/derive_spec_fn.rs @@ -79,7 +79,7 @@ pub fn derive_fn_spec( // signature_payload of type Bytes (32 size), to be a Hash. let allow_hash = ident == "__check_auth" && i == 0; - match map_type(&pat_type.ty, false, allow_hash) { + match map_type(&pat_type.ty, true, allow_hash) { Ok(type_) => { let name = name.try_into().unwrap_or_else(|_| { const MAX: u32 = 30; @@ -118,7 +118,7 @@ pub fn derive_fn_spec( // Prepare the output. let spec_result = match output { - ReturnType::Type(_, ty) => vec![match map_type(ty, false, true) { + ReturnType::Type(_, ty) => vec![match map_type(ty, true, true) { Ok(spec) => spec, Err(e) => { errors.push(e); diff --git a/soroban-sdk-macros/src/syn_ext.rs b/soroban-sdk-macros/src/syn_ext.rs index ade42af96..6b10f6191 100644 --- a/soroban-sdk-macros/src/syn_ext.rs +++ b/soroban-sdk-macros/src/syn_ext.rs @@ -48,6 +48,21 @@ pub fn fn_arg_ident(arg: &FnArg) -> Result { )) } +/// Validate that the function argument type is not a mutable reference. +/// +/// Returns an error indicating that contract functions arguments cannot be a mutable reference. +/// Even though reference (&) are supported, mutable references (&mut) are not, because it is +/// semanatically confusing for a contract function to receive an external input that it looks like +/// it could mutate. +pub fn fn_arg_type_validate_no_mut(ty: &Type) -> Result<(), Error> { + match ty { + Type::Reference(TypeReference { mutability: Some(_), .. }) => { + Err(Error::new(ty.span(), "mutable references (&mut) are not supported in contract function parameters, use immutable references (&) instead")) + } + _ => Ok(()), + } +} + /// Modifies a Pat removing any 'mut' on an Ident. pub fn pat_unwrap_mut(p: Pat) -> Pat { match p { @@ -68,19 +83,26 @@ pub fn pat_unwrap_mut(p: Pat) -> Pat { } } -/// Returns a clone of the type from the FnArg. +/// Unwraps a reference, returning the type within the reference. +/// +/// If the type is not a reference, returns the type as-is. +pub fn type_unwrap_ref(t: Type) -> Type { + match t { + Type::Reference(TypeReference { elem, .. }) => *elem, + _ => t, + } +} + +/// Returns a clone of the type from the FnArg, converted into an immutable reference to the type +/// with the given lifetime. pub fn fn_arg_ref_type(arg: &FnArg, lifetime: Option<&Lifetime>) -> Result { if let FnArg::Typed(pat_type) = arg { - if !matches!(*pat_type.ty, Type::Reference(_)) { - Ok(Type::Reference(TypeReference { - and_token: And::default(), - lifetime: lifetime.cloned(), - mutability: None, - elem: pat_type.ty.clone(), - })) - } else { - Ok((*pat_type.ty).clone()) - } + Ok(Type::Reference(TypeReference { + and_token: And::default(), + lifetime: lifetime.cloned(), + mutability: None, + elem: Box::new(type_unwrap_ref(*pat_type.ty.clone())), + })) } else { Err(Error::new( arg.span(), @@ -89,23 +111,21 @@ pub fn fn_arg_ref_type(arg: &FnArg, lifetime: Option<&Lifetime>) -> Result) -> FnArg { if let FnArg::Typed(pat_type) = arg { - if !matches!(*pat_type.ty, Type::Reference(_)) { - return FnArg::Typed(PatType { - attrs: pat_type.attrs.clone(), - pat: Box::new(pat_unwrap_mut(*pat_type.pat.clone())), - colon_token: pat_type.colon_token, - ty: Box::new(Type::Reference(TypeReference { - and_token: And::default(), - lifetime: lifetime.cloned(), - mutability: None, - elem: pat_type.ty.clone(), - })), - }); - } + return FnArg::Typed(PatType { + attrs: pat_type.attrs.clone(), + pat: Box::new(pat_unwrap_mut(*pat_type.pat.clone())), + colon_token: pat_type.colon_token, + ty: Box::new(Type::Reference(TypeReference { + and_token: And::default(), + lifetime: lifetime.cloned(), + mutability: None, + elem: Box::new(type_unwrap_ref(*pat_type.ty.clone())), + })), + }); } arg.clone() } diff --git a/soroban-sdk/src/tests/contract_fn.rs b/soroban-sdk/src/tests/contract_fn.rs index 3e0058e8f..e783bef08 100644 --- a/soroban-sdk/src/tests/contract_fn.rs +++ b/soroban-sdk/src/tests/contract_fn.rs @@ -22,6 +22,10 @@ impl Contract { b *= 1; a + b } + + pub fn add_with_ref_arg(_e: &Env, a: i32, b: &i32) -> i32 { + a + b + } } #[contract] @@ -46,6 +50,9 @@ fn test_functional() { let c = ContractClient::new(&e, &contract_id).add_with_mut_arg(&a, &b); assert_eq!(c, 22); + + let c = ContractClient::new(&e, &contract_id).add_with_ref_arg(&a, &b); + assert_eq!(c, 22); } #[test] diff --git a/soroban-sdk/test_snapshots/tests/contract_fn/test_functional.1.json b/soroban-sdk/test_snapshots/tests/contract_fn/test_functional.1.json index 3d940b4d1..721d018c4 100644 --- a/soroban-sdk/test_snapshots/tests/contract_fn/test_functional.1.json +++ b/soroban-sdk/test_snapshots/tests/contract_fn/test_functional.1.json @@ -5,6 +5,7 @@ "mux_id": 0 }, "auth": [ + [], [], [], [] diff --git a/tests-expanded/test_generics_tests.rs b/tests-expanded/test_generics_tests.rs index 088738024..2789eb273 100644 --- a/tests-expanded/test_generics_tests.rs +++ b/tests-expanded/test_generics_tests.rs @@ -140,24 +140,26 @@ impl<'a, 'b> Contract where 'a: 'b, { - pub fn empty() {} + pub fn exec(i1: u32, i2: &u32, i3: &'b u32) -> u32 { + i1 + i2 + *i3 + } } #[doc(hidden)] #[allow(non_snake_case)] -pub mod __Contract__empty__spec { +pub mod __Contract__exec__spec { #[doc(hidden)] #[allow(non_snake_case)] #[allow(non_upper_case_globals)] - pub static __SPEC_XDR_FN_EMPTY: [u8; 28usize] = super::Contract::spec_xdr_empty(); + pub static __SPEC_XDR_FN_EXEC: [u8; 76usize] = super::Contract::spec_xdr_exec(); } impl Contract { #[allow(non_snake_case)] - pub const fn spec_xdr_empty() -> [u8; 28usize] { - *b"\0\0\0\0\0\0\0\0\0\0\0\x05empty\0\0\0\0\0\0\0\0\0\0\0" + pub const fn spec_xdr_exec() -> [u8; 76usize] { + *b"\0\0\0\0\0\0\0\0\0\0\0\x04exec\0\0\0\x03\0\0\0\0\0\0\0\x02i1\0\0\0\0\0\x04\0\0\0\0\0\0\0\x02i2\0\0\0\0\0\x04\0\0\0\0\0\0\0\x02i3\0\0\0\0\0\x04\0\0\0\x01\0\0\0\x04" } } impl<'a> ContractClient<'a> { - pub fn empty(&self) -> () { + pub fn exec(&self, i1: &u32, i2: &u32, i3: &u32) -> u32 { use core::ops::Not; let old_auth_manager = self .env @@ -184,20 +186,30 @@ impl<'a> ContractClient<'a> { &self.address, &{ #[allow(deprecated)] - const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("empty"); + const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("exec"); SYMBOL }, - ::soroban_sdk::Vec::new(&self.env), + ::soroban_sdk::Vec::from_array( + &self.env, + [ + i1.into_val(&self.env), + i2.into_val(&self.env), + i3.into_val(&self.env), + ], + ), ); if let Some(old_auth_manager) = old_auth_manager { self.env.host().set_auth_manager(old_auth_manager).unwrap(); } res } - pub fn try_empty( + pub fn try_exec( &self, + i1: &u32, + i2: &u32, + i3: &u32, ) -> Result< - Result<(), <() as soroban_sdk::TryFromVal>::Error>, + Result>::Error>, Result, > { use core::ops::Not; @@ -222,10 +234,17 @@ impl<'a> ContractClient<'a> { &self.address, &{ #[allow(deprecated)] - const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("empty"); + const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("exec"); SYMBOL }, - ::soroban_sdk::Vec::new(&self.env), + ::soroban_sdk::Vec::from_array( + &self.env, + [ + i1.into_val(&self.env), + i2.into_val(&self.env), + i3.into_val(&self.env), + ], + ), ); if let Some(old_auth_manager) = old_auth_manager { self.env.host().set_auth_manager(old_auth_manager).unwrap(); @@ -236,47 +255,75 @@ impl<'a> ContractClient<'a> { impl ContractArgs { #[inline(always)] #[allow(clippy::unused_unit)] - pub fn empty<'i>() -> () { - () + pub fn exec<'i>(i1: &'i u32, i2: &'i u32, i3: &'i u32) -> (&'i u32, &'i u32, &'i u32) { + (i1, i2, i3) } } #[doc(hidden)] #[allow(non_snake_case)] -pub mod __Contract__empty { +pub mod __Contract__exec { use super::*; - #[deprecated(note = "use `ContractClient::new(&env, &contract_id).empty` instead")] - pub fn invoke_raw(env: soroban_sdk::Env) -> soroban_sdk::Val { + #[deprecated(note = "use `ContractClient::new(&env, &contract_id).exec` instead")] + pub fn invoke_raw( + env: soroban_sdk::Env, + arg_0: soroban_sdk::Val, + arg_1: soroban_sdk::Val, + arg_2: soroban_sdk::Val, + ) -> soroban_sdk::Val { <_ as soroban_sdk::IntoVal>::into_val( #[allow(deprecated)] - &::empty(), + &::exec( + <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_0), + ), + &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_1), + ), + &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_2), + ), + ), &env, ) } - #[deprecated(note = "use `ContractClient::new(&env, &contract_id).empty` instead")] + #[deprecated(note = "use `ContractClient::new(&env, &contract_id).exec` instead")] pub fn invoke_raw_slice(env: soroban_sdk::Env, args: &[soroban_sdk::Val]) -> soroban_sdk::Val { - if args.len() != 0usize { + if args.len() != 3usize { { ::core::panicking::panic_fmt(format_args!( "invalid number of input arguments: {0} expected, got {1}", - 0usize, + 3usize, args.len(), )); }; } #[allow(deprecated)] - invoke_raw(env) + invoke_raw(env, args[0usize], args[1usize], args[2usize]) } - #[deprecated(note = "use `ContractClient::new(&env, &contract_id).empty` instead")] - pub extern "C" fn invoke_raw_extern() -> soroban_sdk::Val { + #[deprecated(note = "use `ContractClient::new(&env, &contract_id).exec` instead")] + pub extern "C" fn invoke_raw_extern( + arg_0: soroban_sdk::Val, + arg_1: soroban_sdk::Val, + arg_2: soroban_sdk::Val, + ) -> soroban_sdk::Val { #[allow(deprecated)] - invoke_raw(soroban_sdk::Env::default()) + invoke_raw(soroban_sdk::Env::default(), arg_0, arg_1, arg_2) } use super::*; } #[doc(hidden)] #[allow(non_snake_case)] #[allow(unused)] -fn __Contract__2e1cfa82b035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be8f10d_ctor() { +fn __Contract__2706c619fe73f0cf112473c6ee02e66c04e1c01c110b0c37b88d8eb509630c9f_ctor() { #[allow(unsafe_code)] { #[link_section = ".init_array"] @@ -288,7 +335,7 @@ fn __Contract__2e1cfa82b035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be8f10d_ #[allow(non_snake_case)] extern "C" fn f() -> ::ctor::__support::CtorRetType { unsafe { - __Contract__2e1cfa82b035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be8f10d_ctor(); + __Contract__2706c619fe73f0cf112473c6ee02e66c04e1c01c110b0c37b88d8eb509630c9f_ctor(); }; core::default::Default::default() } @@ -297,9 +344,9 @@ fn __Contract__2e1cfa82b035c26cbbbdae632cea070514eb8b773f616aaeaf668e2f0be8f10d_ } { ::register( - "empty", + "exec", #[allow(deprecated)] - &__Contract__empty::invoke_raw_slice, + &__Contract__exec::invoke_raw_slice, ); } } @@ -315,9 +362,9 @@ mod test { ignore: false, ignore_message: ::core::option::Option::None, source_file: "tests/generics/src/lib.rs", - start_line: 28usize, + start_line: 30usize, start_col: 8usize, - end_line: 28usize, + end_line: 30usize, end_col: 18usize, compile_fail: false, no_run: false, @@ -333,7 +380,20 @@ mod test { let e = Env::default(); let contract_id = e.register(Contract, ()); let client = ContractClient::new(&e, &contract_id); - client.empty(); + let res = client.exec(&1, &2, &3); + match (&res, &6) { + (left_val, right_val) => { + if !(*left_val == *right_val) { + let kind = ::core::panicking::AssertKind::Eq; + ::core::panicking::assert_failed( + kind, + &*left_val, + &*right_val, + ::core::option::Option::None, + ); + } + } + }; } } #[rustc_main] diff --git a/tests-expanded/test_generics_wasm32v1-none.rs b/tests-expanded/test_generics_wasm32v1-none.rs index 344706dad..908840fc9 100644 --- a/tests-expanded/test_generics_wasm32v1-none.rs +++ b/tests-expanded/test_generics_wasm32v1-none.rs @@ -28,42 +28,54 @@ impl<'a, 'b> Contract where 'a: 'b, { - pub fn empty() {} + pub fn exec(i1: u32, i2: &u32, i3: &'b u32) -> u32 { + i1 + i2 + *i3 + } } #[doc(hidden)] #[allow(non_snake_case)] -pub mod __Contract__empty__spec { +pub mod __Contract__exec__spec { #[doc(hidden)] #[allow(non_snake_case)] #[allow(non_upper_case_globals)] #[link_section = "contractspecv0"] - pub static __SPEC_XDR_FN_EMPTY: [u8; 28usize] = super::Contract::spec_xdr_empty(); + pub static __SPEC_XDR_FN_EXEC: [u8; 76usize] = super::Contract::spec_xdr_exec(); } impl Contract { #[allow(non_snake_case)] - pub const fn spec_xdr_empty() -> [u8; 28usize] { - *b"\0\0\0\0\0\0\0\0\0\0\0\x05empty\0\0\0\0\0\0\0\0\0\0\0" + pub const fn spec_xdr_exec() -> [u8; 76usize] { + *b"\0\0\0\0\0\0\0\0\0\0\0\x04exec\0\0\0\x03\0\0\0\0\0\0\0\x02i1\0\0\0\0\0\x04\0\0\0\0\0\0\0\x02i2\0\0\0\0\0\x04\0\0\0\0\0\0\0\x02i3\0\0\0\0\0\x04\0\0\0\x01\0\0\0\x04" } } impl<'a> ContractClient<'a> { - pub fn empty(&self) -> () { + pub fn exec(&self, i1: &u32, i2: &u32, i3: &u32) -> u32 { use core::ops::Not; use soroban_sdk::{FromVal, IntoVal}; let res = self.env.invoke_contract( &self.address, &{ #[allow(deprecated)] - const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("empty"); + const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("exec"); SYMBOL }, - ::soroban_sdk::Vec::new(&self.env), + ::soroban_sdk::Vec::from_array( + &self.env, + [ + i1.into_val(&self.env), + i2.into_val(&self.env), + i3.into_val(&self.env), + ], + ), ); res } - pub fn try_empty( + pub fn try_exec( &self, + i1: &u32, + i2: &u32, + i3: &u32, ) -> Result< - Result<(), <() as soroban_sdk::TryFromVal>::Error>, + Result>::Error>, Result, > { use soroban_sdk::{FromVal, IntoVal}; @@ -71,10 +83,17 @@ impl<'a> ContractClient<'a> { &self.address, &{ #[allow(deprecated)] - const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("empty"); + const SYMBOL: soroban_sdk::Symbol = soroban_sdk::Symbol::short("exec"); SYMBOL }, - ::soroban_sdk::Vec::new(&self.env), + ::soroban_sdk::Vec::from_array( + &self.env, + [ + i1.into_val(&self.env), + i2.into_val(&self.env), + i3.into_val(&self.env), + ], + ), ); res } @@ -82,27 +101,55 @@ impl<'a> ContractClient<'a> { impl ContractArgs { #[inline(always)] #[allow(clippy::unused_unit)] - pub fn empty<'i>() -> () { - () + pub fn exec<'i>(i1: &'i u32, i2: &'i u32, i3: &'i u32) -> (&'i u32, &'i u32, &'i u32) { + (i1, i2, i3) } } #[doc(hidden)] #[allow(non_snake_case)] -pub mod __Contract__empty { +pub mod __Contract__exec { use super::*; - #[deprecated(note = "use `ContractClient::new(&env, &contract_id).empty` instead")] - pub fn invoke_raw(env: soroban_sdk::Env) -> soroban_sdk::Val { + #[deprecated(note = "use `ContractClient::new(&env, &contract_id).exec` instead")] + pub fn invoke_raw( + env: soroban_sdk::Env, + arg_0: soroban_sdk::Val, + arg_1: soroban_sdk::Val, + arg_2: soroban_sdk::Val, + ) -> soroban_sdk::Val { <_ as soroban_sdk::IntoVal>::into_val( #[allow(deprecated)] - &::empty(), + &::exec( + <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_0), + ), + &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_1), + ), + &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized( + <_ as soroban_sdk::TryFromValForContractFn< + soroban_sdk::Env, + soroban_sdk::Val, + >>::try_from_val_for_contract_fn(&env, &arg_2), + ), + ), &env, ) } - #[deprecated(note = "use `ContractClient::new(&env, &contract_id).empty` instead")] - #[export_name = "empty"] - pub extern "C" fn invoke_raw_extern() -> soroban_sdk::Val { + #[deprecated(note = "use `ContractClient::new(&env, &contract_id).exec` instead")] + #[export_name = "exec"] + pub extern "C" fn invoke_raw_extern( + arg_0: soroban_sdk::Val, + arg_1: soroban_sdk::Val, + arg_2: soroban_sdk::Val, + ) -> soroban_sdk::Val { #[allow(deprecated)] - invoke_raw(soroban_sdk::Env::default()) + invoke_raw(soroban_sdk::Env::default(), arg_0, arg_1, arg_2) } use super::*; } diff --git a/tests/generics/src/lib.rs b/tests/generics/src/lib.rs index 6827bef19..64bb2ee80 100644 --- a/tests/generics/src/lib.rs +++ b/tests/generics/src/lib.rs @@ -15,7 +15,9 @@ impl<'a, 'b> Contract where 'a: 'b, { - pub fn empty() {} + pub fn exec(i1: u32, i2: &u32, i3: &'b u32) -> u32 { + i1 + i2 + *i3 + } } #[cfg(test)] @@ -30,6 +32,7 @@ mod test { let contract_id = e.register(Contract, ()); let client = ContractClient::new(&e, &contract_id); - client.empty(); + let res = client.exec(&1, &2, &3); + assert_eq!(res, 6); } }