Skip to content
This repository has been archived by the owner on Nov 21, 2019. It is now read-only.

Combined WithdrawDeposit #15

Open
HarryR opened this issue Nov 20, 2017 · 0 comments
Open

Combined WithdrawDeposit #15

HarryR opened this issue Nov 20, 2017 · 0 comments

Comments

@HarryR
Copy link
Contributor

HarryR commented Nov 20, 2017

The combined Withdraw and Deposit function is used to transfer a token from one ring to another in a single operation.

This could be 'churning' a token, or it could be transferring the token to a new owner.

For example:

  • Alice deposits 1 Token into Ring, using pubx,puby that she controls
  • Alice must wait for the ring to fill up before it can be moved
  • Alice performs WithdrawDeposit, depositing the token into another Ring with another pubx,puby

To do this Alice generates the Withdraw parameters, and passes the additional Ring address and pubx,puby.

This relies on the replay protection modification from #12 to bind the signature to a specific set of parameters.

The only thing necessary for this too be possible is to extend the 'Withdraw' function to include target address and msg.data, this can be used to implement arbitrary smart contract calls.

This requires additional parameters to the Withdraw function, in the case of sending ETH it would be:

  • target address
  • message data

Care needs to be taken to ensure that this functionality cannot be used to perform internal calls or abuse trust relationships, that's a significant problem - if there is any trust relationship between the Ring and a Parent contract the withdraw function must be barred from using the Parent as the Target.

However, with ERC20 the payable function attribute isn't really applicable - e.g. you can send tokens to a contract and all it does is modify a counter in the Token - but it doesn't invoke the target method on the third-party contract.

output = Token.transferFrom(this, msg.sender, PaymentAmount);

The code would have to be changed to something like:

            if (UsingToken) {            
                if( ! Token.transferFrom(this, Target, PaymentAmount) ) {
                    revert();
                }
            }
            output = Target.call.gas(X).value(PaymentAmount)(TargetData)

Where Target is the contract address, and TargetData is the arbitrary msg.data to be passed.

For more information about handling transfer of Gas from one call to another, see: ethereum/solidity#2999

The problem is - the call needs to be performed last, and we may need to save some gas for a handful of cleanup operations.

The second problem is, if you just want to send a token to a target address, it's not necessary to perform the second call() on the target afterwards.

Need to handle the following conditions:

  • Perform token transfer to non-contract address
  • Perform ETH transfer to non-contract address
  • Perform token transfer to contract (without call)
  • Perform ETH transfer to contract (with call)
  • Perform token transfer to contract (with call)

Lets call these two functions:

  • Withdraw (compatible with current function)
  • ExWithdraw (includes additional parameters)

Third problem:

In Deposit, if UsingToken it performs Token.transferFrom from msg.sender, this doesn't work if the function has been chained from the ExWithdraw function.

When using ERC223 this doesn't seem to be a problem because it supports the tokenFallback function. The from parameter of tokenFallback must be the address of the Ring or Parent.

With ERC20 and ERC223 tokens, it seems that it's not immediately possible to perform arbitrary calls and a commbined WithdrawDeposit method must be used to ensure the right actions are performed.

For the sake of simplicity we are making an assumption that, for now at least, we will not do anything special when withdrawing tokens?


This rule of thumb should be followed:

Anybody can send the message, but only the owner can decide what it does.

This means that msg.sender should be ignored, the owner should instead specify who the token/eth should be transferred too. etc.


ERC223 EIP: ethereum/EIPs#223
ERC223 reference implementation: https://github.com/Dexaran/ERC223-token-standard

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

No branches or pull requests

1 participant