ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.82k stars 5.24k forks source link

ERC865: Pay transfers in tokens instead of gas, in one transaction #865

Closed lgalabru closed 2 years ago

lgalabru commented 6 years ago

Preamble

EIP: <to be assigned>
Title: Token standard
Author: Ludovic Galabru
Type: Informational
Category: ERC
Status: Draft
Created: 2018-01-30
Requires: EIP20

Simple Summary

Ability for token holders to pay transfer transactions in tokens instead of gas, in one transaction.

Abstract

The following describes one standard function a token contract can implement to allow a user to delegate transfer of tokens to a third party. The third party pays for the gas, and takes a fee in tokens.

Motivation

When it comes to using tokens as utility tokens, we need to strive for a good UX. Introducing wallets and transactions to end users is a challenge, and having to explain that token holders needs ETH to send tokens is adding some friction to the process. The goal of this EIP is to abstract the gas for the end user, by introducing a fee paid in tokens. A third party can then bring the transaction on-chain, pay for the gas of that given transaction and get tokens from that user.

Specification

Process

The user A gets a quote from the delegate D for the value of the fee Y for 1 transaction (depending on gas price + value of token in ETH).

With their private key, the user generates {V,R,S} for the sha3 of the payload P {N,A,B,D,X,Y,T}.

The user sends {V,R,S} and P (unhashed, unsigned) to the delegate.

The delegate verifies that Y and D have not been altered.

The delegate proceeds to submit the transaction from his account D:

T.delegatedTransfer(N,A,B,X,Y,V,R,S)

The delegatedTransfer method reconstructs the sha3 H of the payload P (where T is the address of the current contract and D is the msg.sender).

We can then call ecrecover(H, V, R, S), make sure that the result matches A, and if that’s the case, safely move X tokens from A to B and Y tokens from A to D.

The challenge mainly resides in imitating the Non-standard Packed Mode on the client side, and obtaining the exact same sha3 as the one generated on-chain.

Methods

delegatedTransfer

function transferPreSigned(
    bytes _signature,
    address _to,
    uint256 _value,
    uint256 _fee,
    uint256 _nonce
)
    public
    returns (bool);

Is called by the delegate, and performs the transfer.

Events

TransferPreSigned

event TransferPreSigned(address indexed from, address indexed to, address indexed delegate, uint256 amount, uint256 fee);

Is triggered whenever delegatedTransfer is successfully called.

Implementation proposal

Assuming a StandardToken and SafeMath available, one could come up with the following implementation.

On-chain operation (solidity)

    /**
     * @notice Submit a presigned transfer
     * @param _signature bytes The signature, issued by the owner.
     * @param _to address The address which you want to transfer to.
     * @param _value uint256 The amount of tokens to be transferred.
     * @param _fee uint256 The amount of tokens paid to msg.sender, by the owner.
     * @param _nonce uint256 Presigned transaction number. Should be unique, per user.
     */
    function transferPreSigned(
        bytes _signature,
        address _to,
        uint256 _value,
        uint256 _fee,
        uint256 _nonce
    )
        public
        returns (bool)
    {
        require(_to != address(0));

        bytes32 hashedParams = transferPreSignedHashing(address(this), _to, _value, _fee, _nonce);
        address from = recover(hashedParams, _signature);
        require(from != address(0));

        bytes32 hashedTx = keccak256(from, hashedParams);
        require(hashedTxs[hashedTx] == false);

        balances[from] = balances[from].sub(_value).sub(_fee);
        balances[_to] = balances[_to].add(_value);
        balances[msg.sender] = balances[msg.sender].add(_fee);
        hashedTxs[hashedTx] = true;

        Transfer(from, _to, _value);
        Transfer(from, msg.sender, _fee);
        TransferPreSigned(from, _to, msg.sender, _value, _fee);
        return true;
    }

    /**
     * @notice Hash (keccak256) of the payload used by transferPreSigned
     * @param _token address The address of the token.
     * @param _to address The address which you want to transfer to.
     * @param _value uint256 The amount of tokens to be transferred.
     * @param _fee uint256 The amount of tokens paid to msg.sender, by the owner.
     * @param _nonce uint256 Presigned transaction number.
     */
    function transferPreSignedHashing(
        address _token,
        address _to,
        uint256 _value,
        uint256 _fee,
        uint256 _nonce
    )
        public
        pure
        returns (bytes32)
    {
        /* "48664c16": transferPreSignedHashing(address,address,address,uint256,uint256,uint256) */
        return keccak256(bytes4(0x48664c16), _token, _to, _value, _fee, _nonce);
    }

Off-chain usage (js)


    describe(`if Charlie performs a transaction T, transfering 100 tokens from Alice to Bob (fee=10)`, () => {
      beforeEach(async () => {
        const nonce = 32;
        const from = alice;
        const to = bob;
        const delegate = charlie;
        const fee = 10;
        const amount = 100;
        const alicePrivateKey = Buffer.from('c87509a1c067bbde78beb793e6fa76530b6382a4c0241e5e4a9ec0a0f44dc0d3', 'hex');

        const components = [
          Buffer.from('48664c16', 'hex'),
          formattedAddress(this.token.address),
          formattedAddress(to),
          formattedInt(amount),
          formattedInt(fee),
          formattedInt(nonce)
        ];
        const vrs = ethUtil.ecsign(hashedTightPacked(components), alicePrivateKey);
        const sig = ethUtil.toRpcSig(vrs.v, vrs.r, vrs.s);
        await this.token.transferPreSigned(
          sig,
          to,
          amount,
          fee,
          nonce
          , {from: charlie}).should.be.fulfilled;
      });

Full implementation available

OpenZeppelin/zeppelin-solidity#741

Additional documentation

Transfer Ethereum tokens without Ether — An ERC20 Improvement to Seriously Consider

Copyright

Copyright and related rights waived via CC0.

3esmit commented 6 years ago

I'm working on something simmilar for about a week now, but I've come with a different implementation. I've realized this idea while writing this comment https://ethresear.ch/t/pos-and-economic-abstraction-stakers-would-be-able-to-accept-gas-price-in-any-erc20-token/721/7?u=3esmit as I was told it was probably not going to happen I came up with this solution https://github.com/status-im/ideas/issues/73 A first version was deployed here: https://ropsten.etherscan.io/address/0x9787564e1bd7da95ee9dcdf17cc57a7225084632 And the latest version is available here https://github.com/status-im/contracts/blob/presigned-token/contracts/token/MiniMeTokenPreSigned.sol An example of gas paid in Token transfer: https://ropsten.etherscan.io/tx/0x52a886755876e7f88fed90cae3f58ee8e00cdcaa2dac24382202d0e37ed14059

ptrwtts commented 6 years ago

Nice work @3esmit. Do you have any comments on the pros / cons of this implementation vs yours?

lgalabru commented 6 years ago

3esmit pretty cool! I really like your approach on having a separate method for generating the keccak, that can be re-used with web3 for simplifying the keccak generation off-chain. For the off chain side, my initial approach was the following:

      const fee = 10;
      const amount = 100;
      const token = this.token.address;
      const from = alice;
      const to = bob;
      const delegate = charlie;
      const nonce = 32;

      const bufferedAddress = (address) => {
        return  Buffer.from(ethUtil.stripHexPrefix(address), 'hex');
      };
      const bufferedInt = (int) => {
        return ethUtil.setLengthLeft(int, 32);
      };
      const formattedByte32 = (bytes) => {
        return ethUtil.addHexPrefix(bytes.toString('hex'));
      };

      const components = [
        bufferedAddress(delegate),
        bufferedAddress(token),
        bufferedInt(nonce),
        bufferedAddress(from),
        bufferedAddress(to),
        bufferedInt(amount),
        bufferedInt(fee)
      ];

      const tightPack = Buffer.concat(components);

      const hashedTightPack = ethUtil.sha3(tightPack);

      const alicePrivateKey = Buffer.from('c88b703fb08cbea894b6aeff5a544fb92e78a18e19814cd85da83b71f772aa6c', 'hex');

      const sig = ethUtil.ecsign(hashedTightPack, alicePrivateKey)

      const pubkey = ethUtil.ecrecover(hashedTightPack, sig.v, sig.r, sig.s)
      const address = ethUtil.publicToAddress(pubkey)

      const tx = await this.token.delegatedTransfer(
        nonce,
        from,
        to,
        amount,
        fee,
        sig.v,
        formattedByte32(sig.r),
        formattedByte32(sig.s), {from: charlie});

Pretty verbose and error prone. Now, if we change your implementation from

    function getTransferHash(
        address _to,
        uint256 _value,
        uint256 _gasPrice,
        uint256 _nonce
    )
        constant
        public
        returns(bytes32 txHash)
    {
        //"edde766e": "transferPreSigned(uint8,bytes32,bytes32,address,uint256,uint256,uint256)",
        txHash = keccak256(address(this), bytes4(0xedde766e), _to, _value, _gasPrice, _nonce);
    }

to

    function getTransferHash(
        address _contract
        address _to,
        uint256 _value,
        uint256 _gasPrice,
        uint256 _nonce
    )
        constant
        public
        returns(bytes32 txHash)
    {
        //"12345678": "getTransferHash(address,address,uint256,uint256,uint256)",
        txHash = keccak256(bytes4(0x12345678), _contract,  _to, _value, _gasPrice, _nonce);
    }

We now have a reliable way to build the exact same keccak on chain and off chain, by simply using the ABI of the contract and directly hashing the data coming from:

token.getTransferHash.request(_contract,  _to, _value, _gasPrice, _nonce)

What do you think?

bokkypoobah commented 6 years ago

Here's an implementation that is live - https://github.com/bokkypoobah/BokkyPooBahsTokenTeleportationServiceSmartContract/blob/master/contracts/BTTSTokenFactory.sol#L365-L395

See https://github.com/bokkypoobah/BokkyPooBahsTokenTeleportationServiceSmartContract#how-it-works

3esmit commented 6 years ago

@ptrwtts My approch support arbitrary contract executions in SNT network, so we can even create a new contract and pay the gas relayed by SNT "gas". Aswell don't need quotes from delegates, the system works simillar way ETH gasPrice market works, but instead of being backed by USD value is by ETH value.

@lgalabru I'm not sure why supporting any contract address for building the txHash, initially I did so and used address(this) at it's calling, and should not be a problem. If you prefer that, I suggest setting function as pure.

@bokkypoobah live in which network? Mainnet? I deployed an example of SNTPreSigned at Ropsten, check it out https://ropsten.etherscan.io/address/0xb473fd6c1206655bf2385013b23589f713fe3213

bokkypoobah commented 6 years ago

@3esmit BTTSTokenFactory on mainnet - https://etherscan.io/address/0x14aabc5ade82240330e5be05d8ef350661aebb8a#code and a token - https://etherscan.io/address/0x4ac00f287f36a6aad655281fe1ca6798c9cb727b#code .

And while developing it, I was getting an out of stack space for variables problem in Solidity when using r,s and v, so switched to using sig.

I also added *Check functions so the service provider is able to confirm that the transaction has not already been executed. And to get the exact error before executing the transaction, if the transaction is expected to fail.

lgalabru commented 6 years ago

@3esmit the idea was to use the same method (indeed, pure), on-chain and off chain. From the smart contract, you could have use:

 bytes32 hash = getTransferHash(
        address(this)
        _to,
        _value,
        _gasPrice,
        _nonce
    )

And off-chain, you could have use:

const payload = await token.getTransferHash.request(token.address,  _to, _value, _gasPrice, _nonce);
const data = payload.params[0].data;
const hash = ethUtil.sha3(data);

After testing this approach, the problem is that off chain, when building data, all the arguments are padded vs being tightly-packed on chain. Instead of hashing:

d1d4e623d10f9fba5db95830f7d3839406c6af22932b7a2355d6fecc4b5c0b6bd44cc31df247a2e

you're hashing:

0000000000000000000000000d1d4e623d10f9fba5db95830f7d3839406c6af20000000000000000000000002932b7a2355d6fecc4b5c0b6bd44cc31df247a2e
3esmit commented 6 years ago

Fundamentally to process the contract call you need to be synced with state, the only difference is that you can call that function pure then, there is no other benefict, I would stay with being processed internally because this is a known common header of signature and to benefict of a simplier function signature. Maybe if gets cheaper in gas by using the pure function, then would be interesting, but I don't think this will make a difference overall.

sponnet commented 6 years ago

https://github.com/swarmcity/SCLabs-gasstation-service and https://github.com/swarmcity/SCLabs-gasstation-client

seweso commented 6 years ago

Wouldn't a way simpler solution be to have contracts set gasprice & startgas so miners are allowed to take gas from the contracts balance?

Then nodes/miners would use limited gas per transaction to determine if these transactions are mineable at all (gas price is high enough) and warrant to be re-broadcast over the network. In other words, nodes/miners are executing the code of transactions (where the sender doesn't have enough funds themselves), and which contain 'gasprice & startgas'-opcodes.

Contracts can then convert tokens to gas themselves. Basically wallets would allow users to set gasprice & startgas in the used token, and the contract converts that to ETH. Or if the contract is rather predictable in gas usage make it even easier for users by allowing users to only set gasprice.

aakilfernandes commented 6 years ago

Very cool. This would be extremely useful from a UX perspective (users don't have to hold multiple tokens).

lgalabru commented 6 years ago

@seweso I think the change you're describing needs to happen on the EVM level (and is on the roadmap if my understanding is correct). This is less trivial than just iterating on top of ERC20.

mattdf commented 6 years ago

It would be good to standardize this to cover delegated ETH transfers as well when token = 0x0, for things like withdrawing from ring signature mixers. As otherwise, you would have to "pre-fund" the withdrawing account, thereby linking your deposit and withdraw addresses.

seweso commented 6 years ago

@lgalabru Yes agreed. And it is a direction Ethereum needs to go into.

Do you know how this is called on the roadmap? I couldn't find it.

ojanssens commented 6 years ago

I think this is a great idea and an answer to my problem I’ve been struggling with: https://forum.ethereum.org/discussion/16990/transaction-fees-for-an-erc20-currency

I only have 1 concern though: Wouldn’t this make Ether itself redundant? If miners would be able to directly accept tokens as payment, the utility of Ether would become less relevant except for staking and securing the network (which is still extremely valuable of course).

Philipinho commented 6 years ago

@bokkypoobah, i'm curious to ask. What happens if the tokens are worthless? How will BTTS pay for the gas in ether when the token has no value?

bokkypoobah commented 6 years ago

@Philipinho I plan to provide a smart contract that the token contract owner has to top up with ETH, and this smart contract will buy tokens from the BTTS service provider at a specified rate.

3esmit commented 6 years ago

I think that defining a gasPrice instead a fee is more safe and dynamic, there is no reason for not doing this. It's trivial to calculate the gas cost of an operation inside the smart contract. The global msg.gas returns the remaining gas, so simply read this value at bounds of operation (right after sending the gas).

This is really important for approveAndCallPreSigned, because this function can call arbritary execution in other contract that can be hard to be estimated, so we place the responsability of being safe to signer. gasLimit could be interesting to safety of both sides, but the limiting already exists in native gas.

Please review my work in MiniMeTokenPreSigned.sol and derive from it, it's the same GNU license followed by MiniMeToken.sol (I guess GPLv3, @jbaylina?).

The interface I suggest is the following:

pragma solidity ^0.4.17;

/**
   @notice Implements PreSigned ERC20Token operations (and approveAndCall(address,uint256,bytes);
 */
contract ERC865 {
    /**
     * @notice Include a presigned `"a9059cbb": "transfer(address,uint256)"`
     * @param _signature Signed transfer 
     * @param _to The address of the recipient
     * @param _value The value of tokens to be transferred
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */
    function transferPreSigned(bytes _signature, address _to, uint256 _value, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @notice Include a presigned `""095ea7b3": "approve(address,uint256)"`
     * @param _signature Signed transfer 
     * @param _to The address of the recipient
     * @param _value The value of tokens to be transferred
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */    
    function approvePreSigned(bytes _signature, address _to, uint256 _value, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @notice Include a presigned `"cae9ca51": "approveAndCall(address,uint256,bytes)"`
     * @param _signature Signed transfer 
     * @param _to The address of the recipient
     * @param _value The value of tokens to be transferred
     * @param _extraData option data to send to contract
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */
    function approveAndCallPreSigned(bytes _signature, address _to, uint256 _value, uint256 _extraData, uint256 _gasPrice, uint256 _nonce) public;

}

I'm not absolutely sure about the usage of bytes _signature instead of the offchain processed uint8 v, bytes32 r, bytes32 s. Maybe to save gas we should use it preprocessed?

Supporting additional approveAndCall is important due 2 reasons:

  1. Push forward a real solution to ERC20 aproove + 1 custom call at contract`
  2. Makes possible executing arbtirary operations on other contracts (implementing ApproveAndCallFallBack.sol), even calls with value 0 in approveAndCall (pure data calls), extended even to creation of new contracts - all gas cost relayed through gasPrice offer in contract token.

Including a fixed fee method in side of this 3 signatures would be an option, I'm not totally against it and don't see an actual problem in having this more option, or could be included as a parameter, however I think users will prefer including gasPriced transaction instead of fixedFee because its easier to calculate cost/risk (?).

PreSigned contract calls (intended to relay native gas) should also be standarized, so we can have other types of tokens which can use different call methods and wallets would recognize it. I suggest some types of hashing we can agree on:

//"edde766e": "transfer(address,uint256)",
txHash = keccak256(address(this), bytes4(0xedde766e), keccak256(_to, _value), _gasPrice, _nonce);
txHash = keccak256(address(this), keccak256(bytes4(0xedde766e), _to, _value), _gasPrice, _nonce);
txHash = keccak256(address(this), bytes4(0xedde766e), _to, _value, _gasPrice, _nonce); //seems to be better because cheaper and no reason to tie separatedly the elements.

This way will be easier to understand what is being signed, specially for wallets that want give more details about where that hash came from.

Also the signatures should be directed towards https://github.com/ethereum/EIPs/issues/191 to motivate wallet developers in agreeing in the common signing method defined there.

Arachnid commented 6 years ago

Excellent idea! Why explicitly encode the sender when it's provided by ecrecover, though?

Also, why encode the delegate, instead of letting the first person to get a transaction in harvest the fee?

jdkanani commented 6 years ago

Great. @lgalabru nonce wouldn't work this way. Possible race condition - require(_nonce > nonces[_from]) prevents older transactions to go through, even if sig is valid.

Could use mapping(delegateTxHash => bool).

lgalabru commented 6 years ago

@jdkanani Amazing, I had the race condition in my radar and was dubitative, thanks for your workaround!

@Arachnid Right, we need to pass it, but it doesn't have to be encoded. Good catch

Letting the first person to get a transaction in harvest the fee

Great idea!

@3esmit Using msg.gas price makes sense, but we should probably come up with another name, since "gas price" is usually in wei. Concerning the naming of the methods, what would you think of prefixing the methods with "delegated", instead of suffixing with "presign", for insisting on the fact that someone else is bringing the transaction on chain? Concerning the direction, we probably want to open a pull request on Open Zeppelin (cc @spalladino). Would it work with the constraints of your licence?

izqui commented 6 years ago

I have been working on this idea for a side-project, the pay-protocol. This feature is something that can be achieved in a layer 2 protocol rather than adding this functionality to every token contract. Which also has the ability that every token currently deployed could use it, just by having people that want to use this create a ERC20 allowance to the PayProtocol contract or setting it a ERC777 operator.

In any case, I think it would be super interesting to standardize the offchain part of this, which would allow people to run nodes looking for profitable token transfers and settle them.

3esmit commented 6 years ago

@lgalabru "Delegated" means that you transfered temporarily trust to other, is not the case here, so I think the naming transferPreSigned() or signedTransfer() are more correct, but transferPreSigned actually describe exacly what the function is (a transfer which been pre signed.)

I don't think is bad to use gasPrice as the name, it also describe exactly what is this parameter for, and it works just like regular native ether gasPrice, however it is under the token you're interacting. Just like in native token, gasPrice is set in the minimum unit (wei), tokens that have decimals need to be aware of this. A good UX would handle this fine.

@izqui I think your project is a little bit different because is not the token itself that is handeling the moving, seems like is a TokenController? but should work like the same. About the ethereum signed message, I see that you do this: keccak256(SCHEMA_HASH, keccak256(this, token, from, to, value, expires, pull, push, keccak256(deps))); Maybe we can have your opinion on why you did this and why we also should do something like this.

axcrest commented 6 years ago

How do you handle the ERC-20 approve() requirement? No one can withdraw funds from your token account without you first approving them, and that approve() step costs ETH.

izqui commented 6 years ago

@axcrest you are right that in pay protocol if you already have tokens you need to create an approval to the contract. But if you receive the tokens directly in your pay protocol account, then you never need ether to operate.

@3esmit you are right the token isn't handling the moving and that's why it is very efficient, token transfers are only settled in the token contracts when tokens need to move in or out of the protocol. This allows to settle MiniMe transfers for ~15k gas (if using bulk transfers, ~36k otherwise) instead of +100k that takes if operating directly on the token.

The signed message conforms to the latest https://github.com/ethereum/EIPs/pull/712 spec. It allows for signing providers to show exactly what is going in the hash they are going to be signing.

3esmit commented 6 years ago

@izqui Thanks for the information about signed message spec, I think the community should consensus in a default signed standard ASAP. I liked #712 and I think it should be a subtype (version) of #191. You are right about pay-protocol being cheaper, and it's an interesting solution, and should be used together with approveAndCallPreSigned. The first transaction from someone entering this network could be paid in the token itself directly. I think whats being discussed here and the network you showed us are different things and compliment each other. Users always want cheaper transactions, and also may want to not hold ether.

The main issue this EIP resolves is the need of an account to hold Ether in their balance to move their token. Even being more expansive, it might be cheaper then the process of transferring ether to that account and moving it.

MiniMeTokenPresigned behaves differnt when gasPrice is zero, this can be used for an user which have 2 balances, 1 with ether and other with the token, so they can control this account using gas from other account.

I can upgrade Raiden contracts to be able of accepting approveAndCallPreSigned to open a channel, so users could participate in Raiden network without ever holding any ether. I think this is certainly an improvement for the token that reaches almost the same result as a true economic abstraction, with the restriction that contracts must be implemented ApproveAndCallFallBack.sol properly. I'll prepare examples in the week for StandardBounty contracts and fees being paid with approveAndCallPresigned.

lgalabru commented 6 years ago

A first implementation of this EIP is available here: https://github.com/PROPSProject/props-token-distribution/blob/master/contracts/token/ERC865Token.sol. I'll probably open a pull request on zeppelin after getting some feedbacks from you guys.

@3esmit I'm not convinced by this idea of computing the gas cost on the fly because you're adding more complexity to the method, when you want to be really accurate.

I skipped the approveAndCall, since it's not part of the ERC20 protocol + there is no clear settlement at this point (ERC223 vs ERC827).

ptrwtts commented 6 years ago

@izqui I'm curious how you envisage a decentralized settlement system working for these sorts of transactions?

We realized that if you leave the transactions open for anyone to settle, then it's possible for someone to watch the mempool and front run them.

You can avoid it, but there needs to be some back and forth. First, the user broadcasts their intent to make a transaction. Then delegates provide quotes. Then the user picks a delegate, and signs the transaction with their address included, so that no one else can front run.

In both cases there is a compromise, so not sure which is best.

Arachnid commented 6 years ago

I agree - being conformant with #191 makes much more sense to me.

lgalabru commented 6 years ago

@Arachnid what should I change on https://github.com/PROPSProject/props-token-distribution/blob/master/contracts/token/ERC865Token.sol if we want to conform to #191?

AugustoL commented 6 years ago

@lgalabru love the idea! please tag when you add the PR to zeppelin-solidity, looking forward to have it there!

lgalabru commented 6 years ago

@AugustoL PR available here: https://github.com/OpenZeppelin/zeppelin-solidity/pull/741

3esmit commented 6 years ago

I'm working here with a contract that should be able to use approveAndCallPreSigned, https://github.com/status-im/StandardBounties/blob/bounty-manager/contracts/BountyManagerPreSigned.sol#L51

It's not ready yet, but thats the idea.

nakajo2011 commented 6 years ago

this is cool proposal! I think it is more useful to add that "bytes inputData" argument to a delegate transaction.

3esmit commented 6 years ago

@Illasera If someone willing to pay ether-gas to get a valueless token then is problem of who is including the presigned transaction to the contract.

SimonFricker commented 6 years ago

What if tokens are created with an embedded GAS value. This way the GAS can be deducted from the token, and the token destroyed?

0xbitcoin commented 6 years ago

This is fantastic. The problem I see is that many tokens have been launched and will not add this functionality. Furthermore, you do not need to pass in V R S you can just pass in bytes for the full signature and use ECRecover sample library from Zeppelin.

In order to allow for older ERC20 tokens to have this functionality, I am building a special 'wallet' contract called LavaWallet. This will allow all deposited tokens to be transferred to other parties via this same metholodogy, by relayers who turn in offchain signatures. Unfortunately it will have to deviate from your standard since its a related but different concept.

https://github.com/0xbitcoin/lava-wallet

wadealexc commented 6 years ago

Hey all! @RobertMCForster and I are working to implement Coinvest's COIN token to be ERC865 compatible. Obviously, that's difficult, seeing as ERC865 isn't finalized, but we're coming up with a few workarounds to (hopefully) have a forward-compatible system in place.

I think we've come up with a potential problem with the standard implemented as-is. For some context, the standard implementation of the ERC20 approve function is regarded by many as being vulnerable to a racing condition, wherein the following occurs:

1. Alice approves N tokens to Bob (N > 0)
2. Alice wants to approve M tokens to Bob instead (M > 0). She signs and broadcasts the transaction.
3. Bob notices the transaction, and quickly signs and broadcasts a transferFrom transaction with a much higher gas price.
4. Bob's transferFrom is picked up first by a miner, and Bob transfers N tokens from Alice's balance. Bob's allowance from Alice is now 0.
5. Alice's transaction is picked up by a miner, and Bob is now re-approved to the tune of M tokens.
6. Bob is now able to perform another transferFrom to transfer another M tokens from Alice.

@RobertMCForster and I feel that the ERC865 approvePreSigned and approveAndCallPreSigned functions are particularly vulnerable to this issue, given that if Alice provides some party P with a pre-signed approve or approveAndCall transaction, P now has a ticket to exploit the above racing condition without even needing to rely on a miner picking up their transaction first. If P is the party to which the tokens are approved, they can perform the exploit themselves. If not, they can sell the exploit to the affected party (or even just notify them of the pending approval). Regardless, it is now up to P to behave honestly.

Our proposed solution is two-fold:

First, the functions increaseApprovalPreSigned and decreaseApprovalPreSigned, should be added to the standard as well. approvePreSigned and approveAndCallPreSigned do not necessarily need to be removed - we can leave it to the signer to decide if the interaction is safe or not.

Second, signers need an option to revoke a signed transaction, in the event they believe the broadcasting party will behave maliciously. Or, maybe they simply want to revoke the transaction without having to execute it themselves/increase the nonce with other transactions (depending on the implementation). We suggest the function revokeSignature as a standard function to address this scenario. The ERC865 interface would then be the following:

pragma solidity ^0.4.23;

/**
   @notice Implements PreSigned ERC20Token operations (and approveAndCall(address,uint256,bytes);
 */
interface ERC865 {
    /**
     * @notice Include a presigned `"a9059cbb": "transfer(address,uint256)"`
     * @param _signature Signed transfer 
     * @param _to The address of the recipient
     * @param _value The value of tokens to be transferred
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */
    function transferPreSigned(bytes _signature, address _to, uint256 _value, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @notice Include a presigned `"095ea7b3": "approve(address,uint256)"`
     * @param _signature Signed approval hash 
     * @param _spender The address of the spender
     * @param _value The amount of tokens to approve
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */    
    function approvePreSigned(bytes _signature, address _spender, uint256 _value, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @notice Include a presigned `"cae9ca51": "approveAndCall(address,uint256,bytes)"`
     * @param _signature Signed approval hash 
     * @param _spender The address of the spender
     * @param _value The amount of tokens to approve
     * @param _extraData option data to send to contract
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */
    function approveAndCallPreSigned(bytes _signature, address _spender, uint256 _value, bytes _extraData, uint256 _gasPrice, uint256 _nonce) public;

     /**
     * @notice Include a presigned `"d73dd623": "increaseApproval(address,uint256)"`
     * @param _signature Signed approval hash 
     * @param _spender The address of the spender
     * @param _amount The amount by which the spender's allowance will be increased
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */    
    function increaseApprovalPreSigned(bytes _signature, address _to, uint256 _amount, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @notice Include a presigned `"66188463": "decreaseApproval(address,uint256)"`
     * @param _signature Signed approval hash 
     * @param _spender The address of the spender
     * @param _amount The amount by which the spender's allowance will be decreased
     * @param _gasPrice How much tokens willing to pay per gas
     * @param _nonce Presigned transaction number.
     */    
    function decreaseApprovalPreSigned(bytes _signature, address _to, uint256 _amount, uint256 _gasPrice, uint256 _nonce) public;

    /**
     * @param _signature: Signature to be revoked by the sender
    */
    function revokeSignature(bytes _signature) public;

}

I would recommend one of the two following simple implementations for revokeSignature, but there are several possibilities:

/// Incrementing-nonce based:

// Maps a signer to their current pre-signed transaction nonce
mapping (address => uint256) public nonces;

// Functions only pass if the given signature has not been invalidated by the signer
modifier validSignature(address _signer, uint _nonce) {
    require(_nonce == nonces[_signer] + 1);
    _;
}    

/**
 * @param _signature: Signature to be revoked by the sender. In this case, the signature is irrelevant
*/
function revokeSignature(bytes _signature) public {
    _signature;
    nonces[msg.sender]++;
}

-or-

/// Non-incrementing-nonce based. We will likely use this to avoid a one-to-many scenario where nonces may become out of order and to avoid users needing to wait for a valid pending transaction to be mined before they may sign another transaction (nonces will be input but may be out of order):

// Maps a signer's address to the hash of a signature to a boolean indicating that signature's validity
mapping (address => mapping(bytes32 => bool)) public invalid_signatures;

// Functions only pass if the given signature has not been invalidated by the signer
modifier validSignature(address _signer, bytes _signature) {
    require(invalid_signatures[_signer][keccak256(_signature)] == false);
    _;
}    

/**
 * @param _signature: Signature to be revoked by the sender
*/
function revokeSignature(bytes _signature) public {
    invalid_signatures[msg.sender][keccak256(_signature)] = true;
}
shanefontaine commented 6 years ago

@bokkypoobah Are there any BTTS contracts in production on the main net right now? Have you found anything wrong with your proposal since the time of writing?

bokkypoobah commented 6 years ago

@shanefontaine There was a bug in v1.00 of the smart contract preventing the correct use of the signedApproveAndCall(...) function. This has now been fixed n v1.10. A v1.10 token contract in production - https://etherscan.io/address/0x4ac00f287f36a6aad655281fe1ca6798c9cb727b#code

ptrwtts commented 6 years ago

Nice work @wadeAlexC

We actually came to the same conclusion around increase / decrease approval, and included them in our implementation that was submitted as a PR to Open Zeppelin here: https://github.com/OpenZeppelin/openzeppelin-solidity/pull/741. That code also includes a couple tweaks that were the result of an audit. Unfortunately we didn't update the spec here. Will try to get it updated so that we can work towards a common implementation.

In regards to the ability to revoke signatures, I agree that it seems valuable. I'll circle it internally to see if other's agree, and get an opinion on which implementation seems better.

One final thing that feels like an open question to me, and would be good to get a second opinion on, is whether a delegate should be explicitly defined in the presigned transaction. There was some discussion in the thread, but we never reached a consensus. On one hand, leaving it undefined allows anyone to pick up the transaction and submit it. But on the other hand, this opens it up to front running. My latest thinking is that a delegate should be explicitly specified. You could create a secondary fee market by having delegates first submit quotes, and then the user select someone. This seems better to me than a free-for-all where delegates are front running each other to claim the fee. What do others think?

ethereumdegen commented 6 years ago

I think it could be valuable to have an explicit delegate address specified but make it optional. If it is 0x0 allow anyone to be a delegate? The only downside then is feature creep

gleissonassis commented 6 years ago

I'm trying to use this pattern in my project but I'm getting error so I've checked if the recovered address from the signature was the same and I got different results. Is there something wrong with this piece of code?

transferPreSigned: function(from, to, amount) {
      var fee = 10;
      var chain = Promise.resolve();
      var nonce = 0;
      var hash = null;
      var signature = null;

      return chain
        .then(function(r) {
          nonce = 32;

          return token.methods.transferFromPreSignedHashing(
            token.options.address,
            from.address,
            to.address,
            amount,
            fee,
            nonce).call();
        })
        .then(function(r) {
          hash = r;
          console.log('Hash: ' + r);

          return web3.eth.accounts.sign(hash, from.privateKey);
        })
        .then(function(r) {
          signature = r.signature;
          console.log('Signature: ' + signature);

          return token.methods.recover(
            hash,
            signature).call();
        })
        .then(function(r) {
          console.log('Address (ori)', from.address);
          console.log('Address (rec)', r);
        });
    }

Variables sent to the function:

var from = {
  address: '0xa933582bD31552b04790131dD885C2c7BEE0F0e5',
  privateKey: '0x0ef00d222ef18638bb5ba8bbbca54611d06aeabf26ffd2c9e8c5b760aa6c7bfe'
}

var to = {
  address: '0xf370872B4bAee32A73205b133D9Ad974F3AD437e',
  privateKey: '0x70a0b6f4bb138f3da385c39fd141d4615fdf1cd74a6fd6b590f4e2929d05ad72'
}

var amount = 100

token.options.address = '0x0DE841e872e246B59564F66953E078B59372dB2E'

I'm getting these results:

Hash: 0x49c58ae7dc453bd37ed678e032079abfa15d52cc4f97e3083c5008e5c02f7af6
Signature: 0x712e444e7c6f2c6c2d3f4144ad06cde80bafe35257016678908ecb29a21a6b3e4fa0f473df9d1f56da5dcc6563a8639d7d35eb0f17811c91825c03c80a9d93d91b
Address (ori) 0xa933582bD31552b04790131dD885C2c7BEE0F0e5
Address (rec) 0x139917E0C08a43c542Aa18e934a293533E0336B9

But if I try use this the values are the same:

var sig = web3.eth.accounts.sign(hash, from.privateKey)
var rec = web3.eth.accounts.recover(hash, sig.signature);

console.log('Address (ori)', from.address);
console.log('Address (rec)', rec);
RobertMCForster commented 6 years ago

@gleissonassis This is likely because web3.eth.accounts.sign is using the ERC191 signing prefix whereas the original Solidity is not (I assume "ethUtil", that was used in the lgalabru implementation, doesn't use the prefix).

Check out the 3esmit and bokkypoobah implementations above to see what they do with the prefix. We've also got a full and audited token live at the Coinvest GitHub you can check out.

shrugs commented 6 years ago

uport's MetaTx from ages ago seems relevant here for some prior-art/context: https://github.com/uport-project/uport-identity/blob/develop/contracts/TxRelay.sol

gleissonassis commented 6 years ago

@RobertMCForster i've changed the recover method to add the prefix:

      if (v != 27 && v != 28) {
        return (address(0));
      } else {
        bytes memory prefix = "\x19Ethereum Signed Message:\n32";
        bytes32 prefixedHash = keccak256(prefix, hash);
        return ecrecover(prefixedHash, v, r, s);
      }

And now is working fine, but I will read the code you suggested to find a better way to solve this issue.

Thanks!

lavawallet commented 6 years ago

Hello,

I improved upon this specification and have a wonderful working version live at http://lavawallet.io .

Here is what was improved:

  1. The relayer is not defined on-chain so that anyone can be a relay
  2. There is a defined 'relayerReward' uint instead which defines a number of tokens which will be given from 'From' to 'msg.sender', whoever submits the tx to the mainnet and pays the gwei to do so
  3. There is an expiration field so that the 'signed data' wont last forever, it is expressed in a block number
  4. There is a contractAddress field so that the signed data is locked to a specific contract which holds the tokens and which is conducting the logic for this system (called a Wallet Contract)
  5. There is a 'method' bytes field which specifies the method which is allowed to be remotely called by the relay with this signed offchain data: approve, transfer, withdraw, etc.

All of this data is signed using ECRecovery by Zeppelin

Please see the contract here (yes it works): 0xf226b12c03514571c5a473b2627f5528da46d263

A lava packet is a small JSON file with 10 fields: From, To, WalletAddress, TokenAddress, TokenAmount, RelayerReward, Expires, Nonce, Method and Signature. The first 9 fields describe an ERC20 token transfer which the 'token sender' (the From field) wants to perform. The 10th field, 'signature', is a special cryptographic signature that only the 'token sender' (From) can create, typically using Metamask. Now that this signature exists, any other person may submit this 'lava packet' (which is very similar to a signed bank check) to the LavaWallet contract on the Ethereum Mainnet. When they do this, they will recieve any tokens defined in 'RelayerReward' as compensation since they had to pay gas using ETH.

Working javascript mocha tests are here... 7/7 pass

https://github.com/lavawallet/lava-wallet Thoughts ??? Should this be a different EIP or an improvement to this EIP ???

PaulinJean commented 5 years ago

var to = { address: '0x1846349d593c964B001Dc564fe38fc2099BB3b14', privateKey: 'ea3b591c158733c3c007f1cd6cc7e0455acc7ba72c5d1f0867de57e0e1585a59' }

var amount = 100

token.options.address = '0x1846349d593c964B001Dc564fe38fc2099BB3b14'

AusIV commented 5 years ago

Is there any plan to align this with the finalized version of the EIP-712 spec? It matched a draft of EIP-712 from February, but the addition of a domain separator to the EIP-712 spec has broken the EIP-865 signature format, meaning there will likely be clients incapable of signing the required messages.

znss1989 commented 5 years ago

Does the implementation of this proposal work with EIP155, which includes more elements into hashing?

If block.number >= FORK_BLKNUM and v = CHAIN_ID 2 + 35 or v = CHAIN_ID 2 + 36, then when computing the hash of a transaction for purposes of signing or recovering, instead of hashing only the first six elements (i.e. nonce, gasprice, startgas, to, value, data), hash nine elements, with v replaced by CHAIN_ID, r = 0 and s = 0. The currently existing signature scheme using v = 27 and v = 28 remains valid and continues to operate under the same rules as it does now.