when destination.call() fails, funds get destroyed about burner-wallet HOT 6 CLOSED

It is correct but there is another thing to mention. Freeing space in smart contracts storage (delete) gives you extra gas to run in your transaction, this is useful for gas free transactions. As you said, I agree with the smart contract execution which can change the execution flow.
If a transaction reverts, also the delete will be reverted.

from burner-wallet.

Not sure how the extra gas you're mentioning adds to OP. Deleting is fine when you use destination.transfer() which will revert on failure. When it doesn't revert, delete frees some gas, yup.

But I'm curious, maybe I didn't understand your comment?

from burner-wallet.

I'll talk about 3 different cases:

1. Send value to EOA - Probably this is the desired case for burner wallets. If a transaction gets to the delete instruction, it will get gas to run the call and send the value. It will be extremely hard (not impossible) to fail on sending the value with this extra boost.
2. Send value/tx to a contract (Honest smart contract that reverts) - In this case all the transaction states will be reverted, including the delete. However if it gets out of gas on the external contract, the fund will be deleted without revert.
3. Send value/tx to a contract (Bad smart contract actor) - Calling an external contract can change the execution flow by trying reentrancy attacks or calling functions in the original contract that can alter the storage state.

As you said, destination.transfer() will revert in case of failure. Depending on the design, it could be or not the desired effect. destination.send() has the same gas limitation as transfer but it doesn't reverts. It only returns a boolean status.

from burner-wallet.

Thanks for explaining!

1. Do you have some numbers? How much gas is freed vs gas cost of simply sending ETH (2300gas?)
2. According to this answer, you are not correct here. If an honest smart contract reverts, the tx will succeed.
3. Agreed

from burner-wallet.

1. From Ethereum Yellow Paper p.25,7,8
   15000 Refund given (added into refund counter) when the storage value is set to zero from
   non-zero.
   24000 Refund given (added into refund counter) for self-destructing an account.
   Even with this numbers it is difficult to estimate. I added some measurements in the audit tests but they are not very consistent in every run.
   If you find a better way to measure it, please let me know.

2. You are right. External calls return a status value. It becomes an independent call which can revert without affecting the original call.
   To revert the original transaction, some more checks should have been needed:
   require(destination.call.value(value).gas(gasleft())())

from burner-wallet.

I'll get this changed in the contract. Thanks guys!

from burner-wallet.