search

mimblewimble / grin

Minimal implementation of the Mimblewimble protocol.
https://grin.mw/
Apache License 2.0
5.04k stars 990 forks source link

Peer total_difficulty should be updated on Header/Block/CompactBlock (not just Ping/Pong) #3167

Open antiochp opened 4 years ago

antiochp commented 4 years ago

Problem

We track height and total_difficulty for all our connected peers on the PeerLiveInfo struct.

These are updated when we handle Ping and Pong messages. These are the only places we update height and total_difficulty for a connected peer.

Surprisingly we do not update these when we process and accept a new block header... and we must wait (potentially up to 10s) for the next Ping or Pong to arrive to update the peer in our list of connected peers.

I am not entirely sure what the implications of this unnecessary delay means but its probably not great.

Proposal

1) We should update height and total_difficulty as soon as we accept a valid block header (or compact block or full block) from a peer (if this increases total_difficulty).

2) We should then update height and total_difficulty for any peer that subsequently sends us that header, compact_block or full block, even if we treat this as "already known" on our side.

Why update peer on block header and not just a full block? As soon as a peer sends us a new block header it is advertising that it indeed has this full block. In the same way as a Ping/Pong advertises the existence of a block with certain total_difficulty and height.

The block_accepted() on the chain adapter might be a good approach to this, but we likely need a header_accepted() as well. And we'll need to consider the peer locks carefully.

Related

The Hand and Shake msgs include total_difficulty but do not include height.

~We do not use this total_difficulty when initializing the PeerLiveInfo for the connected peer.~

Edit: We do use this, but we don't have the height which is why these appear as 0 on the tui during startup.

We should probably add height to these msgs for completeness or maybe just fire off a Ping as soon as the handshake negotiation has completed successfully.

DavidBurkett commented 4 years ago

Related: https://github.com/mimblewimble/grin/issues/2789

antiochp commented 4 years ago

Also related: https://github.com/mimblewimble/grin/issues/1779