Merge pull request #429 from kleros/fix/message-encoding

mani99brar · web-flow · commit ffb594f582de · 2025-08-27T09:50:51.000+05:30
Fix/message encoding
diff --git a/contracts/src/arbitrumToEth/VeaInboxArbToEth.sol b/contracts/src/arbitrumToEth/VeaInboxArbToEth.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-/// @custom:authors: [@jaybuidl, @shotaronowhere]
+/// @custom:authors: [@jaybuidl, @mani99brar, @shotaronowhere]
 /// @custom:reviewers: []
 /// @custom:auditors: []
 /// @custom:bounties: []
@@ -70,26 +70,16 @@ contract VeaInboxArbToEth is IVeaInbox {
     ///      Amortized cost is constant.
     /// Note: See docs for details how inbox manages merkle tree state.
     /// @param _to The address of the contract on the receiving chain which receives the calldata.
-    /// @param _fnSelector The function selector of the receiving contract.
-    /// @param _data The message calldata, abi.encode(param1, param2, ...)
+    /// @param _data The message data for the receiving gateway.
     /// @return msgId The zero based index of the message in the inbox.
-    function sendMessage(address _to, bytes4 _fnSelector, bytes memory _data) external override returns (uint64) {
+    function sendMessage(address _to, bytes calldata _data) external override returns (uint64) {
         uint64 oldCount = count;
 
         // Given arbitrum's speed limit of 7 million gas / second, it would take atleast 8 million years of full blocks to overflow.
         // It *should* be impossible to overflow, but we check to be safe when appending to the tree.
         require(oldCount < type(uint64).max, "Inbox is full.");
 
-        bytes memory nodeData = abi.encodePacked(
-            oldCount,
-            _to,
-            // _data is abi.encode(param1, param2, ...), we need to encode it again to get the correct leaf data
-            abi.encodePacked( // equivalent to abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-                _fnSelector,
-                bytes32(uint256(uint160(msg.sender))), // big endian padded encoding of msg.sender, simulating abi.encodeWithSelector
-                _data
-            )
-        );
+        bytes memory nodeData = abi.encodePacked(oldCount, _to, msg.sender, _data);
 
         // single hashed leaf
         bytes32 newInboxNode = keccak256(nodeData);
diff --git a/contracts/src/arbitrumToEth/VeaOutboxArbToEth.sol b/contracts/src/arbitrumToEth/VeaOutboxArbToEth.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-/// @custom:authors: [@jaybuidl, @shotaronowhere]
+/// @custom:authors: [@jaybuidl, @mani99brar, @shotaronowhere]
 /// @custom:reviewers: []
 /// @custom:auditors: []
 /// @custom:bounties: []
@@ -12,6 +12,7 @@ import "../canonical/arbitrum/ISequencerInbox.sol";
 import "../canonical/arbitrum/IBridge.sol";
 import "../canonical/arbitrum/IOutbox.sol";
 import "../interfaces/outboxes/IVeaOutboxOnL1.sol";
+import "../interfaces/gateways/IReceiverGateway.sol";
 
 /// @dev Vea Outbox From Arbitrum to Ethereum.
 /// Note: This contract is deployed on Ethereum.
@@ -355,11 +356,18 @@ contract VeaOutboxArbToEth is IVeaOutboxOnL1 {
     /// @param _proof The merkle proof to prove the message inclusion in the inbox state root.
     /// @param _msgId The zero based index of the message in the inbox.
     /// @param _to The address of the contract on Ethereum to call.
-    /// @param _message The message encoded in the vea inbox as abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-    function sendMessage(bytes32[] calldata _proof, uint64 _msgId, address _to, bytes calldata _message) external {
+    /// @param _from The address of the contract on Arbitrum that sent the message.
+    /// @param _message The message in the vea inbox.
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
 
-        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _message));
+        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _from, _message));
 
         // double hashed leaf
         // avoids second order preimage attacks
@@ -407,8 +415,7 @@ contract VeaOutboxArbToEth is IVeaOutboxOnL1 {
         relayed[relayIndex] = replay | bytes32(1 << offset);
 
         // UNTRUSTED.
-        (bool success, ) = _to.call(_message);
-        require(success, "Failed to call contract");
+        IReceiverGateway(_to).receiveMessage(_from, _message);
 
         emit MessageRelayed(_msgId);
     }
