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

Refactor RPC and network events #530

Merged
merged 7 commits into from
Jul 2, 2020
Merged

Refactor RPC and network events #530

merged 7 commits into from
Jul 2, 2020

Conversation

austinabell
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Replaces RPC module with new RequestResponse module in libp2p. Was just going to refactor, but I think this is a better long term solution
    • wasn't usable as is, which is why the fork, but there isn't anything unreasonable in my PR I don't think, so will circle back to switch back to a release if/when that happens
  • Updates the events emitted from the network service and the interface in slightly

Reference issue to close (if applicable)

Closes #505

Other information and links

@ec2
Copy link
Member

ec2 commented Jul 1, 2020

What do you think of getting rid of RPCEvent? Doesn't make sense to put em together IMO if the Hello and BS behaviours are separated. Doesn't seem to make sense to treat them as variants of some RPCEvent enum as they're handled and used completely differently in most parts of Forest

@austinabell
Copy link
Contributor Author

What do you think of getting rid of RPCEvent? Doesn't make sense to put em together IMO if the Hello and BS behaviours are separated. Doesn't seem to make sense to treat them as variants of some RPCEvent enum as they're handled and used completely differently in most parts of Forest

Idk I don't have a strong opinion but it seemed way less clean with duplication and unnecessary conversions. I might just update anyway though because I guess the interface from outside the service is better not to have to wrap the RPC variant.

I'll just commit and can always just pull it off again. Seemed more usable way it was, but it's fine by me to have it this way

@austinabell austinabell merged commit 0facf1b into main Jul 2, 2020
@austinabell austinabell deleted the austin/rpcreplace branch July 2, 2020 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Network Libp2p and PubSub stuff Status: Merge Ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC responses from go impl polling incomplete bytes from substream
3 participants