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

feat(nodebuilder/p2p): Implement Module #1285

Merged
merged 16 commits into from
Nov 23, 2022
Merged

feat(nodebuilder/p2p): Implement Module #1285

merged 16 commits into from
Nov 23, 2022

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Oct 26, 2022 •
edited
Loading

Implements the P2P Module and provides it on our RPC.

Fixes one TODO in #977

@renaynay renaynay added area:rpc kind:feat Attached to feature PRs area:api Related to celestia-node API labels Oct 26, 2022
@renaynay renaynay self-assigned this Oct 26, 2022
distractedm1nd
distractedm1nd reviewed Oct 31, 2022
View reviewed changes
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/tests/rpc_test.go Outdated Show resolved Hide resolved
vgonkivs
vgonkivs reviewed Oct 31, 2022
View reviewed changes
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
@renaynay
Copy link
Member Author

renaynay commented Nov 1, 2022

Use assert

@renaynay renaynay force-pushed the impl-p2p-mod branch 2 times, most recently from bd2232b to e19f179 Compare November 1, 2022 10:17
@renaynay renaynay marked this pull request as ready for review November 1, 2022 10:18
renaynay
renaynay commented Nov 1, 2022
View reviewed changes
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
distractedm1nd
distractedm1nd previously approved these changes Nov 9, 2022
View reviewed changes
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
distractedm1nd
distractedm1nd previously approved these changes Nov 11, 2022
View reviewed changes
nodebuilder/p2p/p2p_test.go Outdated Show resolved Hide resolved
nodebuilder/rpc/rpc.go Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2022 •
edited
Loading

Codecov Report

Merging #1285 (90b6152) into main (039facd) will increase coverage by 0.06%.
The diff coverage is 84.28%.

@@            Coverage Diff             @@
##             main    #1285      +/-   ##
==========================================
+ Coverage   55.21%   55.28%   +0.06%     
==========================================
  Files         178      181       +3     
  Lines       10744    10962     +218     
==========================================
+ Hits         5932     6060     +128     
- Misses       4232     4300      +68     
- Partials      580      602      +22
Impacted Files Coverage Δ
nodebuilder/share/service.go 100.00% <ø> (ø)
nodebuilder/header/service.go 57.14% <25.00%> (ø)
nodebuilder/p2p/p2p.go 85.18% <85.18%> (ø)
api/rpc/client/client.go 78.57% <100.00%> (+0.79%) ⬆️
nodebuilder/p2p/flags.go 67.46% <100.00%> (ø)
nodebuilder/p2p/host.go 90.90% <100.00%> (+9.65%) ⬆️
nodebuilder/p2p/module.go 93.75% <100.00%> (+0.20%) ⬆️
nodebuilder/rpc/rpc.go 100.00% <100.00%> (ø)
nodebuilder/core/opts.go 0.00% <0.00%> (-50.00%) ⬇️
nodebuilder/das/daser.go 75.00% <0.00%> (-25.00%) ⬇️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

distractedm1nd
distractedm1nd previously approved these changes Nov 11, 2022
View reviewed changes
Wondertan
Wondertan requested changes Nov 16, 2022
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Requesting some changes

nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/p2p/p2p.go Show resolved Hide resolved
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/p2p/resource.go Outdated Show resolved Hide resolved
nodebuilder/p2p/module.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Nov 17, 2022
View reviewed changes
nodebuilder/p2p/resource.go Outdated Show resolved Hide resolved
nodebuilder/p2p/p2p_test.go Show resolved Hide resolved
nodebuilder/p2p/p2p_test.go Outdated Show resolved Hide resolved
@renaynay renaynay mentioned this pull request Nov 19, 2022
Closed
3 tasks
Wondertan
Wondertan requested changes Nov 23, 2022
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Almost there

nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
nodebuilder/p2p/resource.go Outdated Show resolved Hide resolved
Wondertan
Wondertan reviewed Nov 23, 2022
View reviewed changes
nodebuilder/p2p/p2p.go Outdated Show resolved Hide resolved
Wondertan
Wondertan approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

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

Blessed

distractedm1nd
distractedm1nd approved these changes Nov 23, 2022
View reviewed changes
Copy link
Collaborator

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

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

blessed + based

liamsi
liamsi approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

ADR changes are tiny and make sense

adlerjohn
adlerjohn approved these changes Nov 23, 2022
View reviewed changes
Copy link
Member

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

utACK

@renaynay renaynay merged commit 3d8817a into celestiaorg:main Nov 23, 2022
@renaynay renaynay deleted the impl-p2p-mod branch November 24, 2022 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wondertan Wondertan Wondertan approved these changes

@distractedm1nd distractedm1nd distractedm1nd approved these changes

@liamsi liamsi liamsi approved these changes

@adlerjohn adlerjohn adlerjohn approved these changes

@vgonkivs vgonkivs Awaiting requested review from vgonkivs vgonkivs is a code owner

Assignees

@renaynay renaynay

Labels
area:api Related to celestia-node API area:rpc kind:feat Attached to feature PRs
Projects
No open projects
Celestia Node
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@renaynay @codecov-commenter @adlerjohn @liamsi @distractedm1nd @Wondertan @vgonkivs