@@ -477,7 +484,7 @@ contract VeaOutboxArbToEth is IVeaOutboxOnL1 {
             } else {
                 address challenger = _claim.challenger;
                 _claim.challenger = address(0);
-                claimHashes[_epoch] == hashClaim(_claim);
+                claimHashes[_epoch] = hashClaim(_claim);
                 payable(challenger).send(deposit); // User is responsible for accepting ETH.
             }
         }
diff --git a/contracts/src/arbitrumToGnosis/VeaInboxArbToGnosis.sol b/contracts/src/arbitrumToGnosis/VeaInboxArbToGnosis.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-/// @custom:authors: [@jaybuidl, @shotaronowhere]
+/// @custom:authors: [@jaybuidl, @mani99brar, @shotaronowhere]
 /// @custom:reviewers: []
 /// @custom:auditors: []
 /// @custom:bounties: []
@@ -69,27 +69,17 @@ contract VeaInboxArbToGnosis is IVeaInbox {
     ///      `O(log(count))` where count is the number of messages already sent.
     ///      Amortized cost is constant.
     /// Note: See docs for details how inbox manages merkle tree state.
-    /// @param _to The address of the contract on the receiving chain which receives the calldata.
-    /// @param _fnSelector The function selector of the receiving contract.
-    /// @param _data The message calldata, abi.encode(param1, param2, ...)
+    /// @param _to The address of the contract on the receiving chain which receives the message.
+    /// @param _data The message data for the receiving gateway.
     /// @return msgId The zero based index of the message in the inbox.
-    function sendMessage(address _to, bytes4 _fnSelector, bytes memory _data) external override returns (uint64) {
+    function sendMessage(address _to, bytes calldata _data) external override returns (uint64) {
         uint64 oldCount = count;
 
         // Given arbitrum's speed limit of 7 million gas / second, it would take atleast 8 million years of full blocks to overflow.
         // It *should* be impossible to overflow, but we check to be safe when appending to the tree.
         require(oldCount < type(uint64).max, "Inbox is full.");
 
-        bytes memory nodeData = abi.encodePacked(
-            oldCount,
-            _to,
-            // _data is abi.encode(param1, param2, ...), we need to encode it again to get the correct leaf data
-            abi.encodePacked( // equivalent to abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-                _fnSelector,
-                bytes32(uint256(uint160(msg.sender))), // big endian padded encoding of msg.sender, simulating abi.encodeWithSelector
-                _data
-            )
-        );
+        bytes memory nodeData = abi.encodePacked(oldCount, _to, msg.sender, _data);
 
         // single hashed leaf
         bytes32 newInboxNode = keccak256(nodeData);
diff --git a/contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol b/contracts/src/arbitrumToGnosis/VeaOutboxArbToGnosis.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-/// @custom:authors: [@jaybuidl, @shotaronowhere]
+/// @custom:authors: [@jaybuidl, @mani99brar, @shotaronowhere]
 /// @custom:reviewers: []
 /// @custom:auditors: []
 /// @custom:bounties: []
@@ -12,6 +12,7 @@ import "../canonical/gnosis-chain/IAMB.sol";
 import "../interfaces/outboxes/IVeaOutboxOnL1.sol";
 import "../interfaces/updaters/ISequencerDelayUpdatable.sol";
 import "../interfaces/tokens/gnosis/IWETH.sol";
+import "../interfaces/gateways/IReceiverGateway.sol";
 
 /// @dev Vea Outbox From Arbitrum to Gnosis.
 /// Note: This contract is deployed on Gnosis.
@@ -299,11 +300,18 @@ contract VeaOutboxArbToGnosis is IVeaOutboxOnL1, ISequencerDelayUpdatable {
     /// @param _proof The merkle proof to prove the message inclusion in the inbox state root.
     /// @param _msgId The zero based index of the message in the inbox.
     /// @param _to The address of the contract on Gnosis to call.
-    /// @param _message The message encoded in the vea inbox as abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-    function sendMessage(bytes32[] calldata _proof, uint64 _msgId, address _to, bytes calldata _message) external {
+    /// @param _from The address of the contract on Arbitrum that sent the message.
+    /// @param _message The message in the vea inbox
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
 
-        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _message));
+        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _from, _message));
 
         // double hashed leaf
         // avoids second order preimage attacks
