Invalid tracking of received at field of block

[manav2401]

During the whole block fetching mechanism, in certain cases, the ReceivedAt field of a block is tracked incorrectly due to reasons explained below.

On new block announcement, the header is requested here, which ultimately uses request one header as mentioned here. When the dispatcher flow completes and response is received, we track the time in the response as mentioned here. On receiving the response here back in the channel, the time used to pass in the FilterHeaders function is time.Now().Add(res.Time) where res.Time is what we set before in the dispatcher. According to this logic, I believe it sets a future time by adding the delay of p2p request to current time. The logic here should just use time.Now() or time.Now().Sub(res.Time) instead. When we process the headers and create block objects, we set the same time as set above in the block's ReceivedAt field as mentioned here, thus incorrectly tracking the time and updating metrics using that incorrect time.

The FilterBodies function on the other hand uses a correct time i.e. time.Now(). Hence, to be specific, wrong field is only set when the body is empty as we don't try to fetch bodies in that case.

The fix for should be pretty simple: use time.Now() or time.Now().Sub(res.Time) instead over here as the last parameter.

TL;DR: The time used to set the received at field in block is wrongly set as the initial function uses wrong time value.

If this turns out to be an issue, I can raise a quick PR fixing this. Thank you!

EDIT: the tag was auto chosen to "docs". I'd put it under "bug" but have created a generic issue as we it's not yet finalised if this is an issue yet.

[holiman]

• res.Time: Time it took for the request to be served
• &headerFilterTask{peer: peer, headers: headers, time: time}: time is Arrival time of the headers.

Setting Arrival time of the headers to now + time it took for request to be served, a.k.a :

time.Now().Add(res.Time)

yeah, it makes no sense at all.

The fix for should be pretty simple: use time.Now() or time.Now().Sub(res.Time) instead over here as the last parameter.

Yes please make a PR for this. It seems to me (after only a quick glance) that time.Now() ought to be the correct fix.

[manav2401]

Thank you. Created a PR here: #28637