Allow Voting While Delegating

[PolkadotDom]

Rendered

WIP (STF + Unit Tests Complete)

Draft PR

[bkchr]

Thank you! Generally a highly requested feature and I think the general idea looks good. I wrote up some edge cases, for which I already have some ideas how to solve them (probably), but would be really good to have them included.

[bkchr]

I would like to have the following edge cases covered here in the document:

• The delegate votes, the delegator votes as well, the delegate removes their vote, the delegator removes their delegation, the delegate votes again
• Generally the case of delegator voting on their own, delegate removing their vote and adding it back
• Delegator voting first and then the delegate

There are probably some other edge cases we need to think of.

[PolkadotDom]

Thank you for the comment. I'll address here and also update the document.

In general these edge cases are handled by the persistence of the poll_index -> retracted_votes state even when the delegate doesn't have a recorded vote for that poll_index. To showcase, let's take a look at the second edge case: The delegator votes -> their vote is added, the retracted vote count is added to their delegate, tally is updated accordingly. Next, the delegate removes their vote -> The vote is removed, tally is updated in accordance with retracted votes, and crucially retracted votes data persists because it is not yet 0. Finally, the delegate adds their vote back -> The new vote can take into account the retracted votes as it has persisted.

The other two cases are similar and I leave up to the reader.

An important note: Because poll_index -> retracted_votes data can persist when there is no vote, it's important that vote removal by delegators also cleans up the delegate's voting state (when retracted votes is 0 and vote is None), otherwise the delegate has no immediate monetary incentive to clean their voting state as no lock is incurred by that data item. Will add this to document as well.

Both of these are handled in the current WIP. The way I've chosen to handle poll_index -> retracted_votes persistence is by bundling that data with the pre-existing vote data tuple and making the actual vote an Option. old vs new. Then the logic just checks that the vote is None and the retracted_votes is 0 before fully removing a vote.

Edit: RFC is updated

Edit 2: Unit tests are up. The cases pointed out by Basti above can be found here

[josepot]

The Voting Enum is first collapsed, as there's no longer a distinction between the variants.

Then how do we know who someone is delegating towards? Wouldn't a new storage entry be needed for keeping track of the delegations?

Then a (poll index -> retracted votes count) data item would be added to the user's voting data stored in VotingFor. This would keep track of the per poll balance that has been clawed back from the user by those delegating to them.

I think that it doesn't make sense to have this inside the VotingFor data-structure. I think this belongs into the new storage that previously mentioned (the one that defines the delegations). Otherwise, if I'm delegating to Bob and then I cast my vote to ref X before Bob casts their vote to that same ref, then my vote will be computed twice b/c the storage entry for Bob's vote on that ref didn't exist when I voted.

[josepot]

Oh, wait, I _think _ that I get it now!

I think that what you are saying is that instead of having an Enum, there would be a struct, where one fields would be votes, and the other field would be delegation, and the type of the delegation would be an Enum more or less like this?

enum Delegation<Balance, AccountId, ReferendumIndex, MaxVotes: Get<u32>> {
  Delegating(Option<(AccountId, Balance, Conviction)>),
  Receiving {
    delegations: Balance,
    clawback: BoundedVec<(ReferendumIndex, Balance), MaxVotes>
  },
}

and then the votes would probably be something more or less like this?

struct Votes {
  votes: BoundedVec<(ReferendumIndex, AccountVote<Balance>), MaxVotes>,
  prior: PriorLock<BlockNumber, Balance>,
}

Ok, in that case, that makes sense... Also, I think that with this change then we could consolidate the prior, so that it's only a property on the Votes.

If what you are proposing is something similar to this, then would you mind explaining this a bit better? 🙏

[PolkadotDom]

Yes looking through it again I do believe it was missing some needed clarity. Thank you for the feedback on this. I've updated the RFC, let me know if it still follows poorly.

And yes! Something like that, where the underlying fields from the enums are consolidated into one struct. Though, in the end, mine looked like this:

