Arb l1 to l2 hook and ISM #3106

exp7l · 2023-12-28T03:00:28Z

Description

fixes #2845:

This implements a hook that calls (L1) Arbitrum's Inbox's createRetryableTicket and provides a quoting method for message ID dispatching.
This also implements an ISM that mostly follows OPStackISM. Specifically, provided the ISM is the message originator (i.e. the address that creates the Retryable ticket on L1), ISM escrows any ETH sent and verifies provided message ID. When relayer relays the message later, the escrowed ETH is released and message is relayed to recipient.
Implementation includes deploy scripts for deploying to testnet or any network if you edit the network settings.

Testing

Made a dispatch call on Sepolia, checked rollup node picked up the Retryable ticket and sent to Arbitrum Sepolia .
- Note because Mailbox isn't deployed on Arbitrum Sepolia, I didn't try relaying the message. But because the ISM is almost the same as OpStackISM, the dispatch call test should be enough.
Local test against dispatch implemented in solidity/test/isms/ArbitrumOrbitIsm.t.sol

changeset-bot · 2023-12-28T03:00:31Z

⚠️ No Changeset found

Latest commit: 2d9f3ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

solidity/contracts/script/ArbitrumDispatcher.s.sol

+    function _dispatch() private {
+        vm.createSelectFork("sepolia");
+        vm.startBroadcast(sk);
+        bytes memory metadata = StandardHookMetadata.formatMetadata(
+            TEST_MSG_VALUE,
+            GAS_LIMIT,
+            sender,
+            abi.encodePacked(MAX_FEE_PER_GAS)
+        );
+        uint256 deposit = ArbitrumOrbitHook(ARBHOOK).quoteDispatch(
+            metadata,
+            TEST_MESSAGE
+        );
+        IMailbox(MAILBOX).dispatch{
+            value: deposit + MAX_FEE_PER_GAS * GAS_LIMIT
+        }(
+            ARBITRUM_DOMAIN,
+            TypeCasts.addressToBytes32(address(testRecipient)),
+            TEST_MESSAGE,
+            metadata,
+            ArbitrumOrbitHook(ARBHOOK)
+        );
+        vm.stopBroadcast();
+    }


solidity/contracts/script/ArbitrumDispatcher.s.sol

+    function _dispatch() private {
+        vm.createSelectFork("sepolia");
+        vm.startBroadcast(sk);
+        bytes memory metadata = StandardHookMetadata.formatMetadata(
+            TEST_MSG_VALUE,
+            GAS_LIMIT,
+            sender,
+            abi.encodePacked(MAX_FEE_PER_GAS)
+        );
+        uint256 deposit = ArbitrumOrbitHook(ARBHOOK).quoteDispatch(
+            metadata,
+            TEST_MESSAGE
+        );
+        IMailbox(MAILBOX).dispatch{
+            value: deposit + MAX_FEE_PER_GAS * GAS_LIMIT
+        }(
+            ARBITRUM_DOMAIN,
+            TypeCasts.addressToBytes32(address(testRecipient)),
+            TEST_MESSAGE,
+            metadata,
+            ArbitrumOrbitHook(ARBHOOK)
+        );
+        vm.stopBroadcast();
+    }


solidity/contracts/script/ArbitrumL2Deployer.s.sol

+    function run() external {
+        vm.createSelectFork("sepolia_arb");
+        string memory seed = vm.envString("SEEDPHRASE");
+        vm.startBroadcast(vm.deriveKey(seed, 0));
+        new ArbitrumOrbitIsm();
+        vm.stopBroadcast();
+    }


solidity/contracts/script/ArbitrumDispatcher.s.sol

+    function run() external {
+        vm.createSelectFork("sepolia_arb");
+        vm.startBroadcast(sk);
+        testRecipient = new TestRecipient();
+        ArbitrumOrbitIsm(ARBISM).setAuthorizedHook(
+            TypeCasts.addressToBytes32(ARBHOOK)
+        );
+        vm.stopBroadcast();
+        _dispatch();
+    }


solidity/contracts/script/ArbitrumL1Deployer.s.sol

solidity/contracts/hooks/ArbitrumOrbitHook.sol

