refactor(protocol): apply a few improvements (#16627)

Co-authored-by: smtmfft <[email protected]>
Co-authored-by: D <[email protected]>

packages/protocol/contracts/L1/TaikoL1.sol

@@ -62,8 +62,8 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
     )
         external
         payable
-        nonReentrant
         whenNotPaused
+        nonReentrant
         emitEventForClient
         returns (TaikoData.BlockMetadata memory meta_, TaikoData.EthDeposit[] memory deposits_)
     {
@@ -82,9 +82,9 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
         bytes calldata _input
     )
         external
-        nonReentrant
         whenNotPaused
         whenProvingNotPaused
+        nonReentrant
         emitEventForClient
     {
         (
@@ -105,9 +105,9 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
     /// @inheritdoc ITaikoL1
     function verifyBlocks(uint64 _maxBlocksToVerify)
         external
-        nonReentrant
         whenNotPaused
         whenProvingNotPaused
+        nonReentrant
         emitEventForClient
     {
         LibVerifying.verifyBlocks(state, getConfig(), this, _maxBlocksToVerify);
@@ -123,7 +123,7 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
     /// @notice Deposits Ether to Layer 2.
     /// @param _recipient Address of the recipient for the deposited Ether on
     /// Layer 2.
-    function depositEtherToL2(address _recipient) external payable nonReentrant whenNotPaused {
+    function depositEtherToL2(address _recipient) external payable whenNotPaused nonReentrant {
         LibDepositing.depositEtherToL2(state, getConfig(), this, _recipient);
     }
 
@@ -201,7 +201,7 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
             // This value is set based on `gasTargetPerL1Block = 15_000_000 * 4` in TaikoL2.
             // We use 8x rather than 4x here to handle the scenario where the average number of
             // Taiko blocks proposed per Ethereum block is smaller than 1.
-            blockMaxGasLimit: 30_000_000 * 8,
+            blockMaxGasLimit: 240_000_000,
             livenessBond: 250e18, // 250 Taiko token
             // ETH deposit related.
             ethDepositRingBufferSize: 1024,
@@ -210,7 +210,7 @@ contract TaikoL1 is EssentialContract, ITaikoL1, TaikoEvents, TaikoErrors {
             ethDepositMinAmount: 1 ether,
             ethDepositMaxAmount: 10_000 ether,
             ethDepositGas: 21_000,
-            ethDepositMaxFee: 1 ether / 10,
+            ethDepositMaxFee: 1e17, //0.1 ether,
             blockSyncThreshold: 16
         });
     }

packages/protocol/contracts/L1/TaikoToken.sol

@@ -34,6 +34,7 @@ contract TaikoToken is EssentialContract, ERC20SnapshotUpgradeable, ERC20VotesUp
         initializer
     {
         __Essential_init(_owner, _addressManager);
+        __Context_init_unchained();
         __ERC20_init(_name, _symbol);
         __ERC20Snapshot_init();
         __ERC20Votes_init();

packages/protocol/contracts/L1/gov/TaikoGovernor.sol

@@ -88,7 +88,7 @@ contract TaikoGovernor is
     /// @notice The number of votes required in order for a voter to become a proposer.
     /// @return The number of votes required.
     function proposalThreshold() public pure override returns (uint256) {
-        return 1_000_000_000 ether / 10_000; // 0.01% of Taiko Token
+        return 100_000 ether; // 0.01% of Taiko Token
     }
 
     /// @dev Cancel a proposal with GovernorBravo logic.

packages/protocol/contracts/L1/hooks/AssignmentHook.sol

@@ -64,14 +64,14 @@ contract AssignmentHook is EssentialContract, IHook {
 
     /// @inheritdoc IHook
     function onBlockProposed(
-        TaikoData.Block memory _blk,
-        TaikoData.BlockMetadata memory _meta,
-        bytes memory _data
+        TaikoData.Block calldata _blk,
+        TaikoData.BlockMetadata calldata _meta,
+        bytes calldata _data
     )
         external
         payable
-        nonReentrant
         onlyFromNamed("taiko")
+        nonReentrant
     {
         // Note that
         // - 'msg.sender' is the TaikoL1 contract address
@@ -94,8 +94,9 @@ contract AssignmentHook is EssentialContract, IHook {
 
         // Hash the assignment with the blobHash, this hash will be signed by
         // the prover, therefore, we add a string as a prefix.
-        address taikoL1Address = msg.sender;
-        bytes32 hash = hashAssignment(assignment, taikoL1Address, _meta.blobHash);
+
+        // msg.sender is taikoL1Address
+        bytes32 hash = hashAssignment(assignment, msg.sender, _meta.blobHash);
 
         if (!_blk.assignedProver.isValidSignatureNow(hash, assignment.signature)) {
             revert HOOK_ASSIGNMENT_INVALID_SIG();
@@ -107,7 +108,7 @@ contract AssignmentHook is EssentialContract, IHook {
         // Note that we don't have to worry about
         // https://github.com/crytic/slither/wiki/Detector-Documentation#arbitrary-from-in-transferfrom
         // as `assignedProver` has provided a signature above to authorize this hook.
-        tko.safeTransferFrom(_blk.assignedProver, taikoL1Address, _blk.livenessBond);
+        tko.safeTransferFrom(_blk.assignedProver, msg.sender, _blk.livenessBond);
 
         // Find the prover fee using the minimal tier
         uint256 proverFee = _getProverFee(assignment.tierFees, _meta.minTier);
@@ -133,7 +134,7 @@ contract AssignmentHook is EssentialContract, IHook {
 
         // Send all remaining Ether back to TaikoL1 contract
         if (address(this).balance != 0) {
-            taikoL1Address.sendEtherAndVerify(address(this).balance);
+            msg.sender.sendEtherAndVerify(address(this).balance);
         }
 
         emit BlockAssigned(_blk.assignedProver, _meta, assignment);

packages/protocol/contracts/L1/hooks/IHook.sol

@@ -11,9 +11,9 @@ interface IHook {
     /// @param _meta The metadata of the proposed block.
     /// @param _data The data of the proposed block.
     function onBlockProposed(
-        TaikoData.Block memory _blk,
-        TaikoData.BlockMetadata memory _meta,
-        bytes memory _data
+        TaikoData.Block calldata _blk,
+        TaikoData.BlockMetadata calldata _meta,
+        bytes calldata _data
     )
         external
         payable;

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

@@ -132,6 +132,8 @@ library LibProposing {
         } else {
             // This function must be called as the outmost transaction (not an internal one) for
             // the node to extract the calldata easily.
+            // Warning, this code will break after the Pectra hardfork with EIP 7645: Alias ORIGIN
+            // to SENDER
             if (msg.sender != tx.origin) revert L1_PROPOSER_NOT_EOA();
 
             meta_.blobHash = keccak256(_txList);

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

@@ -20,7 +20,7 @@ library LibProving {
     bytes32 public constant RETURN_LIVENESS_BOND = keccak256("RETURN_LIVENESS_BOND");
 
     /// @notice The tier name for optimistic proofs.
-    bytes32 public constant TIER_OP = bytes32("tier_optimistic");
+    bytes32 private constant TIER_OP = bytes32("tier_optimistic");
 
     // Warning: Any events defined here must also be defined in TaikoEvents.sol.
     /// @notice Emitted when a transition is proved.
@@ -88,7 +88,7 @@ library LibProving {
     /// @param _meta The block's metadata.
     /// @param _tran The transition data.
     /// @param _proof The proof.
-    /// @param maxBlocksToVerify_ The number of blocks to be verified with this transaction.
+    /// @return The number of blocks to be verified with this transaction.
     function proveBlock(
         TaikoData.State storage _state,
         TaikoData.Config memory _config,
@@ -98,7 +98,7 @@ library LibProving {
         TaikoData.TierProof memory _proof
     )
         internal
-        returns (uint8 maxBlocksToVerify_)
+        returns (uint8)
     {
         // Make sure parentHash is not zero
         // To contest an existing transition, simply use any non-zero value as

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

@@ -185,14 +185,17 @@ library LibVerifying {
                 // reward to the actual prover, while the assigned prover
                 // forfeits his liveness bond due to failure to fulfill their
                 // commitment.
-                uint256 bondToReturn = uint256(ts.validityBond) + blk.livenessBond;
+                uint256 bondToReturn = ts.validityBond;
 
                 // Nevertheless, it's possible for the actual prover to be the
                 // same individual or entity as the block's assigned prover.
                 // Consequently, we have chosen to grant the actual prover only
                 // half of the liveness bond as a reward.
-                if (ts.prover != blk.assignedProver) {
-                    bondToReturn -= blk.livenessBond >> 1;
+                if (blk.livenessBond != 0) {
+                    // livenessBond could have been returned in proving by guardian
+                    bondToReturn += ts.prover != blk.assignedProver
+                        ? blk.livenessBond >> 1 // half is burnt
+                        : blk.livenessBond;
                 }
 
                 IERC20 tko = IERC20(_resolver.resolve("taiko_token", false));

packages/protocol/contracts/L1/provers/Guardians.sol

@@ -107,11 +107,13 @@ abstract contract Guardians is EssentialContract {
         uint256 id = guardianIds[msg.sender];
         if (id == 0) revert INVALID_GUARDIAN();
 
+        uint32 _version = version;
+
         unchecked {
-            _approvals[version][_hash] |= 1 << (id - 1);
+            _approvals[_version][_hash] |= 1 << (id - 1);
         }
 
-        uint256 _approval = _approvals[version][_hash];
+        uint256 _approval = _approvals[_version][_hash];
         approved_ = isApproved(_approval);
         emit Approved(_operationId, _approval, approved_);
     }

packages/protocol/contracts/L2/TaikoL2.sol

@@ -143,10 +143,9 @@ contract TaikoL2 is EssentialContract {
         }
 
         // Verify the base fee per gas is correct
-        uint256 basefee;
-        (basefee, gasExcess) = getBasefee(_l1BlockId, _parentGasUsed);
+        (uint256 _basefee, uint64 _gasExcess) = getBasefee(_l1BlockId, _parentGasUsed);
 
-        if (!skipFeeCheck() && block.basefee != basefee) {
+        if (!skipFeeCheck() && block.basefee != _basefee) {
             revert L2_BASEFEE_MISMATCH();
         }
 
@@ -161,13 +160,15 @@ contract TaikoL2 is EssentialContract {
         }
 
         // Update state variables
-        l2Hashes[parentId] = blockhash(parentId);
+        bytes32 _parentHash = blockhash(parentId);
+        l2Hashes[parentId] = _parentHash;
         publicInputHash = publicInputHashNew;
 
         parentTimestamp = __currentBlockTimestamp;
         __currentBlockTimestamp = uint64(block.timestamp);
+        gasExcess = _gasExcess;
 
-        emit Anchored(blockhash(parentId), gasExcess);
+        emit Anchored(_parentHash, _gasExcess);
     }
 
     /// @notice Withdraw token or Ether from this address
@@ -178,9 +179,9 @@ contract TaikoL2 is EssentialContract {
         address _to
     )
         external
+        whenNotPaused
         onlyFromOwnerOrNamed("withdrawer")
         nonReentrant
-        whenNotPaused
     {
         if (_to == address(0)) revert L2_INVALID_PARAM();
         if (_token == address(0)) {

packages/protocol/contracts/automata-attestation/utils/SigVerifyLib.sol

@@ -17,11 +17,11 @@ contract SigVerifyLib is ISigVerifyLib {
     }
 
     function verifyES256Signature(
-        bytes memory tbs,
-        bytes memory signature,
-        bytes memory publicKey
+        bytes calldata tbs,
+        bytes calldata signature,
+        bytes calldata publicKey
     )
-        public
+        external
         view
         returns (bool sigValid)
     {

packages/protocol/contracts/bridge/Bridge.sol

@@ -93,21 +93,20 @@ contract Bridge is EssentialContract, IBridge {
         for (uint256 i; i < _msgHashes.length; ++i) {
             bytes32 msgHash = _msgHashes[i];
 
+            ProofReceipt storage receipt = proofReceipt[msgHash];
+            uint64 _receivedAt = receipt.receivedAt;
+
             if (_suspend) {
-                if (proofReceipt[msgHash].receivedAt == 0) revert B_MESSAGE_NOT_PROVEN();
-                if (proofReceipt[msgHash].receivedAt == type(uint64).max) {
-                    revert B_MESSAGE_SUSPENDED();
-                }
+                if (_receivedAt == 0) revert B_MESSAGE_NOT_PROVEN();
+                if (_receivedAt == type(uint64).max) revert B_MESSAGE_SUSPENDED();
 
-                proofReceipt[msgHash].receivedAt = type(uint64).max;
+                receipt.receivedAt = type(uint64).max;
                 emit MessageSuspended(msgHash, true, 0);
             } else {
                 // Note before we set the receivedAt to current timestamp, we have to be really
                 // careful that this message must have been proven then suspended.
-                if (proofReceipt[msgHash].receivedAt != type(uint64).max) {
-                    revert B_MESSAGE_NOT_SUSPENDED();
-                }
-                proofReceipt[msgHash].receivedAt = uint64(block.timestamp);
+                if (_receivedAt != type(uint64).max) revert B_MESSAGE_NOT_SUSPENDED();
+                receipt.receivedAt = uint64(block.timestamp);
                 emit MessageSuspended(msgHash, false, uint64(block.timestamp));
             }
         }
@@ -118,8 +117,8 @@ contract Bridge is EssentialContract, IBridge {
         external
         payable
         override
-        nonReentrant
         whenNotPaused
+        nonReentrant
         returns (bytes32 msgHash_, Message memory message_)
     {
         // Ensure the message owner is not null.
@@ -159,9 +158,9 @@ contract Bridge is EssentialContract, IBridge {
         bytes calldata _proof
     )
         external
-        nonReentrant
         whenNotPaused
         sameChain(_message.srcChainId)
+        nonReentrant
     {
         bytes32 msgHash = hashMessage(_message);
 
@@ -226,9 +225,9 @@ contract Bridge is EssentialContract, IBridge {
         bytes calldata _proof
     )
         external
-        nonReentrant
         whenNotPaused
         sameChain(_message.destChainId)
+        nonReentrant
     {
         bytes32 msgHash = hashMessage(_message);
         if (messageStatus[msgHash] != Status.NEW) revert B_STATUS_MISMATCH();
@@ -338,9 +337,9 @@ contract Bridge is EssentialContract, IBridge {
         bool _isLastAttempt
     )
         external
-        nonReentrant
         whenNotPaused
         sameChain(_message.destChainId)
+        nonReentrant
     {
         // If the gasLimit is set to 0 or isLastAttempt is true, the caller must
         // be the message.destOwner.
@@ -397,7 +396,7 @@ contract Bridge is EssentialContract, IBridge {
     /// if requested.
     /// @param _message The message.
     /// @param _proof The merkle inclusion proof.
-    /// @return true if the message has failed, false otherwise.
+    /// @return true if the message has been received, false otherwise.
     function proveMessageReceived(
         Message calldata _message,
         bytes calldata _proof

packages/protocol/contracts/common/AddressResolver.sol

@@ -83,9 +83,10 @@ abstract contract AddressResolver is IAddressResolver, Initializable {
         view
         returns (address payable addr_)
     {
-        if (addressManager == address(0)) revert RESOLVER_INVALID_MANAGER();
+        address _addressManager = addressManager;
+        if (_addressManager == address(0)) revert RESOLVER_INVALID_MANAGER();
 
-        addr_ = payable(IAddressManager(addressManager).getAddress(_chainId, _name));
+        addr_ = payable(IAddressManager(_addressManager).getAddress(_chainId, _name));
 
         if (!_allowZeroAddress && addr_ == address(0)) {
             revert RESOLVER_ZERO_ADDR(_chainId, _name);

packages/protocol/contracts/common/EssentialContract.sol

@@ -99,17 +99,9 @@ abstract contract EssentialContract is UUPSUpgradeable, Ownable2StepUpgradeable,
     /// @notice Initializes the contract.
     /// @param _owner The owner of this contract. msg.sender will be used if this value is zero.
     /// @param _addressManager The address of the {AddressManager} contract.
-    function __Essential_init(
-        address _owner,
-        address _addressManager
-    )
-        internal
-        virtual
-        onlyInitializing
-    {
-        __Essential_init(_owner);
-
+    function __Essential_init(address _owner, address _addressManager) internal onlyInitializing {
         if (_addressManager == address(0)) revert ZERO_ADDR_MANAGER();
+        __Essential_init(_owner);
         __AddressResolver_init(_addressManager);
     }