Consensys / surya

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

Find correct contract name for UsingForStatement. #107

Closed aoli-al closed 4 years ago

aoli-al commented 5 years ago

This pl helps ftrace to find correct library contract when using statement is used. However, this pl does not cover all cases. For example, it doesn't catch the function call to a temporary variable:

using SafeInt for uint256;

function foo() {
  uint256 a;
  a = a.add(1);
}
GNSPS commented 5 years ago

Thank you for contributing @Leeleo3x! 😄 I'll review and merge this real soon!

maurelian commented 5 years ago

This would be really nice to have, but especially in graph, where safemath adds a lot of noise:

surya-graph

aoli-al commented 5 years ago

I do see some duplicate logic in different components. Maybe those code should be shared.

maurelian commented 5 years ago

@Leeleo3x any thoughts about the right way to think about that?

maurelian commented 4 years ago

Hey, thanks for persisting here.

I tested your branch on this file: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol

Using the command ./bin/surya ftrace 'IncreasingPriceCrowdsale::_getTokenAmount' all ~/openzeppelin-contracts/contracts/crowdsale/price/IncreasingPriceCrowdsale.sol

I get

/Users/primary/Projects/Development/surya/node_modules/yargs/yargs.js:1133
      else throw err
           ^

TypeError: Cannot read property 'hasOwnProperty' of undefined
    at Object.FunctionCall (/Users/primary/Projects/Development/surya/lib/ftrace.js:424:44)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:111:30)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:118:7)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:118:7)
    at /Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:102:14
    at Array.forEach (<anonymous>)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:101:10)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:118:7)
    at visit (/Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:118:7)
    at /Users/primary/Projects/Development/surya/node_modules/solidity-parser-antlr/dist/index.js:102:14
aoli-al commented 4 years ago

Hi,

Yes I also notice this issue. It is because while using libraries for primitives such as safeint, surya needs to distinguish between inter-contract call and library call. This requires surya to have type information. For example,

using SafeInt for uint256;
uint256 totalSupply;
totalSupply.add(123);

while analyzing totalSupply.add(123), surya first needs to know totalSupply is uint256 and then it knows add is from SafeInt library.

Commit ff0b0e6 is my personal workaround and I did not intend to commit it to upstream. I may not have time to implement it correctly. Should I close this PR?