self review

crates/abi/src/builder.rs

@@ -1,4 +1,4 @@
-use crate::elements::{AbiEncoding, Component, Contract, Event, EventField, ModuleAbis};
+use crate::elements::{Component, Contract, Event, EventField, JsonAbi, ModuleAbis};
 use crate::errors::AbiError;
 use fe_analyzer::context::Context;
 use fe_parser::ast as fe;

crates/abi/src/elements.rs

@@ -72,15 +72,15 @@ pub struct AbiComponent {
 }
 
 /// Information relevant to ABI encoding.
-pub trait AbiEncoding {
+pub trait JsonAbi {
     /// Name of the type as it appears in the Json ABI.
     fn abi_json_name(&self) -> String;
 
     /// The components of an ABI tuple.
     fn abi_components(&self) -> Vec<AbiComponent>;
 }
 
-impl AbiEncoding for FixedSize {
+impl JsonAbi for FixedSize {
     fn abi_json_name(&self) -> String {
         match self {
             FixedSize::Array(array) => array.abi_json_name(),
@@ -104,7 +104,7 @@ impl AbiEncoding for FixedSize {
     }
 }
 
-impl AbiEncoding for Base {
+impl JsonAbi for Base {
     fn abi_json_name(&self) -> String {
         match self {
             Base::Numeric(Integer::U256) => "uint256".to_string(),
@@ -130,7 +130,7 @@ impl AbiEncoding for Base {
     }
 }
 
-impl AbiEncoding for Array {
+impl JsonAbi for Array {
     fn abi_json_name(&self) -> String {
         if self.inner == Base::Numeric(Integer::U8) {
             "bytes".to_string()
@@ -144,7 +144,7 @@ impl AbiEncoding for Array {
     }
 }
 
-impl AbiEncoding for Struct {
+impl JsonAbi for Struct {
     fn abi_json_name(&self) -> String {
         "tuple".to_string()
     }
@@ -161,7 +161,7 @@ impl AbiEncoding for Struct {
     }
 }
 
-impl AbiEncoding for Tuple {
+impl JsonAbi for Tuple {
     fn abi_json_name(&self) -> String {
         "tuple".to_string()
     }
@@ -179,7 +179,7 @@ impl AbiEncoding for Tuple {
     }
 }
 
-impl AbiEncoding for FeString {
+impl JsonAbi for FeString {
     fn abi_json_name(&self) -> String {
         "string".to_string()
     }

crates/yulgen/src/operations/revert.rs

@@ -2,11 +2,12 @@ use crate::names;
 use crate::types::AbiType;
 use yultsur::*;
 
-// this is more of an assert operation
+/// Revert with an error message
 pub fn error_revert(typ: &AbiType, msg: yul::Expression) -> yul::Statement {
     revert("Error", typ, msg)
 }
 
+/// Revert with a panic code
 pub fn panic_revert(val: usize) -> yul::Statement {
     revert(
         "Panic",
@@ -15,6 +16,7 @@ pub fn panic_revert(val: usize) -> yul::Statement {
     )
 }
 
+/// Revert with a name and a single value
 pub fn revert(name: &str, typ: &AbiType, val: yul::Expression) -> yul::Statement {
     let func_name = names::revert(name, typ);
     statement! { [func_name]([val]) }

crates/yulgen/src/runtime/abi_dispatcher.rs

@@ -1,6 +1,6 @@
 use crate::names::abi as abi_names;
 use crate::operations::abi as abi_operations;
-use crate::types::{to_abi_types, AbiDecodeLocation, AbiType};
+use crate::types::{to_abi_selector_names, to_abi_types, AbiDecodeLocation, AbiType};
 use fe_abi::utils as abi_utils;
 use fe_analyzer::context::FunctionAttributes;
 use yultsur::*;
@@ -78,11 +78,7 @@ fn dispatch_arm(attributes: FunctionAttributes) -> yul::Case {
 }
 
 fn selector(name: &str, params: &[AbiType]) -> yul::Literal {
-    let params = params
-        .iter()
-        .map(|param| param.selector_name())
-        .collect::<Vec<String>>();
-
+    let params = to_abi_selector_names(params);
     literal! { (abi_utils::func_selector(name, &params)) }
 }
 

crates/yulgen/src/runtime/functions/abi.rs

@@ -13,6 +13,10 @@ pub fn all() -> Vec<yul::Statement> {
         unpack(),
         is_left_padded(),
         is_right_padded(),
+        // This is needed for `revert_with_Panic_uint256`, which is included in the std batch of
+        // revert functions.
+        // It will be removed in https://github.com/ethereum/fe/pull/478 along with all other
+        // batches of runtime functions.
         encode(vec![AbiType::Uint { size: 32 }]),
     ]
 }

crates/yulgen/src/runtime/functions/contracts.rs

@@ -1,7 +1,7 @@
 use crate::names;
 use crate::names::abi as abi_names;
 use crate::operations::abi as abi_operations;
-use crate::types::{to_abi_types, AbiDecodeLocation};
+use crate::types::{to_abi_selector_names, to_abi_types, AbiDecodeLocation, AbiType};
 use fe_abi::utils as abi_utils;
 use fe_analyzer::namespace::types::Contract;
 use yultsur::*;
@@ -21,24 +21,20 @@ pub fn calls(contract: Contract) -> Vec<yul::Statement> {
         .map(|function| {
             // get the name of the call function and its parameters
             let function_name = names::contract_call(&contract_name, &function.name);
-            let param_names = to_abi_types(&function.param_types())
-                .iter()
-                .map(|typ| typ.selector_name())
-                .collect::<Vec<String>>();
+            let param_types = to_abi_types(&function.param_types());
 
             // create a pair of identifiers and expressions for the parameters
             let (param_idents, param_exprs) = abi_names::vals("param", function.params.len());
             // the function selector must be added to the first 4 bytes of the calldata
             let selector = {
-                let selector = abi_utils::func_selector(&function.name, &param_names);
+                let selector =
+                    abi_utils::func_selector(&function.name, &to_abi_selector_names(&param_types));
                 literal_expression! { (selector) }
             };
             // the operations used to encode the parameters
-            let encoding_operation =
-                abi_operations::encode(&to_abi_types(&function.param_types()), param_exprs.clone());
+            let encoding_operation = abi_operations::encode(&param_types, param_exprs.clone());
             // the size of the encoded data
-            let encoding_size =
-                abi_operations::encoding_size(&to_abi_types(&function.param_types()), param_exprs);
+            let encoding_size = abi_operations::encoding_size(&param_types, param_exprs);
 
             if function.return_type.is_unit() {
                 // there is no return data to handle
@@ -52,7 +48,7 @@ pub fn calls(contract: Contract) -> Vec<yul::Statement> {
                 }
             } else {
                 let decoding_operation = abi_operations::decode_data(
-                    &to_abi_types(&[function.return_type]),
+                    &[AbiType::from(&function.return_type)],
                     expression! { outstart },
                     expression! { add(outstart, outsize) },
                     AbiDecodeLocation::Memory,

crates/yulgen/src/runtime/functions/revert.rs

@@ -4,23 +4,22 @@ use crate::types::{to_abi_selector_names, AbiType};
 use yultsur::*;
 
 /// Generate a YUL function to revert with the `Error` signature and the
-/// given set of params.
-/// NOTE: This is currently used for `assert False, "message"` statements which are
-/// encoded as `Error(msg="message")`. This will be removed in the future.
+/// given string.
 pub fn error_revert(typ: &AbiType) -> yul::Statement {
     revert("Error", typ)
 }
 
-/// Generate a YUL function to revert with panic codes
+/// Generate a YUL function to revert with a panic code.
 pub fn panic_revert() -> yul::Statement {
     revert("Panic", &AbiType::Uint { size: 32 })
 }
 
-/// Generate a YUL function to revert with data
+/// Generate a YUL function to revert with any signature name and type.
+/// Note: The parentheses on a tuple are removed in the selector preimage.
 pub fn revert(name: &str, typ: &AbiType) -> yul::Statement {
     let func_name = names::revert(name, typ);
     // the selector parens around a tuple are removed for the selector preimage
-    // e.g. we use `MyError(bool, address)` instead of `MyError(bool, address)`
+    // e.g. we use `MyError(bool, address)` instead of `MyError((bool, address))`
     let selector = {
         let selector_params = match typ.clone() {
             AbiType::Tuple { components } => components,
@@ -41,7 +40,7 @@ pub fn revert(name: &str, typ: &AbiType) -> yul::Statement {
     }
 }
 
-/// Return all revert runtime functions
+/// Return all revert functions used by default.
 pub fn all() -> Vec<yul::Statement> {
     vec![panic_revert()]
 }

crates/yulgen/src/runtime/mod.rs

@@ -31,8 +31,7 @@ pub fn build(context: &Context, contract: &Node<fe::Contract>) -> Vec<yul::State
                 .collect::<Vec<_>>();
 
             let contracts_batch = external_functions
-                .clone()
-                .into_iter()
+                .iter()
                 .map(|function| to_abi_types(&function.param_types()))
                 .collect();
 
@@ -44,8 +43,7 @@ pub fn build(context: &Context, contract: &Node<fe::Contract>) -> Vec<yul::State
 
             let revert_errors_batch = context
                 .revert_errors
-                .clone()
-                .into_iter()
+                .iter()
                 .map(|val| to_abi_types(&val.get_field_types()))
                 .collect::<Vec<_>>();
 

crates/yulgen/src/types.rs

@@ -3,6 +3,7 @@ use fe_analyzer::namespace::types::{
 };
 
 pub trait EvmSized {
+    /// The amount of bytes used by the type when being stored.
     fn size(&self) -> usize;
 }
 
@@ -79,6 +80,7 @@ impl EvmSized for Contract {
     }
 }
 
+/// Solidity ABI type with extra information needed for generation encoding/decoding functions.
 #[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq)]
 pub enum AbiType {
     StaticArray { inner: Box<AbiType>, size: usize },