Covalent contest - harleythedog's results

Handle

harleythedog

Vulnerability details

Impact

There are a few functions declared as public that are never called internally within the contract. It is best practice to mark such functions as external instead, as this saves gas (especially in the case where the function takes arguments, as external functions can read arguments directly from calldata instead of having to allocate memory). The following functions could be changed from public to external:

• getValidatorsDetails()
• getDelegatorDetails(address)
• getMetadata()

Proof of Concept

getValidatorsDetails: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=getValidatorsDetails

getDelegatorDetails: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=function-,getDelegatorDetails,-(address%20delegator)%20public

getMetadata: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=function-,getMetadata,-()%20public%20view%20returns

Tools Used

Manual Inspection.

Recommended Mitigation Steps

Change these functions from public to external.

Handle

harleythedog

Vulnerability details

Impact

The addValidator function is a non-view function that reads the storage variable validatorsN multiple times. Reading from storage is significantly more expensive than reading from memory, so it would save a lot of gas to first copy validatorsN into memory at the start of the function, and then read from this memory variable.

Proof of Concept

See the addValidator function here: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=function-,addValidator,-(address%20validator%2C%20address

If we instead changed this code to be:

    uint128 N = validatorsN;
    validators[N]._address = validator;
    validators[N].operator = operator;
    validators[N].commissionRate = commissionRate;
    emit ValidatorAdded(N, validator, operator);
    validatorsN += 1;

we would remove 3 SLOADS in favour of much cheaper MLOADS, saving a lot of gas.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Change storage reads to memory reads as described above.

Handle

harleythedog

Vulnerability details

Impact

The IERC20 variable named "CQT" is set in the "initialize" function (as a constant value, it does not depend on the input to the function call), and this variable is not written to anywhere else in the contract. It would save gas to declare this variable as a constant and initialize it in the contract as a global variable.

Proof of Concept

Variable declaration here: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=id%20-%3E%20validator%20instance-,IERC20%20CQT,-%3B

Variable initialization (and only location where variable is written to) here: https://github.com/code-423n4/2021-10-covalent/blob/main/contracts/DelegatedStaking.sol#:~:text=CQT%20%3D%20IERC20(0xD417144312DbF50465b1C641d016962017Ef6240)%3B

Tools Used

Manual inspection.

Recommended Mitigation Steps

Declare the "CQT" variable as a constant variable in the contract and initialize it similarily.