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(p2p)!: Extend Head interface's Head method with ...HeadOption, introduce WithTrustedHead opt #53

Merged
merged 32 commits into from
Aug 3, 2023

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Jun 21, 2023

This PR introduces variadic HeadOptions and adds them as optional param to the Head method in the Head interface.

It will allow callers to the Exchange's Head method to specify if subjective initialisation is required, determining which peerset (trusted or tracked) to choose from (the default is tracked) via the WithTrustedHead option.

It also exposes a getPeers endpoint on the tracker that returns its currently tracked peers.

@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2023

Codecov Report

Merging #53 (299731a) into main (87ab66e) will increase coverage by 0.93%.
Report is 3 commits behind head on main.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
+ Coverage   67.27%   68.21%   +0.93%     
==========================================
  Files          35       36       +1     
  Lines        2827     2888      +61     
==========================================
+ Hits         1902     1970      +68     
+ Misses        772      766       -6     
+ Partials      153      152       -1     
Files Changed Coverage Δ
interface.go 0.00% <ø> (ø)
headertest/dummy_header.go 65.62% <50.00%> (ø)
p2p/peer_tracker.go 77.95% <81.57%> (+0.03%) ⬆️
p2p/exchange.go 82.78% <93.47%> (+1.79%) ⬆️
headertest/dummy_suite.go 100.00% <100.00%> (ø)
headertest/store.go 88.13% <100.00%> (+5.08%) ⬆️
local/exchange.go 47.61% <100.00%> (ø)
opts.go 100.00% <100.00%> (ø)
store/store.go 68.02% <100.00%> (ø)
sync/sync_getter.go 90.47% <100.00%> (ø)
... and 1 more

... and 2 files with indirect coverage changes

p2p/exchange.go Outdated Show resolved Hide resolved
p2p/peer_tracker.go Outdated Show resolved Hide resolved
vgonkivs
vgonkivs previously approved these changes Jun 26, 2023
@renaynay renaynay dismissed vgonkivs’s stale review June 26, 2023 15:47

The merge-base changed after approval.

vgonkivs
vgonkivs previously approved these changes Jun 28, 2023
gupadhyaya
gupadhyaya previously approved these changes Jun 29, 2023
Copy link
Contributor

@gupadhyaya gupadhyaya left a comment

Choose a reason for hiding this comment

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

lgtm.

p2p/exchange.go Outdated Show resolved Hide resolved
p2p/exchange.go Show resolved Hide resolved
@renaynay renaynay dismissed stale reviews from gupadhyaya and vgonkivs via 60bda34 June 30, 2023 10:12
@renaynay renaynay marked this pull request as draft June 30, 2023 10:44
@renaynay renaynay changed the title feat(p2p): Extend Head request with RequestOptions and introduce WithSubjectiveInit opt feat(p2p)!: Extend Head interface's Head method with ...RequestOptions, introduce WithSubjectiveInit opt Jun 30, 2023
@renaynay renaynay marked this pull request as ready for review June 30, 2023 11:29
@renaynay
Copy link
Member Author

Updated title + description

@renaynay renaynay marked this pull request as draft June 30, 2023 12:08
p2p/peer_tracker.go Outdated Show resolved Hide resolved
p2p/peer_tracker.go Outdated Show resolved Hide resolved
p2p/peer_tracker.go Outdated Show resolved Hide resolved
…block" on attempts to connect to bootstrappers and spawn async attempts for all peers in pidstore
@renaynay renaynay changed the title feat(p2p)!: Extend Head interface's Head method with ...HeadOption, introduce DisableSubjectiveInit opt feat(p2p)!: Extend Head interface's Head method with ...HeadOption, introduce WithTrustedHead opt Aug 1, 2023
p2p/peer_tracker.go Outdated Show resolved Hide resolved
p2p/peer_tracker_test.go Outdated Show resolved Hide resolved
@MSevey MSevey removed their request for review August 2, 2023 13:45
p2p/peer_tracker.go Outdated Show resolved Hide resolved
p2p/peer_tracker.go Outdated Show resolved Hide resolved
vgonkivs
vgonkivs previously approved these changes Aug 3, 2023
Wondertan
Wondertan previously approved these changes Aug 3, 2023
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.

Last nits and GTG

p2p/exchange.go Outdated Show resolved Hide resolved
p2p/exchange.go Outdated Show resolved Hide resolved
sync/sync_head.go Outdated Show resolved Hide resolved
@renaynay renaynay dismissed stale reviews from Wondertan and vgonkivs via fa3c102 August 3, 2023 14:40
@renaynay renaynay merged commit fe54fd9 into celestiaorg:main Aug 3, 2023
2 checks passed
@renaynay renaynay deleted the subjective-init branch August 3, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants