Governable Manual Review Findings

GEL-01M: Inefficient Validation of Uniqueness

Type	Severity	Location
Gas Optimization		Governable.sol:L26-L28

Description:

The Governable::onlyConsensus function will validate that each signer entry is unique by iterating all previously validated signers until that point which is highly inefficient and unscalable for large signer arrays.

Impact:

A severity of minor has been specified given that the system is presently not scalable for large signer arrays but will be once this exhibit has been remediated.

Example:

Copy
12/**
13 * @notice Verifies the message hash against the signatures. Requires a majority.
14 * @param _ethHash hash to verify
15 * @param _signatures governance hash signatures
16 */
17function onlyConsensus(bytes32 _ethHash, bytes[] memory _signatures) internal view {
18    uint256 members = LibGovernance.membersCount();
19    require(_signatures.length <= members, 'Governance: Invalid number of signatures');
20    require(_signatures.length > members / 2, 'Governance: Invalid number of signatures');
21
22    address[] memory signers = new address[](_signatures.length);
23    for (uint256 i = 0; i < _signatures.length; i++) {
24        address signer = ECDSA.recover(_ethHash, _signatures[i]);
25        require(LibGovernance.isMember(signer), 'Governance: invalid signer');
26        for (uint256 j = 0; j < i; j++) {
27            require(signer != signers[j], 'Governance: duplicate signatures');
28        }
29        signers[i] = signer;
30    }
31}

Recommendation:

We advise the code to instead ensure the caller has ordered the signatures in an ascending / descending manner, validating this order during the code's execution.

This is a common optimization to this problem whereby the previously validated signer is stored to a local variable that is compared on each iteration, ensuring that each new signer validated is less / greater than the previous one thereby guaranteeing all signers are unique.

Alleviation (54fd570de24631ca65a7cea022aebe43225a08c7):

The uniqueness validation was optimized as advised, ensuring that the signers recovered from the signatures are provided in a strictly ascending order and thus greatly optimizing the codebase and permitting it to scale properly.