search

polkadot-js / api

Promise and RxJS APIs around Polkadot and Substrate based chains via RPC calls. It is dynamically generated based on what the Substrate runtime provides in terms of metadata.
Apache License 2.0
1.07k stars 354 forks source link

majority support calculation incorrect #3156

Closed moonrocket16 closed 3 years ago

moonrocket16 commented 3 years ago

Hi, I have been looking at the latest Kulupu vote and it appears that the majority support calculation is incorrect compared to the polkadot wiki calculation. On top of this the vote is going from approve to reject (green to red) without any votes changing. please see attached my calculations vs the polka wiki and the video of the vote decision changing with no votes changing.

IMG_0297 IMG_0299

moonrocket16 commented 3 years ago

video

https://user-images.githubusercontent.com/78681761/107142952-4f2fd100-6986-11eb-83e5-5c6a48b4ac5c.MOV

jacogr commented 3 years ago

The green to red is correct- since the totalIssuance is part of the calculation, when that changes the outcome can change without any votes changing.

All of votes, convictions and issuance are inputs.

moonrocket16 commented 3 years ago

please see my calculations jaco, they are very big difference to being close. it shouldnt be changing from green to red

moonrocket16 commented 3 years ago

I also have consensus with Hacklick on Kulupu discord.

jacogr commented 3 years ago

This is the actual implementation on the Rust side, i.e. it is the source of truth as to how it is handled on the node - https://github.com/paritytech/substrate/blob/master/frame/democracy/src/vote_threshold.rs#L78-L88

Which is - 

    fn approved(&self, tally: Tally<Balance>, electorate: Balance) -> bool {
        let sqrt_voters = tally.turnout.integer_sqrt();
        let sqrt_electorate = electorate.integer_sqrt();
        if sqrt_voters.is_zero() { return false; }
        match *self {
            VoteThreshold::SuperMajorityApprove =>
                compare_rationals(tally.nays, sqrt_voters, tally.ayes, sqrt_electorate),
            VoteThreshold::SuperMajorityAgainst =>
                compare_rationals(tally.nays, sqrt_electorate, tally.ayes, sqrt_voters),
            VoteThreshold::SimpleMajority => tally.ayes > tally.nays,
        }
    }

So for super-majority (which most community referendums are), it is

compare_rationals(tally.nays, sqrt_voters, tally.ayes, sqrt_electorate)

The tally comes from the actual referendum state itself. The JS side for this is at https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/democracy/util.ts#L68

Which is the following

compareRationals(votedNay, sqrtVoters, votedAye, sqrtElectorate)

The compare-rationals function is the same between both sides, see

https://github.com/paritytech/substrate/blob/master/frame/democracy/src/vote_threshold.rs#L46-L71 vs https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/democracy/util.ts#L35-L60

On both the JS & Node side the calculations are only done on integer numbers, hence the rational compare. Same with the sqrt of the numbers, these don't actually have the decimals attached. (This does have an impact when compared to non-integer calculations. There are no floating-point numbers used in the runtime.)

moonrocket16 commented 3 years ago

This is the actual implementation on the Rust side, i.e. it is the source of truth as to how it is handled on the node - https://github.com/paritytech/substrate/blob/master/frame/democracy/src/vote_threshold.rs#L78-L88

Which is -

  fn approved(&self, tally: Tally<Balance>, electorate: Balance) -> bool {
      let sqrt_voters = tally.turnout.integer_sqrt();
      let sqrt_electorate = electorate.integer_sqrt();
      if sqrt_voters.is_zero() { return false; }
      match *self {
          VoteThreshold::SuperMajorityApprove =>
              compare_rationals(tally.nays, sqrt_voters, tally.ayes, sqrt_electorate),
          VoteThreshold::SuperMajorityAgainst =>
              compare_rationals(tally.nays, sqrt_electorate, tally.ayes, sqrt_voters),
          VoteThreshold::SimpleMajority => tally.ayes > tally.nays,
      }
  }

So for super-majority (which most community referendums are), it is

compare_rationals(tally.nays, sqrt_voters, tally.ayes, sqrt_electorate)

The tally comes from the actual referendum state itself. The JS side for this is at https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/democracy/util.ts#L68

Which is the following

compareRationals(votedNay, sqrtVoters, votedAye, sqrtElectorate)

The compare-rationals function is the same between both sides, see

https://github.com/paritytech/substrate/blob/master/frame/democracy/src/vote_threshold.rs#L46-L71 vs https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/democracy/util.ts#L35-L60

On both the JS & Node side the calculations are only done on integer numbers, hence the rational compare. Same with the sqrt of the numbers, these don't actually have the decimals attached. (This does have an impact when compared to non-integer calculations. There are no floating-point numbers used in the runtime.)

Hi Jaco, Either the Polka Wiki is wrong with its calculation or the code is. The example and my figures matched with calculation on the Polka Wiki, the error seems to be with how the formulae is written into the code or the Polka Wiki calculation is wrong and needs to be fixed.

moonrocket16 commented 3 years ago

Either way the Kulupu vote needs to be passing right now which it is not or the Polka Wiki is wrong and needs to be fixed.

jacogr commented 3 years ago

The implementation does match the wiki - the differences are that it is only working with integers. So for instance assume that the turnout is 15360, the sqrt would be 123 when working with integers, but 123.935 when working with non-integers.

e.g. in the Rust node code, this is the following let sqrt_voters = tally.turnout.integer_sqrt(); (non-floating point, hence the integer-sqrt, which is what the JS side also follows, i.e. it also only has access to integers, no floats)

moonrocket16 commented 3 years ago

right now against / SQRT turnout < approve / SQRT electorate 2.0911m / SQRT 9.663 < 7.5719 ? SQRT 36.1287

= 672.69595 < 1259.734

should be passing but is not.

jacogr commented 3 years ago

So atm, with the exact numbers from the state -

For the roots

For the various sides -

the difference -

So left > right. Your turnout numbers are wrong, see the actual state, the turnout is part of the referendum info. (Turnout doesn't take the conviction into account, only the raw funds that have participated) -

image

jacogr commented 3 years ago

With inexact numbers, the same outcome is derived -

Outcome with inexact: 1.2855716777 > 1.25973356851

Overall, it is indeed failing even when you just use the numbers as pulled from the UI (formatted for human-readability) without getting the exact values from the state. (Turnout as per the screenshot shared above, as used)

moonrocket16 commented 3 years ago

Ok thanks I see my error

polkadot-js-bot commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.