Merge branch 'sc-rotation' into sc-rotation-upgrade

.gas-snapshot

@@ -26,7 +26,7 @@ ArbitrumVestingWalletTest:testDelegateFailsForNonBeneficiary() (gas: 16008435)
 ArbitrumVestingWalletTest:testDoesDeploy() (gas: 15971342)
 ArbitrumVestingWalletTest:testReleaseAffordance() (gas: 16008649)
 ArbitrumVestingWalletTest:testVestedAmountStart() (gas: 16074917)
-E2E:testE2E() (gas: 85825323)
+E2E:testE2E() (gas: 85848560)
 FixedDelegateErc20WalletTest:testInit() (gas: 5822575)
 FixedDelegateErc20WalletTest:testInitZeroToken() (gas: 5816805)
 FixedDelegateErc20WalletTest:testTransfer() (gas: 5932218)
@@ -94,14 +94,14 @@ L2GovernanceFactoryTest:testSanityCheckValues() (gas: 28415658)
 L2GovernanceFactoryTest:testSetMinDelay() (gas: 28364371)
 L2GovernanceFactoryTest:testSetMinDelayRevertsForCoreAddress() (gas: 28417242)
 L2GovernanceFactoryTest:testUpgraderCanCancel() (gas: 28657360)
-L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 31669215)
-L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 31673446)
-L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26712481)
-L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 31671446)
-L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 31692326)
+L2SecurityCouncilMgmtFactoryTest:testMemberElectionGovDeployment() (gas: 31692452)
+L2SecurityCouncilMgmtFactoryTest:testNomineeElectionGovDeployment() (gas: 31696683)
+L2SecurityCouncilMgmtFactoryTest:testOnlyOwnerCanDeploy() (gas: 26735718)
+L2SecurityCouncilMgmtFactoryTest:testRemovalGovDeployment() (gas: 31694683)
+L2SecurityCouncilMgmtFactoryTest:testSecurityCouncilManagerDeployment() (gas: 31715563)
 NomineeGovernorV2UpgradeActionTest:testAction() (gas: 8153)
 OfficeHoursActionTest:testConstructor() (gas: 9050)
-OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317072, ~: 317184)
+OfficeHoursActionTest:testFuzzOfficeHoursDeployment(uint256,uint256,int256,uint256,uint256,uint256) (runs: 256, μ: 317070, ~: 317184)
 OfficeHoursActionTest:testInvalidConstructorParameters() (gas: 235740)
 OfficeHoursActionTest:testPerformBeforeMinimumTimestamp() (gas: 8646)
 OfficeHoursActionTest:testPerformDuringOfficeHours() (gas: 9140)
@@ -125,7 +125,7 @@ SecurityCouncilManagerTest:testAddSCAffordances() (gas: 112133)
 SecurityCouncilManagerTest:testCantUpdateCohortWithADup() (gas: 123130)
 SecurityCouncilManagerTest:testCohortMethods() (gas: 136182)
 SecurityCouncilManagerTest:testInitialization() (gas: 209054)
-SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5162749)
+SecurityCouncilManagerTest:testPostUpgradeInit() (gas: 5191924)
 SecurityCouncilManagerTest:testRemoveMember() (gas: 213164)
 SecurityCouncilManagerTest:testRemoveMemberAffordances() (gas: 99124)
 SecurityCouncilManagerTest:testRemoveSCAffordances() (gas: 81287)
@@ -156,7 +156,7 @@ SecurityCouncilMemberElectionGovernorTest:testOnlyNomineeElectionGovernorCanProp
 SecurityCouncilMemberElectionGovernorTest:testProperInitialization() (gas: 49388)
 SecurityCouncilMemberElectionGovernorTest:testProposeReverts() (gas: 32916)
 SecurityCouncilMemberElectionGovernorTest:testRelay() (gas: 42229)
-SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 340091, ~: 339846)
+SecurityCouncilMemberElectionGovernorTest:testSelectTopNominees(uint256) (runs: 256, μ: 340178, ~: 340008)
 SecurityCouncilMemberElectionGovernorTest:testSelectTopNomineesFails() (gas: 273335)
 SecurityCouncilMemberElectionGovernorTest:testSetFullWeightDuration() (gas: 34951)
 SecurityCouncilMemberElectionGovernorTest:testVotesToWeight() (gas: 152898)

src/security-council-mgmt/SecurityCouncilManager.sol

@@ -159,7 +159,7 @@ contract SecurityCouncilManager is
             _addSecurityCouncil(_securityCouncils[i]);
         }
 
-        setMinRotationPeriodImpl(_minRotationPeriod);
+        _setMinRotationPeriod(_minRotationPeriod);
 
         // we do our own 712 functionality because inheriting the OZ version
         // would change our storage layout
@@ -170,11 +170,11 @@ contract SecurityCouncilManager is
     function postUpgradeInit(uint256 _minRotationPeriod, address minRotationPeriodSetter)
         external
     {
-        address proxyAdmin = ProxyUtil.getProxyAdmin();
-        require(msg.sender == proxyAdmin, "NOT_FROM_ADMIN");
+        require(msg.sender == ProxyUtil.getProxyAdmin(), "NOT_FROM_ADMIN");
+        require(minRotationPeriod == 0, "MIN_ROTATION_ALREADY_SET");
 
         _grantRole(MIN_ROTATION_PERIOD_SETTER_ROLE, minRotationPeriodSetter);
-        setMinRotationPeriodImpl(_minRotationPeriod);
+        _setMinRotationPeriod(_minRotationPeriod);
 
         NAME_HASH = keccak256(bytes("SecurityCouncilManager"));
         VERSION_HASH = keccak256(bytes("1"));
@@ -191,10 +191,10 @@ contract SecurityCouncilManager is
         external
         onlyRole(MIN_ROTATION_PERIOD_SETTER_ROLE)
     {
-        setMinRotationPeriodImpl(_minRotationPeriod);
+        _setMinRotationPeriod(_minRotationPeriod);
     }
 
-    function setMinRotationPeriodImpl(uint256 _minRotationPeriod) internal {
+    function _setMinRotationPeriod(uint256 _minRotationPeriod) internal {
         minRotationPeriod = _minRotationPeriod;
         emit MinRotationPeriodSet(_minRotationPeriod);
     }

test/security-council-mgmt/SecurityCouncilManager.t.sol

@@ -631,6 +631,12 @@ contract SecurityCouncilManagerTest is Test {
         );
         assertEq(s.NAME_HASH(), keccak256(bytes("SecurityCouncilManager")));
         assertEq(s.VERSION_HASH(), keccak256(bytes("1")));
+
+        vm.expectRevert("MIN_ROTATION_ALREADY_SET");
+        vm.prank(address(pa));
+        TransparentUpgradeableProxy(payable(address(s))).upgradeToAndCall(
+            address(logic), abi.encodeCall(s.postUpgradeInit, (mr, mrs))
+        );
     }
 
     function testSetMinRotationPeriod() public {