Custody punishments for late reveals & deterministic staggering #872
Conversation
|
Cool! One question: Theoretically, we do not need latest_custody_reveal_period to be an integer, right? It could be a boolean that stores whether a reveal was published in this period. |
Wait, why do you need to store when each validator's latest reveal period started? In my proposal, the only thing that gets staggered is the minimum reveal block, custody periods are still aligned. If custody periods are all aligned, then I would argue that my staggering strategy is superior because relying on validator index means that there's persistent unfairness where some validators get always earlier reveal times than others. If custody periods are not aligned, I agree your approach is best. What do you see as being the benefits and costs either way?
No because the +1 lateness bound is too tight to be safe (see my other comment), it needs to be +8. |
This comes down to the stringency of my slashing mechanism. As everyone who has not revealed is slashed by the start of their next period, it would be unfair to allow certain validators only 1 block to reveal their key.
I was staggering the periods of each validator (up to 1 custody_period phase difference between any two validators). I was thus targeting non-aligned custody periods hence the ridiculous naming of
I don't think it really matters much either way. The downsides of doing it your way are increased hashing (minor) and lack of predictability of when a particular validator's reveal period begins. On the other hand, my system has constant bias (as described by me as "modulo-bias" in my initial comment) in the sense that blocks at the start of a reveal period could contain 1 more reveal every time. |
Co-Authored-By: CarlBeek <carl.beek@gmail.com>
I like the approach that Carl is suggesting here, because it minimises the time from when a custody secret is used to its corresponding reveal period. The expected rational behaviour is to reveal a custody secret as soon as possible after using it, so this approach has two advantages:
Also the logic does seem to have very little overhead. But overall, the difference is fairly minor, so I don't feel very strongly about it. |
* Limits bit challenges to maximally ~73 + 9 days in the future. * Prevents exists if ∃ open custody bit challenges * Updates bit-challenge processing to correctly check whether the challenger is slashable
| # Cannot exit if you have not revealed all of your custody keys | ||
| elif epoch_to_custody_period(revealer.activation_epoch) + validator.custody_reveal_index <= epoch_to_custody_period(validator.exit_epoch): | ||
| elif validator.latest_custody_reveal_period <= epoch_to_custody_period(validator.exit_epoch): |
There was a problem hiding this comment.
I think if it's latest reveal rather than next reveal, the inequality should be <, not <=. The rationale is that if you exit during epoch N, that means that you used the subkey for epoch N and previous epochs, so you need to reveal all the subkeys up to and including N, so the "latest reveal" should be N for an exit to be valid.
So if the slashing mechanism only works after 8 epochs, this seems like it matters less.
I like how it stabilizes the time between use and reveal, but I actually think minimizing the time could be a bad idea. Revealing data literally a block after the period ends opens you up to slashing if the chain reorgs. That said, I suppose this is something that we could leave open as a tradeoff to the user - if they wait longer, then they get more safety, but then they would have to store more data in the meantime. |
| # Verify the attestation | ||
| 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.
We can't get rid of the max delay because then the validator would be liable for an unboundedly growing period of data. We also can't set max delay to equal the custody response deadline because that would just be too much data to store (2 MB per epoch * 2**14 epochs = 32 GB); we want a validator to only be responsible for the data in a single period (unless the validator screws up eg. by delaying subkey reveals). So we need a max delay of 1 in the normal case, but if the validator does delay subkey reveals we need to increase the max delay to make sure that when the subkey is revealed the data can still be challenges. This was the rationale for max_reveal_lateness.
There was a problem hiding this comment.
@JustinDrake and I thought about this and couldn't come up with a way to solve this issue without having that variable, despite its ugliness, though what we can do is ameliorate the problem where if you're late once you're stuck with a longer period forever, by having max_reveal_lateness be measured in a finer-grained time interval (eg. epochs) and have it slowly decrease over time if a validator shows good behavior (another isomorphic approach is to have a min_challengeable_epoch that, if it is more than a full period behind the current epoch, moves forward eg. 2 epochs per epoch.
There was a problem hiding this comment.
Ah, now I see the rationale for max_reveal_lateness. I like the idea of having it decrease over time.
| (of the particular validator) *AFTER* which the validator was activated. | ||
| = get_validators_current_custody_reveal_period(...) + 1 | ||
| ''' | ||
| 'latest_custody_reveal_period': 'uint64', |
There was a problem hiding this comment.
So the first subkey that the validator will be revealing is from the period that is two periods ahead of the current one? Not sure I see the logic here. You sure we don't want - 1?
And if it is -1, then it seems better to avoid negative numbers and keep track of next_custody_reveal_period instead of latest_custody_reveal_period.
Good point. Would it make sense to only allow slashing while the custody secret is actually in use? Then you can never get slashed for revealing your secret, which makes sense, and if it is accidentally disclosed after the custody period it is presumably not really an offense worth slashing. |
Aren't we doing that already? At least we were when I wrote the original code; that's definitely how it should work IMO. |
| proposer_index = get_beacon_proposer_index(state, state.slot) | ||
| increase_balance(state, proposer_index, base_reward(state, index) // MINOR_REWARD_QUOTIENT) | ||
|
|
||
| # Case 2: masked punitive early reveal | ||
| else: | ||
| assert reveal.period > current_custody_period | ||
| assert revealer.slashed is False | ||
| assert reveal.period >= validators_next_custody_reveal_period |
There was a problem hiding this comment.
| assert reveal.period >= validators_next_custody_reveal_period | |
| assert reveal.period >= get_validators_current_custody_reveal_period(state, reveal.masker_index) |
Only allow slashing for secrets that are still in use or will be used in the future
Yup, probably my bad, I accidentally removed that logic in a suggestion earlier |
|
This is being deprecated due to the improved rewrite that is #934 :) |
This PR implements the following:
validator_index%EPOCHS_PER_CUSTODY_PERIOD. This introduces modulo-bias and the earlier epochs in a period could have one more reveal each. The staggering strategy from Stagger reveal slots #869 is not used for two reasons: it requires storing when each validator's latest reveal period started (as it cannot be calculated later as it is dependent on the RANDAO state) and secondly, this mechanism allows validators to know when (approximately) their reveal period will begin.max_reveal_latenessI think all the functionality that was provided bymax_reveal_latenesshas been replaced, but please check my logic.Hereby items 6, 8, and 9 from #864 are addressed.