Hello Ludovic :)

Considering the approve/transferFrom pattern risks, what do you think of removing the approvePreSigned function from it?

There seems to be two potential paths:

1. Have a presigned version of all ERC20 transfer-related functions
2. Exclude the presigned approve(), so developers aren't tempted to use a bad pattern

I lean towards including it, for consistency, but at the end of the day, it's not critical (especially if increaseApproval can achieve the same functionality). What do others think?

Let's review the comments in ethereum/EIPs#865, see if there are any major objections or issues, and merge. @martriay can you handle this, as you're familiar with the code?

The code looks good to me. @lgalabru @ptrwtts would you mind updating the base branch and adding some tests?

The problem with the current implementation is in my opinion that we have a second implementation of transfer(...), transferFrom(...) and all the other functions. Lot of code duplication which would need to be covered with tests. Which would need to be updated in ERC865Token whenever a modification in BasicToken is.

Why not modify BasicToken.sol as follow:

function transfer(address _to, uint256 _value) public returns (bool) {
  transfer(msg.sender, _to, _value);
}

//new function. 
function transfer(address _from, address _to, uint256 _value) internal returns (bool) {
    require(_to != address(0));
    require(_value <= balances[_from]);

    // SafeMath.sub will throw if there is not enough balance.
    balances[_from] = balances[_from].sub(_value);
    balances[_to] = balances[_to].add(_value);
    Transfer(_from, _to, _value);
    return true;
}

In ERC865Token.sol

function transferPreSigned(
    bytes _signature,
    address _to,
    uint256 _value,
    uint256 _fee,
    uint256 _nonce
)
    public
    returns (bool)
{
    require(signatures[_signature] == false);
    bytes32 hashedTx = transferPreSignedHashing(address(this), _to, _value, _fee, _nonce);
    address from = recover(hashedTx, _signature);
    require(from != address(0));

    require(transfer(from, _to, _value));
    require(transfer(from, msg.sender, _fee));

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

The downside would be a bit more GAS usage.

In addition I like the idea of GAS free transactions. However transferPreSigned and transferFromPreSigned could be good enough. To have a PreSigned function for every smart contract function will bloat up the implementation and also the contract code which needs to be deployed.

Hi, I am very interested in ERC865.
I read this pull request and I commented some problems above.
Best,

I agree with some of the reservations about duplication noted by @kabl above. I wonder if an attempt to closely mirror the ERC20 functions leads the code off-target a bit in terms of its functional objectives (at least as I understand them). What I mean by that is the approve and transferFrom pattern makes sense within the context of ERC20 tokens, where we have a "pure" decentralized and fully on-chain implementation; but perhaps for this proposal it is better to more closely mirror the legacy processes that transferFrom was devised to emulate with the help of some off-chain functionality (hear me out).

For instance, when I authorize my credit card to debit my bank account for the amount of my monthly minimum payment, I obviously don't have to do anything like what's done with approve. Again, that process flow makes sense within the context of ERC20's approve and transferFrom functions, but one of the great potentials of ERC865 is to create a more user-friendly experience for novice and/or mainstream users (or those who simply don't want to hold Ether in their wallet). So, within the context of ERC865, I would (humbly) suggest that a basic version of ERC865 reduce the amount of duplication from ERC20 and focus on the main objective to enable Ether-free token transfers. I'll sketch out below a very high-level overview of how only relying on transferPreSigned can be effectively and securely used with less friction for the user and less duplication of code.

Going with the example of a recurring bill payment, we can potentially bring in the best of both worlds here, by automating recurring payments but still requiring the user "approve" each transfer prior to execution. In this way, you can simulate transferFrom without bloating the code base. Let's imagine the following takes place through a browser or mobile interface:

1. The user selects a payment method (wallet address) and an option for automated payments (e.g. monthly minimum payment).
2. At the time of payment processing, the app generates a message containing the required params.
3. The app prompts the user to approve the payment.
4. The user "approves" the payment by signing the transaction.
5. The app sends the signed message to a delegate to "transferFrom" on behalf of the token holder by calling transferPreSigned.

I've followed the same basic pattern here for a different use case and I've also included tests.

Thoughts?

I agree with @jsdavis28, I'd like to see a more general solution to this as opposed to something tightly coupled to ERC20.

That said, the EIP is still a draft, and doesn't seem very active. Closing due to staleness.