From b549a3ed53f6503d23f87cc27390c8650a8dc0e2 Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Fri, 4 Jan 2019 15:41:02 +0000
Subject: [PATCH 01/11] Add new APIs to core.contract: - functions_inherited -
 functions_not_inherited - modifiers_inherited - modifiers_not_inherited -
 functions_and_modifiers_inherited - functions_and_modifiers_not_inherited -
 functions_entry_points - get_functions_overridden_by

---
 slither/core/declarations/contract.py | 74 ++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 6 deletions(-)

diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py
index c2c9983f7..a4e5bbc90 100644
--- a/slither/core/declarations/contract.py
+++ b/slither/core/declarations/contract.py
@@ -85,12 +85,6 @@ class Contract(ChildSlither, SourceMapping):
     def enums_as_dict(self):
         return self._enums
 
-    @property
-    def modifiers(self):
-        '''
-            list(Modifier): List of the modifiers
-        '''
-        return list(self._modifiers.values())
 
     def modifiers_as_dict(self):
         return self._modifiers
@@ -113,6 +107,74 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return [f for f in self.functions if f.contract != self]
 
+    @property
+    def functions_not_inherited(self):
+        '''
+            list(Function): List of the functions defined within the contract (not inherited)
+        '''
+        return [f for f in self.functions if f.contract == self]
+
+    @property
+    def functions_entry_points(self):
+        '''
+            list(Functions): List of public and external functions
+        '''
+        return [f for f in self.functions if f.visibility in ['public', 'external']]
+
+    @property
+    def modifiers(self):
+        '''
+            list(Modifier): List of the modifiers
+        '''
+        return list(self._modifiers.values())
+
+    @property
+    def modifiers_inherited(self):
+        '''
+            list(Modifier): List of the inherited modifiers
+        '''
+        return [m for m in self.modifiers if m.contract != self]
+
+    @property
+    def modifiers_not_inherited(self):
+        '''
+            list(Modifier): List of the modifiers defined within the contract (not inherited)
+        '''
+        return [m for m in self.modifiers if m.contract == self]
+
+    @property
+    def functions_and_modifiers(self):
+        '''
+            list(Function|Modifier): List of the functions and modifiers
+        '''
+        return self.functions + self.modifiers
+
+    @property
+    def functions_and_modifiers_inherited(self):
+        '''
+            list(Function|Modifier): List of the inherited functions and modifiers
+        '''
+        return self.functions_inherited + self.modifiers_inherited
+
+    @property
+    def functions_and_modifiers_not_inherited(self):
+        '''
+            list(Function|Modifier): List of the functions and modifiers defined within the contract (not inherited)
+        '''
+        return self.functions_not_inherited + self.modifiers_not_inherited
+
+    def get_functions_overridden_by(self, function):
+        '''
+            Return the list of functions overriden by the function
+        Args:
+            (core.Function)
+        Returns:
+            list(core.Function)
+
+        '''
+        candidates = self.functions_inherited
+        return [f for f in candidates if f.name == function]
+
     @property
     def all_functions_called(self):
         '''

From 57a0918f2434de0e0d23fad511f2fc4e63853cea Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Fri, 4 Jan 2019 17:27:37 +0000
Subject: [PATCH 02/11] API changes: - Remove
 contract.get_functions_overridden_by - Add function.functions_shadowed

---
 slither/core/declarations/contract.py | 11 -----------
 slither/core/declarations/function.py | 12 ++++++++++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py
index a4e5bbc90..724d3b035 100644
--- a/slither/core/declarations/contract.py
+++ b/slither/core/declarations/contract.py
@@ -163,17 +163,6 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return self.functions_not_inherited + self.modifiers_not_inherited
 
-    def get_functions_overridden_by(self, function):
-        '''
-            Return the list of functions overriden by the function
-        Args:
-            (core.Function)
-        Returns:
-            list(core.Function)
-
-        '''
-        candidates = self.functions_inherited
-        return [f for f in candidates if f.name == function]
 
     @property
     def all_functions_called(self):
diff --git a/slither/core/declarations/function.py b/slither/core/declarations/function.py
index d8e4a9931..3f31429b4 100644
--- a/slither/core/declarations/function.py
+++ b/slither/core/declarations/function.py
@@ -319,6 +319,18 @@ class Function(ChildContract, SourceMapping):
         name, parameters, _ = self.signature
         return name+'('+','.join(parameters)+')'
 
+    @property
+    def functions_shadowed(self):
+        '''
+            Return the list of functions shadowed
+        Returns:
+            list(core.Function)
+
+        '''
+        candidates = [c.functions_not_inherited for c in self.contract.inheritance]
+        candidates = [candidate for sublist in candidates for candidate in sublist]
+        return [f for f in candidates if f.full_name == self.full_name]
+
 
     @property
     def slither(self):

From a704635de3ab1a204648d6f41e9b4350d5100ff6 Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Fri, 4 Jan 2019 17:32:08 +0000
Subject: [PATCH 03/11] API: - Add contract.derived_contracts

---
 slither/core/declarations/contract.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/slither/core/declarations/contract.py b/slither/core/declarations/contract.py
index 724d3b035..51ed5e6ce 100644
--- a/slither/core/declarations/contract.py
+++ b/slither/core/declarations/contract.py
@@ -68,6 +68,14 @@ class Contract(ChildSlither, SourceMapping):
     def setInheritance(self, inheritance):
         self._inheritance = inheritance
 
+    @property
+    def derived_contracts(self):
+        '''
+            list(Contract): Return the list of contracts derived from self
+        '''
+        candidates = self.slither.contracts
+        return [c for c in candidates if self in c.inheritance]
+
     @property
     def structures(self):
         '''
