From 54a235f8959ecffb1916cf5693ec9bbd695cbf71 Mon Sep 17 00:00:00 2001
From: Vladislav Volosnikov <vladyan18@gmail.com>
Date: Sat, 5 Aug 2023 00:12:41 +0200
Subject: [PATCH] Refactor Governor proposal struct for efficient access
 (#4495)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
---
 contracts/governance/Governor.sol | 51 +++++++++++++------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
index b9874c820..9d9e2eb59 100644
--- a/contracts/governance/Governor.sol
+++ b/contracts/governance/Governor.sol
@@ -40,22 +40,13 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint32 voteDuration;
         bool executed;
         bool canceled;
-    }
-
-    struct ProposalExtra {
         uint48 eta;
     }
 
-    // Each object in this should fit into a single slot so it can be cached efficiently
-    struct ProposalFull {
-        ProposalCore core;
-        ProposalExtra extra;
-    }
-
     bytes32 private constant _ALL_PROPOSAL_STATES_BITMAP = bytes32((2 ** (uint8(type(ProposalState).max) + 1)) - 1);
     string private _name;
 
-    mapping(uint256 => ProposalFull) private _proposals;
+    mapping(uint256 => ProposalCore) private _proposals;
 
     // This queue keeps track of the governor operating on itself. Calls to functions protected by the
     // {onlyGovernance} modifier needs to be whitelisted in this queue. Whitelisting is set in {_beforeExecute},
@@ -144,13 +135,16 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
      * @dev See {IGovernor-state}.
      */
     function state(uint256 proposalId) public view virtual override returns (ProposalState) {
-        // ProposalCore is just one slot. We can load it from storage to memory with a single sload and use memory
-        // object as a cache. This avoid duplicating expensive sloads.
-        ProposalCore memory core = _proposals[proposalId].core;
+        // ProposalCore is just one slot. We can load it from storage to stack with a single sload
+        ProposalCore storage proposal = _proposals[proposalId];
+        bool proposalExecuted = proposal.executed;
+        bool proposalCanceled = proposal.canceled;
 
-        if (core.executed) {
+        if (proposalExecuted) {
             return ProposalState.Executed;
-        } else if (core.canceled) {
+        }
+
+        if (proposalCanceled) {
             return ProposalState.Canceled;
         }
 
@@ -190,28 +184,28 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
      * @dev See {IGovernor-proposalSnapshot}.
      */
     function proposalSnapshot(uint256 proposalId) public view virtual override returns (uint256) {
-        return _proposals[proposalId].core.voteStart;
+        return _proposals[proposalId].voteStart;
     }
 
     /**
      * @dev See {IGovernor-proposalDeadline}.
      */
     function proposalDeadline(uint256 proposalId) public view virtual override returns (uint256) {
-        return _proposals[proposalId].core.voteStart + _proposals[proposalId].core.voteDuration;
+        return _proposals[proposalId].voteStart + _proposals[proposalId].voteDuration;
     }
 
     /**
      * @dev See {IGovernor-proposalProposer}.
      */
     function proposalProposer(uint256 proposalId) public view virtual override returns (address) {
-        return _proposals[proposalId].core.proposer;
+        return _proposals[proposalId].proposer;
     }
 
     /**
      * @dev See {IGovernor-proposalEta}.
      */
     function proposalEta(uint256 proposalId) public view virtual override returns (uint256) {
-        return _proposals[proposalId].extra.eta;
+        return _proposals[proposalId].eta;
     }
 
     /**
@@ -311,20 +305,17 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         if (targets.length != values.length || targets.length != calldatas.length || targets.length == 0) {
             revert GovernorInvalidProposalLength(targets.length, calldatas.length, values.length);
         }
-        if (_proposals[proposalId].core.voteStart != 0) {
+        if (_proposals[proposalId].voteStart != 0) {
             revert GovernorUnexpectedProposalState(proposalId, state(proposalId), bytes32(0));
         }
 
         uint256 snapshot = clock() + votingDelay();
         uint256 duration = votingPeriod();
 
-        _proposals[proposalId].core = ProposalCore({
-            proposer: proposer,
-            voteStart: SafeCast.toUint48(snapshot),
-            voteDuration: SafeCast.toUint32(duration),
-            executed: false,
-            canceled: false
-        });
+        ProposalCore storage proposal = _proposals[proposalId];
+        proposal.proposer = proposer;
+        proposal.voteStart = SafeCast.toUint48(snapshot);
+        proposal.voteDuration = SafeCast.toUint32(duration);
 
         emit ProposalCreated(
             proposalId,
@@ -357,7 +348,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint48 eta = _queueOperations(proposalId, targets, values, calldatas, descriptionHash);
 
         if (eta != 0) {
-            _proposals[proposalId].extra.eta = eta;
+            _proposals[proposalId].eta = eta;
             emit ProposalQueued(proposalId, eta);
         } else {
             revert GovernorQueueNotImplemented();
@@ -406,7 +397,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         );
 
         // mark as executed before calls to avoid reentrancy
-        _proposals[proposalId].core.executed = true;
+        _proposals[proposalId].executed = true;
 
         // before execute: register governance call in queue.
         if (_executor() != address(this)) {
@@ -494,7 +485,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
                 _encodeStateBitmap(ProposalState.Executed)
         );
 
-        _proposals[proposalId].core.canceled = true;
+        _proposals[proposalId].canceled = true;
         emit ProposalCanceled(proposalId);
 
         return proposalId;