Skip to content
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

Merged
merged 7 commits into from
Jul 8, 2024

Conversation

lightclient
Copy link
Member

@lightclient lightclient commented Jun 19, 2024

This is an idea originally proposed by @vbuterin.

One aspect of 7702 that hasn't sat right with many involved is the fact that some types of 7702 authorizations do not expire. For delegations that are ephemeral, it's important to have a type of authorization that is replayable (which often means irrevocable), because it allows users to distance their activity from the master key of the account, e.g. delegate control to a smart wallet which can have subkeys with different privileges.

Because previous attempts at account delegation have relied on just-in-time provision of authorizations, the only way to make the authorizations replayable has been to make them irrevocable (or revocation impractical, see maxNonce mechanism). If we took a step back and thought about what we really want to have, regardless of the technical implications, the mechanism we are trying to create should allow an EOA to list on-chain that it has delegated control of it's account to a smart contract.

We've shied away from making changes to the account structure in the trie many times, thus the design of many of these "make EOAs better" proposals. An alternative route is to use an already existing member of the account structure to designate the code the EOA wants to delegate to. With EIP-3541, we now have a rich sequence of code prefixes that cannot be deployed on-chain and can be used to denote other meanings.

This is the core of the proposal: keep the 7702 auth message similar to how it is (although require nonce in sig) and actually set a designator in the EOA's account. The delegation designation would be of the form 0xef01 || address where address is the account whose code should be used in place of the designation.

This mechanism should simplify how we reason about authorizations and revocations. Other than it being more complicated than many of the current proposals, I don't see many downsides to this approach.

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Jun 19, 2024
@eth-bot
Copy link
Collaborator

eth-bot commented Jun 19, 2024

✅ All reviewers have approved.

@eth-bot eth-bot changed the title 7702: add delegation designation Update EIP-7702: add delegation designation Jun 19, 2024
@Jrachman
Copy link

If an EOA were to have an active authorization with this approach and then the EOA sends a regular, non-7702 transaction after, will that invalidate the active authorization when the EOA then wants to execute a 7702 transaction?

EIPS/eip-7702.md Outdated Show resolved Hide resolved
EIPS/eip-7702.md Outdated Show resolved Hide resolved
4. If nonce list item is length one, verify the nonce of `authority` is equal to `nonce`.
5. Set the code of `authority` to code associated with `address`.
6. Add the `authority` account to `accessed_addresses` (as defined in [EIP-2929](./eip-2929.md).)
3. Verify that the code of `authority` is either empty or already delegated.
Copy link
Contributor

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 the authority is in the format of 0xef01 || address, right?

I assume this check is to prevent that this flow in case that any other code besides the delegation is set, right?

Copy link
Contributor

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:

  1. delegation are persisted - which is not defined by this doc (if/when we can change the delegation?)
  2. there is no need for an authorizer entry, since it is already on-chain
  3. if the repeated authorizer signature is the mechanism to change an on-chain delegation, then please specify it explicitly. (and it has a whole bunch of security implications)

Copy link
Member Author

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.

Copy link
Contributor

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.

@lightclient
Copy link
Member Author

If an EOA were to have an active authorization with this approach and then the EOA sends a regular, non-7702 transaction after, will that invalidate the active authorization when the EOA then wants to execute a 7702 transaction?

@Jrachman no, the authorization would persist in the account. As spec'd it is currently impossible to "invalidate" a delegation, it is only possible to delegate to something else.

@shemnon
Copy link
Contributor

shemnon commented Jun 20, 2024

@lightclient then there should be an "undelegate" option. Such as delegating to yourself or keeping the address portion of the message empty and just signing two bytes.

@lightclient
Copy link
Member Author

@shemnon what purpose would an undelegation mechanism serve vs. simply delegating to zero address like I mentioned?

EIPS/eip-7702.md Outdated
4. Verify the nonce of `authority` is equal to `nonce`.
5. Set the code of `authority` to be `0xef0100 || address`. This is a delegation designation.
6. Increase the nonce of `authority` by one.
7. Add the `authority` account to `accessed_addresses` (as defined in [EIP-2929](./eip-2929.md).)

If any of the above steps fail, immediately stop processing that tuple and continue to the next tuple in the list. It will In the case of multiple tuples for the same authority, set the code specified by the address in the first occurrence.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case of multiple tuples for the same authority, set the code specified by the address in the first occurrence.

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 and nonce + 1 that could need to be submitted together to "catch up" to the latest one.

