Put block header and block body together in one object

[jangko]

nimbus-eth1/nimbus/p2p/chain.nim

Line 128 in e76e014

method persistBlocks*(c: Chain, headers: openarray[BlockHeader], bodies: openarray[BlockBody]): ValidationResult {.gcsafe.} =

One of the most annoying method appears in both nimbus-eth1 and nim-eth is persistBlocks. It require block header and block body separated in two object.

The reason why it need to be in one object:

• all test data from EF test suite encode block in one RLP blob, no header and body separation.
• the --import command line options required by hive also provide block header and body in one RLP blob.
• graphql test suite reveals this separation become annoyance because we need to repeatedly separate header and body when doing block validation while it is obvious the more natural thing to do is put them in one object.

Currently what we have is:

type
  BlockHeader* = object
    parentHash*:    Hash256
    ommersHash*:    Hash256
    coinbase*:      EthAddress
    ...

  BlockBody* = object
    transactions*:  seq[Transaction]
    uncles*:        seq[BlockHeader]

The new object will be like this:

type
  EthBlock* = object
     header*: BlockHeader
     transactions*:  seq[Transaction]
     uncles*:        seq[BlockHeader]

Although the eth/xx network protocol might not request or send a block in one message, we can assembly the pieces when they arrived. why assemble it into headers and bodies while we can assemble them into blocks.

[jlokier]

We won't be fetching block headers and block bodies exactly synchronised. In some cases we only want the header in memory without even fetching the body. Perhaps also in storage. So there will necessarily be block headers that don't have bodies, and operations need to reflect that.

Why is the current setup annoying? Provided you can look up the body given a header, wouldn't you just do all operations with BlockHeader representing a "block", and then accessing the body fields is almost as trivial as chasing a reference field, maybe using a helper proc to make it look exactly like that? The annoying part I think of with that, is sometimes accessing the body fields (transactions, uncles, etc) will report "we don't have those". But that's intrinsic, we won't have them for some blocks.

I'm not sure what the GraphQL service reports exactly, but if it's to let the client query whatever data we have, perhaps it needs to report block headers that we have without bodies.

[jlokier]

The definitions of BlockHeader and BlockBody are just a deserialised version of the RLP, "plain old data" (POD). They aren't really for using as a data structure, for traversals, indexing, attached run-time state etc. Same is true of Transaction, etc.

I think that is the underlying cause of annoyance, and having to keep track of both things as a pair all the time is a symptom from that cause. Same thing affects other areas, especially the sync and storage processes.

Real data structures are needed in other areas too in nimbus-eth1. An EthBlock object, that contains a reference to BlockBody (or a method for that) as well as BlockHeader would be fine., if that helps you avoid keeping track of both things as separate refs all the time, or doing block-number/hash lookups all the time.

Just keep in mind, some EthBlock that are in memory won't always have an attached BlockBody because the body either has not been fetched (and might not be), or it hasn't been read from the database (which is a heavy operation when the database is busy, and some operations don't need the body).

[jangko]

this issue posing no problem anymore.