Emergency consensus withdraw and account transfer functions are not implemented in `stROSEMinter.sol`

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x1cbefdbadef6bc8bae581e330e19a557797e98e7d4637d72cb667aa17350afc1 Severity: medium

Description: Description\ stROSEMinter.sol has implemented following emergency functions from OASIS's Subcall library:

    function emergencyTakeReceiptDelegate(uint64 receiptId) public onlyOwner returns (uint128 shares) {
        shares = Subcall.consensusTakeReceiptDelegate(receiptId);
        emit EmergencyTakeReceiptDelegate(receiptId, shares);
    }

    function emergencyUndelegate(StakingAddress from, uint128 shares, uint64 receiptId) public onlyOwner
    {
        Subcall.consensusUndelegate(from, shares, receiptId);
        emit EmergencyUndelegate(from, shares, receiptId);
    }

    function emergencyTakeReceiptUndelegateStart(uint64 receiptId) public onlyOwner returns (uint64 epoch, uint64 endReceiptId) {
        (epoch, endReceiptId) = Subcall.consensusTakeReceiptUndelegateStart(receiptId);
        emit EmergencyTakeReceiptUndelegateStart(receiptId, epoch, endReceiptId);
    }

    function emergencyTakeReceiptUndelegateDone(uint64 endReceiptId) public onlyOwner returns (uint128 amount) {
        amount = Subcall.consensusTakeReceiptUndelegateDone(endReceiptId);
        emit EmergencyTakeReceiptUndelegateDone(endReceiptId, amount);
    }

All of these functions can be invoked by contract owner in case of emergency via Subcall library of Oasis chain.

The issue is that, Two major important functionalities from SubCall is not implemented as external functions which must be called by contract owner. These are internal functions like consensusWithdraw() and accountsTransfer(). It should be noted that, the contract has explicitely used constants like CONSENSUS_WITHDRAW and ACCOUNTS_TRANSFER which indicates the use of these internal functions. These are implemented as:

    string private constant CONSENSUS_WITHDRAW = "consensus.Withdraw";
    string private constant ACCOUNTS_TRANSFER = "accounts.Transfer";

   . . . some code . . . 

    function consensusWithdraw(StakingAddress to, uint128 value) internal {
        (uint64 status, bytes memory data) = _subcallWithToAndAmount(
            CONSENSUS_WITHDRAW,
            to,
            value,
            ""
        );

        if (status != 0) {
            revert ConsensusWithdrawError(status, string(data));
        }
    }

    function accountsTransfer(address to, uint128 value) internal {
        (uint64 status, bytes memory data) = _subcallWithToAndAmount(
            ACCOUNTS_TRANSFER,
            StakingAddress.wrap(bytes21(abi.encodePacked(uint8(0x00), to))),
            value,
            ""
        );

        if (status != 0) {
            revert AccountsTransferError(status, string(data));
        }
    }

consensusWithdraw() function is used to transfer from an account in this runtime to a consensus staking account and accountsTransfer() is used to perform a transfer to another account address. Both of these are implemented as internal function and can not be accessed by owner.

Impact\ Consensus withdraw and account transfer functions from subcall can not be called in case of emergency. This is due to missing implementation as external/public function which is expected to be called by contract owner. In case of emergency, both of these functions are important and must be implemented otherwise this would break the core functionality of stROSEMinter contract.

Recommendations\ Consider implementing emergencyConsensusWithdraw() and emergencyAccountsTransfer() functions in stROSEMinter.sol contract.

+    function emergencyConsensusWithdraw(StakingAddress to, uint128 value) public onlyOwner {
+        Subcall.consensusWithdraw(to, value);
+    }

+    function emergencyAccountsTransfer(StakingAddress to, uint128 value) public onlyOwner {
+        Subcall.accountsTransfer(to, value);
+    }

[ilzheev]

Subcall lib is provided by Oasis dev team and implemented "as is". Not all functions of Subcall lib are used by stROSEMinter – particularly consensusWithdraw and accountsTransfer are not used, and so we do not need emergency functions to call them.