-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Add EIP: EOA private key deactivation/reactivation #9193
base: master
Are you sure you want to change the base?
Conversation
File
|
EIPS/eip-xxxx.md
Outdated
@@ -0,0 +1,201 @@ | |||
--- |
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.
--- | |
--- | |
eip: 7851 |
Assigning next sequential EIP/ERC/RIP number.
Numbers are assigned by editors & associates.
Please also update the filename.
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.
got it. updated in commit 314ed2b.
EIPS/eip-xxxx.md
Outdated
title: Private key deactivation and reactivation | ||
description: Introduce a new precompiled contract to enable Externally Owned Accounts (EOAs) to deactivate and reactivate their private keys. | ||
author: Liyi Guo (@colinlyguo) | ||
discussions-to: <URL> |
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.
Please create a discussions topic in Eth Magicians using the template: https://ethereum-magicians.org/c/eips/5
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.
thanks. updated in 314ed2b.
I don't see how this works with 7702 though. If I have the private key of account A, then if I "deactivate" sending txs from A then I can still auth-sign from address B to A, thus removing the "deactivation" from A? Should this add an extra check to 7702 when checking the auth-tuples? If the |
EIPS/eip-7851.md
Outdated
|
||
A new precompiled contract is introduced at address `PRECOMPILE_ADDRESS`. For each call, it consumes `PRECOMPILE_GAS_COST` gas, and the precompiled contract executes the following steps: | ||
|
||
- The precompiled contract checks that the caller is an EOA with delegated code (i.e., its account code begins with the prefix `0xef0100`, as defined in [EIP-7702](./eip-7702)). If the code does not conform to the required prefix, the contract MUST terminate without making any state changes. |
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.
"Terminate" here means just "stop" the call to this precompile without making any state changes? (One could also "revert", thus the item put on stack when using the CALL*
opcodes to this precompile would put 0 on stack if these conditions are not met (so no state changes are done?))
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.
Thanks for the clarification, the error case of precompile is updated as: "Returns a precompile contract error and consumes all provided gas" (refer to 0f2aaba).
The handling logic is consistent with other precompiles mentioned in https://www.evm.codes/precompiled. Also, consuming all gas for invalid calls (reducing DoS attack vectors) looks reasonable.
For the error return, I searched client implementation in geth and found it handled outside by CALL
, STATICCALL
, CALLCODE
, DELEGATECALL
. When precompiles return an error, it will be returned to CALL
, and CALL
will set return value as 0
. But formal documents would help clarify this for sure.
EIPS/eip-7851.md
Outdated
- The precompile determines the current state of the delegated code based on its byte length: | ||
- If the delegated code is 24 bytes (`0xef0100 || address || 0x00`), it removes the last byte (`0x00`), transitioning to the active state (`0xef0100 || address`). | ||
- If the delegated code is 23 bytes (`0xef0100 || address`), it appends `0x00`, transitioning to the deactivated state (`0xef0100 || address || 0x00`). | ||
- The updated delegated code is saved as the new account code for the EOA. |
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 a rule should be added here. In case that we are updating the delegated code, but we are running in "static" mode (so a call chain after using STATICCALL
) this should abort (revert, no state-changes).
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.
yes. you're right! I added the STATICCALL
error case in this commit: 0f2aaba
nice catch. added the extra check you mentioned in the draft. pls refer to this commit: 0f2aaba |
The commit e8778dc (as a parent of 0008744) contains errors. |
Asking for reviews from the review list to check if the pr can be considered as a draft. (appreciate it, so that it can be further discussed). |
EIPS/eip-7851.md
Outdated
|
||
If the account is verified as an EOA with a delegated code (begins with the prefix `0xef0100`), the following validations MUST be performed: | ||
|
||
- Transactions signed by the private key MUST be rejected if the delegated code is in the deactivated state (i.e., `24` bytes long). |
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 extra clarification here is necessary. What does "rejected" mean? I am assuming that "rejected" here means that the transactions which do not meet this rule can be included in the block, however any state changes like EVM execution, publishing blobs, or authorizing (EIP-7702) are not done and instead the transaction immediately exits. However, this does mean that these state changes are still done:
- Nonce update
- Paying for gas (so balance update)
Also, for this rejection: is the entire gas limit consumed or only the intrinsic gas (which includes for instance the calldata fee)
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.
true. after rethink, "rejected" means the transaction cannot be included in a block at all, as it is filtered out during the pre-check phase of the execution-layer consensus rules. No EVM execution or state changes (e.g., nonce updates or gas payment) occur. The transaction pool should implement the same validation to prevent invalid transactions from being propagated.
This will introduce an additional storage read when checking transaction validity though (thanks to your comment below).
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.
added clarifications in this commit: 7f93c93.
also removing the basic 21000 gas discussion, since it seems to be unrelated now.
Co-authored-by: Jochem Brouwer <[email protected]>
EIPS/eip-7851.md
Outdated
|
||
### Gas Cost | ||
|
||
No changes to the base transaction gas cost (`21000`) are required, as the additional valid check for the deactivation status is minimal. Thus it is reasonable to consider the overhead covered by the base gas cost. |
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 check if the current account is deactivated actually is more expensive than a base transaction due to the nature of the state trie. To read an account from the trie, one gets these items:
- Nonce
- Balance
codeHash
storageHash
In order to read the actual code (to figure out if the account is delegated and thus to also perform the private key deactivation check) we have to actual read from the database again (to find what code codeHash
points to). Since this is a disk read this is expensive and since this is now necessary for all transactions this might be a reason to increase the base transaction cost.
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.
it makes sense. I missed this (code
read cost).
In the first draft before opening this pr, the proposal plans to add a new bool (1 byte) field in the account state, so that it can be read together with nonce
, balance
, codeHash
, and storageHash
.
this may add some complexities in backward compatibility discussions while keeping the basic cost of transactions cheaper. do you think it makes sense to change the implementation of the draft to this method? so that the basic transaction cost can be kept unchanged.
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.
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.
@colinlyguo This extra field in state will make this rather complex, because you are adding an extra field to the account state. This also means there will be logic introduced when enabling this EIP: each time if an account is hit, there will be logic to see if this account is "upgraded" to have this boolean in state. If this is not the case, then it could either add this flag to the state, or it could also do it only when necessary (so, if a transaction is sent, there will first be a check if the private key is deactivated. To check, first see if there is a boolean field at all: if this is not the case, then we can know for sure that this is an "old" account state and the private key is thus active. We can then add this field to the state. But this mechanism of checking and adding these fields will make the test vectors for this very big and it will also add a lot of somewhat boilerplate "upgrade" code to ensure the old accounts will update to the new ones (with this private key enabled flag). I would not recommend it.
I do recall in the past there have been discussions of using the nonce
field. One could, for instance, use the highest bit of the nonce for this flag. One could use this flag to see if the accounts private key is disabled. What is handy here is that the nonce data is already in the account, so no extra disk lookups are necessary (when compared to putting it in the code).
However some nonce-limiting EIPs are around: https://eips.ethereum.org/EIPS/eip-3338, https://eips.ethereum.org/EIPS/eip-4803, putting a flag in nonce would likely throw away the benefits/rationales of these EIPs (4803 ensures that it can be decoded as int).
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.
agree. introduced a new field in the account state will add a lot of unnecessary "upgrade" code, which should be avoided.
adding a flag in nonce
looks like a good idea.
for EIP-3338 which limits the nonce <= 2^52, using the highest bit of 2^52 seems not to make a large difference:
- the number is still large, 2^51 ~= 2.25 * 10^15.
- compared with 2^64-1, it's also quite small (same as 2^52).
For forward compatibility, can move the "deactivated bit" based on the nonce length after an upgrade. i.e. modifying the transaction verification rules.
an alternative to avoid encoding in the highest bit would be to encode the "deactivated bit" in the lowest bit of nonce, the actual nonce = nonce / 2.
9b8824a
to
4ff1e76
Compare
4ff1e76
to
7f93c93
Compare
Introduce a new precompiled contract to enable Externally Owned Accounts (EOAs) to deactivate and reactivate their private keys.