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

[DEMO — Do not merge] Use Split-Delegation Strategy from Gnosis Guild #8

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

samepant
Copy link

Description

This PR demonstrates the changes required to leverage the Split-Delegation system that we've built at Gnosis Guild. This system uses a new delegation contract, an indexing service, and an api for making and tracking delegations.

This new delegation system supports:

  • split delegations (i can delegate 50% of my vote weight to someone and 50% to someone else)
  • transitive delegations (i delegate to A, A delegates to B, my delegation flows to B)
  • expiring delegations

All the code for the Split-Delegation system can be viewed here: https://github.com/gnosisguild/split-delegation

We've also made changes to the Snapshot frontend that allow this system to be used natively in the snapshot app, but are awaiting PR review. A demo of these changes can be viewed here: https://snapshot-ochre.vercel.app/#/cow.ggtest.eth/delegates

Changes

In order to use this strategy, existing strategies used to get the voting scores should be moved to the strategies param on the Split-Delegation strategy. This makes them sub-strategies, and the split-delegation system will add up the scores from the sub-strategies and then will apply the vote weight calculations to compute the final voting power of any address.

Copy link

github-actions bot commented May 22, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@samepant
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request May 22, 2024
@cmagan
Copy link
Collaborator

cmagan commented May 23, 2024

Thanks @samepant, this is very useful for our planned CIP launching a delegation campaign!

@mfw78
Copy link
Contributor

mfw78 commented May 24, 2024

Looks neat - but how can the results of the poll be independently verified?

@samepant
Copy link
Author

Looks neat - but how can the results of the poll be independently verified?

Hey @mfw78 ! Curious what you mean by poll? The code is open source and is running through snapshot's system, so the trust assumptions are similar to the current setup.

@samepant
Copy link
Author

samepant commented Jun 4, 2024

Just pushed another commit, after talking with Snapshot people, we've decided it makes sense to add split-delegation to the top level space settings as well as in the strategy settings. This helps Snapshot's frontend decide which delegation UI components to use on its delegation dashboard.

@wa0x6e
Copy link

wa0x6e commented Aug 15, 2024

Hey, I'm wan from the snapshot team.

I just took a look at the settings.json file, and it technically looks good.

  • json file is valid
  • "erc20-balance-of-delegation" strategies has been removed (since now supported by the split-delegation natively now)
  • delegation portal settings are correct

I plugged those same settings into a test space, and it's working. Did not test the delegation side though, as I don't have cow.

The only point I am not sure is the

"totalSupply": 1284949976,

Can you confirm this is your coin total supply ?

@cmagan
Copy link
Collaborator

cmagan commented Sep 3, 2024

@wa0x6e thanks for reviewing!
Can you please share the test space you've deployed?
I also created a test space https://snapshot.org/#/cowtesting.eth
Not sure why it allows me to create proposal from wallets which doesn't have the min 10k COW tokens?
On the main space interface I can't seem to find the delegation dashboard, is there any additional setting for adding it or should users access it from a different link?
image

@cmagan
Copy link
Collaborator

cmagan commented Sep 3, 2024

Hey, I'm wan from the snapshot team.

I just took a look at the settings.json file, and it technically looks good.

  • json file is valid
  • "erc20-balance-of-delegation" strategies has been removed (since now supported by the split-delegation natively now)
  • delegation portal settings are correct

I plugged those same settings into a test space, and it's working. Did not test the delegation side though, as I don't have cow.

The only point I am not sure is the

"totalSupply": 1284949976,

Can you confirm this is your coin total supply ?

The total COW max supply is 1b tokens: https://etherscan.io/token/0xdef1ca1fb7fbcdc777520aa7f396b4e015f497ab
but about 50% is kept in the COW DAO treasury unallocated.
What number should be there?

@wa0x6e
Copy link

wa0x6e commented Sep 3, 2024

@wa0x6e thanks for reviewing! Can you please share the test space you've deployed? I also created a test space https://snapshot.org/#/cowtesting.eth Not sure why it allows me to create proposal from wallets which doesn't have the min 10k COW tokens? On the main space interface I can't seem to find the delegation dashboard, is there any additional setting for adding it or should users access it from a different link? image

