fix: clarify what erc1363 does with eoa

[vittominacori]

Since I have participated in a discussion of what this EIP should do with EOA due to lack of specs, I would like to clarify that here.

ERC1363 was made for contracts, so I implicitly assumed it couldn't be used on EOA and missed an explicit statement.
The issue was someone wanted to implement ERC1363 methods like a safeTransfer thus allowing them to be successful on EOA. This is not the original purpose as the name [transfer|transferFrom|approve]AndCall, also, say. We are expecting a call on the receiver or spender after the transfer or the approve. Then a returned value that is the method selector. This cannot be true on EOA.

This PR serves to clarify this missing specification in order to cover the discussions here, here and here.

[eth-bot]

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

---

(fail) eip-1363.md

classification
updateEIP

• eip-1363.md is in state final at the head commit, not draft or last call or review; an EIP editor needs to approve this change
• This PR requires review from one of [@lightclient, @axic, @SamWilsn, @Pandapip1]

[Pandapip1]

@MicahZoltu should we remove the external links?

[Pandapip1]

A few changes I think need to be made to bring it back up to the current EIP format.

[vittominacori]

@Pandapip1 just pushed your suggestion changes

[lukehutch]

@vittominacori Do you need a PR for the changes I suggested?

[vittominacori]

@lukehutch sorry, what changes?

[lukehutch]

@vittominacori see my several comments above.

[vittominacori]

@lukehutch check the updated file. I added all the suggested changes here. Can't find your comments here. Please check the updated file, if you want to suggest something just comment on line

[lukehutch]

@lukehutch check the updated file. I added all the suggested changes here. Can't find your comments here. Please check the updated file, if you want to suggest something just comment on line

I did comment on the file already. Please search this webpage for "lukehutch started a review". I left several comments.

[MicahZoltu]

@lukehutch I also don't see your comments. Did you perhaps forget to click the button that actually submits the review?

[lukehutch]

OK, I see a button "Submit review" -- does this make the comments visible?

By the way some of the lines I clicked on to leave comments aren't the right line, it's only letting me comment on lines that have changes in the PR that I'm commenting on, not on other lines that weren't changed.

[MicahZoltu]

Yep, clicking submit review was the issue. Sometimes I'll leave comments in the Review Comments box if the feedback is for an unchanged line. What you did is fine though.

[vittominacori]

@lukehutch just pushed the requested changes.
I updated params name. I've already done it in my own implementation but not updated the EIP.
As we are updating now it should be safe to rename some params.

[lukehutch]

A few more suggested tweaks and clarifications

[vittominacori]

@Pandapip1 is there any blocking request on this PR or can we go ahead?

[Pandapip1]

I thought I had already submitted this, oops.

[Pandapip1]

Suggested change

	There is no way to execute code after an [EIP-20](./eip-20.md) `transfer` or `approval` (i.e. making a payment), so to make an action it is required to send another transaction and pay GAS twice.
	There is no way to execute code after an [EIP-20](./eip-20.md) `transfer` or `approval` (i.e. payment), so to make an action it is required to send another transaction and pay GAS twice.

Nitpick

[vittominacori]

This has been updated with a new text. Please check here.

https://github.com/ethereum/EIPs/pull/5167/files#diff-d03e52deda369b57949a2bf014c8b41fb8a5c84e072f7fed33b192ed748c2714R24

[Pandapip1]

