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

ERC1003 Token Standard (ERC20 Extension) #1003

Closed
k06a opened this issue Apr 15, 2018 · 17 comments
Closed

ERC1003 Token Standard (ERC20 Extension) #1003

k06a opened this issue Apr 15, 2018 · 17 comments
Labels

Comments

@k06a
Copy link
Contributor

k06a commented Apr 15, 2018

EIP: 1003
Title: ERC1003 Token Standard (ERC20 Extension)
Author: Anton Bukov <k06aaa@gmail.com>
Type: Token Standard
Category: ERC
Created: 2018-04-15
Updated: 2018-08-09

This standard is still a draft and is proven to be unsafe to be used

Simple Summary

An extension of the standard interface ERC20 for tokens with method that allows safely pass and handle tokens into smart contracts.

Abstract

I propose to move tokens from spender to destination contract over fake caller, which will be msg.sender.

Also this standard protects from ERC20 stealing, example:

function depositTokenCanBeStolen(address _spender, uint taskId, uint _value) public {
    require(token.transferFrom(_spender, this, _value));
    tasksBalance[_spender] += _value;
}

In case of separate calls for ERC20: approve and depositTokenCanBeStolen tokens of the stranger can be used for the task he doesn't wanna support. Someone approves tokens, and another person can spend it, that's why usually developers use tx.origin – but this prevents tx chain length scaling.

ERC827 comparison

  • In ERC827 msg.sender tells that _spender pays, you can check it only using tx.origin.
  • In ERC1003 msg.sender pays himself for _beneficiary, nothing to worry about.

Token

ERC20 Methods

All usual ERC20 methods.

ERC1003 Methods

transferToContract - ERC1003

Implementation example:

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

contract ERC1003Caller is Ownable {
    function makeCall(address _target, bytes _data) external payable onlyOwner returns (bool) {
        return _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 _data) public payable returns (bool) {
        sendersStack_.push(msg.sender);
        approve(_to, _value);
        require(caller_.makeCall.value(msg.value)(_to, _data));
        sendersStack_.length -= 1;
        return true;
    }

    function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
        address from = (_from != address(caller_)) ? _from : sendersStack_[sendersStack_.length - 1];
        return super.transferFrom(from, _to, _value);
    }
}

Example of smart contract, compatible with both ERC20 and ERC1003 :

function depositToken(address _beneficiary, uint _value) public {
    require(token.transferFrom(msg.sender, this, _value));
    balanceOfAccount[_beneficiary] += _value;
}

Also can deposit all allowed tokens:

function depositToken(address _beneficiary) public {
    uint256 amount = token.allowed(msg.sender, this);
    require(token.transferFrom(msg.sender, this, amount));
    balanceOfAccount[_beneficiary] += amount;
}

The only mandatory argument is _beneficiary.

@k06a k06a changed the title ERC999: New transferring token to smart contract mechanism ERC999: New transferring token to smart contract mechanism (DRAFT) Apr 15, 2018
@k06a k06a changed the title ERC999: New transferring token to smart contract mechanism (DRAFT) ERC1003: New transferring token to smart contract mechanism (DRAFT) Apr 15, 2018
@k06a k06a changed the title ERC1003: New transferring token to smart contract mechanism (DRAFT) ERC1003 Token Standard (ERC20 Extension) Apr 15, 2018
@mad2sm0key
Copy link

You are transferring tokens to the tokens contract. I'm not sure that's a great idea. Also, if you want the receiver contract to get the tokens from the token contract you need to approve the amount first (before the .call(_data)). The receiving functions need to be payable as well if you intend to transfer value (eth).
Everyone that uses your transfer will pass the tokens through the token contract. I don't feel this approach is a good idea

@k06a
Copy link
Contributor Author

k06a commented Apr 15, 2018

@mad2sm0key thanks! Forgot to add this code, this was code from my mind.

@k06a
Copy link
Contributor Author

k06a commented Apr 15, 2018

@mad2sm0key think of it as of everyone is spending only it’s own tokens. And you will see, this idea will solve ERC827 tx.origin issues. Also payable is optional, is msg.value is zero destination method can be not payable as in provided example.

@mad2sm0key
Copy link

@k06a The updated code makes more sense now. However, your example depositToken can be used to deposit tokens for any account. There is no authentication. Maybe one is not needed in your particular case. That's pretty much the same as in ERC827. You can't really know who the actual sender is.

