Rename lastElectionId -> currentElectionId (#1780)

* rename lastElectionId to currentElectionId

* fix testable storage generation

protocol/governance/contracts/modules/core/BaseElectionModule.sol

@@ -160,11 +160,11 @@ contract BaseElectionModule is
 
         Council.Data storage store = Council.load();
 
-        uint epochIndex = store.lastElectionId;
+        uint electionId = store.currentElectionId;
 
-        _removeCouncilMembers(membersToDismiss, epochIndex);
+        _removeCouncilMembers(membersToDismiss, electionId);
 
-        emit CouncilMembersDismissed(membersToDismiss, epochIndex);
+        emit CouncilMembersDismissed(membersToDismiss, electionId);
 
         if (store.getCurrentPeriod() != Council.ElectionPeriod.Administration) return;
 
@@ -177,7 +177,7 @@ contract BaseElectionModule is
 
         store.jumpToNominationPeriod();
 
-        emit EmergencyElectionStarted(epochIndex);
+        emit EmergencyElectionStarted(electionId);
     }
 
     function nominate() public virtual override {
@@ -188,7 +188,7 @@ contract BaseElectionModule is
 
         nominees.add(msg.sender);
 
-        emit CandidateNominated(msg.sender, Council.load().lastElectionId);
+        emit CandidateNominated(msg.sender, Council.load().currentElectionId);
 
         // TODO: add ballot id to emitted event
     }
@@ -201,7 +201,7 @@ contract BaseElectionModule is
 
         nominees.remove(msg.sender);
 
-        emit NominationWithdrawn(msg.sender, Council.load().lastElectionId);
+        emit NominationWithdrawn(msg.sender, Council.load().currentElectionId);
     }
 
     /// @dev ElectionVotes needs to be extended to specify what determines voting power
@@ -216,7 +216,7 @@ contract BaseElectionModule is
         }
 
         Ballot.Data storage ballot = Ballot.load(
-            Council.load().lastElectionId,
+            Council.load().currentElectionId,
             msg.sender,
             block.chainid
         );
@@ -255,9 +255,9 @@ contract BaseElectionModule is
 
         Council.Data storage council = Council.load();
         Election.Data storage election = council.getCurrentElection();
-        uint256 lastElectionId = council.lastElectionId;
+        uint256 currentElectionId = council.currentElectionId;
 
-        Ballot.Data storage storedBallot = Ballot.load(lastElectionId, voter, precinct);
+        Ballot.Data storage storedBallot = Ballot.load(currentElectionId, voter, precinct);
 
         storedBallot.copy(ballot);
         storedBallot.validate();
@@ -269,7 +269,7 @@ contract BaseElectionModule is
 
         election.ballotPtrs.push(ballotPtr);
 
-        emit VoteRecorded(msg.sender, precinct, lastElectionId, ballot.votingPower);
+        emit VoteRecorded(msg.sender, precinct, currentElectionId, ballot.votingPower);
     }
 
     /// @dev ElectionTally needs to be extended to specify how votes are counted
@@ -282,7 +282,7 @@ contract BaseElectionModule is
 
         _evaluateNextBallotBatch(numBallots);
 
-        uint currentEpochIndex = Council.load().lastElectionId;
+        uint currentEpochIndex = Council.load().currentElectionId;
 
         uint totalBallots = election.ballotPtrs.length;
         if (election.numEvaluatedBallots < totalBallots) {
@@ -306,7 +306,7 @@ contract BaseElectionModule is
 
         if (!election.evaluated) revert ElectionNotEvaluated();
 
-        uint newEpochIndex = store.lastElectionId + 1;
+        uint newEpochIndex = store.currentElectionId + 1;
 
         _removeAllCouncilMembers(newEpochIndex);
         _addCouncilMembers(election.winners.values(), newEpochIndex);
@@ -343,7 +343,7 @@ contract BaseElectionModule is
     }
 
     function getEpochIndex() external view override returns (uint) {
-        return Council.load().lastElectionId;
+        return Council.load().currentElectionId;
     }
 
     function getCurrentPeriod() external view override returns (uint) {
@@ -361,7 +361,7 @@ contract BaseElectionModule is
 
     function hasVoted(address user, uint256 precinct) public view override returns (bool) {
         Council.Data storage council = Council.load();
-        Ballot.Data storage ballot = Ballot.load(council.lastElectionId, user, precinct);
+        Ballot.Data storage ballot = Ballot.load(council.currentElectionId, user, precinct);
         return ballot.votingPower > 0 && ballot.votedCandidates.length > 0;
     }
 

protocol/governance/contracts/modules/core/SnapshotVotePowerModule.sol

@@ -19,10 +19,10 @@ contract SnapshotVotePowerModule is ISnapshotVotePowerModule {
         Council.Data storage council = Council.load();
         SnapshotVotePower.Data storage snapshotVotePower = SnapshotVotePower.load(snapshotContract);
         if (enabled) {
-            snapshotVotePower.validFromEpoch = (council.lastElectionId + 1).to128();
+            snapshotVotePower.validFromEpoch = (council.currentElectionId + 1).to128();
             snapshotVotePower.validToEpoch = 0;
         } else {
-            snapshotVotePower.validToEpoch = (council.lastElectionId + 1).to128();
+            snapshotVotePower.validToEpoch = (council.currentElectionId + 1).to128();
         }
     }
 
@@ -33,7 +33,7 @@ contract SnapshotVotePowerModule is ISnapshotVotePowerModule {
         Council.onlyInPeriod(Council.ElectionPeriod.Nomination);
         SnapshotVotePowerEpoch.Data storage snapshotVotePowerEpoch = SnapshotVotePower
             .load(snapshotContract)
-            .epochs[Council.load().lastElectionId.to128()];
+            .epochs[Council.load().currentElectionId.to128()];
         if (snapshotVotePowerEpoch.snapshotId > 0) {
             revert SnapshotAlreadyTaken(snapshotVotePowerEpoch.snapshotId);
         }
@@ -67,7 +67,7 @@ contract SnapshotVotePowerModule is ISnapshotVotePowerModule {
     ) external override returns (uint256 power) {
         Council.Data storage council = Council.load();
         Council.onlyInPeriod(Council.ElectionPeriod.Vote);
-        uint128 currentEpoch = council.lastElectionId.to128();
+        uint128 currentEpoch = council.currentElectionId.to128();
         SnapshotVotePower.Data storage snapshotVotePower = SnapshotVotePower.load(snapshotContract);
 
         if (snapshotVotePower.epochs[currentEpoch].snapshotId == 0) {

protocol/governance/contracts/storage/Council.sol

@@ -24,8 +24,8 @@ library Council {
         SetUtil.AddressSet councilMembers;
         // Council token id's by council member address
         mapping(address => uint) councilTokenIds;
-        // id of the last election
-        uint lastElectionId;
+        // id of the current epoch
+        uint currentElectionId;
     }
 
     enum ElectionPeriod {
@@ -48,40 +48,40 @@ library Council {
 
     function newElection(Data storage self) internal returns (uint newElectionId) {
         getNextElectionSettings(self).copyMissingFrom(getCurrentElectionSettings(self));
-        newElectionId = ++self.lastElectionId;
+        newElectionId = ++self.currentElectionId;
         initScheduleFromSettings(self);
     }
 
     function getCurrentElection(
         Data storage self
     ) internal view returns (Election.Data storage election) {
-        return Election.load(self.lastElectionId);
+        return Election.load(self.currentElectionId);
     }
 
     function getPreviousElection(
         Data storage self
     ) internal view returns (Election.Data storage election) {
         // NOTE: will revert if there was no previous election
-        return Election.load(self.lastElectionId - 1);
+        return Election.load(self.currentElectionId - 1);
     }
 
     function getCurrentElectionSettings(
         Data storage self
     ) internal view returns (ElectionSettings.Data storage settings) {
-        return ElectionSettings.load(self.lastElectionId);
+        return ElectionSettings.load(self.currentElectionId);
     }
 
     function getPreviousElectionSettings(
         Data storage self
     ) internal view returns (ElectionSettings.Data storage settings) {
         // NOTE: will revert if there was no previous settings
-        return ElectionSettings.load(self.lastElectionId - 1);
+        return ElectionSettings.load(self.currentElectionId - 1);
     }
 
     function getNextElectionSettings(
         Data storage self
     ) internal view returns (ElectionSettings.Data storage settings) {
-        return ElectionSettings.load(self.lastElectionId + 1);
+        return ElectionSettings.load(self.currentElectionId + 1);
     }
 
     /// @dev Determines the current period type according to the current time and the epoch's dates

protocol/governance/package.json

@@ -4,7 +4,7 @@
   "description": "On-Chain elections for all Synthetix councils",
   "private": true,
   "scripts": {
-    "clean": "hardhat clean && rm -rf test/generated",
+    "clean": "hardhat clean && rm -rf test/generated contracts/generated",
     "build": "yarn clean && hardhat storage:verify && hardhat generate-testable && hardhat cannon:build",
     "build-testable": "yarn build cannonfile.test.toml",
     "check:storage": "git diff --exit-code storage.dump.sol",

protocol/governance/storage.dump.sol

@@ -216,7 +216,7 @@ library Council {
         address councilToken;
         SetUtil.AddressSet councilMembers;
         mapping(address => uint) councilTokenIds;
-        uint lastElectionId;
+        uint currentElectionId;
     }
     function load() internal pure returns (Data storage store) {
         bytes32 s = _SLOT_COUNCIL_STORAGE;

protocol/governance/test/contracts/BaseElectionModule/Voting.test.ts

@@ -18,7 +18,7 @@ describe('ElectionModule - voting', () => {
 
   before('create voting power for user', async () => {
     await c.CoreProxy.Ballot_set_votingPower(
-      await c.CoreProxy.Council_get_lastElectionId(),
+      await c.CoreProxy.Council_get_currentElectionId(),
       await user.getAddress(),
       13370,
       100

protocol/governance/test/contracts/SnapshotVotePowerModule.test.ts

@@ -26,6 +26,7 @@ describe('SnapshotVotePowerModule', function () {
         c.CoreProxy
       );
     });
+
     it('should set snapshot contract', async function () {
       await c.CoreProxy.setSnapshotContract(c.SnapshotRecordMock.address, true);
       // shouldn't be valid for current epoch
@@ -41,7 +42,7 @@ describe('SnapshotVotePowerModule', function () {
     });
 
     it('should allow for snapshot contracts to be removed on the next epoch', async function () {
-      await c.CoreProxy.Council_set_lastElectionId(0);
+      await c.CoreProxy.Council_set_currentElectionId(0);
       await c.CoreProxy.setSnapshotContract(c.SnapshotRecordMock.address, true);
       await c.CoreProxy.Council_newElection();
       await c.CoreProxy.setSnapshotContract(c.SnapshotRecordMock.address, false);
@@ -83,15 +84,15 @@ describe('SnapshotVotePowerModule', function () {
         assertBn.equal(
           await c.CoreProxy.getVotePowerSnapshotId(
             c.SnapshotRecordMock.address,
-            await c.CoreProxy.Council_get_lastElectionId()
+            await c.CoreProxy.Council_get_currentElectionId()
           ),
           0
         );
         await c.CoreProxy.takeVotePowerSnapshot(c.SnapshotRecordMock.address);
         assertBn.gt(
           await c.CoreProxy.getVotePowerSnapshotId(
             c.SnapshotRecordMock.address,
-            await c.CoreProxy.Council_get_lastElectionId()
+            await c.CoreProxy.Council_get_currentElectionId()
           ),
           0
         );
@@ -118,7 +119,7 @@ describe('SnapshotVotePowerModule', function () {
 
       const snapshotId = await c.CoreProxy.getVotePowerSnapshotId(
         c.SnapshotRecordMock.address,
-        await c.CoreProxy.Council_get_lastElectionId()
+        await c.CoreProxy.Council_get_currentElectionId()
       );
 
       await c.SnapshotRecordMock.setBalanceOfOnPeriod(await user.getAddress(), 100, snapshotId);
@@ -156,7 +157,7 @@ describe('SnapshotVotePowerModule', function () {
         assertBn.equal(foundVotingPower, 100);
 
         const ballotVotingPower = await c.CoreProxy.Ballot_get_votingPower(
-          await c.CoreProxy.Council_get_lastElectionId(),
+          await c.CoreProxy.Council_get_currentElectionId(),
           await user.getAddress(),
           13370 // precinct is current chain id
         );

protocol/governance/test/contracts/UpgradeProposalModule.test.ts

@@ -1,6 +1,9 @@
+import assert from 'node:assert';
 import assertRevert from '@synthetixio/core-utils/utils/assertions/assert-revert';
 import { ethers } from 'ethers';
+import hre from 'hardhat';
 import { bootstrap } from '../bootstrap';
+import { InitialModuleBundle } from '../generated/typechain';
 
 describe('UpgradeProposalModule', function () {
   const { c, getSigners, snapshotCheckpoint } = bootstrap();
@@ -32,20 +35,24 @@ describe('UpgradeProposalModule', function () {
       const current = await c.CoreProxy.getImplementation();
       await assertRevert(c.CoreProxy.upgradeTo(current), 'NoChange');
     });
-  });
 
-  // describe('#simulateUpgradeTo', function () {
-  //   let NewImplementation: InitialModuleBundle;
+    describe('when proposing a valid implementation', function () {
+      let NewImplementation: InitialModuleBundle;
+
+      snapshotCheckpoint();
 
-  //   snapshotCheckpoint();
+      before('create new implementation', async function () {
+        const factory = await hre.ethers.getContractFactory('InitialModuleBundle', owner);
+        NewImplementation = await factory.deploy();
+      });
 
-  //   before('create new implementation', async function () {
-  //     const factory = await hre.ethers.getContractFactory('InitialModuleBundle', owner);
-  //     NewImplementation = await factory.deploy();
-  //   });
+      before('call to upgradeTo', async function () {
+        await c.CoreProxy.upgradeTo(NewImplementation.address);
+      });
 
-  //   it('reverts', async function () {
-  //     await c.CoreProxy.simulateUpgradeTo(NewImplementation.address);
-  //   });
-  // });
+      it('saves the value', async function () {
+        assert.equal(await c.CoreProxy.getProposedImplementation(), NewImplementation.address);
+      });
+    });
+  });
 });

utils/hardhat-storage/templates/TestableStorage.sol.mustache

@@ -11,56 +11,56 @@ pragma solidity ^0.8.0;
 import "{{{relativeSourceName}}}";
 
 contract Testable{{{libraryName}}}Storage {
-	{{#supportedLoadFunction}}
+	{{#loadParams}}
     function _getInstanceStore({{{loadParams}}}) internal pure returns ({{{libraryName}}}.Data storage) {
         return {{{libraryName}}}.load({{{loadInject}}});
     }
-    {{/supportedLoadFunction}}
-    {{^supportedLoadFunction}}
+    {{/loadParams}}
+    {{^loadParams}}
     function _getInstanceStore() internal pure returns ({{{libraryName}}}.Data storage data) {
         bytes32 s = keccak256(abi.encode("{{{libraryName}}}"));
         assembly {
             data.slot := s
         }
     }
-    {{/supportedLoadFunction}}
+    {{/loadParams}}
 
     {{#fields}}
     function {{{libraryName}}}_set_{{{name}}}({{#loadParams}}{{{loadParams}}}, {{/loadParams}}{{{type}}} val) external {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         store.{{{name}}} = val;
     }
 
     function {{{libraryName}}}_get_{{{name}}}({{{loadParams}}}) external view returns ({{type}}) {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         return store.{{{name}}};
     }
 
     {{/fields}}
     {{#indexedFields}}
     function {{{libraryName}}}_set_{{{name}}}({{#loadParams}}{{{loadParams}}}, {{/loadParams}}{{{indexType}}} idx, {{{type}}} val) external {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         store.{{{name}}}[idx] = val;
     }
 
     function {{{libraryName}}}_get_{{{name}}}({{#loadParams}}{{{loadParams}}}, {{/loadParams}}{{{indexType}}} idx) external view returns ({{type}}) {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         return store.{{{name}}}[idx];
     }
 
     {{#isArray}}
     function {{{libraryName}}}_push_{{{name}}}({{#loadParams}}{{{loadParams}}}, {{/loadParams}}{{{type}}} val) external {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         store.{{{name}}}.push(val);
     }
 
     function {{{libraryName}}}_pop_{{{name}}}({{{loadParams}}}) external {
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         store.{{{name}}}.pop();
     }
@@ -69,7 +69,7 @@ contract Testable{{{libraryName}}}Storage {
     {{/indexedFields}}
     {{#methods}}
     function {{{libraryName}}}_{{{name}}}({{#loadParams}}{{{loadParams}}}{{#params}}, {{/params}}{{/loadParams}}{{{params}}}) external{{#mutability}} {{{mutability}}}{{/mutability}} {{#returns}}returns ({{{returns}}}) {{/returns}}{
-        {{{libraryName}}}.Data storage store = _getInstanceStore({{#supportedLoadFunction}}{{{loadInject}}}{{/supportedLoadFunction}});
+        {{{libraryName}}}.Data storage store = _getInstanceStore({{#loadParams}}{{{loadInject}}}{{/loadParams}});
 
         return {{{libraryName}}}.{{{name}}}({{{paramsInject}}});
     }