Skip to content

Commit

Permalink
Rename slither-check-upgradability -> slither-check-upgradeability
Browse files Browse the repository at this point in the history
Improve slither-check-upgradeability output
  • Loading branch information
montyly committed Apr 5, 2019
1 parent 40ee332 commit bffa59f
Show file tree
Hide file tree
Showing 22 changed files with 81 additions and 48 deletions.
24 changes: 12 additions & 12 deletions scripts/travis_test_upgradability.sh
Original file line number Diff line number Diff line change
@@ -1,54 +1,54 @@
#!/usr/bin/env bash

### Test slither-check-upgradability
### Test slither-check-upgradeability

DIR_TESTS="tests/check-upgradability"
DIR_TESTS="tests/check-upgradeability"

slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 > test_1.txt 2>&1
slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 > test_1.txt 2>&1
DIFF=$(diff test_1.txt "$DIR_TESTS/test_1.txt")
if [ "$DIFF" != "" ]
then
echo "slither-check-upgradability failed"
echo "slither-check-upgradeability failed"
cat test_1.txt
cat "$DIR_TESTS/test_1.txt"
exit -1
fi

slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2.sol" --new-contract-name ContractV2 > test_2.txt 2>&1
slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2.sol" --new-contract-name ContractV2 > test_2.txt 2>&1
DIFF=$(diff test_2.txt "$DIR_TESTS/test_2.txt")
if [ "$DIFF" != "" ]
then
echo "slither-check-upgradability failed"
echo "slither-check-upgradeability failed"
cat test_2.txt
cat "$DIR_TESTS/test_2.txt"
exit -1
fi

slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug.sol" --new-contract-name ContractV2 > test_3.txt 2>&1
slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug.sol" --new-contract-name ContractV2 > test_3.txt 2>&1
DIFF=$(diff test_3.txt "$DIR_TESTS/test_3.txt")
if [ "$DIFF" != "" ]
then
echo "slither-check-upgradability failed"
echo "slither-check-upgradeability failed"
cat test_3.txt
cat "$DIR_TESTS/test_3.txt"
exit -1
fi

slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug2.sol" --new-contract-name ContractV2 > test_4.txt 2>&1
slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contractV1.sol" ContractV1 --solc solc-0.5.0 --new-version "$DIR_TESTS/contractV2_bug2.sol" --new-contract-name ContractV2 > test_4.txt 2>&1
DIFF=$(diff test_4.txt "$DIR_TESTS/test_4.txt")
if [ "$DIFF" != "" ]
then
echo "slither-check-upgradability failed"
echo "slither-check-upgradeability failed"
cat test_4.txt
cat "$DIR_TESTS/test_4.txt"
exit -1
fi

slither-check-upgradability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contract_initialization.sol" Contract_no_bug --solc solc-0.5.0 > test_5.txt 2>&1
slither-check-upgradeability "$DIR_TESTS/proxy.sol" Proxy "$DIR_TESTS/contract_initialization.sol" Contract_no_bug --solc solc-0.5.0 > test_5.txt 2>&1
DIFF=$(diff test_5.txt "$DIR_TESTS/test_5.txt")
if [ "$DIFF" != "" ]
then
echo "slither-check-upgradability failed"
echo "slither-check-upgradeability failed"
cat test_5.txt
cat "$DIR_TESTS/test_5.txt"
exit -1
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
entry_points={
'console_scripts': [
'slither = slither.__main__:main',
'slither-check-upgradability = utils.upgradability.__main__:main',
'slither-check-upgradeability = utils.upgradeability.__main__:main',
'slither-find-paths = utils.possible_paths.__main__:main'
]
}
Expand Down
4 changes: 0 additions & 4 deletions tests/check-upgradability/test_1.txt

This file was deleted.

6 changes: 0 additions & 6 deletions tests/check-upgradability/test_2.txt

This file was deleted.

