fix(cast): parse tempo scope signatures

Author: mattsse

crates/cast/src/cmd/tempo_policy_args.rs

@@ -1,5 +1,7 @@
-use alloy_primitives::{Address, hex, keccak256};
+use alloy_primitives::{Address, hex};
 use foundry_cli::utils::parse_fee_token_address;
+use foundry_common::abi::get_func;
+use std::borrow::Cow;
 use tempo_contracts::precompiles::{
     IAccountKeychain::{CallScope, SelectorRule},
     PATH_USD_ADDRESS,
@@ -25,6 +27,10 @@ impl SelectorArg {
 /// (`transfer(address,uint256)`), or a well-known TIP-20 shorthand.
 pub(crate) fn parse_selector_bytes(s: &str) -> Result<[u8; 4], String> {
     let s = s.trim();
+    if s.is_empty() {
+        return Err("selector cannot be empty".to_string());
+    }
+
     if s.starts_with("0x") || s.starts_with("0X") {
         let hex_str = &s[2..];
         if hex_str.len() != 8 {
@@ -35,24 +41,23 @@ pub(crate) fn parse_selector_bytes(s: &str) -> Result<[u8; 4], String> {
         arr.copy_from_slice(&bytes);
         Ok(arr)
     } else {
-        let sig = if s.contains('(') {
-            s.to_string()
+        let sig = if s.contains('(') || s.contains(')') {
+            Cow::Borrowed(s)
         } else {
             match s {
-                "transfer" => "transfer(address,uint256)".to_string(),
-                "approve" => "approve(address,uint256)".to_string(),
-                "transferFrom" => "transferFrom(address,address,uint256)".to_string(),
-                "transferWithMemo" => "transferWithMemo(address,uint256,bytes32)".to_string(),
+                "transfer" => Cow::Borrowed("transfer(address,uint256)"),
+                "approve" => Cow::Borrowed("approve(address,uint256)"),
+                "transferFrom" => Cow::Borrowed("transferFrom(address,address,uint256)"),
+                "transferWithMemo" => Cow::Borrowed("transferWithMemo(address,uint256,bytes32)"),
                 "transferFromWithMemo" => {
-                    "transferFromWithMemo(address,address,uint256,bytes32)".to_string()
+                    Cow::Borrowed("transferFromWithMemo(address,address,uint256,bytes32)")
                 }
-                _ => format!("{s}()"),
+                _ => Cow::Owned(format!("{s}()")),
             }
         };
-        let hash = keccak256(sig.as_bytes());
-        let mut arr = [0u8; 4];
-        arr.copy_from_slice(&hash[..4]);
-        Ok(arr)
+        get_func(&sig)
+            .map(|func| func.selector().into())
+            .map_err(|e| format!("invalid function signature '{sig}': {e}"))
     }
 }
 
@@ -73,18 +78,32 @@ pub(crate) fn parse_scope(s: &str) -> Result<CallScope, String> {
 
     let selector_rules = match selectors_str {
         None => vec![],
+        Some(sel_str) if sel_str.trim().is_empty() => {
+            return Err(
+                "selector list cannot be empty; omit ':' to allow all selectors".to_string()
+            );
+        }
         Some(sel_str) => parse_selector_rules(sel_str)?,
     };
 
     Ok(CallScope { target, selectorRules: selector_rules })
 }
 
 fn parse_selector_rules(s: &str) -> Result<Vec<SelectorRule>, String> {
-    let mut rules = Vec::new();
+    let mut rules: Vec<SelectorRule> = Vec::new();
 
-    for part in s.split(',') {
+    for part in split_selector_rule_parts(s)? {
         let part = part.trim();
         if part.is_empty() {
+            return Err(format!("empty selector in scope '{s}'"));
+        }
+
+        if let Some(rule) = rules.last_mut()
+            && !rule.recipients.is_empty()
+            && !part.contains('@')
+            && let Ok(recipient) = part.parse::<Address>()
+        {
+            rule.recipients.push(recipient);
             continue;
         }
 
@@ -93,20 +112,25 @@ fn parse_selector_rules(s: &str) -> Result<Vec<SelectorRule>, String> {
             None => (part, None),
         };
 
+        let selector_str = selector_str.trim();
+        if selector_str.is_empty() {
+            return Err(format!("missing selector in scope '{part}'"));
+        }
+
         let selector = parse_selector_bytes(selector_str)?;
 
         let recipients = match recipients_str {
             None => vec![],
-            Some(r) => r
-                .split(',')
-                .filter(|s| !s.trim().is_empty())
-                .map(|addr_str| {
-                    let addr_str = addr_str.trim();
-                    addr_str
-                        .parse::<Address>()
-                        .map_err(|e| format!("invalid recipient address '{addr_str}': {e}"))
-                })
-                .collect::<Result<Vec<_>, _>>()?,
+            Some(r) => {
+                let r = r.trim();
+                if r.is_empty() {
+                    return Err(format!("missing recipient after '@' in selector '{part}'"));
+                }
+                vec![
+                    r.parse::<Address>()
+                        .map_err(|e| format!("invalid recipient address '{r}': {e}"))?,
+                ]
+            }
         };
 
         rules.push(SelectorRule { selector: selector.into(), recipients });
@@ -115,6 +139,35 @@ fn parse_selector_rules(s: &str) -> Result<Vec<SelectorRule>, String> {
     Ok(rules)
 }
 
+fn split_selector_rule_parts(s: &str) -> Result<Vec<&str>, String> {
+    let mut parts = Vec::new();
+    let mut depth = 0usize;
+    let mut start = 0usize;
+
+    for (idx, ch) in s.char_indices() {
+        match ch {
+            '(' => depth += 1,
+            ')' => {
+                depth = depth
+                    .checked_sub(1)
+                    .ok_or_else(|| format!("unbalanced ')' in selector list '{s}'"))?;
+            }
+            ',' if depth == 0 => {
+                parts.push(&s[start..idx]);
+                start = idx + ch.len_utf8();
+            }
+            _ => {}
+        }
+    }
+
+    if depth != 0 {
+        return Err(format!("unbalanced '(' in selector list '{s}'"));
+    }
+
+    parts.push(&s[start..]);
+    Ok(parts)
+}
+
 /// Parse a policy token label or address into an address.
 pub(crate) fn parse_policy_token(s: &str) -> Result<Address, String> {
     match s.to_ascii_lowercase().as_str() {
@@ -158,6 +211,7 @@ pub(crate) fn parse_period(s: &str) -> Result<u64, String> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use alloy_primitives::keccak256;
     use std::str::FromStr;
 
     #[test]
@@ -192,6 +246,16 @@ mod tests {
     fn parse_selector_bytes_full_signature() {
         let sel = parse_selector_bytes("increment()").unwrap();
         assert_eq!(sel, keccak256(b"increment()")[..4]);
+
+        let sel = parse_selector_bytes("transfer(address,uint256)").unwrap();
+        assert_eq!(sel, keccak256(b"transfer(address,uint256)")[..4]);
+    }
+
+    #[test]
+    fn parse_selector_bytes_rejects_invalid_signature() {
+        assert!(parse_selector_bytes("").is_err());
+        assert!(parse_selector_bytes("transfer(address,uint256").is_err());
+        assert!(parse_selector_bytes("transfer)").is_err());
     }
 
     #[test]
@@ -242,6 +306,74 @@ mod tests {
         assert_eq!(scope.selectorRules[0].recipients.len(), 1);
     }
 
+    #[test]
+    fn parse_scope_full_signature_with_comma_argument_list() {
+        let scope =
+            parse_scope("0x20c0000000000000000000000000000000000001:transfer(address,uint256)")
+                .unwrap();
+        assert_eq!(scope.selectorRules.len(), 1);
+        assert_eq!(scope.selectorRules[0].selector.0, keccak256(b"transfer(address,uint256)")[..4]);
+    }
+
+    #[test]
+    fn parse_scope_full_signatures_split_outside_parentheses() {
+        let scope = parse_scope(
+            "0x20c0000000000000000000000000000000000001:transfer(address,uint256),approve(address,uint256)",
+        )
+        .unwrap();
+        assert_eq!(scope.selectorRules.len(), 2);
+        assert_eq!(scope.selectorRules[0].selector.0, keccak256(b"transfer(address,uint256)")[..4]);
+        assert_eq!(scope.selectorRules[1].selector.0, keccak256(b"approve(address,uint256)")[..4]);
+    }
+
+    #[test]
+    fn parse_scope_selector_with_multiple_recipients() {
+        let scope = parse_scope(
+            "0x20c0000000000000000000000000000000000001:transfer@0x1111111111111111111111111111111111111111,0x2222222222222222222222222222222222222222",
+        )
+        .unwrap();
+        assert_eq!(scope.selectorRules.len(), 1);
+        assert_eq!(
+            scope.selectorRules[0].recipients,
+            vec![
+                Address::from_str("0x1111111111111111111111111111111111111111").unwrap(),
+                Address::from_str("0x2222222222222222222222222222222222222222").unwrap(),
+            ]
+        );
+    }
+
+    #[test]
+    fn parse_scope_selector_with_multiple_recipients_and_next_selector() {
+        let scope = parse_scope(
+            "0x20c0000000000000000000000000000000000001:transfer@0x1111111111111111111111111111111111111111,0x2222222222222222222222222222222222222222,approve",
+        )
+        .unwrap();
+        assert_eq!(scope.selectorRules.len(), 2);
+        assert_eq!(scope.selectorRules[0].recipients.len(), 2);
+        assert!(scope.selectorRules[1].recipients.is_empty());
+    }
+
+    #[test]
+    fn parse_scope_rejects_empty_selectors() {
+        assert!(parse_scope("0x20c0000000000000000000000000000000000001:").is_err());
+        assert!(parse_scope("0x20c0000000000000000000000000000000000001:transfer,").is_err());
+        assert!(parse_scope("0x20c0000000000000000000000000000000000001:,transfer").is_err());
+        assert!(
+            parse_scope(
+                "0x20c0000000000000000000000000000000000001:@0x1111111111111111111111111111111111111111"
+            )
+            .is_err()
+        );
+    }
+
+    #[test]
+    fn parse_scope_rejects_empty_or_invalid_recipient() {
+        assert!(parse_scope("0x20c0000000000000000000000000000000000001:transfer@").is_err());
+        assert!(
+            parse_scope("0x20c0000000000000000000000000000000000001:transfer@approve").is_err()
+        );
+    }
+
     #[test]
     fn parse_policy_token_path_usd() {
         assert_eq!(parse_policy_token("PathUSD").unwrap(), PATH_USD_ADDRESS);