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.

Sign up for GitHub

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

Jump to bottom

WIP: implement EIP-5806 #3312

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

WIP: implement EIP-5806 #3312

wants to merge 2 commits into from

Conversation

Amxx
Copy link

@Amxx Amxx commented Mar 9, 2024 •
edited

See https://github.com/Amxx/EIP-5806/tree/main to track the other implementation effort.

Amxx added 2 commits March 8, 2024 23:44
@Amxx
WIP: Implementation of EIP-5806
b25e6e6 
Missing:
- Delegate transaction must have a to
- Restricted opcodes
@Amxx
Fix delegate calls and implement restricted opcodes
e40a37c
@codecov Codecov
Copy link

codecov bot commented Mar 11, 2024 •
edited

Codecov Report

Attention: Patch coverage is 46.05263% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 86.54%. Comparing base (5e3cfdd) to head (e40a37c).

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 88.43% <ø> (ø)
blockchain 91.61% <ø> (ø)
client 84.89% <37.50%> (-0.01%) ⬇️
common 98.44% <100.00%> (+<0.01%) ⬆️
devp2p 82.12% <ø> (ø)
ethash ∅ <ø> (∅)
evm 73.91% <44.44%> (-0.15%) ⬇️
genesis 99.98% <ø> (ø)
rlp ∅ <ø> (∅)
statemanager 77.00% <ø> (ø)
trie 89.63% <ø> (+0.35%) ⬆️
tx ?
util 89.19% <ø> (ø)
vm 79.49% <36.00%> (-0.37%) ⬇️
wallet 88.35% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@holgerd77
Copy link
Member

Hi there,
super interesting work! 🤩 To be honest, we cannot promise at the point that we will take this into the main code base, so you might need to live with the situation to reference this as a PR or your own fork.

We'll see. We do not have super transparent ruling what to take in and what not (also to be honest here), at the moment we mostly integrate EIPS already included in HFs and occassionally we do some exceptions for "promising EIPs" which "doesn't bloat the code base too much". 😝

Admittedly these are very blurry "criterias". We'll discuss internally how we want to proceed here, maybe also worth to document this in a more explicit way.

We might also want to think along stuff like...

  • See if we can make our stack (even more) modular, to make these kind of integrations easier and/or
  • Make it easier to fork and publish the whole stack

(both rather for the cases that we do not integrate)

Again, we'll see. Will report back here, happy to take opinions/suggestions on that in between! 🙂

@Amxx
Copy link
Author

Amxx commented Mar 11, 2024 •
edited

super interesting work! 🤩 To be honest, we cannot promise at the point that we will take this into the main code base, so you might need to live with the situation to reference this as a PR or your own fork.

That is ok.

The goal if for project like Remix to be able to track progress, and use the wip branch for their own prototypes.

We are trying to evaluate impact of the EIP and get feedback from devs ...

@Amxx Amxx marked this pull request as draft March 11, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
package: client package: common package: evm package: tx package: vm PR state: do-not-merge PR state: WIP type: feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Amxx @holgerd77