minConfirmations check does not follow best practices

[hats-bug-reporter[bot]]

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

Description:

Description

Inside BitcoinProver.sol the following check is performed inside function ProveTransaction

    function proveTransaction(
        bytes memory _transaction,
        BitcoinMerkleTree.ProofNode[] memory _transactionMerklePath,
        uint256 _txOutIndex,
        bytes memory _blockHeader,
        TEERollup.FullComputationsProof memory confirmationsChunkProof
    ) public view returns (IBitcoinDepositProcessingCallback.Transaction memory) {
        BitcoinUtils.BitcoinTransaction memory _tx = BitcoinUtils.deserializeTransaction(Buffer.BufferIO(_transaction, 0));
        BitcoinUtils.BitcoinBlockHeaders memory _block = BitcoinUtils.deserializeBlockHeaders(Buffer.BufferIO(_blockHeader, 0));

        BlockChunk memory confirmations = getBlockChunkProof(confirmationsChunkProof);
        require(confirmations.virtualEpochIndex <= 
 getLastAcknowledgedAnchorBlockIndex(), "Invalid epoch");
->       require(confirmations.chunkSize >= minConfirmations + 1, "Too few confirmations blocks");
//..Ommited code

Inside this require statement the chunksize is checked against the minConfirmations + 1.

This however is inconsistent with how minimum values work. Whenever minimum values are used the variable checking against it should pass if it is >= than the minimum value, but as we can see here due to the + 1 in practice it will not pass if it is greater than or equal to minConfirmations

Example

• minConfirmations is set to 5
• the checks against minConfirmations should pass if the minimum value is met which is atleast 5 in this case
• But due to the + 1, the user needs to meet atleast 6 to pass, which defeats the meaning of min values

Obviously there is no real impact here, but according to the Hats Severity Evaluation Low findings are valid in the following case:

// Minor deviations from best practices that don't lead to security risks. Small bugs that do not affect the protocol's functionality or security.

This finding fits the description, since it shows a minor deviation from best practices with no affect to the protocol's functionality or security.

Reccomendation

Might want to change the name of minConfirmations to something which does make sense with the use case of this value