Panic on invalid unsigned election solution.

[kianenigma]

Closes #5980

---

This is small change, but has been the consequence of a long period of monitoring.

Context

Validators submit solutions to the staking election via an unsigned transaction that they can place in only the blocks that they author. Technically though, nothing is preventing a validator from placing a wrong solution in the block that they author, forcing all nodes to check the solution, only to realise later that the solution was wrong. It was proposed by Gav here to panic in such cases to prevent such behaviour.

Initially though, I did not implement this, and it turned out to be a good choice.

Previously

The reason for not putting this was that I was not yet confident that our code for generating the solution and checking it is 100% correct. In case we had a bug, I didnd't want to to cause problem for out validators. It turned out in #6106 that we were actually sometimes submitting solutions that are slightly worse, due to an optimisation that led to removing the pre_dispatch. The situation was that validators were submitting solutions that were deemed to be valid x blocks before, now, because they skip pre_dispatch, which was obviously wrong. These wrong solutions were actually backed into blocks and got propagated. Absolute waste of time and cpu cycles for everyone.

Now

#6173 Fixed the above.

I've been monitoring both Kusama and Polkadot and since this patch, we have not yet had a single solution by validators that were incorrect. Hence, we can apply the panic in the runtime, ensuring that no sane validator would intentionally misbehave here.

Example log from Recent eras in polkadot:

[🤑][Era 21, block 400040] Submit election solution with score [22950041619623102, 3684200937834987110, 130592181899507675556038715493485588] // Event = Some("SolutionStored")
[✅ // [0.00000% ,0.00000% ,-0.05059% ,]][Era 21, block 400063] Submit election solution with score [22950041619623102, 3684200937834987110, 130526117831985403178900621494263402] // Event = Some("SolutionStored")
[✅ // [0.00000% ,0.00000% ,-0.05326% ,]][Era 21, block 400123] Submit election solution with score [22950041619623102, 3684200937834987110, 130456600308203553849338983768954394] // Event = Some("SolutionStored")
[🤑][Era 20, block 385757] Submit election solution with score [21167650000000000, 3480991383199356235, 129128496884674807500286734142526857] // Event = Some("SolutionStored")
[✅ // [0.00000% ,0.00000% ,-0.23229% ,]][Era 20, block 385760] Submit election solution with score [21167650000000000, 3480991383199356235, 128828546426915343923818636055221171] // Event = Some("SolutionStored")
[🤑][Era 19, block 371527] Submit election solution with score [15837963726980462, 3281658404272283292, 131310769044686788245009156267438228] // Event = Some("SolutionStored")
[🤑][Era 18, block 369159] Submit election solution with score [15436087488857207, 3272493354354849800, 140754040550817604129771214279897632] // Event = Some("SolutionStored")
[🤑][Era 17, block 366787] Submit election solution with score [27725010000000000, 3259900284909882343, 147360731290682736657392552349970199] // Event = Some("SolutionStored")
[🤑][Era 16, block 364386] Submit election solution with score [27325010000000000, 3255516218813399944, 157556832542189171084547716335453964] // Event = Some("SolutionStored")
[🤑][Era 15, block 361985] Submit election solution with score [42636334572595680, 3225084255044351507, 166794184360492348405008122270889119] // Event = Some("SolutionStored")
[🤑][Era 14, block 359587] Submit election solution with score [53641665575646601, 3220882606315638480, 181504197556687913409982990039970884] // Event = Some("SolutionStored")
[✅ // [0.23326% ,0.00000% ,-0.00160% ,]][Era 14, block 359589] Submit election solution with score [53766789555199512, 3220882606315638480, 181501286131130814149153285889749532] // Event = Some("SolutionStored")
[🤑][Era 13, block 357187] Submit election solution with score [40729481246662089, 2797436123654819907, 152468229479098170468655436641477281] // Event = Some("SolutionStored")
[✅ // [1.56887% ,0.00000% ,-0.07975% ,]][Era 13, block 357188] Submit election solution with score [41368473021604635, 2797436123654819907, 152346641739837129585727713602076469] // Event = Some("SolutionStored")
[🤑][Era 12, block 354786] Submit election solution with score [45977221927701680, 2797955374522807813, 171171983750914543519177413321127047] // Event = Some("SolutionStored")
[🤑][Era 11, block 352387] Submit election solution with score [46500443327992676, 2797060059922584085, 185788536137105766524633196802669079] // Event = Some("SolutionStored")
[🤑][Era 10, block 349988] Submit election solution with score [52336759801012136, 2761958931557714578, 196981604141726722206643682275517312] // Event = Some("SolutionStored")
[🤑][Era 9, block 347592] Submit election solution with score [61205498004416460, 2751699917483289990, 214486085224683968911164822075687954] // Event = Some("SolutionStored")
[🤑][Era 8, block 345193] Submit election solution with score [68058389463645380, 2704220325940846387, 224177302021124722096038563629047479] // Event = Some("SolutionStored")
[✅ // [4.78206% ,0.00000% ,-0.12951% ,]][Era 8, block 345196] Submit election solution with score [71312981556466362, 2704220325940846387, 223886963004528602164282195293612161] // Event = Some("SolutionStored")

I like to reason about the panic in two cases:

1. In bock construction, it would invalidate the extrinsic, so the author will NOT include it in the block. So in the sane setting, it should never reach the second case, block import:
2. If for some reason an author decided to ignore the above panic and propagate this anyhow, then this will panic and invalidate the whole block.

Review ✅

The code needs not any review. Please read the above and approve only if all of it makes sense.

[bkchr]

Let's assume the following:

1. I create a transaction with a solution.
2. I import block X that actually contains a much better solution.
3. I start building a block, without having the time to revalidate my tx from 1.

Is that going to happen often?

[kianenigma]

I create a transaction with a solution.
I import block X that actually contains a much better solution.
I start building a block, without having the time to revalidate my tx from 1.

If you are building one a block Y which is older than X, and X happens to be cannon, then your block is eventually retracted anyhow because you are building on top of an old block, as far as I know.

But I am not sure proficient in these topics. Maybe @NikVolf or @tomusdrw can verify as well.

[bkchr]

I create a transaction with a solution.
I import block X that actually contains a much better solution.
I start building a block, without having the time to revalidate my tx from 1.

If you are building one a block Y which is older than X, and X happens to be cannon, then your block is eventually retracted anyhow because you are building on top of an old block, as far as I know.

But I am not sure proficient in these topics. Maybe @NikVolf or @tomusdrw can verify as well.

Not sure how this is related.

I'm speaking about X being canon and you produce the next canon block.

[kianenigma]

I create a transaction with a solution.
I import block X that actually contains a much better solution.
I start building a block, without having the time to revalidate my tx from 1.

If you are building one a block Y which is older than X, and X happens to be cannon, then your block is eventually retracted anyhow because you are building on top of an old block, as far as I know.
But I am not sure proficient in these topics. Maybe @NikVolf or @tomusdrw can verify as well.

Not sure how this is related.

I'm speaking about X being canon and you produce the next canon block.

How I would analyse this:

I see only two states in the world:

the X that you imported is cannon or not

if it is not, then who cares.
if it is, then there are two cases:
2.1 either you are building on top of this cannon one, in which case you see the state of X, and you know your solution sucks (checked in pre_dispatch) so you are good.
2.2 or building on some old block Y and then you received X, in which case anything you build is basically garbage because now you have a new cannon X. I assume the authoring logic should handle this.

[tomusdrw]

The pre_dispatch fix makes sure that the behaviour that @bkch describes won't happen.

If our local solution is shitty, pre_dispatch will reject it and the transaction will be marked as invalid, removed from the pool and not included in the block. Previously it would be in block, but would reach the error here. So now that we've fixed pre_dispatch we can safely panic here, since error here would mean that pre_dispatch does not cover some stuff.

[kianenigma]

bot merge

[ghost]

Waiting for commit status...

[ghost]

Checks failed; merge cancelled

[kianenigma]

bot merge

[ghost]

Trying merge.