Bug/ OS-725 Subgraph token detection (#472)

* fix(subgraph): improve erc721 and erc20 transfer processing * refactor action handler & erc721 & erc1155 * remove redundant commented out part * rename determineDecodeABI & use getMethodSignature --------- Co-authored-by: Mathias Scherer <mathias@aragon.org>
aragon · Oct 6, 2023 · 40a0206 · 40a0206
1 parent aab6b9b
commit 40a0206
Show file tree

Hide file tree

Showing 4 changed files with 252 additions and 161 deletions.
diff --git a/packages/subgraph/src/dao/utils.ts b/packages/subgraph/src/dao/utils.ts
@@ -23,8 +23,9 @@ import {handleERC20Action} from '../utils/tokens/erc20';
 import {handleERC721Action} from '../utils/tokens/erc721';
 import {handleNativeAction} from '../utils/tokens/eth';
 import {handleERC1155Action} from '../utils/tokens/erc1155';
+import {getMethodSignature} from '../utils/bytes';
 
-// AssemblyScript struggles having mutliple return types. Due to this,
+// AssemblyScript struggles having multiple return types. Due to this,
 // The below seems most effective way.
 export function updateProposalWithFailureMap(
   proposalId: string,
@@ -65,26 +66,11 @@ export function handleAction<
   T extends ExecutedActionsStruct,
   R extends Executed
 >(action: T, proposalId: string, index: i32, event: R): void {
-  let actionId = proposalId.concat('_').concat(index.toString());
-  let actionEntity = Action.load(actionId);
-
-  // In case the execute on the dao is called by the address
-  // That we don't currently index for the actions in the subgraph,
-  // we fallback and still create an action.
-  // NOTE that it's important to generate action id differently to not allow overwriting.
-  if (!actionEntity) {
-    actionEntity = new Action(actionId);
-    actionEntity.to = action.to;
-    actionEntity.value = action.value;
-    actionEntity.data = action.data;
-    actionEntity.proposal = proposalId;
-    actionEntity.dao = event.address.toHexString();
-  }
-
+  let actionEntity = getOrCreateActionEntity(action, proposalId, index, event);
   actionEntity.execResult = event.params.execResults[index];
   actionEntity.save();
 
-  if (action.data.toHexString() == '0x' && action.value.gt(BigInt.zero())) {
+  if (isNativeTokenAction(action)) {
     handleNativeAction(
       event.address,
       action.to,
@@ -97,48 +83,94 @@ export function handleAction<
     return;
   }
 
-  let methodSig = action.data.toHexString().slice(0, 10);
-
-  // Since ERC721 transferFrom and ERC20 transferFrom have the same signature,
-  // The below first checks if it's ERC721 by calling `supportsInterface` and then
-  // moves to ERC20 check. Currently, if `action` is transferFrom, it will still check
-  // both `handleERC721Action`, `handleERC20Action`.
-  if (
-    methodSig == ERC721_transferFrom ||
-    methodSig == ERC721_safeTransferFromNoData ||
-    methodSig == ERC721_safeTransferFromWithData
-  ) {
-    handleERC721Action(
+  handleTokenTransfers(action, proposalId, index, event);
+}
+
+function getOrCreateActionEntity<
+  T extends ExecutedActionsStruct,
+  R extends Executed
+>(action: T, proposalId: string, index: i32, event: R): Action {
+  const actionId = [proposalId, index.toString()].join('_');
+  let entity = Action.load(actionId);
+
+  // In case the execute on the dao is called by the address
+  // That we don't currently index for the actions in the subgraph,
+  // we fallback and still create an action.
+  // NOTE that it's important to generate action id differently to not allow
+
+  if (!entity) {
+    entity = new Action(actionId);
+    entity.to = action.to;
+    entity.value = action.value;
+    entity.data = action.data;
+    entity.proposal = proposalId;
+    entity.dao = event.address.toHexString();
+  }
+
+  return entity;
+}
+
+function handleTokenTransfers<
+  T extends ExecutedActionsStruct,
+  R extends Executed
+>(action: T, proposalId: string, actionIndex: i32, event: R): void {
+  const methodSig = getMethodSignature(action.data);
+
+  let handledByErc721: bool = false;
+  let handledByErc1155: bool = false;
+
+  if (isERC721Transfer(methodSig)) {
+    handledByErc721 = handleERC721Action(
       action.to,
       event.address,
       action.data,
       proposalId,
-      index,
+      actionIndex,
       event
     );
   }
-  if (
-    methodSig == ERC1155_safeBatchTransferFrom ||
-    methodSig == ERC1155_safeTransferFrom
-  ) {
-    handleERC1155Action(
+
+  if (isERC1155TransferMethod(methodSig)) {
+    handledByErc1155 = handleERC1155Action(
       action.to,
       event.address,
       action.data,
       proposalId,
-      index,
+      actionIndex,
       event
     );
   }
 
-  if (methodSig == ERC20_transfer || methodSig == ERC20_transferFrom) {
+  if (isERC20Transfer(methodSig) && !handledByErc721 && !handledByErc1155) {
     handleERC20Action(
       action.to,
       event.address,
       proposalId,
       action.data,
-      index,
+      actionIndex,
       event
     );
   }
 }
+
+function isERC721Transfer(methodSig: string): bool {
+  return [
+    ERC721_transferFrom,
+    ERC721_safeTransferFromNoData,
+    ERC721_safeTransferFromWithData
+  ].includes(methodSig);
+}
+
+function isERC20Transfer(methodSig: string): bool {
+  return [ERC20_transfer, ERC20_transferFrom].includes(methodSig);
+}
+
+function isNativeTokenAction<T extends ExecutedActionsStruct>(action: T): bool {
+  return action.data.toHexString() === '0x' && action.value.gt(BigInt.zero());
+}
+
+function isERC1155TransferMethod(methodSig: string): bool {
+  return [ERC1155_safeBatchTransferFrom, ERC1155_safeTransferFrom].includes(
+    methodSig
+  );
+}
diff --git a/packages/subgraph/src/utils/bytes.ts b/packages/subgraph/src/utils/bytes.ts
@@ -1,4 +1,4 @@
-import {BigInt} from '@graphprotocol/graph-ts';
+import {BigInt, Bytes} from '@graphprotocol/graph-ts';
 
 export function bigIntToBytes32(input: BigInt): string {
   const hexString = input
@@ -7,3 +7,7 @@ export function bigIntToBytes32(input: BigInt): string {
     .padStart(64, '0'); // pad left with '0' until reaching target length of 32 bytes
   return `0x${hexString}`; // add 0x to the start
 }
+
+export function getMethodSignature(data: Bytes): string {
+  return data.toHexString().slice(0, 10);
+}
diff --git a/packages/subgraph/src/utils/tokens/erc1155.ts b/packages/subgraph/src/utils/tokens/erc1155.ts
@@ -18,6 +18,7 @@ import {
   getERC1155TransferId,
   getTokenIdBalanceId
 } from './common';
+import {getMethodSignature} from '../bytes';
 
 export function supportsERC1155(token: Address): bool {
   // Double check that it's ERC1155 by calling supportsInterface checks.
@@ -198,88 +199,132 @@ export function handleERC1155Action(
   proposalId: string,
   actionIndex: number,
   event: ethereum.Event
-): void {
+): bool {
   let contract = fetchERC1155(token);
   if (!contract) {
-    return;
+    return false;
   }
 
-  let functionSelector = data.toHexString().substring(0, 10);
+  let functionSelector = getMethodSignature(data);
+  let decodeABI = determineERC1155DecodeABI(functionSelector);
+
+  // If decodeABI is not determined, return false
+  if (!decodeABI) return false;
+
   let calldata = DECODE_OFFSET + data.toHexString().slice(10);
+  let bytes = Bytes.fromHexString(calldata);
+  let decoded = ethereum.decode(decodeABI as string, bytes);
 
-  let decodeABI = '';
+  if (!decoded) {
+    return false;
+  }
+
+  let tuple = decoded.toTuple();
 
   if (functionSelector == ERC1155_safeTransferFrom) {
-    decodeABI = '(address,address,uint256,uint256,bytes)';
+    handleERC1155SingleTransfer(
+      tuple,
+      dao,
+      token,
+      proposalId,
+      event,
+      actionIndex
+    );
+  } else if (functionSelector == ERC1155_safeBatchTransferFrom) {
+    handleERC1155BatchTransfer(
+      tuple,
+      dao,
+      token,
+      proposalId,
+      event,
+      actionIndex
+    );
   }
 
-  if (functionSelector == ERC1155_safeBatchTransferFrom) {
-    decodeABI = '(address,address,uint256[],uint256[],bytes)';
+  return true;
+}
+
+function determineERC1155DecodeABI(functionSelector: string): string | null {
+  if (functionSelector == ERC1155_safeTransferFrom) {
+    return '(address,address,uint256,uint256,bytes)';
   }
-  let bytes = Bytes.fromHexString(calldata);
-  let decoded = ethereum.decode(decodeABI, bytes);
-  if (!decoded) {
-    return;
+
+  if (functionSelector == ERC1155_safeBatchTransferFrom) {
+    return '(address,address,uint256[],uint256[],bytes)';
   }
 
-  let tuple = decoded.toTuple();
+  return null;
+}
 
-  let from = tuple[0].toAddress();
-  let to = tuple[1].toAddress();
-  if (functionSelector == ERC1155_safeTransferFrom) {
-    // in single transfer create a single transfer
-    let tokenId = tuple[2].toBigInt();
-    let amount = tuple[3].toBigInt();
-    // generate unique transfer id
+function handleERC1155SingleTransfer(
+  tuple: ethereum.Tuple,
+  dao: Address,
+  token: Address,
+  proposalId: string,
+  event: ethereum.Event,
+  actionIndex: number
+): void {
+  let tokenId = tuple[2].toBigInt();
+  let amount = tuple[3].toBigInt();
+
+  // generate unique transfer id
+  let transferId = getERC1155TransferId(
+    event.transaction.hash,
+    event.transactionLogIndex,
+    actionIndex,
+    0
+  );
+
+  createErc1155Transfer(
+    transferId,
+    dao, // operator field, the operator is going to be the dao since is the one executing the action
+    tuple[0].toAddress(),
+    tuple[1].toAddress(),
+    dao,
+    token,
+    tokenId,
+    amount,
+    proposalId,
+    event.transaction.hash,
+    event.block.timestamp
+  );
+}
+
+function handleERC1155BatchTransfer(
+  tuple: ethereum.Tuple,
+  dao: Address,
+  token: Address,
+  proposalId: string,
+  event: ethereum.Event,
+  actionIndex: number
+): void {
+  let tokenIds = tuple[2].toBigIntArray();
+  let amounts = tuple[3].toBigIntArray();
+
+  // in batch transfer iterate over the tokenIds and create a transfer for each
+  for (let i = 0; i < tokenIds.length; i++) {
+    // generate unique transfer id by adding the index of the tokenIds array
     let transferId = getERC1155TransferId(
       event.transaction.hash,
       event.transactionLogIndex,
       actionIndex,
-      0
+      i
     );
-    // create transfer
+
     createErc1155Transfer(
       transferId,
       dao, // operator field, the operator is going to be the dao since is the one executing the action
-      from,
-      to,
+      tuple[0].toAddress(),
+      tuple[1].toAddress(),
       dao,
       token,
-      tokenId,
-      amount,
+      tokenIds[i],
+      amounts[i],
       proposalId,
       event.transaction.hash,
       event.block.timestamp
     );
   }
-  if (functionSelector == ERC1155_safeBatchTransferFrom) {
-    let tokenIds = tuple[2].toBigIntArray();
-    let amounts = tuple[3].toBigIntArray();
-    // in batch transfer iterate over the tokenIds and create a transfer for each
-    for (let i = 0; i < tokenIds.length; i++) {
-      // generate unique transfer id by adding the index of the tokenIds array
-      let transferId = getERC1155TransferId(
-        event.transaction.hash,
-        event.transactionLogIndex,
-        actionIndex,
-        i
-      );
-      // create transfer
-      createErc1155Transfer(
-        transferId,
-        dao, // operator field, the operator is going to be the dao since is the one executing the action
-        from,
-        to,
-        dao,
-        token,
-        tokenIds[i],
-        amounts[i],
-        proposalId,
-        event.transaction.hash,
-        event.block.timestamp
-      );
-    }
-  }
 }
 
 function createErc1155Transfer(