ERC1003, ERC827: ERC20 extensions for sending tokens to smart contracts

General
1

I wish to propose ERC1003 to be included into OpenZeppelin-solidity library.

Pros:

  • no checks required on client side: just get tokens from msg.sender and spend as arguments describe
  • full compatibility with ERC20 approve+transferFrom mechanism, even compatible with some existing smart contracts including DEXes

Cons:

  • any tokens mistakenly sent to the ERC1003-compatible smart contract helper contract became available for everyone to be stolen.

Example Implementation:

import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
import "openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";

contract ERC1003Caller is Ownable {
    function makeCall(address target, bytes calldata data) external payable onlyOwner returns (bool success) {
        (success,) = target.call.value(msg.value)(data);
    }
}

contract ERC1003Token is ERC20 {
    ERC1003Caller public caller_ = new ERC1003Caller();
    address[] internal sendersStack_;

    function transferToContract(address to, uint256 value, bytes memory data) public payable returns (bool) {
        sendersStack_.push(msg.sender);
        approve(to, value);
        require(caller_.makeCall.value(msg.value)(to, data));
        sendersStack_.pop();
        return true;
    }

    function transferFrom(address from, address to, uint256 value) public returns (bool) {
        address source = (from != address(caller_)) ? from : sendersStack_[sendersStack_.length - 1];
        return super.transferFrom(source, to, value);
    }
}
1 Like
2

Hi again @k06a, thanks for this proposal!

I went through the EIP text and had some issues understanding exactly what the ERC is about, perhaps the original intent was lost over the multiple edits? Could go a bit deeper into an explanation behind the motivation? :slight_smile:

2 Likes
3

@nventuro the whole idea behind this EIP is to allow send tokens to smart contract with a single transaction without separate approve+transferFrom transactions. Also, the main difference to other EIPs is that there is no need to check anything additionally like msg.sender or spender or tx.origin. This method example on the abstract DEX will be compatible with both ERC20 2-step deposit and ERC1003 1-step deposit:

function depositToken(IERC20 token, address beneficiary, uint value) public {
    require(token.safeTransferFrom(msg.sender, address(this), value));
    balancesOfAccount[beneficiary][token] += value;
}
1 Like
4

I like the idea of a lightweight ERC20 compatible approve and call standard.
Approve and call in a single transaction is essential in my opinion for good user experience.

Though I wonder now that ERC777 standard is final and has an OpenZeppelin implementation (OpenZeppelin 2.3) if we will see adoption of ERC777 for its approve and call alone.

5

Indeed, it may be useful for scenarios such as this one: Contract standard for accepting any ERC20 (or ERC721?) as payment

1 Like
6

