Skip to content

Latest commit

 

History

History
65 lines (57 loc) · 2.72 KB

pauliax-Q.md

File metadata and controls

65 lines (57 loc) · 2.72 KB
  1. Some contracts miss the explicit initialization of inherited contracts. It does not cause any harm but it would be better to stay consistent: xRenzoBridge -> __ReentrancyGuard_init(); xRenzoDeposit -> __ReentrancyGuard_init(); WithdrawQueue -> __ReentrancyGuard_init

  2. The value of coolDownPeriod may change between the withdrawal initiation and claim. It would be more fair to cache this value in every withdrawal request:

if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();
  1. When initializing EzEthToken, the name should come first and then the symbol:
__ERC20_init("ezETH", "Renzo Restaked ETH");
    function __ERC20_init(string memory name_, string memory symbol_) internal onlyInitializing {
        __ERC20_init_unchained(name_, symbol_);
    }

but the issue is fixed by overriding name() and symbol() functions later to return correct values.

  1. This hardcoded address is used across different contracts. extract it to a separate Constants contract: address public constant IS_NATIVE = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;

  2. Could emit OraclePriceFeedUpdated(oracle, address(0));:

// set oracle Price Feed struct
oracle = _oracle;
  1. No need nonReentrant because only an event is emitted:
function fillEthWithdrawBuffer() external payable nonReentrant onlyDepositQueue {
        emit EthBufferFilled(msg.value);
    }
  1. When initiating the withdrawal it checks if the asset is supported:
  // check if provided assetOut is supported
  if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();

It is not possible to disable withdrawal asset in updateWithdrawBufferTarget:

  if (_newBufferTarget[i].asset == address(0) || _newBufferTarget[i].bufferAmount == 0)
                revert InvalidZeroInput();

If an asset misbehaves, an admin should be capable of disabling it.

  1. Claim uses bad practices to push withdrawal assets to the user.
  1. .transfer which is not recommended due to limited forwarded gas.
  2. ERC20 transfer does not check the returned value (bool) indicating success.
        // send selected redeem asset to user
        if (_withdrawRequest.collateralToken == IS_NATIVE) {
            payable(msg.sender).transfer(_withdrawRequest.amountToRedeem);
        } else {
            IERC20(_withdrawRequest.collateralToken).transfer(
                msg.sender,
                _withdrawRequest.amountToRedeem
            );
        }
  1. I believe the withdrawal queue needs the possibility to cancel the withdrawal request and return ezETH to the user. Especially considering that .transfer might fail on some recipients.