Merge pull request #16 from GenerationSoftware/gen-318-c4-issue-449

Optimized sloads

Author: asselstine

src/TwabController.sol

@@ -21,6 +21,8 @@ error PeriodOffsetInFuture(uint32 periodOffset);
 // The minimum period length
 uint32 constant MINIMUM_PERIOD_LENGTH = 1 hours;
 
+address constant SPONSORSHIP_ADDRESS = address(1);
+
 /**
  * @title  Time-Weighted Average Balance Controller
  * @author PoolTogether Inc.
@@ -37,7 +39,6 @@ contract TwabController {
   using SafeCast for uint256;
 
   /// @notice Allows users to revoke their chances to win by delegating to the sponsorship address.
-  address public constant SPONSORSHIP_ADDRESS = address(1);
 
   /// @notice Sets the minimum period length for Observations. When a period elapses, a new Observation is recorded, otherwise the most recent Observation is updated.
   uint32 public immutable PERIOD_LENGTH;
@@ -679,7 +680,8 @@ contract TwabController {
     (
       ObservationLib.Observation memory _observation,
       bool _isNewObservation,
-      bool _isObservationRecorded
+      bool _isObservationRecorded,
+      TwabLib.AccountDetails memory accountDetails
     ) = TwabLib.increaseBalances(PERIOD_LENGTH, PERIOD_OFFSET, _account, _amount, _delegateAmount);
 
     // Always emit the balance change event
@@ -692,8 +694,8 @@ contract TwabController {
       emit ObservationRecorded(
         _vault,
         _user,
-        _account.details.balance,
-        _account.details.delegateBalance,
+        accountDetails.balance,
+        accountDetails.delegateBalance,
         _isNewObservation,
         _observation
       );
@@ -717,7 +719,8 @@ contract TwabController {
     (
       ObservationLib.Observation memory _observation,
       bool _isNewObservation,
-      bool _isObservationRecorded
+      bool _isObservationRecorded,
+      TwabLib.AccountDetails memory accountDetails
     ) = TwabLib.decreaseBalances(
         PERIOD_LENGTH,
         PERIOD_OFFSET,
@@ -737,8 +740,8 @@ contract TwabController {
       emit ObservationRecorded(
         _vault,
         _user,
-        _account.details.balance,
-        _account.details.delegateBalance,
+        accountDetails.balance,
+        accountDetails.delegateBalance,
         _isNewObservation,
         _observation
       );
@@ -761,7 +764,8 @@ contract TwabController {
     (
       ObservationLib.Observation memory _observation,
       bool _isNewObservation,
-      bool _isObservationRecorded
+      bool _isObservationRecorded,
+      TwabLib.AccountDetails memory accountDetails
     ) = TwabLib.decreaseBalances(
         PERIOD_LENGTH,
         PERIOD_OFFSET,
@@ -780,8 +784,8 @@ contract TwabController {
     if (_isObservationRecorded) {
       emit TotalSupplyObservationRecorded(
         _vault,
-        _account.details.balance,
-        _account.details.delegateBalance,
+        accountDetails.balance,
+        accountDetails.delegateBalance,
         _isNewObservation,
         _observation
       );
@@ -804,7 +808,8 @@ contract TwabController {
     (
       ObservationLib.Observation memory _observation,
       bool _isNewObservation,
-      bool _isObservationRecorded
+      bool _isObservationRecorded,
+      TwabLib.AccountDetails memory accountDetails
     ) = TwabLib.increaseBalances(PERIOD_LENGTH, PERIOD_OFFSET, _account, _amount, _delegateAmount);
 
     // Always emit the balance change event
@@ -816,8 +821,8 @@ contract TwabController {
     if (_isObservationRecorded) {
       emit TotalSupplyObservationRecorded(
         _vault,
-        _account.details.balance,
-        _account.details.delegateBalance,
+        accountDetails.balance,
+        accountDetails.delegateBalance,
         _isNewObservation,
         _observation
       );

src/libraries/TwabLib.sol

@@ -98,9 +98,9 @@ library TwabLib {
     uint96 _delegateAmount
   )
     internal
-    returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded)
+    returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded, AccountDetails memory accountDetails)
   {
-    AccountDetails memory accountDetails = _account.details;
+    accountDetails = _account.details;
     isObservationRecorded = _delegateAmount != uint96(0);
 
     accountDetails.balance += _amount;
@@ -136,9 +136,9 @@ library TwabLib {
     string memory _revertMessage
   )
     internal
-    returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded)
+    returns (ObservationLib.Observation memory observation, bool isNew, bool isObservationRecorded, AccountDetails memory accountDetails)
   {
-    AccountDetails memory accountDetails = _account.details;
+    accountDetails = _account.details;
 
     if (accountDetails.balance < _amount) {
       revert BalanceLTAmount(accountDetails.balance, _amount, _revertMessage);
@@ -549,15 +549,14 @@ library TwabLib {
     AccountDetails memory _accountDetails,
     uint32 _targetTime
   ) private view returns (ObservationLib.Observation memory prevOrAtObservation) {
-    uint32 currentTime = uint32(block.timestamp);
-
-    uint16 oldestTwabIndex;
-
     // If there are no observations, return a zeroed observation
     if (_accountDetails.cardinality == 0) {
       return
         ObservationLib.Observation({ cumulativeBalance: 0, balance: 0, timestamp: PERIOD_OFFSET });
     }
+    
+    uint32 currentTime = uint32(block.timestamp);
+    uint16 oldestTwabIndex;
 
     (oldestTwabIndex, prevOrAtObservation) = getOldestObservation(_observations, _accountDetails);
 

test/TwabController.t.sol

@@ -10,6 +10,7 @@ import {
   CannotTransferToSponsorshipAddress,
   MINIMUM_PERIOD_LENGTH,
   PeriodLengthTooShort,
+  SPONSORSHIP_ADDRESS,
   PeriodOffsetInFuture
 } from "../src/TwabController.sol";
 import {
@@ -402,7 +403,7 @@ contract TwabControllerTest is BaseTest {
     assertEq(twabController.balanceOf(mockVault, alice), _amount);
     assertEq(twabController.delegateBalanceOf(mockVault, alice), 0);
 
-    address _sponsorshipAddress = twabController.SPONSORSHIP_ADDRESS();
+    address _sponsorshipAddress = SPONSORSHIP_ADDRESS;
     assertEq(twabController.balanceOf(mockVault, _sponsorshipAddress), 0);
     assertEq(twabController.delegateBalanceOf(mockVault, _sponsorshipAddress), 0);
 
@@ -438,7 +439,7 @@ contract TwabControllerTest is BaseTest {
     assertEq(twabController.balanceOf(mockVault, bob), _amount);
     assertEq(twabController.delegateBalanceOf(mockVault, alice), _amount);
     assertEq(twabController.delegateBalanceOf(mockVault, bob), _amount);
-    assertEq(twabController.delegateBalanceOf(mockVault, twabController.SPONSORSHIP_ADDRESS()), 0);
+    assertEq(twabController.delegateBalanceOf(mockVault, SPONSORSHIP_ADDRESS), 0);
 
     twabController.sponsor(bob);
     vm.stopPrank();
@@ -451,7 +452,7 @@ contract TwabControllerTest is BaseTest {
     // Delegate balances have changed
     assertEq(twabController.delegateBalanceOf(mockVault, alice), 0);
     assertEq(twabController.delegateBalanceOf(mockVault, bob), _amount);
-    assertEq(twabController.delegateBalanceOf(mockVault, twabController.SPONSORSHIP_ADDRESS()), 0);
+    assertEq(twabController.delegateBalanceOf(mockVault, SPONSORSHIP_ADDRESS), 0);
   }
 
   function testMint() external {
@@ -657,7 +658,7 @@ contract TwabControllerTest is BaseTest {
 
   function testTransfer_rejectSponsorshipAddress() public {
     twabController.mint(alice, 100e18);
-    address sponsorship = twabController.SPONSORSHIP_ADDRESS();
+    address sponsorship = SPONSORSHIP_ADDRESS;
     vm.expectRevert(abi.encodeWithSelector(CannotTransferToSponsorshipAddress.selector));
     twabController.transfer(alice, sponsorship, 100e18);
   }
@@ -735,7 +736,7 @@ contract TwabControllerTest is BaseTest {
   }
 
   function testDelegateToSponsorship() external {
-    address _sponsorshipAddress = twabController.SPONSORSHIP_ADDRESS();
+    address _sponsorshipAddress = SPONSORSHIP_ADDRESS;
 
     assertEq(twabController.delegateOf(mockVault, alice), alice);