To enable the delegation dashboard, you have to go to your space settings, then delegation.

In the form, you have to select "Split delegation", then enter your api and contract address

Then, go to the strategies section, and add the "Split delegation" strategy

@cmagan
Copy link
Collaborator

cmagan commented Sep 3, 2024

The only way for CoW DAO to update its space settings is though updating the settings json file which we're discussing ATM...
Would be lovely if you can help us add this change (delegation dashboard) to the PR 🙏

@wa0x6e
Copy link

wa0x6e commented Sep 3, 2024

Hey, I'm wan from the snapshot team.
I just took a look at the settings.json file, and it technically looks good.

  • json file is valid
  • "erc20-balance-of-delegation" strategies has been removed (since now supported by the split-delegation natively now)
  • delegation portal settings are correct

I plugged those same settings into a test space, and it's working. Did not test the delegation side though, as I don't have cow.
The only point I am not sure is the

"totalSupply": 1284949976,

Can you confirm this is your coin total supply ?

The total COW max supply is 1b tokens: https://etherscan.io/token/0xdef1ca1fb7fbcdc777520aa7f396b4e015f497ab but about 50% is kept in the COW DAO treasury unallocated. What number should be there?

I believe it's 1,000,000,000 (from etherscan), maybe @samepant can confirm. Where did you get that 1284949976 number from ?

"moderators": [
"0x387fe763ddac7a6b568ee344fefc31f626bc837b",
"0xa312c2ae6a06504cbebe89468e6f9dc01c8b23d4",
"0xF44217A8b6b3f258BFFEaD635c226528aa516aea"
],
"delegationPortal": {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmagan delegation settings are here, this should enable the portal on the space

@samepant
Copy link
Author

samepant commented Sep 3, 2024

I believe it's 1,000,000,000 (from etherscan), maybe @samepant can confirm. Where did you get that 1284949976 number from ?
^^ this can be ignored, it was a dummy value based on the previous strategies.

i think it should be the current circulating supply of tokens, not the total supply, since those are the only ones that can be used to vote.

@mfw78
Copy link
Contributor

mfw78 commented Sep 4, 2024

What API is required for this? Concretely what is required to make this work?

@cmagan
Copy link
Collaborator

cmagan commented Sep 12, 2024

What API is required for this? Concretely what is required to make this work?

@mfw78 the API that is required is open source and available here.

The api going down would interrupt vote calculation — but the vote could be recalculated once it's back up.
If it's a concern, the api backend is open source so anyone can run their own version of it

@cmagan cmagan requested a review from mfw78 September 12, 2024 11:02
@mfw78
Copy link
Contributor

mfw78 commented Sep 13, 2024

Where is the settings.json documented for the delegation portal etc? Given that we set the settings.json via IPFS, having a UI guided setup is not appropriate / sufficient. It is much better that we have a source of truth (documentation) that we can draw from with which to verify / validate settings.

@wa0x6e
Copy link

wa0x6e commented Sep 13, 2024

Where is the settings.json documented for the delegation portal etc? Given that we set the settings.json via IPFS, having a UI guided setup is not appropriate / sufficient. It is much better that we have a source of truth (documentation) that we can draw from with which to verify / validate settings.

This delegation type is largely handled by the guys at Gnosis (https://github.com/gnosisguild/split-delegation), snapshot is only using their API to display the delegates, and their contract to handle the delegation.

We don't have an updated documentation yet, but the settings is only asking 2 things: a contract address, and API url, which are both provided by gnosis at the moment.

@mfw78
Copy link
Contributor

mfw78 commented Oct 10, 2024

Concretely, there should be documentation cited for the changes, and a test methodology established (sure, it can be on some test site - but there should be written a solid methodology from which reviewers can establish the basis for the PR).

Additionally, I'm obviously not going to approve something that is "DEMO - Do not merge". If this is to be pushed to prod, can we please have the PR updated with a step-by-step testing methodology and appropriate PR description/title changes 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants