Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Re-evaluate how peer HEAD chain info is handled. #54

Closed
pipermerriam opened this issue Aug 31, 2018 · 2 comments
Closed

Re-evaluate how peer HEAD chain info is handled. #54

pipermerriam opened this issue Aug 31, 2018 · 2 comments

Comments

@pipermerriam
Copy link
Member

pipermerriam commented Aug 31, 2018

What is wrong?

When we connect to a peer we extract some information from the Status message to store as indicators to the peer's current chain head.

https://github.com/ethereum/trinity/blob/master/trinity/protocol/eth/peer.py#L78-L79

Then, whenever a NewBlock message comes through, we update this information

https://github.com/ethereum/trinity/blob/master/trinity/protocol/eth/peer.py#L49-L53

This update assumes multiple things about the info transmitted via NewBlock which I do not believe we can reliably trust.

  • That the block is genuine
  • That the block's parent is canonical

It appears that under these assumptions a malicious peer should be capable of luring trinity towards a header hash that is entirely bogus.

How can it be fixed

On connection, request the most recent N headers leading up to the peer's head_hash. This will be referred to as the "recent headers" chain.

When a NewBlock arrives validate it against the chain of N headers we have on-file. Using the block number from the new block, request the ancestor chain for the new block far enough back to reliably link up with the known recent headers. This should probably include a few extra headers to account for small chain re-orgs if the new block happens to be on a longer fork chain from the known recent header chain. If the new block can be validated against the peer's recent headers then extend the recent header chain up to the new block and discard the oldest headers to keep the recent headers list under a max size N

Validating the recent header chain should be as thorough as possible without incurring unreasonable overhead.

The recent header chain can also likely be a shared resource across multiple peers to avoid re-requesting the same headers from each peer.

@gsalgado
Copy link
Contributor

gsalgado commented Sep 1, 2018

I guess this wouldn't be a problem if we changed our header syncing implementation to be like geth's: fetch a non-consecutive batch of headers (I think they fetch every 128th header) from the peer we're syncing with, and then try to fill the gaps with headers fetched from other peers

@carver
Copy link
Contributor

carver commented Sep 20, 2018

Vaguely related to #53 - picking the best header and syncing to it

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

No branches or pull requests

3 participants