Set function access in Json ABI based on presence of ctx and self par…

…ameter (#783)

Co-authored-by: Sean Billig <sean.billig@gmail.com>

crates/abi/src/function.rs

@@ -14,6 +14,40 @@ pub enum StateMutability {
     Payable,
 }
 
+pub enum SelfParam {
+    None,
+    Imm,
+    Mut,
+}
+pub enum CtxParam {
+    None,
+    Imm,
+    Mut,
+}
+
+impl StateMutability {
+    pub fn from_self_and_ctx_params(self_: SelfParam, ctx: CtxParam) -> Self {
+        // Check ABI conformity
+        // See https://github.com/ethereum/fe/issues/558
+        //
+        //              no self   |   self   |  mut self  |
+        //           ......................................
+        // no ctx    :    pure    |   view    |  payable  |
+        // ctx       :    view    |   view    |  payable  |
+        // mut ctx   :   payable  |  payable  |  payable  |
+
+        match (self_, ctx) {
+            (SelfParam::None, CtxParam::None) => StateMutability::Pure,
+            (SelfParam::None, CtxParam::Imm) => StateMutability::View,
+            (SelfParam::None, CtxParam::Mut) => StateMutability::Payable,
+            (SelfParam::Imm, CtxParam::None) => StateMutability::View,
+            (SelfParam::Imm, CtxParam::Imm) => StateMutability::View,
+            (SelfParam::Imm, CtxParam::Mut) => StateMutability::Payable,
+            (SelfParam::Mut, _) => StateMutability::Payable,
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 pub struct AbiFunction {
     #[serde(rename = "type")]
@@ -31,6 +65,7 @@ impl AbiFunction {
         name: String,
         args: Vec<(String, AbiType)>,
         ret_ty: Option<AbiType>,
+        state_mutability: StateMutability,
     ) -> Self {
         let inputs = args
             .into_iter()
@@ -45,10 +80,7 @@ impl AbiFunction {
             name,
             inputs,
             outputs,
-            // In the future we will derive that based on the fact whether `self` or `ctx` are taken as `mut` or not.
-            // For now, we default to payable so that tooling such as hardhat simply assumes all functions need to be
-            // called with a transaction.
-            state_mutability: StateMutability::Payable,
+            state_mutability,
         }
     }
 
@@ -131,7 +163,8 @@ mod tests {
 
         AbiType::Tuple(vec![field1, field2])
     }
-    fn test_func() -> AbiFunction {
+
+    fn test_func(state_mutability: StateMutability) -> AbiFunction {
         let i32_ty = AbiType::Int(32);
         let tuple_ty = simple_tuple();
         let u64_ty = AbiType::UInt(64);
@@ -141,12 +174,13 @@ mod tests {
             "test_func".into(),
             vec![("arg1".into(), i32_ty), ("arg2".into(), tuple_ty)],
             Some(u64_ty),
+            state_mutability,
         )
     }
 
     #[test]
     fn serialize_func() {
-        let func = test_func();
+        let func = test_func(StateMutability::Payable);
 
         assert_ser_tokens(
             &func,
@@ -211,9 +245,57 @@ mod tests {
         )
     }
 
+    #[test]
+    fn test_state_mutability() {
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::None, CtxParam::None),
+            StateMutability::Pure
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::None, CtxParam::Imm),
+            StateMutability::View
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::None, CtxParam::Mut),
+            StateMutability::Payable
+        );
+
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Imm, CtxParam::None),
+            StateMutability::View
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Imm, CtxParam::Imm),
+            StateMutability::View
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Imm, CtxParam::Mut),
+            StateMutability::Payable
+        );
+
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Mut, CtxParam::None),
+            StateMutability::Payable
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Mut, CtxParam::Imm),
+            StateMutability::Payable
+        );
+        assert_eq!(
+            StateMutability::from_self_and_ctx_params(SelfParam::Mut, CtxParam::Mut),
+            StateMutability::Payable
+        );
+
+        let pure_func = test_func(StateMutability::Pure);
+        assert_eq!(pure_func.state_mutability, StateMutability::Pure);
+
+        let impure_func = test_func(StateMutability::Payable);
+        assert_eq!(impure_func.state_mutability, StateMutability::Payable);
+    }
+
     #[test]
     fn func_selector() {
-        let func = test_func();
+        let func = test_func(StateMutability::Payable);
         let selector = func.selector();
 
         debug_assert_eq!(

crates/analyzer/src/db/queries/functions.rs

@@ -6,7 +6,7 @@ use crate::namespace::items::{
     DepGraph, DepGraphWrapper, DepLocality, FunctionId, FunctionSigId, Item, TypeDef,
 };
 use crate::namespace::scopes::{BlockScope, BlockScopeType, FunctionScope, ItemScope};
-use crate::namespace::types::{self, Generic, SelfDecl, Type, TypeId};
+use crate::namespace::types::{self, CtxDecl, Generic, SelfDecl, Type, TypeId};
 use crate::traversal::functions::traverse_statements;
 use crate::traversal::types::{type_desc, type_desc_to_trait};
 use fe_common::diagnostics::Label;
@@ -29,6 +29,7 @@ pub fn function_signature(
     let fn_parent = function.parent(db);
 
     let mut self_decl = None;
+    let mut ctx_decl = None;
     let mut names = HashMap::new();
     let mut labels = HashMap::new();
 
@@ -149,6 +150,9 @@ pub fn function_signature(
                                 "`ctx: Context` must be the first parameter",
                             );
                         }
+                        else {
+                            ctx_decl = Some(CtxDecl {span: arg.span,  mut_: *mut_})
+                        }
                     }
                 }
 
@@ -237,6 +241,7 @@ pub fn function_signature(
     Analysis {
         value: Rc::new(types::FunctionSignature {
             self_decl,
+            ctx_decl,
             params,
             return_type,
         }),

crates/analyzer/src/namespace/types.rs

@@ -353,6 +353,7 @@ pub struct FeString {
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct FunctionSignature {
     pub self_decl: Option<SelfDecl>,
+    pub ctx_decl: Option<CtxDecl>,
     pub params: Vec<FunctionParam>,
     pub return_type: Result<TypeId, TypeError>,
 }
@@ -362,12 +363,19 @@ pub struct SelfDecl {
     pub span: Span,
     pub mut_: Option<Span>,
 }
+
 impl SelfDecl {
     pub fn is_mut(&self) -> bool {
         self.mut_.is_some()
     }
 }
 
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+pub struct CtxDecl {
+    pub span: Span,
+    pub mut_: Option<Span>,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct FunctionParam {
     label: Option<SmolStr>,
@@ -768,6 +776,7 @@ impl DisplayWithDb for FunctionSignature {
     fn format(&self, db: &dyn AnalyzerDb, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let FunctionSignature {
             self_decl,
+            ctx_decl: _,
             params,
             return_type,
         } = self;

crates/codegen/src/db/queries/abi.rs

@@ -1,10 +1,16 @@
 use fe_abi::{
     contract::AbiContract,
     event::{AbiEvent, AbiEventField},
-    function::{AbiFunction, AbiFunctionType},
+    function::{AbiFunction, AbiFunctionType, CtxParam, SelfParam, StateMutability},
     types::{AbiTupleField, AbiType},
 };
-use fe_analyzer::{constants::INDEXED, namespace::items::ContractId};
+use fe_analyzer::{
+    constants::INDEXED,
+    namespace::{
+        items::ContractId,
+        types::{CtxDecl, SelfDecl},
+    },
+};
 use fe_mir::ir::{self, FunctionId, TypeId};
 
 use crate::db::CodegenDb;
@@ -58,7 +64,27 @@ pub fn abi_function(db: &dyn CodegenDb, function: FunctionId) -> AbiFunction {
         AbiFunctionType::Function
     };
 
-    AbiFunction::new(func_type, name.to_string(), args, ret_ty)
+    // The "stateMutability" field is derived from the presence & mutability of
+    // `self` and `ctx` params in the analyzer fn sig.
+    let analyzer_sig = sig.analyzer_func_id.signature(db.upcast());
+    let self_param = match analyzer_sig.self_decl {
+        None => SelfParam::None,
+        Some(SelfDecl { mut_: None, .. }) => SelfParam::Imm,
+        Some(SelfDecl { mut_: Some(_), .. }) => SelfParam::Mut,
+    };
+    let ctx_param = match analyzer_sig.ctx_decl {
+        None => CtxParam::None,
+        Some(CtxDecl { mut_: None, .. }) => CtxParam::Imm,
+        Some(CtxDecl { mut_: Some(_), .. }) => CtxParam::Mut,
+    };
+
+    AbiFunction::new(
+        func_type,
+        name.to_string(),
+        args,
+        ret_ty,
+        StateMutability::from_self_and_ctx_params(self_param, ctx_param),
+    )
 }
 
 pub fn abi_function_argument_maximum_size(db: &dyn CodegenDb, function: FunctionId) -> usize {

crates/codegen/src/yul/runtime/revert.rs

@@ -5,7 +5,7 @@ use crate::{
 
 use super::{DefaultRuntimeProvider, RuntimeFunction, RuntimeProvider};
 
-use fe_abi::function::{AbiFunction, AbiFunctionType};
+use fe_abi::function::{AbiFunction, AbiFunctionType, StateMutability};
 use fe_mir::ir::{self, TypeId};
 use yultsur::*;
 
@@ -76,8 +76,15 @@ fn type_signature_for_revert(db: &dyn CodegenDb, name: &str, ty: TypeId) -> yul:
         }
     };
 
-    let selector =
-        AbiFunction::new(AbiFunctionType::Function, name.to_string(), args, None).selector();
+    // selector and state mutability is independent we can set has_self and has_ctx any value.
+    let selector = AbiFunction::new(
+        AbiFunctionType::Function,
+        name.to_string(),
+        args,
+        None,
+        StateMutability::Pure,
+    )
+    .selector();
     let type_sig = selector.hex();
     literal_expression! {(format!{"0x{}", type_sig })}
 }

crates/mir/src/ir/function.rs

@@ -25,7 +25,6 @@ pub struct FunctionSignature {
     pub module_id: analyzer_items::ModuleId,
     pub analyzer_func_id: analyzer_items::FunctionId,
     pub linkage: Linkage,
-    pub has_self: bool,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]

crates/mir/src/lower/function.rs

@@ -42,9 +42,8 @@ pub fn lower_monomorphized_func_signature(
 ) -> FunctionId {
     // TODO: Remove this when an analyzer's function signature contains `self` type.
     let mut params = vec![];
-    let has_self = func.takes_self(db.upcast());
 
-    if has_self {
+    if func.takes_self(db.upcast()) {
         let self_ty = func.self_type(db.upcast()).unwrap();
         let source = self_arg_source(db, func);
         params.push(make_param(db, "self", self_ty, source));
@@ -83,7 +82,6 @@ pub fn lower_monomorphized_func_signature(
         module_id: func.module(db.upcast()),
         analyzer_func_id: func,
         linkage,
-        has_self,
     };
 
     db.mir_intern_function(sig.into())

newsfragments/783.feature.md

@@ -0,0 +1,16 @@
+When the Fe compiler generates a JSON ABI file for a contract, the
+"stateMutability" field for each function now reflects whether the function can
+read or modify chain or contract state, based on the presence or absence of the
+`self` and `ctx` parameters, and whether those parameters are `mut`able.
+
+If a function doesn't take `self` or `ctx`, it's "pure".
+If a function takes `self` or `ctx` immutably, it can read state but not mutate
+state, so it's a "view"
+If a function takes `mut self` or `mut ctx`, it can mutate state, and is thus
+marked "payable".
+
+Note that we're following the convention set by Solidity for this field, which
+isn't a perfect fit for Fe. The primary issue is that Fe doesn't currently
+distinguish between "payable" and "nonpayable" functions; if you want a function
+to revert when Eth is sent, you need to do it manually
+(eg `assert ctx.msg_value() == 0`).