LayerXcom / verified-vyper-contracts

FVyper: A collection of useful Vyper contracts developed with formal methods
Apache License 2.0
55 stars 15 forks source link

Add ERC20MintableBurnable test #43

Closed yudetamago closed 5 years ago

yudetamago commented 5 years ago

Translate erc20_test_1.py ( https://github.com/ethereum/vyper/blob/97ed5fec89947d41fd91f5190c4478ffcadbb9fb/tests/examples/tokens/ERC20_solidity_compatible/test/erc20_tests_1.py ) to pytest

nrryuya commented 5 years ago

remove test_failed_send_in_withdraw because it tests solidity code?

I think the solidity contract is there to call the Vyper contract as an external contract.

yudetamago commented 5 years ago

I think the solidity contract is there to call the Vyper contract as an external contract.

ah, I got it.

nrryuya commented 5 years ago

remove emitting no event assertions in test_raw_logs because checking events with call operation or failed tx may not be supported in py-evm?

py-evm is not used in our repo, is it? I think event is not emitted when EVM reverts?

yudetamago commented 5 years ago

py-evm is not used in our repo, is it?

Ah, I misunderstood, eth_tester is used. (EDITED)

I think event is not emitted in revert?

Right, the original tests check it. https://github.com/ethereum/vyper/blob/97ed5fec89947d41fd91f5190c4478ffcadbb9fb/tests/examples/tokens/ERC20_solidity_compatible/test/erc20_tests_1.py#L272-L280 EDITED: My question is that there is something corresponding to this in eth_tester?

nrryuya commented 5 years ago

remove tests about overflow (test_maxInts, test_bad_transfer, test_bad_deposit, and test_bad_transferFrom) because overflow assertion is supported by default in vyper

I think they are necessary to check all the corner cases. If we have a variable X whose range is from A to B, we need to test with A and B for "complete testing". The statement that the tx fail in overflows is the implicit specification so we need to check it.

BTW,tThe issue below was fixed in Vyper compiler so we can mint MAX_UINT256. https://github.com/ethereum/vyper/blob/97ed5fec89947d41fd91f5190c4478ffcadbb9fb/tests/examples/tokens/ERC20_solidity_compatible/test/erc20_tests_1.py#L175

nrryuya commented 5 years ago

The statement that the tx fail in overflows is the implicit specification so we need to check it.

This isn't limited to overflow because any type has range. e.g. This code causes revert.

    a: int128 = MAX_INT128 - 1
    a += 2
nrryuya commented 5 years ago

I don't know why it's in the original but test for Solidity ERC20 can be omitted? https://github.com/ethereum/vyper/blob/97ed5fec89947d41fd91f5190c4478ffcadbb9fb/tests/examples/tokens/ERC20_solidity_compatible/test/erc20_tests_1.py#L393

nrryuya commented 5 years ago

~All what remains are the "bad_code" test. I'll work on this.~

EDIT: Done in #51

yudetamago commented 5 years ago

yeah!