enrichSigHash : The order of function calling would lead to multiple issues.

[hats-bug-reporter]

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

Description:
Description\

AbstractTxSerializer is inherited by the TxSerilier and RefuelTxSerilzer.

There are three different functions . partiallySignOutgoingTransaction, serializeOutgoingTransaction and enrichSigHash.

The enrichSigHash should be able to call even if the serializeOutgoingTransaction set the isFinished() is true.

But, the function enrichSigHash has the following check. when the serializeOutgoingTransaction comppletes and update the _serializing.state, then the enrichSigHash will not pass through.

enrichSigHash

    function enrichSigHash(uint256 inputIndex, uint256 count) public virtual onlyRelayer {
        require(_skeleton.hasSufficientInputs, "IVM");
        require(_skeleton.sigHashes[inputIndex] == bytes32(0), "AH");
        require(!isFinished(), "AF"); --->> this is not needed.

The another issue is,

serializeOutgoingTransaction updates the _skeleton.tx.inputs[i].scriptSig by calling the _writeScriptSigs

The function enrichSigHash uses this _skeleton.tx.inputs[i].scriptSig to wrtie the _sigHashSerializer

refer the code flow

enrichSigHash - serializeTx -> serializeTransactionInputs

if relayer calls the enrichSigHash function first , then then _skeleton.tx.inputs[i].scriptSig written will not be a valid value.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

enrichSigHash

    function enrichSigHash(uint256 inputIndex, uint256 count) public virtual onlyRelayer {
        require(_skeleton.hasSufficientInputs, "IVM");
        require(_skeleton.sigHashes[inputIndex] == bytes32(0), "AH");
        require(_skeleton.tx.hash =! bytes32(0)); -->> add this line
        require(!isFinished(), "AF"); --->> remove 

[aktech297]

@party-for-illuminati

[party-for-illuminati]

The enrichSigHash should be able to call even if the serializeOutgoingTransaction set the isFinished() is true.

"The enrichSigHash should be able to call even if the serializeOutgoingTransaction set the isFinished() is true." - why would it be needed?

[aktech297]

We think that the enrichSigHash and serializeOutgoingTransaction both are different functions.
Pls explain the sequence of calling these three functions
partiallySignOutgoingTransaction, serializeOutgoingTransaction and enrichSigHash.

[party-for-illuminati]

We think that the enrichSigHash and serializeOutgoingTransaction both are different functions. Pls explain the sequence of calling these three functions partiallySignOutgoingTransaction, serializeOutgoingTransaction and enrichSigHash.

https://github.com/hats-finance/illuminex-0x0bb4aa1f58719707405c231fcdf0b405714799cf/blob/main/docs/out/withdraw_flow/Withdraw%20flow.png