Livepeer contest - Jujic's results

Handle

Jujic

Vulnerability details

Impact

Here you could use unchecked{++i} to save gas since it is more efficient then i++.

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1Migrator.sol#L472

for (uint256 i = 0; i < _unbondingLockIds.length; i++)

Tools Used

Remix

Recommended Mitigation Steps

Handle

Jujic

Vulnerability details

Impact

Some of these variables can be cached to slightly reduce gas usage

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L2/gateway/L2LPTDataCache.sol#L92-L93

function l1CirculatingSupply() public view returns (uint256) {
        // After the first update from L1, l1TotalSupply should always be >= l2SupplyFromL1
        // The below check is defensive to avoid reverting if this invariant for some reason violated
        return
            l1TotalSupply >= l2SupplyFromL1
                ? l1TotalSupply - l2SupplyFromL1
                : 0;
    }

Tools Used

Remix

Recommended Mitigation Steps

Consider caching the storage variables in the stack.

Handle

Jujic

Vulnerability details

Impact

Increase gas costs on all onlyRole operations

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L2/token/LivepeerToken.sol#L9-L10

The MINTER_ROLE and BURNER_ROLE variables are marked as constant:

 bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");
 bytes32 public constant BURNER_ROLE = keccak256("BURNER_ROLE");

This results in the keccak operation being performed whenever the variable is used, increasing gas costs relative to just storing the output hash. Changing to immutable will only perform hashing on contract deployment which will save gas.

Tools Used

https://github.com/ethereum/solidity/issues/9232#issuecomment-646131646

Recommended Mitigation Steps

Change the variable to be immutable rather than constant

Handle

Jujic

Vulnerability details

Impact

Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1Migrator.sol#L508

require(
            _l2Addr != address(0),
            "L1Migrator#requireValidMigration: INVALID_L2_ADDR"
        );
        require(
            msg.sender == _l1Addr ||
                recoverSigner(_structHash, _sig) == _l1Addr,
            "L1Migrator#requireValidMigration: FAIL_AUTH"
        );

Tools Used

https://planetcalc.com/9029/

Recommended Mitigation Steps

Shorten the revert strings to fit in 32 bytes.

Handle

Jujic

Vulnerability details

Impact

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L2/gateway/L2LPTDataCache.sol#L65

function decreaseL2SupplyFromL1(uint256 _amount) external onlyL2LPTGateway {

        if (_amount > l2SupplyFromL1) {
            l2SupplyFromL1 = 0;
        } else {
            l2SupplyFromL1 -= _amount;
        }

Tools Used

Remix

Recommended Mitigation Steps

Consider using 'unchecked' where it is safe to do so.

unchecked {
       l2SupplyFromL1 -= _amount;
 }

Handle

Jujic

Vulnerability details

Impact

The local variable used as for loop index need not be initialized to 0 because the default value is 0. Avoiding this anti-pattern can save a few opcodes and therefore a tiny bit of gas.

Proof of Concept

https://github.com/livepeer/arbitrum-lpt-bridge/blob/ebf68d11879c2798c5ec0735411b08d0bea4f287/contracts/L1/gateway/L1Migrator.sol#L472

for (uint256 i = 0; i < _unbondingLockIds.length; i++)

Tools Used

Remix

Recommended Mitigation Steps

Remove explicit 0 initialization of for loop index variable.