fix: [codex] Only count completed Binance withdrawals in callers (#3176)

* filter callers to completed Binance withdrawals

* fix: [codex] Handle failed Binance withdrawals (#3175)

* handle failed Binance withdrawals

* preserve failed Binance withdrawal tags

* match only terminal Binance withdrawals

* reuse Binance withdrawal status enum

Author: nicholaspai

src/adapter/bridges/BinanceCEXBridge.ts

@@ -24,6 +24,7 @@ import {
   getBinanceDepositType,
   BinanceTransactionType,
   getBinanceWithdrawalType,
+  isCompletedBinanceWithdrawal,
   toAddressType,
 } from "../../utils";
 import { BaseBridgeAdapter, BridgeTransactionDetails, BridgeEvents, BridgeEvent } from "./BaseBridgeAdapter";
@@ -155,6 +156,7 @@ export class BinanceCEXBridge extends BaseBridgeAdapter {
     const withdrawalHistory = await filterAsync(_withdrawalHistory, async (withdrawal) => {
       const withdrawalType = await getBinanceWithdrawalType(withdrawal);
       return (
+        isCompletedBinanceWithdrawal(withdrawal.status) &&
         withdrawal.network === BINANCE_NETWORKS[CHAIN_IDs.BSC] &&
         compareAddressesSimple(withdrawal.recipient, toAddress.toNative()) &&
         withdrawalType !== BinanceTransactionType.SWAP

src/adapter/l2Bridges/BinanceCEXBridge.ts

@@ -21,6 +21,7 @@ import {
   getBinanceDepositType,
   BinanceTransactionType,
   getBinanceWithdrawalType,
+  isCompletedBinanceWithdrawal,
   getOutstandingBinanceDeposits,
 } from "../../utils";
 import { L1Token } from "../../interfaces";
@@ -110,6 +111,7 @@ export class BinanceCEXBridge extends BaseL2BridgeAdapter {
     const withdrawHistory = await filterAsync(_withdrawHistory, async (withdrawal) => {
       const withdrawalType = await getBinanceWithdrawalType(withdrawal);
       return (
+        isCompletedBinanceWithdrawal(withdrawal.status) &&
         withdrawal.network === BINANCE_NETWORKS[CHAIN_IDs.MAINNET] &&
         compareAddressesSimple(withdrawal.recipient, fromAddress.toNative()) &&
         withdrawalType !== BinanceTransactionType.SWAP

src/finalizer/utils/binance.ts

@@ -23,6 +23,7 @@ import {
   getBinanceDepositType,
   BinanceTransactionType,
   getBinanceWithdrawalType,
+  isCompletedBinanceWithdrawal,
   truncate,
 } from "../../utils";
 import { HubPoolClient, SpokePoolClient } from "../../clients";
@@ -130,7 +131,7 @@ export async function binanceFinalizer(
       // as the existing inventory client logic does not yet tag withdrawals with this BRIDGE type.
       const withdrawals = await filterAsync(_withdrawals, async (withdrawal) => {
         const withdrawalType = await getBinanceWithdrawalType(withdrawal);
-        return withdrawalType !== BinanceTransactionType.SWAP;
+        return isCompletedBinanceWithdrawal(withdrawal.status) && withdrawalType !== BinanceTransactionType.SWAP;
       });
 
       // @dev Since we cannot determine the address of the binance depositor without querying the transaction receipt, we need to assume that all tokens

src/rebalancer/adapters/binance.ts

@@ -3,6 +3,7 @@ import {
   assert,
   BigNumber,
   BINANCE_NETWORKS,
+  BINANCE_WITHDRAWAL_STATUS,
   BinanceTransactionType,
   BinanceWithdrawal,
   bnZero,
@@ -33,6 +34,7 @@ import { AugmentedTransaction } from "../../clients";
 import { RebalancerConfig } from "../RebalancerConfig";
 import { CctpAdapter } from "./cctpAdapter";
 import { OftAdapter } from "./oftAdapter";
+
 interface SPOT_MARKET_META {
   symbol: string;
   baseAssetName: string;
@@ -43,6 +45,29 @@ interface SPOT_MARKET_META {
   isBuy: boolean;
 }
 
+export function isFailedBinanceWithdrawal(status?: number): boolean {
+  switch (status) {
+    case BINANCE_WITHDRAWAL_STATUS.CANCELLED:
+    case BINANCE_WITHDRAWAL_STATUS.REJECTED:
+    case BINANCE_WITHDRAWAL_STATUS.FAILURE:
+      return true;
+    default:
+      return false;
+  }
+}
+
+export function isTerminalBinanceWithdrawal(status?: number): boolean {
+  switch (status) {
+    case BINANCE_WITHDRAWAL_STATUS.CANCELLED:
+    case BINANCE_WITHDRAWAL_STATUS.REJECTED:
+    case BINANCE_WITHDRAWAL_STATUS.FAILURE:
+    case BINANCE_WITHDRAWAL_STATUS.COMPLETED:
+      return true;
+    default:
+      return false;
+  }
+}
+
 export class BinanceStablecoinSwapAdapter extends BaseAdapter {
   private binanceApiClient: Binance;
 
@@ -307,12 +332,25 @@ export class BinanceStablecoinSwapAdapter extends BaseAdapter {
         continue;
       } // Only proceed to update the order status if it has finalized:
       // @todo: Can we cache this result to avoid making the same query for orders with the same destination token and withdrawal network?
-      const { unfinalizedWithdrawals, finalizedWithdrawals } = await this._getBinanceWithdrawals(
+      const { unfinalizedWithdrawals, finalizedWithdrawals, failedWithdrawals } = await this._getBinanceWithdrawals(
         orderDetails.destinationToken,
         binanceWithdrawalNetwork,
         Math.floor(matchingFill.time / 1000) - 5 * 60 // Floor this so we can grab the initiated withdrawal data whose
         // ID we've already saved into Redis
       );
+      const failedWithdrawal = failedWithdrawals.find((withdrawal) => withdrawal.id === initiatedWithdrawalId);
+      if (failedWithdrawal) {
+        this.logger.warn({
+          at: "BinanceStablecoinSwapAdapter.updateRebalanceStatuses",
+          message: `Withdrawal for order ${cloid} failed on Binance, resetting it to retry withdrawal`,
+          cloid,
+          initiatedWithdrawalId,
+          failedWithdrawal,
+        });
+        await this._redisDeleteInitiatedWithdrawalId(cloid);
+        await this._redisUpdateOrderStatus(cloid, STATUS.PENDING_WITHDRAWAL, STATUS.PENDING_SWAP);
+        continue;
+      }
       const initiatedWithdrawalIsUnfinalized = unfinalizedWithdrawals.find(
         (withdrawal) => withdrawal.id === initiatedWithdrawalId
       );
@@ -976,16 +1014,19 @@ export class BinanceStablecoinSwapAdapter extends BaseAdapter {
         withdrawal.coin === token &&
         withdrawal.network === BINANCE_NETWORKS[chain] &&
         withdrawal.recipient === this.baseSignerAddress.toNative() &&
-        withdrawal.status > 4
-      // @dev (0: Email Sent, 1: Cancelled 2: Awaiting Approval, 3: Rejected, 4: Processing, 5: Failure, 6: Completed)
+        isTerminalBinanceWithdrawal(withdrawal.status)
     );
   }
 
   private async _getBinanceWithdrawals(
     destinationToken: string,
     destinationChain: number,
     startTimeSeconds: number
-  ): Promise<{ unfinalizedWithdrawals: BinanceWithdrawal[]; finalizedWithdrawals: BinanceWithdrawal[] }> {
+  ): Promise<{
+    unfinalizedWithdrawals: BinanceWithdrawal[];
+    finalizedWithdrawals: BinanceWithdrawal[];
+    failedWithdrawals: BinanceWithdrawal[];
+  }> {
     assert(isDefined(BINANCE_NETWORKS[destinationChain]), "Destination chain should be a Binance network");
     const provider = await getProvider(destinationChain);
     // @dev Binance withdrawals are fast, so setting a lookback of 6 hours should capture any unfinalized withdrawals.
@@ -1013,7 +1054,12 @@ export class BinanceStablecoinSwapAdapter extends BaseAdapter {
 
     const finalizedWithdrawals: BinanceWithdrawal[] = [];
     const unfinalizedWithdrawals: BinanceWithdrawal[] = [];
+    const failedWithdrawals: BinanceWithdrawal[] = [];
     for (const initiated of initiatedWithdrawals) {
+      if (isFailedBinanceWithdrawal(initiated.status)) {
+        failedWithdrawals.push(initiated);
+        continue;
+      }
       const withdrawalAmount = toBNWei(
         initiated.amount, // @dev This should be the post-withdrawal fee amount so it should match perfectly
         // with the finalized amount.
@@ -1030,7 +1076,7 @@ export class BinanceStablecoinSwapAdapter extends BaseAdapter {
         unfinalizedWithdrawals.push(initiated);
       }
     }
-    return { unfinalizedWithdrawals, finalizedWithdrawals };
+    return { unfinalizedWithdrawals, finalizedWithdrawals, failedWithdrawals };
   }
 
   private _redisGetInitiatedWithdrawalKey(cloid: string): string {
@@ -1043,6 +1089,11 @@ export class BinanceStablecoinSwapAdapter extends BaseAdapter {
     return initiatedWithdrawal;
   }
 
+  private async _redisDeleteInitiatedWithdrawalId(cloid: string): Promise<void> {
+    const initiatedWithdrawalKey = this._redisGetInitiatedWithdrawalKey(cloid);
+    await this.redisCache.del(initiatedWithdrawalKey);
+  }
+
   protected async _bridgeToChain(
     token: string,
     originChain: number,

src/utils/BinanceUtils.ts

@@ -78,6 +78,16 @@ export type BinanceWithdrawal = Omit<BinanceDeposit, "insertTime"> & {
   applyTime: string;
 };
 
+export enum BINANCE_WITHDRAWAL_STATUS {
+  EMAIL_SENT = 0,
+  CANCELLED = 1,
+  AWAITING_APPROVAL = 2,
+  REJECTED = 3,
+  PROCESSING = 4,
+  FAILURE = 5,
+  COMPLETED = 6,
+}
+
 // ParsedAccountCoins represents a simplified return type of the Binance `accountCoins` endpoint.
 type ParsedAccountCoins = Coin[];
 
@@ -229,6 +239,10 @@ export async function getBinanceWithdrawalType(
   }
 }
 
+export function isCompletedBinanceWithdrawal(status?: number): boolean {
+  return status === BINANCE_WITHDRAWAL_STATUS.COMPLETED;
+}
+
 /**
  * Gets all binance deposits for the Binance account starting from `startTime`-present.
  * @returns An array of parsed binance deposits.

test/BinanceAdapter.withdrawals.ts

@@ -0,0 +1,29 @@
+import { expect } from "./utils";
+import { isFailedBinanceWithdrawal, isTerminalBinanceWithdrawal } from "../src/rebalancer/adapters/binance";
+
+describe("Binance adapter withdrawal state", function () {
+  it("treats terminal Binance withdrawal failures as failed", function () {
+    expect(isFailedBinanceWithdrawal(1)).to.equal(true);
+    expect(isFailedBinanceWithdrawal(3)).to.equal(true);
+    expect(isFailedBinanceWithdrawal(5)).to.equal(true);
+  });
+
+  it("does not treat in-flight or completed Binance withdrawals as failed", function () {
+    expect(isFailedBinanceWithdrawal(0)).to.equal(false);
+    expect(isFailedBinanceWithdrawal(2)).to.equal(false);
+    expect(isFailedBinanceWithdrawal(4)).to.equal(false);
+    expect(isFailedBinanceWithdrawal(6)).to.equal(false);
+    expect(isFailedBinanceWithdrawal(undefined)).to.equal(false);
+  });
+
+  it("only treats terminal Binance withdrawals as terminal", function () {
+    expect(isTerminalBinanceWithdrawal(1)).to.equal(true);
+    expect(isTerminalBinanceWithdrawal(3)).to.equal(true);
+    expect(isTerminalBinanceWithdrawal(5)).to.equal(true);
+    expect(isTerminalBinanceWithdrawal(6)).to.equal(true);
+    expect(isTerminalBinanceWithdrawal(0)).to.equal(false);
+    expect(isTerminalBinanceWithdrawal(2)).to.equal(false);
+    expect(isTerminalBinanceWithdrawal(4)).to.equal(false);
+    expect(isTerminalBinanceWithdrawal(undefined)).to.equal(false);
+  });
+});

test/BinanceUtils.ts

@@ -1,5 +1,10 @@
 import { expect } from "./utils";
-import { BinanceDeposit, BinanceWithdrawal, getOutstandingBinanceDeposits } from "../src/utils";
+import {
+  BinanceDeposit,
+  BinanceWithdrawal,
+  getOutstandingBinanceDeposits,
+  isCompletedBinanceWithdrawal,
+} from "../src/utils";
 
 function makeDeposit(network: string, amount: number, insertTime: number): BinanceDeposit {
   return { network, amount, coin: "USDT", txId: `0x${insertTime}`, insertTime };
@@ -99,3 +104,13 @@ describe("BinanceUtils: getOutstandingBinanceDeposits", function () {
     expect(deposits.length).to.equal(1);
   });
 });
+describe("BinanceUtils withdrawal helpers", function () {
+  it("only treats completed Binance withdrawals as completed", function () {
+    expect(isCompletedBinanceWithdrawal(6)).to.equal(true);
+    expect(isCompletedBinanceWithdrawal(0)).to.equal(false);
+    expect(isCompletedBinanceWithdrawal(2)).to.equal(false);
+    expect(isCompletedBinanceWithdrawal(4)).to.equal(false);
+    expect(isCompletedBinanceWithdrawal(5)).to.equal(false);
+    expect(isCompletedBinanceWithdrawal(undefined)).to.equal(false);
+  });
+});