Custody game changes

[vbuterin]

1. Don't store the full chunk bits, instead only store a Merkle root. Increased history size complexity from N to N + log(N) but with the benefit of decreasing storage requirements from N to a single 32 byte hash.
2. custody_bit is computed as the first bit of the hash of the custody bits, not the xor. This allows us to more safely use functions with more risky security assumptions for computing the chunk mix.

[JustinDrake]

cc @dankrad for point 2

[dankrad]

Yes, I agree with point 2. I think using SHA256 instead of the XOR of custody bits will have the following implications:

1. Any PRF (weak or strong) will do as base for the chunk bit computations. We can concentrate purely on efficient computation and (lack of) outsourcability.
2. Small validator pools can compute the hash inside an MPC, so would be covered by the security of the hash function
3. Large pools (>10) would not be able to do the hash inside an MPC, and so would be vulnerable to attacks on the PRF should the PRF turn out to be weak. These attacks are only griefing attacks inside the pool itself.

I think 3 is probably an acceptable weakness of the proof of custody scheme. The only reason to change this if we come across a strong, efficient, MPC-friendly PRF with well-tested crypto assumptions.

[dankrad]

Actually, to be even clearer: If the PRF is strong, there is no disadvantage for having a SHA256 at all, because pools can then just do the SHA256 in public. Overall, there is no disadvantage over XOR as far as I can see.

[JustinDrake]

custody_bit is computed as the first bit of the hash of the custody bits, not the xor. This allows us to more safely use functions with more risky security assumptions for computing the chunk mix.

To make things more MPC-friendly, can we get away with a single hash? For example, can we xor 256-bit segments of custody_bit and then hash that?

[dankrad]

To make things more MPC-friendly, can we get away with a single hash? For example, can we xor 256-bit segments of custody_bit and then hash that?

I think Vitalik's construction only has a single hash: The custody bits (mix function of the custody chunks) are used as one big input to the hash function.

[JustinDrake]

I think Vitalik's construction only has a single hash

The code has get_bitfield_bit(merkle_root(pad_to_power_of_2((challenge.chunk_bits))), 0). This is better than a monolithic hash because you unlock parallelism. (The monolithic hash is inherently sequential hence not MPC-friendly.) My suggestion is to avoid the inherent sequentiality from the depth of the Merkle tree.

The custody bits

The nomenclature is "chunk bits", short for "custody chunk bits" :) There's only one custody bit (singular).

[dankrad]

I think Vitalik's construction only has a single hash

The code has get_bitfield_bit(merkle_root(pad_to_power_of_2((challenge.chunk_bits))), 0). This is better than a monolithic hash because you unlock parallelism. (The monolithic hash is inherently sequential hence not MPC-friendly.) My suggestion is to avoid the inherent sequentiality from the depth of the Merkle tree.
True, I only read the commentary, not the code. You are correct then, to make it as MPC-friendly as possible I suggest the following construction:

1. Recusively aggregate 2 chunk bits into 1 via XOR, until the number of bits left is less than 256
2. Hash the remainder (this will always be at least 128 bits)

[vbuterin]

We could do something like that; I was just using the Merkle tree function for simplicity, as it's the same as the function used to calculate the chunk bits root that gets stored in the state.

[dankrad]

I added my suggestion for an XOR-pre-aggregation, so that we only need to compile a single hash. Note that we still need to store the Merkle root in the CustodyBitChallengeRecord, as we want to use it for Merkle proving later. This does not matter though, except it may be slightly confusing to have to different "roots", so I renamed it to Merkle root

One remark: For consistency, should we call it hash_tree_root instead of merkle_root?

[vbuterin]

I don't think either of the proposals actually require two different roots to be stored in the state. One of the roots is there to check against the custody bit and then can be thrown out. The other root is there to check Merkle branches of responses, so it needs to be stored in the state.

[dankrad]

I don't think either of the proposals actually require two different roots to be stored in the state. One of the roots is there to check against the custody bit and then can be thrown out. The other root is there to check Merkle branches of responses, so it needs to be stored in the state.

Yeah, the custody bit root does not need to be stored (and the code I pushed does not store it). This was just a remark about there being two different "roots" which could cause confusion in the future.

[vbuterin]

Hence my support for using the same root in both cases :)

But the difference seems minor in any case...

[djrtwo]

ping @vbuterin on my couple of questions here. Would like to get these phase 1 PRs merged before the weekend if possible

[djrtwo]

This looks good, but noticed an issue outside the context of this PR.

We are still using convert_to_standalone and verify_standalone_attestation rather than convert_to_indexed and verify_indexed_attestation. More importantly, CustodyChunkChallenge and CustodyBitChallenge both have an Attestation as a field but are expected to be challenges from epochs in the past. Both of these objects instead need an IndexedAttestation (and thus are much larger than currently being accounted for)

[vbuterin]

More importantly, CustodyChunkChallenge and CustodyBitChallenge both have an Attestation as a field but are expected to be challenges from epochs in the past. Both of these objects instead need an IndexedAttestation (and thus are much larger than currently being accounted for)

#1009 resolved this.