5 ms·
Poking around in that project, I don't see any specific tests of the Wallet contract... would be interested to see what UTs/STs were included if anyone knows wh
by theptip 9y ago
Poking around in that project, I don't see any specific tests of the Wallet contract... would be interested to see what UTs/STs were included if anyone knows where they are -- or was it just manually tested on the testnet? It seems that a basic set of UTs for this contract should have caught this issue.
It's not even in the category of weird timing attacks or complicated internal states allowing unexpected state transitions -- this is a trivial bug that could be spotted by viewing the contract in Remix (the Solidity IDE).
There are certainly some weird edge cases in the Ethereum VM programming model that are hard to UT, and I think that dev efforts in this area are under-resourced (e.g. it's currently impossible to test time-dependent contracts as there's no way of mocking out the clock in the simulated blockchain).
If you are considering writing a smart contract, read and re-read https://github.com/ConsenSys/smart-contract-best-practices https://github.com/ConsenSys/smart-contract-best-practices, and then write UTs for each of these cases so that you can convince yourself that you've covered them.
Relevant to this bug:
> Understand that your public functions are public, and may be called maliciously. Your private data is also viewable by anyone.
- kmf 9y agoI've been writing some smart contracts over the last couple weeks and have learned a lot from OpenZeppelin's solidity repo–you can either use them directly (importing and extending off of them) or use them as a good resource for building tokens, crowdsales, etc–can't recommend them enough. https://github.com/OpenZeppelin/zeppelin-solidity https://github.com/OpenZeppelin/zeppelin-solidity
- hrpnk 9y agoAgreed, it's definitely the way to go. Hope the project will continue to evolve. Both Aragon [0] and Storj [1] are making active use of them, if you look for example usage. [0] https://github.com/aragon/aragon-network-token https://github.com/aragon/aragon-network-token [1] https://github.com/Storj/storj-contracts https://github.com/Storj/storj-contracts
- legolassexyman 9y agoAs I said here last week "Also the lead developer is some kid who believes very much in moving fast and breaking things. This philosophy may work for a social network site but not for other people's money." This philosophy permeates the community of Etherum development. Bitcoin disabled most of the smart contracts op codes for a very good reason. I wouldn't recommend putting anything of value into an Etherum smart contract and find a project with more responsible developers who take a more "wait and see" approach.
- ghayes 9y agoDo you have examples of this, or just hearsay?
- coldtea 9y agoBeyond the one we're already discussing?
- corndoge 9y agothe story you are commenting on is a good example it should not be this easy to lose sixteen million dollars
- patejam 9y agoI feel like we shouldn't be expressed in dollars, but in percentage of total funds. In this case 16 million is about 1% of the total. This would be a $16 billion case if the US banking system was the target and used this technology.
- rubidium 9y agoThat... doesn't make a difference in my mind. Losing 1% of people's money is _very_ unacceptable and worth being laughed at for. Financial institutions are accurate to tiny tiny fractions of percentage points. Not "eh we got 99% of it right".
- hrpnk 9y agoThe 20 cases for underflow and overflow [0] show how difficult it is to develop error-free contracts. Would have been great if the VM would detect such cases and thus make the contract code less polluted with boundary and overflow checks. Some projects have started sharing audit results publicly directly in their repos [1]. It's a great read to see common errors and yet unsolved issues that originate in the ERC20 token specification. [0] https://github.com/ConsenSys/smart-contract-best-practices#integer-overflow-and-underflow https://github.com/ConsenSys/smart-contract-best-practices#i... [1] https://github.com/status-im/status-network-token#reviewers-and-audits https://github.com/status-im/status-network-token#reviewers-...