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).
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, 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.