hash proposal is view

OpenZeppelin · Nov 1, 2024 · 8755aa6 · 8755aa6
1 parent 9063080
commit 8755aa6
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 23 deletions.
diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
@@ -110,15 +110,6 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         return "1";
     }
 
-    function _getProposalId(
-        address[] memory targets,
-        uint256[] memory values,
-        bytes[] memory calldatas,
-        bytes32 descriptionHash
-    ) internal virtual returns (uint256) {
-        return hashProposal(targets, values, calldatas, descriptionHash);
-    }
-
     /**
      * @dev See {IGovernor-hashProposal}.
      *
@@ -137,7 +128,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) public pure virtual returns (uint256) {
+    ) public view virtual returns (uint256) {
         return uint256(keccak256(abi.encode(targets, values, calldatas, descriptionHash)));
     }
 
@@ -326,7 +317,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         string memory description,
         address proposer
     ) internal virtual returns (uint256 proposalId) {
-        proposalId = _getProposalId(targets, values, calldatas, keccak256(bytes(description)));
+        proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
 
         if (targets.length != values.length || targets.length != calldatas.length || targets.length == 0) {
             revert GovernorInvalidProposalLength(targets.length, calldatas.length, values.length);
@@ -367,7 +358,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) public virtual returns (uint256) {
-        uint256 proposalId = _getProposalId(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Succeeded));
 
@@ -415,7 +406,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) public payable virtual returns (uint256) {
-        uint256 proposalId = _getProposalId(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(
             proposalId,
@@ -478,7 +469,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         // The proposalId will be recomputed in the `_cancel` call further down. However we need the value before we
         // do the internal call, because we need to check the proposal state BEFORE the internal `_cancel` call
         // changes it. The `hashProposal` duplication has a cost that is limited, and that we accept.
-        uint256 proposalId = _getProposalId(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         // public cancel restrictions (on top of existing _cancel restrictions).
         _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
@@ -501,7 +492,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) internal virtual returns (uint256) {
-        uint256 proposalId = _getProposalId(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(
             proposalId,

diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol
@@ -210,7 +210,7 @@ interface IGovernor is IERC165, IERC6372 {
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) external pure returns (uint256);
+    ) external view returns (uint256);
 
     /**
      * @notice module:core

diff --git a/contracts/governance/extensions/GovernorSequentialProposalId.sol b/contracts/governance/extensions/GovernorSequentialProposalId.sol
@@ -8,15 +8,28 @@ abstract contract GovernorSequentialProposalId is Governor {
     uint256 private _numberOfProposals;
     mapping(uint256 proposalHash => uint256 proposalId) private _proposalIds;
 
-    function _getProposalId(
+    function hashProposal(
         address[] memory targets,
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) internal virtual override returns (uint256) {
-        uint256 proposalHash = super._getProposalId(targets, values, calldatas, descriptionHash);
+    ) public view virtual override returns (uint256) {
+        uint256 proposalHash = super.hashProposal(targets, values, calldatas, descriptionHash);
 
         uint256 storedProposalId = _proposalIds[proposalHash];
-        return storedProposalId == 0 ? (_proposalIds[proposalHash] = ++_numberOfProposals) : storedProposalId;
+        return storedProposalId == 0 ? (_numberOfProposals + 1) : storedProposalId;
+    }
+
+    function _propose(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        string memory description,
+        address proposer
+    ) internal virtual override returns (uint256) {
+        uint256 proposalHash = super.hashProposal(targets, values, calldatas, keccak256(bytes(description)));
+        _proposalIds[proposalHash] = ++_numberOfProposals;
+
+        return super._propose(targets, values, calldatas, description, proposer);
     }
 }
diff --git a/contracts/mocks/governance/GovernorSequentialProposalIdMock.sol b/contracts/mocks/governance/GovernorSequentialProposalIdMock.sol
@@ -18,12 +18,22 @@ abstract contract GovernorSequentialProposalIdMock is
         return super.proposalThreshold();
     }
 
-    function _getProposalId(
+    function hashProposal(
         address[] memory targets,
         uint256[] memory values,
         bytes[] memory calldatas,
         bytes32 descriptionHash
-    ) internal virtual override(Governor, GovernorSequentialProposalId) returns (uint256) {
-        return super._getProposalId(targets, values, calldatas, descriptionHash);
+    ) public view virtual override(Governor, GovernorSequentialProposalId) returns (uint256) {
+        return super.hashProposal(targets, values, calldatas, descriptionHash);
+    }
+
+    function _propose(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        string memory description,
+        address proposer
+    ) internal virtual override(Governor, GovernorSequentialProposalId) returns (uint256 proposalId) {
+        return super._propose(targets, values, calldatas, description, proposer);
     }
 }