pub struct Voting<Balance, AccountId, BlockNumber, PollIndex, MaxVotes>
where
	MaxVotes: Get<u32>,
{
	/// The current voting data of the account.
	pub votes: BoundedVec<VoteRecord<PollIndex, Balance>, MaxVotes>,
	/// The amount of balance delegated to some voting power.
	pub delegated_balance: Balance,
	/// A possible account to which the voting power is delegating.
	pub maybe_delegate: Option<AccountId>,
	/// The possible conviction with which the voting power is delegating. When this gets
	/// undelegated, the relevant lock begins.
	pub maybe_conviction: Option<Conviction>,
	/// The total amount of delegations that this account has received, post-conviction-weighting.
	pub delegations: Delegations<Balance>,
	/// Any pre-existing locks from past voting/delegating activity.
	pub prior: PriorLock<BlockNumber, Balance>,
}

With the retracted votes held in the vote record:

pub struct VoteRecord<PollIndex, Balance> {
	/// The poll index this information concerns
	pub poll_index: PollIndex,
	/// The vote this account has cast. Can be none if only retracted_votes info is needed
	pub maybe_vote: Option<AccountVote<Balance>>,
	/// The amount of votes retracted from this user for this poll. Can't be more than is
	/// delegated to them. Votes are retracted when a delegator votes in stead of their delegate.
	pub retracted_votes: RetractedVotes<Balance>,
}

I would think the final implementation would be more of a PR conversation though.

[josepot]

The implementation must allow for the (poll index -> retracted votes) data to exist even if the user does not currently have a vote for that poll.

This is IMO too vague. How would that be accomplished without a new storage on this pallet?

[PolkadotDom]

I believe answered by the conversation above, but it would be held in the new voting data struct. So it would slot in with the pre-existing VotingFor storage item.

[josepot]

I think that the RFC is missing a detailed explanation on how the migration from the old to the new system would take place.

[PolkadotDom]

Fair 👍 I've updated this section. The migration itself is fairly simple, so still not a lot to say about it, but let me know if the new version is not detailed/clear enough.

[PolkadotDom]

As there have been no further comments, I will now begin the proposal process.

/rfc propose

[paritytech-rfc-bot]

Hey @PolkadotDom, here is a link you can use to create the referendum aiming to approve this RFC number 0150.

Instructions

1. Open the link.

2. Switch to the Submission tab.

3. Adjust the transaction if needed (for example, the proposal Origin).

4. Submit the Transaction

---

It is based on commit hash 544d610a8a976f4d81d23ffb5a80ca9955c440f4.

The proposed remark text is: RFC_APPROVE(0150,c6ea7e8972e0cbf75592c57206daf133c34ec6b1151970930886239692fe34d4).

[bkchr]

@PolkadotDom I know that you pinged me, but maybe I have missed any pings in the polkadot fellowship channel?

[PolkadotDom]

@bkchr I had talked about it/called for reviews during the last OpenDev fellowship call. I wrote about it in the fellowship channel a while back, but that would have been a couple months ago now.

Full transparency-- I'm still learning the best ways to navigate code changes as a non-parity fellow. If you feel I did something out of turn here, just let me know and I'll fix it moving forward.

[ggwpez]

Looks good, thanks for the details.

[bkchr]

Still reading better, but it would be really nice to get a proper explanation of the algorithm and not just some "guessing" on how it will work.

[bkchr]

However, this will incentivize delegates to stay on top of voting data clearance

In which way is the delegate incentivized? It could prevent someone from voting for a proposal this way.

[bkchr]

It is more that teh delegator would be incentivized to call removeOtherVote.

[PolkadotDom]

It could prevent someone from voting for a proposal this way.

Luckily the same property that allows for voting while delegating also allows for undelegating / delegating while having a voting record. So if this attack was attempted, the user could just undelegate. Presumably after a removeOtherVote attempt. Really the only person who can lose in this situation is the delegate.

With that in mind, I believe there is sufficient political power gained from being a delegate that the delegate will attempt to retain as many delegators as possible (by keeping their voting record clean).

[bkchr]

Could you please rephrase this entire section. It should explain how the algorithm is working and not how the data structures are looking. It is right now possible to come up with this already, but the RFC is not laying out exactly how it works.

It would be nice to have the logic layed out for the different states, aka delegator voting first, delegate voting first, delegator removing votes etc.

[PolkadotDom]

Okay I've updated the explanation section. Let me know what you think 🙌