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?
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?
I’d remove them altogether (talking about “should return 0/empty string”).
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 theallowance
is set by anowner
.
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.
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
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.
@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.
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.
Great ERC draft. Few comments.
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.