feat(protocol): add message owner parameter to vault operations (#15770)

packages/protocol/contracts/L1/libs/LibProving.sol

@@ -153,10 +153,7 @@ library LibProving {
                 IVerifier.Context memory ctx = IVerifier.Context({
                     metaHash: blk.metaHash,
                     blobHash: meta.blobHash,
-                    // TODO(Brecht): Quite limiting this is required to be the same address as
-                    // msg.sender, less flexibility on the prover's side for proof generation/proof
-                    // submission using multiple accounts.
-                    // Added msgSender to allow the prover to be any address in the future.
+                    // Separate msgSender to allow the prover to be any address in the future.
                     prover: msg.sender,
                     msgSender: msg.sender,
                     blockId: blk.blockId,
@@ -319,8 +316,6 @@ library LibProving {
                 // In scenarios where this transition is not the first one, we
                 // straightforwardly reset the transition prover to address
                 // zero.
-                // TODO(Brecht): Is it sure that in all cases all the neccessary data is stored
-                // in the transition in this case after this code?
                 ts.prover = address(0);
 
                 // Furthermore, we index the transition for future retrieval.

packages/protocol/contracts/bridge/Bridge.sol

@@ -137,7 +137,9 @@ contract Bridge is EssentialContract, IBridge {
         returns (bytes32 msgHash, Message memory _message)
     {
         // Ensure the message owner is not null.
-        if (message.owner == address(0)) revert B_INVALID_USER();
+        if (message.srcOwner == address(0) || message.destOwner == address(0)) {
+            revert B_INVALID_USER();
+        }
 
         // Check if the destination chain is enabled.
         (bool destChainEnabled,) = isDestChainEnabled(message.destChainId);
@@ -227,7 +229,7 @@ contract Bridge is EssentialContract, IBridge {
                 // Reset the context after the message call
                 _resetContext();
             } else {
-                message.owner.sendEther(message.value);
+                message.srcOwner.sendEther(message.value);
             }
             emit MessageRecalled(msgHash);
         } else if (!isMessageProven) {
@@ -238,7 +240,7 @@ contract Bridge is EssentialContract, IBridge {
     }
 
     /// @notice Processes a bridge message on the destination chain. This
-    /// function is callable by any address, including the `message.owner`.
+    /// function is callable by any address, including the `message.destOwner`.
     /// @dev The process begins by hashing the message and checking the message
     /// status in the bridge  If the status is "NEW", the message is invoked. The
     /// status is updated accordingly, and processing fees are refunded as
@@ -254,10 +256,6 @@ contract Bridge is EssentialContract, IBridge {
         whenNotPaused
         sameChain(message.destChainId)
     {
-        // TODO(Brecht): `message.owner`, but this is the `msg.sender` on the source chain.
-        // If the address is not owned by the same entity on the destination chain
-        // (e.g. can be the case for smart wallets/general contracts) this can give unexpected
-        // results (especially with refunding).
         bytes32 msgHash = hashMessage(message);
         if (messageStatus[msgHash] != Status.NEW) revert B_STATUS_MISMATCH();
 
@@ -277,7 +275,7 @@ contract Bridge is EssentialContract, IBridge {
             if (invocationDelay != 0) {
                 proofReceipt[msgHash] = ProofReceipt({
                     receivedAt: receivedAt,
-                    preferredExecutor: message.gasLimit == 0 ? message.owner : msg.sender
+                    preferredExecutor: message.gasLimit == 0 ? message.destOwner : msg.sender
                 });
             }
         }
@@ -292,7 +290,7 @@ contract Bridge is EssentialContract, IBridge {
 
         if (block.timestamp >= invocationDelay + receivedAt) {
             // If the gas limit is set to zero, only the owner can process the message.
-            if (message.gasLimit == 0 && msg.sender != message.owner) {
+            if (message.gasLimit == 0 && msg.sender != message.destOwner) {
                 revert B_PERMISSION_DENIED();
             }
 
@@ -312,7 +310,7 @@ contract Bridge is EssentialContract, IBridge {
             } else {
                 // Use the specified message gas limit if called by the owner, else
                 // use remaining gas
-                uint256 gasLimit = msg.sender == message.owner ? gasleft() : message.gasLimit;
+                uint256 gasLimit = msg.sender == message.destOwner ? gasleft() : message.gasLimit;
 
                 if (_invokeMessageCall(message, msgHash, gasLimit)) {
                     _updateMessageStatus(msgHash, Status.DONE);
@@ -322,7 +320,7 @@ contract Bridge is EssentialContract, IBridge {
             }
 
             // Determine the refund recipient
-            address refundTo = message.refundTo == address(0) ? message.owner : message.refundTo;
+            address refundTo = message.refundTo == address(0) ? message.destOwner : message.refundTo;
 
             // Refund the processing fee
             if (msg.sender == refundTo) {
@@ -343,7 +341,7 @@ contract Bridge is EssentialContract, IBridge {
     /// @notice Retries to invoke the messageCall after releasing associated
     /// Ether and tokens.
     /// @dev This function can be called by any address, including the
-    /// `message.owner`.
+    /// `message.destOwner`.
     /// It attempts to invoke the messageCall and updates the message status
     /// accordingly.
     /// @param message The message to retry.
@@ -359,9 +357,9 @@ contract Bridge is EssentialContract, IBridge {
         sameChain(message.destChainId)
     {
         // If the gasLimit is set to 0 or isLastAttempt is true, the caller must
-        // be the message.owner.
+        // be the message.destOwner.
         if (message.gasLimit == 0 || isLastAttempt) {
-            if (msg.sender != message.owner) revert B_PERMISSION_DENIED();
+            if (msg.sender != message.destOwner) revert B_PERMISSION_DENIED();
         }
 
         bytes32 msgHash = hashMessage(message);

packages/protocol/contracts/bridge/IBridge.sol

@@ -27,11 +27,14 @@ interface IBridge {
         uint64 srcChainId;
         // Destination chain ID where the `to` address lives.
         uint64 destChainId;
-        // The owner of the message.
-        address owner;
+        // The owner of the message on the source chain.
+        address srcOwner;
+        // The owner of the message on the destination chain.
+        address destOwner;
         // The destination address on the destination chain.
         address to;
-        // Alternate address to send any refund. If blank, defaults to owner.
+        // Alternate address to send any refund on the destination chain.
+        // If blank, defaults to destOwner.
         address refundTo;
         // value to invoke on the destination chain.
         uint256 value;

packages/protocol/contracts/tokenvault/BaseNFTVault.sol

@@ -35,6 +35,8 @@ abstract contract BaseNFTVault is BaseVault {
     struct BridgeTransferOp {
         // Destination chain ID.
         uint64 destChainId;
+        // The owner of the bridge message on the destination chain.
+        address destOwner;
         // Recipient address.
         address to;
         // Address of the token.

packages/protocol/contracts/tokenvault/ERC1155Vault.sol

@@ -65,7 +65,8 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
         IBridge.Message memory message;
         message.destChainId = op.destChainId;
         message.data = data;
-        message.owner = msg.sender;
+        message.srcOwner = msg.sender;
+        message.destOwner = op.destOwner != address(0) ? op.destOwner : msg.sender;
         message.to = resolve(message.destChainId, name(), false);
         message.gasLimit = op.gasLimit;
         message.value = msg.value - op.fee;
@@ -81,7 +82,7 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
         // Emit TokenSent event
         emit TokenSent({
             msgHash: msgHash,
-            from: _message.owner,
+            from: _message.srcOwner,
             to: op.to,
             destChainId: _message.destChainId,
             ctoken: ctoken.addr,
@@ -152,13 +153,13 @@ contract ERC1155Vault is BaseNFTVault, ERC1155ReceiverUpgradeable {
             abi.decode(message.data[4:], (CanonicalNFT, address, address, uint256[], uint256[]));
 
         // Transfer the ETH and tokens back to the owner
-        address token = _transferTokens(ctoken, message.owner, tokenIds, amounts);
-        message.owner.sendEther(message.value);
+        address token = _transferTokens(ctoken, message.srcOwner, tokenIds, amounts);
+        message.srcOwner.sendEther(message.value);
 
         // Emit TokenReleased event
         emit TokenReleased({
             msgHash: msgHash,
-            from: message.owner,
+            from: message.srcOwner,
             ctoken: ctoken.addr,
             token: token,
             tokenIds: tokenIds,

packages/protocol/contracts/tokenvault/ERC20Vault.sol

@@ -40,6 +40,7 @@ contract ERC20Vault is BaseVault {
 
     struct BridgeTransferOp {
         uint64 destChainId;
+        address destOwner;
         address to;
         address token;
         uint256 amount;
@@ -186,7 +187,8 @@ contract ERC20Vault is BaseVault {
             _handleMessage({ user: msg.sender, token: op.token, amount: op.amount, to: op.to });
 
         message.destChainId = op.destChainId;
-        message.owner = msg.sender;
+        message.srcOwner = msg.sender;
+        message.destOwner = op.destOwner != address(0) ? op.destOwner : msg.sender;
         message.to = resolve(op.destChainId, name(), false);
         message.gasLimit = op.gasLimit;
         message.value = msg.value - op.fee;
@@ -200,7 +202,7 @@ contract ERC20Vault is BaseVault {
 
         emit TokenSent({
             msgHash: msgHash,
-            from: _message.owner,
+            from: _message.srcOwner,
             to: op.to,
             destChainId: op.destChainId,
             ctoken: ctoken.addr,
@@ -262,12 +264,12 @@ contract ERC20Vault is BaseVault {
             abi.decode(message.data[4:], (CanonicalERC20, address, address, uint256));
 
         // Transfer the ETH and tokens back to the owner
-        address token = _transferTokens(ctoken, message.owner, amount);
-        message.owner.sendEther(message.value);
+        address token = _transferTokens(ctoken, message.srcOwner, amount);
+        message.srcOwner.sendEther(message.value);
 
         emit TokenReleased({
             msgHash: msgHash,
-            from: message.owner,
+            from: message.srcOwner,
             ctoken: ctoken.addr,
             token: token,
             amount: amount

packages/protocol/contracts/tokenvault/ERC721Vault.sol

@@ -55,7 +55,8 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable {
         IBridge.Message memory message;
         message.destChainId = op.destChainId;
         message.data = data;
-        message.owner = msg.sender;
+        message.srcOwner = msg.sender;
+        message.destOwner = op.destOwner != address(0) ? op.destOwner : msg.sender;
         message.to = resolve(message.destChainId, name(), false);
         message.gasLimit = op.gasLimit;
         message.value = msg.value - op.fee;
@@ -69,7 +70,7 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable {
 
         emit TokenSent({
             msgHash: msgHash,
-            from: _message.owner,
+            from: _message.srcOwner,
             to: op.to,
             destChainId: _message.destChainId,
             ctoken: ctoken.addr,
@@ -133,12 +134,12 @@ contract ERC721Vault is BaseNFTVault, IERC721ReceiverUpgradeable {
             abi.decode(message.data[4:], (CanonicalNFT, address, address, uint256[]));
 
         // Transfer the ETH and tokens back to the owner
-        address token = _transferTokens(ctoken, message.owner, tokenIds);
-        message.owner.sendEther(message.value);
+        address token = _transferTokens(ctoken, message.srcOwner, tokenIds);
+        message.srcOwner.sendEther(message.value);
 
         emit TokenReleased({
             msgHash: msgHash,
-            from: message.owner,
+            from: message.srcOwner,
             ctoken: ctoken.addr,
             token: token,
             tokenIds: tokenIds,

packages/protocol/genesis/GenerateGenesis.g.sol

@@ -160,7 +160,8 @@ contract TestGenerateGenesis is Test, AddressResolver {
                 from: address(0),
                 srcChainId: 1,
                 destChainId: 167,
-                owner: address(0),
+                srcOwner: address(0),
+                destOwner: address(0),
                 to: address(0),
                 refundTo: address(0),
                 value: 0,
@@ -185,7 +186,8 @@ contract TestGenerateGenesis is Test, AddressResolver {
                 from: address(0),
                 srcChainId: 1,
                 destChainId: 167,
-                owner: address(0),
+                srcOwner: address(0),
+                destOwner: address(0),
                 to: address(0),
                 refundTo: address(0),
                 value: 0,

packages/protocol/test/bridge/Bridge.t.sol

@@ -129,7 +129,8 @@ contract BridgeTest is TaikoTest {
             from: address(bridge),
             srcChainId: uint64(block.chainid),
             destChainId: destChainId,
-            owner: Alice,
+            srcOwner: Alice,
+            destOwner: Alice,
             to: Alice,
             refundTo: Alice,
             value: 1000,
@@ -166,7 +167,8 @@ contract BridgeTest is TaikoTest {
             from: address(bridge),
             srcChainId: uint64(block.chainid),
             destChainId: destChainId,
-            owner: Alice,
+            srcOwner: Alice,
+            destOwner: Alice,
             to: Alice,
             refundTo: Alice,
             value: 1000,
@@ -222,7 +224,8 @@ contract BridgeTest is TaikoTest {
             from: address(bridge),
             srcChainId: uint64(block.chainid),
             destChainId: destChainId,
-            owner: Alice,
+            srcOwner: Alice,
+            destOwner: Alice,
             to: Alice,
             refundTo: Alice,
             value: 1000,
@@ -273,7 +276,8 @@ contract BridgeTest is TaikoTest {
             from: address(bridge),
             srcChainId: uint64(block.chainid),
             destChainId: destChainId,
-            owner: Alice,
+            srcOwner: Alice,
+            destOwner: Alice,
             to: address(goodReceiver),
             refundTo: Alice,
             value: 1000,
@@ -310,7 +314,8 @@ contract BridgeTest is TaikoTest {
             from: address(bridge),
             srcChainId: uint64(block.chainid),
             destChainId: destChainId,
-            owner: Alice,
+            srcOwner: Alice,
+            destOwner: Alice,
             to: address(goodReceiver),
             refundTo: Alice,
             value: 1000,
@@ -586,12 +591,12 @@ contract BridgeTest is TaikoTest {
 
         vm.stopPrank();
 
-        vm.prank(message.owner);
+        vm.prank(message.destOwner);
         destChainBridge.retryMessage(message, false);
         Bridge.Status postRetryStatus = destChainBridge.messageStatus(msgHash);
         assertEq(postRetryStatus == Bridge.Status.RETRIABLE, true);
 
-        vm.prank(message.owner);
+        vm.prank(message.destOwner);
         destChainBridge.retryMessage(message, true);
         postRetryStatus = destChainBridge.messageStatus(msgHash);
         assertEq(postRetryStatus == Bridge.Status.FAILED, true);
@@ -658,7 +663,8 @@ contract BridgeTest is TaikoTest {
             from: 0xDf08F82De32B8d460adbE8D72043E3a7e25A3B39,
             srcChainId: 1336,
             destChainId: dest,
-            owner: 0xDf08F82De32B8d460adbE8D72043E3a7e25A3B39,
+            srcOwner: 0xDf08F82De32B8d460adbE8D72043E3a7e25A3B39,
+            destOwner: 0xDf08F82De32B8d460adbE8D72043E3a7e25A3B39,
             to: 0x200708D76eB1B69761c23821809d53F65049939e,
             refundTo: 0x10020FCb72e27650651B05eD2CEcA493bC807Ba4,
             value: 1000,
@@ -687,7 +693,8 @@ contract BridgeTest is TaikoTest {
         returns (IBridge.Message memory)
     {
         return IBridge.Message({
-            owner: owner,
+            srcOwner: owner,
+            destOwner: owner,
             destChainId: destChain,
             to: to,
             value: value,