Check for duplicate selectors

Signed-off-by: Sean Young <sean@mess.org>

src/sema/contracts.rs

@@ -302,6 +302,7 @@ fn check_inheritance(contract_no: usize, ns: &mut ast::Namespace) {
     let mut variable_syms: HashMap<String, ast::Symbol> = HashMap::new();
     let mut override_needed: BTreeMap<String, Vec<(usize, usize)>> = BTreeMap::new();
     let mut diagnostics = Diagnostics::default();
+    let mut selectors: HashMap<Vec<u8>, usize> = HashMap::new();
 
     for base_contract_no in ns.contract_bases(contract_no) {
         // find file number where contract is defined
@@ -629,6 +630,40 @@ fn check_inheritance(contract_no: usize, ns: &mut ast::Namespace) {
                     .insert(signature, function_no);
             }
 
+            let selector = cur.selector();
+
+            // On Solana, concrete contracts have selectors of 4 bytes
+            // TODO: this will change with the switch to borsh!
+            if ns.contracts[contract_no].is_concrete() && selector.len() != 4 {
+                diagnostics.push(ast::Diagnostic::error(
+                    cur.loc,
+                    format!(
+                        "function '{}' selector '{}' must be 4 bytes rather than {} bytes",
+                        cur.name,
+                        hex::encode(&selector),
+                        selector.len()
+                    ),
+                ));
+            }
+
+            if let Some(other_func_no) = selectors.get(&selector) {
+                let other = &ns.functions[*other_func_no];
+
+                if other.signature != cur.signature {
+                    diagnostics.push(ast::Diagnostic::error_with_note(
+                        cur.loc,
+                        format!(
+                            "{} '{}' selector is the same as {} '{}'",
+                            cur.ty, cur.name, other.ty, other.name
+                        ),
+                        other.loc,
+                        format!("definition of {} '{}'", other.ty, other.name),
+                    ));
+                }
+            } else {
+                selectors.insert(selector, function_no);
+            }
+
             ns.contracts[contract_no]
                 .all_functions
                 .insert(function_no, usize::MAX);

src/sema/functions.rs

@@ -362,8 +362,7 @@ pub fn contract_function(
                 ResolveTo::Unknown,
             ) {
                 Ok(Expression::BytesLiteral(_, _, v)) => {
-                    // TODO: Solana uses different length discriminators
-                    if v.len() != 4 {
+                    if ns.target != Target::Solana && v.len() != 4 {
                         ns.diagnostics.push(Diagnostic::error(
                             selector.loc(),
                             format!("selector is {} bytes, 4 bytes expected", v.len()),

tests/contract_testcases/solana/selectors-must-be-different.dot

@@ -0,0 +1,80 @@
+strict digraph "tests/contract_testcases/solana/selectors-must-be-different.sol" {
+	contract [label="contract c\ntests/contract_testcases/solana/selectors-must-be-different.sol:1:1-7:2"]
+	f1 [label="function f1\ncontract: c\ntests/contract_testcases/solana/selectors-must-be-different.sol:2:2-39\nsignature f1()\nvisibility public\nmutability nonpayable\nselector 01"]
+	f2 [label="function f2\ncontract: c\ntests/contract_testcases/solana/selectors-must-be-different.sol:4:2-39\nsignature f2()\nvisibility public\nmutability nonpayable\nselector 01"]
+	f3 [label="function f3\ncontract: c\ntests/contract_testcases/solana/selectors-must-be-different.sol:6:2-39\nsignature f3()\nvisibility public\nmutability nonpayable\nselector 01"]
+	contract_5 [label="contract d\ntests/contract_testcases/solana/selectors-must-be-different.sol:8:1-14:2"]
+	var [label="variable c\nvisibility public\ntests/contract_testcases/solana/selectors-must-be-different.sol:10:2-14"]
+	c [label="function c\ncontract: d\ntests/contract_testcases/solana/selectors-must-be-different.sol:10:13-14\nsignature c()\nvisibility public\nmutability view"]
+	returns [label="returns\nint256 "]
+	return [label="return\nimplicit"]
+	storage_load [label="storage load int256\nimplicit"]
+	storage_var [label="storage variable\nd.c\nint256 storage\nimplicit"]
+	f1_12 [label="function f1\ncontract: d\ntests/contract_testcases/solana/selectors-must-be-different.sol:13:2-45\nsignature f1()\nvisibility public\nmutability nonpayable\nselector c3da42b8"]
+	contract_13 [label="contract e\ntests/contract_testcases/solana/selectors-must-be-different.sol:15:1-19:2"]
+	f1_14 [label="function f1\ncontract: e\ntests/contract_testcases/solana/selectors-must-be-different.sol:18:2-39\nsignature f1()\nvisibility public\nmutability nonpayable\nselector 01"]
+	contract_15 [label="contract f\ntests/contract_testcases/solana/selectors-must-be-different.sol:20:1-26:2"]
+	f1_16 [label="function f1\ncontract: f\ntests/contract_testcases/solana/selectors-must-be-different.sol:23:2-45\nsignature f1()\nvisibility public\nmutability nonpayable\nselector 41424344"]
+	f2_17 [label="function f2\ncontract: f\ntests/contract_testcases/solana/selectors-must-be-different.sol:24:2-45\nsignature f2()\nvisibility public\nmutability nonpayable\nselector 41424344"]
+	f3_18 [label="function f3\ncontract: f\ntests/contract_testcases/solana/selectors-must-be-different.sol:25:2-22\nsignature f3()\nvisibility public\nmutability nonpayable"]
+	contract_19 [label="contract g\ntests/contract_testcases/solana/selectors-must-be-different.sol:27:1-32:2"]
+	f1_20 [label="function f1\ncontract: g\ntests/contract_testcases/solana/selectors-must-be-different.sol:29:2-22\nsignature f1()\nvisibility public\nmutability nonpayable"]
+	f3_21 [label="function f3\ncontract: g\ntests/contract_testcases/solana/selectors-must-be-different.sol:31:2-45\nsignature f3()\nvisibility public\nmutability nonpayable\nselector c27fc305"]
+	diagnostic [label="found abstract contract 'c'\nlevel Debug\ntests/contract_testcases/solana/selectors-must-be-different.sol:1:1-7:2"]
+	diagnostic_24 [label="function 'f2' selector is the same as function 'f1'\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:4:2-39"]
+	note [label="definition of function 'f1'\ntests/contract_testcases/solana/selectors-must-be-different.sol:2:2-39"]
+	diagnostic_26 [label="function 'f3' selector is the same as function 'f1'\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:6:2-39"]
+	note_27 [label="definition of function 'f1'\ntests/contract_testcases/solana/selectors-must-be-different.sol:2:2-39"]
+	diagnostic_28 [label="found contract 'd'\nlevel Debug\ntests/contract_testcases/solana/selectors-must-be-different.sol:8:1-14:2"]
+	diagnostic_29 [label="c is already defined as a contract name\nlevel Warning\ntests/contract_testcases/solana/selectors-must-be-different.sol:10:13-14"]
+	note_30 [label="location of previous definition\ntests/contract_testcases/solana/selectors-must-be-different.sol:1:1-7:2"]
+	diagnostic_31 [label="function 'f1' selector is the same as function 'c'\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:13:2-45"]
+	note_32 [label="definition of function 'c'\ntests/contract_testcases/solana/selectors-must-be-different.sol:10:13-14"]
+	diagnostic_33 [label="found contract 'e'\nlevel Debug\ntests/contract_testcases/solana/selectors-must-be-different.sol:15:1-19:2"]
+	diagnostic_34 [label="function 'f1' selector '01' must be 4 bytes rather than 1 bytes\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:18:2-39"]
+	diagnostic_35 [label="found contract 'f'\nlevel Debug\ntests/contract_testcases/solana/selectors-must-be-different.sol:20:1-26:2"]
+	diagnostic_36 [label="function 'f2' selector is the same as function 'f1'\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:24:2-45"]
+	note_37 [label="definition of function 'f1'\ntests/contract_testcases/solana/selectors-must-be-different.sol:23:2-45"]
+	diagnostic_38 [label="found contract 'g'\nlevel Debug\ntests/contract_testcases/solana/selectors-must-be-different.sol:27:1-32:2"]
+	diagnostic_39 [label="function 'f3' selector is the same as function 'f1'\nlevel Error\ntests/contract_testcases/solana/selectors-must-be-different.sol:31:2-45"]
+	note_40 [label="definition of function 'f1'\ntests/contract_testcases/solana/selectors-must-be-different.sol:29:2-22"]
+	contracts -> contract
+	contract -> f1 [label="function"]
+	contract -> f2 [label="function"]
+	contract -> f3 [label="function"]
+	contracts -> contract_5
+	contract_5 -> var [label="variable"]
+	contract_5 -> c [label="function"]
+	c -> returns [label="returns"]
+	c -> return [label="body"]
+	return -> storage_load [label="expr"]
+	storage_load -> storage_var [label="expr"]
+	contract_5 -> f1_12 [label="function"]
+	contracts -> contract_13
+	contract_13 -> f1_14 [label="function"]
+	contracts -> contract_15
+	contract_15 -> f1_16 [label="function"]
+	contract_15 -> f2_17 [label="function"]
+	contract_15 -> f3_18 [label="function"]
+	contracts -> contract_19
+	contract_19 -> f1_20 [label="function"]
+	contract_19 -> f3_21 [label="function"]
+	diagnostics -> diagnostic [label="Debug"]
+	diagnostics -> diagnostic_24 [label="Error"]
+	diagnostic_24 -> note [label="note"]
+	diagnostics -> diagnostic_26 [label="Error"]
+	diagnostic_26 -> note_27 [label="note"]
+	diagnostics -> diagnostic_28 [label="Debug"]
+	diagnostics -> diagnostic_29 [label="Warning"]
+	diagnostic_29 -> note_30 [label="note"]
+	diagnostics -> diagnostic_31 [label="Error"]
+	diagnostic_31 -> note_32 [label="note"]
+	diagnostics -> diagnostic_33 [label="Debug"]
+	diagnostics -> diagnostic_34 [label="Error"]
+	diagnostics -> diagnostic_35 [label="Debug"]
+	diagnostics -> diagnostic_36 [label="Error"]
+	diagnostic_36 -> note_37 [label="note"]
+	diagnostics -> diagnostic_38 [label="Debug"]
+	diagnostics -> diagnostic_39 [label="Error"]
+	diagnostic_39 -> note_40 [label="note"]
+}

tests/contract_testcases/solana/selectors-must-be-different.sol

@@ -0,0 +1,32 @@
+abstract contract c {
+	function f1() selector=hex"01" public {}
+	// error: selector must be unique
+	function f2() selector=hex"01" public {}
+	// error: selector must be unique
+	function f3() selector=hex"01" public {}
+}
+
+contract d {
+	int public c;
+
+	// error: selector is the same as c
+	function f1() selector=hex"c3da42b8" public {}
+}
+
+contract e {
+	// error: selector must be 4 bytes
+	function f1() selector=hex"01" public {}
+}
+
+contract f {
+	// error: selectors are the same
+	function f1() selector=hex"41424344" public {}
+	function f2() selector=hex"41424344" public {}
+	function f3() public {}
+}
+
+contract g {
+	function f1() public {}
+	// error: selector for f3 matches f1
+	function f3() selector=hex"c27fc305" public {}
+}