From e8301cb15966b4e4c45116e23ad2167ec6117070 Mon Sep 17 00:00:00 2001 From: Gabriel Rondon Date: Tue, 24 Mar 2026 01:37:20 +0000 Subject: [PATCH 1/2] Prevent proposals with duplicate actions in GovernorTimelockCompound Override _propose in GovernorTimelockCompound to reject proposals containing duplicate actions (same target, value, and calldata) at creation time. The Compound Timelock identifies queued transactions by hash, so duplicate actions would cause queueing to fail. This gives proposers early feedback instead of a silent failure after the voting period ends. Fixes #6431 --- .../extensions/GovernorTimelockCompound.sol | 33 +++++++++++++++++++ .../GovernorTimelockCompoundMock.sol | 10 ++++++ .../GovernorTimelockCompound.test.js | 31 ++++++++++------- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/contracts/governance/extensions/GovernorTimelockCompound.sol b/contracts/governance/extensions/GovernorTimelockCompound.sol index c3225f19865..86a2f6a6d7d 100644 --- a/contracts/governance/extensions/GovernorTimelockCompound.sol +++ b/contracts/governance/extensions/GovernorTimelockCompound.sol @@ -21,6 +21,12 @@ import {SafeCast} from "../../utils/math/SafeCast.sol"; abstract contract GovernorTimelockCompound is Governor { ICompoundTimelock private _timelock; + /** + * @dev The proposal contains duplicate actions (same target, value, and calldata), which would cause queueing to + * fail in the Compound Timelock. + */ + error GovernorDuplicateProposalAction(uint256 index); + /** * @dev Emitted when the timelock controller used for proposal execution is modified. */ @@ -58,6 +64,33 @@ abstract contract GovernorTimelockCompound is Governor { return true; } + /** + * @dev Override of {Governor-_propose} that rejects proposals containing duplicate actions. The Compound Timelock + * identifies queued transactions by a hash of (target, value, signature, calldata, eta). If two actions in a + * proposal share the same target, value, and calldata, they would produce the same hash, causing the second one + * to fail during queueing. This check prevents such proposals from being created in the first place. + */ + function _propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description, + address proposer + ) internal virtual override returns (uint256) { + for (uint256 i = 1; i < targets.length; ++i) { + for (uint256 j = 0; j < i; ++j) { + if ( + targets[i] == targets[j] && + values[i] == values[j] && + keccak256(calldatas[i]) == keccak256(calldatas[j]) + ) { + revert GovernorDuplicateProposalAction(i); + } + } + } + return super._propose(targets, values, calldatas, description, proposer); + } + /** * @dev Function to queue a proposal to the timelock. */ diff --git a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol index 71508cd5ac0..8cbdc9f3da7 100644 --- a/contracts/mocks/governance/GovernorTimelockCompoundMock.sol +++ b/contracts/mocks/governance/GovernorTimelockCompoundMock.sol @@ -34,6 +34,16 @@ abstract contract GovernorTimelockCompoundMock is return super.proposalNeedsQueuing(proposalId); } + function _propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description, + address proposer + ) internal override(Governor, GovernorTimelockCompound) returns (uint256) { + return super._propose(targets, values, calldatas, description, proposer); + } + function _queueOperations( uint256 proposalId, address[] memory targets, diff --git a/test/governance/extensions/GovernorTimelockCompound.test.js b/test/governance/extensions/GovernorTimelockCompound.test.js index fc8e1e561b2..44b93e50d39 100644 --- a/test/governance/extensions/GovernorTimelockCompound.test.js +++ b/test/governance/extensions/GovernorTimelockCompound.test.js @@ -146,18 +146,27 @@ describe('GovernorTimelockCompound', function () { target: this.token.target, data: this.token.interface.encodeFunctionData('approve', [this.receiver.target, ethers.MaxUint256]), }; - const { id } = this.helper.setProposal([action, action], ''); + this.helper.setProposal([action, action], ''); - await this.helper.propose(); - await this.helper.waitForSnapshot(); - await this.helper.connect(this.voter1).vote({ support: VoteType.For }); - await this.helper.waitForDeadline(); - await expect(this.helper.queue()) - .to.be.revertedWithCustomError(this.mock, 'GovernorAlreadyQueuedProposal') - .withArgs(id); - await expect(this.helper.execute()) - .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState') - .withArgs(id, ProposalState.Succeeded, GovernorHelper.proposalStatesToBitMap([ProposalState.Queued])); + await expect(this.helper.propose()) + .to.be.revertedWithCustomError(this.mock, 'GovernorDuplicateProposalAction') + .withArgs(1n); + }); + + it('if proposal contains non-adjacent duplicate calls', async function () { + const action1 = { + target: this.token.target, + data: this.token.interface.encodeFunctionData('approve', [this.receiver.target, ethers.MaxUint256]), + }; + const action2 = { + target: this.receiver.target, + data: this.receiver.interface.encodeFunctionData('mockFunction'), + }; + this.helper.setProposal([action1, action2, action1], ''); + + await expect(this.helper.propose()) + .to.be.revertedWithCustomError(this.mock, 'GovernorDuplicateProposalAction') + .withArgs(2n); }); }); From 204985992379e4fcd4cc647ec5048b39820e6606 Mon Sep 17 00:00:00 2001 From: Gabriel Rondon Date: Tue, 31 Mar 2026 17:55:56 +0100 Subject: [PATCH 2/2] add changeset for duplicate proposal actions fix --- .changeset/prevent-duplicate-proposal-actions.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/prevent-duplicate-proposal-actions.md diff --git a/.changeset/prevent-duplicate-proposal-actions.md b/.changeset/prevent-duplicate-proposal-actions.md new file mode 100644 index 00000000000..4e007de8994 --- /dev/null +++ b/.changeset/prevent-duplicate-proposal-actions.md @@ -0,0 +1,5 @@ +--- +"openzeppelin-solidity": patch +--- + +Prevent proposals with duplicate actions from being submitted in `GovernorTimelockCompound`