Suggested change

	This proposal has been inspired also by [ERC-223](https://github.com/ethereum/EIPs/issues/223) and [ERC-677](https://github.com/ethereum/EIPs/issues/677), but it doesn't override the [EIP-20](./eip-20.md) `transfer` and `transferFrom` methods and defines the interfaces IDs to be implemented maintaining the [EIP-20](./eip-20.md) backwards compatibility.
	This proposal has been inspired by many previous informal standards, but it doesn't override the [EIP-20](./eip-20.md) `transfer` and `transferFrom` methods and defines the interfaces IDs to be implemented maintaining backward compatibility with [EIP-20](./eip-20.md).

Happens to also remove all external links

[vittominacori]

Updated here https://github.com/ethereum/EIPs/pull/5167/files#diff-d03e52deda369b57949a2bf014c8b41fb8a5c84e072f7fed33b192ed748c2714R203

[lightclient]

Hi, so I think for one there are too many changes to this EIP to be acceptable for an EIP already in final status. I realize the standard format has changed a bit, but once an EIP is final we try to not change it at all.

Since there are a lot of changes, it's a bit difficult to follow exactly the heart of the issue, but I think I get it. Unfortunately it feels like this is too big of a change to make to a final EIP as it is seriously modifying the expected functionality. If an EIP-1363 contract was previously deployed and we merged this change it would stop conforming to the standard. I think the best action would be to create a new EIP with the correct behavior.

[vittominacori]

@lightclient yes in fact my original commit only changes few words and typos 7dc4bb7 but later reviewer has requested many text changes and to remove any external link to be compliant to new guideline.

Consider that nothing in the original EIP was changed except for a clarification on what it does with EOA.
I contributed to deploy several ERC1363 tokens with my own implementation package and with token generator and all of them already behave as per the current EIP.

Now it is more clean and follow the last guideline on EIP creation.

[lukehutch]

Correct, the core issue was only that the original EIP said that receiver contracts MUST implement the receiver interface and spender contracts MUST implement the spender interface, but it did not specify what happens if the receiver or spender is an EOA. ERC 777 and ERC 4524 explicit allow for successfully sending to EOAs, but the reference implementation of ERC1363 did not. This led to a disagreement over the OpenZeppelin implementation of ERC1363, where I was saying OpenZeppelin should look to the reference implementation to resolve the ambiguity in wording, but the OpenZeppelin guy who created the implementation interpreted the ambiguous wording the opposite way, allowing for sending to, and spending by, EOAs. He said that the reference implementation shouldn't count for anything, because it was not linked from the spec (the link had to be removed before the spec was finalized, at the request of a reviewer), and he pointed out that git repos could be changed at any time anyway.

All the other changes (including any that I suggested, at least the ones that were accepted) are only clarifications to wording or resolutions of ambiguity that do not materially change the semantics of this EIP.

[MicahZoltu]

The question one must ask for changes to final EIPs is:
Is it possible for someone to have deployed something that was compliant before the change and non-compliant after the change. If so, then the change is normative and cannot be included. Generally speaking, this means that there are functionally no changes allowed, including clarification of ambiguous behavior.

I think the best you can do for this EIP in this situation is to add a note in the Rationale or Specification indicating that both interpretations are valid and people interacting with or implementing this EIP cannot make assumptions about which one is correct.

The person you mentioned is correct here, anything not in the Specification section of the EIP is purely supplemental, and the specification is the only thing that matters as far as what the specification is.

[vittominacori]

@MicahZoltu @lightclient @Pandapip1
yes but the original EIP says

Defines a token interface for ERC-20 tokens that supports executing recipient code after transfer or transferFrom, or spender code after approve. and EOA does not allow to execute code.

and then

here there are no references to EOA but it refers to contracts.

And also It allows to make a callback after a transfer or approval in a single transaction. that is not possible with EOA. EOA will never make a callback in a single transaction.

A contract that wants to accept token payments via transferAndCall or transferFromAndCall MUST implement the following interface: ... and A contract that wants to accept token payments via approveAndCall MUST implement the following interface: ... they always refer to contracts.

And again ...with the addition of a callback on receiver or spender. that is not possible on EOA.

And this is the original implementation linked in EIP discussion topic https://github.com/vittominacori/erc1363-payable-token.

I mean that also if the behavior with EOA was not explicit, it is really clear that this EIP does not consider addresses because the purpose was to make contracts able to receive callbacks.

At OpenZeppelin they added a PR to introduce EIP1363 because they want to have a safeTransfer method as expressed here. They want to force EIP1363 transferAndCall to use the safeTransfer behavior taken from ERC4524 in order to not to add both in their library.
I think we must not change the EIP1363 original behavior to just "win" the battle with ERC4524 and be added in OpenZeppelin library. ERC1363 original purpose was to be used with contracts. If we want a safeTransfer that do not revert on EOA we can use ERC4524.

I just wanted to clarify that behavior adding a note in specification (if for some people that was not already clear), then reviewers requested typo and semantic changes so I accepted. If this PR is more than what can be accepted I will close and then open again just with the notes on EOA behavior.

[MicahZoltu]

The specification currently is pretty clear about how a token must behave. When transferAndCall is called, it must transfer the tokens and then call onTransferReceived on the recipient. For an EOA, the onTransferReceived call is a no-op waste of gas but that is the behavior that must be complied with.

While your request to disallow using this method to talk to EOAs may be reasonable, it cannot be made as a change to this EIP and must be a new EIP as it would be a normative change. Specifically, a contract that implements this standard today and calls onTransferReceived on an EOA recipient would suddenly become non-compliant after your change, which means this would be an unacceptable change.

To reiterate here, only the ## Specification section actually matters. The rest is supplemental and any ambiguity or disagreement in wording between the specification section and the other sections is resolved by whatever the specification says (that section is the authority). If there is a disagreement between the specification section and the other sections, we should resolve that by correcting the other sections to match the specification section.

[lukehutch]

@MicahZoltu I disagree that the current EIP should be interpreted this way -- I interpret it the opposite way -- and I am not even the author. The author @vittominacori has made the intent behind the EIP clear here.

[vittominacori]

@MicahZoltu

    function transferAndCall(
        address to,
        uint256 amount,
        bytes memory data
    ) public virtual override returns (bool) {
        transfer(to, amount);
        require(_checkOnTransferReceived(_msgSender(), to, amount, data), "ERC1363: _checkOnTransferReceived reverts");
        return true;
    }

    function _checkOnTransferReceived(
        address sender,
        address recipient,
        uint256 amount,
        bytes memory data
    ) internal virtual returns (bool) {
        if (!recipient.isContract()) {
            return false;
        }
        bytes4 retval = IERC1363Receiver(recipient).onTransferReceived(_msgSender(), sender, amount, data);
        return retval == IERC1363Receiver.onTransferReceived.selector;
    }

this is my suggested implementation.

Also if we remove the if (!recipient.isContract()) { return false; } the retval will never be the requested value retval == IERC1363Receiver.onTransferReceived.selector. So why we are dealing on EOA when it is required to return the selector value? We are speculating on a missing declaration that was clear for me when I introduced the EIP. I do not mention EOA because they are not part of this spec. The only one requesting for EOA to not revert is OpenZeppelin that wanted to use EIP1363 (that is final) with the ERC4524 (draft) behavior.

The EIP1363 was born to make developers able to build callbacks on contracts after a token transfer or approve. They must be sure that the callback has been executed. So there are no reasons to make the token able to transfer and call an EOA because an EOA cannot make any action and this may break the token workflow as it expects the callback to be executed. To transfer to an EOA we already have the transfer method.
If people wanted to use an ambiguous method that sometimes returns a bytes4 and makes a callback and sometimes does nothing without notify nobody they could use ERC4524.

I think that this PR just clarify a thing that was implied in the original EIP.

[lukehutch]

That's a good point -- what if nothing were changed about the EIP other than the suggested implementation be added?

[vittominacori]

That suggested implementation is taken from my own implementation repo https://github.com/vittominacori/erc1363-payable-token

I think we must not edit the EIP adding implementation, but just adding the note about the EOA behavior.
Others are only typos or changes to phrase wording.

[MicahZoltu]

I can appreciate that this EIP may not represent the intent of the authors, but once an EIP is final intent no longer matters. The specification currently says that you need to take a specific action when transferAndCall is made, and it doesn't say that the action is conditional on whether the recipient is an EOA or a contract so that means the action must be taken regardless of whether the recipient is an EOA or a contract.

Unfortunately, the EIP also doesn't specify how the token contract should deal with recipients who do not return the expected return value, so we have to assume that compliant tokens can interpret the return value however they want (including either failing the transfer or allowing the transfer).

My recommendation at this point would be to create a new EIP that makes your intent very clear. When authoring this new EIP, I recommend you go into great detail how a compliant implementation should deal with all scenarios. For example, specify the behavior when the recipient doesn't return the expected value,e and specify the behavior when they do, and specify the behavior when the recipient throws.

[vittominacori]

I can appreciate that this EIP may not represent the intent of the authors, but once an EIP is final intent no longer matters. The specification currently says that you need to take a specific action when transferAndCall is made, and it doesn't say that the action is conditional on whether the recipient is an EOA or a contract so that means the action must be taken regardless of whether the recipient is an EOA or a contract.

Unfortunately, the EIP also doesn't specify how the token contract should deal with recipients who do not return the expected return value, so we have to assume that compliant tokens can interpret the return value however they want (including either failing the transfer or allowing the transfer).

Yes it doesn't specify what to do with other return values, but token specification says to return true unless throwing and that it must call onTransferReceived on receiver:

  /**
   * @notice Transfer tokens from `msg.sender` to another address and then call `onTransferReceived` on receiver
   * @param to address The address which you want to transfer to
   * @param value uint256 The amount of tokens to be transferred
   * @return true unless throwing
   */
  function transferAndCall(address to, uint256 value) external returns (bool);

And receiver says:

  /**
   * @notice Handle the receipt of ERC1363 tokens
   * @dev Any ERC1363 smart contract calls this function on the recipient
   * after a `transfer` or a `transferFrom`. This function MAY throw to revert and reject the
   * transfer. Return of other than the magic value MUST result in the
   * transaction being reverted.
   * Note: the token contract address is always the message sender.
   * @param operator address The address which called `transferAndCall` or `transferFromAndCall` function
   * @param from address The address which are token transferred from
   * @param value uint256 The amount of tokens transferred
   * @param data bytes Additional data with no specified format
   * @return `bytes4(keccak256("onTransferReceived(address,address,uint256,bytes)"))`
   *  unless throwing
   */
  function onTransferReceived(address operator, address from, uint256 value, bytes memory data) external returns (bytes4);

So basically that Any ERC1363 smart contract calls this function on the recipient after a 'transfer' or a 'transferFrom'. and Return of other than the magic value MUST result in the transaction being reverted..

Anyway, I think that what I'm saying is already specified.
In my opinion calling transferAndCall on EOA must revert like a non compliant contract.
Token contracts that do not revert are not compliant with the ERC1363 specification also without adding the below notes.

Note that `transferAndCall`, `transferFromAndCall` MUST revert if the recipient is an EOA address, because EOA recipients do not implement the required ERC1363Receiver interface.  
Note that `approveAndCall` MUST revert if the spender is an EOA address, because EOA spenders do not implement the required ERC1363Spender interface.

So, if we can't add the note to clarify you can also close this PR but it doesn't mean that we can use transferAndCall to transfer tokens to EOA.

[lukehutch]

@MicahZoltu the fact is that both you and the OlenZeppelin implementer interpreted the EIP wrongly, despite what (to me) seems quite clear language that specifies the opposite intent. So please give some latitude to fix this, as allowed by the EIP errata rules.

[MicahZoltu]

The fact that there are multiple interpretations from multiple people is precisely why a change should not be allowed. Standards that reach final are final, even if they include ambiguities that result in multiple implementations.

---

@vittominacori A token can correctly implement this this EIP and interact with a non-compliant receivers since this EIP doesn't specify how a compliant token should behave when interacting with a non-compliant receiver. Essentially, since the EIP doesn't specify a specific behavior when interacting with a non-compliant receiver (such as an EOA) it is up to the individual token to decide how to behave in that scenario.

[vittominacori]

The reason because OpenZeppelin wanted to force use the non intended behavior was to avoid including both 1363 and 4524. There are thousands of tokens built using my suggested implementation and no one reported that issue. If a dev wants to use the transferAndCall he should have clear what the call means so basically that the callback must be executed and not dealing with addresses hoping that the callback was executed. This was my initial point of view.

Anyway if the EIP cannot be updated, please close that PR.
Everyone will be able to build their own implementation and both will be compliant.