Broadening the scope of this thread to include ERC827 which is very closely related and was recently proposed for inclusion in OpenZeppelin too, by @augustol. (https://github.com/OpenZeppelin/openzeppelin-solidity/pull/1764)

Both of these ERCs propose to extend ERC20 with a function that can trigger a call on a contract, and they both do it through a proxy so that the token's address is not used as msg.sender for arbitrary transactions.

I don't think that using a proxy is such big of an improvement because there's still a single address that is being reused and which can be "controlled" by any token holder. I think this is the core problem, the fact that this single address is shared.

Most importantly though I think these ideas need to be discussed in context. Could the authors share examples of real contract functions that users would be interacting with? @k06a above shared an abstract example, but I would like to see a real world DEX example.

On a different note, have you guys considered using a new proxy address for every new transfer with a call? This would solve the address reuse problem.

1 Like
7

@frangio we can make a temp proxy for each call to avoid “single shared address”.

contract ERC1003OneTimeCaller {
    constructor(address target, bytes data) external payable {
        target.call.value(msg.value)(data);
        selfdestruct(msg.sender);
    }
}
8

@frangio or we can create 1 proxy for each msg.sender.

9

Would you actually need to selfdestruct? I'd expect that constructor to return no bytecode, given that the contract has no code.

This approach may be more expensive in terms of gas, but I haven't really done any measurements on using single-use contracts. Could someone shed some light on this?

1 Like
10

Exactly. It should revert if the target call fails, though, right?

Also interested in how it affects gas costs. create costs exactly the same as call I think, plus a cost proportional to the bytecode, so it shouldn’t be a lot more expensive than the single proxy implementation.

1 Like
11

@frangio this is version for 1 contract per msg.sender:

contract ERC1003CallerPerSender is Ownable {
    function makeCall(address target, bytes calldata data) external payable onlyOwner returns (bool success) {
        (success,) = target.call.value(msg.value)(data);
    }
}

contract ERC1003Token is ERC20 {
    mapping(address => ERC1003CallerPerSender) public callers_;
    mapping(address => address) public senders_;

    function transferToContract(address to, uint256 value, bytes memory data) public payable returns (bool) {
        if (callers_[msg.sender] == ERC1003CallerPerSender(0)) {
            callers_[msg.sender] = new ERC1003CallerPerSender();
            senders_[address(callers_[msg.sender])] = msg.sender;
        }
        approve(to, value);
        require(callers_[msg.sender].makeCall.value(msg.value)(to, data));
        return true;
    }

    function transferFrom(address from, address to, uint256 value) public returns (bool) {
        address source = (senders_[from] == address(0)) ? from : senders_[from];
        return super.transferFrom(source, to, value);
    }
}

This would allow retrieving any tokens from this contract for those who was responsible to use it as proxy caller.

12

Improved implementation and added some additional features. Following code allows to perform exchange almost on any DEX in single transaction: 0x, EtherDelta, BancorNetwork, KyberSwap, Uniswap.

pragma solidity ^0.5.0;
pragma experimental ABIEncoderV2;

import "github.com/OpenZeppelin/openzeppelin-solidity/contracts/token/ERC20/ERC20.sol";

contract ERC1003Proxy {
    address public token;
    address public owner;
    
    constructor(address _owner) public {
        token = msg.sender;
        owner = _owner;
    }
    
    function makeCalls(
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory datas
    )
        public
        payable
    {
        for (uint i = 0; i < targets.length; i++) {
            (bool success,) = makeCall(targets[i], values[i], datas[i]);
            require(success);
        }
    }
    
    function makeCall(
        address target,
        uint256 value,
        bytes memory data
    )
        public
        payable
        returns (bool, bytes memory)
    {
        require(msg.sender == token || msg.sender == owner);
        return target.call.value(value)(data);
    }
}

contract ERC1003Token is ERC20 {
    mapping(address => ERC1003Proxy) public proxies;
    mapping(address => address) public proxyOwners;

    function approveAndMekeCalls(
        address to,
        uint256 amount,
        address[] memory targets,
        uint256[] memory values,
        bytes[] memory datas
    )
        public
        payable
        returns (bool)
    {
        ERC1003Proxy proxy = proxies[msg.sender];
        if (proxy == ERC1003Proxy(0)) {
            proxy = new ERC1003Proxy(msg.sender);
            proxies[msg.sender]= proxy;
            proxyOwners[address(proxy)] = msg.sender;
        }
        
        approve(to, amount);
        proxy.makeCalls.value(msg.value)(targets, values, datas);
        return true;
    }
    
    function allowance(address from, address to) public view returns (uint256) {
        address source = (proxyOwners[from] == address(0)) ? from : proxyOwners[from];
        return super.allowance(source, to);
    }
    
    function transfer(address to, uint256 value) public returns (bool) {
        address destination = (proxyOwners[to] == address(0)) ? to : proxyOwners[to];
        return super.transfer(destination, value);
    }

    function transferFrom(address from, address to, uint256 value) public returns (bool) {
        address source = (proxyOwners[from] == address(0)) ? from : proxyOwners[from];
        return super.transferFrom(source, to, value);
    }
}

Adding 2 following methods to the proxy would allow retrieving full balances after any asset swaps:

function transferAllEtherBalance(address payable to) external {
    require(msg.sender == address(this));
    to.transfer(address(this).balance);
}

function transferAllTokenBalance(IERC20 theToken, address to) external {
    require(msg.sender == address(this));
    theToken.transfer(to, theToken.balanceOf(address(this)));
}
1 Like
13

Hola a todos, thx @frangio for throwing the idea of using one proxy per call. I ended up implementing it and I have to say this looks much better and secure than using one single proxy contract.

Advantage: By having a mapping of nonces for ERC827 senders that execute extra calls and increments each time a call is executed successfully I can protect the proxies against replay attacks.

But the best thing so far is that now the ERC827 receiver can verify that the call that he is receiving form the proxy is actually submitted by the real msg.sender. Since the create2 address computation inputs are the token address, msg.sender, the contract to call and the nonce all this data is available when the ERC827Receiver receives the call.

PR in WT/ERC827 repo: https://github.com/windingtree/erc827/pull/18

ERC827 internal _call funciton that executes the call, creates a proxy for each call.

  function _call(address _to, bytes memory _data) internal {
    bytes32 salt = keccak256(abi.encodePacked(msg.sender, _to, nonces[msg.sender]));
    address proxy = Create2.deploy(salt, proxyBytecode);
    (bool success, bytes memory data) = address(proxy).call.value(msg.value)(
      abi.encodeWithSelector(bytes4(keccak256("callContract(address,bytes)")), _to, _data)
    );
    require(success, "Call to external contract failed");
    nonces[msg.sender].add(1);
  }

ERC827 Receiver can verify that the call is coming from the original msg.sender. The salt used in ERC827 protects it against replay attacks and allows on chain verification.

  function receiveVerifiedTokens(address sender, ERC827 token) public {
    address proxy = Create2.computeAddress(
      address(token),
      keccak256(abi.encodePacked(sender, address(this), token.nonces(sender))),
      token.proxyBytecode()
    );
    require(msg.sender == proxy, "ERC827Receiver: Sender invalid");
  }

Also, the proxy cant end with token or eth balance, and it gets destroyed after the call.

Looking forward to your opinions @frangio @nventuro @abcoathup @k06a

3 Likes
14

The main idea of my proposal is to avoid any checks on the receiver side :frowning:

1 Like
15

Glad you liked the proposal @augustol!

@k06a I really don't understand the scheme proposed in ERC1003, and perhaps others aren't understanding either? It seems very complex. Could you show a simple "end-to-end" example? What does a "receiver" contract look like?

For example, @augustol posted a sample receiveVerifiedTokens function above. This is very valuable! I think it should be in the ERC itself.

A remaining question for me is: does the receiver need to verify that msg.sender is a proxy controlled by sender (the function argument)? It sounds like (at least in most cases) the receiver can just use sender without validating its association with msg.sender. The reason I think this question is important is that it will determine whether widespread adoption of this protocol will be necessary for its success, or if its something that can be adopted by each project as needed.

Concrete examples to think through would be extremely valuable.

Above was mentioned: 0x, EtherDelta, BancorNetwork, KyberSwap, Uniswap. Could you share the links to the functions that the token would be interacting with?

2 Likes
16

@frangio EtherDelta methods:

function depositToken(address token, uint amount) {
    //remember to call Token(address).approve(this, amount) or this contract will not be able to do the transfer on your behalf.
    if (token==0) throw;
    if (!Token(token).transferFrom(msg.sender, this, amount)) throw;
    tokens[token][msg.sender] = safeAdd(tokens[token][msg.sender], amount);
    Deposit(token, msg.sender, amount, tokens[token][msg.sender]);
}

function withdrawToken(address token, uint amount) {
    if (token==0) throw;
    if (tokens[token][msg.sender] < amount) throw;
    tokens[token][msg.sender] = safeSub(tokens[token][msg.sender], amount);
    if (!Token(token).transfer(msg.sender, amount)) throw;
    Withdraw(token, msg.sender, amount, tokens[token][msg.sender]);
}

function trade(address tokenGet, uint amountGet, address tokenGive, uint amountGive, uint expires, uint nonce, address user, uint8 v, bytes32 r, bytes32 s, uint amount) {
    //amount is in amountGet terms
    bytes32 hash = sha256(this, tokenGet, amountGet, tokenGive, amountGive, expires, nonce);
    if (!(
      (orders[user][hash] || ecrecover(sha3("\x19Ethereum Signed Message:\n32", hash),v,r,s) == user) &&
      block.number <= expires &&
      safeAdd(orderFills[user][hash], amount) <= amountGet
    )) throw;
    tradeBalances(tokenGet, amountGet, tokenGive, amountGive, user, amount);
    orderFills[user][hash] = safeAdd(orderFills[user][hash], amount);
    Trade(tokenGet, amount, tokenGive, amountGive * amount / amountGet, user, msg.sender);
}

Could be called like this:

myToken.approveAndCall(
    etherDelta.address,
    myTokenAmount,
    [
        etherDelta.address,
        etherDelta.address,
        etherDelta.address,
        getToken.address
    ],
    [0, 0, 0, 0],
    [
        etherDelta.depositToken(myToken.address, myTokenAmount).encodeABI(),
        etherDelta.trade(
            getToken.address,
            getTokenAmount,
            myToken.address,
            myTokenAmount,
            expiresBlockNumber,
            nonce,
            user,
            v,
            r,
            s,
            amount
        ).encodeABI(),
        etherDelta.withdrawToken(getToken.address, amount).encodeABI(),
        getToken.transfer(account.address, retrunAmount).encodeABI()
    ],
);
1 Like
17

@frangio is my description clear? Or I should write more examples?

1 Like