Consensys / surya

A set of utilities for exploring Solidity contracts
Apache License 2.0
1.07k stars 118 forks source link

WIP: Simple CFG #140

Closed maurelian closed 4 years ago

maurelian commented 4 years ago

This handles my desire for #139

It is close to fully functional with a few bugs to work out.

Bug here, the main calling contract, and most of the callee contracts should appear as defined:

open_egYusng6

Bug here, the using for syntax is not being handle properly, ie. totalBaseTokens.mul() results in a new node that shouldn't be created.

open_t8tBVXmo

tintinweb commented 4 years ago
tintinweb commented 4 years ago

Here's a diff that removes the duplicate contract nodes (one in grey that was formerly a cluster and one in white). This should also make sure that defined contracts are highlighted in light-grey. I am, however, not sure if this diff is valid in all cases. can you verify @GNSPS ? 🤗

alternative fix: remove the light grey nodes from the legend and only have white nodes.

diff --git a/src/graphSimple.js b/src/graphSimple.js
index 3fc5387..38054d8 100644
--- a/src/graphSimple.js
+++ b/src/graphSimple.js
@@ -182,10 +182,11 @@ export function graphSimple(files, options = {}) {
         functionsPerContract[contractName] = [];
         contractUsingFor[contractName] = {};

-        if (!(contractNode = digraph.getNode('contractName'))) {
-          contractNode = digraph.addNode('contractName');
+        if (!(contractNode = digraph.getNode(contractName))) {
+          contractNode = digraph.addNode(contractName);
+        }

-          contractNode.set('label', contractName + kind);
+          contractNode.set('label', contractName);
           contractNode.set('color', colorScheme.contract.defined.color);
           if (colorScheme.contract.defined.fontcolor) {
             contractNode.set('fontcolor', colorScheme.contract.undefined.fontcolor);
@@ -200,13 +201,6 @@ export function graphSimple(files, options = {}) {

           // colorScheme.contract.defined.bgcolor && contractNode.set('bgcolor', colorScheme.contract.defined.bgcolor);

-        } else {
-          if (colorScheme.contract.defined.style) {
-            contractNode.set('style', colorScheme.contract.defined.style);
-          } else {
-            contractNode.set('style', 'filled');
-          }
-        }

         dependencies[contractName] = node.baseContracts.map(spec =>
           spec.baseName.namePath
@@ -247,7 +241,21 @@ export function graphSimple(files, options = {}) {
     // find all the contracts, and create anode for them
     parser.visit(ast, {
       ContractDefinition(node) {
-        node = digraph.addNode(node.name);
+        if (!(node = digraph.getNode(node.name))) {
+          node = digraph.addNode(node.name);
+          node.set('label', contractName);
+          node.set('color', colorScheme.contract.defined.color);
+          if (colorScheme.contract.defined.fontcolor) {
+            contranodectNode.set('fontcolor', colorScheme.contract.undefined.fontcolor);
+          }
+
+          if (colorScheme.contract.defined.style) {
+            node.set('style', colorScheme.contract.defined.style || "filled");
+            // contractNode.set('bgcolor', colorScheme.contract.defined.color);
+          } else {
+            node.set('style', 'filled');
+          }
+        }
       }
     });
maurelian commented 4 years ago

Nice, I applied those changes.

The only thing left is dealing with the calls to using ... for ... library methods:

niftygraph

tintinweb commented 4 years ago

@maurelian added a fix that removes nodes that are created for member-access of local/state non-userdefined types. Fixes your problem but might add minor inaccuracies to the graph.

maurelian commented 4 years ago

OK, I think this is good enough for now. Shall we merge?

tintinweb commented 4 years ago

let me know when this is released and I'll add it to the auditor extension 🥳

tintinweb commented 4 years ago

LGTM 👍

refs: https://github.com/ConsenSys/vscode-solidity-auditor/pull/49 - "contract interaction graph" feature