Copy link
Contributor

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the signer has all the information to detect and remove duplicate entries.

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.

Copy link
Contributor

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.

Copy link
Contributor

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 and n + 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.

Copy link
Contributor

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.

Copy link
Contributor

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.

@smartprogrammer93
Copy link
Contributor

smartprogrammer93 commented Jun 20, 2024

@lightclient, if you delegate to zero address, wouldn't any CALL to your account go to zero address, including ones with value?

Delegating to self also would be problematic as first opcode (ef) would result in an abort. Maybe we should mention how we should deal with this? Maybe disallow it?

@shemnon
Copy link
Contributor

shemnon commented Jun 21, 2024

It would be trapped out to delete the ef code. In some L2s there may be code at zeros, which is why I didn't recommend that. Removal would be to restore "pure EOA" behavior, where calling the EOA only deposits value.


The delegation designation uses the banned opcode `0xef` from [EIP-3541](./eip-3541) to designate the code has a special purpose. This designator requires all code retrieving operations follow the address pointer to fill the accounts observable code. The following instructions are impacted: `EXTCODESIZE`, `EXTCODECOPY`, `EXTCODEHASH`, `CALL`, `CALLCODE`, `STATICCALL`, `DELEGATECALL`.

For example, `EXTCODESIZE` would return the size of the code pointed to by `address` instead of `24` which would represent the delegation designation. `CALL` would similarly load the code from `address` and execute it in the context of `authority`.
Copy link
Contributor

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:

  1. Ban it? If so, what would be the behavior of these opcodes when hitting a cascaded delegation?
  2. Allow it? If so, we need to consider DoS (e.g. loading an uncapped number of accounts at the cost of a single EXTCODESIZE). And we need to add loop detection.

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor

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)

Copy link
Contributor

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

Copy link
Contributor

@shemnon shemnon Jun 24, 2024

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:

  • Delegate to (an empty account | a balance only account | an undelegated EOA), which later (gains balance | becomes an EOA | becomes a contract | becomes a delegated EOA)
  • call the EOA delegating (in the same TX | in a latter TX after the permutation happens).

EIPS/eip-7702.md Outdated Show resolved Hide resolved
@smartprogrammer93
Copy link
Contributor

@shemnon then we can trap out delegating to self as removing and delete the code. But this needs to be specified in the spec then

EIPS/eip-7702.md Outdated Show resolved Hide resolved
EIPS/eip-7702.md Outdated Show resolved Hide resolved
EIPS/eip-7702.md Outdated Show resolved Hide resolved
Copy link

@Parasos Parasos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EIPS/eip-7702.md

@lightclient
Copy link
Member Author

Last call on this, will merge later today unless there is more feedback.

@frangio
Copy link
Contributor

frangio commented Jul 5, 2024

See #8677 (comment). IMO this part makes no sense in the new design and should be removed:

In the case of multiple tuples for the same authority, set the code specified by the address in the first occurrence.

@lightclient
Copy link
Member Author

@frangio this seems reasonable, I will update it.

@drortirosh
Copy link
Contributor

drortirosh commented Jul 6, 2024

@lightclient ,
The spec still doesn't specify the cost of the operation.
it references "PER_CONTRACT_CODE_BASE_COST", which is not defined.
"PER_AUTH_BASE_COST" is defined but if that constant should be used, it is too cheap: being only 2500 gas, it is half of the SSTORE "modify" cost and 1/8th of the cost of SSTORE to empty cell

@lightclient lightclient marked this pull request as ready for review July 8, 2024 16:28
@lightclient lightclient requested a review from eth-bot as a code owner July 8, 2024 16:28
@eth-bot eth-bot enabled auto-merge (squash) July 8, 2024 16:29
Copy link
Collaborator

@eth-bot eth-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Reviewers Have Approved; Performing Automatic Merge...

@lightclient
Copy link
Member Author

@drortirosh sorry I though I updated to remove the ref to PER_CONTRACT_CODE_BASE_COST. It's fixed now.

The cost is not finalized and should definitely be updated now that code is deployed to the account. In particular, we need to update it to deal with creating new accounts (usually 25k gas). I'll leave that for another PR. If you have a suggestion for gas schedule, please post it on FEM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.