Introduce a ZeroSig constant for the zero_signature

[mratsim]

This is the same suggestion as ethereum/eth2.0-tests#27

The serialized BLS signature: 0x000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 is not a valid compressed BLS signature on the curve. A valid one instead would be 0xc00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

While we all have some machinery to disable valid signature checks during testing, due to that invalid zero signature we also need to add specific test versus zero buffer before serialization/deserialization of every BLS signature instead of just handing it to the BLS library.

This is especially important for process_block_header which requires a zero-signature
https://github.com/ethereum/eth2.0-specs/blob/dev/specs/core/0_beacon-chain.md#block-header

[JustinDrake]

Would it be fair to say that process_block_header only writes BLSSignature() to state (specifically, state.latest_block_header.signature = BLSSignature()) and that this "invalid zero signature" in the state never has to verified, i.e. never has to be fed to bls_verify? I don't understand why BLSSignature() has to be checked as a valid compressed BLS signature.

[mratsim]

When serializing/deserializing, BLS Signatures have a specific conversion step that is handled by the BLS library. BLS libraries refuse to de/serialize invalid signatures.

This means that before serialization and/or deserialization we need a additional layer that handles this specific zero signature case.

Furthermore, it impacts the hash_tree_root serialization, meaning I expect all clients to have some kind of Option type to indicate zero-signature or not.

I feel like it is overhead that we can avoid, and it simplifies the logic by removing the special case of zero signatures, i.e. signatures are then fully handled by hardened BLS libraries from serialization to deserialization.

We already have the special case of invalid test signatures (which I hope we can remove at one point) but I hope to not have special cases just for serialization/deserialization of zero signatures in production implementations as well.

[JustinDrake]

BLS libraries refuse to de/serialize invalid signatures

Why do BLS libraries ever have to de/serialize the "zero" BLSSignature()? As far as I can tell state.latest_block_header.signature never has to be serialized or deserialised as a BLS signature (of course, it needs to be serialised into bytes in the context of SSZ but I don't think that's what you mean).

[arnetheduck]

I'd favour a solution which removes signature from BeaconState.latest_block_header completely. It will never contain a valid signature causing confusion and requires a workaround / kludge to serialize / deserialize.

We can achieve this in a very simple way: use a different type in latest_block_headers. It is a trivial change which arguably improves ease of understanding of the BeaconState data since there's no confusing and unused signature in there - it would help build intuition for the otherwise circular relationship between state and block processing.

From an implementation point of view, we deserialize BLS into its point representation before doing processing, along with other kinds of trivial validations on the data (like length checks, buffer overflow protection etc - basic data vailidity checks). Similarly, when calculating the tree hash, we serialize a an abstract point representation into its octet representation before passing it to the hash function (for reasons of efficiency, so that we only have to store the point version which more readily is useful)

In the draft BLS spec, one can see that there's a method called octets_to_point which returns INVALID if it is unable to deserialize the point - crucially though, it returns that value for any INVALID octet stream, not just the BLSSignature() that casually assumes an all-zeroes serialization. It means that we need to differentiate between all_zeroes, invalid and valid signatures, for no good reason really, except making it more difficult to write a conforming implementation.

it needs to be serialised into bytes in the context of SSZ but I don't think that's what you mean

this is exactly what we mean - the same SSZ bytes are also used on the wire on incoming blocks etc..

[djrtwo]

Why do BLS libraries ever have to de/serialize the "zero" BLSSignature()

This would happen in the context of serializing or deserializing a beacon state that has this signature. And I expect this is particularly an issue in a strongly typed language that is putting tight constraints on the incoming and outgoing data.

I'd favour a solution which removes signature from BeaconState.latest_block_header completely. It will never contain a valid signature causing confusion and requires a workaround / kludge to serialize / deserialize.

You certainly can make a different ssz type that doesn't have the signature but then you have hash_tree_root(latest_block_header) == signing_root(block) rather than signing_root(latest_block_header) == signing_root(block) which obfuscates the connection between these two structures and might be a source of bugs down the line, imo.

I'm not terribly opposed to ZeroSig but want to hear input from more teams.

[arnetheduck]

You certainly can make a different ssz type that doesn't have the signature but then you have

the idea is that you keep the relevant fields in BeaconState (perhaps in a separate type, or directly), explicitly construct a BeaconBlock from those fields then sign it as usual, instead of reusing the current type. You never call signing_root(latest_block_header).

[mratsim]

Related: #1487 and #1491

[djrtwo]

Closed via #1491. The removal of signing_root and the addition of explicit containers for signed objects prevents the need for ZERO SIGS in the consensus