Block header signature issue

[paulhauner]

state.latest_block_header.signature is different between producing and processing a block and this is causing block processing to fail because of a mismatch on block.state_root.

Producing

When calling per_block_processing, state.latest_block_header.signature is EMPTY_SIGNATURE.

It's impossible to have the real block signature here as you need the output of per_block_processing(..) (the updated state root) before you can produce the state.latest_block_header.signature and the value of state.latest_block_header.signature changes the state root, which changes the signature, and so on.

Processing

When calling per_block_processing, state.latest_block_header.signature is (hopefully) a real block signature.

[paulhauner]

Initially, I thought this could be fixed by ensuring that get_temporary_block_header(..) uses EMPTY_SIGNATURE instead of block.signature.

However, that raises another issue: all the hashes in latest block roots are just hashes of:

• slot
• previous_block_root
• block_body_root

We no longer have signature, which represented a ton of information (state root, fork, etc).

I'm guessing this arose as a desire to remove previous_block_root as a parameter to per_slot_processing. Thinking about that some more, I'm not sure you can avoid passing the previous block (or its signature) to the next state if you want the block signatures in the state. If the block needs to sign across the state, it's impossible for the state to contain the block signature. Your only solution is to store the block signature and pass it into a state that isn't included in the block signature (the next state).

[vbuterin]

Yeah, you're right there's a circular dependency. If we want the pre-state root of block N+1 to include a commitment to the signature, then the signature can't be over the post-state-root of block N as that's the same as the pre-state root of block N+1.

I'm inclined to say get_temporary_block_header using EMPTY_SIGNATURE is a pretty clean solution. But note that this does introduce a slight change to protocol properties: the block signature is no longer part of the block hash. I suppose this is meaningless because at least with the current BLS scheme there's only one possible valid signature for each block hash anyway, but it is something that we'd need to keep in mind in case we change the signature algo in the future (eg. we'd need to guard against DoS issues arising from a proposer issuing 1000 identical blocks with 1000 different but all valid signatures).

We no longer have signature, which represented a ton of information (state root, fork, etc).

I don't think we need to worry about this, because the state root itself does get included in the header (at the start of processing the next block).

[JustinDrake]

there's a circular dependency

Argh, I messed up!

I suppose this is meaningless because at least with the current BLS scheme there's only one possible valid signature for each block hash anyway

One of the considerations discussed with @djrtwo is cleanliness around identifier schemes. We probably want to have a consistent "canonical" notion of message identifier, if anything for block explorers. If we exclude signatures then we may end up with the two types of identifiers used in the wild. Also, if we exclude signatures from block identifiers we may want to the same thing for attestations, transfers, etc.

Maybe the best approach is simply to revert to a (pre_state, previous_block_root, block) -> (post_state, error) state transition function with "oracle access" to previous_block_root. How much do you value a (pre_state, block) -> (post_state, error) state transition function @vbuterin?

[JustinDrake]

Another approach would be to add a previous_block_signature field to BeaconBlocks (and possibly remove previous_block_root).

[JustinDrake]

Ok, I think I've found a satisfactory solution (see #799). Basically we remove latest_block_header and instead cache hash_tree_root(block) after checking block.state_root == hash_tree_root(state). This "hack" allows us to maintain a clean (pre_state, block) -> (post_state, error) state transition function. It can also be rephrased as a state transition function with oracle access to previous_block_root if people prefer thinking about it this way.

[djrtwo]

I'm currently thinking that the cleanest way forward is to use signed_root(block) for both signing and prev_block purposes. This allows us to use the current mechanics and avoid any sort of oracle because the relevant block information is cached in the latest_block_header except for the state_root which cached at the next slot.

@JustinDrake We can use signed_root(obj) for "canonical" identifier for anything that is signed and hash_tree_root otherwise.

EDIT: I was wondering why this didn't come up in tests and realized it is essentially what was being done in the tests anyway. I was assuming latest_block_header was valid in state and useful and was thus building the chain the using hash_tree_root(latest_block_header) as the prev_block. Modifying the signed_root(block) would be trivial and do essentially the same thing.

[vbuterin]

Basically we remove latest_block_header and instead cache hash_tree_root(block) after checking block.state_root == hash_tree_root(state).

The main challenge with this is that it introduces another point of ugliness which is the distinction between "signed post-state root" and "real post-state root".

[vbuterin]

Here's what seems to be a complete categorization of alternatives:

1. Roots that go in the state don't include signatures, and those signatures forever "disappear" from the state transition function's knowledge; they never get merkled in anywhere
2. Signatures get added in after calculating post-state roots
3. Signatures get passed as an argument to the next block's state transition function
4. The block gets passed as an argument to the next block's state transition function
5. The next block includes the signature of the previous block. At network layer, blocks have to be bundled with proposer signatures.

(1) is current status quo, (4) is original status quo. (3) is just a version of (4). (5) seems bad because it means that "a chain" does not contain enough information to build on top of the chain (namely, the signature). (2) seems annoying because it creates the distinction between "signed post-state root" and "real post-state root", unless we change the rule so the state root of a block is the pre-state root, which seems like a bad idea.

[djrtwo]

I'm opting for (1) with the modification that we cache block roots as signed_root(block) to be very explicit about what the root is and not have to have clients set block.signature = EMPTY_SIGNATURE before getting a block root.

PR here #816

[JustinDrake]

This was fixed in 0.5.1