@k06a
Copy link
Contributor Author

k06a commented Apr 15, 2018

@mad2sm0key the msg.sender pays with tokens for any _benefeciary he wants. This code is fully compatible with both ERC20 (2 tx) and ERC1003 (1 tx). Improved code to support surrender.

@k06a
Copy link
Contributor Author

k06a commented Apr 15, 2018

Hey @spalladino @shrugs @facuspagnuolo what do you think?

@mad2sm0key
Copy link

@k06a How much gas does this use ? Oh, and the surrender is a nice idea.

@k06a
Copy link
Contributor Author

k06a commented Apr 17, 2018

!!!NOT VALID ANYMORE, IMPLEMENTATION CHANGED COMPLETELY!!!

ERC827 and ERC1003 gas comparison

https://gist.github.com/k06a/e3fb2b2b4cfed193338193f88d5dc376

ERC827

Field Value
status 0x1 Transaction mined and execution succeed
from 0xca35b7d915458ef540ade6068dfe2f44e8fa733c
to MyToken.approve(address,uint256,bytes) 0x692a70d2e424a56d2c6c27aa97d1a86395877b3a
gas 3000000 gas
transaction cost 62256 gas
execution cost 52016 gas
hash 0x12833914473d7d06ec75b9e25dd084220f86d642e7b971521854f1d6a80d331a
input 0x5c17f9f4
000000000000000000000000bbf289d846208c16edc8474705c748aff07732db
0000000000000000000000000000000000000000000000000000000000000064
0000000000000000000000000000000000000000000000000000000000000060
0000000000000000000000000000000000000000000000000000000000000044
0bb92e1a
000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c
0000000000000000000000000000000000000000000000000000000000000064
00000000000000000000000000000000000000000000000000000000
decoded input {
    "address _spender": "0xbbf289d846208c16edc8474705c748aff07732db",
    "uint256 _value": "100"
}
decoded output {
    "0": "bool: true"
}
logs [
    {
        "topic": "8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925",
        "event": "Approval",
        "args": [
            "ca35b7d915458ef540ade6068dfe2f44e8fa733c",
            "bbf289d846208c16edc8474705c748aff07732db",
            "100"
        ]
    },
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "ca35b7d915458ef540ade6068dfe2f44e8fa733c",
            "bbf289d846208c16edc8474705c748aff07732db",
            "100"
        ]
    }
]
value 0 wei

ERC1003 without surrender

Field Value
status 0x1 Transaction mined and execution succeed
from 0xca35b7d915458ef540ade6068dfe2f44e8fa733c
to MyToken.transferToContract(address,uint256,bytes) 0x692a70d2e424a56d2c6c27aa97d1a86395877b3a
gas 3000000 gas
transaction cost 108161 gas
execution cost 112921 gas
hash 0x37fa283222a1d39d648331ca8420bb0860aa9b90bd6025cb47021b166624e877
input 0x3f35d033
000000000000000000000000bbf289d846208c16edc8474705c748aff07732db
0000000000000000000000000000000000000000000000000000000000000064
0000000000000000000000000000000000000000000000000000000000000060
0000000000000000000000000000000000000000000000000000000000000044
badccf2d
000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c
0000000000000000000000000000000000000000000000000000000000000064
00000000000000000000000000000000000000000000000000000000
decoded input {
    "address _to": "0xbbf289d846208c16edc8474705c748aff07732db",
    "uint256 _value": "100",
    "bytes _data": "0xbadccf2d000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c0000000000000000000000000000000000000000000000000000000000000064"
}
decoded output {
    "0": "bool: true"
}
logs [
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "ca35b7d915458ef540ade6068dfe2f44e8fa733c",
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "100"
        ]
    },
    {
        "topic": "8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925",
        "event": "Approval",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "bbf289d846208c16edc8474705c748aff07732db",
            "100"
        ]
    },
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "bbf289d846208c16edc8474705c748aff07732db",
            "100"
        ]
    }
]
value 0 wei

ERC1003 with surrender

Field Value
status 0x1 Transaction mined and execution succeed
from 0xca35b7d915458ef540ade6068dfe2f44e8fa733c
to MyToken.transferToContract(address,uint256,bytes) 0x692a70d2e424a56d2c6c27aa97d1a86395877b3a
gas 3000000 gas
transaction cost 103299 gas
execution cost 108059 gas
hash 0x36a9544f551d94dd3890668dfc45c355602a9bb939da550b4e9d114b620f4f6e
input 0x3f35d033
000000000000000000000000bbf289d846208c16edc8474705c748aff07732db
0000000000000000000000000000000000000000000000000000000000000064
0000000000000000000000000000000000000000000000000000000000000060
0000000000000000000000000000000000000000000000000000000000000044
badccf2d
000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c
0000000000000000000000000000000000000000000000000000000000000032
00000000000000000000000000000000000000000000000000000000
decoded input {
    "address _to": "0xbbf289d846208c16edc8474705c748aff07732db",
    "uint256 _value": "100",
    "bytes _data": "0xbadccf2d000000000000000000000000ca35b7d915458ef540ade6068dfe2f44e8fa733c0000000000000000000000000000000000000000000000000000000000000032"
}
decoded output {
    "0": "bool: true"
}
logs [
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "ca35b7d915458ef540ade6068dfe2f44e8fa733c",
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "100"
        ]
    },
    {
        "topic": "8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925",
        "event": "Approval",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "bbf289d846208c16edc8474705c748aff07732db",
            "100"
        ]
    },
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "bbf289d846208c16edc8474705c748aff07732db",
            "50"
        ]
    },
    {
        "topic": "ddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "event": "Transfer",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "ca35b7d915458ef540ade6068dfe2f44e8fa733c",
            "50"
        ]
    },
    {
        "topic": "8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925",
        "event": "Approval",
        "args": [
            "692a70d2e424a56d2c6c27aa97d1a86395877b3a",
            "bbf289d846208c16edc8474705c748aff07732db",
            "0"
        ]
    }
]
value 0 wei

@mg6maciej
Copy link
Contributor

While this might solve some issues with #827, it's vulnerable the same way #827 is, so I'd suggest putting a big warning "This standard is still a draft and is proven to be unsafe to be used" (copied from #827) at the top of this issue and closing it.
If you want to learn more, please read discussion on #827 and here: https://gitter.im/ERC827/Lobby

@k06a
Copy link
Contributor Author

k06a commented Jul 2, 2018

@mg6maciej ok, but why to close this issue?

@mg6maciej
Copy link
Contributor

mg6maciej commented Jul 2, 2018

Unless you want to actively work on making it secure (maybe together with @AugustoL, author of #827), it's better if less eyes see it. Open issues are just a bit easier to browse though. Closed issue is also a warning sign. See #821, which was an alternative to #721 before they joined forces. Maybe @AnthonyAkentiev from Thetta/Thetta-DAO-Framework would not reference this and #827 if they were both closed before.
And of course you could reopen it, if you find time to propose nonvulnerable form.

@k06a
Copy link
Contributor Author

k06a commented Jul 18, 2018

One more try:

contract ERC1003Caller is Ownable {
    function makeCall(address _target, bytes _data) external payable onlyOwner returns (bool) {
        // solium-disable-next-line security/no-call-value
        return _target.call.value(msg.value)(_data);
    }
}
contract ERC1003Token {
    ERC1003Caller public caller_;
    address[] internal sendersStack_;

    constructor() public {
        caller_ = new ERC1003Caller();
    }

    function approveAndCall(address _to, uint256 _value, bytes _data) public payable returns (bool) {
        sendersStack_.push(msg.sender);
        approve(_to, _value);
        require(caller_.makeCall.value(msg.value)(_to, _data));
        sendersStack_.length -= 1;
        return true;
    }

    function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
        if (_from != caller_) {
            return super.transferFrom(_from, _to, _value);
        }

        require(sendersStack_.length > 0);
        address from = sendersStack_[sendersStack_.length - 1];
        return super.transferFrom(from, _to, _value);
    }
}

@k06a
Copy link
Contributor Author

k06a commented Jul 31, 2018

Or even shorter version of transferFrom overriding:

function transferFrom(address _from, address _to, uint256 _value) public returns (bool) {
    return super.transferFrom(_from != caller_ ? _from : sendersStack_[sendersStack_.length - 1], _to, _value);
}

@k06a
Copy link
Contributor Author

k06a commented Aug 9, 2018

Updated theme message

@k06a
Copy link
Contributor Author

k06a commented Sep 10, 2018

It seems ERC1003 will work for ERC721

@github-actions
Copy link

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Dec 18, 2021
@github-actions
Copy link

github-actions bot commented Jan 1, 2022

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

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

No branches or pull requests

3 participants