-
Notifications
You must be signed in to change notification settings - Fork 997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate document for phase 1 custody game #818
Conversation
Text moved away from `1_shard-data-chains.md` (see #812 rewrite).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice start, but either I am missing something or there are quite a few things that need to be changed with this.
A further thought: this game is missing challenger penalties for challenges that fail.
Something also doesn't quite sit right with me regarding the way bad responders are slashed, but I'll check again in the morning.
# Add new chunk challenge record | ||
state.chunk_challenge_records.append(ChunkChallengeRecord( | ||
challenge_index=state.challenge_index, | ||
challenger_index=get_beacon_proposer_index(state, state.slot), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of forcing the chunk challenger to be the proposer, but I agree it is much simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is also a question I have. To be discussed on today's call.
)) | ||
state.challenge_index += 1 | ||
# Postpone responder withdrawability | ||
responder.withdrawable_epoch = FAR_FUTURE_EPOCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this not open a grieving vector whereby a responder's exit is delayed up to CUSTODY_RESPONSE_DEADLINE
by repeatedly posing challenges?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vitalik is planning to revamp withdrawability. See item 23 in #675. To be discussed in today's call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not see how validators are forced to reveal their custody secrets? Don't we need to add a deadline for that?
assert min_challengeable_epoch <= slot_to_epoch(challenge.attestation.data.slot) | ||
# Verify the responder participated in the attestation | ||
assert challenge.responder_index in attestation.validator_indices | ||
# A validator can be the challenger or responder for at most one challenge at a time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rationale behind only being responder to one challenge at a time? Would that not mean that I can prevent being challenged by challenging myself (possibly with another validator)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a question I have for Vitalik :) To be discussed in today's call.
assert verify_standalone_attestation(state, convert_to_standalone(state, challenge.attestation)) | ||
# Verify the attestation is eligible for challenging | ||
responder = state.validator_registry[challenge.responder_index] | ||
min_challengeable_epoch = responder.exit_epoch - EPOCHS_PER_CUSTODY_PERIOD * (1 + responder.max_reveal_lateness) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rationale behind this? Am I misreading this or does that mean if the responder has taken longer on past reveals, they will get "more buffering" from getting challenged before their exit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that the logic around max_reveal_lateness
smells. I'd like to remove max_reveal_lateness
. To be discussed on today's call.
See also #818. === * Replace custody challenge game with JABS Replace the existing proof of custody game with a new game ("Justin's Awesome Bit Sum" or JABS) that works as follows: * The data `D` is split up into 512-byte chunks `D[0] .... D[n-1]`, and use a mix function `mix(subkey, data) -> {0,1}` (currently the first bit of the hash of `subkey+data`). We calculate `M[i] = (mix(D[0]) + ... + mix(D[i-1])) % 2`, and set the custody bit to `M[n-1]` * Anyone can challenge by providing the full `M` where `M[n-1]` is not equal to the custody bit * Anyone can respond to a challenge by providing a specific position in `M` along with a branch of the data where `M[i-1] ^ mix(D[i]) != M[i]` The maximum size of data is now `2**6` epochs * `2**6` blocks * `2**14` bytes = `2**26` bytes, so assuming 512-byte mix chunks the maximum mix size is `2**17` bits or `2**14` bytes. The average mix size is `2**8` bytes.
We now have two phase 1 documents:
1_shard-data-chains.md
: details for the fork choice rule for shard data chains1_custody-game.md
: details changes to the beacon chain to support the custody game