solidity/contracts/script/ArbitrumDispatcher.s.sol

yorhodes

great start! I like how you reused the existing abstractions

solidity/test/isms/ArbitrumOrbitIsm.t.sol

yorhodes · 2024-01-02T18:34:41Z

solidity/contracts/hooks/ArbitrumOrbitHook.sol

+     * [2:34] msg.value
+     * [34:66] Gas limit for message
+     * [66:86] Refund address for message
+     * [86:117] Max fee per gas (custom)


it would be good to have a default here otherwise this is quite a breaking change for developers

See comment below.

yorhodes · 2024-01-02T18:35:41Z

solidity/contracts/hooks/ArbitrumOrbitHook.sol

+        require(
+            metadata.msgValue(0) < 2 ** 255,
+            "ArbitrumOrbitHook: msgValue must be less than 2 ** 255"
+        );
+        // To make sure the default value for each meta datum IS NOT used.
+        require(
+            metadata.length >= MIN_METADATA_LENGTH,
+            "ArbitrumOrbitHook: invalid metadata length"
+        );


I wonder if we should just move these checks into the AbstractMessageIdAuthHook

Agreed, moved msg value check into AbstractMessageIdAuthHook and changed the OPStackhook as a drive-by change.

To make UX better, I made max fee per gas an optional metadata, defaulted to use IGP's gas price (like you suggested below) unless user provides max fee per gas through metadata. So the length check shouldn't be needed anymore.

yorhodes · 2024-01-02T18:36:38Z

solidity/contracts/hooks/ArbitrumOrbitHook.sol

