From 465e3a3c38275f12b7cc21730101f91d6600ed3b Mon Sep 17 00:00:00 2001 From: Rekard0 <5880388+Rekard0@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:50:00 +0200 Subject: [PATCH] OS-784 Bug: fix delegatee wrong deletion (#488) * fix delegatee wrong deletion * update change log * apply prettier * refactor to allow delegatee to be null * adapt test to handle null delegatee * address comments * add Uninstallation to PluginPreparationType --- packages/subgraph/CHANGELOG.md | 1 + packages/subgraph/schema.graphql | 3 +- .../src/packages/token/governance-erc20.ts | 65 ++++++++++++------ .../subgraph/tests/helpers/method-classes.ts | 15 ++++- .../tests/token/governance-erc20.test.ts | 67 +++++++++++++++++-- packages/subgraph/tests/token/utils.ts | 14 ++++ 6 files changed, 136 insertions(+), 29 deletions(-) diff --git a/packages/subgraph/CHANGELOG.md b/packages/subgraph/CHANGELOG.md index dc587f0a7..3fb805a12 100644 --- a/packages/subgraph/CHANGELOG.md +++ b/packages/subgraph/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Renamed `fetchERC20` & `fetchWrappedERC20` to `fetchOrCreateERC20Entity` & `fetchOrCreateWrappedERC20Entity` respectively. - Changed type of `token` attribute of `ERC20Transfer` from `ERC20Contract` to `Token`. - Refactored `Permission` entity & added `pluginRepo` attribute. +- Fixed wrong token voting member deletion, when balance & voting power was zero, but it was still delegating to another address. ### Removed diff --git a/packages/subgraph/schema.graphql b/packages/subgraph/schema.graphql index dda53c7b3..75b5cb271 100644 --- a/packages/subgraph/schema.graphql +++ b/packages/subgraph/schema.graphql @@ -305,6 +305,7 @@ type PluginInstallation @entity { enum PluginPreparationType { Installation Update + Uninstallation } enum PluginPreparationState { @@ -405,7 +406,7 @@ type TokenVotingMember @entity { plugin: TokenVotingPlugin! # delegates - delegatee: TokenVotingMember! + delegatee: TokenVotingMember votingPower: BigInt # we assume token owners and/or delegatees are members delegators: [TokenVotingMember!]! @derivedFrom(field: "delegatee") diff --git a/packages/subgraph/src/packages/token/governance-erc20.ts b/packages/subgraph/src/packages/token/governance-erc20.ts index 448cb6088..48fa707e5 100644 --- a/packages/subgraph/src/packages/token/governance-erc20.ts +++ b/packages/subgraph/src/packages/token/governance-erc20.ts @@ -1,4 +1,5 @@ import {TokenVotingMember} from '../../../generated/schema'; +import {GovernanceERC20 as GovernanceERC20Contract} from '../../../generated/templates/GovernanceERC20/GovernanceERC20'; import { DelegateChanged, DelegateVotesChanged, @@ -7,7 +8,7 @@ import {Transfer} from '../../../generated/templates/TokenVoting/ERC20'; import {Address, BigInt, dataSource, store} from '@graphprotocol/graph-ts'; function getOrCreateMember(user: Address, pluginId: string): TokenVotingMember { - let id = user.toHexString().concat('_').concat(pluginId); + let id = [user.toHexString(), pluginId].join('_'); let member = TokenVotingMember.load(id); if (!member) { member = new TokenVotingMember(id); @@ -15,7 +16,7 @@ function getOrCreateMember(user: Address, pluginId: string): TokenVotingMember { member.balance = BigInt.zero(); member.plugin = pluginId; - member.delegatee = id; // we assume by default member delegates itself + member.delegatee = null; member.votingPower = BigInt.zero(); } @@ -42,14 +43,15 @@ export function handleTransfer(event: Transfer): void { export function handleDelegateChanged(event: DelegateChanged): void { let context = dataSource.context(); let pluginId = context.getString('pluginId'); + const toDelegate = event.params.toDelegate; // make sure `fromDelegate` & `toDelegate`are members if (event.params.fromDelegate != Address.zero()) { let fromMember = getOrCreateMember(event.params.fromDelegate, pluginId); fromMember.save(); } - if (event.params.toDelegate != Address.zero()) { - let toMember = getOrCreateMember(event.params.toDelegate, pluginId); + if (toDelegate != Address.zero()) { + let toMember = getOrCreateMember(toDelegate, pluginId); toMember.save(); } @@ -58,32 +60,53 @@ export function handleDelegateChanged(event: DelegateChanged): void { let delegator = getOrCreateMember(event.params.delegator, pluginId); // set delegatee - let delegatee = event.params.toDelegate - .toHexString() - .concat('_') - .concat(pluginId); + let delegatee: string | null = null; + if (toDelegate != Address.zero()) { + delegatee = [toDelegate.toHexString(), pluginId].join('_'); - delegator.delegatee = delegatee; + delegator.delegatee = delegatee; + } delegator.save(); } } export function handleDelegateVotesChanged(event: DelegateVotesChanged): void { - let context = dataSource.context(); - let pluginId = context.getString('pluginId'); + const delegate = event.params.delegate; + if (delegate == Address.zero()) return; + const newVotingPower = event.params.newBalance; - if (event.params.delegate != Address.zero()) { - let member = getOrCreateMember(event.params.delegate, pluginId); - if ( - member.balance.equals(BigInt.zero()) && - event.params.newBalance.equals(BigInt.zero()) - ) { + const context = dataSource.context(); + const pluginId = context.getString('pluginId'); + let member = getOrCreateMember(delegate, pluginId); + + if (isZeroBalanceAndVotingPower(member.balance, newVotingPower)) { + if (shouldRemoveMember(event.address, delegate)) { store.remove('TokenVotingMember', member.id); - } else { - // Assign the cumulative delegated votes to this member from all their delegators. - member.votingPower = event.params.newBalance; - member.save(); + return; } } + member.votingPower = newVotingPower; + member.save(); +} + +function isZeroBalanceAndVotingPower( + memberBalance: BigInt, + votingPower: BigInt +): boolean { + return ( + memberBalance.equals(BigInt.zero()) && votingPower.equals(BigInt.zero()) + ); +} + +function shouldRemoveMember( + contractAddress: Address, + delegate: Address +): boolean { + const governanceERC20Contract = GovernanceERC20Contract.bind(contractAddress); + const delegates = governanceERC20Contract.try_delegates(delegate); + if (!delegates.reverted) { + return delegates.value == delegate || delegates.value == Address.zero(); + } + return false; } diff --git a/packages/subgraph/tests/helpers/method-classes.ts b/packages/subgraph/tests/helpers/method-classes.ts index cf22d18d9..cfff39a31 100644 --- a/packages/subgraph/tests/helpers/method-classes.ts +++ b/packages/subgraph/tests/helpers/method-classes.ts @@ -99,6 +99,7 @@ import { createNewProposalExecutedEvent, createNewVoteCastEvent, createNewVotingSettingsUpdatedEvent, + delegatesCall, getProposalCountCall, } from '../token/utils'; import { @@ -783,16 +784,24 @@ class TokenVotingMemberMethods extends TokenVotingMember { this.address = Address.fromHexString(memberAddress); this.balance = BigInt.zero(); this.plugin = plugin.toHexString(); - this.delegatee = id; + this.delegatee = null; this.votingPower = BigInt.zero(); return this; } + mockCall_delegatesCall( + tokenContractAddress: string, + account: string, + returns: string + ): void { + delegatesCall(tokenContractAddress, account, returns); + } + createEvent_DelegateChanged( delegator: string = this.address.toHexString(), - fromDelegate: string = ADDRESS_ONE, - toDelegate: string = ADDRESS_ONE, + fromDelegate: string = this.address.toHexString(), + toDelegate: string = this.address.toHexString(), tokenContract: string = Address.fromHexString( DAO_TOKEN_ADDRESS ).toHexString() diff --git a/packages/subgraph/tests/token/governance-erc20.test.ts b/packages/subgraph/tests/token/governance-erc20.test.ts index eb0535dbf..9f68a3b3b 100644 --- a/packages/subgraph/tests/token/governance-erc20.test.ts +++ b/packages/subgraph/tests/token/governance-erc20.test.ts @@ -9,6 +9,7 @@ import { ADDRESS_TWO, ONE_ETH, ADDRESS_THREE, + DAO_TOKEN_ADDRESS, } from '../constants'; import {ExtendedTokenVotingMember} from '../helpers/extended-schema'; import {createNewERC20TransferEvent, createTokenVotingMember} from './utils'; @@ -156,6 +157,9 @@ describe('Governance ERC20', () => { handleDelegateChanged(event); + // expected changes + member.delegatee = [memberAddress, pluginAddress].join('_'); + member.assertEntity(); assert.entityCount('TokenVotingMember', 1); }); @@ -275,7 +279,7 @@ describe('Governance ERC20', () => { assert.entityCount('TokenVotingMember', 1); }); - test('it should delete a member without voting power or balance', () => { + test('it should delete a member without voting power and balance and not delegating to another address', () => { let memberOneAddress = ADDRESS_ONE; let memberTwoAddress = ADDRESS_TWO; let pluginAddress = ADDRESS_SIX; @@ -287,7 +291,7 @@ describe('Governance ERC20', () => { memberTwoAddress, pluginAddress ); - /* member one has 100s token delegated to member two*/ + /* member one has 100 token delegated to member two*/ memberOne.balance = BigInt.fromString('100'); memberOne.votingPower = BigInt.fromString('0'); /* member two balance is 0 but has 100 voting power from the delegation of member one */ @@ -300,10 +304,16 @@ describe('Governance ERC20', () => { assert.entityCount('TokenVotingMember', 2); - // member one undelegates from member two + // member one un-delegates from member two let eventOne = memberOne.createEvent_DelegateVotesChanged('100'); let eventTwo = memberTwo.createEvent_DelegateVotesChanged('0'); + memberTwo.mockCall_delegatesCall( + DAO_TOKEN_ADDRESS, + ADDRESS_TWO, + ADDRESS_TWO + ); + handleDelegateVotesChanged(eventOne); handleDelegateVotesChanged(eventTwo); @@ -311,9 +321,58 @@ describe('Governance ERC20', () => { // expected changes memberOne.votingPower = BigInt.fromString('100'); memberOne.assertEntity(); - // member two should be deleted because it has no balance or voting power + // member two should be deleted because it has no (balance and voting power) and not delegates to another address. assert.notInStore('TokenVotingMember', memberTwo.id); assert.entityCount('TokenVotingMember', 1); }); + + test('it should not delete a member without voting power and balance, but delegating to another address', () => { + let memberOneAddress = ADDRESS_ONE; + let memberTwoAddress = ADDRESS_TWO; + let pluginAddress = ADDRESS_SIX; + let memberOne = new ExtendedTokenVotingMember().withDefaultValues( + memberOneAddress, + pluginAddress + ); + let memberTwo = new ExtendedTokenVotingMember().withDefaultValues( + memberTwoAddress, + pluginAddress + ); + /* member one has 100 token delegated to member two*/ + memberOne.balance = BigInt.fromString('100'); + memberOne.votingPower = BigInt.fromString('0'); + /* member two balance is 0 but has 100 voting power from the delegation of member one */ + memberTwo.balance = BigInt.fromString('0'); + memberTwo.votingPower = BigInt.fromString('100'); + /* member three has 100 tokens and none delegated */ + + memberOne.buildOrUpdate(); + memberTwo.buildOrUpdate(); + + assert.entityCount('TokenVotingMember', 2); + + // member one un-delegates from member two + let eventOne = memberOne.createEvent_DelegateVotesChanged('100'); + let eventTwo = memberTwo.createEvent_DelegateVotesChanged('0'); + + memberTwo.mockCall_delegatesCall( + DAO_TOKEN_ADDRESS, + ADDRESS_TWO, + ADDRESS_ONE + ); + + handleDelegateVotesChanged(eventOne); + handleDelegateVotesChanged(eventTwo); + + // assert + // expected changes + memberOne.votingPower = BigInt.fromString('100'); + memberOne.assertEntity(); + + assert.fieldEquals('TokenVotingMember', memberOne.id, 'id', memberOne.id); + // memberTwo should not be deleted because it has no (balance and voting power), but it delegates to another address. + assert.fieldEquals('TokenVotingMember', memberTwo.id, 'id', memberTwo.id); + assert.entityCount('TokenVotingMember', 2); + }); }); }); diff --git a/packages/subgraph/tests/token/utils.ts b/packages/subgraph/tests/token/utils.ts index 32cd69ead..eec075fa6 100644 --- a/packages/subgraph/tests/token/utils.ts +++ b/packages/subgraph/tests/token/utils.ts @@ -289,6 +289,20 @@ export function getProposalCountCall( .returns([ethereum.Value.fromSignedBigInt(BigInt.fromString(returns))]); } +export function delegatesCall( + contractAddress: string, + account: string, + returns: string +): void { + createMockedFunction( + Address.fromString(contractAddress), + 'delegates', + 'delegates(address):(address)' + ) + .withArgs([ethereum.Value.fromAddress(Address.fromString(account))]) + .returns([ethereum.Value.fromAddress(Address.fromString(returns))]); +} + // state export function createTokenVotingProposalEntityState(