Potential silent revert condition due to underflow in `_onActionDeposit`

[hats-bug-reporter[bot]]

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

Description: Description:

In VaultBitcoinWallet, the _onActionDeposit is potential for a silent revert condition due to underflow.

Looking on how illuminex in some other functions did a require check and capture any potential revert condition with a short revert message, this underflow condition is currently not being captured and may revert silently.

    function _onActionDeposit(
        uint64 value,
        bytes memory _vaultScriptHash,
        bytes memory _recoveryData
    ) internal returns (bytes32) {
 ...
        uint64 protocolFees = value * depositFee / 1000;
        if (isExcludedFromFees[destination]) {
            protocolFees = 0;
        }

        uint64 importFees = ((BYTES_PER_INCOMING_TRANSFER + INPUT_EXTRA_FIXED_BYTES_FEE) * satoshiPerByte);
@>      protocolFees += importFees;

@>      uint64 valueAfterFees = value - protocolFees;
...
    }

Initially protocolFees is set as share of depositFee against value amount. Thus when substracting value with protocolFees it will surely not be underflow. But then since there is an importFees being added to protocolFees, this can have a potential underflow. If value is less than protocolFees it will be underflow and reverted.

Currently, there is no check condition value > protocolFees to prevent any unknown revert happening, thus there will be a silent revert.

Scenario:

Impact:

Potential revert due to underflow without any information

Mitigation:

Add a check to make sure every revert is being captured, for example

        uint64 importFees = ((BYTES_PER_INCOMING_TRANSFER + INPUT_EXTRA_FIXED_BYTES_FEE) * satoshiPerByte);
        protocolFees += importFees;
+       require(value => protocolFees, "NDRFLW");
        uint64 valueAfterFees = value - protocolFees;