Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Anyone should be able to activate an account's vote #6467

Closed
nambrot opened this issue Jan 13, 2021 · 18 comments · Fixed by #7517
Closed

Anyone should be able to activate an account's vote #6467

nambrot opened this issue Jan 13, 2021 · 18 comments · Fixed by #7517
Labels
CAP Component: Contracts enhancement an improvement to an existing feature good first issue Good for newcomers Priority: P3 Minor

Comments

@nambrot
Copy link
Contributor

nambrot commented Jan 13, 2021

Expected Behavior

Any account should be able to activate a vote.

Current Behavior

The account needs to activate it's own votes to receive rewards

@nambrot nambrot added Component: Contracts CAP enhancement an improvement to an existing feature good first issue Good for newcomers labels Jan 13, 2021
@nambrot
Copy link
Contributor Author

nambrot commented Jan 26, 2021

This feels like an easy one that could prevent a bunch of voting but non-active CELO which applies to about 3M CELO right now @aslawson

@aslawson
Copy link
Contributor

to clarify this means any account can activate another account's 'pending' votes?
what is the use-case for "anyone"? cLabs or other vote platforms activate pending votes on behalf of accounts (to avoid users forgetting to activate a day later)?

@aslawson
Copy link
Contributor

i guess this would only prevent a bunch on non-active voting CELO is if the "activator" could regularly identify the non-active votes, but I'm not sure there is a way to access all accounts with non-active votes with the current storage structure.

@nambrot
Copy link
Contributor Author

nambrot commented Jan 27, 2021

cLabs or other vote platforms activate pending votes on behalf of accounts (to avoid users forgetting to activate a day later)?

That is correct.

There probably isn't an easy way of doing so with pure contract storage, but with any on-chain indexer it should be pretty easy. I.e. we track this metric with eksportisto

@aslawson aslawson added the Priority: P3 Minor label Jan 27, 2021
@timmoreton timmoreton changed the title Anyone should be able to active an account's vote Anyone should be able to activate an account's vote Feb 12, 2021
@deepakhb2
Copy link
Contributor

@nambrot I am working on this issue.

@deepakhb2
Copy link
Contributor

deepakhb2 commented Mar 20, 2021

Hi all, Please give me a direction on which contract is handling this. I am finding it difficult to understand current logic that is handling the current votes. Thank you in advance.
Is this the right doc to start with https://docs.celo.org/celo-owner-guide/quick-start#the-next-day-activate-your-vote?

@jmrossy
Copy link
Contributor

jmrossy commented Mar 20, 2021

@nambrot @aslawson Could we take this a step further and remove the need for a manual activation tx entirely?
Could the distributeEpochRewards method in Election (or something similar) search for unactive votes and activate them?

Having some kind of service monitoring + activating would definitely be an improvement but it's still another service to pay for and manage.

@deepakhb2 here is the solidity code that controls this: https://github.com/celo-org/celo-monorepo/blob/master/packages/protocol/contracts/governance/Election.sol#L271

@deepakhb2
Copy link
Contributor

Thank you @jmrossy. Current implementation is the current pending vote will selected from group by sender address to activate the vote. This needs to be changed like activate function should accept anther account address parameter to get the pending vote of a account. So the sender can pass other account to activate the pending vote. Is my understanding correct?
Or the other option is to activate all the pending votes in the group.

@nambrot
Copy link
Contributor Author

nambrot commented Mar 22, 2021

@jmrossy The search unfortunately would require brute-force, so without substantially expanding the storage requirements or processing time in the epoch block (which is very sensitive), I would imagine it's gonna be a while until we get automated vote activation.

@deepakhb2 Your understanding is correct, another activate function that allows someone to activate in any account's behalf should be good. I do not believe it is easily possible to activate all pending votes in the group as you need to know the voters addresses.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 49.0 CELO (199.92 USD @ $4.08/CELO) attached to it as part of the celo-org fund.

@gitcoinbot
Copy link

gitcoinbot commented Mar 22, 2021

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 265 years, 8 months from now.
Please review their action plans below:

1) deepakhb2 has been approved to start work.

I have experience working on celo project and I have already looking into this issue. My initial research is complete.

Learn more on the Gitcoin Issue Details page.

@deepakhb2
Copy link
Contributor

@nambrot Thank you. I will work on this change and raise the PR for review.

@deepakhb2
Copy link
Contributor

@nambrot I have made the changes and I haven't raised PR yet I need to add unit tests to cover the change. I checking the tests to find best way to add the test coverage.
In mean time please review the change I have made https://github.com/deepakhb2/celo-monorepo/blob/issue/6467-activate-account/packages/protocol/contracts/governance/Election.sol#L290. Let me know if I am missing anything?
There is a dependency like account address depends on group address like passed account should belong to group.

@nambrot
Copy link
Contributor Author

nambrot commented Mar 22, 2021

Looking good, one thing I might suggest is to rename the function. Even though it does clearly the same thing, I'd personally prefer to semantically distinguish the functions. Curious to get @yorhodes @m-chrzan's take on it. I would also consider extracting the common functionality into a _activate function.

@gitcoinbot
Copy link

@deepakhb2 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • reminder (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@deepakhb2
Copy link
Contributor

@nambrot This issue is complete. Please review the PR when you get a chance. Thank you.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 49.0 CELO (221.97 USD @ $4.53/CELO) has been submitted by:


@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


The funding of 49.0 CELO (220.99 USD @ $4.51/CELO) attached to this issue has been approved & issued to @deepakhb2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CAP Component: Contracts enhancement an improvement to an existing feature good first issue Good for newcomers Priority: P3 Minor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants