Support the examples from the building-secure-contracts repository #1819
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 4 commits
October 30, 2020 12:31
As I understand it, this is necessary to get the [building secure contracts](https://github.com/crytic/building-secure-contracts) repo passing by default. It's also a frequent first step for real-world EVM analysis, so perhaps it will save some time. The tests are performing suspiciously well locally. I would have expected some changes to be required. Let's see how GH Actions handles them.
Previously, contract creation would ignore the `consts.oog` variable and always check that the caller had the requisite balance. This change checks that we haven't explicitly ignored gas calculations when evaluating whether the transaction has enough balance to proceed.
Evidently we relied on the gas behavior in some of these tests. Though admittedly, the failure of test_integer_overflow_storageinvariant seems backward to me.
```
_____________ EthBenchmark.test_integer_overflow_storageinvariant ______________
[gw1] linux -- Python 3.6.12 /opt/hostedtoolcache/Python/3.6.12/x64/bin/python
self = <tests.ethereum_bench.test_consensys_benchmark.EthBenchmark testMethod=test_integer_overflow_storageinvariant>
def test_integer_overflow_storageinvariant(self):
> self._test("integer_overflow_storageinvariant", set())
tests/ethereum_bench/test_consensys_benchmark.py:112:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/ethereum_bench/test_consensys_benchmark.py:57: in _test
self.assertEqual(expected_findings, actual_findings)
E AssertionError: Items in the second set but not the first:
E ('Unsigned integer overflow at ADD instruction', False)
----------------------------- Captured stdout call -----------------
```
ehennenfent
changed the title
feliam
reviewed
Nov 6, 2020
| except TerminateState as e: | ||
| result = str(e) | ||
| self.assertEqual(result, "SELFDESTRUCT") | ||
| consts.oog = default_gas |
Contributor
There was a problem hiding this comment.
This is possible going to fail if the tests get executed concurrently. Config must go with mcore instance and be frozen and etc etc.
Also there this
manticore/manticore/utils/config.py
Lines 73 to 80 in c8e5e25
feliam
approved these changes
Nov 6, 2020
Contributor
feliam
left a comment
feliam
left a comment
There was a problem hiding this comment.
We need better configuration nobs all around evm.
And also config need to be able to be frzen and to be attached to each mcore instance
Contributor
Author
|
The building-secure-contracts repo works with #1820, so we don't need the additional gas and timeout changes present in this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, contract creation would ignore the
consts.oogvariable and always check that the caller had the requisite balance. This change checks that we haven't explicitly ignored gas calculations when evaluating whether the transaction has enough balance to proceed.