Gas in FlashGovernanceArbiter.assertGovernanceApproved(): flashGovernanceConfig.asset and flashGovernanceConfig.amount should get cached

[code423n4]

Handle

Dravee

Vulnerability details

Impact

SLOADs are expensive (~100 gas) compared to MLOADs/MSTOREs (~3 gas). Minimizing them can save gas.

Proof of Concept

The code is as such (see @Audit tags):

File: FlashGovernanceArbiter.sol
60:   function assertGovernanceApproved(
61:     address sender,
62:     address target,
63:     bool emergency
64:   ) public {
65:     if (
66:       pendingFlashDecision[target][sender].unlockTime < block.timestamp && 
67:       IERC20(flashGovernanceConfig.asset).transferFrom(sender, address(this), flashGovernanceConfig.amount) //@audit flashGovernanceConfig.amount SLOAD 1 //@audit flashGovernanceConfig.asset SLOAD 1
68:     ) {
69:       require(
70:         emergency || (block.timestamp - security.lastFlashGovernanceAct > security.epochSize), 
71:         "Limbo: flash governance disabled for rest of epoch"
72:       );
73:       pendingFlashDecision[target][sender] = flashGovernanceConfig;
74:       pendingFlashDecision[target][sender].unlockTime += block.timestamp;
75: 
76:       security.lastFlashGovernanceAct = block.timestamp;
77:       emit flashDecision(sender, flashGovernanceConfig.asset, flashGovernanceConfig.amount, target); //@audit flashGovernanceConfig.amount SLOAD 2 //@audit flashGovernanceConfig.asset SLOAD 2
78:     } else {
79:       revert("LIMBO: governance decision rejected.");
80:     }
81:   }

It's possible to save 2 SLOAD (~200 gas) by caching flashGovernanceConfig.asset in a address variable, and flashGovernanceConfig.amount in a uint256 memory variables and use them instead of reading them repeatedly from storage.

Tools Used

VS Code

Recommended Mitigation Steps

Cache the storage values in memory variables and use them instead of repeatedly reading them from storage.

[jack-the-pug]

Making this the main issue for all: "Cache the storage values in memory" Gas Optimization