rm signing root (fixes #1487)

[protolambda]

The reasoning for the signing-root removal can be found in #1487

This is a proposal. So far there was reasonable support expressed, but more discussion/suggestions are welcome.

Changelog:

• Introduce signed containers:
  • DepositData already uses "data". And since we sign over a message, I think I like "msg" as general field name for the (msg, signature) type wrappers.
  • May generalize it to SignedContainer[MsgType] helper type construction. Just want to see it working first.
• Update containers/state to use new signed and bare container types.
  • No empty zeroed signature in the state anymore
  • Operations (and things like proposer slashings) use the new signed containers.
• DepositData is an edge case: I made a DepositMsg type to sign the contents over, replicating the exact old signature of signing root. I don't think this is much of a problem, as DepositData is not a message anyway, and within Deposit it is indeed more like data (with a proof) than a message (with a signature).
• Improve transition function for proposers: to verify the signature apart from the bare header contents (which don't contain a signature anymore). Blocks and headers are still transparent, but can now both be wrapped in Signed variants to (block/header into msg field, and add the signature field)
  • Validator API doc is improved with this now: a minimal working function to produce a valid block state root.
  • Split previous test case into an invalid-sig and zeroed sig case, just to be sure.
  • Changed the validate_state_root to just "checked": the signature signs over the state root. It makes no sense to ignore the state root, but then enforce it partially with the signature check (testing infra previously signed twice: one dummy signature for missing state root, then the real one once the state root could be computed).
    • Also note that process_blockheader is now so clean it could have been a BeaconBlockHeader as input type. But kept it as block, as scoping it to just the header data only requires the spec to convert from normal block to header. Clients can abstract/interface this nicely how they like it (or nearly keep it as is).
• Updated validator and network docs to account for this removal of signing root. Verbosely again to make it clear: SignedBeaconBlock.msg is a BeaconBlock, and the hash_tree_root of this BeaconBlock msg is signed over to create signature signature.
• Testing infra improved:
  • Use new signed container types
  • always clearly state "signed_block" when it's a (msg, signature) pair.
  • hash_tree_root everywhere
  • fix signature problems with fork-choice pyspec tests (Ran full test suite with BLS forced on to catch problems, and noticed these got in, since we don't output them as vectors, and CI didn't check the BLS part of these tests)
  • No more double signature hack or just-in-time randao signature creation. Clear bare data is passed around and when signed wrapped in the appropriate signed container type.
• no signingroot 🎉

Note: left old Phase 1 use of signing root as-is, it's changing anyway with the new PR (see #1483 )

We may want to squash this PR when merging. Or I can rebase and split the test work from the spec changes.

[hwwhww]

1. IMHO It’s a bit annoying to see msg/Msg instead of message/Message in the spec.
   • Reason: msg is usually coming with sig, but we use signature in the spec.
   • However, the longer name may be verbose. I'd like to hear what other people's taste is.
2. I think the complexity of implementing this change won't be too scary and painful. Mostly many search and replace commands.

(editted)

[protolambda]

@hwwhww renamed checked to validate_result. Two things to consider here:

• I think it should absolutely be True by default, even if overruled elsewhere. Verification of the block signature and state root are probably the two most important things to eth2 phase0...
• Added an unsigned-transition method in the testing machinery. Which is a clean call to process_slots, process_blocks, and done. Same thing in the validator spec for proposers to build their block. But for spec purposes, I think it's useful to highlight the option as part of the beacon chain spec itself, or alternatively we link to the validator spec which describes the transition for a proposer building its block.

[djrtwo]

Although I'm resistant to change, this is very good

[djrtwo]

Let's group all signed containers in a sub-section below the declaration of other containers. Let's put a brief explanatory text at the top of the sub-section

[djrtwo]

bump, still prefer this method. The spec building fixes topological ordering so that's fine

[protolambda]

See afb9a1d, you can change the wording if you like.

[protolambda]

Changed msg -> message and DepositMsg -> DepositMessage, and squashed the PR commits.

[arnetheduck]

LGTM - generally, naming has been brought up as a potential area of improvement and I'd generally agree that it would be good to do a holistic rename round once something like this is merged so that there is structure to the naming (DepositMessage for example looks a bit odd)