Hard coded dated seems to be outdated and `PreICOProxyBuyer` State enums doesn't match with `Crowdsale` enums
Opened this issue · 5 comments
https://github.com/TokenMarketNet/ico/blob/d80e6d1afe8569512e0b53e6e927354ef476b7bf/ico/tests/contracts/test_deploy_acceptance.py#L70-L72
In the code above it can be seen that the date has been hardcoded. Howver, the date has already passed away.
https://github.com/TokenMarketNet/ico/blob/d80e6d1afe8569512e0b53e6e927354ef476b7bf/ico/tests/contracts/test_deploy_acceptance.py#L97-L107
The above mentioned date is used to instantiate PreICOProxyBuyer.
The PreICOProxyBuyer contract has the following states:
https://github.com/TokenMarketNet/ico/blob/d80e6d1afe8569512e0b53e6e927354ef476b7bf/contracts/PreICOProxyBuyer.sol#L74
The getState function has the following logic:
https://github.com/TokenMarketNet/ico/blob/d80e6d1afe8569512e0b53e6e927354ef476b7bf/contracts/PreICOProxyBuyer.sol#L305-L318
and the test has the following assertion:
https://github.com/TokenMarketNet/ico/blob/d80e6d1afe8569512e0b53e6e927354ef476b7bf/ico/tests/contracts/test_deploy_acceptance.py#L107
Since now >= freezeEndsAt, getState will return State.Refunding which has a enum value of 3,
and so the above assertion will fail.
The test was added on 28/08/2017 and the freezeEndsAt date being 20/09/2017, the test might have worked well at the time of adding.
What I don't understand is that this test passed successfully till date, but after upgrading populus the test seems to fail.
how can it be fixed?
Simply set the date to a distant future value.
@villesundell Here's some investigation. The old populus used eth-testrpc, but the new populus uses eth-tester for testing.
eth-tester sets timestamp of the genesis block as the current_time:
https://github.com/ethereum/eth-tester/blob/41c6fd155db0f9cb1e79fdfef5b83c10f42d6248/eth_tester/backends/pyevm/main.py#L141
eth-testrpc has a hardcoded timestamp https://github.com/ethereum/pyethereum/blob/ddaac54b35ecbfb9d49fc48967722a77c8e0c4f4/ethereum/tester.py#L182.
time_travel has nothing to do with this. The hardcoded value fooled us to believe that our test-suite was working fine
Re ^ just setting the date to a distant future value in the test, as a fix: how about instead adding a delta to current timestamp?
BTW looks like we have another implementation as well (with a nice way to set the freeze end):
And both of these seem to have wrong return type in the annotation.
We don't have mypy enabled yet to tell us about annotation bugs