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

Extract the RLP and remove go-ethereum dependency #290

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

Conversation

sideninja
Copy link
Contributor

@sideninja sideninja commented Jun 6, 2022

Closes: #221

Description

This PR extracts the RLP implementation of the go-etheruem package.

It uses git subtree to include the repository and allow updates with the upstream branch. The contents were modified to only include the RLP implementation and remove all the unnecessary files.

I was trying to use git submodule but I had problems due to the fact that it only referenced a whole subrepo as a commit reference, but that commit wasn't really pushed to the ethereum repo for obvious reasons.

DOD:

  • Implemented
  • Released

For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@sideninja sideninja marked this pull request as ready for review June 9, 2022 11:30
@sideninja sideninja marked this pull request as draft December 19, 2022 09:10
@nozim
Copy link
Contributor

nozim commented Aug 20, 2023

@sideninja I'm trying to understand what is missing here? I can follow up on missing bits if you don't mind.

@sideninja
Copy link
Contributor Author

@nozim it's pretty much done, but I wanted to make sure everything was correct but just then didn't have time. I would say making sure this way is the right way.

@nozim
Copy link
Contributor

nozim commented Aug 21, 2023

I think cutting dependency while keeping same functionality is already good for this scope. So should be good to merge :)

@sideninja
Copy link
Contributor Author

We need to make sure it is updated too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RLP Implementation
2 participants