@@ -163,6 +171,18 @@ class Contract(ChildSlither, SourceMapping):
         '''
         return self.functions_not_inherited + self.modifiers_not_inherited
 
+    def get_functions_overridden_by(self, function):
+        '''
+            Return the list of functions overriden by the function
+        Args:
+            (core.Function)
+        Returns:
+            list(core.Function)
+
+        '''
+        candidates = [c.functions_not_inherited for c in self.inheritance]
+        candidates = [candidate for sublist in candidates for candidate in sublist]
+        return [f for f in candidates if f.full_name == function.full_name]
 
     @property
     def all_functions_called(self):

From 510aa29b9952315eb8a0f1ad6ced6f15c49439a7 Mon Sep 17 00:00:00 2001
From: David Pokora <dpokora@gmail.com>
Date: Sun, 6 Jan 2019 13:00:02 -0500
Subject: [PATCH 04/11] Rewrote external-function detector to handle
 false-positives as a result of inheritance.

---
 .../detectors/functions/external_function.py  | 138 +++++++++++++++---
 tests/external_function_test_3.sol            |  55 +++++++
 2 files changed, 176 insertions(+), 17 deletions(-)
 create mode 100644 tests/external_function_test_3.sol

diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py
index 006cb0192..6b39f6273 100644
--- a/slither/detectors/functions/external_function.py
+++ b/slither/detectors/functions/external_function.py
@@ -27,7 +27,10 @@ class ExternalFunction(AbstractDetector):
             (list): List of all InternallCall, SolidityCall
         """
         result = []
+
+        # Obtain all functions reachable by this contract.
         for func in contract.all_functions_called:
+            # Loop through all nodes in the function, add all calls to a list.
             for node in func.nodes:
                 for ir in node.irs:
                     if isinstance(ir, (InternalCall, SolidityCall)):
@@ -36,6 +39,12 @@ class ExternalFunction(AbstractDetector):
 
     @staticmethod
     def _contains_internal_dynamic_call(contract):
+        """
+        Checks if a contract contains a dynamic call either in a direct definition, or through inheritance.
+
+        Returns:
+            (boolean): True if this contract contains a dynamic call (including through inheritance).
+        """
         for func in contract.all_functions_called:
             for node in func.nodes:
                 for ir in node.irs:
@@ -43,31 +52,126 @@ class ExternalFunction(AbstractDetector):
                         return True
         return False
 
+    @staticmethod
+    def get_base_most_function(function):
+        """
+        Obtains the base function definition for the provided function. This could be used to obtain the original
+        definition of a function, if the provided function is an override.
+
+        Returns:
+            (function): Returns the base-most function of a provided function. (The original definition).
+        """
+        # Loop through the list of inherited contracts and this contract, to find the first function instance which
+        # matches this function's signature. Note here that `inheritance` is in order from most basic to most extended.
+        for contract in function.contract.inheritance + [function.contract]:
+
+            # Loop through the functions not inherited (explicitly defined in this contract).
+            for f in contract.functions_not_inherited:
+
+                # If it matches names, this is the base most function.
+                if f.full_name == function.full_name:
+                    return f
+
+        # Somehow we couldn't resolve it, which shouldn't happen, as the provided function should be found if we could
+        # not find some any more basic.
+        raise Exception("Could not resolve the base-most function for the provided function.")
+
+    @staticmethod
+    def get_all_function_definitions(base_most_function):
+        """
+        Obtains all function definitions given a base-most function. This includes the provided function, plus any
+        overrides of that function.
+
+        Returns:
+            (list): Returns any the provided function and any overriding functions defined for it.
+        """
+        # We assume the provided function is the base-most function, so we check all derived contracts
+        # for a redefinition
+        return [base_most_function] + [function for derived_contract in base_most_function.contract.derived_contracts
+                                       for function in derived_contract.functions
+                                       if function.full_name == base_most_function.full_name]
+
     def detect(self):
         results = []
 
-        public_function_calls = []
         all_info = ''
 
-        for contract in self.slither.contracts_derived:
+        # Create a set to track contracts with dynamic calls. All contracts with dynamic calls could potentially be
+        # calling functions internally, and thus we can't assume any function in such contracts isn't called by them.
+        dynamic_call_contracts = set()
+
+        # Create a completed functions set to skip over functions already processed (any functions which are the base
+        # of, or override hierarchically are processed together).
+        completed_functions = set()
+
+        # First we build our set of all contracts with dynamic calls
+        for contract in self.contracts:
             if self._contains_internal_dynamic_call(contract):
+                dynamic_call_contracts.add(contract)
+
+        # Loop through all not-inherited contracts.
+        for contract in self.slither.contracts_derived:
+
+            # Filter false-positives: Immediately filter this contract if it's in blacklist
+            if contract in dynamic_call_contracts:
                 continue
 
-            func_list = self.detect_functions_called(contract)
-            public_function_calls.extend(func_list)
-
-            for func in [f for f in contract.functions if f.visibility == 'public' and\
-                                                           not f in public_function_calls and\
-                                                           not f.is_constructor]:
-                txt = "{}.{} ({}) should be declared external\n"
-                info = txt.format(func.contract.name,
-                                  func.name,
-                                  func.source_mapping_str)
-                all_info += info
-
-                json = self.generate_json_result(info)
-                self.add_function_to_json(func, json)
-                results.append(json)
+            # Next we'll want to loop through all functions defined directly in this contract.
+            for function in contract.functions_not_inherited:
+
+                # If the function is a constructor, or is public, we skip it.
+                if function.is_constructor or function.visibility != "public":
+                    continue
+
+                # Optimization: If this function has already been processed, we stop.
+                if function in completed_functions:
+                    continue
+
+                # Get the base-most function to know our origin of this function.
+                base_most_function = self.get_base_most_function(function)
+
+                # Get all possible contracts which can call this function (or an override).
+                all_possible_sources = [base_most_function.contract] + base_most_function.contract.derived_contracts
+
+                # Get all function signatures (overloaded and not), mark as completed and we process them now.
+                # Note: We mark all function definitions as the same, as they must all share visibility to override.
+                all_function_definitions = set(self.get_all_function_definitions(base_most_function))
+                completed_functions = completed_functions.union(all_function_definitions)
+
+                # Filter false-positives: Determine if any of these sources have dynamic calls, if so, flag all of these
+                # function definitions, and then flag all functions in all contracts that make dynamic calls.
+                sources_with_dynamic_calls = set(all_possible_sources) & dynamic_call_contracts
+                if sources_with_dynamic_calls:
+                    functions_in_dynamic_call_sources = set([f for dyn_contract in sources_with_dynamic_calls
+                                                             for f in dyn_contract if not f.is_constructor])
+                    completed_functions = completed_functions.union(functions_in_dynamic_call_sources)
+                    continue
+
+                # Detect all functions called in each source, if any match our current signature, we skip
+                # otherwise, this is a candidate (in all sources) to be changed visibility for.
+                is_called = False
+                for possible_source in all_possible_sources:
+                    functions_called = self.detect_functions_called(possible_source)
+                    if set(functions_called) & all_function_definitions:
+                        is_called = True
+                        break
+
+                # If any of this function's definitions are called, we skip it.
+                if is_called:
+                    continue
+
+                # Loop for each function definition, and recommend it be declared external.
+                for function_definition in all_function_definitions:
+                    txt = "{}.{} ({}) should be declared external\n"
+                    info = txt.format(function_definition.contract.name,
+                                      function_definition.name,
+                                      function_definition.source_mapping_str)
+                    all_info += info
+
+                    json = self.generate_json_result(info)
+                    self.add_function_to_json(function_definition, json)
+                    results.append(json)
+
         if all_info != '':
             self.log(all_info)
         return results
diff --git a/tests/external_function_test_3.sol b/tests/external_function_test_3.sol
new file mode 100644
index 000000000..07dab167c
--- /dev/null
+++ b/tests/external_function_test_3.sol
@@ -0,0 +1,55 @@
+// This tests against false-positives. This test should output no recommendations from the external-function detector.
+
+
+contract ContractWithBaseFunctionCalled {
+    function getsCalledByBase() public;
+    function callsOverrideMe() external {
+        getsCalledByBase();
+    }
+}
+
+
+contract DerivingContractWithBaseCalled is ContractWithBaseFunctionCalled {
+    function getsCalledByBase() public {
+        // This should not be recommended to be marked external because it is called by the base class.
+    }
+}
+
+
+// All the contracts below should not recommend changing to external since inherited contracts have dynamic calls.
+contract ContractWithDynamicCall {
+    function() returns(uint) ptr;
+
+    function test1() public returns(uint){
+        return 1;
+    }
+
+    function test2() public returns(uint){
+        return 2;
+    }
+
+    function set_test1() external{
+        ptr = test1;
+    }
+
+    function set_test2() external{
+        ptr = test2;
+    }
+
+    function exec() external returns(uint){
+        return ptr();
+    }
+}
+
+contract DerivesFromDynamicCall is ContractWithDynamicCall{
+    function getsCalledDynamically() public returns (uint){
+        // This should not be recommended because it is called dynamically.
+        return 3;
+    }
+    function set_test3() public {
+        // This should not be recommended because we inherit from a contract that calls dynamically, and we cannot be
+        // sure it did not somehow call this function.
+
+        ptr = getsCalledDynamically;
+    }
+}
\ No newline at end of file

From af646da29ed40f8602a9755cf90cc20f1b8669dd Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 09:13:54 +0000
Subject: [PATCH 05/11] Update travis tests

---
 scripts/tests_generate_expected_json_4.sh     | 51 ++++++++++---------
 scripts/tests_generate_expected_json_5.sh     | 16 +++---
 scripts/travis_test_4.sh                      |  1 +
 scripts/travis_test_5.sh                      |  3 +-
 ...external_function_2.external-function.json |  1 +
 tests/external_function.sol                   |  2 +-
 ...ion_test_3.sol => external_function_2.sol} |  8 +--
 ...est_2.sol => external_function_import.sol} |  2 +-
 8 files changed, 44 insertions(+), 40 deletions(-)
 create mode 100644 tests/expected_json/external_function_2.external-function.json
 rename tests/{external_function_test_3.sol => external_function_2.sol} (93%)
 rename tests/{external_function_test_2.sol => external_function_import.sol} (68%)

diff --git a/scripts/tests_generate_expected_json_4.sh b/scripts/tests_generate_expected_json_4.sh
index 5bedd2f69..00d0b2b14 100755
--- a/scripts/tests_generate_expected_json_4.sh
+++ b/scripts/tests_generate_expected_json_4.sh
@@ -14,28 +14,29 @@ generate_expected_json(){
 
 }
 
-generate_expected_json tests/uninitialized.sol "uninitialized-state"
-generate_expected_json tests/backdoor.sol "backdoor"
-generate_expected_json tests/backdoor.sol "suicidal"
-generate_expected_json tests/pragma.0.4.24.sol "pragma"
-generate_expected_json tests/old_solc.sol.json "solc-version"
-generate_expected_json tests/reentrancy.sol "reentrancy"
-generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
-generate_expected_json tests/tx_origin.sol "tx-origin"
-generate_expected_json tests/unused_state.sol "unused-state"
-generate_expected_json tests/locked_ether.sol "locked-ether"
-generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
-generate_expected_json tests/inline_assembly_contract.sol "assembly"
-generate_expected_json tests/inline_assembly_library.sol "assembly"
-generate_expected_json tests/low_level_calls.sol "low-level-calls"
-generate_expected_json tests/const_state_variables.sol "constable-states"
-generate_expected_json tests/external_function.sol "external-function"
-generate_expected_json tests/naming_convention.sol "naming-convention"
-generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
-generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
-generate_expected_json tests/constant.sol "constant-function"
-generate_expected_json tests/unused_return.sol "unused-return"
-generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
-generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
-generate_expected_json tests/timestamp.sol "timestamp"
-generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
+#generate_expected_json tests/uninitialized.sol "uninitialized-state"
+#generate_expected_json tests/backdoor.sol "backdoor"
+#generate_expected_json tests/backdoor.sol "suicidal"
+#generate_expected_json tests/pragma.0.4.24.sol "pragma"
+#generate_expected_json tests/old_solc.sol.json "solc-version"
+#generate_expected_json tests/reentrancy.sol "reentrancy"
+#generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
+#generate_expected_json tests/tx_origin.sol "tx-origin"
+#generate_expected_json tests/unused_state.sol "unused-state"
+#generate_expected_json tests/locked_ether.sol "locked-ether"
+#generate_expected_json tests/arbitrary_send.sol "arbitrary-send"
+#generate_expected_json tests/inline_assembly_contract.sol "assembly"
+#generate_expected_json tests/inline_assembly_library.sol "assembly"
+#generate_expected_json tests/low_level_calls.sol "low-level-calls"
+#generate_expected_json tests/const_state_variables.sol "constable-states"
+#generate_expected_json tests/external_function.sol "external-function"
+generate_expected_json tests/external_function_2.sol "external-function"
+#generate_expected_json tests/naming_convention.sol "naming-convention"
+#generate_expected_json tests/uninitialized_local_variable.sol "uninitialized-local"
+#generate_expected_json tests/controlled_delegatecall.sol "controlled-delegatecall"
+#generate_expected_json tests/constant.sol "constant-function"
+#generate_expected_json tests/unused_return.sol "unused-return"
+#generate_expected_json tests/shadowing_state_variable.sol "shadowing-state"
+#generate_expected_json tests/shadowing_abstract.sol "shadowing-abstract"
+#generate_expected_json tests/timestamp.sol "timestamp"
+#generate_expected_json tests/multiple_calls_in_loop.sol "calls-loop"
diff --git a/scripts/tests_generate_expected_json_5.sh b/scripts/tests_generate_expected_json_5.sh
index a6b0feea2..63a60a668 100755
--- a/scripts/tests_generate_expected_json_5.sh
+++ b/scripts/tests_generate_expected_json_5.sh
@@ -14,16 +14,16 @@ generate_expected_json(){
 
 }
 
-generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state"
+#generate_expected_json tests/uninitialized-0.5.1.sol "uninitialized-state"
 #generate_expected_json tests/backdoor.sol "backdoor"
 #generate_expected_json tests/backdoor.sol "suicidal"
 #generate_expected_json tests/pragma.0.4.24.sol "pragma"
 #generate_expected_json tests/old_solc.sol.json "solc-version"
-generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy"
+#generate_expected_json tests/reentrancy-0.5.1.sol "reentrancy"
 #generate_expected_json tests/uninitialized_storage_pointer.sol "uninitialized-storage"
-generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin"
-generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether"
-generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
-generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
-generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
-generate_expected_json tests/constant-0.5.1.sol "constant-function"
+#generate_expected_json tests/tx_origin-0.5.1.sol "tx-origin"
+#generate_expected_json tests/locked_ether-0.5.1.sol "locked-ether"
+#generate_expected_json tests/arbitrary_send-0.5.1.sol "arbitrary-send"
+#generate_expected_json tests/inline_assembly_contract-0.5.1.sol "assembly"
+#generate_expected_json tests/inline_assembly_library-0.5.1.sol "assembly"
+#generate_expected_json tests/constant-0.5.1.sol "constant-function"
diff --git a/scripts/travis_test_4.sh b/scripts/travis_test_4.sh
index 6395537a8..29b9bceef 100755
--- a/scripts/travis_test_4.sh
+++ b/scripts/travis_test_4.sh
@@ -81,6 +81,7 @@ test_slither tests/inline_assembly_library.sol "assembly"
 test_slither tests/low_level_calls.sol "low-level-calls"
 test_slither tests/const_state_variables.sol "constable-states"
 test_slither tests/external_function.sol "external-function"
+test_slither tests/external_function_2.sol "external-function"
 test_slither tests/naming_convention.sol "naming-convention"
 #test_slither tests/complex_func.sol "complex-function"
 test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
diff --git a/scripts/travis_test_5.sh b/scripts/travis_test_5.sh
index d7af73720..24f258f19 100755
--- a/scripts/travis_test_5.sh
+++ b/scripts/travis_test_5.sh
@@ -79,8 +79,9 @@ test_slither tests/inline_assembly_library-0.5.1.sol "assembly"
 test_slither tests/low_level_calls.sol "low-level-calls"
 test_slither tests/const_state_variables.sol "constable-states"
 test_slither tests/external_function.sol "external-function"
+test_slither tests/external_function_2.sol "external-function"
 test_slither tests/naming_convention.sol "naming-convention"
-#test_slither tests/complex_func.sol "complex-function"
+##test_slither tests/complex_func.sol "complex-function"
 test_slither tests/controlled_delegatecall.sol "controlled-delegatecall"
 test_slither tests/constant-0.5.1.sol "constant-function"
 test_slither tests/unused_return.sol "unused-return"
diff --git a/tests/expected_json/external_function_2.external-function.json b/tests/expected_json/external_function_2.external-function.json
new file mode 100644
index 000000000..0637a088a
--- /dev/null
+++ b/tests/expected_json/external_function_2.external-function.json
@@ -0,0 +1 @@
+[]
\ No newline at end of file
diff --git a/tests/external_function.sol b/tests/external_function.sol
index 714e6997f..1ed7c70c0 100644
--- a/tests/external_function.sol
+++ b/tests/external_function.sol
@@ -1,6 +1,6 @@
 //pragma solidity ^0.4.24;
 
-import "./external_function_test_2.sol";
+import "./external_function_import.sol";
 
 contract ContractWithFunctionCalledSuper is ContractWithFunctionCalled {
     function callWithSuper() public {
diff --git a/tests/external_function_test_3.sol b/tests/external_function_2.sol
similarity index 93%
rename from tests/external_function_test_3.sol
rename to tests/external_function_2.sol
index 07dab167c..97eed10a0 100644
--- a/tests/external_function_test_3.sol
+++ b/tests/external_function_2.sol
@@ -28,11 +28,11 @@ contract ContractWithDynamicCall {
         return 2;
     }
 
-    function set_test1() external{
+    function setTest1() external{
         ptr = test1;
     }
 
-    function set_test2() external{
+    function setTest2() external{
         ptr = test2;
     }
 
@@ -46,10 +46,10 @@ contract DerivesFromDynamicCall is ContractWithDynamicCall{
         // This should not be recommended because it is called dynamically.
         return 3;
     }
-    function set_test3() public {
+    function setTest3() public {
         // This should not be recommended because we inherit from a contract that calls dynamically, and we cannot be
         // sure it did not somehow call this function.
 
         ptr = getsCalledDynamically;
     }
-}
\ No newline at end of file
+}
diff --git a/tests/external_function_test_2.sol b/tests/external_function_import.sol
similarity index 68%
rename from tests/external_function_test_2.sol
rename to tests/external_function_import.sol
index ae7388993..efddea28e 100644
--- a/tests/external_function_test_2.sol
+++ b/tests/external_function_import.sol
@@ -1,4 +1,4 @@
-//pragma solidity ^0.4.24;
+// This file is imported by external_function.sol
 
 contract ContractWithFunctionCalled {
     function funcCalled() external  {

From 33a8cf422443eee93eb9a1e053d2bebe7180cc9c Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 10:01:23 +0000
Subject: [PATCH 06/11] Add support for subdenomination

---
 .../expressions/expression_parsing.py         | 38 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py
index 53e3111d0..c0ac0a1e2 100644
--- a/slither/solc_parsing/expressions/expression_parsing.py
+++ b/slither/solc_parsing/expressions/expression_parsing.py
@@ -237,6 +237,34 @@ def filter_name(value):
         value = value[:idx+1]
     return value
 
+def convert_subdenomination(value, sub):
+    if sub is None:
+        return value
+    value = int(value)
+    if sub == 'wei':
+        return value
+    if sub == 'szabo':
+        return value * int(1e12)
+    if sub == 'finney':
+        return value * int(1e15)
+    if sub == 'ether':
+        return value * int(1e18)
+    if sub == 'seconds':
+        return value
+    if sub == 'minutes':
+        return value * 60
+    if sub == 'hours':
+        return value * 60 * 60
+    if sub == 'days':
+        return value * 60 * 60 * 24
+    if sub == 'weeks':
+        return value * 60 * 60 * 24 * 7
+    if sub == 'years':
+        return value * 60 * 60 * 24 * 7 * 365
+
+    logger.error('Subdemoniation not found {}'.format(sub))
+    return value
+
 def parse_expression(expression, caller_context):
     """
 
@@ -380,11 +408,17 @@ def parse_expression(expression, caller_context):
 
         if is_compact_ast:
             value = expression['value']
-            if not value and value != "":
+            if value:
+                if 'subdenomination' in expression and expression['subdenomination']:
+                    value = str(convert_subdenomination(value, expression['subdenomination']))
+            elif not value and value != "":
                 value = '0x'+expression['hexValue']
         else:
             value = expression['attributes']['value']
-            if value is None:
+            if value:
+                if 'subdenomination' in expression['attributes'] and expression['attributes']['subdenomination']:
+                    value = str(convert_subdenomination(value, expression['attributes']['subdenomination']))
+            elif value is None:
                 # for literal declared as hex
                 # see https://solidity.readthedocs.io/en/v0.4.25/types.html?highlight=hex#hexadecimal-literals
                 assert 'hexvalue' in expression['attributes']

From 809b36355b2afa4260456f5b5ddef493ce38a8ec Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 10:46:00 +0000
Subject: [PATCH 07/11] Add support for implicit Literal conversion from
 uint256 -> int256 on signature lookup (close #120)

---
 slither/slithir/convert.py | 50 +++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/slither/slithir/convert.py b/slither/slithir/convert.py
index a8a629f14..ba5213653 100644
--- a/slither/slithir/convert.py
+++ b/slither/slithir/convert.py
@@ -316,10 +316,19 @@ def get_type(t):
     return str(t)
 
 def get_sig(ir):
+    '''
+        Return a list of potential signature
+        It is a list, as Constant variables can be converted to int256
+    Args:
+        ir (slithIR.operation)
+    Returns:
+        list(str)
+    '''
     sig = '{}({})'
     name = ir.function_name
 
-    args = []
+    # list of list of arguments
+    argss = [[]]
     for arg in ir.arguments:
         if isinstance(arg, (list,)):
             type_arg = '{}[{}]'.format(get_type(arg[0].type), len(arg))
@@ -327,14 +336,31 @@ def get_sig(ir):
             type_arg = arg.signature_str
         else:
             type_arg = get_type(arg.type)
-        args.append(type_arg)
-    return sig.format(name, ','.join(args))
+        if isinstance(arg, Constant) and arg.type == ElementaryType('uint256'):
+            # If it is a constant
+            # We dupplicate the existing list
+            # And we add uint256 and int256 cases
+            # There is no potential collision, as the compiler
+            # Prevent it with a 
+            # "not unique after argument-dependent loopkup" issue
+            argss_new = [list(args) for args in argss]
+            for args in argss:
+                args.append(str(ElementaryType('uint256')))
+            for args in argss_new:
+                args.append(str(ElementaryType('int256')))
+            argss = argss + argss_new
+        else:
+            for args in argss:
+                args.append(type_arg)
+    return [sig.format(name, ','.join(args)) for args in argss]
 
 def convert_type_library_call(ir, lib_contract):
-    sig = get_sig(ir)
-    func = lib_contract.get_function_from_signature(sig)
-    if not func:
-        func = lib_contract.get_state_variable_from_name(ir.function_name)
+    sigs = get_sig(ir)
+    func = None
+    for sig in sigs:
+        func = lib_contract.get_function_from_signature(sig)
+        if not func:
+            func = lib_contract.get_state_variable_from_name(ir.function_name)
     # In case of multiple binding to the same type
     if not func:
         # specific lookup when the compiler does implicit conversion
@@ -363,10 +389,12 @@ def convert_type_library_call(ir, lib_contract):
     return ir
 
 def convert_type_of_high_level_call(ir, contract):
-    sig = get_sig(ir)
-    func = contract.get_function_from_signature(sig)
-    if not func:
-        func = contract.get_state_variable_from_name(ir.function_name)
+    func = None
+    sigs = get_sig(ir)
+    for sig in sigs:
+        func = contract.get_function_from_signature(sig)
+        if not func:
+            func = contract.get_state_variable_from_name(ir.function_name)
     if not func:
         # specific lookup when the compiler does implicit conversion
         # for example

From 0dc548a8b7f6f42c71ab6b70a0670ba7636192a4 Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 11:08:18 +0000
Subject: [PATCH 08/11] Improve the error message if the codebase contains a
 dupplicate contract names

---
 slither/solc_parsing/slitherSolc.py | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/slither/solc_parsing/slitherSolc.py b/slither/solc_parsing/slitherSolc.py
index 2d249dfaa..f16826ce1 100644
--- a/slither/solc_parsing/slitherSolc.py
+++ b/slither/solc_parsing/slitherSolc.py
@@ -10,6 +10,8 @@ from slither.core.slither_core import Slither
 from slither.core.declarations.pragma_directive import Pragma
 from slither.core.declarations.import_directive import Import
 
+from slither.utils.colors import red
+
 class SlitherSolc(Slither):
 
     def __init__(self, filename):
@@ -157,11 +159,18 @@ class SlitherSolc(Slither):
         for contract in self._contractsNotParsed:
             # remove the first elem in linearizedBaseContracts as it is the contract itself
             fathers = []
-            for i in contract.linearizedBaseContracts[1:]:
-                if i in contract.remapping:
-                    fathers.append(self.get_contract_from_name(contract.remapping[i]))
-                else:
-                    fathers.append(self._contracts_by_id[i])
+            try:
+                for i in contract.linearizedBaseContracts[1:]:
+                    if i in contract.remapping:
+                        fathers.append(self.get_contract_from_name(contract.remapping[i]))
+                    else:
+                        fathers.append(self._contracts_by_id[i])
+            except KeyError:
+                logger.error(red('A contract was not find, it is likely that your codebase contains muliple contracts with the same name'))
+                logger.error(red('Truffle does not handle this case during compilation'))
+                logger.error(red('Please read https://github.com/trailofbits/slither/wiki#keyerror-or-nonetype-error'))
+                logger.error(red('And update your code to remove the dupplicate'))
+                exit(-1)
             contract.setInheritance(fathers)
 
         contracts_to_be_analyzed = self.contracts

From bdca73056ce6b16e10c3375b4447f47cc1dc610a Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 11:30:54 +0000
Subject: [PATCH 09/11] Improve subdenomination support

---
 .../expressions/expression_parsing.py         | 25 ++++++++++---------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/slither/solc_parsing/expressions/expression_parsing.py b/slither/solc_parsing/expressions/expression_parsing.py
index c0ac0a1e2..1f79e8e75 100644
--- a/slither/solc_parsing/expressions/expression_parsing.py
+++ b/slither/solc_parsing/expressions/expression_parsing.py
@@ -240,30 +240,31 @@ def filter_name(value):
 def convert_subdenomination(value, sub):
     if sub is None:
         return value
-    value = int(value)
+    # to allow 0.1 ether conversion
+    value = float(value)
     if sub == 'wei':
-        return value
+        return int(value)
     if sub == 'szabo':
-        return value * int(1e12)
+        return int(value * int(1e12))
     if sub == 'finney':
-        return value * int(1e15)
+        return int(value * int(1e15))
     if sub == 'ether':
-        return value * int(1e18)
+        return int(value * int(1e18))
     if sub == 'seconds':
-        return value
+        return int(value)
     if sub == 'minutes':
-        return value * 60
+        return int(value * 60)
     if sub == 'hours':
-        return value * 60 * 60
+        return int(value * 60 * 60)
     if sub == 'days':
-        return value * 60 * 60 * 24
+        return int(value * 60 * 60 * 24)
     if sub == 'weeks':
-        return value * 60 * 60 * 24 * 7
+        return int(value * 60 * 60 * 24 * 7)
     if sub == 'years':
-        return value * 60 * 60 * 24 * 7 * 365
+        return int(value * 60 * 60 * 24 * 7 * 365)
 
     logger.error('Subdemoniation not found {}'.format(sub))
-    return value
+    return int(value)
 
 def parse_expression(expression, caller_context):
     """

From d7ebe32361df47f82a59ec2e7f2444f4ef21a800 Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 13:12:44 +0000
Subject: [PATCH 10/11] Loop parsing: expression -> condition, rather than
 expression -> begin loop

---
 slither/solc_parsing/declarations/function.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/slither/solc_parsing/declarations/function.py b/slither/solc_parsing/declarations/function.py
index 424ffd96e..9ebe7ca1d 100644
--- a/slither/solc_parsing/declarations/function.py
+++ b/slither/solc_parsing/declarations/function.py
@@ -191,9 +191,9 @@ class FunctionSolc(Function):
 
         if loop_expression:
             node_LoopExpression = self._parse_statement(loop_expression, node_body)
-            link_nodes(node_LoopExpression, node_startLoop)
+            link_nodes(node_LoopExpression, node_condition)
         else:
-            link_nodes(node_body, node_startLoop)
+            link_nodes(node_body, node_condition)
 
         if not condition:
             if not loop_expression:
@@ -286,7 +286,7 @@ class FunctionSolc(Function):
         if not hasCondition and not hasLoopExpression:
             link_nodes(node, node_endLoop)
 
-        link_nodes(node_LoopExpression, node_startLoop)
+        link_nodes(node_LoopExpression, node_condition)
 
         return node_endLoop
 

From 7813fdf81d3ec54a54ca2171b4309b9c636ee5e6 Mon Sep 17 00:00:00 2001
From: Josselin <josselin@trailofbits.com>
Date: Mon, 7 Jan 2019 16:39:24 +0000
Subject: [PATCH 11/11] Add support for return(0,) style

---
 slither/slithir/operations/return_operation.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/slither/slithir/operations/return_operation.py b/slither/slithir/operations/return_operation.py
index 21758d4d4..2ef776b13 100644
--- a/slither/slithir/operations/return_operation.py
+++ b/slither/slithir/operations/return_operation.py
@@ -18,6 +18,11 @@ class Return(Operation):
             else:
                 values = [values]
         else:
+            # Remove None
+            # Prior Solidity 0.5
+            # return (0,)
+            # was valid for returns(uint)
+            values = [v for v in values if not v is None]
             self._valid_value(values)
         super(Return, self).__init__()
         self._values = values