Fix/message encoding

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 calldata, abi.encodeWithSelector(fnSelector, param1, param2, ...)
     /// @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);

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: []
@@ -41,6 +41,7 @@ contract VeaOutboxArbToEth is IVeaOutboxOnL1 {
 
     mapping(uint256 epoch => bytes32) public claimHashes; // epoch => claim
     mapping(uint256 messageId => bytes32) internal relayed; // msgId/256 => packed replay bitmap, preferred over a simple boolean mapping to save 15k gas per message
+    mapping(address => mapping(address => bool)) public allowlist; // to => from => allowed, Enforces allowed sender addresses for a receiver contract. Address(0) allowing all senders.
 
     uint256 public sequencerDelayLimit; // This is MaxTimeVariation.delaySeconds from the arbitrum sequencer inbox, it is the maximum seconds the sequencer can backdate L2 txns relative to the L1 clock.
     SequencerDelayLimitDecreaseRequest public sequencerDelayLimitDecreaseRequest; // Decreasing the sequencerDelayLimit requires a delay to avoid griefing by sequencer, so we keep track of the request here.
@@ -351,15 +352,32 @@ contract VeaOutboxArbToEth is IVeaOutboxOnL1 {
         emit Verified(_epoch);
     }
 
+    /// @dev Sets the allowlist for the sender gateway.
+    /// Note: Address(0) is used to allow all addresses.
+    /// @param _from The address to allow or disallow
+    /// @param _allowed Whether to allow or disallow the address.
+    function setAllowlist(address _from, bool _allowed) external {
+        allowlist[msg.sender][_from] = _allowed;
+    }
+
     /// @dev Verifies and relays the message. UNTRUSTED.
     /// @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 as abi.encodeWithSelector(fnSelector, param1, param2, ...)
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
+        bool isAllowed = allowlist[_to][_from] || allowlist[_to][address(0)];
+        require(isAllowed, "Message sender not allowed to call receiver.");
 
-        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

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: []
@@ -70,26 +70,16 @@ contract VeaInboxArbToGnosis 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 calldata, abi.encodeWithSelector(fnSelector, param1, param2, ...)
     /// @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);

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: []
@@ -42,6 +42,7 @@ contract VeaOutboxArbToGnosis is IVeaOutboxOnL1, ISequencerDelayUpdatable {
 
     mapping(uint256 epoch => bytes32) public claimHashes; // epoch => claim
     mapping(uint256 messageId => bytes32) internal relayed; // msgId/256 => packed replay bitmap, preferred over a simple boolean mapping to save 15k gas per message
+    mapping(address => mapping(address => bool)) public allowlist; // to => from => allowed,  Enforces allowed sender addresses for a receiver contract. Address(0) allowing all senders.
 
     uint256 public sequencerDelayLimit; // This is MaxTimeVariation.delaySeconds from the arbitrum sequencer inbox, it is the maximum seconds the sequencer can backdate L2 txns relative to the L1 clock.
     uint256 public timestampDelayUpdated; // The timestamp of the last sequencer delay update.
@@ -295,15 +296,31 @@ contract VeaOutboxArbToGnosis is IVeaOutboxOnL1, ISequencerDelayUpdatable {
         emit Verified(_epoch);
     }
 
+    /// @dev Sets the allowlist for the sender gateway.
+    /// @param _from The address to allow or disallow
+    /// @param _allow Whether to allow or disallow the address.
+    function setAllowlist(address _from, bool _allow) external {
+        allowlist[msg.sender][_from] = _allow;
+    }
+
     /// @dev Verifies and relays the message. UNTRUSTED.
     /// @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 as abi.encodeWithSelector(fnSelector, param1, param2, ...)
+    function sendMessage(
+        bytes32[] calldata _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes calldata _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
+        bool isAllowed = allowlist[_to][_from] || allowlist[_to][address(0)];
+        require(isAllowed, "Message sender not allowed to call receiver.");
 
-        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

contracts/src/gnosisToArbitrum/VeaInboxGnosisToArb.sol

@@ -71,26 +71,16 @@ contract VeaInboxGnosisToArb 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 calldata, abi.encodeWithSelector(fnSelector, param1, param2, ...)
     /// @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);

contracts/src/gnosisToArbitrum/VeaOutboxGnosisToArb.sol

@@ -41,6 +41,7 @@ contract VeaOutboxGnosisToArb is IVeaOutboxOnL2 {
     mapping(uint256 epoch => Claim) public claims; // epoch => claim
     mapping(uint256 epoch => address) public challengers; // epoch => challenger
     mapping(uint256 messageId => bytes32) internal relayed; // msgId/256 => packed replay bitmap, preferred over a simple boolean mapping to save 15k gas per message
+    mapping(address => mapping(address => bool)) public allowlist; // to => from => allowed
 
     enum Party {
         None,
@@ -281,15 +282,32 @@ contract VeaOutboxGnosisToArb is IVeaOutboxOnL2 {
         emit Verified(_epoch);
     }
 
+    /// @dev Sets the allowlist for the sender gateway.
+    /// Note: Address(0) is used to allow all addresses.
+    /// @param _from The address to allow or disallow
+    /// @param _allowed Whether to allow or disallow the address.
+    function setAllowlist(address _from, bool _allowed) external {
+        allowlist[msg.sender][_from] = _allowed;
+    }
+
     /// @dev Verifies and relays the message. UNTRUSTED.
     /// @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 as abi.encodeWithSelector(fnSelector, param1, param2, ...)
+    function sendMessage(
+        bytes32[] memory _proof,
+        uint64 _msgId,
+        address _to,
+        address _from,
+        bytes memory _message
+    ) external {
         require(_proof.length < 64, "Proof too long.");
+        bool isAllowed = allowlist[_to][_from] || allowlist[_to][address(0)];
+        require(isAllowed, "Message sender not allowed to call receiver.");
 
-        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

contracts/src/interfaces/inboxes/IVeaInbox.sol

@@ -12,10 +12,9 @@ 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 _data The message calldata, abi.encodeWithSelector(...)
     /// @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.

contracts/src/interfaces/outboxes/IVeaOutboxOnL1.sol

@@ -11,19 +11,32 @@ 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.
     /// @param _epoch The epoch to verify.
     /// @param _stateRoot The true state root for the epoch.
     /// @param _claim The claim associated with the epoch.
     function resolveDisputedClaim(uint256 _epoch, bytes32 _stateRoot, Claim memory _claim) external;
+
+    /// @dev Sets the allowlist for the sender gateway.
+    /// Note: Address(0) is used to allow all addresses.
+    /// @param _from The address to allow or disallow
+    /// @param _allow Whether to allow or disallow the address.
+    function setAllowlist(address _from, bool _allow) external;
 }

contracts/src/interfaces/outboxes/IVeaOutboxOnL2.sol

@@ -15,12 +15,25 @@ 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.
     /// @param _epoch The epoch to verify.
     /// @param _stateRoot The true state root for the epoch.
     function resolveDisputedClaim(uint256 _epoch, bytes32 _stateRoot) external;
+
+    /// @dev Sets the allowlist for the sender gateway.
+    /// Note: Address(0) is used to allow all addresses.
+    /// @param _from The address to allow or disallow
+    /// @param _allow Whether to allow or disallow the address.
+    function setAllowlist(address _from, bool _allow) external;
 }

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 receiveMessage(uint256 _data) external;
+
+    /// Receive the message array from the sender gateway.
+    function receiveMessageArray(uint256[] calldata _data) external;
 }