NPoS Challenge Mode

[coriolinus]

Builds on #8160.

• Updates the PJR check to return a counterexample if one exists
• Adds functions to cheaply check counterexamples

This is in support of off-chain PJR challenges: if a miner discovers
that an accepted election solution does not satisfy PJR, it will be
eligible for substantial rewards. This helps ensure that validator
elections have an absolute quality floor, so even if someone manages
to censor well-behaved solutions to give themselves unfair representation,
we can catch them in the act and penalize them.

[kianenigma]

You probably intended to change this to merge into your other branch?

[coriolinus]

I'll follow whatever the convention is; my intent was to rebase once #8160 is merged.

[kianenigma]

API is confused. The challenge, ideally has just one input:

1. counter_example candidate. Next to this, we will need to pass in all the voters as well, and nothing more to validate the challenge.

From this, we just compute prescore(counter_example, voters, t).

So no need to pass support and all_candidates` in.

[coriolinus]

The code doesn't iterate over all candidates, except to the degree that it searches the candidate list for the one of interest. However, it does require the full list of supports, all candidate, all voters. The chain of requirements goes like this:

• To validate a PJR challenge fundamentally requires calculating its pre_score
• pre_score requires calculating the slack for each voter
• slack requires an accurate list of the voter's edges: all the candidates it voted for, and by how much
• Therefore we need, bare minimum, &[Voter<AccountId>].
• Voter is a struct with private fields and a complicated internal structure. It would be hard for users to accurately construct it on their own even if we made the fields public or provided a constructor.
• We can construct the list of voters simply by reusing fn prepare_pjr_input, which requires the full list of supports, candidates, voters.

A reasonable argument could be, "but why not extract a subset of prepare_pjr_input which can handle constructing the inputs relevant to a single candidate?" My answer to that argument is that it's more trouble than it's worth. This module assumes that the lists of supports, all candidates, all voters are available to modules who care about the election. Making those inputs a prerequisite for counterexample proofs seems to be a small additional burden.

[kianenigma]

My answer to that argument is that it's more trouble than it's worth. This module assumes that the lists of supports, all candidates, all voters are available to modules who care about the election. Making those inputs a prerequisite for counterexample proofs seems to be a small additional burden.

We will surely need to run the pjr-counter-example check on-chain and per proof that we receive. I suppose this could be due to you not being yet very familiar with the internals of frame, but something that runs on-chain and can be triggered by the user is def. worth being optimized. So in your argument of more trouble than it's worth, you are very much underestimating the worth part.

All in all, we could also meet in the middle and merge this as-is and update the API once the challenge phase is in. But note that that is not imminent and we first need to finish the unsigned + signed phase, so I see it happening in the next ~3 months or so, based on how fast we move.

[coriolinus]

but something that runs on-chain and can be triggered by the user is def. worth being optimized.

Could we apply an economic incentive instead? We're already planning on incentivizing users to submit counterexamples by providing some reward to them. Couldn't we also apply some penalty in the event that the user submits an invalid counterexample?

[coriolinus]

Reduced to unique commits; should be easier to review now.

[kianenigma]

One random note unrelated to this PR per se: this pallet had quite a bad rap for not having a well defined behaviour with duplicate votes and candidates and targets etc. Now, finally, everything that goes through setup_input (called prior to seq_phragmen and phragmms) gets sanitized. Would be good to ensure these functions have the same standard.

[coriolinus]

prepare_pjr_input mentions setup_inputs in its documentation, but doesn't actually delegate. Shall I create an issue to add explicit delegation there to ensure that the behaviors are identical?

[kianenigma]

An issue would be good, bit I am not sure if delegation is the best way, we have to see. We can maybe mix this into #4593

[kianenigma]

The only thing I see missing is a test. Otherwise looks clean.

[shawntabrizi]

this exact text is copied on all these functions?

[shawntabrizi]

Also any advantage to breaking up the steps into such minimal functions? I feel if anything, this may actually loose context of what other operations are happening between the steps, where optimizations may be added.

[coriolinus]

The difference in the text is

- /// This function uses the standard threshold.
+ /// This function uses a supplied threshold.

The documentation goal is to ensure that each function's documentation completely describes what that function does, even though a similar function exists.

As for why the functions are factored in this way, it comes down to two things. On the one hand, it mirrors the factorization of the pjr_check functions. On the other hand, it means that users can supply a threshold, or not, as desired. We have use cases for both.

[shawntabrizi]

looks clean

[coriolinus]

bot merge

[ghost]

Trying merge.