fix: `ArbL2ToL1Ism` will support msg.value (#4328)

### Description

- Added support for msg.value by rearranging the
`releaseValueToRecipient()` in the verify call

### Drive-by changes

- uncommented out
`test_postDispatch_revertWhen_notLastDispatchedMessage` test

### Related issues

<!--
- Fixes #[issue number here]
-->

### Backward compatibility

<!--
Are these changes backward compatible? Are there any infrastructure
implications, e.g. changes that would prohibit deploying older commits
using this infra tooling?

Yes/No
-->

### Testing

Unit tests

.changeset/stale-planets-dance.md

@@ -0,0 +1,6 @@
+---
+'@hyperlane-xyz/sdk': minor
+'@hyperlane-xyz/core': minor
+---
+
+ArbL2ToL1Ism handles value via the executeTransaction branch

solidity/contracts/isms/hook/ArbL2ToL1Ism.sol

@@ -61,11 +61,11 @@ contract ArbL2ToL1Ism is
         bytes calldata metadata,
         bytes calldata message
     ) external override returns (bool) {
-        bool verified = isVerified(message);
-        if (verified) {
-            releaseValueToRecipient(message);
+        if (!isVerified(message)) {
+            _verifyWithOutboxCall(metadata, message);
         }
-        return verified || _verifyWithOutboxCall(metadata, message);
+        releaseValueToRecipient(message);
+        return true;
     }
 
     // ============ Internal function ============
@@ -78,7 +78,7 @@ contract ArbL2ToL1Ism is
     function _verifyWithOutboxCall(
         bytes calldata metadata,
         bytes calldata message
-    ) internal returns (bool) {
+    ) internal {
         (
             bytes32[] memory proof,
             uint256 index,
@@ -87,6 +87,7 @@ contract ArbL2ToL1Ism is
             uint256 l2Block,
             uint256 l1Block,
             uint256 l2Timestamp,
+            uint256 value,
             bytes memory data
         ) = abi.decode(
                 metadata,
@@ -98,6 +99,7 @@ contract ArbL2ToL1Ism is
                     uint256,
                     uint256,
                     uint256,
+                    uint256,
                     bytes
                 )
             );
@@ -120,8 +122,6 @@ contract ArbL2ToL1Ism is
             convertedBytes == messageId,
             "ArbL2ToL1Ism: invalid message id"
         );
-
-        // value send to 0
         arbOutbox.executeTransaction(
             proof,
             index,
@@ -130,11 +130,9 @@ contract ArbL2ToL1Ism is
             l2Block,
             l1Block,
             l2Timestamp,
-            0,
+            value,
             data
         );
-        // the above bridge call will revert if the verifyMessageId call fails
-        return true;
     }
 
     /// @inheritdoc AbstractMessageIdAuthorizedIsm

solidity/test/isms/ArbL2ToL1Ism.t.sol

