BitGo / eth-multisig-v2

Multi-Sig Wallet v2, supporting original Wallet.sol methods with additional confirmAndExecute improvements to allow for single-transaction signing by multiple owners.
Apache License 2.0
271 stars 101 forks source link

consider checking the return value of all ".call" and ".send" #2

Closed ethers closed 8 years ago

ethers commented 8 years ago

For example in https://github.com/BitGo/eth-multisig-v2/blob/master/contracts/Wallet.sol#L572-L573 _to.call.value(_value)(_data); MultiTransact(msg.sender, _r, _value, _to, _data);

The .call could fail, but the MultiTransact will still be emitted, and the event could then be misleading ("value wasn't really transferred).

.call() and .send() don't propagate exceptions https://github.com/ethereum/solidity/issues/610 that's why usually the guidance is to manually do "if !.call then throw".

Something to consider. (I know the original wallet.sol might not have done it, and it may have been missed.)

bencxr commented 8 years ago

thanks, we are checking it now