etherfi: gate claim list by on-chain finalized+valid (#304)

Subgraph can report already-claimed requests as claimable, so
claimEtherFiWithdrawals reverted with "ERC721: invalid token ID".

Author: sparrowDom

src/js/tasks/etherfiQueue.js

@@ -1,5 +1,5 @@
 const { gql } = require("@apollo/client/core");
-const { parseUnits } = require("ethers");
+const { Contract, parseUnits } = require("ethers");
 
 const { baseWithdrawAmount } = require("./liquidityAutomation");
 const {
@@ -8,13 +8,19 @@ const {
   requestBaseAssetWithdrawal,
   resolveArmBase,
 } = require("../utils/arm");
+const addresses = require("../utils/addresses");
 const { createApolloClient } = require("../utils/apollo");
 const { logTxDetails } = require("../utils/txLogger");
 
 const log = require("../utils/logger")("task:etherfiQueue");
 
 const uri = "https://origin.squids.live/ops-squid/graphql";
 
+const ETHERFI_WITHDRAWAL_NFT_ABI = [
+  "function isFinalized(uint256 requestId) view returns (bool)",
+  "function getRequest(uint256 requestId) view returns (tuple(uint96 amountOfEEth, uint96 shareOfEEth, bool isValid, uint32 feeGwei))",
+];
+
 const requestEtherFiWithdrawals = async (options) => {
   const { signer, amount } = options;
   const baseContext = await resolveArmBase(options);
@@ -43,7 +49,7 @@ const claimEtherFiWithdrawals = async (options) => {
     ? // If an id is provided, just claim that one
       [id]
     : // Get the outstanding EtherFi withdrawal requests for the ARM
-      await claimableEtherFiRequests();
+      await claimableEtherFiRequests(signer);
 
   if (baseContext.version === "legacy") {
     if (requestIds.length > 0) {
@@ -84,7 +90,32 @@ const claimEtherFiWithdrawals = async (options) => {
   await logTxDetails(tx, "claim EtherFi withdraws");
 };
 
-const claimableEtherFiRequests = async () => {
+// Read the on-chain finalized/valid state of each subgraph-reported request.
+// EtherFi never reverts on these views (isFinalized is a numeric comparison and
+// getRequest returns a zeroed struct for burnt/unknown ids), so one stale id
+// can't break the batch.
+const etherFiRequestStatuses = async (withdrawalNFT, requestIds) =>
+  Promise.all(
+    requestIds.map(async (requestId) => {
+      const [isFinalized, request] = await Promise.all([
+        withdrawalNFT.isFinalized(requestId),
+        withdrawalNFT.getRequest(requestId),
+      ]);
+      return { requestId, isFinalized, isValid: request.isValid };
+    }),
+  );
+
+// Only finalized requests whose withdrawal NFT still exists (isValid) can be
+// claimed. EtherFi's isFinalized() stays true after a claim and the ops-squid
+// subgraph can lag on its `claimed` flag, so without this on-chain gate an
+// already-claimed request keeps coming back and claimEtherFiWithdrawals reverts
+// with "ERC721: invalid token ID" (the burnt NFT), wedging the action.
+const selectClaimableEtherFiRequests = (statuses) =>
+  statuses
+    .filter(({ isFinalized, isValid }) => isFinalized && isValid)
+    .map(({ requestId }) => requestId);
+
+const claimableEtherFiRequests = async (signer) => {
   const client = createApolloClient(uri);
 
   log(`About to get claimable EtherFi withdrawal requests`);
@@ -100,28 +131,47 @@ const claimableEtherFiRequests = async () => {
     }
   `;
 
+  let candidateIds;
   try {
     const { data } = await client.query({
       query,
     });
-
-    const claimableRequests = data.etherfiWithdrawalRequests.map(
+    candidateIds = data.etherfiWithdrawalRequests.map(
       (request) => request.requestId,
     );
-
-    log(
-      `Found ${claimableRequests.length} claimable withdrawal requests: ${claimableRequests}`,
-    );
-
-    return claimableRequests;
   } catch (error) {
     const msg = `Failed to get claimable EtherFi withdrawal requests`;
     console.error(msg);
     throw Error(msg, { cause: error });
   }
+
+  const withdrawalNFT = new Contract(
+    addresses.mainnet.etherfiWithdrawalQueue,
+    ETHERFI_WITHDRAWAL_NFT_ABI,
+    signer,
+  );
+  const statuses = await etherFiRequestStatuses(withdrawalNFT, candidateIds);
+  const claimableRequests = selectClaimableEtherFiRequests(statuses);
+
+  const skipped = statuses
+    .filter(({ isFinalized, isValid }) => !(isFinalized && isValid))
+    .map(({ requestId }) => requestId);
+  if (skipped.length > 0) {
+    log(
+      `Skipping ${skipped.length} subgraph requests not claimable on-chain (already claimed or not finalized): ${skipped}`,
+    );
+  }
+
+  log(
+    `Found ${claimableRequests.length} claimable withdrawal requests: ${claimableRequests}`,
+  );
+
+  return claimableRequests;
 };
 
 module.exports = {
   requestEtherFiWithdrawals,
   claimEtherFiWithdrawals,
+  etherFiRequestStatuses,
+  selectClaimableEtherFiRequests,
 };

test/js/etherfiQueue.test.js

@@ -0,0 +1,85 @@
+const assert = require("assert");
+
+const { AbiCoder, Contract, id } = require("ethers");
+
+const {
+  etherFiRequestStatuses,
+  selectClaimableEtherFiRequests,
+} = require("../../src/js/tasks/etherfiQueue");
+
+const coder = AbiCoder.defaultAbiCoder();
+
+const selector = (signature) => id(signature).slice(0, 10);
+
+const run = async () => {
+  // Pure filter: finalized AND still valid on-chain is the only claimable state.
+  {
+    const statuses = [
+      // Already claimed: the NFT is burnt so isValid is false even though
+      // EtherFi still reports isFinalized true. Regression for the recurring
+      // "ERC721: invalid token ID" failure (mainnet request 80636).
+      { requestId: 80636, isFinalized: true, isValid: false },
+      // Requested but not yet finalized: valid but can't be claimed yet.
+      { requestId: 80648, isFinalized: false, isValid: true },
+      // Genuinely claimable.
+      { requestId: 80641, isFinalized: true, isValid: true },
+    ];
+
+    assert.deepStrictEqual(selectClaimableEtherFiRequests(statuses), [80641]);
+  }
+
+  // End-to-end status read + filter against a mock WithdrawRequestNFT, proving
+  // the stale request 80636 that wedged autoClaimEtherFiWithdraw is dropped.
+  {
+    const selectors = {
+      isFinalized: selector("isFinalized(uint256)"),
+      getRequest: selector("getRequest(uint256)"),
+    };
+    const requestId = (data) => BigInt(`0x${data.slice(10)}`).toString();
+
+    // isFinalized mirrors lastFinalizedRequestId: true for ids <= 80647.
+    const finalizedById = { 80636: true, 80641: true, 80648: false };
+    // isValid is false for the already-claimed/burnt 80636.
+    const validById = { 80636: false, 80641: true, 80648: true };
+
+    const runner = {
+      call: async (tx) => {
+        const fn = tx.data.slice(0, 10);
+        const key = requestId(tx.data);
+        if (fn === selectors.isFinalized) {
+          return coder.encode(["bool"], [finalizedById[key]]);
+        }
+        if (fn === selectors.getRequest) {
+          return coder.encode(
+            ["tuple(uint96,uint96,bool,uint32)"],
+            [[0n, 0n, validById[key], 0]],
+          );
+        }
+        throw new Error(`unexpected selector ${tx.data}`);
+      },
+    };
+
+    const withdrawalNFT = new Contract(
+      "0x7d5706f6ef3F89B3951E23e557CDFBC3239D4E2c",
+      [
+        "function isFinalized(uint256 requestId) view returns (bool)",
+        "function getRequest(uint256 requestId) view returns (tuple(uint96 amountOfEEth, uint96 shareOfEEth, bool isValid, uint32 feeGwei))",
+      ],
+      runner,
+    );
+
+    const statuses = await etherFiRequestStatuses(
+      withdrawalNFT,
+      [80636, 80641, 80648],
+    );
+
+    assert.deepStrictEqual(selectClaimableEtherFiRequests(statuses), [80641]);
+  }
+};
+
+run()
+  .then(() => console.log("etherfiQueue tests passed"))
+  .catch((error) => {
+    console.error(error);
+    process.exitCode = 1;
+  });