PR fixes: add comments, rename constracts, more interfaces to contrac…

…t files

contracts/optimism/DepositDataCodec.sol → contracts/lib/DepositDataCodec.sol

@@ -5,7 +5,7 @@ pragma solidity 0.8.10;
 
 /// @author kovalgek
 /// @notice encodes and decodes DepositData for crosschain transfering.
-contract DepositDataCodec {
+library DepositDataCodec {
 
     uint8 internal constant RATE_FIELD_SIZE = 12;
     uint8 internal constant TIMESTAMP_FIELD_SIZE = 5;

contracts/lib/ECDSA.sol

@@ -0,0 +1,57 @@
+// SPDX-FileCopyrightText: 2024 Lido <[email protected]>
+// SPDX-License-Identifier: MIT
+
+// Extracted from:
+// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v3.4.0/contracts/cryptography/ECDSA.sol#L53
+// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/541e821/contracts/utils/cryptography/ECDSA.sol#L112
+
+pragma solidity 0.8.10;
+
+library ECDSA {
+    /**
+     * @dev Returns the address that signed a hashed message (`hash`).
+     * This address can then be used for verification purposes.
+     * Receives the `v`, `r` and `s` signature fields separately.
+     *
+     * The `ecrecover` EVM opcode allows for malleable (non-unique) signatures:
+     * this function rejects them by requiring the `s` value to be in the lower
+     * half order, and the `v` value to be either 27 or 28.
+     *
+     * IMPORTANT: `hash` _must_ be the result of a hash operation for the
+     * verification to be secure: it is possible to craft signatures that
+     * recover to arbitrary addresses for non-hashed data.
+     */
+    function recover(bytes32 hash, uint8 v, bytes32 r, bytes32 s) internal pure returns (address)
+    {
+        // EIP-2 still allows signature malleability for ecrecover(). Remove this possibility and make the signature
+        // unique. Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines
+        // the valid range for s in (281): 0 < s < secp256k1n ÷ 2 + 1, and for v in (282): v ∈ {27, 28}. Most
+        // signatures from current libraries generate a unique signature with an s-value in the lower half order.
+        //
+        // If your library generates malleable signatures, such as s-values in the upper range, calculate a new s-value
+        // with 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141 - s1 and flip v from 27 to 28 or
+        // vice versa. If your library also generates signatures with 0/1 for v instead 27/28, add 27 to v to accept
+        // these malleable signatures as well.
+        require(uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0, "ECDSA: invalid signature 's' value");
+
+        // If the signature is valid (and not malleable), return the signer address
+        address signer = ecrecover(hash, v, r, s);
+        require(signer != address(0), "ECDSA: invalid signature");
+
+        return signer;
+    }
+
+    /**
+     * @dev Overload of `recover` that receives the `r` and `vs` short-signature fields separately.
+     * See https://eips.ethereum.org/EIPS/eip-2098[EIP-2098 short signatures]
+     */
+    function recover(bytes32 hash, bytes32 r, bytes32 vs) internal pure returns (address) {
+        bytes32 s;
+        uint8 v;
+        assembly {
+            s := and(vs, 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+            v := add(shr(255, vs), 27)
+        }
+        return recover(hash, v, r, s);
+    }
+}

contracts/lib/SignatureChecker.sol

@@ -1,35 +1,22 @@
-// SPDX-FileCopyrightText: 2024 OpenZeppelin, Lido <[email protected]>
-// SPDX-License-Identifier: GPL-3.0
-// Written based on (utils/cryptography/SignatureChecker.sol from d398d68
+// SPDX-FileCopyrightText: 2024 Lido <[email protected]>
+// SPDX-License-Identifier: MIT
 
 pragma solidity 0.8.10;
 
-import {ECDSA} from "@openzeppelin/contracts-v4.9/utils/cryptography/ECDSA.sol";
-import {IERC1271} from "@openzeppelin/contracts-v4.9/interfaces/IERC1271.sol";
+import {ECDSA} from "./ECDSA.sol";
 
-
-
-/**
- * @dev Signature verification helper that can be used instead of `ECDSA.recover` to seamlessly support both ECDSA
- * signatures from externally owned accounts (EOAs) as well as ERC-1271 signatures from smart contract wallets like
- * Argent and Safe Wallet (previously Gnosis Safe).
- */
+/// @dev A copy of SignatureUtils.sol contract from Lido Core Protocol
+/// https://github.com/lidofinance/lido-dao/blob/master/contracts/common/lib/SignatureUtils.sol
 library SignatureChecker {
     /**
-     * @dev Checks if a signature is valid for a given signer and data hash. If the signer is a smart contract, the
-     * signature is validated against that smart contract using ERC-1271, otherwise it's validated using `ECDSA.recover`.
+     * @dev The selector of the ERC1271's `isValidSignature(bytes32 hash, bytes signature)` function,
+     * serving at the same time as the magic value that the function should return upon success.
+     *
+     * See https://eips.ethereum.org/EIPS/eip-1271.
      *
-     * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus
-     * change through time. It could return true at block N and false at block N+1 (or the opposite).
+     * bytes4(keccak256("isValidSignature(bytes32,bytes)")
      */
-    function isValidSignatureNow(address signer, bytes32 hash, bytes memory signature) internal view returns (bool) {
-        if (signer.code.length == 0) {
-            (address recovered, ECDSA.RecoverError err) = ECDSA.tryRecover(hash, signature);
-            return err == ECDSA.RecoverError.NoError && recovered == signer;
-        } else {
-            return isValidERC1271SignatureNow(signer, hash, signature);
-        }
-    }
+    bytes4 internal constant ERC1271_IS_VALID_SIGNATURE_SELECTOR = 0x1626ba7e;
 
     /**
      * @dev Checks signature validity.
@@ -38,39 +25,40 @@ library SignatureChecker {
      * and the signature is a ECDSA signature generated using its private key. Otherwise, issues a
      * static call to the signer address to check the signature validity using the ERC-1271 standard.
      */
-    function isValidSignatureNow(
+    function isValidSignature(
         address signer,
         bytes32 msgHash,
         uint8 v,
         bytes32 r,
         bytes32 s
     ) internal view returns (bool) {
-        if (signer.code.length == 0) {
-            (address recovered, ECDSA.RecoverError err) = ECDSA.tryRecover(msgHash, v, r, s);
-            return err == ECDSA.RecoverError.NoError && recovered == signer;
+        if (_hasCode(signer)) {
+            bytes memory sig = abi.encodePacked(r, s, v);
+            // Solidity <0.5 generates a regular CALL instruction even if the function being called
+            // is marked as `view`, and the only way to perform a STATICCALL is to use assembly
+            bytes memory data = abi.encodeWithSelector(ERC1271_IS_VALID_SIGNATURE_SELECTOR, msgHash, sig);
+            bytes32 retval;
+            /// @solidity memory-safe-assembly
+            assembly {
+                // allocate memory for storing the return value
+                let outDataOffset := mload(0x40)
+                mstore(0x40, add(outDataOffset, 32))
+                // issue a static call and load the result if the call succeeded
+                let success := staticcall(gas(), signer, add(data, 32), mload(data), outDataOffset, 32)
+                if and(eq(success, 1), eq(returndatasize(), 32)) {
+                    retval := mload(outDataOffset)
+                }
+            }
+            return retval == bytes32(ERC1271_IS_VALID_SIGNATURE_SELECTOR);
         } else {
-            bytes memory signature = abi.encodePacked(r, s, v);
-            return isValidERC1271SignatureNow(signer, msgHash, signature);
+            return ECDSA.recover(msgHash, v, r, s) == signer;
         }
     }
 
-    /**
-     * @dev Checks if a signature is valid for a given signer and data hash. The signature is validated
-     * against the signer smart contract using ERC-1271.
-     *
-     * NOTE: Unlike ECDSA signatures, contract signatures are revocable, and the outcome of this function can thus
-     * change through time. It could return true at block N and false at block N+1 (or the opposite).
-     */
-    function isValidERC1271SignatureNow(
-        address signer,
-        bytes32 hash,
-        bytes memory signature
-    ) internal view returns (bool) {
-        (bool success, bytes memory result) = signer.staticcall(
-            abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature)
-        );
-        return (success &&
-            result.length >= 32 &&
-            abi.decode(result, (bytes32)) == bytes32(IERC1271.isValidSignature.selector));
+    function _hasCode(address addr) internal view returns (bool) {
+        uint256 size;
+        /// @solidity memory-safe-assembly
+        assembly { size := extcodesize(addr) }
+        return size > 0;
     }
 }

contracts/lido/TokenRateNotifier.sol

@@ -41,6 +41,9 @@ contract TokenRateNotifier is Ownable, IPostTokenRebaseReceiver {
 
     /// @param initialOwner_ initial owner
     constructor(address initialOwner_) {
+        if (initialOwner_ == address(0)) {
+            revert ErrorZeroAddressOwner();
+        }
         _transferOwnership(initialOwner_);
     }
 
@@ -56,6 +59,9 @@ contract TokenRateNotifier is Ownable, IPostTokenRebaseReceiver {
         if (observers.length >= MAX_OBSERVERS_COUNT) {
             revert ErrorMaxObserversCountExceeded();
         }
+        if (_observerIndex(observer_) != INDEX_NOT_FOUND) {
+            revert ErrorAddExistedObserver();
+        }
 
         observers.push(observer_);
         emit ObserverAdded(observer_);
@@ -70,8 +76,9 @@ contract TokenRateNotifier is Ownable, IPostTokenRebaseReceiver {
         if (observerIndexToRemove == INDEX_NOT_FOUND) {
             revert ErrorNoObserverToRemove();
         }
-
-        observers[observerIndexToRemove] = observers[observers.length - 1];
+        if (observers.length > 1) {
+            observers[observerIndexToRemove] = observers[observers.length - 1];
+        }
         observers.pop();
 
         emit ObserverRemoved(observer_);
@@ -89,6 +96,7 @@ contract TokenRateNotifier is Ownable, IPostTokenRebaseReceiver {
         uint256  /* sharesMintedAsFees */
     ) external {
         for (uint256 obIndex = 0; obIndex < observers.length; obIndex++) {
+            // solhint-disable-next-line no-empty-blocks
             try ITokenRatePusher(observers[obIndex]).pushTokenRate() {}
             catch (bytes memory lowLevelRevertData) {
                 /// @dev This check is required to prevent incorrect gas estimation of the method.
@@ -131,4 +139,6 @@ contract TokenRateNotifier is Ownable, IPostTokenRebaseReceiver {
     error ErrorBadObserverInterface();
     error ErrorMaxObserversCountExceeded();
     error ErrorNoObserverToRemove();
+    error ErrorZeroAddressOwner();
+    error ErrorAddExistedObserver();
 }

contracts/lido/stubs/OpStackTokenRatePusherWithOutOfGasErrorStub.sol

@@ -6,6 +6,7 @@ pragma solidity 0.8.10;
 import {ITokenRatePusher} from "../interfaces/ITokenRatePusher.sol";
 import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
 
+/// @dev For testing purposes.
 contract OpStackTokenRatePusherWithOutOfGasErrorStub is ERC165, ITokenRatePusher {
 
     uint256 public constant OUT_OF_GAS_INCURRING_MAX = 1000000000000;

contracts/lido/stubs/OpStackTokenRatePusherWithSomeErrorStub.sol

@@ -6,11 +6,12 @@ pragma solidity 0.8.10;
 import {ITokenRatePusher} from "../interfaces/ITokenRatePusher.sol";
 import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
 
+/// @dev For testing purposes.
 contract OpStackTokenRatePusherWithSomeErrorStub is ERC165, ITokenRatePusher {
 
     error SomeError();
 
-    function pushTokenRate() external {
+    function pushTokenRate() pure external {
         revert SomeError();
     }
 

contracts/optimism/L1ERC20ExtendedTokensBridge.sol

@@ -119,14 +119,11 @@ abstract contract L1ERC20ExtendedTokensBridge is
         onlyFromCrossDomainAccount(L2_TOKEN_BRIDGE)
         onlySupportedL1L2TokensPair(l1Token_, l2Token_)
     {
-        if(_isRebasable(l1Token_)) {
-            uint256 rebasableTokenAmount = IERC20Wrapper(L1_TOKEN_NON_REBASABLE).unwrap(amount_);
-            IERC20(l1Token_).safeTransfer(to_, rebasableTokenAmount);
-            emit ERC20WithdrawalFinalized(l1Token_, l2Token_, from_, to_, rebasableTokenAmount, data_);
-        } else {
-            IERC20(l1Token_).safeTransfer(to_, amount_);
-            emit ERC20WithdrawalFinalized(l1Token_, l2Token_, from_, to_, amount_, data_);
-        }
+        uint256 amountToWithdraw = _isRebasable(l1Token_) ?
+            IERC20Wrapper(L1_TOKEN_NON_REBASABLE).unwrap(amount_) :
+            amount_;
+        IERC20(l1Token_).safeTransfer(to_, amountToWithdraw);
+        emit ERC20WithdrawalFinalized(l1Token_, l2Token_, from_, to_, amountToWithdraw, data_);
     }
 
     /// @dev Performs the logic for deposits by informing the L2 token bridge contract

contracts/optimism/L1LidoTokensBridge.sol

@@ -4,7 +4,14 @@
 pragma solidity 0.8.10;
 
 import {L1ERC20ExtendedTokensBridge} from "./L1ERC20ExtendedTokensBridge.sol";
-import {IERC20WstETH} from "../token/interfaces/IERC20WstETH.sol";
+
+/// @author kovalgek
+/// @notice A subset of wstETH token interface of core LIDO protocol.
+interface IERC20WstETH {
+    /// @notice Get amount of wstETH for a one stETH
+    /// @return Amount of wstETH for a 1 stETH
+    function stEthPerToken() external view returns (uint256);
+}
 
 /// @author kovalgek
 /// @notice Hides wstETH concept from other contracts to keep `L1ERC20ExtendedTokensBridge` reusable.

contracts/optimism/L2ERC20ExtendedTokensBridge.sol

@@ -5,10 +5,11 @@ pragma solidity 0.8.10;
 
 import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import {Address} from "@openzeppelin/contracts/utils/Address.sol";
 import {IL1ERC20Bridge} from "./interfaces/IL1ERC20Bridge.sol";
 import {IL2ERC20Bridge} from "./interfaces/IL2ERC20Bridge.sol";
-import {IERC20Bridged} from "../token/interfaces/IERC20Bridged.sol";
-import {ITokenRateOracle} from "../token/interfaces/ITokenRateOracle.sol";
+import {IERC20Bridged} from "../token/ERC20Bridged.sol";
+import {ITokenRateUpdatable} from "../optimism/interfaces/ITokenRateUpdatable.sol";
 import {IERC20Wrapper} from "../token/interfaces/IERC20Wrapper.sol";
 import {ERC20Rebasable} from "../token/ERC20Rebasable.sol";
 import {BridgingManager} from "../BridgingManager.sol";
@@ -30,7 +31,7 @@ contract L2ERC20ExtendedTokensBridge is
 {
     using SafeERC20 for IERC20;
 
-    address public immutable L1_TOKEN_BRIDGE;
+    address private immutable L1_TOKEN_BRIDGE;
 
     /// @param messenger_ L2 messenger address being used for cross-chain communications
     /// @param l1TokenBridge_  Address of the corresponding L1 bridge
@@ -69,6 +70,9 @@ contract L2ERC20ExtendedTokensBridge is
         whenWithdrawalsEnabled
         onlySupportedL2Token(l2Token_)
     {
+        if (Address.isContract(msg.sender)) {
+            revert ErrorSenderNotEOA();
+        }
         _withdrawTo(l2Token_, msg.sender, msg.sender, amount_, l1Gas_, data_);
         emit WithdrawalInitiated(_l1Token(l2Token_), l2Token_, msg.sender, msg.sender, amount_, data_);
     }
@@ -103,7 +107,7 @@ contract L2ERC20ExtendedTokensBridge is
         onlyFromCrossDomainAccount(L1_TOKEN_BRIDGE)
     {
         DepositDataCodec.DepositData memory depositData = DepositDataCodec.decodeDepositData(data_);
-        ITokenRateOracle tokenRateOracle = ERC20Rebasable(L2_TOKEN_REBASABLE).TOKEN_RATE_ORACLE();
+        ITokenRateUpdatable tokenRateOracle = ERC20Rebasable(L2_TOKEN_REBASABLE).TOKEN_RATE_ORACLE();
         tokenRateOracle.updateRate(depositData.rate, depositData.timestamp);
 
         uint256 depositedAmount = _mintTokens(l1Token_, l2Token_, to_, amount_);
@@ -165,4 +169,6 @@ contract L2ERC20ExtendedTokensBridge is
         IERC20Bridged(l2Token_).bridgeBurn(from_, amount_);
         return amount_;
     }
+
+    error ErrorSenderNotEOA();
 }

contracts/optimism/OpStackTokenRatePusher.sol

@@ -5,8 +5,8 @@ pragma solidity 0.8.10;
 
 import {CrossDomainEnabled} from "./CrossDomainEnabled.sol";
 import {ITokenRatePusher} from "../lido/interfaces/ITokenRatePusher.sol";
-import {IERC20WstETH} from "../token/interfaces/IERC20WstETH.sol";
-import {ITokenRateOracle} from "../token/interfaces/ITokenRateOracle.sol";
+import {IERC20WstETH} from "./L1LidoTokensBridge.sol";
+import {ITokenRateUpdatable} from "../optimism/interfaces/ITokenRateUpdatable.sol";
 import {ERC165} from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
 
 /// @author kovalgek
@@ -19,7 +19,11 @@ contract OpStackTokenRatePusher is CrossDomainEnabled, ERC165, ITokenRatePusher
     /// @notice Non-rebasable token of Core Lido procotol.
     address public immutable WSTETH;
 
-    /// @notice Gas limit required to complete pushing token rate on L2.
+    /// @notice Gas limit for L2 required to finish pushing token rate on L2 side.
+    ///         Client pays for gas on L2 by burning it on L1.
+    ///         Depends linearly on deposit data length and gas used for finalizing deposit on L2.
+    ///         Formula to find value:
+    ///         (gas cost of L2Bridge.finalizeDeposit() + OptimismPortal.minimumGasLimit(depositData.length)) * 1.5
     uint32 public immutable L2_GAS_LIMIT_FOR_PUSHING_TOKEN_RATE;
 
     /// @param messenger_ L1 messenger address being used for cross-chain communications
@@ -42,7 +46,7 @@ contract OpStackTokenRatePusher is CrossDomainEnabled, ERC165, ITokenRatePusher
         uint256 tokenRate = IERC20WstETH(WSTETH).stEthPerToken();
 
         bytes memory message = abi.encodeWithSelector(
-            ITokenRateOracle.updateRate.selector,
+            ITokenRateUpdatable.updateRate.selector,
             tokenRate,
             block.timestamp
         );

contracts/optimism/README.md

@@ -514,7 +514,7 @@ Transfers `amount` of token from the `from_` account to `to_` using the allowanc
 
 ## `ERC20Bridged`
 
-**Implements:** [`IERC20Bridged`](https://github.com/lidofinance/lido-l2/blob/main/contracts/token/interfaces/IERC20Bridged.sol)
+**Implements:** [`IERC20Bridged`](https://github.com/lidofinance/lido-l2/blob/main/contracts/token/ERC20Bridged.sol)
 **Inherits:** [`ERC20Metadata`](#ERC20Metadata) [`ERC20Core`](#ERC20CoreLogic)
 
 Inherits the `ERC20` default functionality that allows the bridge to mint and burn tokens.