Refactor header roots

[adlerjohn]

• evidence root
• validators/state roots

[liamsi]

Rendered:
https://github.com/LazyLedger/lazyledger-specs/blob/adlerjohn-refactor_header_roots/specs/data_structures.md
and
https://github.com/LazyLedger/lazyledger-specs/blob/adlerjohn-refactor_header_roots/specs/architecture.md

[adlerjohn]

@liamsi Refactored the header to remove validator roots (both), and added a stateCommitment field, which is computed as hash(stateRoot, validatorRoot). Does this seem reasonable and in line with previous discussion? If yes, I'll update the data structures figure and flag this for review.

[liamsi]

Still need to go through the tendermint changes required for this (celestiaorg/celestia-core#3) but independent of that this seems reasonable.

Question: Is hash in hash(stateRoot, validatorRoot) the same hash function as used for the sparse merkle tree that yields, e.g. stateRoot

[adlerjohn]

Is hash in hash(stateRoot, validatorRoot) the same hash function as used for the sparse merkle tree that yields, e.g. stateRoot

Yes, my understanding is that only a single hash function is used everywhere (keccak-256).

[liamsi]

Do we need to prefix the stateRoot and the validatorRoot with the tree's inner node prefix?

[adlerjohn]

I don't think we need to prefix it with anything, since it's a fixed operation. But it wouldn't hurt to just prefix it, that way it might be easier to parse, as it can be interpreted as the parent node of two potentially-asymmetric subtrees.

[liamsi]

From https://github.com/LazyLedger/lazyledger-specs/blob/adlerjohn-refactor_header_roots/specs/architecture.md

Transactions are signed and must be processed by clients to determine the validator set, while messages are un-signed data blobs that will usually represent an app's block data.

Didn't we settle on including messages inside of Tx?

[adlerjohn]

Didn't we settle on including messages inside of Tx?

I don't think so? Txs that pay fees for a message to be included (not all txs do this, some just e.g. change the validator set) shouldn't include the binary data as part of the tx itself, otherwise some client profiles would have to fully download messages even if they don't care about them. For example, if I only care about the LazyLedger validator set, I can just ignore the entire message namespace. Txs should only include a commitment to a message.

[liamsi]

I misunderstood the outcome of this discussion then: #2 (comment)

I thought it might be simple if we have a single Tx type type Tx {msg: Msg} and that model could also be used for validator set changes and everything.

This is kinda orthogonal to this PR. We can continue the discussion in a separate issue. If I want to include a Msg, what do I do / what happens (and if sending Tx{commitToMsg} and Message, is two operations couldn't I spam the network with unsigned messages that do not have a corresponding Tx)?

[adlerjohn]

Transactions can certainly be sent around the network as Tx {amount, recipient, witness[, msgBytes]}, then encoded in the block as Tx{amount, recipient, witness[, msgID]} and Msg{msgBytes} (where msgID = hash(msgbytes), and can be computed before relaying the transaction).

The point of splitting up txs and msgs inside the block is that there can be users that want to ensure the integrity of the LazyLedger coin and make sure that the message data is available, but they don't care what the message data actually is.

[liamsi]

Makes perfect sense inside the block! I was looking from an encoded on / decoded from the wire perspective I guess.

[liamsi]

LGTM 👍