hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

0 stars 1 forks source link

tx's can end up being stuck #75

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb824a53f38852f4597382ed81a4a6679b30508e556ee576bb37c5352e4399c45 Severity: high

Description:

Description

function startOutgoingTxSerializing starts an outgoing Tx. Inside this function createSerializer is called to create a new serializer:

    function startOutgoingTxSerializing() public onlyRelayer {
        uint256 _index = outboundTransactionsCount;
        require(address(_serializers[_index]) == address(0), "AC");

        (OutgoingQueue.OutgoingTransfer[] memory _transfers,) = queue.popBufferedTransfersToBatch();
        require(_transfers.length > 0, "NT");

->        TxSerializer _sr = serializerFactory.createSerializer(
            AbstractTxSerializer.FeeConfig({
                outgoingTransferCost: BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte,
                incomingTransferCost: BYTES_PER_INCOMING_TRANSFER * satoshiPerByte
            }),
            _transfers
        );

        _serializers[_index] = _sr;
        _sr.toggleRelayer(msg.sender);
    }

The feeConfig is calculated:

            AbstractTxSerializer.FeeConfig({
                outgoingTransferCost: BYTES_PER_OUTGOING_TRANSFER * satoshiPerByte,
                incomingTransferCost: BYTES_PER_INCOMING_TRANSFER * satoshiPerByte

Whenever the outgoing Tx has to be completed the following function is called:

    function finaliseOutgoingTxSerializing() public onlyRelayer {
        uint256 _index = outboundTransactionsCount++;
        TxSerializer _sr = _serializers[_index];

        (bytes memory txData, bytes32 txHash) = _sr.getRaw();

        emit SignedTxBroadcast(txHash, _index, txData);

        (uint256 _changeSystemIdx, uint64 _changeValue, uint256 _changeIdx) = _sr.getChangeInfo();
        _addOutboundTransaction(
            _index,
            txHash,
            _changeIdx,
            _changeSystemIdx,
            _changeValue
        );
    }

For this function to successfully execute, the _serializers have to be Finished

require(_serializing.state == TxSerializerLib.TxSerializingState.Finished, "NF");

The problem however is that if the network fees were to go up due to whatever external reason, the Tx's could get stuck with currently no way to increase the set fees inside feeConfig. Note that increasing satoshiPerByte is ineffective since the fees have already been set upon calling the function.

There is also currently no way to retry this transaction

Recommendation

implement some sort of logic to retry this transaction if it were to get stuck. Could be with a nonce or simply allowing the gas to be increased whenever necessary.

party-for-illuminati commented 2 months ago

https://github.com/hats-finance/illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf/blob/3ad7c2aedf991493aab45d3e0847b7e07f5c0d07/packages/contracts/contracts/illuminex/xengine/chains/btc/wallet/VaultBitcoinWallet.sol#L292