diff --git a/contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol b/contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol index 1bf07368..f4517cd8 100644 --- a/contracts/src/mocks/plugin/extensions/proposal/ProposalMock.sol +++ b/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) {} } diff --git a/contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol b/contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol index ff06644a..b4122d6f 100644 --- a/contracts/src/mocks/plugin/extensions/proposal/ProposalUpgradeableMock.sol +++ b/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) {} } diff --git a/contracts/src/plugin/extensions/proposal/IProposal.sol b/contracts/src/plugin/extensions/proposal/IProposal.sol index f921eb76..9d775281 100644 --- a/contracts/src/plugin/extensions/proposal/IProposal.sol +++ b/contracts/src/plugin/extensions/proposal/IProposal.sol @@ -54,11 +54,11 @@ interface IProposal { /// @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); } diff --git a/contracts/src/plugin/extensions/proposal/Proposal.sol b/contracts/src/plugin/extensions/proposal/Proposal.sol index 99425050..93aed44f 100644 --- a/contracts/src/plugin/extensions/proposal/Proposal.sol +++ b/contracts/src/plugin/extensions/proposal/Proposal.sol @@ -11,17 +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 sirt@aragon.org 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 timestamp and chain id to ensure more probability of uniqueness. + /// @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.timestamp, address(this), salt))); + 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. @@ -30,13 +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, createProposalParamsABI`. + // `createProposal, canExecute, customProposalParamsABI`. return _interfaceId == type(IProposal).interfaceId ^ IProposal.createProposal.selector ^ IProposal.canExecute.selector ^ - IProposal.createProposalParamsABI.selector || + IProposal.customProposalParamsABI.selector || _interfaceId == type(IProposal).interfaceId || super.supportsInterface(_interfaceId); } diff --git a/contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol b/contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol index 1bd5f5b1..37cbdae8 100644 --- a/contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol +++ b/contracts/src/plugin/extensions/proposal/ProposalUpgradeable.sol @@ -14,20 +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 timestamp and chain id to ensure more probability of uniqueness. + /// @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.timestamp, address(this), salt))); + 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. @@ -36,13 +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, createProposalParamsABI`. + // `createProposal, canExecute, customProposalParamsABI`. return _interfaceId == type(IProposal).interfaceId ^ IProposal.createProposal.selector ^ IProposal.canExecute.selector ^ - IProposal.createProposalParamsABI.selector || + IProposal.customProposalParamsABI.selector || _interfaceId == type(IProposal).interfaceId || super.supportsInterface(_interfaceId); } diff --git a/contracts/test/plugin/extensions/proposal.ts b/contracts/test/plugin/extensions/proposal.ts index 4bea8272..3f833634 100644 --- a/contracts/test/plugin/extensions/proposal.ts +++ b/contracts/test/plugin/extensions/proposal.ts @@ -27,9 +27,7 @@ function proposalBaseTests(fixture: () => Promise) { 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 () => {