Fix to set next proposal id correctly (M-01) (#87)

* Fix to initialize next proposal ID to be GovernorBravo proposal count + 1 * Added explanatory comment to setNextProposalId
compound-finance · Dec 16, 2024 · cf39115 · cf39115
1 parent 20f51ed
commit cf39115
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 3 deletions.
diff --git a/contracts/CompoundGovernor.sol b/contracts/CompoundGovernor.sol
@@ -124,7 +124,11 @@ contract CompoundGovernor is
         if (_executor() != _msgSender()) {
             revert GovernorOnlyExecutor(_msgSender());
         }
-        _setNextProposalId(compoundGovernorBravo.proposalCount());
+
+        // In GovernorBravo, proposal IDs start at 1, so its proposalCount() function is the most recent 
+        // proposal ID created. This function sets the first proposal ID for the CompoundGovernor to 1 beyond that,
+        // so the first proposal ID of compoundGovernor is one more that the last proposal ID of GovernorBravo.
+        _setNextProposalId(compoundGovernorBravo.proposalCount() + 1);
     }
 
     /// @notice A modified `hashProposal` that supports sequential proposal IDs.

diff --git a/contracts/test/GovernorSequentialProposalIdUpgradeable.t.sol b/contracts/test/GovernorSequentialProposalIdUpgradeable.t.sol
@@ -30,7 +30,7 @@ contract GovernorSequentialProposalIdUpgradeableTest is CompoundGovernorTest {
 
 contract ProposalCount is GovernorSequentialProposalIdUpgradeableTest {
     function test_ReturnsCorrectProposalCount() public {
-        assertEq(governor.proposalCount(), compoundGovernorBravo.proposalCount());
+        assertEq(governor.proposalCount(), compoundGovernorBravo.proposalCount() + 1);
     }
 
     function testFuzz_ProposalCreatedEventEmittedWithEnumeratedProposalId(uint256 _newValue) public {
@@ -71,7 +71,7 @@ contract ProposalCount is GovernorSequentialProposalIdUpgradeableTest {
 contract GetNextProposalId is GovernorSequentialProposalIdUpgradeableTest {
     function testFuzz_ReturnsCorrectNextProposalId(uint256) public {
         uint256 _nextProposalId = governor.getNextProposalId();
-        assertEq(_nextProposalId, compoundGovernorBravo.proposalCount());
+        assertEq(_nextProposalId, compoundGovernorBravo.proposalCount() + 1);
         assertTrue(_nextProposalId > 0);
     }
 }