Merge branch 'brecht-review' into additional_changes

packages/protocol/contracts/L1/TaikoData.sol

@@ -84,6 +84,7 @@ library TaikoData {
 
     struct BlockParams {
         address assignedProver;
+        address coinbase;
         bytes32 extraData;
         bytes32 blobHash;
         uint24 txListByteOffset;

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

@@ -78,6 +78,10 @@ library LibProposing {
             revert L1_INVALID_PROVER();
         }
 
+        if (params.coinbase == address(0)) {
+            params.coinbase = msg.sender;
+        }
+
         // Taiko, as a Based Rollup, enables permissionless block proposals.
         // However, if the "proposer" address is set to a non-zero value, we
         // ensure that only that specific address has the authority to propose
@@ -104,7 +108,7 @@ library LibProposing {
 
         // Each transaction must handle a specific quantity of L1-to-L2
         // Ether deposits.
-        depositsProcessed = LibDepositing.processDeposits(state, config, msg.sender);
+        depositsProcessed = LibDepositing.processDeposits(state, config, params.coinbase);
 
         // Initialize metadata to compute a metaHash, which forms a part of
         // the block data to be stored on-chain for future integrity checks.
@@ -117,12 +121,7 @@ library LibProposing {
                 blobHash: 0, // to be initialized below
                 extraData: params.extraData,
                 depositsHash: keccak256(abi.encode(depositsProcessed)),
-                // TODO(Brecht): A bit limiting this is required to be the same address as
-                // msg.sender.
-                // And fees will be collected on L2 so if a smart contract is used as the proposer
-                // it requires the same contract to be deployed on L2 at the same address owned by
-                // the same entity.
-                coinbase: msg.sender,
+                coinbase: params.coinbase,
                 id: b.numBlocks,
                 gasLimit: config.blockMaxGasLimit,
                 timestamp: uint64(block.timestamp),

packages/protocol/contracts/bridge/Bridge.sol

@@ -358,10 +358,9 @@ contract Bridge is EssentialContract, IBridge {
         whenNotPaused
         sameChain(message.destChainId)
     {
-        // If isLastAttempt is true, the caller must be the message.owner.
-        // TODO(Brecht): why not allow anyone to call when message.gasLimit == 0?
-        // There is no fee to be earned, and there won't be any gas limit anyway.
-        if (isLastAttempt) {
+        // If the gasLimit is set to 0 or isLastAttempt is true, the caller must
+        // be the message.owner.
+        if (message.gasLimit == 0 || isLastAttempt) {
             if (msg.sender != message.owner) revert B_PERMISSION_DENIED();
         }
 
@@ -544,37 +543,13 @@ contract Bridge is EssentialContract, IBridge {
         }
     }
 
-    /// @notice Checks if the signal was received.
-    /// @param signalService The signalService
-    /// @param signal The signal.
-    /// @param chainId The ID of the chain the signal is stored on
-    /// @param proof The merkle inclusion proof.
-    /// @return True if the message was received.
-    function _proveSignalReceived(
-        address signalService,
-        bytes32 signal,
-        uint64 chainId,
-        bytes calldata proof
-    )
-        private
-        view
-        returns (bool)
-    {
-        bytes memory data = abi.encodeCall(
-            ISignalService.proveSignalReceived,
-            (chainId, resolve(chainId, "bridge", false), signal, proof)
-        );
-        (bool success, bytes memory ret) = signalService.staticcall(data);
-        return success ? abi.decode(ret, (bool)) : false;
-    }
-
     /// @notice Resets the call context
-    function _resetContext() internal {
+    function _resetContext() private {
         _storeContext(bytes32(0), address(0), uint64(0));
     }
 
     /// @notice Stores the call context
-    function _storeContext(bytes32 msgHash, address from, uint64 srcChainId) internal {
+    function _storeContext(bytes32 msgHash, address from, uint64 srcChainId) private {
         assembly {
             tstore(_CTX_SLOT, msgHash)
             tstore(add(_CTX_SLOT, 1), from)
@@ -583,7 +558,7 @@ contract Bridge is EssentialContract, IBridge {
     }
 
     /// @notice Loads the call context
-    function _loadContext() internal view returns (Context memory) {
+    function _loadContext() private view returns (Context memory) {
         bytes32 msgHash;
         address from;
         uint64 srcChainId;
@@ -594,4 +569,28 @@ contract Bridge is EssentialContract, IBridge {
         }
         return Context({ msgHash: msgHash, from: from, srcChainId: srcChainId });
     }
+
+    /// @notice Checks if the signal was received.
+    /// @param signalService The signalService
+    /// @param signal The signal.
+    /// @param chainId The ID of the chain the signal is stored on
+    /// @param proof The merkle inclusion proof.
+    /// @return True if the message was received.
+    function _proveSignalReceived(
+        address signalService,
+        bytes32 signal,
+        uint64 chainId,
+        bytes calldata proof
+    )
+        private
+        view
+        returns (bool)
+    {
+        bytes memory data = abi.encodeCall(
+            ISignalService.proveSignalReceived,
+            (chainId, resolve(chainId, "bridge", false), signal, proof)
+        );
+        (bool success, bytes memory ret) = signalService.staticcall(data);
+        return success ? abi.decode(ret, (bool)) : false;
+    }
 }

packages/protocol/contracts/common/OwnerUUPSUpgradable.sol

@@ -93,14 +93,14 @@ abstract contract OwnerUUPSUpgradable is UUPSUpgradeable, OwnableUpgradeable {
     }
 
     // Stores the reentry lock
-    function _storeReentryLock(uint8 reentry) internal {
+    function _storeReentryLock(uint8 reentry) private {
         assembly {
             tstore(_REENTRY_SLOT, reentry)
         }
     }
 
     // Loads the reentry lock
-    function _loadReentryLock() internal view returns (uint8 reentry) {
+    function _loadReentryLock() private view returns (uint8 reentry) {
         assembly {
             reentry := tload(_REENTRY_SLOT)
         }

packages/protocol/contracts/tokenvault/BridgedERC20Base.sol

@@ -85,10 +85,10 @@ abstract contract BridgedERC20Base is EssentialContract, IBridgedERC20 {
         return super.owner();
     }
 
+    function _mintToken(address account, uint256 amount) internal virtual;
+    function _burnToken(address from, uint256 amount) internal virtual;
+
     function _isMigratingOut() internal view returns (bool) {
         return migratingAddress != address(0) && !migratingInbound;
     }
-
-    function _mintToken(address account, uint256 amount) internal virtual;
-    function _burnToken(address from, uint256 amount) internal virtual;
 }

packages/protocol/contracts/tokenvault/ERC20Vault.sol

@@ -132,9 +132,6 @@ contract ERC20Vault is BaseVault {
 
         btokenOld = canonicalToBridged[ctoken.chainId][ctoken.addr];
 
-        // TODO(Brecht): if the ctoken is on the current chain, should we check the
-        // symbol/name/decimals against the actual token?
-
         if (btokenOld != address(0)) {
             CanonicalERC20 memory _ctoken = bridgedToCanonical[btokenOld];
 

packages/protocol/test/L1/TaikoL1TestBase.sol

@@ -221,7 +221,7 @@ abstract contract TaikoL1TestBase is TaikoTest {
 
         vm.prank(proposer, proposer);
         (meta, depositsProcessed) = L1.proposeBlock{ value: msgValue }(
-            abi.encode(TaikoData.BlockParams(prover, 0, 0, 0, 0, false, 0, hookcalls)),
+            abi.encode(TaikoData.BlockParams(prover, address(0), 0, 0, 0, 0, false, 0, hookcalls)),
             new bytes(txListSize)
         );
     }