`RefuelTxSerializerFactory.sol` and `TxSerializerFactory.sol` factories are not initialized so some functions will always revert

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

Description: Description\

VaultBitcoinWallet.sol contract passed the RefuelTxSerializerFactory and TxSerializerFactory address as an argument in its constructor

From these factories, serializers are created by calling startRefuelTxSerializing() and `

    function startRefuelTxSerializing(bytes32 outgoingTxHash) public onlyRelayer {

           . . . some code . . . 

@>        RefuelTxSerializer _sr = refuelSerializerFactory.createRefuelSerializer(_serializers[_index]);

           . . . some code . . . 

    }

startOutgoingTxSerializing()

    function startOutgoingTxSerializing() public onlyRelayer {

           . . . some code . . . 

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

           . . . some code . . . 

    }

Both refuelSerializerFactory.createRefuelSerializer() and serializerFactory.createSerializer() can only be accessed by allowedCreator which is nothing but VaultBitcoinWallet contract.

    function createRefuelSerializer(TxSerializer parent) public returns (RefuelTxSerializer _serializer) {
        require(msg.sender == allowedCreator, "NAC");

and,

    function createSerializer(
        TxSerializer.FeeConfig memory _fees,
        OutgoingQueue.OutgoingTransfer[] memory _transfers
    ) public returns (TxSerializer _serializer) {
        require(msg.sender == allowedCreator, "NAC");

Both, RefuelTxSerializerFactory.sol and TxSerializerFactory.sol inherits AbstractTxSerializerFactory.sol as base contract where init() function is implemented.

    function init(address _creator) public {
        require(msg.sender == initializer && !isInitialized);
        isInitialized = true;

        allowedCreator = _creator;

        inputsStorage = ITxInputsStorage(_creator);
        secretsStorage = ITxSecretsStorage(_creator);
    }

init() function is not called in both RefuelTxSerializerFactory.sol and TxSerializerFactory.sol contracts and even this is not initialized in deployment script but init() can only be called by initializer.

Since, this function is not called anywhere in contract nor it is atomically called in contracts so both createRefuelSerializer() and createSerializer() will always revert and startRefuelTxSerializing() and startOutgoingTxSerializing() will also revert. Therefore, this would break the one of the core contracts functionalities. Since Refuel transaction serializing and outgoing transaction serializing can not acheived.

Recommendation to fix\ Initilize the factory contracts init() function atomically

OR.

Consider, deploying the factory contracts via VaultBitcoinWallet.sol constructor and call init() function to initialize factories.

For example understanding, consider below changes:

+  event ContractsDeployed(address TxSerializerFactory , address RefuelTxSerializerFactory );

    TxSerializerFactory public immutable serializerFactory;
    RefuelTxSerializerFactory public immutable refuelSerializerFactory;

    constructor(
        address _prover,
        bytes memory _offchainSigner,
        BitcoinUtils.WorkingScriptSet memory _loadScripts,
        address _queue,
-       TxSerializerFactory _serializerFactory,
-       RefuelTxSerializerFactory _refuelSerializerFactory
+      BitcoinUtils.WorkingScriptSet memory _scripts
    )
    BitcoinAbstractWallet(_prover)
    RotatingKeys(keccak256(abi.encodePacked(block.number)), type(VaultBitcoinWallet).name)
    {
        btcToken = new PeggedBTC();
        queue = OutgoingQueue(_queue);

        workingScriptSet = _loadScripts;

        IScript[] memory _scripts = new IScript[](3);
        _scripts[0] = workingScriptSet.vaultScript;
        _scripts[1] = workingScriptSet.p2pkhScript;
        _scripts[2] = workingScriptSet.p2shScript;

        _setSupportedScripts(_scripts);
        _updateOffchainSignerPubKey(_offchainSigner);

        feeSetter = msg.sender;

-        serializerFactory = _serializerFactory;
+        serializerFactory = new TxSerializerFactory(_scripts);
+       serializerFactory.init(address(this);              @audit // setting VaultBitcoinWallet address as allowedCreator address
-        refuelSerializerFactory = _refuelSerializerFactory;
+        refuelSerializerFactory = new RefuelTxSerializerFactory(_scripts);
+        refuelSerializerFactory.init(address(this);              @audit // setting VaultBitcoinWallet address as allowedCreator address

+     emit ContractsDeployed(serializerFactory, refuelSerializerFactory );
    }

hats-finance / illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf

`RefuelTxSerializerFactory.sol` and `TxSerializerFactory.sol` factories are not initialized so some functions will always revert #50