6 changes: 0 additions & 6 deletions tests/check-upgradability/test_3.txt

This file was deleted.

6 changes: 0 additions & 6 deletions tests/check-upgradability/test_4.txt

This file was deleted.

File renamed without changes.
File renamed without changes.
File renamed without changes.
7 changes: 7 additions & 0 deletions tests/check-upgradeability/test_1.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CompareFunctions:Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)
INFO:CompareFunctions:No function ids collision found
INFO:VariablesOrder:Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Variable in the proxy: destination address
INFO:VariablesOrder:No variables ordering error found between implementation and the proxy
11 changes: 11 additions & 0 deletions tests/check-upgradeability/test_2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CompareFunctions:Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)
INFO:CompareFunctions:No function ids collision found
INFO:VariablesOrder:Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Variable in the proxy: destination address
INFO:VariablesOrder:No variables ordering error found between implementation and the proxy
INFO:VariablesOrder:Run variables order checks between implementations... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:No variables ordering error found between implementations
11 changes: 11 additions & 0 deletions tests/check-upgradeability/test_3.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CompareFunctions:Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)
INFO:CompareFunctions:Shadowing between proxy and implementation found myFunc()
INFO:VariablesOrder:Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Different variables between proxy and implem: destination address -> destination uint256
INFO:VariablesOrder:Run variables order checks between implementations... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Different variables between v1 and v2: destination address -> destination uint256
INFO:VariablesOrder:New variable: myFunc uint256
11 changes: 11 additions & 0 deletions tests/check-upgradeability/test_4.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Initializable contract not found, the contract does not follow a standard initalization schema.
INFO:CompareFunctions:Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)
INFO:CompareFunctions:No function ids collision found
INFO:VariablesOrder:Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Different variables between proxy and implem: destination address -> val uint256
INFO:VariablesOrder:Run variables order checks between implementations... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Different variables between v1 and v2: destination address -> val uint256
INFO:VariablesOrder:New variable: destination address
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
INFO:CheckInitialization:Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)
INFO:CheckInitialization:Contract_lack_to_call_modifier.initialize does not call initializer
INFO:CheckInitialization:Missing call to Contract_no_bug.initialize in Contract_not_called_super_init
INFO:CheckInitialization:Contract_no_bug.initialize() is called multiple time in Contract_double_call
Expand All @@ -8,6 +9,8 @@ Contract_not_called_super_init needs to be initialized by initialize()
Contract_no_bug_inherits needs to be initialized by initialize()
Contract_double_call needs to be initialized by initialize()

INFO:CompareFunctions:No function id collision found
INFO:CompareFunctions:Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)
INFO:CompareFunctions:No function ids collision found
INFO:VariablesOrder:Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)
INFO:VariablesOrder:Variable in the proxy: destination address
INFO:VariablesOrder:No error found (variables ordering proxy <-> implementation)
INFO:VariablesOrder:No variables ordering error found between implementation and the proxy
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@
from .check_initialization import check_initialization

logging.basicConfig()
logging.getLogger("Slither-check-upgradability").setLevel(logging.INFO)
logging.getLogger("Slither-check-upgradeability").setLevel(logging.INFO)
logging.getLogger("Slither").setLevel(logging.INFO)


def parse_args():

parser = argparse.ArgumentParser(description='Slither Upgradability Checks',
usage="slither-check-upgradability proxy.sol ProxyName implem.sol ContractName")
parser = argparse.ArgumentParser(description='Slither Upgradeability Checks. For usage information see https://github.com/crytic/slither/wiki/Upgradeability-Checks.',
usage="slither-check-upgradeability proxy.sol ProxyName implem.sol ContractName")


parser.add_argument('proxy.sol', help='Proxy filename')
Expand All @@ -40,11 +40,11 @@ def main():
args = parse_args()

proxy_filename = vars(args)['proxy.sol']
proxy = Slither(proxy_filename, solc=args.solc, disable_solc_warnings=True)
proxy = Slither(proxy_filename, solc=args.solc, disable_solc_warnings=True, truffle_ignore_compile=True, embark_ignore_compile=True)
proxy_name = args.ProxyName

