-
Notifications
You must be signed in to change notification settings - Fork 6k
Update EIP-7723: Update 7723 to include EELS/EEST in CFI #9290
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
Changes from 2 commits
5c84565
d20d9b5
1c3fd5d
d38d9a6
4898448
a39a049
d67d3b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,12 @@ | |
|
|
||
| An EIP **MAY** be moved from `Considered for Inclusion` to `Declined for Inclusion` if client teams are against including the EIP in the network upgrade. | ||
|
|
||
| An EIP **SHOULD** have a Python implementation in [execution-specs](https://github.com/ethereum/execution-specs) in an open PR. | ||
|
Check failure on line 60 in EIPS/eip-7723.md
|
||
|
|
||
| An EIP **SHOULD** have tests implemented in [execution-spec-tests](https://github.com/ethereum/execution-spec-tests) in an open PR. | ||
|
Check failure on line 62 in EIPS/eip-7723.md
|
||
|
|
||
| The EIP writer is encouraged to reach out to the maintainers of both repositories for assistance with implementation. | ||
|
|
||
| ### Declined for Inclusion | ||
|
|
||
| At any time during the network upgrade planning process, client developers **MAY** move EIPs from any other stage to the `Declined for Inclusion` stage if client teams are against including the EIP in the network upgrade. Once a decision is made by client teams to move an EIP to `Declined for Inclusion`, the Upgrade Meta EIP **SHOULD** be updated to reflect this. | ||
|
|
@@ -67,6 +73,10 @@ | |
|
|
||
| When client teams agree to implement a Core EIP in the **next** Upgrade Devnet, the EIP **SHOULD** move to the `Scheduled for Inclusion` stage, and the Upgrade Meta EIP **SHOULD** be updated to reflect this. Non-Core EIPs **SHOULD** move to `Scheduled for Inclusion` when client teams agree to immediately prioritize their implementation. | ||
|
|
||
| An EIP **MUST** have a Python implementation in [execution-specs](https://github.com/ethereum/execution-specs) merged to the `devnets/upgradeName/version` branch of the repository before it can be moved to this stage. | ||
|
Check failure on line 76 in EIPS/eip-7723.md
|
||
|
|
||
| An EIP **MUST** have tests implemented in [execution-spec-tests](https://github.com/ethereum/execution-spec-tests) and merged to the `main` branch of the repository before it can be moved to this stage. | ||
|
Check failure on line 78 in EIPS/eip-7723.md
|
||
|
|
||
| `Scheduled for Inclusion` signals that implementation and testing work are underway. The EIP **SHOULD** be included in the network upgrade if no issues arise. The latest Upgrade Devnet must contain all `Scheduled for Inclusion` Core EIPs. | ||
|
|
||
| An EIP **MAY** be moved from `Scheduled for Inclusion` to `Declined for Inclusion` if client teams are against including the EIP in the network upgrade. An EIP **MAY** also be moved from `Scheduled for Inclusion` to `Considered for Inclusion` if client teams are in favor of including the EIP in the network upgrade but cannot commit to including it in the **next** Upgrade Devnet. | ||
|
|
||
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.
One thing our team discussed was changing this to any client implementation, out of concern for overhead. A neutral (and arguably disposable) implementation of an EIP prior to it being considered is a high bar, and not strictly necessary to fill tests against. Our thoughts are nuanced on this matter though.
This doc could also provide some guidance on when it is ok for an exception to the SHOULD, and set the expectation that ACD rough consensus would be the arbiter of that exception.
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.
Appreciate the feedback, we could add a line regarding the CFI'd stage requirements on the basis of an ACD consensus.
Regarding the EELS overhead, the really nice thing about having everything in EELS from the start is that when the time comes to join all EIPs together to fill tests for the entire fork, everything is one implementation instead of scattered on different client implementations, because it's a big bottleneck to testing to fill one part of the tests with one t8n tool and then the rest with another.