Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It's not possible to remove ETH/Tokens from a canceled vault. #20

Open
jbaylina opened this issue Apr 20, 2017 · 6 comments
Open

It's not possible to remove ETH/Tokens from a canceled vault. #20

jbaylina opened this issue Apr 20, 2017 · 6 comments

Comments

@jbaylina
Copy link
Contributor

A specific function should be created to allow that.

@omnibusinc
Copy link

Should it be sent to the main vault, or to an address provided by the caller?

@dipenjoshi
Copy link

Ideally if a vault is cancelled. all of the balance should go to parent vault by default.
** Out of context: A vault shouldn't be cancel-able if there are child vaults under them. ( to not break the chain)

@jbaylina
Copy link
Contributor Author

Right now, when you cancel a voult, you cancel all the vaults under that vault also. And the Ether is sent to the parent vault.
The problem is that after canceling some body else can still send Ether to the vault and there will not be a way to recover that Ether.

@omnibusinc
Copy link

omnibusinc commented Apr 27, 2017

What should happen to the Ether sent to the cancelled vault? Should it just be sent to the primaryVault via sendBackOverflow by setting the vault's highestAcceptableBalance to 0 when it is canceled?

@GriffGreen
Copy link
Member

if you try to send ether to a canceled vault it should throw right? it doesnt have a payable fall back function.... the real issue is if someone send ETH in a creative way (like suicide()) or if someone sends tokens (there is no way to stop this).

Now, this is where I would hope that the escape hatch can be used. I know jordi doesn't like planning to use it in the code, but this is an edge case that I think it would cover... In my mind this is not a real issue because of the escape hatch is here for these edge cases...

I want to illustrate a relevant example:

When we were making the hard fork withdraw contract for The DAO, we kept it very simple because there is security in simplicity:

https://etherscan.io/address/0xbf4ed7b27f1d666546e30d74d50d173d20bca754#code

The trusteeWithdraw()function was intended to allow us (The Curators) to get the money out for the extraBalance and ChildDAO refunds.

However, I use the trusteeWithdraw() a lot lately as an escapeHatch for the DAOWithdrawContract because people keep sending ETH and DAO tokens to it!!! :-P

This is not it's intended use, but it works great, it's actually kind of funny but if you send DAO tokens to the withdraw contract, and tell me about it i can call trusteeWithdraw() and get ether sent to the Curator multisig and we can send it to you....

This would be the same mechanism we have if we use the escapeHatch and it would save us A LOT of complexity, and i don't think it is a misuse of that function.

@adria0
Copy link

adria0 commented Apr 27, 2017

Another option is to add a isPayable flag into the Escapable, throw if someone calls a payable when not set. When a vaultcontroller is cancelled the vault is setPayable(false)

contract Escapable {
    bool public isPayable = true;
    function setPayable(bool _isPayable) onlyOwner {
        isPayable = _isPayable;
    }
    function receiveEther() payable {
        if (!ispayable) throw;
        // Do not accept ether if baseToken is not ETH
        if (address(baseToken) != 0) throw;
        EtherReceived(msg.sender, msg.value);
    }
    function () payable {
        receiveEther();
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants