Skipping the deposit queue

[tinchoabbate]

Hey there.
I wanted to raise a potential oversight in the way the deposits queue is implemented in the _stake function of the Pool contract.

I'm aware that nowadays this queue is deactivated both in Ethereum Mainnet and Gnosis Chain (due to the minActivatingDeposit parameter having been set to the max uint256 after this proposal). So as far as I understand, there's no immediate risk of taking advantage of the behavior I'm about to describe. I should also say that it doesn't seem it'd be in scope of your bug bounty program. That's why I'm reporting it here.

Currently, the _stake function implements an early minting mechanism of sETH2 for deposits that would be below the minActivatingDeposit threshold.

function _stake(address recipient, uint256 value) internal whenNotPaused {
    ...
    if (value <= minActivatingDeposit) {
        stakedEthToken.mint(recipient, value);
        return;
    }
    ...
}

Any deposit equal or lower than minActivatingDeposit would mint sETH2 tokens directly. That is, without putting the deposit in the queue before minting. Given minActivatingDeposit is set to the max uint256, today all deposits mint sETH2. Regardless of their size. That all seems intended.

However, I wanted to make sure you're aware of the fact that even if you set minActivatingDeposit to some sensible lower value (so as to prevent large deposits minting sETH2 and instead go to the queue), anyone could still mint whatever sETH2 directly, skipping the queue altogether. In a single transaction. All they'd need is to stake via a smart contract like the one below. I'm making it simple and quite explicit to show the use case.

//SPDX-License-Identifier: MIT
pragma solidity 0.7.5;

import "./Pool.sol";
import "../tokens/StakedEthToken.sol";

contract QueueSkipper {
    Pool private constant pool = Pool(0xC874b064f465bdD6411D45734b56fac750Cda29A);
    StakedEthToken private constant sETH2 = StakedEthToken(0xFe2e637202056d30016725477c5da089Ab0A043A);

    function stake() external payable {
        uint256 initialTokenBalance = sETH2.balanceOf(msg.sender);

        uint256 maxDepositPerCall = pool.minActivatingDeposit();

        uint256 numberOfCalls = msg.value / maxDepositPerCall;
        
        for (uint256 i = 0; i < numberOfCalls; i++) {
            pool.stakeOnBehalf{value: maxDepositPerCall}(msg.sender);
        }

        uint256 remaining = msg.value - maxDepositPerCall * numberOfCalls;
        if (remaining > 0) {
            pool.stakeOnBehalf{value: remaining}(msg.sender);
        }
        
        require(sETH2.balanceOf(msg.sender) == initialTokenBalance + msg.value);
    }
}

At its core, the contract simply stakes ETH in repeated calls, ensuring that each call has a max of minActivatingDeposit ETH. This way each individual call always mints sETH2, regardless of the fact that the total deposit could be much larger than minActivatingDeposit. This allows the staker to skip the deposit queue altogether, in a single transaction. Needless to say the staker could do it without the smart contract using in multiple txs, but that might be more expensive.

Perhaps you were already aware of this. Though I wasn't able to find it mentioned in the available audit reports. So just in case you weren't, I thought it best to bring it up.