ERC-6909-multi-token-standard

ERCs
,
62

if the wording is kept, then definition would be “no supply”

re rm “should”: do you think the statement should be replaced with something else like “must” or just remove it altogether?

63

I’d remove them altogether (talking about “should return 0/empty string”).

1 Like
64

I’m preparing an ERC draft for the extension I shared above (ERC-6909 Usability & Security Extensions).

I have a question about compliance: if a token does a temporary approval or sets a temporary operator that resets at the end of the function call, is it necessary to emit the Approval or OperatorSet events?

Argument for emitting: an indexer might break if it sees a “transferFrom event” without the corresponding allowance or operator status. Argument for not emitting: significant gas savings (at least one fewer event, possibly two), and there is no need to track the action in an indexer anyway because the value is set transiently.

I would prefer not to emit the event(s) but the ERC states:

Approval MUST be logged when the allowance is set by an owner.
OperatorSet MUST be logged when the operator status is set.

The fact that the values are set transiently feels like a significant change that may justify a lack of events.

CC: @jtriley-eth

Edit: I am realizing the ERC is still technically in Draft status. It could be interesting to encode these relaxed rules in the ERC if the authors are open to it. Namely, that the events may be omitted if the values are only set transiently.

2 Likes
65

good catch. i think it could use a little rewording

thinking we make allowance and isOperator storage variables explicit in event log conditions, so since transient approvals don’t mutate these storage variables, there’s no need to log the event

defs leaning into the ‘no transient logs’ camp since the transient variables being dropped at the end of the transaction would also require an event

though mention of transient variables in the spec may be useful as well

66

The spec should try to be implementation agnostic, so I wouldn’t do it this way.

That said, after thinking about this a bit more I realized that omitting these events may mess up allowance indexers, which we previously said were a Good Thing. So I would make the event optional but recommended in these cases, so it’s up to the implementer to take the risk or not.

Perhaps something like this…?

OperatorSet

MUST be logged in a transaction that results in a change of operator status.
SHOULD be logged for transient changes within a transaction. If omitted, offchain indexers may underestimate allowance.

Approval

MUST be logged in a transaction that results in a change of allowance, unless it was caused by transferFrom.
SHOULD be logged for transient changes within a transaction. If omitted, offchain indexers may underestimate allowance.

67

@jtriley-eth Small nit but in the Metadata Extension you have:

name

The name of the contract.

symbol

The ticker symbol of the contract.

Given that there’s an id input argument, I assume these should be the name and symbol of the given token, not the contract.

68

In the section on “backwards compatibility” it says a wrapper can be implemented for ERC-20.

Is my understanding correct: the idea here is that the ERC-6909 contract would have the ERC-20 methods added, so that it appeared to be a normal ERC-20. However, the methods would all be interacting with just ONE of the collection of ERC-20s that live inside the ERC-6909.

69

Great ERC draft. Few comments.

  1. It could be clearer if the token must implement ERC-165. If the intention is that the token must implement it, please add it explicitly into the proposal. If not, I would argue that it should be included as it’s the most convenient way how to discover tokens transfer functions. ERC20 doesn’t have it, and it’s a big pain for protocols working with several token standards at the same time.
  2. I would vote for removing the transfer function and keeping only transferFrom. Any protocol where transfer logic is independent of transfer implementation (e.g., transfer this asset to this address, not knowing who the operator is) will have to perform the check either way, wasting gas savings from keeping both functions.
70

appreciate your patience on this, quick update:

  • moved to review
  • name/symbol method description updated (per @zachobront)
  • erc-165 requirement is now explicit (per @ashhanai)
71

i’d prefer the transfer & transferFrom, following erc20’s lead & for off-chain interactions for simple transfers, as it’s significantly more expensive to do the latter as an eoa

72

Small nit on the spec for allowance:

allowance
The total amount of a token id that a spender is permitted to transfer on behalf of an owner.

Based on the current spec, it seems that if one calls allowance(owner,spender,id) where spender is an operator for owner, the return value should be type(uint256).max. Based on the reference implementation that is not the case. Should this be clarified?

2 Likes
73

I have deployed an educational, though hopefully also useful example of ERC6909 singleton, Coins.sol, to this address on Ethereum and Base, 0x0000000000009710cd229bf635c4500029651ee8. It is backwards compatible with ERC20. Each new ID creation also creates “twin” ERC20 clone with create2, such that the ERC20 address matches the ERC6909 ID. Calls to the ERC20 also update the ERC6909 state, and vice versa. Also to @arr00 recommendation, the ERC20 allowance method returns max uint256 if an operator is set (though I did not include this in ERC6909 side, as want to follow existing standard recommendation).

1 Like
74

I believe this EIP should be moved to final call if not already given that it is in production for different protocols like ZAMM and UniswapV4.

75

In the current implementation, two offchain tx simulations are needed to see how much is spender permitted to transfer on behalf of an owner in the worst case.
On the other side, if the requirement was understood literally as you pointed out, the exact state wouldn’t always be discoverable – it would be uncertain what the allowance will be after a spender is no longer an operator for an owner.

I think having all the state viewable is far more important than being able to check the amount spendable in a single offchain simulation. So, I think the specification should be altered, and the implementation should remain as is.

Great catch.

1 Like
76

Two reads can be combined into one via a custom reader contract.

1 Like
77

I think the current standard is fine to be finalized as it is.

ERC-6909 was designed with the goal of being minimal in runtime gas for the majority use case. Niceties that can be made off-chain should be off-chain. Footguns that can be patched in UI should be patched in UI.

1 Like
78

One minor suggestion: I believe it would help integrations if tokenId = 0 was disallowed, by not allowing this in the specification.

For integrations, the interface of a transfer function that supports both ERC20 and ERC6909 tokens, can be simplified if it can be assumed that tokenId == 0 means it is an ERC20 token. This is what ZAMM has done: ZAMM/src/ZAMM.sol at main · z0r0z/ZAMM · GitHub And at Centrifuge we have also implemented it in the same way: protocol-v3/src/spoke/BalanceSheet.sol at 290bf9beb7b25e4c8509dac3a5c5079e71929268 · centrifuge/protocol-v3 · GitHub

There is also many cases where tokenId == 0 is impossible to be used already, e.g. Uniswap V4 converts the ERC20 address to a tokenId, so address(0) is not possible: v4-core/src/types/Currency.sol at main · Uniswap/v4-core · GitHub.

1 Like
79

I don’t think this is correct. Id 0 represents the native currency: