sync on discovering a "longer" chain

[antiochp]

Here's the scenario (contrived example, I believe this happened over a larger number of blocks on both forks of the chain) -

We're happily processing blocks on the current chain -

b1 ____ b2 ____ b3 ____ b4

A new peer joins the network and advertises a previously unknown longer chain (both in terms of total_difficulty and height) -

b1 ____ b2 ______ b3 ___________________ b4
            |
            L___ b3' ____ b4' ____ b5' ____ b6'

• Before we sync -
  • head points at b4.
  • header_head points at b4.
• We quickly sync headers so that -
  • head remains at b4
  • sync_head points now at b6'
  • header_head is updated to b6' (header with most work)
  • But note - b4 has greater total_difficulty than both b4' and b5'

We now build a Locator to go request blocks [b3', b4', b5', b6'].

Processing blocks b3', b4' b5' are all fine -

• head remains pointing original b4, still block with most work

Processing block b6 fails validation (and is flagged an Orphan)
because while b6' has more total_difficulty than b4 it is more than one block higher than the original b4. There is no b5' between b4 and b6'.

It actually fails validation twice.
Once in validate_header (and if this were to pass) again in validate_block -

	if b.header.height > ctx.head.height + 1 {
		return Err(Error::Orphan);
	}

Q1) Should header_head be updated during sync if we process a new header on the longest chain (at which point it diverges from the current body_head)? These can be on divergent forks as in the above scenario.
Q2) When processing a block we validate both the header and the block. If the header_head and the body_head can diverge then this necessitates passing in two separate contexts to process_block (for the two divergent chain heads)?
Q3) How can we robustly handle Orphan detection in this case where we jump suddenly from one fork to another and the heights are not strictly sequential?

[ignopeverell]

You're right, this is bogus:

  if header.height > ctx.head.height + 1 {
    return Err(Error::Orphan);
  }

And this later on should match for a NotFoundErr and in that case return an Error::Orphan.

  let prev = try!(ctx.store.get_block_header(&header.previous,).map_err(|e| {
    Error::StoreErr(e, format!("previous block header {}", header.previous))
  },));

Forks do not require a head. They just need to have some previous block that's findable. Does that make sense?

[antiochp]

Fixed (I believe) on testnet1 via #413 .
Will be fixed on master in #410.