Livepeer contest - byterocket's results

Handle

byterocket

Vulnerability details

Impact

Caching the array length outside a loop in a variable saves reading it on each iteration, as long as the array's length is not changed during the loop.

Therefore, the following loops could be refactored:

arbitrum-lpt-bridge:
    - L1/gateway/L1Migrator -> Line 472
    - L2/gateway/L2Migrator -> Line 197

Recommended Mitigation Steps

Use the following pattern for refactoring:

uint256 len = array.length;
for (uint256 i; i < len; i++) {
    // ...
}

Handle

byterocket

Vulnerability details

Impact

Assigned operations to constant variables are re-evaluated every time. See here.

Therefore, refactor following AccessControl roles from being constant to immutable:

arbitrum-lpt-bridge:
  - L2/gateway/L2Migrator
    * GOVERNOR_ROLE
  - ControlledGateway
    * GOVERNOR_ROLE
  - L1/gateway/L1Migrator
    * GOVERNOR_ROLE
  - token/LivepeerToken
    * MINTER_ROLE
    * BURNER_ROLE

Handle

byterocket

Vulnerability details

Impact

The documentation for OpenZeppelins AccessControl contract states: "By default, the admin role for all roles is DEFAULT_ADMIN_ROLE". See here.

Therefore it is unnecessary to call _setRoleAdmin for a role with DEFAULT_ADMIN_ROLE as argument.

Following contracts call _setRoleAdmin with DEFAULT_ADMIN_ROLE as argument in their constructor:

arbitrum-lpt-bridge:
  - ControlledGateway.sol
  - L2/gateway/L2Migrator.sol
  - token/LivepeerToken.sol
  - L1/gateway/L1Migrator.sol

Recommended Mitigation Steps

Remove the unnecessary calls and, in case this is needed, emit AccessControl's RoleAdminChanged events directly with emit RoleAdminChanged(role, DEFAULT_ADMIN_ROLE, DEFAULT_ADMIN_ROLE).