search

code-423n4 / 2021-10-tally-findings

0 stars 0 forks source link

Unnecessary `SLOAD`s in `EmergencyGovernable.onlyTimelockOrEmergencyGovernance()` #61

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

pants

Vulnerability details

The modifier EmergencyGovernable.onlyTimelockOrEmergencyGovernance() calls governor() twice, resulting with two SLOADs operations for the _owner state variable that was inherited from Ownable.

Impact

Storage reads are much more expensive than reading local variables.

Tool Used

Manual code review.

Recommended Mitigation Steps

Call governor() only once and cache the return value in a local variable. Then, use the local variable to load this value again.

Shadowfiend commented 2 years ago

May or may not update this, depending on available time.