create proposal id not enforced (#106)

* create proposal id not enforced

* new changes

contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol

@@ -27,5 +27,5 @@ contract ProposalMock is Proposal {
         bytes memory metadata
     ) external view returns (uint256) {}
 
-    function createProposalParamsABI() external view returns (string memory) {}
+    function customProposalParamsABI() external view returns (string memory) {}
 }

contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol

@@ -27,5 +27,5 @@ contract ProposalUpgradeableMock is ProposalUpgradeable {
         bytes memory metadata
     ) external view returns (uint256) {}
 
-    function createProposalParamsABI() external view returns (string memory) {}
+    function customProposalParamsABI() external view returns (string memory) {}
 }

contracts/src/plugin/extensions/proposal/IProposal.sol

@@ -51,23 +51,14 @@ interface IProposal {
     /// @return bool Returns if proposal can be executed or not.
     function canExecute(uint256 proposalId) external view returns (bool);
 
-    /// @notice Creates a proposal Id.
-    /// @param actions The actions that will be executed after the proposal passes.
-    /// @param metadata The custom metadata that is passed when creating a proposal.
-    /// @return proposalId The id of the proposal.
-    function createProposalId(
-        Action[] memory actions,
-        bytes memory metadata
-    ) external view returns (uint256);
-
     /// @notice The human-readable abi format for extra params included in `data` of `createProposal`.
     /// @dev Used for UI to easily detect what extra params the contract expects.
     /// @return abi ABI of params in `data` of `createProposal`.
-    function createProposalParamsABI() external view returns (string memory abi);
+    function customProposalParamsABI() external view returns (string memory abi);
 
     /// @notice Returns the proposal count determining the next proposal ID.
-    /// @dev This function has been deprecated but due to backwards compatibility, it still stays in the interface
-    /// but returns maximum value of uint256 to let consumers know not to depend on it anymore.
+    /// @dev This function has been deprecated but due to backwards compatibility, it still exists
+    /// in the interface but reverts to avoid ambiguity.
     /// @return The proposal count.
     function proposalCount() external view returns (uint256);
 }

contracts/src/plugin/extensions/proposal/Proposal.sol

@@ -11,9 +11,19 @@ import {IProposal} from "./IProposal.sol";
 /// @notice An abstract contract containing the traits and internal functionality to create and execute proposals that can be inherited by non-upgradeable DAO plugins.
 /// @custom:security-contact [email protected]
 abstract contract Proposal is IProposal, ERC165 {
+    error FunctionDeprecated();
+
     /// @inheritdoc IProposal
-    function proposalCount() public pure override returns (uint256) {
-        return type(uint256).max;
+    function proposalCount() public view virtual override returns (uint256) {
+        revert FunctionDeprecated();
+    }
+
+    /// @notice Creates a proposal Id.
+    /// @dev Uses block number and chain id to ensure more probability of uniqueness.
+    /// @param salt The extra salt to help with uniqueness.
+    /// @return The id of the proposal.
+    function _createProposalId(bytes32 salt) internal view virtual returns (uint256) {
+        return uint256(keccak256(abi.encode(block.chainid, block.number, address(this), salt)));
     }
 
     /// @notice Checks if this or the parent contract supports an interface by its ID.
@@ -22,14 +32,13 @@ abstract contract Proposal is IProposal, ERC165 {
     function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
         // In addition to the current interfaceId, also support previous version of the interfaceId
         // that did not include the following functions:
-        // `createProposal, canExecute, createProposalId, createProposalParamsABI`.
+        // `createProposal, canExecute, customProposalParamsABI`.
         return
             _interfaceId ==
             type(IProposal).interfaceId ^
                 IProposal.createProposal.selector ^
                 IProposal.canExecute.selector ^
-                IProposal.createProposalId.selector ^
-                IProposal.createProposalParamsABI.selector ||
+                IProposal.customProposalParamsABI.selector ||
             _interfaceId == type(IProposal).interfaceId ||
             super.supportsInterface(_interfaceId);
     }

contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol

@@ -14,12 +14,22 @@ import {IProposal} from "./IProposal.sol";
 abstract contract ProposalUpgradeable is IProposal, ERC165Upgradeable {
     using CountersUpgradeable for CountersUpgradeable.Counter;
 
+    error FunctionDeprecated();
+
     /// @notice The incremental ID for proposals and executions.
     CountersUpgradeable.Counter private proposalCounter;
 
     /// @inheritdoc IProposal
-    function proposalCount() public pure override returns (uint256) {
-        return type(uint256).max;
+    function proposalCount() public view virtual override returns (uint256) {
+        revert FunctionDeprecated();
+    }
+
+    /// @notice Creates a proposal Id.
+    /// @dev Uses block number and chain id to ensure more probability of uniqueness.
+    /// @param salt The extra salt to help with uniqueness.
+    /// @return The id of the proposal.
+    function _createProposalId(bytes32 salt) internal view virtual returns (uint256) {
+        return uint256(keccak256(abi.encode(block.chainid, block.number, address(this), salt)));
     }
 
     /// @notice Checks if this or the parent contract supports an interface by its ID.
@@ -28,14 +38,13 @@ abstract contract ProposalUpgradeable is IProposal, ERC165Upgradeable {
     function supportsInterface(bytes4 _interfaceId) public view virtual override returns (bool) {
         // In addition to the current interfaceId, also support previous version of the interfaceId
         // that did not include the following functions:
-        // `createProposal, canExecute, createProposalId, createProposalParamsABI`.
+        // `createProposal, canExecute, customProposalParamsABI`.
         return
             _interfaceId ==
             type(IProposal).interfaceId ^
                 IProposal.createProposal.selector ^
                 IProposal.canExecute.selector ^
-                IProposal.createProposalId.selector ^
-                IProposal.createProposalParamsABI.selector ||
+                IProposal.customProposalParamsABI.selector ||
             _interfaceId == type(IProposal).interfaceId ||
             super.supportsInterface(_interfaceId);
     }

contracts/test/plugin/extensions/proposal.ts

@@ -27,9 +27,7 @@ function proposalBaseTests(fixture: () => Promise<FixtureResult>) {
   it('returns max value of integer for backwards-compatibility', async () => {
     const {proposalMock} = await loadFixture(fixture);
 
-    expect(await proposalMock.proposalCount()).to.equal(
-      ethers.constants.MaxUint256
-    );
+    await expect(proposalMock.proposalCount()).to.be.reverted;
   });
 
   describe('ERC-165', async () => {