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

feat: Implement peer empty connection #822

Merged
merged 7 commits into from
Jun 3, 2022
Merged

Conversation

ivanovpetr
Copy link
Contributor

No description provided.

@ivanovpetr ivanovpetr linked an issue May 30, 2022 that may be closed by this pull request
@ivanovpetr ivanovpetr added this to the Milestone B milestone May 30, 2022
@ivanovpetr ivanovpetr self-assigned this May 30, 2022
@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #822 (0982df7) into develop (8259847) will decrease coverage by 0.01%.
The diff coverage is 3.06%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #822      +/-   ##
===========================================
- Coverage    10.51%   10.50%   -0.02%     
===========================================
  Files          332      332              
  Lines        77548    77711     +163     
===========================================
+ Hits          8157     8164       +7     
- Misses       69185    69342     +157     
+ Partials       206      205       -1     
Impacted Files Coverage Δ
x/launch/types/genesis_validator.pb.go 0.56% <0.62%> (+0.01%) ⬆️
x/launch/types/peer.go 95.00% <100.00%> (+0.55%) ⬆️
x/campaign/simulation/store.go 84.45% <0.00%> (+1.35%) ⬆️

@lumtis
Copy link
Contributor

lumtis commented May 30, 2022

I'm trying to figure out if the NodeID, id in Peer, is still necessary for empty connection or if it could be moved into tcpAddress and httpTunnel

The node always has a node ID in the P2P network even if the public address is not exposed. But potentially this information is not necessary on-chain

lumtis
lumtis previously approved these changes May 30, 2022
proto/launch/genesis_validator.proto Outdated Show resolved Hide resolved
Pantani
Pantani previously approved these changes Jun 1, 2022
@ilgooz
Copy link
Member

ilgooz commented Jun 2, 2022

I'm trying to figure out if the NodeID, id in Peer, is still necessary for empty connection or if it could be moved into tcpAddress and httpTunnel

The node always has a node ID in the P2P network even if the public address is not exposed. But potentially this information is not necessary on-chain

Good point, any updates on this?

In such case we don't need this PR, we can just NOT set the peer field in GenesisValidator which makes sense to me.

@lumtis
Copy link
Contributor

lumtis commented Jun 2, 2022

Good point, any updates on this?
In such case we don't need this PR, we can just NOT set the peer field in GenesisValidator which makes sense to me.

We let it for now as the Node ID is a general information

Co-authored-by: Alex Johnson <alex.johnson@tendermint.com>
@lumtis lumtis dismissed stale reviews from Pantani and themself via 49500c4 June 2, 2022 19:11
@lumtis lumtis merged commit 53277e4 into develop Jun 3, 2022
@lumtis lumtis deleted the feat/peer-empty-connection branch June 3, 2022 07:38
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.

Support Peer with no connection
6 participants