search

ethereum / portal-network-specs

Official repository for specifications for the Portal Network
313 stars 85 forks source link

Add HistoricalRootsBlockProof for merge till capella blocks #287

Closed kdeme closed 3 weeks ago

kdeme commented 7 months ago

The way for proving these blocks was already presented/discussed a while back and in the mean while we also have test vectors added thanks to @ogenev (https://github.com/ethereum/portal-spec-tests/pull/9), so I suggest that we start thinking about a path for activating this in the network.

Aside from potentially adjusting the description in this PR and having it implemented in the clients, we will also need changes to the bridges to build these proofs and, more importantly, discuss on how we will get rid of the old headers without the proof that might already live on the network. Hence, keeping this in draft for now.

kdeme commented 2 months ago

IIUC this gives us a proof format for blocks from the merge to Capella, during which the beacon chain was using HistoricalRoots, after which the beacon chain migrates to HistoricalSummaries.

That's right.

I have an open question about what the HistoricalRootsBlockProof.beaconBlockRoot field represents. My assumption (which may be wrong) is that the field is the beacon block for which the proof is anchored.

This is the root of the BeaconBlock of which the EL block hash is part of (BeaconBlock -> BeaconBlockBody -> ExecutionPayload). The BeaconBlockProof is then the SSZ merkle proof that this EL block hash is part of the BeaconBlock with that root. So the verify part requires this proof + the BeaconBlock root as root and the EL block hash as leaf.

The other part of the verification is to make sure that the BeaconBlock used is canonical. This uses the historical_roots and works pretty much the same as our accumulator pre-merge. So the main difference here is that we get to proof if a BeaconBlock is canonical, and not an EL block. But as an EL block basically part of the BeaconBlock, we can used the above explained extra step to get also the EL block (hash) proven.

The historical_roots can/will be baked in the binary as it is frozen.

What if we anchored all of these proofs to a fixed root that could be baked into every client, so that all blocks from this range would be able to be proven against a fixed root hash. That root hash would probably be either the last block before the fork or the first block after the fork, and then all of the proofs of this type could be proven against that singular block root without need to source data from another network.

I don't think I fully understand this part. Is it still applicable with above explanation?

kdeme commented 2 months ago

I have rebased the PR and updated it with renaming of the proofs + extra text clarification + some charts in 6d7a5227479bd1e8959756dbb4dcb3df3698e461, fbb2a43 and af07120.

kdeme commented 1 month ago

This PR is blocked by a solution/action taken for removal on the None in the Union of the block proofs and also https://github.com/ethereum/portal-network-specs/issues/337

pipermerriam commented 1 month ago

Looks like we should probably:

  1. Leave the None in for now.
  2. Move to have support for the new content types at the position in the Union where they are in this PR.
  3. Wait until after Devcon and remove the none as part of #341

Clients can still accept headers without proofs. We'll end up with a network with some awkward mixed databases where some have proven headers and some have unproven headers for certain blocks. We'll clean it up later with #341 getting merged.

kdeme commented 1 month ago

I'm fine with merging this as is now as long as we later do still have a look at either: a. Removing the None and thus shifting al the selector b. or Removing the Union all together.

KolbyML commented 1 month ago

I'm fine with merging this as is now as long as we later do still have a look at either: a. Removing the None and thus shifting al the selector b. or Removing the Union all together.

Kim and I discussed this a bit offline. I am kind of a fan of b but we will have a wider discussion with the teams on this issue

morph-dev commented 1 month ago

In general, it looks good to me.

Since there was some discussion about naming, I have an idea but I'm not sure if it was considered before. How about naming top level proofs something like: BlockPreMergeProof and BlockPreCapellaProof? It makes it clear when each proof type should be used.