Bug: Release vote stake not working (for very specific scenario)

[mochet]

Two users have reported this issue so far.
This bug only seems to impact users who match a very specific set of criteria. Please see 1st comment on issue for examples showing this to be the case.

• Both users voted during cycle id: 0
• Both users revealed votes for unsuccessful candidates from cycle id: 0
• Both users attempted to release the stake during cycle id: 1
• Both users were trying to release stake for votes for candidates who were unsuccessful in cycle id: 0 but were successfully elected in cycle id: 1
• Both user's votes still exist within chain state > referendum.votes
• Both user's vote accounts show balances.locks with only locks for id: voting as existing
• Both users attempted to use extrinsic > referendum.releaseVoteStake and got the error referendum.UnstakingForbidden (details say: Unstaking has been forbidden for the user (at least for now)
• Doing the extrinsic via Pioneer/Polkadot.js makes no difference, results in the same failure.
• This goes against what is described in the handbook.

Addresses/links:

• 5CUefaByXT5B2uxaeuAPiDrGDMVEHd8Ds52bz9XAi2xXp4y1
  • voted for 2182 - ACTIVE CM in cycle ID: 1
  • Error: https://polkadot.js.org/apps/#/explorer/query/0x43f463c83e48bcdfac0ae900156e76f26052f0c517b5e7624d1f7fcfe34fd8b3
• 5D34bqj2H1vAgHaABts6Qn4nqPCKXKKfEthA2KPQtNzDaA5j
  • voted for 644 - ACTIVE CM in cycle ID: 1
  • Error: https://polkadot.js.org/apps/#/explorer/query/0xed74cfa3276e63fed5c941d1d3491a729bafb86e9cd8ea07a79f5413e6319f4c
• 5GBpm9ghjsgRPjvbzjjnb3pCJarMUWLsXHnjNynKd71mRQb8
  • voted for 306 - ACTIVE CM in cycle ID: 1
  • Error: I don't have a link for this one

[mochet]

Further investigation:

• referendum.votes contains 50 votes
• Query Node shows 28 votes were released successfully

This shows an example of an example of a successful release similar to the problem scenarios. They voted in the first election for someone who was successful in the second election.

            "id": "00000B",
            "castBy": "5HT8Mty5U5hFKVwv24uFswX2e41vurU3F5rkLMoETR8XmjFy",
            "deletedAt": null,
            "electionRoundId": "000000",
            "stake": "30200000",
            "stakeLocked": false,
            "voteFor": {
              "memberId": "2182"
            },
            "voteForId": "000002"

The stake for this was released at the following block: https://polkadot.js.org/apps/#/explorer/query/59578

        "inExtrinsic": "0xe648bbb527c15b9942c0f56c67c323eeee1e14bdbe229991d58f59a1cc45908f",
        "inBlock": 59578,
        "id": "OLYMPIA-59578-2",
        "stakingAccount": "5HT8Mty5U5hFKVwv24uFswX2e41vurU3F5rkLMoETR8XmjFy"

This means the stake for this vote was successfully released prior to the next election starting:

{
        "id": "000002",
        "createdAt": "2022-03-26T14:04:42.000Z",
        "stage": {
          "__typename": "CouncilStageIdle"
        },
        "changedAt": "43200",
        "electedCouncilId": "000001",
        "electionProblem": null
      },
      {
        "id": "000003",
        "createdAt": "2022-03-30T17:02:18.000Z",
        "stage": {
          "__typename": "CouncilStageAnnouncing"
        },
        "changedAt": "100800",
        "electedCouncilId": "000001",
        "electionProblem": null
      },

I was able to find other successful stake releases for votes from council 0 that were revealed recently (both revealed and unrevealed) so this bug seems to be restricted to only the examples explained in the description.

[mochet]

I have asked the affected users to attempt releasing now (as there is a new election in progress) as well as throughout the next few days to understand if this stake can ever be released or was just impacted by the candidates being on the current council.

Will update when I get more information.

One of the users was able to release their stake successfully:

• https://polkadot.js.org/apps/#/explorer/query/0xcde64271c903405f375a0848ed532bb011a5e15cb54718036f7157a7212204d3

[mochet]

All 3 accounts appear to have been able to release stake successfully, albeit they weren't able to do so at the same stage as other users and were delayed in doing so. This means this "bug" is likely confined to only a certain subset of users and the "worst result" is that they have some delay in releasing their stake.

@bedeho is this considered something that requires a fix?

[bedeho]

Can you add more explicitly what the handbook says should happen that is violated?

[mochet]

Can you add more explicitly what the handbook says should happen that is violated?

From: https://joystream.gitbook.io/testnet-workspace/system/council#vote

• If the vote is for an ongoing election, then it is not recoverable. - These votes were not for ongoing elections.
• If the vote is for the last concluded election, then it is recoverable only if it was unsealed in favor of a losing candidate, otherwise it is not. - These votes for unsealed in favor of losing candidates.
  From: https://joystream.gitbook.io/testnet-workspace/system/council#recover-voting-stake
• Vote is either for a prior election cycle - These votes were for a prior election cycle.
• the current election cycle, and is Unsealed for a candidate which did not make it into the council - These votes met these conditions

[ondratra]

@mochet thank you for a very nice inspection of the issue.

The only place where UnstakingForbidden error can be thrown is here https://github.com/Joystream/joystream/blob/master/runtime-modules/referendum/src/lib.rs#L994 , and for that, council's can_unlock_vote_stake(https://github.com/Joystream/joystream/blob/master/runtime-modules/council/src/lib.rs#L1184) function must return an error. The function seems to be implemented properly and corresponds to the description in the handbook. Although, it would be great if another pair of eyes could check that.

It seems to me that the problem may be an invalid AnnouncementPeriodNr value in the block when a user tries to unstake.

Some relevant information follows. Let's focus on user 5CUefaByXT5B2uxaeuAPiDrGDMVEHd8Ds52bz9XAi2xXp4y1.

By querying QN (https://query.joystream.org/graphql), we can see new councils were elected at blocks 43200, 144000, and 244800.

query:
electedCouncils {id, isResigned, electedAtBlock, endedAtBlock}

result:

"electedCouncils": [
  {
    "id": "00000000",
    "isResigned": true,
    "electedAtBlock": 0,
    "endedAtBlock": 43200
  },
  {
    "id": "00000001",
    "isResigned": true,
    "electedAtBlock": 43200,
    "endedAtBlock": 144000
  },
  {
    "id": "00000002",
    "isResigned": true,
    "electedAtBlock": 144000,
    "endedAtBlock": 244800
  },
  ...
]

By looking directly into Indexer's database I've found that the user voted in the block 26145, and revealed it in 28974. (I can't include the indexer db here, because it has 3GB). Btw: the user only did some token transfers and then cast vote, revealed vote, and tried to unstake; nothing else.
https://polkadot.js.org/apps/#/explorer/query/26145
https://polkadot.js.org/apps/#/explorer/query/28974

The failed unstake happened in the block 176343, that means two elections after the vote was cast.
https://polkadot.js.org/apps/#/explorer/query/0x43f463c83e48bcdfac0ae900156e76f26052f0c517b5e7624d1f7fcfe34fd8b3

In the block 176343, the storage value AnnouncementPeriodNr is 1, which is IMHO invalid, and I would expect it to be 3 at this point.
https://polkadot.js.org/apps/#/chainstate set council -> announcementPeriodNr and block hash 0x43f463c83e48bcdfac0ae900156e76f26052f0c517b5e7624d1f7fcfe34fd8b3.

We have unity test(s) that cover the whole election process, including vote unstaking; see https://github.com/Joystream/joystream/blob/master/runtime-modules/council/src/tests.rs#L167 .

I haven't found the root of the problem yet, but it is likely that the increment of AnnouncementPeriodNr is somehow skipped.

[mnaamani]

I've finally had the time to go through this issue. There are some issues with council::can_unlock_vote_stake() implementation.

One small fix that would have allowed the votes to be unlocked during cycle 0 while council was in idle stage after the conclusion of the election would be simply replacing:

joystream/runtime-modules/council/src/lib.rs

Line 1217 in 9753012

if voting_for_winner || !matches!(Stage::<T>::get().stage, CouncilStage::Idle) {

with

if voting_for_winner && matches!(Stage::<T>::get().stage, CouncilStage::Idle) {

However if user had waited for next announcing period to start, they would still not have been able to unlock the vote because:

joystream/runtime-modules/council/src/lib.rs

Line 1204 in 9753012

// allow release for vote from previous elections only when not voted for winner

And as had to eventually occur, the user had to wait until an additional cycle completed for the condition to be met:

joystream/runtime-modules/council/src/lib.rs

Line 1188 in 9753012

if current_voting_cycle_id > vote.cycle_id + 1 {

There is bad assumption in this can_unlock_vote_stake() about what a "previous concluded election" means. It is not simply one minus the current AnnouncementPeriodNr since you could have multiple failed announcement and/or election stages, and the cycle_id is incremented every time the announcing stage is re-entered.

One side affect of the current implementation is that any vote stake can be unlocked after 2 failed announcement stages while the same council is still elected. If my understanding is correct as long as an elected member is in the council the vote stake that contributed to electing them must not be unlocked. can you confirm this @bedeho ?

Currently the council pallet does not store in which cycle the council was elected in. Adding this information would allow us to properly address the original bug described and the new one.

[mnaamani]

After discussing with bedeo and removing some ambiguity about the way I interpreted the rules in the handbook, the final implementation to solve the bug, which is easier to read:

    // Check that it is a proper time to release stake.
    fn can_unlock_vote_stake(vote: &CastVoteOf<T>) -> Result<(), Error<T>> {
        let current_voting_cycle_id = AnnouncementPeriodNr::get();

        // If the vote is for an election prior to the last concluded..
        if vote.cycle_id != current_voting_cycle_id {
            // ..it is always recoverable.
            return Ok(());
        }

        // The vote is for the current election cycle.

        if Stage::<T>::get().stage == CouncilStage::Idle {
            // The election is concluded..
            let voted_for_winner = CouncilMembers::<T>::get()
                .iter()
                .map(|council_member| council_member.membership_id)
                .any(|membership_id| vote.vote_for == Some(membership_id));

            if voted_for_winner {
                // ..and vote is for a winning candidate, so it is not recoverable.
                Err(Error::CantReleaseStakeNow)
            } else {
                // ..and vote is for a losing candidate, so it is recoverable.
                Ok(())
            }
        } else {
            // The election is ongoing, so it is not recoverable.
            Err(Error::CantReleaseStakeNow)
        }
    }

Implemented in #3783

I've created a new branch rhodes-update-2 (based off rhodes-update-1 which is the expected next runtime update version) where I will combine this fix and potentially fixes for issues #3530 #3573 and Joystream/pioneer#2312 if it turns out to be a runtime bug.