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

@@ -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
 

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")

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,15 +8,15 @@ 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);
     member.address = user;
     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;
 }

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()

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,20 +304,75 @@ 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);
 
       // assert
       // 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);
+    });
   });
 });

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(