header/p2p: request head from multiple peers

[vgonkivs]

Resolves:

• header/p2p: Ensure Head method sanity checks headers array returned #980
• header/p2p: Request Head from multiple trusted peers instead of just one and compare them #1019

[codecov-commenter]

Codecov Report

Merging #1046 (4e751fb) into main (b37a844) will increase coverage by 0.69%.
The diff coverage is 83.65%.

@@            Coverage Diff             @@
##             main    #1046      +/-   ##
==========================================
+ Coverage   56.33%   57.02%   +0.69%     
==========================================
  Files         136      141       +5     
  Lines        9170     9488     +318     
==========================================
+ Hits         5166     5411     +245     
- Misses       3465     3526      +61     
- Partials      539      551      +12     

Impacted Files	Coverage Δ
header/p2p/request.go	0.00% <0.00%> (ø)
header/p2p/serde.go	0.00% <0.00%> (ø)
service/rpc/das.go	26.66% <33.33%> (-23.34%)	⬇️
das/store.go	61.53% <61.53%> (ø)
header/p2p/server.go	53.70% <69.56%> (+0.44%)	⬆️
das/daser.go	68.60% <74.07%> (-13.22%)	⬇️
header/p2p/pb/extended_header_request.pb.go	40.00% <74.24%> (+7.13%)	⬆️
header/p2p/exchange.go	67.40% <76.36%> (+5.18%)	⬆️
das/done.go	84.61% <84.61%> (ø)
das/subscriber.go	88.23% <88.23%> (ø)
... and 25 more

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

[MSevey]

@renaynay shared this PR with me, I hope my comments are helpful.

[renaynay]

Suggestions in the form of a PR/diff: https://github.com/vgonkivs/celestia-node/pull/50/files

[vgonkivs]

In a discussion with @renaynay and @Wondertan we agreed, that we should take a header with max height that was provided by at least 2 of trusted peers

[renaynay]

Looks really good! I still have yet to test it against arabica bc bridge is down.

• Can we change doRequest to just request since we already have performRequest ?
• Please format parseHead test like this:

		/*
			Height -> Amount
			headerHeight[0]=1 -> 1
			headerHeight[1]=2 -> 1
			headerHeight[2]=3 -> 1
			result -> headerHeight[2]
		*/
		{
			precondition:   gen,
			expectedHeight: 3,
		},
		/*
			Height -> Amount
			headerHeight[0]=1 -> 2
			headerHeight[1]=2 -> 1
			headerHeight[2]=3 -> 1
			result -> headerHeight[0]
		*/
		{
			precondition: func() []*header.ExtendedHeader {
				res := gen()
				res = append(res, res[0])
				return res
			},
			expectedHeight: 1,
		},
		/*
			Height -> Amount
			headerHeight[0]=1 -> 3
			headerHeight[1]=2 -> 2
			headerHeight[2]=3 -> 1
			result -> headerHeight[1]
		*/
		{
			precondition: func() []*header.ExtendedHeader {
				res := gen()
				res = append(res, res[0])
				res = append(res, res[0])
				res = append(res, res[1])
				return res
			},
			expectedHeight: 2,
		},

• i would rename parseHeads to bestHead or something like that to indicate that we're looking for the best head according to documented conditions
• can you add a debug log in the case that inside of parseHeads that you reach the case where no header has at least 2 peers returning it and you have to resort to returning the max height?