sdk: fix eth withdraw bug (#3228)

* sdk: fix eth withdraw bug

Fixes the eth withdrawal bug when creating withdrawl
transactions that include value. There is no need to
send ETH on L1 when finalizing the withdrawal since
the `OptimismPortal` is holding onto the ETH that will
be unlocked and sent to the user.
co-authored-by: smartcontracts <kelvin@optimism.io>

* sdk: cleanup code + fix hanging eth deposits
co-authored-by: smartcontracts <kelvin@optimism.io>

* ci: run devnet on sdk changes

* lint: fix
Co-authored-by: smartcontracts <kelvin@optimism.io>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

.changeset/lazy-singers-sing.md

+---
+'@eth-optimism/sdk': patch
+---
+
+Fix eth withdrawal bug

.circleci/config.yml

@@ -460,7 +460,7 @@ jobs:
       - run:
           name: Check if we should run
           command: |
-            CHANGED=$(bash ./ops/docker/ci-builder/check-changed.sh "(contracts-bedrock|op-bindings|op-batcher|op-node|op-proposer|ops-bedrock)/")
+            CHANGED=$(bash ./ops/docker/ci-builder/check-changed.sh "(contracts-bedrock|op-bindings|op-batcher|op-node|op-proposer|ops-bedrock|sdk)/")
             if [[ "$CHANGED" = "FALSE" ]]; then
               circleci step halt
             fi

packages/sdk/src/cross-chain-messenger.ts

@@ -5,7 +5,6 @@ import {
   TransactionReceipt,
   TransactionResponse,
   TransactionRequest,
-  Log,
 } from '@ethersproject/abstract-provider'
 import { Signer } from '@ethersproject/abstract-signer'
 import {
@@ -250,15 +249,17 @@ export class CrossChainMessenger implements ICrossChainMessenger {
         // Try to pull out the value field, but only if the very next log is a SentMessageExtraData
         // event which was introduced in the Bedrock upgrade.
         let value = ethers.BigNumber.from(0)
-        if (receipt.logs.length > log.logIndex + 1) {
-          const next = receipt.logs[log.logIndex + 1]
-          if (next.address === messenger.address) {
+        const next = receipt.logs.find((l) => {
+          return (
+            l.logIndex === log.logIndex + 1 && l.address === messenger.address
+          )
+        })
+        if (next) {
           const nextParsed = messenger.interface.parseLog(next)
           if (nextParsed.name === 'SentMessageExtension1') {
             value = nextParsed.args.value
           }
         }
-        }
 
         // Convert each SentMessage log into a message object
         const parsed = messenger.interface.parseLog(log)
@@ -1097,7 +1098,6 @@ export class CrossChainMessenger implements ICrossChainMessenger {
           latestBlockhash: block.hash,
         },
         withdrawalProof: ethers.utils.RLP.encode(stateTrieProof.storageProof),
-        // withdrawalProof: toHexString(rlp.encode(stateTrieProof.storageProof)),
       },
       output,
       // TODO(tynes): use better type, typechain?
@@ -1333,15 +1333,6 @@ export class CrossChainMessenger implements ICrossChainMessenger {
         const [proof, output, withdrawalTx] = await this.getBedrockMessageProof(
           message
         )
-        if (!opts) {
-          opts = {}
-        }
-        if (!opts.overrides) {
-          opts.overrides = {}
-        }
-        if (!opts.overrides.value) {
-          opts.overrides.value = withdrawalTx.value
-        }
 
         return this.contracts.l1.OptimismPortal.populateTransaction.finalizeWithdrawalTransaction(
           [
@@ -1360,7 +1351,7 @@ export class CrossChainMessenger implements ICrossChainMessenger {
             proof.outputRootProof.latestBlockhash,
           ],
           proof.withdrawalProof,
-          opts.overrides
+          opts?.overrides || {}
         )
       } else {
         // L1CrossDomainMessenger relayMessage is the only method that isn't fully backwards
@@ -1391,22 +1382,6 @@ export class CrossChainMessenger implements ICrossChainMessenger {
         overrides?: PayableOverrides
       }
     ): Promise<TransactionRequest> => {
-      if (this.bedrock) {
-        const value = BigNumber.from(opts?.overrides?.value ?? 0)
-        if (!value.eq(0) && !value.eq(amount)) {
-          throw new Error(`amount and value mismatch`)
-        }
-        if (!opts) {
-          opts = {}
-        }
-        if (!opts.overrides) {
-          opts.overrides = {}
-        }
-        if (!opts.overrides.value) {
-          opts.overrides.value = value
-        }
-      }
-
       return this.bridges.ETH.populateTransaction.deposit(
         ethers.constants.AddressZero,
         predeploys.OVM_ETH,
@@ -1419,24 +1394,9 @@ export class CrossChainMessenger implements ICrossChainMessenger {
       amount: NumberLike,
       opts?: {
         recipient?: AddressLike
-        overrides?: PayableOverrides
+        overrides?: Overrides
       }
     ): Promise<TransactionRequest> => {
-      const value = BigNumber.from(opts?.overrides?.value ?? 0)
-      if (this.bedrock) {
-        if (!value.eq(0) && !value.eq(amount)) {
-          throw new Error(`amount and value mismatch`)
-        }
-        if (!opts) {
-          opts = {}
-        }
-        if (!opts.overrides) {
-          opts.overrides = {}
-        }
-        if (!opts.overrides.value) {
-          opts.overrides.value = value
-        }
-      }
       return this.bridges.ETH.populateTransaction.withdraw(
         ethers.constants.AddressZero,
         predeploys.OVM_ETH,

packages/sdk/src/interfaces/bridge-adapter.ts

@@ -232,7 +232,7 @@ export interface IBridgeAdapter {
       amount: NumberLike,
       opts?: {
         recipient?: AddressLike
-        overrides?: PayableOverrides
+        overrides?: Overrides
       }
     ): Promise<TransactionRequest>
   }

packages/sdk/tasks/deposit-eth.ts

@@ -67,7 +67,7 @@ task('deposit-eth', 'Deposits WETH9 onto L2.')
       : utils.parseEther('1')
     const withdrawAmount = args.withdrawAmount
       ? utils.parseEther(args.withdrawAmount)
-      : utils.parseEther(amount.div(2).toString())
+      : amount.div(2)
 
     const l2Signer = new hre.ethers.Wallet(
       hre.network.config.accounts[0],
@@ -313,14 +313,9 @@ task('deposit-eth', 'Deposits WETH9 onto L2.')
     const opBalanceFinally = await signer.provider.getBalance(
       OptimismPortal.address
     )
-    // TODO(tynes): fix this bug
-    if (!opBalanceFinally.sub(withdrawAmount).eq(opBalanceAfter)) {
-      console.log('OptimismPortal balance mismatch')
-      console.log(`Balance before deposit: ${opBalanceBefore.toString()}`)
-      console.log(`Balance after deposit: ${opBalanceAfter.toString()}`)
-      console.log(`Balance after withdrawal: ${opBalanceFinally.toString()}`)
-      return
-      // throw new Error('OptimismPortal balance mismatch')
+
+    if (!opBalanceFinally.add(withdrawAmount).eq(opBalanceAfter)) {
+      throw new Error('OptimismPortal balance mismatch')
     }
     console.log('Withdraw success')
   })