v1_filename = vars(args)['implem.sol']
v1 = Slither(v1_filename, solc=args.solc, disable_solc_warnings=True)
v1 = Slither(v1_filename, solc=args.solc, disable_solc_warnings=True, truffle_ignore_compile=True, embark_ignore_compile=True)
v1_name = args.ContractName

check_initialization(v1)
Expand All @@ -53,7 +53,7 @@ def main():
compare_function_ids(v1, v1_name, proxy, proxy_name)
compare_variables_order_proxy(v1, v1_name, proxy, proxy_name)
else:
v2 = Slither(args.new_version, solc=args.solc, disable_solc_warnings=True)
v2 = Slither(args.new_version, solc=args.solc, disable_solc_warnings=True, truffle_ignore_compile=True, embark_ignore_compile=True)
v2_name = v1_name if not args.new_contract_name else args.new_contract_name
check_initialization(v2)
compare_function_ids(v2, v2_name, proxy, proxy_name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ def check_initialization(s):

initializable = s.get_contract_from_name('Initializable')

logger.info(green('Run initialization checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#initialization-checks)'))

if initializable is None:
logger.info(yellow('Initializable contract not found, the contract does not follow a standard initalization schema.'))
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ def get_signatures(c):


def compare_function_ids(implem, implem_name, proxy, proxy_name):

logger.info(green('Run function ids checks... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#functions-ids-checks)'))

implem_contract = implem.get_contract_from_name(implem_name)
if implem_contract is None:
logger.info(red(f'{implem_name} not found in {implem.filename}'))
Expand All @@ -40,9 +43,12 @@ def compare_function_ids(implem, implem_name, proxy, proxy_name):
for (k, _) in signatures_ids_implem.items():
if k in signatures_ids_proxy:
found = True
logger.info(red('Function id collision found {} {}'.format(signatures_ids_implem[k],
signatures_ids_proxy[k])))
if signatures_ids_implem[k] != signatures_ids_proxy[k]:
logger.info(red('Function id collision found {} {}'.format(signatures_ids_implem[k],
signatures_ids_proxy[k])))
else:
logger.info(red('Shadowing between proxy and implementation found {}'.format(signatures_ids_implem[k])))

if not found:
logger.info(green('No function id collision found'))
logger.info(green('No function ids collision found'))

Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

def compare_variables_order_implementation(v1, contract_name1, v2, contract_name2):

logger.info(green('Run variables order checks between implementations... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)'))

contract_v1 = v1.get_contract_from_name(contract_name1)
if contract_v1 is None:
logger.info(red('Contract {} not found in {}'.format(contract_name1, v1.filename)))
Expand Down Expand Up @@ -47,10 +49,12 @@ def compare_variables_order_implementation(v1, contract_name1, v2, contract_name
logger.info(green('New variable: {} {}'.format(name, t)))

if not found:
logger.info(green('No error found (variables ordering between implementations)'))
logger.info(green('No variables ordering error found between implementations'))

def compare_variables_order_proxy(implem, implem_name, proxy, proxy_name):

logger.info(green('Run variables order checks between the implementation and the proxy... (see https://github.com/crytic/slither/wiki/Upgradeability-Checks#variables-order-checks)'))

contract_implem = implem.get_contract_from_name(implem_name)
if contract_implem is None:
logger.info(red('Contract {} not found in {}'.format(implem_name, implem.filename)))
Expand Down Expand Up @@ -91,6 +95,6 @@ def compare_variables_order_proxy(implem, implem_name, proxy, proxy_name):
# logger.info(green('Variable only in implem: {} {}'.format(name, t)))

if not found:
logger.info(green('No error found (variables ordering proxy <-> implementation)'))
logger.info(green('No variables ordering error found between implementation and the proxy'))


0 comments on commit bffa59f

Please sign in to comment.