Skip to content

Commit

Permalink
Remove 'investigator' naming
Browse files Browse the repository at this point in the history
  • Loading branch information
alexroan committed Aug 14, 2024
1 parent cd27c42 commit d8164f5
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::{

use super::WorkspaceCallGraph;

/// Use with [`super::CallGraphInvestigator`]
pub trait CallGraphInvestigatorVisitor {
/// Use with [`super::CallGraph`]
pub trait CallGraphVisitor {
/// Shift all logic to tracker otherwise, you would track state at 2 different places
/// One at the tracker level, and other at the application level. Instead, we must
/// contain all of the tracking logic in the tracker. Therefore, visit entry point
Expand All @@ -33,7 +33,7 @@ pub trait CallGraphInvestigatorVisitor {
}
}

pub struct CallGraphInvestigator {
pub struct CallGraph {
/// Ad-hoc Nodes that we would like to explore from.
pub entry_points: Vec<NodeID>,

Expand All @@ -43,12 +43,12 @@ pub struct CallGraphInvestigator {
pub surface_points: Vec<NodeID>,
}

impl CallGraphInvestigator {
/// Creates a [`CallGraphInvestigator`] by exploring paths from given nodes. This is the starting point.
impl CallGraph {
/// Creates a [`CallGraph`] by exploring paths from given nodes. This is the starting point.
pub fn for_specific_nodes(
context: &WorkspaceContext,
nodes: &[&ASTNode],
) -> super::Result<CallGraphInvestigator> {
) -> super::Result<CallGraph> {
let mut entry_points = vec![];
let mut surface_points = vec![];

Expand Down Expand Up @@ -77,26 +77,23 @@ impl CallGraphInvestigator {
}
}

Ok(CallGraphInvestigator {
Ok(CallGraph {
entry_points,
surface_points,
})
}

pub fn new(
context: &WorkspaceContext,
nodes: &[&ASTNode],
) -> super::Result<CallGraphInvestigator> {
pub fn new(context: &WorkspaceContext, nodes: &[&ASTNode]) -> super::Result<CallGraph> {
Self::for_specific_nodes(context, nodes)
}

/// Visit the entry points and all the plausible function definitions and modifier definitions that
/// EVM may encounter during execution.
pub fn investigate<T>(&self, context: &WorkspaceContext, visitor: &mut T) -> super::Result<()>
pub fn accept<T>(&self, context: &WorkspaceContext, visitor: &mut T) -> super::Result<()>
where
T: CallGraphInvestigatorVisitor,
T: CallGraphVisitor,
{
self._investigate(
self._accept(
context,
context
.callgraph
Expand All @@ -110,14 +107,14 @@ impl CallGraphInvestigator {
/// First, we visit the entry points. Then, we derive the subgraph from the [`WorkspaceCallGraph`]
/// which consists of all the nodes that can be reached by traversing the edges starting
/// from the surface points.
fn _investigate<T>(
fn _accept<T>(
&self,
context: &WorkspaceContext,
callgraph: &WorkspaceCallGraph,
visitor: &mut T,
) -> super::Result<()>
where
T: CallGraphInvestigatorVisitor,
T: CallGraphVisitor,
{
// Visit entry point nodes (so that trackers can track the state across all code regions in 1 place)
for entry_point_id in &self.entry_points {
Expand Down Expand Up @@ -159,7 +156,7 @@ impl CallGraphInvestigator {
blacklist: Option<&HashSet<NodeID>>,
) -> super::Result<()>
where
T: CallGraphInvestigatorVisitor,
T: CallGraphVisitor,
{
if visited.contains(&node_id) {
return Ok(());
Expand All @@ -175,7 +172,7 @@ impl CallGraphInvestigator {
self.make_relevant_visit_call(context, node_id, visitor)?;
}

if let Some(pointing_to) = callgraph.graph.get(&node_id) {
if let Some(pointing_to) = callgraph.raw_callgraph.get(&node_id) {
for destination in pointing_to {
self.dfs_and_visit_subgraph(
*destination,
Expand All @@ -197,7 +194,7 @@ impl CallGraphInvestigator {
visitor: &mut T,
) -> super::Result<()>
where
T: CallGraphInvestigatorVisitor,
T: CallGraphVisitor,
{
if let Some(node) = context.nodes.get(&node_id) {
if node.node_type() != NodeType::FunctionDefinition
Expand Down Expand Up @@ -228,7 +225,7 @@ impl CallGraphInvestigator {
visitor: &mut T,
) -> super::Result<()>
where
T: CallGraphInvestigatorVisitor,
T: CallGraphVisitor,
{
let node = context
.nodes
Expand Down
22 changes: 10 additions & 12 deletions aderyn_core/src/context/callgraph/callgraph_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#[cfg(test)]
mod callgraph_tests {
use crate::context::{
investigator::{CallGraphInvestigator, CallGraphInvestigatorVisitor},
callgraph::callgraph::{CallGraph, CallGraphVisitor},
workspace_context::{ASTNode, WorkspaceContext},
};

Expand Down Expand Up @@ -47,10 +47,10 @@ mod callgraph_tests {

let visit_eighth_floor1 = get_function_by_name(&context, "visitEighthFloor1");

let investigator = CallGraphInvestigator::new(&context, &[&visit_eighth_floor1]).unwrap();
let investigator = CallGraph::new(&context, &[&visit_eighth_floor1]).unwrap();

let mut tracker = Tracker::new(&context);
investigator.investigate(&context, &mut tracker).unwrap();
investigator.accept(&context, &mut tracker).unwrap();

assert!(tracker.func_definitions_names.is_empty());
assert!(tracker.modifier_definitions_names.is_empty());
Expand All @@ -66,11 +66,10 @@ mod callgraph_tests {
let pass_through_ninth_floor2 =
get_modifier_definition_by_name(&context, "passThroughNinthFloor2");

let investigator =
CallGraphInvestigator::new(&context, &[&pass_through_ninth_floor2]).unwrap();
let investigator = CallGraph::new(&context, &[&pass_through_ninth_floor2]).unwrap();

let mut tracker = Tracker::new(&context);
investigator.investigate(&context, &mut tracker).unwrap();
investigator.accept(&context, &mut tracker).unwrap();

assert!(tracker.has_found_functions_with_names(&["visitEighthFloor2"]));
}
Expand All @@ -85,11 +84,10 @@ mod callgraph_tests {
let pass_through_ninth_floor3 =
get_modifier_definition_by_name(&context, "passThroughNinthFloor3");

let investigator =
CallGraphInvestigator::new(&context, &[&pass_through_ninth_floor3]).unwrap();
let investigator = CallGraph::new(&context, &[&pass_through_ninth_floor3]).unwrap();

let mut tracker = Tracker::new(&context);
investigator.investigate(&context, &mut tracker).unwrap();
investigator.accept(&context, &mut tracker).unwrap();

assert!(tracker.has_found_functions_with_names(&["visitEighthFloor3"]));
}
Expand All @@ -103,10 +101,10 @@ mod callgraph_tests {

let recurse = get_function_by_name(&context, "recurse");

let investigator = CallGraphInvestigator::new(&context, &[&recurse]).unwrap();
let investigator = CallGraph::new(&context, &[&recurse]).unwrap();

let mut tracker = Tracker::new(&context);
investigator.investigate(&context, &mut tracker).unwrap();
investigator.accept(&context, &mut tracker).unwrap();

assert!(tracker.has_found_functions_with_names(&["recurse"]));
}
Expand Down Expand Up @@ -135,7 +133,7 @@ mod callgraph_tests {
}
}

impl CallGraphInvestigatorVisitor for Tracker<'_> {
impl CallGraphVisitor for Tracker<'_> {
fn visit_entry_point(&mut self, node: &ASTNode) -> eyre::Result<()> {
self.entry_points
.push(self.context.get_node_sort_key_pure(node));
Expand Down
3 changes: 2 additions & 1 deletion aderyn_core/src/context/callgraph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod investigator;
mod callgraph_tests;
pub mod callgraph;
mod workspace_callgraph;

pub use workspace_callgraph::*;
Expand Down
18 changes: 9 additions & 9 deletions aderyn_core/src/context/callgraph/workspace_callgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ use crate::{

#[derive(Debug)]
pub struct WorkspaceCallGraph {
pub graph: CallGraph,
pub raw_callgraph: RawCallGraph,
}

/**
* Every NodeID in CallGraph should corresponds to [`crate::ast::FunctionDefinition`] or [`crate::ast::ModifierDefinition`]
* Every NodeID in RawCallGraph should corresponds to [`crate::ast::FunctionDefinition`] or [`crate::ast::ModifierDefinition`]
*/
pub type CallGraph = HashMap<NodeID, Vec<NodeID>>;
pub type RawCallGraph = HashMap<NodeID, Vec<NodeID>>;

impl WorkspaceCallGraph {
/// Formula to create [`WorkspaceCallGraph`] for global preprocessing .
pub fn from_context(context: &WorkspaceContext) -> super::Result<WorkspaceCallGraph> {
let mut graph: CallGraph = HashMap::new();
let mut raw_callgraph: RawCallGraph = HashMap::new();
let mut visited: HashSet<NodeID> = HashSet::new();

let funcs = context
Expand All @@ -33,24 +33,24 @@ impl WorkspaceCallGraph {
let modifier_definitions = context.modifier_definitions();

for func in funcs {
dfs_to_create_graph(func.id, &mut graph, &mut visited, context)
dfs_to_create_graph(func.id, &mut raw_callgraph, &mut visited, context)
.map_err(|_| super::Error::WorkspaceCallGraphDFSError)?;
}

for modifier in modifier_definitions {
dfs_to_create_graph(modifier.id, &mut graph, &mut visited, context)
dfs_to_create_graph(modifier.id, &mut raw_callgraph, &mut visited, context)
.map_err(|_| super::Error::WorkspaceCallGraphDFSError)?;
}

Ok(WorkspaceCallGraph { graph })
Ok(WorkspaceCallGraph { raw_callgraph })
}
}

/// Make connections from each of the nodes of [`crate::ast::FunctionDefinition`] and [`crate::ast::ModifierDefinition`]
/// with their connected counterparts.
fn dfs_to_create_graph(
id: NodeID,
graph: &mut CallGraph,
graph: &mut RawCallGraph,
visited: &mut HashSet<NodeID>,
context: &WorkspaceContext,
) -> super::Result<()> {
Expand Down Expand Up @@ -105,7 +105,7 @@ fn dfs_to_create_graph(
Ok(())
}

fn create_connection_if_not_exsits(from_id: NodeID, to_id: NodeID, graph: &mut CallGraph) {
fn create_connection_if_not_exsits(from_id: NodeID, to_id: NodeID, graph: &mut RawCallGraph) {
match graph.entry(from_id) {
hash_map::Entry::Occupied(mut o) => {
// Performance Tip: Maybe later use binary search (it requires keeping ascending order while inserting tho)
Expand Down
14 changes: 6 additions & 8 deletions aderyn_core/src/detect/high/contract_locks_ether.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ mod contract_eth_helper {
ast::{ASTNode, ContractDefinition, StateMutability, Visibility},
context::{
browser::ExtractFunctionDefinitions,
callgraph::investigator::{CallGraphInvestigator, CallGraphInvestigatorVisitor},
callgraph::callgraph::{CallGraph, CallGraphVisitor},
workspace_context::WorkspaceContext,
},
detect::helpers,
Expand Down Expand Up @@ -112,13 +112,11 @@ mod contract_eth_helper {

let mut tracker = EthWithdrawalAllowerTracker::default();

let investigator = CallGraphInvestigator::new(
context,
funcs.iter().collect::<Vec<_>>().as_slice(),
)
.ok()?;
let investigator =
CallGraph::new(context, funcs.iter().collect::<Vec<_>>().as_slice())
.ok()?;

investigator.investigate(context, &mut tracker).ok()?;
investigator.accept(context, &mut tracker).ok()?;

if tracker.has_calls_that_sends_native_eth {
return Some(true);
Expand All @@ -137,7 +135,7 @@ mod contract_eth_helper {
has_calls_that_sends_native_eth: bool,
}

impl CallGraphInvestigatorVisitor for EthWithdrawalAllowerTracker {
impl CallGraphVisitor for EthWithdrawalAllowerTracker {
fn visit_any(&mut self, ast_node: &ASTNode) -> eyre::Result<()> {
if !self.has_calls_that_sends_native_eth
&& helpers::has_calls_that_sends_native_eth(ast_node)
Expand Down
10 changes: 4 additions & 6 deletions aderyn_core/src/detect/high/delegate_call_no_address_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use std::error::Error;
use crate::ast::NodeID;

use crate::capture;
use crate::context::callgraph::investigator::{
CallGraphInvestigator, CallGraphInvestigatorVisitor,
};
use crate::context::callgraph::callgraph::{CallGraph, CallGraphVisitor};
use crate::detect::detector::IssueDetectorNamePool;
use crate::detect::helpers;
use crate::{
Expand All @@ -30,8 +28,8 @@ impl IssueDetector for DelegateCallOnUncheckedAddressDetector {
has_delegate_call_on_non_state_variable_address: false,
context,
};
let investigator = CallGraphInvestigator::new(context, &[&(func.into())])?;
investigator.investigate(context, &mut tracker)?;
let investigator = CallGraph::new(context, &[&(func.into())])?;
investigator.accept(context, &mut tracker)?;

if tracker.has_delegate_call_on_non_state_variable_address
&& !tracker.has_address_checks
Expand Down Expand Up @@ -70,7 +68,7 @@ struct DelegateCallNoAddressChecksTracker<'a> {
context: &'a WorkspaceContext,
}

impl<'a> CallGraphInvestigatorVisitor for DelegateCallNoAddressChecksTracker<'a> {
impl<'a> CallGraphVisitor for DelegateCallNoAddressChecksTracker<'a> {
fn visit_any(&mut self, node: &crate::context::workspace_context::ASTNode) -> eyre::Result<()> {
if !self.has_address_checks && helpers::has_binary_checks_on_some_address(node) {
self.has_address_checks = true;
Expand Down
10 changes: 5 additions & 5 deletions aderyn_core/src/detect/high/msg_value_in_loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use crate::ast::{ASTNode, Expression, NodeID};

use crate::capture;
use crate::context::browser::ExtractMemberAccesses;
use crate::context::callgraph::investigator::{
CallGraphInvestigator, CallGraphInvestigatorVisitor,
use crate::context::callgraph::callgraph::{
CallGraph, CallGraphVisitor,
};
use crate::detect::detector::IssueDetectorNamePool;
use crate::{
Expand Down Expand Up @@ -72,9 +72,9 @@ impl IssueDetector for MsgValueUsedInLoopDetector {

fn uses_msg_value(context: &WorkspaceContext, ast_node: &ASTNode) -> Option<bool> {
let mut tracker = MsgValueTracker::default();
let investigator = CallGraphInvestigator::new(context, &[ast_node]).ok()?;
let investigator = CallGraph::new(context, &[ast_node]).ok()?;

investigator.investigate(context, &mut tracker).ok()?;
investigator.accept(context, &mut tracker).ok()?;
Some(tracker.has_msg_value)
}

Expand All @@ -83,7 +83,7 @@ struct MsgValueTracker {
has_msg_value: bool,
}

impl CallGraphInvestigatorVisitor for MsgValueTracker {
impl CallGraphVisitor for MsgValueTracker {
fn visit_any(&mut self, node: &crate::ast::ASTNode) -> eyre::Result<()> {
if !self.has_msg_value
&& ExtractMemberAccesses::from(node)
Expand Down
10 changes: 4 additions & 6 deletions aderyn_core/src/detect/high/out_of_order_retryable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use crate::ast::{Expression, MemberAccess, NodeID};

use crate::capture;
use crate::context::browser::ExtractFunctionCalls;
use crate::context::callgraph::investigator::{
CallGraphInvestigator, CallGraphInvestigatorVisitor,
};
use crate::context::callgraph::callgraph::{CallGraph, CallGraphVisitor};
use crate::detect::detector::IssueDetectorNamePool;
use crate::detect::helpers;
use crate::{
Expand All @@ -29,8 +27,8 @@ impl IssueDetector for OutOfOrderRetryableDetector {
let mut tracker = OutOfOrderRetryableTracker {
number_of_retry_calls: 0,
};
let investigator = CallGraphInvestigator::new(context, &[&(func.into())])?;
investigator.investigate(context, &mut tracker)?;
let investigator = CallGraph::new(context, &[&(func.into())])?;
investigator.accept(context, &mut tracker)?;
if tracker.number_of_retry_calls >= 2 {
capture!(self, context, func);
}
Expand Down Expand Up @@ -73,7 +71,7 @@ const SEQUENCER_FUNCTIONS: [&str; 3] = [
"unsafeCreateRetryableTicket",
];

impl CallGraphInvestigatorVisitor for OutOfOrderRetryableTracker {
impl CallGraphVisitor for OutOfOrderRetryableTracker {
fn visit_any(&mut self, node: &crate::ast::ASTNode) -> eyre::Result<()> {
if self.number_of_retry_calls >= 2 {
return Ok(());
Expand Down
Loading

0 comments on commit d8164f5

Please sign in to comment.