QA Report

[code423n4]

Title: Not verified input
Severity: Low Risk

external / public functions parameters should be validated to make sure the address is not 0.
Otherwise if not given the right input it can mistakenly lead to loss of user funds.

    
    TWABDelegator.sol.permitAndMulticall _from
    Delegation.sol._executeCall to
    PermitAndMulticall.sol._permitAndMulticall _from
    TWABDelegator.sol.stake _to
    TWABDelegator.sol._requireDelegateeNotZeroAddress _delegatee
    TWABDelegator.sol.setRepresentative _representative
    TWABDelegator.sol._computeAddress _delegator
    TWABDelegator.sol._requireDelegatorNotZeroAddress _delegator
    TWABDelegator.sol.updateDelegatee _delegatee
    TWABDelegator.sol.isRepresentativeOf _delegator
    TWABDelegator.sol._transfer _to
    TWABDelegator.sol._requireContract _address
    TWABDelegator.sol._transferCall _to
    LowLevelDelegator.sol._computeSalt _delegator
    TWABDelegator.sol.createDelegation _delegatee
    TWABDelegator.sol.transferDelegationTo _to
    TWABDelegator.sol.unstake _to
    TWABDelegator.sol.computeDelegationAddress _delegator
    TWABDelegator.sol.fundDelegation _delegator
    TWABDelegator.sol.isRepresentativeOf _representative
    TWABDelegator.sol._delegateCall _delegatee
    TWABDelegator.sol.getDelegation _delegator
    TWABDelegator.sol._requireRecipientNotZeroAddress _to

Title: Named return issue
Severity: Low Risk

Users can mistakenly think that the return value is the named return, but it is actually the actualreturn statement that comes after. To know that the user needs to read the code and is confusing.
Furthermore, removing either the actual return or the named return will save gas.

    TWABDelegator.sol, multicall
    PermitAndMulticall.sol, _multicall

Title: Never used parameters
Severity: Low Risk

Those are functions and parameters pairs that the function doesn't use the parameter. In case those functions are external/public this is even worst since the user is required to put value that never used and can misslead him and waste its time.

    Delegation.sol: function _executeCall parameter value isn't used. (_executeCall is internal)
    TWABDelegator.sol: function _executeCall parameter _data isn't used. (_executeCall is internal)
    Delegation.sol: function _executeCall parameter data isn't used. (_executeCall is internal)
    Delegation.sol: function _executeCall parameter to isn't used. (_executeCall is internal)

Title: Mult instead div in compares
Severity: Low Risk

To improve algorithm precision instead using division in comparison use multiplication in the following scenario:
        
        Instead a < b / c use a * c < b. 
    
In all of the big and trusted contracts this rule is maintained.

    TWABDelegator.sol, 625, require(block.timestamp >= _delegation.lockUntil(), "TWABDelegator/delegation-locked");
    TWABDelegator.sol, 601, require(_amount > 0, "TWABDelegator/amount-gt-zero");
    TWABDelegator.sol, 641, require(_lockDuration <= MAX_LOCK, "TWABDelegator/lock-too-long");

Title: Override function but with different argument location
Severity: Low/Med Risk

    TWABDelegator.sol._computeAddress inherent LowLevelDelegator.sol._computeAddress but the parameters does not match
    TWABDelegator.sol.constructor inherent LowLevelDelegator.sol.constructor but the parameters does not match

[PierrickGT]

This report seems to have been written with the use of an automatic tool that analyzed the code base and reported a lot of false positives that the warden didn't even take the time to properly review.
So I've only fixed one issue about confusing return values.

About Not verified input:
We do check for address zero in most of the functions and since the report isn't very descriptive and full of false positives, I have not dug deeper into it.

About Named return issue:
We can indeed improve the return values for the multicall functions.
I've implemented the changes in the following PR: pooltogether/v4-twab-delegator#19

About never used parameters:

• value, data and to are used in _executeCall of Delegation: https://github.com/pooltogether/v4-twab-delegator/blob/21bb53b2ea54a248bbd1d3170dbadd3a0c83d874/contracts/Delegation.sol#L68
• _data is used in _executeCall of TwabDelegator: https://github.com/pooltogether/v4-twab-delegator/blob/21bb53b2ea54a248bbd1d3170dbadd3a0c83d874/contracts/TWABDelegator.sol#L553

About Mult instead div in compares:
The lines mentioned are comparisons and not math multiplications, so the suggestion is not relevant.

About Override function but with different argument location:
_computeAddress is indeed present in TWABDelegator and LowLevelDelegator but since the functions signature are different, the two don't enter in conflict/collision.
Same remark about the constructor.