@@ -351,8 +359,7 @@ contract VeaOutboxArbToGnosis is IVeaOutboxOnL1, ISequencerDelayUpdatable {
         relayed[relayIndex] = replay | bytes32(1 << offset);
 
         // UNTRUSTED.
-        (bool success, ) = _to.call(_message);
-        require(success, "Failed to call contract");
+        IReceiverGateway(_to).receiveMessage(_from, _message);
 
         emit MessageRelayed(_msgId);
     }
@@ -421,7 +428,7 @@ contract VeaOutboxArbToGnosis is IVeaOutboxOnL1, ISequencerDelayUpdatable {
             } else {
                 address challenger = _claim.challenger;
                 _claim.challenger = address(0);
-                claimHashes[_epoch] == hashClaim(_claim);
+                claimHashes[_epoch] = hashClaim(_claim);
                 require(weth.transfer(challenger, deposit), "Failed WETH transfer."); // should revert on errors, but we check return value anyways
             }
         }
diff --git a/contracts/src/gnosisToArbitrum/VeaInboxGnosisToArb.sol b/contracts/src/gnosisToArbitrum/VeaInboxGnosisToArb.sol
@@ -70,27 +70,17 @@ contract VeaInboxGnosisToArb is IVeaInbox {
     ///      `O(log(count))` where count is the number of messages already sent.
     ///      Amortized cost is constant.
     /// Note: See docs for details how inbox manages merkle tree state.
-    /// @param _to The address of the contract on the receiving chain which receives the calldata.
-    /// @param _fnSelector The function selector of the receiving contract.
-    /// @param _data The message calldata, abi.encode(param1, param2, ...)
+    /// @param _to The address of the contract on the receiving chain which receives the message.
+    /// @param _data The message data for the receiving gateway.
     /// @return msgId The zero based index of the message in the inbox.
-    function sendMessage(address _to, bytes4 _fnSelector, bytes memory _data) external override returns (uint64) {
+    function sendMessage(address _to, bytes calldata _data) external override returns (uint64) {
         uint64 oldCount = count;
 
         // Given arbitrum's speed limit of 7 million gas / second, it would take atleast 8 million years of full blocks to overflow.
         // It *should* be impossible to overflow, but we check to be safe when appending to the tree.
         require(oldCount < type(uint64).max, "Inbox is full.");
 
-        bytes memory nodeData = abi.encodePacked(
-            oldCount,
-            _to,
-            // _data is abi.encode(param1, param2, ...), we need to encode it again to get the correct leaf data
-            abi.encodePacked( // equivalent to abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-                _fnSelector,
-                bytes32(uint256(uint160(msg.sender))), // big endian padded encoding of msg.sender, simulating abi.encodeWithSelector
-                _data
-            )
-        );
+        bytes memory nodeData = abi.encodePacked(oldCount, _to, msg.sender, _data);
 
         // single hashed leaf
         bytes32 newInboxNode = keccak256(nodeData);
diff --git a/contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol b/contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol
@@ -10,6 +10,7 @@ pragma solidity ^0.8.24;
 
 import "../interfaces/outboxes/IVeaOutboxOnL2.sol";
 import "../canonical/arbitrum/AddressAliasHelper.sol";
+import "../interfaces/gateways/IReceiverGateway.sol";
 
 /// @dev Vea Outbox From Gnosis to Arbitrum.
 /// Note: This contract is deployed on Arbitrum.
@@ -285,11 +286,18 @@ contract VeaOutboxGnosisToArb is IVeaOutboxOnL2 {
     /// @param _proof The merkle proof to prove the message inclusion in the inbox state root.
     /// @param _msgId The zero based index of the message in the inbox.
     /// @param _to The address of the contract on Arbitrum to call.
-    /// @param _message The message encoded in the vea inbox as abi.encodeWithSelector(fnSelector, msg.sender, param1, param2, ...)
-    function sendMessage(bytes32[] memory _proof, uint64 _msgId, address _to, bytes memory _message) external {
+    /// @param _from The address of the message sender
+    /// @param _message The message in the vea inbox
+    function sendMessage(
+        bytes32[] memory _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes memory _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
 
-        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _message));
+        bytes32 nodeHash = keccak256(abi.encodePacked(_msgId, _to, _from, _message));
 
         // double hashed leaf
         // avoids second order preimage attacks
@@ -337,8 +345,7 @@ contract VeaOutboxGnosisToArb is IVeaOutboxOnL2 {
         relayed[relayIndex] = replay | bytes32(1 << offset);
 
         // UNTRUSTED.
-        (bool success, ) = _to.call(_message);
-        require(success, "Failed to call contract");
+        IReceiverGateway(_to).receiveMessage(_from, _message);
 
         emit MessageRelayed(_msgId);
     }
diff --git a/contracts/src/interfaces/gateways/IReceiverGateway.sol b/contracts/src/interfaces/gateways/IReceiverGateway.sol
@@ -12,4 +12,6 @@ interface IReceiverGateway {
     function veaOutbox() external view returns (address);
 
     function senderGateway() external view returns (address);
+
+    function receiveMessage(address msgSender, bytes calldata msgData) external;
 }
diff --git a/contracts/src/interfaces/inboxes/IVeaInbox.sol b/contracts/src/interfaces/inboxes/IVeaInbox.sol
@@ -11,11 +11,10 @@ pragma solidity ^0.8.24;
 interface IVeaInbox {
     /// @dev Sends an arbitrary message to receiving chain.
     /// Note: Calls authenticated by receiving gateway checking the sender argument.
-    /// @param _to The cross-domain contract address which receives the calldata.
-    /// @param _fnSelection The function selector of the receiving contract.
-    /// @param _data The message calldata, abi.encode(...)
+    /// @param _to The cross-domain contract address which receives the message.
+    /// @param _data The message data.
     /// @return msgId The index of the message in the inbox, as a message Id, needed to relay the message.
-    function sendMessage(address _to, bytes4 _fnSelection, bytes memory _data) external returns (uint64 msgId);
+    function sendMessage(address _to, bytes calldata _data) external returns (uint64 msgId);
 
     /// @dev Snapshots can be saved a maximum of once per epoch.
     ///      Saves snapshot of state root.
diff --git a/contracts/src/interfaces/outboxes/IVeaOutboxOnL1.sol b/contracts/src/interfaces/outboxes/IVeaOutboxOnL1.sol
@@ -11,14 +11,21 @@ pragma solidity ^0.8.24;
 import "../types/VeaClaim.sol";
 
 /// @dev Interface of the Vea Outbox on L1 chains like Ethereum, Gnosis, Polygon POS where storage is expensive.
+
 interface IVeaOutboxOnL1 {
     /// @dev Verifies and relays the message.
-    /// Note: Gateways expect first argument of message call to be the arbitrum message sender, used for authentication.
     /// @param _proof The merkle proof to prove the message.
     /// @param _msgId The zero based index of the message in the inbox.
     /// @param _to The address to send the message to.
+    /// @param _from The address of the contract on L2 that sent the message, used for authentication.
     /// @param _message The message to relay.
-    function sendMessage(bytes32[] calldata _proof, uint64 _msgId, address _to, bytes calldata _message) external;
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external;
 
     /// @dev Resolves any challenge of the optimistic claim for 'epoch' using the canonical bridge.
     /// Note: Access restricted to canonical bridge.
diff --git a/contracts/src/interfaces/outboxes/IVeaOutboxOnL2.sol b/contracts/src/interfaces/outboxes/IVeaOutboxOnL2.sol
@@ -15,8 +15,15 @@ interface IVeaOutboxOnL2 {
     /// @param _proof The merkle proof to prove the message.
     /// @param _msgId The zero based index of the message in the inbox.
     /// @param _to The address to send the message to.
+    /// @param _from The address of the message sender.
     /// @param _message The message to relay.
-    function sendMessage(bytes32[] calldata _proof, uint64 _msgId, address _to, bytes calldata _message) external;
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external;
 
     /// @dev Resolves any challenge of the optimistic claim for 'epoch' using the canonical bridge.
     /// Note: Access restricted to canonical bridge.
diff --git a/contracts/src/test/gateways/IReceiverGatewayMock.sol b/contracts/src/test/gateways/IReceiverGatewayMock.sol
@@ -12,5 +12,8 @@ import "../../interfaces/gateways/IReceiverGateway.sol";
 
 interface IReceiverGatewayMock is IReceiverGateway {
     /// Receive the message from the sender gateway.
-    function receiveMessage(address msgSender, uint256 _data) external;
+    function digestMessage(uint256 _data) external;
+
+    /// Receive the message array from the sender gateway.
+    function digestMessageArray(uint256[] calldata _data) external;
 }
diff --git a/contracts/src/test/gateways/ReceiverGatewayMock.sol b/contracts/src/test/gateways/ReceiverGatewayMock.sol
@@ -9,6 +9,7 @@
 pragma solidity ^0.8.24;
 
 import "./IReceiverGatewayMock.sol";
+import "../../interfaces/outboxes/IVeaOutboxOnL1.sol";
 
 /// Receiver Gateway Mock
 /// Counterpart of `SenderGatewayMock`
@@ -18,37 +19,40 @@ contract ReceiverGatewayMock is IReceiverGatewayMock {
 
     uint256 public messageCount;
     uint256 public data;
+    uint256[] public dataArray;
 
     constructor(address _veaOutbox, address _senderGateway) {
         veaOutbox = _veaOutbox;
         senderGateway = _senderGateway;
     }
 
-    modifier onlyFromAuthenticatedVeaSender(address messageSender) {
+    modifier onlyFromVeaBridge(address msgSender) {
         require(veaOutbox == msg.sender, "Vea Bridge only.");
-        require(messageSender == senderGateway, "Only the sender gateway is allowed.");
+        require(senderGateway == msgSender, "Sender gateway mismatch.");
         _;
     }
 
-    /// Receive the message from the sender gateway.
-    function receiveMessage(address messageSender) external onlyFromAuthenticatedVeaSender(messageSender) {
-        _receiveMessage();
+    modifier internalCall() {
+        require(msg.sender == address(this), "Internal call only.");
+        _;
     }
 
     /// Receive the message from the sender gateway.
-    function receiveMessage(
-        address messageSender,
-        uint256 _data
-    ) external onlyFromAuthenticatedVeaSender(messageSender) {
-        _receiveMessage(_data);
+    function receiveMessage(address msgSender, bytes calldata data) external override onlyFromVeaBridge(msgSender) {
+        // Internal call to this contract with the provided data
+        (bool success, ) = address(this).call(data);
+        require(success, "Internal call failed");
     }
 
-    function _receiveMessage() internal {
+    /// @dev Only callable via internal call from receiveMessage
+    function digestMessage(uint256 _data) external override internalCall {
         messageCount++;
+        data = _data;
     }
 
-    function _receiveMessage(uint256 _data) internal {
+    /// @dev Only callable via internal call from receiveMessage
+    function digestMessageArray(uint256[] calldata _data) external override internalCall {
         messageCount++;
-        data = _data;
+        dataArray = _data;
     }
 }
diff --git a/contracts/src/test/gateways/SenderGatewayMock.sol b/contracts/src/test/gateways/SenderGatewayMock.sol
diff --git a/contracts/src/utils/veaInboxTouch.sol b/contracts/src/utils/veaInboxTouch.sol
diff --git a/contracts/test/integration/ArbToEth.ts b/contracts/test/integration/ArbToEth.ts
diff --git a/contracts/test/integration/ArbToGnosis.ts b/contracts/test/integration/ArbToGnosis.ts

Original file line number	Diff line number	Diff line change
`@@ -12,4 +12,6 @@ interface IReceiverGateway {`
`12`	`12`	`function veaOutbox() external view returns (address);`
`13`	`13`
`14`	`14`	`function senderGateway() external view returns (address);`
	`15`	`+`
	`16`	`+ function receiveMessage(address msgSender, bytes calldata msgData) external;`
`15`	`17`	`}`