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

p2p: add deterministic name function using peerID #629

Merged
merged 3 commits into from
May 31, 2022

Conversation

xenowits
Copy link
Contributor

@xenowits xenowits commented May 30, 2022

  1. Add deterministic function for peer names by running a polynomial hash function on peerID string.
  2. Replace all p2p.ShortID(pID) with p2p.PeerName(pID)

category: feature
ticket: #585

@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #629 (2eb992f) into main (704bc69) will decrease coverage by 0.02%.
The diff coverage is 69.23%.

@@            Coverage Diff             @@
##             main     #629      +/-   ##
==========================================
- Coverage   56.76%   56.74%   -0.03%     
==========================================
  Files          97       97              
  Lines        9155     9178      +23     
==========================================
+ Hits         5197     5208      +11     
- Misses       3210     3222      +12     
  Partials      748      748              
Impacted Files Coverage Δ
dkg/frostp2p.go 72.34% <0.00%> (ø)
dkg/transport.go 52.83% <0.00%> (ø)
app/app.go 58.01% <20.00%> (-1.02%) ⬇️
cluster/operator.go 60.97% <50.00%> (ø)
dkg/dkg.go 50.98% <50.00%> (ø)
p2p/name.go 68.75% <100.00%> (ø)
p2p/peer.go 52.00% <100.00%> (+2.00%) ⬆️
core/qbft/qbft.go 82.57% <0.00%> (+0.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 704bc69...2eb992f. Read the comment docs.

Copy link
Contributor

@leolara leolara left a comment

Choose a reason for hiding this comment

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

It seems to be changes unrelated to the name thing, but perhaps I don't understand

}

return nil
return qbft.Run[core.Duty, [32]byte](ctx, c.def, qt, duty, peerIdx, hash)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the relation with the task ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, these are unrelated. i'm trying to rebase on latest main

app/app.go Outdated
if err != nil {
return err
// Configure the beacon node api.
var eth2Cl eth2client.Service
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the relation with the task ??

@xenowits xenowits force-pushed the xenowits/sapiens-friendly-names branch from d8a25b8 to 4f75ea2 Compare May 30, 2022 13:24
@xenowits xenowits linked an issue May 30, 2022 that may be closed by this pull request
@xenowits xenowits added the merge when ready Indicates bulldozer bot may merge when all checks pass label May 31, 2022
@obol-bulldozer obol-bulldozer bot merged commit c106399 into main May 31, 2022
@obol-bulldozer obol-bulldozer bot deleted the xenowits/sapiens-friendly-names branch May 31, 2022 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge when ready Indicates bulldozer bot may merge when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Human friendly node/peer names
2 participants