Repository: huff-language/huff-rs (rust)

Issue:
Incorrect usage of macro label not throwing a correct error
The incorrect usage of a macro label in the marco definition results in the compiler not throwing an error and instead using up all system memory and then exiting.

Contract to reproduce:
```
#define macro TEST(<value>) = takes(0) returns(0) {
    <value> 0x00 mstore
    0x20 0x00 return
}

#define macro MAIN() = takes(0) returns(0) {
    TEST(0x01)
}
```

Note the usage of definition of the macro `TEST` using `<value>` instead of `value` 

Pull request description:
fix: Invalid Parameter Types
## Overview

Previously, passing an unacceptable token as a parameter to a macro would cause the parser to loop indefinitely as it tried to find a closing parenthesis.

This pr introduces a check in parameter parsing that makes sure some sort of parameter was parsed (type or name), otherwise we have an invalid token.

Closes #178

Patch:
diff --git a/huff_parser/src/lib.rs b/huff_parser/src/lib.rs
index f559334..f7bd049 100644
--- a/huff_parser/src/lib.rs
+++ b/huff_parser/src/lib.rs
@@ -103,12 +103,12 @@ impl Parser {
                     _ => {
                         tracing::error!(
                             target: "parser",
-                            "Invalid definition. Must be a function, event, constant, error, or macro. Got: {}",
+                            "Invalid definition. Must be a function, event, constant, or macro. Got: {}",
                             self.current_token.kind
                         );
                         return Err(ParserError {
                             kind: ParserErrorKind::InvalidDefinition(self.current_token.kind.clone()),
-                            hint: Some("Definition must be one of: `function`, `event`, `constant`, `error`, `macro`, or `fn`.".to_string()),
+                            hint: Some("Definition must be one of: `function`, `event`, `constant`, `macro`, or `fn`.".to_string()),
                             spans: AstSpan(vec![self.current_token.span.clone()]),
                         })
                     }
@@ -688,6 +688,7 @@ impl Parser {
     ) -> Result<Vec<Argument>, ParserError> {
         let mut args: Vec<Argument> = Vec::new();
         self.match_kind(TokenKind::OpenParen)?;
+        tracing::debug!(target: "parser", "PARSING ARGs: {:?}", self.current_token.kind);
         while !self.check(TokenKind::CloseParen) {
             // The builtin functions `__FUNC_SIG` and `__EVENT_HASH` can accept a single string as
             // input. If the `is_builtin` flag was passed, check to see if a single
@@ -737,6 +738,16 @@ impl Parser {
                 self.consume();
             }
 
+            // If both arg type and arg name are none, we didn't consume anything
+            if arg.arg_type.is_none() && arg.name.is_none() {
+                tracing::error!(target: "parser", "INVALID ARGUMENT TOKEN: {:?}", self.current_token.kind);
+                return Err(ParserError {
+                    kind: ParserErrorKind::InvalidArgs(self.current_token.kind.clone()),
+                    hint: None,
+                    spans: AstSpan(vec![self.current_token.span.clone()]),
+                })
+            }
+
             arg.span = AstSpan(arg_spans);
 
             args.push(arg);
diff --git a/huff_utils/src/evm.rs b/huff_utils/src/evm.rs
index 6cc7f2f..80ed568 100644
--- a/huff_utils/src/evm.rs
+++ b/huff_utils/src/evm.rs
@@ -6,7 +6,7 @@ use strum_macros::EnumString;
 /// They are arranged in a particular order such that all the opcodes that have common
 /// prefixes are ordered by decreasing length to avoid mismatch when lexing.
 /// Example : [origin, or] or [push32, ..., push3]
-pub const OPCODES: [&str; 143] = [
+pub const OPCODES: [&str; 142] = [
     "lt",
     "gt",
     "slt",
@@ -28,7 +28,6 @@ pub const OPCODES: [&str; 143] = [
     "calldatacopy",
     "codesize",
     "codecopy",
-    "basefee",
     "blockhash",
     "coinbase",
     "timestamp",
@@ -175,7 +174,6 @@ pub static OPCODES_MAP: phf::Map<&'static str, Opcode> = phf_map! {
     "calldatacopy" => Opcode::Calldatacopy,
     "codesize" => Opcode::Codesize,
     "codecopy" => Opcode::Codecopy,
-    "basefee" => Opcode::Basefee,
     "blockhash" => Opcode::Blockhash,
     "coinbase" => Opcode::Coinbase,
     "timestamp" => Opcode::Timestamp,
@@ -626,6 +624,7 @@ impl Opcode {
             Opcode::Shr => "1c",
             Opcode::Sar => "1d",
             Opcode::Sha3 => "20",
+            // Opcode::Keccak => "20",
             Opcode::Address => "30",
             Opcode::Balance => "31",
             Opcode::Origin => "32",


Test patch:
diff --git a/huff_core/tests/parser_errors.rs b/huff_core/tests/parser_errors.rs
index a70776b..18831bd 100644
--- a/huff_core/tests/parser_errors.rs
+++ b/huff_core/tests/parser_errors.rs
@@ -95,7 +95,7 @@ fn test_invalid_definition() {
                 ParserError {
                     kind: ParserErrorKind::InvalidDefinition(TokenKind::Ident("test".to_string())),
                     hint: Some(
-                        "Definition must be one of: `function`, `event`, `constant`, `error`, `macro`, or `fn`."
+                        "Definition must be one of: `function`, `event`, `constant`, `macro`, or `fn`."
                             .to_string()
                     ),
                     spans: AstSpan(vec![Span {
diff --git a/huff_parser/tests/breaking_param_invocation.rs b/huff_parser/tests/breaking_param_invocation.rs
new file mode 100644
index 0000000..8da26a1
--- /dev/null
+++ b/huff_parser/tests/breaking_param_invocation.rs
@@ -0,0 +1,53 @@
+use huff_lexer::*;
+use huff_parser::*;
+use huff_utils::prelude::*;
+
+#[test]
+#[should_panic]
+fn test_breaking_param_invocation() {
+    let source: &str = r#"
+        // Here we have a macro invocation directly in the parameter list - this should fail
+        #define macro TEST(<value>) = takes(0) returns(0) {
+            <value> 0x00 mstore
+            0x20 0x00 return
+        }
+
+        #define macro MAIN() = takes(0) returns(0) {
+            TEST(0x01)
+        }
+    "#;
+
+    // Parse tokens
+    let flattened_source = FullFileSource { source, file: None, spans: vec![] };
+    let lexer = Lexer::new(flattened_source);
+    let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::<Vec<Token>>();
+    let mut parser = Parser::new(tokens, None);
+
+    // Should fail here
+    parser.parse().unwrap();
+}
+
+#[test]
+#[should_panic]
+fn test_breaking_param_commas() {
+    let source: &str = r#"
+        // Here we have a macro invocation directly in the parameter list - this should fail
+        #define macro TEST(t,,) = takes(0) returns(0) {
+            <value> 0x00 mstore
+            0x20 0x00 return
+        }
+
+        #define macro MAIN() = takes(0) returns(0) {
+            TEST(0x01)
+        }
+    "#;
+
+    // Parse tokens
+    let flattened_source = FullFileSource { source, file: None, spans: vec![] };
+    let lexer = Lexer::new(flattened_source);
+    let tokens = lexer.into_iter().map(|x| x.unwrap()).collect::<Vec<Token>>();
+    let mut parser = Parser::new(tokens, None);
+
+    // Should fail here
+    parser.parse().unwrap();
+}