Merge pull request #6053 from ethereum-optimism/willc/multiwithdraw-no-test

fix(sdk): Fix multicall3 support for sdk

.changeset/cuddly-panthers-trade.md

+---
+'@eth-optimism/sdk': minor
+---
+
+Add support for claiming multicall3 withdrawals

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

@@ -329,9 +329,13 @@ export class CrossChainMessenger {
    * @returns Bedrock representation of the message.
    */
   public async toBedrockCrossChainMessage(
-    message: MessageLike
+    message: MessageLike,
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<CrossChainMessage> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
 
     // Bedrock messages are already in the correct format.
     const { version } = decodeVersionedNonce(resolved.messageNonce)
@@ -375,9 +379,13 @@ export class CrossChainMessenger {
    * @return Transformed message.
    */
   public async toLowLevelMessage(
-    message: MessageLike
+    message: MessageLike,
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<LowLevelMessage> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
     if (resolved.direction === MessageDirection.L1_TO_L2) {
       throw new Error(`can only convert L2 to L1 messages to low level`)
     }
@@ -386,7 +394,7 @@ export class CrossChainMessenger {
     const { version } = decodeVersionedNonce(resolved.messageNonce)
     let updated: CrossChainMessage
     if (version.eq(0)) {
-      updated = await this.toBedrockCrossChainMessage(resolved)
+      updated = await this.toBedrockCrossChainMessage(resolved, messageIndex)
     } else {
       updated = resolved
     }
@@ -401,6 +409,8 @@ export class CrossChainMessenger {
       updated.message
     )
 
+    // EVERYTHING following here is basically repeating the logic from getMessagesByTransaction 
+    // consider cleaning this up
     // We need to figure out the final withdrawal data that was used to compute the withdrawal hash
     // inside the L2ToL1Message passer contract. Exact mechanism here depends on whether or not
     // this is a legacy message or a new Bedrock message.
@@ -412,10 +422,12 @@ export class CrossChainMessenger {
       messageNonce = resolved.messageNonce
     } else {
       const receipt = await this.l2Provider.getTransactionReceipt(
-        resolved.transactionHash
+        (
+          await this.toCrossChainMessage(message)
+        ).transactionHash
       )
 
-      const withdrawals: any[] = []
+      const withdrawals: ethers.utils.Result[] = []
       for (const log of receipt.logs) {
         if (log.address === this.contracts.l2.BedrockMessagePasser.address) {
           const decoded =
@@ -431,12 +443,12 @@ export class CrossChainMessenger {
         throw new Error(`no withdrawals found in receipt`)
       }
 
-      // TODO: Add support for multiple withdrawals.
-      if (withdrawals.length > 1) {
-        throw new Error(`multiple withdrawals found in receipt`)
+      const withdrawal = withdrawals[messageIndex]
+      if (!withdrawal) {
+        throw new Error(
+          `withdrawal index ${messageIndex} out of bounds there are ${withdrawals.length} withdrawals`
+        )
       }
-
-      const withdrawal = withdrawals[0]
       messageNonce = withdrawal.nonce
       gasLimit = withdrawal.gasLimit
     }
@@ -577,7 +589,11 @@ export class CrossChainMessenger {
    * @returns Message coerced into a CrossChainMessage.
    */
   public async toCrossChainMessage(
-    message: MessageLike
+    message: MessageLike,
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<CrossChainMessage> {
     if (!message) {
       throw new Error('message is undefined')
@@ -621,14 +637,13 @@ export class CrossChainMessenger {
         message as TransactionLike
       )
 
-      // We only want to treat TransactionLike objects as MessageLike if they only emit a single
-      // message (very common). It's unintuitive to treat a TransactionLike as a MessageLike if
-      // they emit more than one message (which message do you pick?), so we throw an error.
-      if (messages.length !== 1) {
-        throw new Error(`expected 1 message, got ${messages.length}`)
+      const out = messages[messageIndex]
+      if (!out) {
+        throw new Error(
+          `withdrawal index ${messageIndex} out of bounds. There are ${messages.length} withdrawals`
+        )
       }
-
-      return messages[0]
+      return out
     }
   }
 
@@ -638,9 +653,16 @@ export class CrossChainMessenger {
    * @param message Cross chain message to check the status of.
    * @returns Status of the message.
    */
-  public async getMessageStatus(message: MessageLike): Promise<MessageStatus> {
-    const resolved = await this.toCrossChainMessage(message)
-    const receipt = await this.getMessageReceipt(resolved)
+  public async getMessageStatus(
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
+  ): Promise<MessageStatus> {
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
+    const receipt = await this.getMessageReceipt(resolved, messageIndex)
 
     if (resolved.direction === MessageDirection.L1_TO_L2) {
       if (receipt === null) {
@@ -656,13 +678,19 @@ export class CrossChainMessenger {
       if (receipt === null) {
         let timestamp: number
         if (this.bedrock) {
-          const output = await this.getMessageBedrockOutput(resolved)
+          const output = await this.getMessageBedrockOutput(
+            resolved,
+            messageIndex
+          )
           if (output === null) {
             return MessageStatus.STATE_ROOT_NOT_PUBLISHED
           }
 
           // Convert the message to the low level message that was proven.
-          const withdrawal = await this.toLowLevelMessage(resolved)
+          const withdrawal = await this.toLowLevelMessage(
+            resolved,
+            messageIndex
+          )
 
           // Attempt to fetch the proven withdrawal.
           const provenWithdrawal =
@@ -679,7 +707,10 @@ export class CrossChainMessenger {
           // Set the timestamp to the provenWithdrawal's timestamp
           timestamp = provenWithdrawal.timestamp.toNumber()
         } else {
-          const stateRoot = await this.getMessageStateRoot(resolved)
+          const stateRoot = await this.getMessageStateRoot(
+            resolved,
+            messageIndex
+          )
           if (stateRoot === null) {
             return MessageStatus.STATE_ROOT_NOT_PUBLISHED
           }
@@ -715,9 +746,14 @@ export class CrossChainMessenger {
    * given message.
    */
   public async getMessageReceipt(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<MessageReceipt> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
     // legacy withdrawals relayed prebedrock are v1
     const messageHashV0 = hashCrossDomainMessagev0(
       resolved.target,
@@ -819,15 +855,20 @@ export class CrossChainMessenger {
       confirmations?: number
       pollIntervalMs?: number
       timeoutMs?: number
-    } = {}
+    } = {},
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<MessageReceipt> {
     // Resolving once up-front is slightly more efficient.
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
 
     let totalTimeMs = 0
     while (totalTimeMs < (opts.timeoutMs || Infinity)) {
       const tick = Date.now()
-      const receipt = await this.getMessageReceipt(resolved)
+      const receipt = await this.getMessageReceipt(resolved, messageIndex)
       if (receipt !== null) {
         return receipt
       } else {
@@ -857,15 +898,20 @@ export class CrossChainMessenger {
     opts: {
       pollIntervalMs?: number
       timeoutMs?: number
-    } = {}
+    } = {},
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<void> {
     // Resolving once up-front is slightly more efficient.
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
 
     let totalTimeMs = 0
     while (totalTimeMs < (opts.timeoutMs || Infinity)) {
       const tick = Date.now()
-      const currentStatus = await this.getMessageStatus(resolved)
+      const currentStatus = await this.getMessageStatus(resolved, messageIndex)
 
       // Handle special cases for L1 to L2 messages.
       if (resolved.direction === MessageDirection.L1_TO_L2) {
@@ -933,7 +979,8 @@ export class CrossChainMessenger {
     opts?: {
       bufferPercent?: number
       from?: string
-    }
+    },
+    messageIndex = 0
   ): Promise<BigNumber> {
     let resolved: CrossChainMessage | CrossChainMessageRequest
     let from: string
@@ -941,7 +988,10 @@ export class CrossChainMessenger {
       resolved = message as CrossChainMessageRequest
       from = opts?.from
     } else {
-      resolved = await this.toCrossChainMessage(message as MessageLike)
+      resolved = await this.toCrossChainMessage(
+        message as MessageLike,
+        messageIndex
+      )
       from = opts?.from || (resolved as CrossChainMessage).sender
     }
 
@@ -971,10 +1021,15 @@ export class CrossChainMessenger {
    * @returns Estimated amount of time remaining (in seconds) before the message can be executed.
    */
   public async estimateMessageWaitTimeSeconds(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<number> {
-    const resolved = await this.toCrossChainMessage(message)
-    const status = await this.getMessageStatus(resolved)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
+    const status = await this.getMessageStatus(resolved, messageIndex)
     if (resolved.direction === MessageDirection.L1_TO_L2) {
       if (
         status === MessageStatus.RELAYED ||
@@ -1012,7 +1067,7 @@ export class CrossChainMessenger {
         // If the message is still within the challenge period, then we need to estimate exactly
         // the amount of time left until the challenge period expires. The challenge period starts
         // when the state root is published.
-        const stateRoot = await this.getMessageStateRoot(resolved)
+        const stateRoot = await this.getMessageStateRoot(resolved, messageIndex)
         const challengePeriod = await this.getChallengePeriodSeconds()
         const targetBlock = await this.l1Provider.getBlock(
           stateRoot.batch.blockNumber
@@ -1045,13 +1100,13 @@ export class CrossChainMessenger {
     const challengePeriod =
       oracleVersion === '1.0.0'
         ? // The ABI in the SDK does not contain FINALIZATION_PERIOD_SECONDS
-          // in OptimismPortal, so making an explicit call instead.
-          BigNumber.from(
-            await this.contracts.l1.OptimismPortal.provider.call({
-              to: this.contracts.l1.OptimismPortal.address,
-              data: '0xf4daa291', // FINALIZATION_PERIOD_SECONDS
-            })
-          )
+        // in OptimismPortal, so making an explicit call instead.
+        BigNumber.from(
+          await this.contracts.l1.OptimismPortal.provider.call({
+            to: this.contracts.l1.OptimismPortal.address,
+            data: '0xf4daa291', // FINALIZATION_PERIOD_SECONDS
+          })
+        )
         : await this.contracts.l1.L2OutputOracle.FINALIZATION_PERIOD_SECONDS()
     return challengePeriod.toNumber()
   }
@@ -1082,9 +1137,14 @@ export class CrossChainMessenger {
    * @returns Bedrock output root.
    */
   public async getMessageBedrockOutput(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<BedrockOutputData | null> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
 
     // Outputs are only a thing for L2 to L1 messages.
     if (resolved.direction === MessageDirection.L1_TO_L2) {
@@ -1133,9 +1193,14 @@ export class CrossChainMessenger {
    * @returns State root for the block in which the message was created.
    */
   public async getMessageStateRoot(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<StateRoot | null> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
 
     // State roots are only a thing for L2 to L1 messages.
     if (resolved.direction === MessageDirection.L1_TO_L2) {
@@ -1318,14 +1383,19 @@ export class CrossChainMessenger {
    * @returns Proof that can be used to finalize the message.
    */
   public async getMessageProof(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<CrossChainMessageProof> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
     if (resolved.direction === MessageDirection.L1_TO_L2) {
       throw new Error(`can only generate proofs for L2 to L1 messages`)
     }
 
-    const stateRoot = await this.getMessageStateRoot(resolved)
+    const stateRoot = await this.getMessageStateRoot(resolved, messageIndex)
     if (stateRoot === null) {
       throw new Error(`state root for message not yet published`)
     }
@@ -1376,19 +1446,24 @@ export class CrossChainMessenger {
    * @returns Proof that can be used to finalize the message.
    */
   public async getBedrockMessageProof(
-    message: MessageLike
+    message: MessageLike,
+
+    /**
+     * The index of the withdrawal if multiple are made with multicall
+     */
+    messageIndex = 0
   ): Promise<BedrockCrossChainMessageProof> {
-    const resolved = await this.toCrossChainMessage(message)
+    const resolved = await this.toCrossChainMessage(message, messageIndex)
     if (resolved.direction === MessageDirection.L1_TO_L2) {
       throw new Error(`can only generate proofs for L2 to L1 messages`)
     }
 
-    const output = await this.getMessageBedrockOutput(resolved)
+    const output = await this.getMessageBedrockOutput(resolved, messageIndex)
     if (output === null) {
       throw new Error(`state root for message not yet published`)
     }
 
-    const withdrawal = await this.toLowLevelMessage(resolved)
+    const withdrawal = await this.toLowLevelMessage(resolved, messageIndex)
     const hash = hashLowLevelMessage(withdrawal)
     const messageSlot = hashMessageHash(hash)
 
@@ -1734,9 +1809,14 @@ export class CrossChainMessenger {
       messageGasLimit: NumberLike,
       opts?: {
         overrides?: Overrides
-      }
+      },
+
+      /**
+       * The index of the withdrawal if multiple are made with multicall
+       */
+      messageIndex = 0
     ): Promise<TransactionRequest> => {
-      const resolved = await this.toCrossChainMessage(message)
+      const resolved = await this.toCrossChainMessage(message, messageIndex)
       if (resolved.direction === MessageDirection.L2_TO_L1) {
         throw new Error(`cannot resend L2 to L1 message`)
       }
@@ -1780,9 +1860,14 @@ export class CrossChainMessenger {
       message: MessageLike,
       opts?: {
         overrides?: PayableOverrides
-      }
+      },
+
+      /**
+       * The index of the withdrawal if multiple are made with multicall
+       */
+      messageIndex = 0
     ): Promise<TransactionRequest> => {
-      const resolved = await this.toCrossChainMessage(message)
+      const resolved = await this.toCrossChainMessage(message, messageIndex)
       if (resolved.direction === MessageDirection.L1_TO_L2) {
         throw new Error('cannot finalize L1 to L2 message')
       }
@@ -1793,8 +1878,8 @@ export class CrossChainMessenger {
         )
       }
 
-      const withdrawal = await this.toLowLevelMessage(resolved)
-      const proof = await this.getBedrockMessageProof(resolved)
+      const withdrawal = await this.toLowLevelMessage(resolved, messageIndex)
+      const proof = await this.getBedrockMessageProof(resolved, messageIndex)
 
       const args = [
         [
@@ -1835,15 +1920,20 @@ export class CrossChainMessenger {
       message: MessageLike,
       opts?: {
         overrides?: PayableOverrides
-      }
+      },
+
+      /**
+       * The index of the withdrawal if multiple are made with multicall
+       */
+      messageIndex = 0
     ): Promise<TransactionRequest> => {
-      const resolved = await this.toCrossChainMessage(message)
+      const resolved = await this.toCrossChainMessage(message, messageIndex)
       if (resolved.direction === MessageDirection.L1_TO_L2) {
         throw new Error(`cannot finalize L1 to L2 message`)
       }
 
       if (this.bedrock) {
-        const withdrawal = await this.toLowLevelMessage(resolved)
+        const withdrawal = await this.toLowLevelMessage(resolved, messageIndex)
         return this.contracts.l1.OptimismPortal.populateTransaction.finalizeWithdrawalTransaction(
           [
             withdrawal.messageNonce,
@@ -1859,7 +1949,7 @@ export class CrossChainMessenger {
         // L1CrossDomainMessenger relayMessage is the only method that isn't fully backwards
         // compatible, so we need to use the legacy interface. When we fully upgrade to Bedrock we
         // should be able to remove this code.
-        const proof = await this.getMessageProof(resolved)
+        const proof = await this.getMessageProof(resolved, messageIndex)
         const legacyL1XDM = new ethers.Contract(
           this.contracts.l1.L1CrossDomainMessenger.address,
           getContractInterface('L1CrossDomainMessenger'),
@@ -2116,10 +2206,14 @@ export class CrossChainMessenger {
       message: MessageLike,
       opts?: {
         overrides?: CallOverrides
-      }
+      },
+      /**
+       * The index of the withdrawal if multiple are made with multicall
+       */
+      messageIndex = 0
     ): Promise<BigNumber> => {
       return this.l1Provider.estimateGas(
-        await this.populateTransaction.proveMessage(message, opts)
+        await this.populateTransaction.proveMessage(message, opts, messageIndex)
       )
     },
 

packages/sdk/test/cross-chain-messenger.spec.ts

@@ -565,23 +565,25 @@ describe('CrossChainMessenger', () => {
       })
 
       describe('when the transaction sent more than one message', () => {
-        it('should throw an error', async () => {
+        it('should be able to get second message by passing in an idex', async () => {
           const messages = [...Array(2)].map(() => {
             return DUMMY_MESSAGE
           })
 
           const tx = await l1Messenger.triggerSentMessageEvents(messages)
-          await expect(messenger.toCrossChainMessage(tx)).to.be.rejectedWith(
-            'expected 1 message, got 2'
+          const foundCrossChainMessages =
+            await messenger.getMessagesByTransaction(tx)
+          expect(await messenger.toCrossChainMessage(tx, 1)).to.deep.eq(
+            foundCrossChainMessages[1]
           )
         })
       })
 
       describe('when the transaction sent no messages', () => {
-        it('should throw an error', async () => {
+        it('should throw an out of bounds error', async () => {
           const tx = await l1Messenger.triggerSentMessageEvents([])
           await expect(messenger.toCrossChainMessage(tx)).to.be.rejectedWith(
-            'expected 1 message, got 0'
+            `withdrawal index 0 out of bounds. There are 0 withdrawals`
           )
         })
       })