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

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

Open
antiochp opened this issue Dec 10, 2019 · 2 comments

Comments

@antiochp
Copy link
Member

antiochp commented Dec 10, 2019

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
Copy link
Contributor

Related: #2789

@antiochp
Copy link
Member Author

Also related: #1779

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants