Skip to content

p2p: add Channels to NodeInfo and don't send for unknown channels#1049

Merged
ebuchman merged 3 commits intodevelopfrom
p2p-channels
Jan 24, 2018
Merged

p2p: add Channels to NodeInfo and don't send for unknown channels#1049
ebuchman merged 3 commits intodevelopfrom
p2p-channels

Conversation

@ebuchman
Copy link
Contributor

@ebuchman ebuchman commented Jan 2, 2018

Solves #957

@ebuchman ebuchman requested a review from melekes as a code owner January 2, 2018 04:41
p2p/types.go Outdated
for _, ch1 := range info.Channels {
for _, ch2 := range other.Channels {
if ch1 == ch2 {
found = true
Copy link
Contributor

Choose a reason for hiding this comment

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

should we break after?

node/node.go Outdated
cs.StateChannel, cs.DataChannel, cs.VoteChannel, cs.VoteSetBitsChannel,
mempl.MempoolChannel,
evidence.EvidenceChannel,
p2p.PexChannel,
Copy link
Contributor

Choose a reason for hiding this comment

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

what if pex is disabled?

@melekes
Copy link
Contributor

melekes commented Jan 6, 2018

Then if a peer sends us a msg for a channel we don't know about, we can mark them as bad and disconnect :)

what about this part?

@ebuchman ebuchman mentioned this pull request Jan 14, 2018
@ebuchman ebuchman changed the base branch from p2p-id to develop January 20, 2018 20:41
@ebuchman ebuchman changed the base branch from develop to addrbook January 21, 2018 16:31
@ebuchman
Copy link
Contributor Author

The old code wasn't actually doing what we wanted. Force pushed the fix. PTAL

@ebuchman
Copy link
Contributor Author

what about this part?

This is implemented already since if we receive a msg for an unknown channel we stopPeerForError.

@ebuchman
Copy link
Contributor Author

We should add a config option for what channels we require from our peers

@ebuchman
Copy link
Contributor Author

Saving that for #1148

@ebuchman ebuchman changed the base branch from addrbook to develop January 24, 2018 04:11
p2p/node_info.go Outdated
for _, ch2 := range other.Channels {
if ch1 == ch2 {
found = true
break // only need one
Copy link
Contributor

@melekes melekes Jan 24, 2018

Choose a reason for hiding this comment

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

I think the intention here was to break from outer loop, no? maybe break OUTER_LOOP

@codecov-io
Copy link

Codecov Report

Merging #1049 into develop will increase coverage by <.01%.
The diff coverage is 71.42%.

@@             Coverage Diff             @@
##           develop    #1049      +/-   ##
===========================================
+ Coverage    59.39%   59.39%   +<.01%     
===========================================
  Files          121      121              
  Lines        10993    11022      +29     
===========================================
+ Hits          6529     6547      +18     
- Misses        3860     3866       +6     
- Partials       604      609       +5

@ebuchman ebuchman merged commit 27ef348 into develop Jan 24, 2018
@ebuchman ebuchman deleted the p2p-channels branch January 24, 2018 20:29
firelizzard18 pushed a commit to AccumulateNetwork/tendermint that referenced this pull request Nov 24, 2023
The PR closes issue #25 partially.

Changes made:
- Bump version v0.37 to v0.38 in docs.
- Update docs to remove reference to `BeginBlock`, `EndBlock`, and `DeliverTx` and consolidate them into `FinalizeBlock.`
- Minor changes to the formatting of docs.

#### PR checklist

- [ ] Tests written/updated
- [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)
- [X] Updated relevant documentation (`docs/` or `spec/`) and code comments

(cherry picked from commit d31be6b)

Co-authored-by: Aliasgar Merchant <44069404+alijnmerchant21@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants