First depositors can avoid the deposit fee

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x40c4b87d9393cd005e5a281148f04a3ecead3d25a2a9c1043b3722e7f9bd9b0a Severity: low

Description: Description\ The deposit fee has a default value of zero:

    uint256 public depositFee = 0; // possible fee to cover bridging costs

This fee can be updated by the owner using the UpdateDepositFee() function. However, after deploying the codebase, some users might immediately call the deposit() function or front-run the UpdateDepositFee() function to avoid the deposit fee.

Since the deposit fee is a percentage of the deposit amount, avoiding this fee on large deposits can lead to a loss of funds for the protocol.

Recommendation\ consider initlizing depositFee in constructor()

[0xdanial]

EDIT: there is no need for constructor(), just initialize depositFee with desired fee amount.

uint256 public depositFee = X;

[0xRizwan]

1) Protocol might intend to keep 0 deposit fee initially as the updateDepositFee() does not restrict fee to be greater than 0.

    function updateDepositFee(uint256 newFee) public onlyOwner {
        require(newFee <= MAX_DEPOSIT_FEE, ">MaxFee");
        depositFee = newFee;
        emit UpdateDepositFee(newFee);
    }

2) Owner is trusted and assumed to be non-stupid as per contest rules:

Issues about the ability for multi-sig owner to set parameters in a way breaking the contract (they are trusted to be both non-malicious and non-stupid)

so, it completely depends on protocol to charge the fee at initially. Usually, such fee params are initialized in deployment script and scripts are not made available/OOS.

avoiding this fee on large deposits can lead to a loss of funds for the protocol.

Check #9 comments

Therefore, i believe its non-issue.

[0xdanial]

@0xRizwan thanks for your comment.

1. As the sponsor said: "The protocol or team does not collect deposit fees; they are naturally deducted if necessary (e.g., bridging fees)." Therefore, it would be an issue because users will avoid bridge fees.

2. There is nothing wrong with the owner's action, so I already assume the owner is trusted and non-stupid. As I reported, users can front-run the updateDepositFee() function to avoid the deposit fee.

3. The report is not about the deployment script, and it's not available or mentioned as out-of-scope (OOS).

This issue can lead to a loss of funds and would typically be marked as Medium severity, but due to its likelihood, I have marked it as Low severity. I believe it is at least a valid Low issue.

Let's see the @ilzheev opinion.

[ilzheev]

@0xdanial if the contract is just deployed, it cannot mint stakingToken because ownership transfer has not made yet.

Deployment workflow:

• deploy multisig
• deploy stToken.sol
• deploy stTokenMinter.sol
• transfer ownership of minter to multisig
• setup minter parameters (fees, minWithdrawal, etc.)
• transfer token ownership to minter

[0xdanial]

thanks @ilzheev I think this is valid because the flow you provided is not mentioned on the Readme page of the contest. EDIT: and deploy script is not available

[0xarshia]

@ilzheev i want to thank from sponsor for reviewing the code carefully but in this case, it seems valid due to platform terms

usually in contest platforms the information about codebase flow will be set and will not be changed in the audit period

and what is written is written and auditors only have to work based on those rules and data

so because of this reason in contest platforms usually sponsors newly provided data cannot be a valid reason for the invalidating issue in most cases

cause otherwise owner would bounce from one flow to another in order to not payout the rewards (for e.g. telling flow will be first calling func a then b or b first then a ) just want to explain why this shouldn't happen

However, i completely know your goal here is not this and you may unintentionally not follow those standards here but

thanks for your understanding i believe it's a valid issue cause there is no INFO in README pages of the audit that flow would be like this as auditors always try to find what happens in worst cases and because there was no mention of the flow this seems valid finding

[ilzheev]

@0xarshia Okay, but assuming there is no deployment workflow:

However, after deploying the codebase, some users might immediately call the deposit() function or front-run the UpdateDepositFee() function to avoid the deposit fee.

If after the deploying stTokenMinter.sol anyone calls deposit(), tx will always be reverted.

We keep it invalid.