@@ -121,10 +121,10 @@ contract ArbL2ToL1IsmTest is Test {
     function test_postDispatch_revertWhen_notLastDispatchedMessage() public {
         deployAll();
 
-        // vm.expectRevert(
-        //     "AbstractMessageIdAuthHook: message not latest dispatched"
-        // );
-        // hook.postDispatch(testMetadata, encodedMessage);
+        vm.expectRevert(
+            "AbstractMessageIdAuthHook: message not latest dispatched"
+        );
+        hook.postDispatch(testMetadata, encodedMessage);
     }
 
     function test_verify_outboxCall() public {
@@ -133,11 +133,14 @@ contract ArbL2ToL1IsmTest is Test {
         bytes memory encodedOutboxTxMetadata = _encodeOutboxTx(
             address(hook),
             address(ism),
-            messageId
+            messageId,
+            1 ether
         );
 
+        vm.deal(address(arbBridge), 1 ether);
         arbBridge.setL2ToL1Sender(address(hook));
         assertTrue(ism.verify(encodedOutboxTxMetadata, encodedMessage));
+        assertEq(address(testRecipient).balance, 1 ether);
     }
 
     function test_verify_statefulVerify() public {
@@ -190,7 +193,8 @@ contract ArbL2ToL1IsmTest is Test {
         bytes memory encodedOutboxTxMetadata = _encodeOutboxTx(
             address(hook),
             address(ism),
-            messageId
+            messageId,
+            1 ether
         );
 
         vm.etch(address(arbBridge), new bytes(0)); // this is a way to test that the arbBridge isn't called again
@@ -211,10 +215,11 @@ contract ArbL2ToL1IsmTest is Test {
         bytes memory encodedOutboxTxMetadata = _encodeOutboxTx(
             address(this),
             address(ism),
-            messageId
+            messageId,
+            0
         );
 
-        arbBridge.setL2ToL1Sender(address(hook));
+        arbBridge.setL2ToL1Sender(address(this));
 
         vm.expectRevert("ArbL2ToL1Ism: l2Sender != authorizedHook");
         ism.verify(encodedOutboxTxMetadata, encodedMessage);
@@ -226,7 +231,8 @@ contract ArbL2ToL1IsmTest is Test {
         bytes memory encodedOutboxTxMetadata = _encodeOutboxTx(
             address(hook),
             address(this),
-            messageId
+            messageId,
+            0
         );
 
         arbBridge.setL2ToL1Sender(address(hook));
@@ -243,7 +249,8 @@ contract ArbL2ToL1IsmTest is Test {
         bytes memory encodedOutboxTxMetadata = _encodeOutboxTx(
             address(hook),
             address(ism),
-            incorrectMessageId
+            incorrectMessageId,
+            0
         );
 
         bytes memory encodedHookData = abi.encodeCall(
@@ -275,14 +282,13 @@ contract ArbL2ToL1IsmTest is Test {
         assertFalse(ism.verify(new bytes(0), encodedMessage));
     }
 
-    // function test_verify_withMsgValue
-
     /* ============ helper functions ============ */
 
     function _encodeOutboxTx(
         address _hook,
         address _ism,
-        bytes32 _messageId
+        bytes32 _messageId,
+        uint256 _value
     ) internal view returns (bytes memory) {
         bytes memory encodedHookData = abi.encodeCall(
             AbstractMessageIdAuthorizedIsm.verifyMessageId,
@@ -299,6 +305,7 @@ contract ArbL2ToL1IsmTest is Test {
                 MOCK_L2_BLOCK,
                 MOCK_L1_BLOCK,
                 block.timestamp,
+                _value,
                 encodedHookData
             );
     }

typescript/sdk/src/ism/metadata/arbL2ToL1.ts

@@ -25,7 +25,7 @@ import { MetadataBuilder, MetadataContext } from './builder.js';
 export type NitroChildToParentTransactionEvent = EventArgs<L2ToL1TxEvent>;
 export type ArbL2ToL1Metadata = Omit<
   NitroChildToParentTransactionEvent,
-  'hash' | 'callvalue'
+  'hash'
 > & {
   proof: BytesLike[]; // bytes32[16]
 };
@@ -226,9 +226,9 @@ export class ArbL2ToL1MetadataBuilder implements MetadataBuilder {
       outboxInterface.functions[
         'executeTransaction(bytes32[],uint256,address,address,uint256,uint256,uint256,uint256,bytes)'
       ].inputs;
-    const executeTransactionTypes = executeTransactionInputs
-      .map((input) => input.type)
-      .filter((_, index, array) => index !== array.length - 2); // remove callvalue from types (because the ArbL2ToL1Ism doesn't allow it)
+    const executeTransactionTypes = executeTransactionInputs.map(
+      (input) => input.type,
+    );
     return abiCoder.encode(executeTransactionTypes, [
       metadata.proof,
       metadata.position,
@@ -237,6 +237,7 @@ export class ArbL2ToL1MetadataBuilder implements MetadataBuilder {
       metadata.arbBlockNum,
       metadata.ethBlockNum,
       metadata.timestamp,
+      metadata.callvalue,
       metadata.data,
     ]);
   }