Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update EIP-7702: add delegation designation #8677
Update EIP-7702: add delegation designation #8677
Changes from 4 commits
87eefe1
7ec0e0e
f141b99
7cd84a1
5080bc2
fa64e0b
e649f2b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or already delegated
this is basically a check that the code of theauthority
is in the format of0xef01 || address
, right?I assume this check is to prevent that this flow in case that any other code besides the delegation is set, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very confusing.
AFAIK, the whole purpose was to define temporary delegations.
If it is "already delegated" it means:
authorizer
entry, since it is already on-chainThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is intended to maintain the 3607 requirement that EOAs may not have code. We made a special allowance for delegation designations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. so remove the "temporary" in text: it is no longer temporary, but permanent assignment of a "proxy" address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the previous spec this used to follow naturally from processing the list in order because the second and later valid tuples for the same authority would fail the check "verify that code is empty", but this is no longer the case since adding "... or already delegated".
I don't see why valid tuples should be ignored in this design. In particular, I think there could be two tuples with
nonce
andnonce + 1
that could need to be submitted together to "catch up" to the latest one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think multiple tuples of the same address should be forbidden: the signer has all the information to detect and remove duplicate entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this. The signatures may already have been generated and the signer may not be available to create a new signature that combines them into one.
I see no reason to reject duplicates if each occurrence is paid for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would the sender of the 7702 transaction include duplicates and pay for them, even if allowed? It won't make both implementations available, only one of them. Any further tuples for the same account seem like a waste of gas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I have two signed delegations from the same account with nonces
n
andn + 1
, and I need to submit an op from this account that requires the latest delegation, I need to submit both... The first one is needed to increment the nonce so that the second one becomes valid.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically you're using it as a way to increment the nonce within the same transaction, while only expecting to use the latest delegation. The previous delegation(s) won't be usable during the transaction (or later), so it's just a way to bump the nonce.
Seems fine to me, but I think there were past concerns about bumping nonces by more than 1 in a single transaction. I'm not sure what they were. From DoS perspective a single transaction can invalidate all pending transactions by emptying the eth from the account so multiple bumps don't really increase the risk. Need feedback from core devs who opposed to it before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the concerns were about being able to bump nonces by an arbitrary amount. In this case it's always incrementing by 1 (per signature) so I don't imagine there would be an issue with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to clarify the behavior of chains longer than 2. E.g. EOA1 points to EOA2, which points to a contract. Do we:
Loops can be detected when setting the delegation, since the last delegation has to point to an already-delegated account. But non-loop chains can't be, since the delegations can be ordered such that each account delegates to an empty account which only gets a delegation later. Therefore it has to be handled in the opcodes during actual delegation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that and i believe it makes no sense to support chainning. Imo, if EOA1 points to EOA2 where EOA2 has a designated address, it just 3xecut3d
EF
which is abort in evm and that is it. No need to handle this as a special case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think it makes no sense to support chaining. Either treat it as an empty account, or let the
EF
execute and abort. Just thought it makes sense to define the behavior so that it's not parsed recursively.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simplest answer would be to require the delegation to go to live contract accounts and not EOAs or delegated EOAs. Getting rid of pointing to EOAs gets rid of the chaining and possible looping. But we also need to ban delegating to empty addresses as they could counterfactually become either an EOA or a Contract with no way to predict which until it happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the best way is to define that delegation is not recursive: if you delegate to an already-delegated entity, the "magic" doesn't happen twice: you end up pointing to a bad code (starting with
0xef
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drortirosh it is already done here: 358fc70
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about delegating to code that is empty? Same as any other empty code account? And then the code shows up in a latter TX?
That happens in a few cases (a) an delegated EOA that has delegated and (b) an empty account, (c) a balance-only account (nonce == 0). This should be called out in the testing section:
Tests that do all combinations of the following: