Skip to content
This repository has been archived by the owner on Dec 2, 2022. It is now read-only.

WIP: Finish Ethereum Wire Protocol implementation #19

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

preston-evans98
Copy link
Contributor

Implement each message type.
Implement some tests cases from EIP 2481

@vorot93
Copy link
Member

vorot93 commented Aug 6, 2021

@preston-evans98 Hey, thanks for the PR! Is this still a draft you will be working on?

@preston-evans98
Copy link
Contributor Author

I do plan to keep working on it! Unfortunately, I’m traveling at the moment and likely won’t make much progress for a few days. If you’d prefer to merge as is, I’m happy to make it a full PR and submit follow ups to fill out the code coverage and fix the decoding bugs.

@vorot93
Copy link
Member

vorot93 commented Aug 6, 2021

Take your time, just make sure to mark it as ready to review when you're done.

@preston-evans98
Copy link
Contributor Author

This commit is ready to merge as soon as rust-ethereum/ethereum#19 is resolved!

@preston-evans98 preston-evans98 marked this pull request as ready for review September 25, 2021 23:06
Copy link
Contributor

@battlmonstr battlmonstr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Awesome tests!

src/downloader/message_decoder.rs Outdated Show resolved Hide resolved
@Rjected
Copy link

Rjected commented Feb 3, 2022

Hey, I'm interested in using some of this - what's left to do on this PR? Could I help out in any way?

@preston-evans98
Copy link
Contributor Author

Hey, I'm interested in using some of this - what's left to do on this PR? Could I help out in any way?

Not much, last I checked! I started a new job and had to get open source contributions approved with legal, so this just fell by the wayside. It is possible that some dependencies have changed though, so there may be some reformatting required. I'll try to take a look tonight.

@preston-evans98
Copy link
Contributor Author

@vorot93 would you mind kicking off the approval workflow when you have a minute?

@battlmonstr
Copy link
Contributor

would you mind kicking off the approval workflow when you have a minute?

@preston-evans98 if you rebase onto master, solve conflicts, and then force push, the workflow should be triggered automatically. Currently GH shows that it has some conflicts, so a manual rebase is still needed. Please try to squash and rebase instead of merging.

Implement all message types.
Test all messages which have EIP2481 test cases
Enable decoding of all tested messages
@preston-evans98
Copy link
Contributor Author

@battlmonstr I've rebased and squashed. I'm not seeing any conflicts on my end but it doesn't look like CI reran automatically. Do you see anything I've missed?

@battlmonstr
Copy link
Contributor

@preston-evans98 thanks. Yes, the CI run approval was required for you. Now the checks are in progress ⏳

@battlmonstr battlmonstr enabled auto-merge (squash) February 8, 2022 15:16
@battlmonstr battlmonstr merged commit 1dcb3a2 into akula-bft:master Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants