Allow ref (&) parameters in contract fns

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;
         }
     }

soroban-sdk-macros/src/derive_fn.rs

@@ -64,7 +64,7 @@ pub fn derive_pub_fn(
                 let allow_hash = ident == "__check_auth" && i == 0;
 
                 // 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 +82,14 @@ 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 { mutability: Some(_), .. }) => quote!(&mut),
+                    Type::Reference(TypeReference { mutability: None, .. }) => 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,

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);

soroban-sdk-macros/src/syn_ext.rs

@@ -5,8 +5,8 @@ use syn::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
     token::Comma,
-    AngleBracketedGenericArguments, Attribute, GenericArgument, Path, PathArguments, PathSegment,
-    ReturnType, Token, TypePath,
+    AngleBracketedGenericArguments, Attribute, GenericArgument, PatIdent, Path, PathArguments,
+    PathSegment, ReturnType, Token, TypePath,
 };
 use syn::{
     spanned::Spanned, token::And, Error, FnArg, Ident, ImplItem, ImplItemFn, ItemImpl, ItemTrait,
@@ -48,19 +48,46 @@ pub fn fn_arg_ident(arg: &FnArg) -> Result<Ident, Error> {
     ))
 }
 
-/// 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,
+    }
+}
+
+/// Modifies a Pat removing any 'mut' on an Ident.
+pub fn pat_unwrap_mut(i: Pat) -> Pat {
+    match i {
+        Pat::Ident(PatIdent {
+            attrs,
+            by_ref,
+            mutability: Some(_),
+            ident,
+            subpat,
+        }) => Pat::Ident(PatIdent {
+            attrs,
+            by_ref,
+            mutability: None,
+            ident,
+            subpat,
+        }),
+        _ => i,
+    }
+}
+
+/// 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<Type, Error> {
     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(),
@@ -69,25 +96,19 @@ pub fn fn_arg_ref_type(arg: &FnArg, lifetime: Option<&Lifetime>) -> Result<Type,
     }
 }
 
-/// Returns a clone of FnArg with the type as a reference if the arg is a typed
-/// arg and its type is not already a reference.
+/// Returns a clone of FnArg, converted into an immutable reference with the given lifetime.
 pub fn fn_arg_make_ref(arg: &FnArg, lifetime: Option<&Lifetime>) -> FnArg {
-    if let FnArg::Typed(pat_type) = arg {
-        if !matches!(*pat_type.ty, Type::Reference(_)) {
+    match arg {
+        FnArg::Typed(pat_type) => {
             return FnArg::Typed(PatType {
                 attrs: pat_type.attrs.clone(),
-                pat: pat_type.pat.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(),
-                })),
+                ty: Box::new(fn_arg_ref_type(arg, lifetime)),
             });
         }
+        _ => arg.clone(),
     }
-    arg.clone()
 }
 
 pub fn fn_arg_make_into(arg: &FnArg) -> FnArg {

soroban-sdk/src/tests/contract_fn.rs

@@ -14,6 +14,20 @@ impl Contract {
         a + b
     }
 
+    pub fn add_with_ref_arg(_e: &Env, a: i32, b: &i32) -> i32 {
+        a + b
+    }
+
+    pub fn add_with_mut_ref_arg(_e: &Env, a: i32, b: &mut i32) -> i32 {
+        *b += 2;
+        a + *b
+    }
+
+    pub fn add_with_mut_arg(_e: &Env, a: i32, mut b: i32) -> i32 {
+        b += 2;
+        a + b
+    }
+
     pub fn add_with_unused_arg(_e: &Env, a: i32, _b: i32) -> i32 {
         a + 2
     }

tests-expanded/test_contract_data_tests.rs

@@ -137,14 +137,14 @@ impl soroban_sdk::testutils::ContractFunctionSet for Contract {
     }
 }
 impl Contract {
-    pub fn put(e: Env, key: Symbol, val: Symbol) {
-        e.storage().persistent().set(&key, &val)
+    pub fn put(e: Env, key: &Symbol, val: &Symbol) {
+        e.storage().persistent().set(key, val)
     }
-    pub fn get(e: Env, key: Symbol) -> Option<Symbol> {
-        e.storage().persistent().get(&key)
+    pub fn get(e: Env, key: &Symbol) -> Option<Symbol> {
+        e.storage().persistent().get(key)
     }
-    pub fn del(e: Env, key: Symbol) {
-        e.storage().persistent().remove(&key)
+    pub fn del(e: Env, key: &Symbol) {
+        e.storage().persistent().remove(key)
     }
 }
 #[doc(hidden)]
@@ -460,13 +460,13 @@ pub mod __Contract__put {
             #[allow(deprecated)]
             &<super::Contract>::put(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ 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::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,
@@ -510,7 +510,7 @@ pub mod __Contract__get {
             #[allow(deprecated)]
             &<super::Contract>::get(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,
@@ -551,7 +551,7 @@ pub mod __Contract__del {
             #[allow(deprecated)]
             &<super::Contract>::del(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,

tests-expanded/test_contract_data_wasm32v1-none.rs

@@ -25,14 +25,14 @@ impl<'a> ContractClient<'a> {
     }
 }
 impl Contract {
-    pub fn put(e: Env, key: Symbol, val: Symbol) {
-        e.storage().persistent().set(&key, &val)
+    pub fn put(e: Env, key: &Symbol, val: &Symbol) {
+        e.storage().persistent().set(key, val)
     }
-    pub fn get(e: Env, key: Symbol) -> Option<Symbol> {
-        e.storage().persistent().get(&key)
+    pub fn get(e: Env, key: &Symbol) -> Option<Symbol> {
+        e.storage().persistent().get(key)
     }
-    pub fn del(e: Env, key: Symbol) {
-        e.storage().persistent().remove(&key)
+    pub fn del(e: Env, key: &Symbol) {
+        e.storage().persistent().remove(key)
     }
 }
 #[doc(hidden)]
@@ -222,13 +222,13 @@ pub mod __Contract__put {
             #[allow(deprecated)]
             &<super::Contract>::put(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ 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::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,
@@ -259,7 +259,7 @@ pub mod __Contract__get {
             #[allow(deprecated)]
             &<super::Contract>::get(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,
@@ -287,7 +287,7 @@ pub mod __Contract__del {
             #[allow(deprecated)]
             &<super::Contract>::del(
                 env.clone(),
-                <_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
+                &<_ as soroban_sdk::unwrap::UnwrapOptimized>::unwrap_optimized(
                     <_ as soroban_sdk::TryFromValForContractFn<
                         soroban_sdk::Env,
                         soroban_sdk::Val,