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
Add EIP: EOA private key deactivation/reactivation #9193
base: master
Are you sure you want to change the base?
Add EIP: EOA private key deactivation/reactivation #9193
Changes from 10 commits
99cc1bc
314ed2b
9c7667a
92f6732
dd2586e
4130922
0f2aaba
e8778dc
f817605
5ca2633
6b6f5bf
83278f7
7f93c93
8cc68c6
35ec718
2023777
8f3f7f4
81a6155
b2c9535
57f71b9
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.
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:
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.
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:
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
, andstorageHash
.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.
21000**
**
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:
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.
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