+        uint256 ticketID = baseInbox.createRetryableTicket{value: msg.value}(
+            TypeCasts.bytes32ToAddress(ism),
+            metadata.msgValue(0),
+            baseInbox.calculateRetryableSubmissionFee(PAYLOAD_LENGTH, 0),


should PAYLOAD_LENGTH actually be derived from message.length?

and can we dedupe this with quoteDispatch somehow

Ah if I understand correctly, PAYLOAD_LENGTH is always 36 bytes because the payload is the verifyMessageId call to the ISM: the calldata has 4 bytes for method ID and 32 bytes for message ID.

Deduped, please take a look.

yorhodes · 2024-01-02T18:38:06Z

solidity/contracts/hooks/ArbitrumOrbitHook.sol

+        uint256 maxFeePerGas = abi.decode(
+            metadata.getCustomMetadata(),
+            (uint256)
+        );


the InterchainGasPaymaster has oracleized gas prices which maybe could be reused here, or added to the "standard metadata" shape

avious00 · 2024-01-25T15:52:57Z

hey @exp7l - as Yorke shared, this is a great start on a much needed hook. Are you stuck on anything? How can we help?

exp7l · 2024-01-29T05:04:02Z

@avious00 my apologies, I'm moving these few weeks and got distracted. I should be able to come back to this pr in the coming week. Thanks for the patience.

yorhodes · 2024-02-01T15:18:51Z

@avious00 my apologies, I'm moving these few weeks and got distracted. I should be able to come back to this pr in the coming week. Thanks for the patience.

Any updates here?

exp7l · 2024-02-01T16:58:52Z

@avious00 my apologies, I'm moving these few weeks and got distracted. I should be able to come back to this pr in the coming week. Thanks for the patience.

Any updates here?

looking now

exp7l · 2024-02-02T04:07:48Z

Added some changes and comments above, what do you think? @yorhodes

exp7l · 2024-02-08T20:47:59Z

@yorhodes @avious00 hi any updates?

yorhodes

almost there, nice work!

solidity/contracts/isms/hook/ArbitrumOrbitIsm.sol

solidity/contracts/hooks/ArbitrumOrbitHook.sol

solidity/contracts/script/ArbitrumDispatcher.s.sol

solidity/contracts/script/ArbitrumL1Deployer.s.sol

solidity/contracts/script/ArbitrumL2Deployer.s.sol

avious00 · 2024-04-11T18:02:46Z

hey @exp7l you're pretty close - would be great to push this over the line since a lot of people have been asking

Co-authored-by: Yorke Rhodes <[email protected]>

This reverts commit 5381ff9 due to misconfigured yarn.lock

exp7l added 2 commits December 27, 2023 21:57

arb hook & ism: add npm deps

aa1abfb

feat: arb hook & ism

59db59f

exp7l added 3 commits December 27, 2023 22:05

arb hook & ism: format & test pragma

74ba97e

arb hook & ism: deploy scripts

4524192

arb hook & ism: clean

f1585a8

exp7l marked this pull request as ready for review December 29, 2023 05:17

exp7l requested review from yorhodes, tkporter and aroralanuk as code owners December 29, 2023 05:17

arb hook & ism: comment

350002d

exp7l changed the title ~~Arb l1 to l2~~ Arb l1 to l2 hook and ISM Dec 29, 2023

github-advanced-security bot found potential problems Jan 2, 2024

View reviewed changes

yorhodes reviewed Jan 2, 2024

View reviewed changes

arb l1->l2: refactor

81980be

yorhodes self-requested a review February 15, 2024 21:35

yorhodes requested changes Feb 15, 2024

View reviewed changes

exp7l mentioned this pull request Feb 20, 2024

Ethereum Sequencing & Preconfs Call #2 ethereum/pm#963

Closed

ltyu and others added 3 commits April 13, 2024 20:55

Update solidity/contracts/hooks/ArbitrumOrbitHook.sol

5381ff9

Co-authored-by: Yorke Rhodes <[email protected]>

Fix conflicts

d055518

Fix according to review

7614dbb

exp7l requested a review from nbayindirli as a code owner April 14, 2024 01:18

ltyu force-pushed the arb-l1-to-l2 branch from d267d8c to 51fd41d Compare April 14, 2024 01:51

ltyu force-pushed the arb-l1-to-l2 branch from 51fd41d to 47ae656 Compare April 14, 2024 01:52

Fix codespells

4a18b7d

ltyu force-pushed the arb-l1-to-l2 branch from 47ae656 to 4a18b7d Compare April 14, 2024 01:57

Revert "Update solidity/contracts/hooks/ArbitrumOrbitHook.sol"

2d9f3ae

This reverts commit 5381ff9 due to misconfigured yarn.lock

avious00 mentioned this pull request Apr 19, 2024

OpStack L2 => L1 Hook and ISM #2844

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arb l1 to l2 hook and ISM #3106

Arb l1 to l2 hook and ISM #3106

exp7l commented Dec 28, 2023 •

edited by avious00

Loading

changeset-bot bot commented Dec 28, 2023 •

edited

Loading

yorhodes left a comment

yorhodes Jan 2, 2024

exp7l Feb 1, 2024

yorhodes Jan 2, 2024

exp7l Feb 1, 2024

yorhodes Jan 2, 2024

yorhodes Jan 2, 2024

exp7l Feb 1, 2024

yorhodes Jan 2, 2024

exp7l Feb 1, 2024

avious00 commented Jan 25, 2024

exp7l commented Jan 29, 2024

yorhodes commented Feb 1, 2024

exp7l commented Feb 1, 2024

exp7l commented Feb 2, 2024 •

edited

Loading

exp7l commented Feb 8, 2024

yorhodes left a comment

avious00 commented Apr 11, 2024

Arb l1 to l2 hook and ISM #3106

Are you sure you want to change the base?

Arb l1 to l2 hook and ISM #3106

Conversation

exp7l commented Dec 28, 2023 • edited by avious00 Loading

Description

Testing

changeset-bot bot commented Dec 28, 2023 • edited Loading

⚠️ No Changeset found

yorhodes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avious00 commented Jan 25, 2024

exp7l commented Jan 29, 2024

yorhodes commented Feb 1, 2024

exp7l commented Feb 1, 2024

exp7l commented Feb 2, 2024 • edited Loading

exp7l commented Feb 8, 2024

yorhodes left a comment

Choose a reason for hiding this comment

avious00 commented Apr 11, 2024

exp7l commented Dec 28, 2023 •

edited by avious00

Loading

changeset-bot bot commented Dec 28, 2023 •

edited

Loading

exp7l commented Feb 2, 2024 •

edited

Loading